diff --git a/DEPLOY.md b/DEPLOY.md index ee3c29b0..d164a1fc 100644 --- a/DEPLOY.md +++ b/DEPLOY.md @@ -284,13 +284,9 @@ Configure these rules in the Cloudflare dashboard under **Security → WAF**. **Where:** Security → WAF → Custom Rules **Action:** Block -**Previous expression (pre-`/_app/` rollout, synthesized from `scripts/waf-smoke-test.sh`):** +**Previous expression (pre-`/_app/` rollout):** ``` -not ( - http.request.uri.path in {"/" "/login" "/oauth/callback" "/onboarding" "/dashboard" "/settings" "/privacy" "/index.html"} or - starts_with(http.request.uri.path, "/assets/") or - starts_with(http.request.uri.path, "/api/") -) +not (http.request.uri.path eq "/" or http.request.uri.path eq "/index.html" or http.request.uri.path eq "/login" or http.request.uri.path eq "/oauth/callback" or http.request.uri.path eq "/onboarding" or http.request.uri.path eq "/dashboard" or http.request.uri.path eq "/settings" or http.request.uri.path eq "/privacy" or starts_with(http.request.uri.path, "/assets/") or starts_with(http.request.uri.path, "/api/")) ``` **Updated expression (allows `/_app/*` hashed chunks and static/metadata routes on `GET`/`HEAD`):** @@ -311,6 +307,20 @@ not ( - Blocks vulnerability scanners and probes for unmapped paths (`/wp-admin`, `/.env`, `/.git/config`, arbitrary paths) at the edge before the Worker or static asset router runs. - `scripts/waf-smoke-test.sh` (`pnpm test:waf`) validates this rule before every commit (`prek`) and deploy (`deploy.yml`). +#### Rule 2: Scanner User-Agents + +**Rule name:** Scanner User-Agents (block empty and malicious User-Agents) +**Where:** Security → WAF → Custom Rules +**Expression:** +``` +(http.user_agent eq "" or lower(http.user_agent) contains "sqlmap" or lower(http.user_agent) contains "nikto" or lower(http.user_agent) contains "nmap" or lower(http.user_agent) contains "masscan" or lower(http.user_agent) contains "zgrab") +``` +**Action:** Block + +**Notes:** +- Blocks automated vulnerability scanners (`sqlmap`, `nikto`, `nmap`, `masscan`, `zgrab`) and requests with an empty `User-Agent` header at the edge before the Worker runs. +- Validated by `scripts/waf-smoke-test.sh` (`pnpm test:waf`). + #### Rule 3: Origin Gate **Rule name:** Block API requests without valid Origin @@ -327,12 +337,12 @@ not (http.request.uri.path eq "/api/error-reporting") **Exemptions:** `/api/csp-report` and `/api/error-reporting` are excluded because the Worker enforces its own strict origin check on both endpoints. See `hack/docs/security-runbook.md` for exemption rationale. **Notes:** -- This uses **1 of the 5 free WAF custom rules** available on all plans. +- Rules 1–3 use **3 of the 5 free WAF custom rules** available on all plans. - Blocks scanners, `curl` without `Origin`, and cross-site browser attacks before the Worker runs (never billed as a Worker request). ### Rate Limiting Rules -> **Conditional:** WAF rate limiting rules may require a **Pro plan** or above. If unavailable on your current Cloudflare plan (Free plan), skip this step. The Workers Rate Limiting Binding provides per-session rate limiting instead, and the WAF custom rule (above) still enforces the Origin check layer. +> **Note:** Cloudflare Free plan supports 1 unmanaged WAF rate limiting rule with a 10-second period (`60 requests per 10 seconds`) and a 10-second mitigation timeout (`Block for 10 seconds` on Free plan; `Block for 60 seconds` or longer on Pro and above). The Workers Rate Limiting Binding provides additional per-session rate limiting inside the Worker. **Rule name:** Rate limit API proxy endpoints **Where:** Security → WAF → Rate Limiting Rules @@ -342,7 +352,7 @@ not (http.request.uri.path eq "/api/error-reporting") (http.request.method ne "OPTIONS") ``` **Rate:** 60 requests per 10 seconds per IP -**Action:** Block for 60 seconds +**Action:** Block for 10 seconds (Free plan) / Block for 60 seconds (Pro+) See `hack/docs/security-runbook.md` for implementation details. diff --git a/scripts/waf-smoke-test.sh b/scripts/waf-smoke-test.sh index 9a2b1f39..26820bd0 100755 --- a/scripts/waf-smoke-test.sh +++ b/scripts/waf-smoke-test.sh @@ -62,7 +62,7 @@ TESTS=( "200|GET /settings|${BASE}/settings" "200|GET /privacy|${BASE}/privacy" "307|GET /index.html (html_handling redirect)|${BASE}/index.html" - "403,200,404|GET /_app/nonexistent.js|${BASE}/_app/nonexistent.js" + "404|GET /_app/nonexistent.js|${BASE}/_app/nonexistent.js" "200|GET /assets/nonexistent.js|${BASE}/assets/nonexistent.js" "200|GET /api/health (with Origin)|-H|Origin: ${BASE}|${BASE}/api/health" "400|POST /api/oauth/token (no body)|-X|POST|-H|Origin: ${BASE}|${BASE}/api/oauth/token" @@ -93,9 +93,9 @@ TESTS=( "403|GET /eval-stdin.php|${BASE}/eval-stdin.php" "403|GET /.aws/credentials|${BASE}/.aws/credentials" "403|GET /.ssh/id_rsa|${BASE}/.ssh/id_rsa" - "403,200|GET /robots.txt|${BASE}/robots.txt" - "403,200|GET /sitemap.xml|${BASE}/sitemap.xml" - "403,200|GET /favicon.ico|${BASE}/favicon.ico" + "200|GET /robots.txt|${BASE}/robots.txt" + "200|GET /sitemap.xml|${BASE}/sitemap.xml" + "200|GET /favicon.ico|${BASE}/favicon.ico" "403|GET /random/garbage/path|${BASE}/random/garbage/path" # Rule 2: Scanner User-Agents — normal UAs "200|Normal browser UA|-H|User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36|${BASE}/" diff --git a/src/app/components/dashboard/ActionsTab.tsx b/src/app/components/dashboard/ActionsTab.tsx index 4ea3adc1..597e7477 100644 --- a/src/app/components/dashboard/ActionsTab.tsx +++ b/src/app/components/dashboard/ActionsTab.tsx @@ -14,7 +14,7 @@ import ExpandCollapseButtons from "../shared/ExpandCollapseButtons"; import RepoLockControls from "../shared/RepoLockControls"; import RepoGitHubLink from "../shared/RepoGitHubLink"; import EmptyLockedRepoRow from "../shared/EmptyLockedRepoRow"; -import { orderRepoGroups, ensureLockedRepoGroups } from "../../lib/grouping"; +import { orderRepoGroups, ensureLockedRepoGroups, groupKeys, indexGroups, arraysShallowEqual } from "../../lib/grouping"; import { createReorderHighlight } from "../../lib/reorderHighlight"; import { createFlashDetection } from "../../lib/flashDetection"; @@ -193,6 +193,9 @@ export default function ActionsTab(props: ActionsTabProps) { ); return orderRepoGroups(withLocked, lockedForTab); }); + const visibleRepoGroups = repoGroups; + const repoGroupKeys = createMemo(() => groupKeys(visibleRepoGroups()), undefined, { equals: arraysShallowEqual }); + const repoGroupIndex = createMemo(() => indexGroups(visibleRepoGroups())); createEffect(() => { const names = activeRepoNames(); @@ -248,18 +251,27 @@ export default function ActionsTab(props: ActionsTabProps) { {/* Repo groups + locked stubs */} - 0) && repoGroups().length > 0}> - - {(repoGroup) => { - const isEmpty = () => repoGroup.workflows.length === 0; - const isExpanded = () => !isEmpty() && isRepoExpanded(tabKey(), repoGroup.repoFullName); + 0) && visibleRepoGroups().length > 0}> + + {(repoName) => { + const repoGroup = () => repoGroupIndex().get(repoName) ?? { repoFullName: repoName, workflows: [] }; + const isEmpty = () => repoGroup().workflows.length === 0; + const isExpanded = () => !isEmpty() && isRepoExpanded(tabKey(), repoGroup().repoFullName); const sortedWorkflows = createMemo(() => - sortWorkflowsByStatus(repoGroup.workflows) + sortWorkflowsByStatus(repoGroup().workflows) + ); + const workflowKeys = createMemo( + () => sortedWorkflows().map((w) => w.workflowId), + undefined, + { equals: arraysShallowEqual }, + ); + const workflowIndex = createMemo( + () => new Map(sortedWorkflows().map((w) => [w.workflowId, w])), ); const workflowCounts = createMemo(() => { - const wfs = repoGroup.workflows; + const wfs = repoGroup().workflows; const total = wfs.length; let passed = 0; let failed = 0; @@ -282,19 +294,19 @@ export default function ActionsTab(props: ActionsTabProps) { + } > -
+
toggleExpandedRepo(tabKey(), repoGroup.repoFullName)} + isHighlighted={highlightedReposActions().has(repoGroup().repoFullName)} + onToggle={() => toggleExpandedRepo(tabKey(), repoGroup().repoFullName)} trailing={ <> - - + + } collapsedSummary={ @@ -333,7 +345,7 @@ export default function ActionsTab(props: ActionsTabProps) { } /> - + {(peek) => (
@@ -346,16 +358,18 @@ export default function ActionsTab(props: ActionsTabProps) { {/* Workflow cards grid */}
- - {(wfGroup) => { - const wfKey = `${repoGroup.repoFullName}:${wfGroup.workflowId}`; + + {(wfId) => { + const wfGroup = () => + workflowIndex().get(wfId) ?? { workflowId: wfId, workflowName: "", runs: [] }; + const wfKey = `${repoGroup().repoFullName}:${wfId}`; const isWfExpanded = () => expandedWorkflows[wfKey] ?? false; return (
toggleWorkflow(wfKey)} onIgnoreRun={handleIgnore} diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index 6d611163..d454f5c6 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -1,5 +1,5 @@ -import { createSignal, createMemo, createEffect, on, Show, Switch, Match, onMount, onCleanup, untrack } from "solid-js"; -import { createStore, produce, unwrap } from "solid-js/store"; +import { batch, createSignal, createMemo, createEffect, on, Show, Switch, Match, onMount, onCleanup, untrack } from "solid-js"; +import { createStore, produce, reconcile, unwrap } from "solid-js/store"; import { z } from "zod"; import Header from "../layout/Header"; import TabBar, { type TabId } from "../layout/TabBar"; @@ -29,9 +29,11 @@ import { clearHotSets, getHotPollGeneration, fetchAllData, + StaleIdentityError, type DashboardData, } from "../../services/poll"; -import { expireToken, user, onAuthCleared, DASHBOARD_STORAGE_KEY, DEP_META_STORAGE_KEY, jiraAuth, setJiraAuth, isJiraAuthenticated, clearJiraAuth } from "../../stores/auth"; +import { expireToken, user, onAuthCleared, identityEpoch, DEP_META_STORAGE_KEY, jiraAuth, setJiraAuth, isJiraAuthenticated, clearJiraAuth } from "../../stores/auth"; +import { loadSnapshot, writeSnapshot, type DashboardSnapshotState } from "../../lib/dashboard-snapshot"; import { JiraApiError } from "../../services/jira-client"; import { createJiraClient, mergeCustomFields, jiraJqlForScope } from "../../lib/jira-utils"; import { JIRA_SETTINGS_ACTION, type JiraIssue } from "../../../shared/jira-types"; @@ -43,7 +45,6 @@ import { detectNewItems, dispatchNotifications } from "../../lib/notifications"; import { getClient, getGraphqlRateLimit, fetchRateLimitDetails } from "../../services/github"; import { canApprovePR, canMarkPRReady, formatCount, prSizeCategory, rateLimitCssClass, stripParenthetical } from "../../lib/format"; import { setsEqual } from "../../lib/collections"; -import { withScrollLock } from "../../lib/scroll"; import { Tooltip } from "../shared/Tooltip"; import { isIssueVisible, isPrVisible, isRunVisible, matchesActionsConclusionFilter } from "../../lib/filters"; import { isUserInvolved } from "../../lib/grouping"; @@ -86,11 +87,6 @@ const globalSortOptions: SortOption[] = [ // ── Shared dashboard store (module-level to survive navigation) ───────────── -// Bump only for breaking schema changes (renames, type changes). Additive optional -// fields (e.g., nodeId?: string) don't require a bump — missing fields deserialize -// as undefined, which consuming code handles gracefully. -const CACHE_VERSION = 2; - interface DashboardStore { issues: Issue[]; pullRequests: PullRequest[]; @@ -99,42 +95,28 @@ interface DashboardStore { lastRefreshedAt: Date | null; } -const initialDashboardState: DashboardStore = { - issues: [], - pullRequests: [], - workflowRuns: [], - loading: true, - lastRefreshedAt: null, -}; - -function loadCachedDashboard(): DashboardStore { - try { - const raw = localStorage.getItem?.(DASHBOARD_STORAGE_KEY); - if (!raw) return { ...initialDashboardState }; - const parsed = JSON.parse(raw) as Record; - // Invalidate cache on schema version mismatch - if (parsed._v !== CACHE_VERSION) return { ...initialDashboardState }; - // Validate expected shape — arrays must be arrays - if (!Array.isArray(parsed.issues) || !Array.isArray(parsed.pullRequests) || !Array.isArray(parsed.workflowRuns)) { - return { ...initialDashboardState }; - } - return { - issues: parsed.issues as Issue[], - pullRequests: parsed.pullRequests as PullRequest[], - workflowRuns: parsed.workflowRuns as WorkflowRun[], - loading: false, - lastRefreshedAt: typeof parsed.lastRefreshedAt === "string" ? new Date(parsed.lastRefreshedAt) : null, - }; - } catch { - return { ...initialDashboardState }; - } +function makeInitialDashboardState(): DashboardStore { + return { + issues: [], + pullRequests: [], + workflowRuns: [], + loading: true, + lastRefreshedAt: null, + }; } -const [dashboardData, setDashboardData] = createStore(loadCachedDashboard()); +let realDataCommitted = false; + +const [dashboardData, setDashboardData] = createStore(makeInitialDashboardState()); function resetDashboardData(): void { - setDashboardData({ ...initialDashboardState }); - localStorage.removeItem?.(DASHBOARD_STORAGE_KEY); + realDataCommitted = false; + batch(() => { + setDashboardData("issues", reconcile([], KEYED)); + setDashboardData("pullRequests", reconcile([], KEYED)); + setDashboardData("workflowRuns", reconcile([], KEYED)); + setDashboardData(makeInitialDashboardState()); + }); } const [hasFetchedFresh, setHasFetchedFresh] = createSignal(false); @@ -387,13 +369,10 @@ function seedCachedDashboardBodies(): void { if (seededAbandoned.size > 0) setAbandonedDepsMap(seededAbandoned); if (seededUrls.size > 0) setDashboardIssueUrls(seededUrls); } -seedCachedDashboardBodies(); const _dashboardBodyFailureCooldown = new Map(); -let _bodyFetchAuthGen = 0; // Clear dashboard data and stop polling on logout to prevent cross-user data leakage onAuthCleared(() => { - _bodyFetchAuthGen++; _targetedPushedEnrichmentDegraded = false; resetDashboardData(); setHasFetchedFresh(false); @@ -436,6 +415,7 @@ onAuthCleared(() => { }); let _targetedPushedEnrichmentDegraded = false; +const KEYED = { key: "id", merge: true } as const; async function pollFetch(force = false): Promise { // Only show skeleton on initial load (no data yet). @@ -444,26 +424,41 @@ async function pollFetch(force = false): Promise { if (!dashboardData.lastRefreshedAt) { setDashboardData("loading", true); } + const fetchEpoch = identityEpoch(); try { // Two-phase rendering: phase 1 callback fires with light issues + PRs // so the UI renders immediately. Phase 2 (enrichment + workflow runs) // arrives when fetchAllData resolves. - let phaseOneFired = false; const data = await fetchAllData((lightData) => { + if (fetchEpoch !== identityEpoch()) return; // Phase 1: render light issues + PRs immediately — but only on initial // load (no cached data). On reload with cached data, the cache already // has enriched PRs; replacing them with light PRs would cause a visible // flicker (badges disappear then reappear when phase 2 arrives). if (dashboardData.pullRequests.length === 0) { - phaseOneFired = true; - setDashboardData({ - issues: lightData.issues, - pullRequests: lightData.pullRequests, - loading: false, - lastRefreshedAt: new Date(), + realDataCommitted = true; + batch(() => { + setDashboardData( + "issues", + reconcile( + lightData.issues.map((i) => Object.assign({}, i)), + KEYED, + ), + ); + setDashboardData( + "pullRequests", + reconcile( + lightData.pullRequests.map((pr) => Object.assign({}, pr)), + KEYED, + ), + ); + setDashboardData({ loading: false, lastRefreshedAt: new Date() }); }); } }, force); + if (fetchEpoch !== identityEpoch()) { + throw new StaleIdentityError(); + } const hasErrors = data.errors.length > 0; setLastFetchHadErrors(hasErrors); @@ -478,125 +473,41 @@ async function pollFetch(force = false): Promise { setHasFetchedFresh(true); const now = new Date(); const currentLogin = user()?.login ?? ""; - for (const pr of data.pullRequests) { + const pullRequests = fallbackToPreviousEnrichment( + unwrap(dashboardData.pullRequests), + data.pullRequests, + ); + for (const pr of pullRequests) { applyRecentReadyGuard(pr); - if (pr.enriched !== false) { - applyRecentApprovalGuard(pr, currentLogin); - } - } - - let resolvedPullRequests: PullRequest[]; - if (phaseOneFired) { - // Phase 1 fired — use fine-grained merge for the light→enriched - // transition. Only update heavy fields to avoid re-rendering the - // entire list (light fields haven't changed within this poll cycle). - const enrichedMap = new Map(); - for (const pr of data.pullRequests) enrichedMap.set(pr.id, pr); - - setDashboardData(produce((state) => { - state.issues = data.issues; - state.workflowRuns = config.enableActions ? data.workflowRuns : []; - state.loading = false; - state.lastRefreshedAt = now; - - let canMerge = state.pullRequests.length === enrichedMap.size; - if (canMerge) { - for (let i = 0; i < state.pullRequests.length; i++) { - if (!enrichedMap.has(state.pullRequests[i].id)) { canMerge = false; break; } - } - } - - if (canMerge) { - for (let i = 0; i < state.pullRequests.length; i++) { - const e = enrichedMap.get(state.pullRequests[i].id)!; - const pr = state.pullRequests[i]; - // A failed backfill batch returns e.enriched === false for PRs it - // couldn't reach — don't let that regress a PR that was already - // enriched from a prior cycle. - const regressing = e.enriched === false && pr.enriched !== false; - if (!regressing) { - const sameRevision = - pr.enriched !== false && !pr.checkStatusStale && isSameRevision(pr, e); - const prevCheckStatus = pr.checkStatus; - Object.assign(pr, pickEnrichmentFields(e)); - pr.checkStatusStale = e.checkStatusStale; - if (e.checkStatusStale) { - if (sameRevision) { - pr.checkStatus = prevCheckStatus; - pr.checkStatusStale = false; - } else { - pr.checkStatus = null; - } - } - pr.reviewDecision = e.reviewDecision; - pr.draft = e.draft; - pr.enriched = e.enriched; - } else { - const revisionChanged = hasRevisionChanged(pr, e); - pr.updatedAt = e.updatedAt; - pr.reviewDecision = e.reviewDecision; - pr.draft = e.draft; - if (revisionChanged || pr.checkStatusStale) markCheckStatusStale(pr); - } - pr.nodeId = e.nodeId; - pr.surfacedBy = e.surfacedBy; - pr.starCount = e.starCount; - applyRecentReadyGuard(pr); - applyRecentApprovalGuard(pr, currentLogin); - } - } else { - state.pullRequests = fallbackToPreviousEnrichment(state.pullRequests, data.pullRequests); - for (const pr of state.pullRequests) { - applyRecentReadyGuard(pr); - applyRecentApprovalGuard(pr, currentLogin); - } - } - })); - resolvedPullRequests = unwrap(dashboardData.pullRequests).map((pr) => Object.assign({}, pr)); - } else { - // Phase 1 did NOT fire (cached data existed or subsequent poll). - // Full atomic replacement — all fields (light + heavy) may have - // changed since the last cycle. Preserve scroll position: SolidJS - // DOM updates are synchronous within the setter, so save/restore - // around it to prevent scroll reset from DOM rebuild. - const pullRequests = fallbackToPreviousEnrichment(unwrap(dashboardData.pullRequests), data.pullRequests); - for (const pr of pullRequests) { - applyRecentReadyGuard(pr); - applyRecentApprovalGuard(pr, currentLogin); - } - resolvedPullRequests = pullRequests; - withScrollLock(() => { - setDashboardData({ - issues: data.issues, - pullRequests, - workflowRuns: config.enableActions ? data.workflowRuns : [], - loading: false, - lastRefreshedAt: now, - }); - }); + applyRecentApprovalGuard(pr, currentLogin); } const filteredRuns = config.enableActions ? data.workflowRuns : []; - const hotPullRequests = pinRecentlyMutatedPRs(resolvedPullRequests); + realDataCommitted = true; + batch(() => { + setDashboardData("issues", reconcile(data.issues, KEYED)); + setDashboardData("pullRequests", reconcile(pullRequests, KEYED)); + setDashboardData("workflowRuns", reconcile(filteredRuns, KEYED)); + setDashboardData({ loading: false, lastRefreshedAt: now }); + }); + if (_dashboardBodyCache.size > 0 && abandonedDepsMap().size === 0) { + seedCachedDashboardBodies(); + } + const hotPullRequests = pinRecentlyMutatedPRs(pullRequests); rebuildHotSets({ ...data, pullRequests: hotPullRequests, workflowRuns: filteredRuns }); // Persist for stale-while-revalidate on full page reload. - // Errors are transient and not persisted. Deferred to avoid blocking paint. - const cachePayload = { - _v: CACHE_VERSION, - issues: data.issues, - pullRequests: resolvedPullRequests, - workflowRuns: filteredRuns, - lastRefreshedAt: now.toISOString(), - }; - setTimeout(() => { - try { - localStorage.setItem(DASHBOARD_STORAGE_KEY, JSON.stringify(cachePayload)); - } catch { - pushNotification("localStorage:dashboard", "Dashboard cache write failed — storage may be full", "warning"); - } - }, 0); + // Errors are transient and not persisted. + const snapshotLogin = user()?.login; + if (snapshotLogin) { + writeSnapshot(currentSnapshotState(), snapshotLogin, "immediate"); + } _targetedPushedEnrichmentDegraded = false; - return { ...data, pullRequests: resolvedPullRequests }; + return { ...data, pullRequests }; } catch (err) { + // A poll started under a previous identity must not touch the current + // identity's store, token, or error surfaces. + if (fetchEpoch !== identityEpoch()) { + throw err instanceof StaleIdentityError ? err : new StaleIdentityError(); + } // Handle 401 auth errors const status = typeof err === "object" && @@ -620,39 +531,39 @@ const [_coordinator, _setCoordinator] = createSignal void } | null>(null); const [_eventsCoordinator, _setEventsCoordinator] = createSignal<{ destroy: () => void } | null>(null); -function persistDashboardSnapshot(): void { - // Capture snapshot eagerly — a concurrent hot poll can mutate the store via produce - // between here and the deferred setTimeout write. - const snapshot = { - _v: CACHE_VERSION, - issues: [...dashboardData.issues], - pullRequests: [...dashboardData.pullRequests], - workflowRuns: [...dashboardData.workflowRuns], - lastRefreshedAt: dashboardData.lastRefreshedAt?.toISOString() ?? null, +function currentSnapshotState(): DashboardSnapshotState { + return { + issues: unwrap(dashboardData.issues), + pullRequests: unwrap(dashboardData.pullRequests), + workflowRuns: unwrap(dashboardData.workflowRuns), + lastRefreshedAt: dashboardData.lastRefreshedAt, }; - setTimeout(() => { - try { - localStorage.setItem(DASHBOARD_STORAGE_KEY, JSON.stringify(snapshot)); - } catch { - // Non-fatal - } - }, 0); +} + +function persistDashboardSnapshot(): void { + const login = user()?.login; + if (!login) return; + writeSnapshot(currentSnapshotState(), login, "debounced"); } // Mutates data.issues[].surfacedBy and data.pullRequests[].surfacedBy in-place before merging. -function handleTargetedData(data: DashboardData, affectedRepos: string[]): void { +function handleTargetedData(data: DashboardData, affectedRepos: string[], epoch: number): void { + if (epoch !== identityEpoch()) return; const affectedSet = new Set(affectedRepos.map(r => r.toLowerCase())); + const currentIssues = unwrap(dashboardData.issues); + const currentPRs = unwrap(dashboardData.pullRequests); + const currentRuns = unwrap(dashboardData.workflowRuns); // Build surfacedBy index from old store items BEFORE the merge const oldSurfacedByIssues = new Map(); - for (const i of dashboardData.issues) { + for (const i of currentIssues) { if (affectedSet.has(i.repoFullName.toLowerCase()) && i.surfacedBy?.length) { oldSurfacedByIssues.set(i.id, i.surfacedBy); } } const oldSurfacedByPRs = new Map(); const existingPRById = new Map(); - for (const pr of dashboardData.pullRequests) { + for (const pr of currentPRs) { existingPRById.set(pr.id, pr); if (affectedSet.has(pr.repoFullName.toLowerCase()) && pr.surfacedBy?.length) { oldSurfacedByPRs.set(pr.id, pr.surfacedBy); @@ -695,36 +606,38 @@ function handleTargetedData(data: DashboardData, affectedRepos: string[]): void applyRecentApprovalGuard(pr, currentLogin); } + // ID-based merge: replace targeted items, keep unaffected + tracked-user-only items + const newIssueIds = new Set(data.issues.map(i => i.id)); + const mergedIssues = [ + ...currentIssues.filter(i => + !affectedSet.has(i.repoFullName.toLowerCase()) || + !newIssueIds.has(i.id) + ), + ...data.issues, + ]; + const newPRIds = new Set(data.pullRequests.map(pr => pr.id)); + const mergedPRs = [ + ...currentPRs.filter(pr => + !affectedSet.has(pr.repoFullName.toLowerCase()) || + !newPRIds.has(pr.id) + ), + ...data.pullRequests, + ]; const targetedRuns = config.enableActions ? data.workflowRuns : []; - - withScrollLock(() => { - setDashboardData(produce((state) => { - // ID-based merge: replace targeted items, keep unaffected + tracked-user-only items - const newIssueIds = new Set(data.issues.map(i => i.id)); - state.issues = [ - ...state.issues.filter(i => - !affectedSet.has(i.repoFullName.toLowerCase()) || - !newIssueIds.has(i.id) - ), - ...data.issues, - ]; - const newPRIds = new Set(data.pullRequests.map(pr => pr.id)); - state.pullRequests = [ - ...state.pullRequests.filter(pr => - !affectedSet.has(pr.repoFullName.toLowerCase()) || - !newPRIds.has(pr.id) - ), - ...data.pullRequests, - ]; - const newRunIds = new Set(targetedRuns.map(r => r.id)); - state.workflowRuns = config.enableActions ? [ - ...state.workflowRuns.filter(r => - !affectedSet.has(r.repoFullName.toLowerCase()) || - !newRunIds.has(r.id) - ), - ...targetedRuns, - ] : []; - })); + const newRunIds = new Set(targetedRuns.map(r => r.id)); + const mergedRuns = config.enableActions ? [ + ...currentRuns.filter(r => + !affectedSet.has(r.repoFullName.toLowerCase()) || + !newRunIds.has(r.id) + ), + ...targetedRuns, + ] : []; + + realDataCommitted = true; + batch(() => { + setDashboardData("issues", reconcile(mergedIssues, KEYED)); + setDashboardData("pullRequests", reconcile(mergedPRs, KEYED)); + setDashboardData("workflowRuns", reconcile(mergedRuns, KEYED)); }); const targetedBackfillErrors = data.errors.filter(isBackfillError); @@ -1003,6 +916,8 @@ export default function DashboardPage() { // so SolidJS registers them as dependencies even when the guard short-circuits const issues = dashboardData.issues; const prs = dashboardData.pullRequests; + void issues.length; + void prs.length; if (!config.enableTracking || viewState.trackedItems.length === 0 || !hasFetchedFresh()) return; // Never prune when the last fetch had errors (rate limit, network failure, etc.) // — the missing items are likely just unfetched, not closed/merged. @@ -1026,6 +941,29 @@ export default function DashboardPage() { }); onMount(() => { + if ( + !realDataCommitted && + dashboardData.issues.length === 0 && + dashboardData.pullRequests.length === 0 && + dashboardData.workflowRuns.length === 0 + ) { + const login = user()?.login; + if (login) { + const mountEpoch = identityEpoch(); + void (async () => { + const snap = await loadSnapshot(login); + if (!snap || mountEpoch !== identityEpoch() || realDataCommitted) return; + batch(() => { + setDashboardData("issues", reconcile(snap.issues, KEYED)); + setDashboardData("pullRequests", reconcile(snap.pullRequests, KEYED)); + setDashboardData("workflowRuns", reconcile(config.enableActions ? snap.workflowRuns : [], KEYED)); + setDashboardData({ loading: false, lastRefreshedAt: snap.lastRefreshedAt }); + }); + seedCachedDashboardBodies(); + })(); + } + } + if (!_coordinator()) { _setCoordinator(createPollCoordinator(() => config.refreshInterval, pollFetch)); } @@ -1048,12 +986,13 @@ export default function DashboardPage() { if (!_hotCoordinator()) { _setHotCoordinator(createHotPollCoordinator( () => config.hotPollInterval, - (prUpdates, runUpdates, fetchGeneration) => { - // Guard against stale hot poll results overlapping with a full refresh. + (prUpdates, runUpdates, fetchGeneration, epoch) => { + // Guard against stale hot poll results overlapping with a full refresh + // or an identity change while fetchHotData() was in flight. // fetchGeneration was captured BEFORE fetchHotData() started its async work. // If a full refresh completed during the fetch, _hotPollGeneration will have // been incremented by rebuildHotSets(), and fetchGeneration will be stale. - if (fetchGeneration !== getHotPollGeneration()) return; // stale, discard + if (fetchGeneration !== getHotPollGeneration() || epoch !== identityEpoch()) return; // stale, discard let terminalPrIds = new Set(); setDashboardData(produce((state) => { const currentLogin = user()?.login ?? ""; @@ -1070,25 +1009,15 @@ export default function DashboardPage() { })); if (terminalPrIds.size > 0) { console.info(`[hot-poll] Spliced ${terminalPrIds.size} terminal PR(s) from store`); - setTimeout(() => { - try { - const cachePayload = { - _v: CACHE_VERSION, - issues: dashboardData.issues, - pullRequests: dashboardData.pullRequests, - workflowRuns: dashboardData.workflowRuns, - lastRefreshedAt: dashboardData.lastRefreshedAt?.toISOString(), - }; - localStorage.setItem(DASHBOARD_STORAGE_KEY, JSON.stringify(cachePayload)); - } catch { - pushNotification("localStorage:dashboard", "Dashboard cache write failed — storage may be full", "warning"); - } - }, 0); } + // Every delivering cycle persists so a reload shows current check/run statuses. + // The debounced write costs one structured clone of the store per cycle + // (a few ms at ~1000 items) and only fires while hot items are in flight. + persistDashboardSnapshot(); // Prune tracked PRs that became closed/merged via hot poll. - // The auto-prune createEffect only fires when the pullRequests array - // reference changes (full refresh). Hot poll mutates nested pr.state - // in-place via produce(), leaving the array ref unchanged. + // Full and targeted refreshes reconcile open-only results into the store + // (triggering the auto-prune createEffect via array membership changes), + // whereas hot poll receives terminal CLOSED/MERGED status updates directly. if (config.enableTracking && viewState.trackedItems.length > 0 && prUpdates.size > 0) { const pruneKeys = new Set(); for (const [prId, update] of prUpdates) { @@ -1513,7 +1442,7 @@ export default function DashboardPage() { const octokit = getClient(); if (!octokit) return; _fetchingDashboardBodies = true; - const authGen = _bodyFetchAuthGen; + const authGen = identityEpoch(); void (async () => { try { const dashboardIssues = findDashboardIssues(dashboardData.issues, trackedBotLogins()); @@ -1558,7 +1487,7 @@ export default function DashboardPage() { octokit, toFetch.map((di) => di.nodeId) ); - if (authGen !== _bodyFetchAuthGen || getClient() !== octokit) return; + if (authGen !== identityEpoch() || getClient() !== octokit) return; failedCount = failedIds.size; let cacheUpdated = false; for (const di of toFetch) { @@ -1593,7 +1522,9 @@ export default function DashboardPage() { setAbandonedDepsMap(newAbandonedMap); } } finally { - if (authGen === _bodyFetchAuthGen) { + // Safe: every identityEpoch bump is followed by onAuthCleared (which resets + // _fetchingDashboardBodies = false) or a full page reload. + if (authGen === identityEpoch()) { _fetchingDashboardBodies = false; } } @@ -1620,6 +1551,7 @@ export default function DashboardPage() { const depPrs = dependencyPullRequests(); const visibleDepPrs = visibleDependencyPullRequests(); const enrichmentPending = awaitingEnrichment(); + void dashboardData.lastRefreshedAt; if (_fetchingDepBodies) { console.debug("[dashboard] depBodies effect: skipped — fetch already in flight"); return; @@ -1652,14 +1584,14 @@ export default function DashboardPage() { } _fetchingDepBodies = true; - const authGen = _bodyFetchAuthGen; + const authGen = identityEpoch(); const effectStart = Date.now(); console.debug(`[dashboard] depBodies effect: fetch started for ${toFetch.length} PRs at ${effectStart}`); void (async () => { try { const nodeIds = toFetch.map((pr) => pr.nodeId!); const { bodies: bodyMap, failedIds } = await fetchDepPRBodies(octokit, nodeIds); - if (authGen !== _bodyFetchAuthGen || getClient() !== octokit) return; + if (authGen !== identityEpoch() || getClient() !== octokit) return; console.debug(`[dashboard] depBodies effect: fetch resolved after ${Date.now() - effectStart}ms`, { requested: toFetch.length, returned: bodyMap.size, failed: failedIds.size }); // Record/clear per-PR cooldown so a batch that errored or timed out isn't @@ -1686,10 +1618,12 @@ export default function DashboardPage() { setDepMeta(merged); console.debug(`[dashboard] depBodies effect: depMeta updated, size=${merged.size}`); setTimeout(() => { - if (authGen === _bodyFetchAuthGen) persistDepMeta(merged); + if (authGen === identityEpoch()) persistDepMeta(merged); }, 0); } finally { - if (authGen === _bodyFetchAuthGen) { + // Safe: every identityEpoch bump is followed by onAuthCleared (which resets + // _fetchingDepBodies = false) or a full page reload. + if (authGen === identityEpoch()) { _fetchingDepBodies = false; } console.debug(`[dashboard] depBodies effect: guard released after ${Date.now() - effectStart}ms`); @@ -1800,10 +1734,11 @@ export default function DashboardPage() { dismissNotificationBySource(`approve-pr:${pr.id}`); setApprovingPRIds((prev) => addIdToSet(prev, pr.id)); + const mutationEpoch = identityEpoch(); try { const result = await approvePullRequest(client, nodeId, approvedHeadSha); - if (getClient() !== client) return; + if (mutationEpoch !== identityEpoch() || getClient() !== client) return; evictPREnrichmentCacheNode(pr.nodeId); const nextDecision: PullRequest["reviewDecision"] = @@ -1834,7 +1769,7 @@ export default function DashboardPage() { persistDashboardSnapshot(); } catch (err) { - if (getClient() !== client) return; + if (mutationEpoch !== identityEpoch() || getClient() !== client) return; if ( handlePRMutationCommonError( err, @@ -1871,10 +1806,11 @@ export default function DashboardPage() { dismissNotificationBySource(`mark-ready-pr:${pr.id}`); setMarkingReadyPRIds((prev) => addIdToSet(prev, pr.id)); + const mutationEpoch = identityEpoch(); try { const result = await markPullRequestReady(client, nodeId); - if (getClient() !== client) return; + if (mutationEpoch !== identityEpoch() || getClient() !== client) return; evictPREnrichmentCacheNode(pr.nodeId); recentlyMarkedReadyPRs.set(pr.id, { @@ -1903,7 +1839,7 @@ export default function DashboardPage() { persistDashboardSnapshot(); } catch (err) { - if (getClient() !== client) return; + if (mutationEpoch !== identityEpoch() || getClient() !== client) return; if ( handlePRMutationCommonError( err, diff --git a/src/app/components/dashboard/IssuesTab.tsx b/src/app/components/dashboard/IssuesTab.tsx index 05cf28b1..a2f004ae 100644 --- a/src/app/components/dashboard/IssuesTab.tsx +++ b/src/app/components/dashboard/IssuesTab.tsx @@ -15,7 +15,7 @@ import SkeletonRows from "../shared/SkeletonRows"; import ExpandCollapseButtons from "../shared/ExpandCollapseButtons"; import { deriveInvolvementRoles } from "../../lib/format"; import RepoGroupHeader from "../shared/RepoGroupHeader"; -import { groupByRepo, computePageLayout, slicePageGroups, orderRepoGroups, ensureLockedRepoGroups, isUserInvolved } from "../../lib/grouping"; +import { groupByRepo, computePageLayout, slicePageGroups, orderRepoGroups, ensureLockedRepoGroups, isUserInvolved, groupKeys, indexGroups, arraysShallowEqual } from "../../lib/grouping"; import { createReorderHighlight } from "../../lib/reorderHighlight"; import RepoLockControls from "../shared/RepoLockControls"; import RepoGitHubLink from "../shared/RepoGitHubLink"; @@ -204,6 +204,8 @@ export default function IssuesTab(props: IssuesTabProps) { const pageGroups = createMemo(() => slicePageGroups(repoGroups(), pageLayout().boundaries, pageCount(), page()) ); + const pageGroupKeys = createMemo(() => groupKeys(pageGroups()), undefined, { equals: arraysShallowEqual }); + const groupIndex = createMemo(() => indexGroups(pageGroups())); createEffect(() => { const max = pageCount() - 1; @@ -312,14 +314,15 @@ export default function IssuesTab(props: IssuesTabProps) { {/* Issue rows + locked stubs */} 0) && pageGroups().length > 0}>
- - {(repoGroup) => { - const isEmpty = () => repoGroup.items.length === 0; - const isExpanded = () => !isEmpty() && isRepoExpanded(tabKey(), repoGroup.repoFullName); + + {(repoName) => { + const repoGroup = () => groupIndex().get(repoName) ?? { repoFullName: repoName, items: [] }; + const isEmpty = () => repoGroup().items.length === 0; + const isExpanded = () => !isEmpty() && isRepoExpanded(tabKey(), repoGroup().repoFullName); const roleSummary = createMemo(() => { const counts: Record = {}; - for (const item of repoGroup.items) { + for (const item of repoGroup().items) { const m = issueMeta().get(item.id); if (m) { for (const role of m.roles) { @@ -334,18 +337,18 @@ export default function IssuesTab(props: IssuesTabProps) { + } > -
+
toggleExpandedRepo(tabKey(), repoGroup.repoFullName)} + isHighlighted={highlightedReposIssues().has(repoGroup().repoFullName)} + onToggle={() => toggleExpandedRepo(tabKey(), repoGroup().repoFullName)} badges={ - + Monitoring all @@ -353,13 +356,13 @@ export default function IssuesTab(props: IssuesTabProps) { } trailing={ <> - - + + } collapsedSummary={ - {repoGroup.items.length} {repoGroup.items.length === 1 ? "issue" : "issues"} + {repoGroup().items.length} {repoGroup().items.length === 1 ? "issue" : "issues"} {([role, count]) => (
- + {(issue) => (
slicePageGroups(repoGroups(), pageLayout().boundaries, pageCount(), page()) ); + const pageGroupKeys = createMemo(() => groupKeys(pageGroups()), undefined, { equals: arraysShallowEqual }); + const groupIndex = createMemo(() => indexGroups(pageGroups())); createEffect(() => { const max = pageCount() - 1; @@ -383,17 +385,18 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { {/* PR rows + locked stubs */} 0) && pageGroups().length > 0}>
- - {(repoGroup) => { - const isEmpty = () => repoGroup.items.length === 0; - const isExpanded = () => !isEmpty() && isRepoExpanded(tabKey(), repoGroup.repoFullName); + + {(repoName) => { + const repoGroup = () => groupIndex().get(repoName) ?? { repoFullName: repoName, items: [] }; + const isEmpty = () => repoGroup().items.length === 0; + const isExpanded = () => !isEmpty() && isRepoExpanded(tabKey(), repoGroup().repoFullName); const summaryMeta = createMemo(() => { const checks = { success: 0, failure: 0, running: 0, waiting: 0, pending: 0, conflict: 0 }; const reviews = { APPROVED: 0, CHANGES_REQUESTED: 0, REVIEW_REQUIRED: 0 }; const roles: Record = {}; - for (const item of repoGroup.items) { + for (const item of repoGroup().items) { if (item.checkStatus === "success") checks.success++; else if (item.checkStatus === "failure") checks.failure++; else if (item.checkStatus === "running") checks.running++; @@ -420,18 +423,18 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { + } > -
+
toggleExpandedRepo(tabKey(), repoGroup.repoFullName)} + isHighlighted={highlightedReposPRs().has(repoGroup().repoFullName)} + onToggle={() => toggleExpandedRepo(tabKey(), repoGroup().repoFullName)} badges={ - + Monitoring all @@ -439,13 +442,13 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { } trailing={ <> - - + + } collapsedSummary={ - {repoGroup.items.length} {repoGroup.items.length === 1 ? "PR" : "PRs"} + {repoGroup().items.length} {repoGroup().items.length === 1 ? "PR" : "PRs"} 0}> @@ -514,7 +517,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) { } /> - + {(peek) => (
@@ -525,7 +528,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
- + {(pr) => (
> | null = null; +let _writeToken = 0; +let _sizeWarned = false; +// Set once an over-cap write has queued the stored-record delete; cleared by the +// next under-cap write so repeated over-cap writes don't queue redundant deletes. +let _discardQueued = false; +let _pendingTimer: ReturnType | null = null; +let _pendingMode: "immediate" | "debounced" | null = null; +let _pendingPayload: DashboardSnapshot | null = null; +let _pendingToken = 0; +let _inFlightMutation: Promise | null = null; + +function getDb(): Promise> { + if (!dbPromise) { + let currentPromise: Promise> | null = null; + currentPromise = openDB(DB_NAME, DB_VERSION, { + upgrade(db) { + if (!db.objectStoreNames.contains(STORE_NAME)) { + db.createObjectStore(STORE_NAME); + } + }, + blocking(_currentVersion, _blockedVersion, event) { + const target = event?.target as IDBDatabase | null | undefined; + target?.close?.(); + void currentPromise?.then((db) => db.close()).catch(() => {}); + if (dbPromise === currentPromise) { + dbPromise = null; + } + }, + terminated() { + if (dbPromise === currentPromise) { + dbPromise = null; + } + }, + }).catch((err: unknown) => { + if (dbPromise === currentPromise) { + dbPromise = null; + } + throw err; + }); + dbPromise = currentPromise; + } + return dbPromise; +} + +function isValidKeyedArray(value: unknown): boolean { + return ( + Array.isArray(value) && + value.every( + (item) => + item !== null && + typeof item === "object" && + typeof (item as Record).id === "number", + ) + ); +} + +export async function loadSnapshot(login: string): Promise { + if (typeof indexedDB === "undefined" || !login) { + return null; + } + try { + const db = await getDb(); + const raw = (await db.get(STORE_NAME, RECORD_KEY)) as unknown; + if (!raw || typeof raw !== "object") { + return null; + } + const record = raw as Record; + if (record._v !== SNAPSHOT_VERSION) { + return null; + } + if ( + typeof record.login !== "string" || + record.login.toLowerCase() !== login.toLowerCase() + ) { + return null; + } + if (record.savedAt !== undefined && typeof record.savedAt !== "number") { + return null; + } + if ( + !isValidKeyedArray(record.issues) || + !isValidKeyedArray(record.pullRequests) || + !isValidKeyedArray(record.workflowRuns) + ) { + return null; + } + let lastRefreshedAt: Date | null = null; + if (record.lastRefreshedAt === null) { + lastRefreshedAt = null; + } else if (record.lastRefreshedAt instanceof Date) { + if (Number.isNaN(record.lastRefreshedAt.getTime())) { + return null; + } + lastRefreshedAt = record.lastRefreshedAt; + } else if (typeof record.lastRefreshedAt === "string") { + const parsedDate = new Date(record.lastRefreshedAt); + if (Number.isNaN(parsedDate.getTime())) { + return null; + } + lastRefreshedAt = parsedDate; + } else { + return null; + } + + return { + _v: SNAPSHOT_VERSION, + login: record.login, + savedAt: typeof record.savedAt === "number" ? record.savedAt : 0, + issues: record.issues as Issue[], + pullRequests: record.pullRequests as PullRequest[], + workflowRuns: record.workflowRuns as WorkflowRun[], + lastRefreshedAt, + }; + } catch (err) { + console.warn("[dashboard-snapshot] Failed to load dashboard snapshot:", err); + return null; + } +} + +async function performWrite(payload: DashboardSnapshot, token: number): Promise { + if (token !== _writeToken) return; + try { + const db = await getDb(); + if (token !== _writeToken) return; + await db.put(STORE_NAME, payload, RECORD_KEY); + } catch (err) { + console.warn("[dashboard-snapshot] Failed to write dashboard snapshot:", err); + pushNotification( + "snapshot:write", + "Dashboard snapshot write failed — storage may be full", + "warning", + ); + } +} + +async function performDelete(token: number): Promise { + if (token !== _writeToken) return; + try { + const db = await getDb(); + if (token !== _writeToken) return; + await db.delete(STORE_NAME, RECORD_KEY); + } catch (err) { + console.warn("[dashboard-snapshot] Failed to discard dashboard snapshot:", err); + } +} + +// Serializes IDB mutations so each runs after the previously queued one settles. +function enqueueMutation(task: () => Promise): Promise { + const previousMutation = _inFlightMutation; + const mutationPromise = (async () => { + if (previousMutation) { + await previousMutation; + } + await task(); + })().finally(() => { + if (_inFlightMutation === mutationPromise) { + _inFlightMutation = null; + } + }); + _inFlightMutation = mutationPromise; + return mutationPromise; +} + +function cancelPendingWrite(): void { + if (_pendingTimer !== null) { + clearTimeout(_pendingTimer); + _pendingTimer = null; + } + _pendingMode = null; + _pendingPayload = null; +} + +function flushPending(): Promise { + if (_pendingTimer !== null) { + clearTimeout(_pendingTimer); + _pendingTimer = null; + } + _pendingMode = null; + const payload = _pendingPayload; + const token = _pendingToken; + _pendingPayload = null; + if (!payload || token !== _writeToken) { + return _inFlightMutation ?? Promise.resolve(); + } + return enqueueMutation(() => performWrite(payload, token)); +} + +export function writeSnapshot( + state: DashboardSnapshotState, + login: string, + mode: "immediate" | "debounced", +): void { + if (typeof indexedDB === "undefined" || !login) { + return; + } + const totalItems = + state.issues.length + state.pullRequests.length + state.workflowRuns.length; + if (totalItems > MAX_SNAPSHOT_ITEMS) { + if (!_sizeWarned) { + _sizeWarned = true; + console.warn( + `[dashboard-snapshot] Snapshot exceeds ${MAX_SNAPSHOT_ITEMS} items (${totalItems}); skipping write and discarding stored snapshot`, + ); + } + // Drop any queued under-cap payload and remove the stored record so a reload + // never restores data older than the current dashboard. + cancelPendingWrite(); + if (!_discardQueued) { + _discardQueued = true; + const deleteToken = _writeToken; + void enqueueMutation(() => performDelete(deleteToken)); + } + return; + } + + _discardQueued = false; + const capturedToken = _writeToken; + _pendingPayload = { + _v: SNAPSHOT_VERSION, + login, + savedAt: Date.now(), + issues: state.issues, + pullRequests: state.pullRequests, + workflowRuns: state.workflowRuns, + lastRefreshedAt: state.lastRefreshedAt, + }; + _pendingToken = capturedToken; + + if (mode === "immediate") { + if (_pendingTimer !== null) { + clearTimeout(_pendingTimer); + } + _pendingMode = "immediate"; + _pendingTimer = setTimeout(() => { + _pendingTimer = null; + _pendingMode = null; + void flushPending(); + }, 0); + } else { + if (_pendingMode === "immediate" && _pendingTimer !== null) { + return; + } + if (_pendingTimer !== null) { + clearTimeout(_pendingTimer); + } + _pendingMode = "debounced"; + _pendingTimer = setTimeout(() => { + _pendingTimer = null; + _pendingMode = null; + void flushPending(); + }, DEBOUNCE_MS); + } +} + +export function flushSnapshot(): Promise { + return flushPending(); +} + +/** + * Clears any pending or persisted dashboard snapshot. + * Invariant: only called from the auth.ts paths that also increment identityEpoch + * (clearAuth, expireToken, clearIdentityData, and the cross-tab identity-switch reload). + */ +export function clearSnapshot(): Promise { + cancelPendingWrite(); + _writeToken++; + _sizeWarned = false; + _discardQueued = false; + + if (typeof indexedDB === "undefined") { + return Promise.resolve(); + } + + return (async () => { + try { + const db = await getDb(); + await db.delete(STORE_NAME, RECORD_KEY); + } catch (err) { + console.warn("[dashboard-snapshot] Failed to clear dashboard snapshot:", err); + } + })(); +} + +if (typeof document !== "undefined" && typeof document.addEventListener === "function") { + document.addEventListener("visibilitychange", () => { + if (document.visibilityState === "hidden") { + void flushSnapshot(); + } + }); +} diff --git a/src/app/lib/grouping.ts b/src/app/lib/grouping.ts index ea4e9a9a..88116dd5 100644 --- a/src/app/lib/grouping.ts +++ b/src/app/lib/grouping.ts @@ -172,3 +172,19 @@ export function detectReorderedRepos( } return moved; } + +export function groupKeys(groups: readonly { repoFullName: string }[]): string[] { + return groups.map((g) => g.repoFullName); +} + +export function indexGroups(groups: readonly G[]): Map { + return new Map(groups.map((g) => [g.repoFullName, g])); +} + +export function arraysShallowEqual(a: readonly T[], b: readonly T[]): boolean { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) return false; + } + return true; +} diff --git a/src/app/pages/PrivacyPage.tsx b/src/app/pages/PrivacyPage.tsx index e1d48a7e..5ac844ab 100644 --- a/src/app/pages/PrivacyPage.tsx +++ b/src/app/pages/PrivacyPage.tsx @@ -31,7 +31,10 @@ export default function PrivacyPage() {
  • IndexedDB — cached API responses with ETags to - reduce GitHub API usage. Cleared on logout. + reduce GitHub API usage, and a snapshot of your dashboard (issues, + pull requests, and workflow runs) so it appears instantly on reload. + Cleared on logout; the dashboard snapshot is also cleared when your + access token expires.
  • diff --git a/src/app/services/poll.ts b/src/app/services/poll.ts index d3319cdd..763dc5a0 100644 --- a/src/app/services/poll.ts +++ b/src/app/services/poll.ts @@ -3,7 +3,7 @@ import * as Sentry from "@sentry/solid"; import { getClient, fetchRateLimitDetails } from "./github"; import { fetchGitHubStatus } from "./github-status"; import { config } from "../stores/config"; -import { user, onAuthCleared, expireToken } from "../stores/auth"; +import { user, onAuthCleared, expireToken, identityEpoch } from "../stores/auth"; import { checkAndResetIfExpired } from "./api-usage"; import { fetchIssuesAndPullRequests, @@ -55,6 +55,13 @@ export interface PollCoordinator { destroy: () => void; } +export class StaleIdentityError extends Error { + constructor() { + super("Stale identity"); + this.name = "StaleIdentityError"; + } +} + // ── Hot poll state ──────────────────────────────────────────────────────────── /** PRs with running/stale/pinned/pending-grace check status: maps GraphQL node ID → databaseId */ @@ -484,6 +491,9 @@ export function createPollCoordinator( const newItems = detectNewItems(data); dispatchNotifications(newItems, config); } catch (err) { + if (err instanceof StaleIdentityError) { + return; + } const message = err instanceof Error ? err.message : "Unknown error during data fetch"; Sentry.captureException(err, { tags: { source: "poll-cycle" } }); pushError("poll", message, true); @@ -833,7 +843,8 @@ export function createHotPollCoordinator( onHotData: ( prUpdates: Map, runUpdates: Map, - generation: number + generation: number, + epoch: number ) => void, options?: { onStart?: (prDbIds: ReadonlySet, runIds: ReadonlySet) => void; @@ -893,11 +904,13 @@ export function createHotPollCoordinator( return; } + const cycleEpoch = identityEpoch(); startedCycle = true; options?.onStart?.(new Set(_hotPRs.values()), new Set(_hotRuns.keys())); try { const { prUpdates, runUpdates, generation, hadErrors } = await fetchHotData(consecutiveFailures); - if (myGeneration !== chainGeneration) return; // Chain destroyed during fetch + // Chain destroyed or identity changed during fetch + if (myGeneration !== chainGeneration || cycleEpoch !== identityEpoch()) return; if (hadErrors) { consecutiveFailures++; pushError("hot-poll", "Some status updates failed — retrying with backoff", true); @@ -906,9 +919,12 @@ export function createHotPollCoordinator( dismissNotificationBySource("hot-poll"); } if (prUpdates.size > 0 || runUpdates.size > 0) { - onHotData(prUpdates, runUpdates, generation); + onHotData(prUpdates, runUpdates, generation, cycleEpoch); } } catch (err) { + // A cycle started under a previous identity must not expire the current + // identity's token or surface errors to it. + if (myGeneration !== chainGeneration || cycleEpoch !== identityEpoch()) return; if (isUnauthorizedError(err)) { // Token invalid — same re-auth path as the full poll (poll.ts throw at // fetchAllData) and cross-tab sync (auth.ts storage listener): clear the @@ -1108,7 +1124,7 @@ export function createEventsPollCoordinator( getUsername: () => string, getTrackedRepoNames: () => Set, isFullRefreshing: () => boolean, - onTargetedData: (data: DashboardData, affectedRepos: string[]) => void, + onTargetedData: (data: DashboardData, affectedRepos: string[], epoch: number) => void, ): { destroy: () => void } { let timeoutId: ReturnType | null = null; let chainGeneration = 0; @@ -1153,6 +1169,8 @@ export function createEventsPollCoordinator( return; } + const cycleEpoch = identityEpoch(); + try { const { events, changed } = await fetchUserEvents(octokit, username); if (myGeneration !== chainGeneration) return; @@ -1194,7 +1212,7 @@ export function createEventsPollCoordinator( } const affectedRepos = [...repoSummaries.values()].map((s) => s.repoFullName); - onTargetedData(data, affectedRepos); + onTargetedData(data, affectedRepos, cycleEpoch); consecutiveFailures = 0; } catch (err) { consecutiveFailures++; diff --git a/src/app/stores/auth.ts b/src/app/stores/auth.ts index 84d6ee99..000eccdd 100644 --- a/src/app/stores/auth.ts +++ b/src/app/stores/auth.ts @@ -2,6 +2,7 @@ import { createSignal } from "solid-js"; import { z } from "zod"; import * as Sentry from "@sentry/solid"; import { clearCache } from "./cache"; +import { clearSnapshot } from "../lib/dashboard-snapshot"; import { CONFIG_STORAGE_KEY, resetConfig, updateConfig, config } from "./config"; import { VIEW_STORAGE_KEY, resetViewState } from "./view"; import { pushNotification } from "../lib/errors"; @@ -59,6 +60,18 @@ export function isAuthenticated(): boolean { export { user }; +let _identityEpoch = 0; + +/** + * Monotonically increasing counter tracking identity boundaries. + * Invariant: Every increment of _identityEpoch is followed by either the + * onAuthCleared callbacks (which reset module-level fetch flags) or a full + * page reload. Never increment on same-user token rotation. + */ +export function identityEpoch(): number { + return _identityEpoch; +} + // ── Jira auth signals ──────────────────────────────────────────────────────── const [_jiraAuth, _setJiraAuth] = createSignal( @@ -246,26 +259,36 @@ export function setAuthFromPat(patToken: string, userData: GitHubUser): void { */ export const setAuthFromCredential = setAuthFromPat; +function clearSnapshotLogged(): Promise { + return clearSnapshot().catch((err: unknown) => { + console.warn("[auth] failed to clear dashboard snapshot:", err); + }); +} + /** * Clears per-identity data: the `onAuthCleared` callback loop (poll data, - * notifications, toast dedup, dashboard cache, Jira signal) AND the IndexedDB - * dashboard cache. The callbacks run synchronously first, so a fire-and-forget - * `void clearIdentityData()` preserves `setAuthFromPat`'s historical synchronous - * callback behavior; the IndexedDB `clearCache()` is AWAITED (not - * fire-and-forget) so a caller doing `await clearIdentityData()` gets a real - * ordering guarantee — required by the pre-auth Login-page import path, where - * `setAuthFromPat`'s own cache-clear is inert (`isIdentitySwitch` needs `user()` - * non-null) and `expireToken()` never clears IndexedDB. + * notifications, toast dedup, dashboard cache, Jira signal), the IndexedDB + * dashboard snapshot, AND the IndexedDB API cache. The callbacks and the + * snapshot clear's synchronous part (cancelling pending snapshot writes) run + * before the first `await`, so a fire-and-forget `void clearIdentityData()` + * preserves `setAuthFromPat`'s historical synchronous callback behavior; both + * IndexedDB clears are AWAITED (not fire-and-forget) so a caller doing + * `await clearIdentityData()` gets a real ordering guarantee — required by the + * pre-auth Login-page import path, where `setAuthFromPat`'s own cache-clear is + * inert (`isIdentitySwitch` needs `user()` non-null) and `expireToken()` never + * clears the API cache (`clearCache`). */ export async function clearIdentityData(): Promise { localStorage.removeItem?.(DASHBOARD_STORAGE_KEY); localStorage.removeItem?.(DEP_META_STORAGE_KEY); localStorage.removeItem?.(DASHBOARD_BODIES_STORAGE_KEY); + _identityEpoch++; // Synchronous per-identity resets first (matches the ordering the identity- // switch branch has always had: callbacks complete before the awaited work). for (const cb of _onClearCallbacks) { try { cb(); } catch (e) { console.warn("[auth] onAuthCleared callback threw during identity data clear:", e); } } + const snapshotClear = clearSnapshotLogged(); // IndexedDB cache clear — AWAITED so callers can sequence work strictly after it // (prevents a new identity rendering the previous identity's cached data). try { @@ -274,6 +297,7 @@ export async function clearIdentityData(): Promise { console.warn("[auth] Cache clear failed during identity data clear:", err); Sentry.captureException(err, { tags: { source: "auth-identity-data-cache-clear" } }); } + await snapshotClear; } const _onClearCallbacks: (() => void)[] = []; @@ -302,11 +326,13 @@ export function clearAuth(): void { localStorage.removeItem(DASHBOARD_BODIES_STORAGE_KEY); _setToken(null); setUser(null); - // Clear IndexedDB cache to prevent data leakage between users + _identityEpoch++; + // Clear IndexedDB cache and snapshot to prevent data leakage between users clearCache().catch((err) => { console.warn("[auth] Cache clear failed during logout:", err); Sentry.captureException(err, { tags: { source: "auth-logout-cache-clear" } }); }); + void clearSnapshotLogged(); // Run registered cleanup callbacks (e.g., poll state reset) for (const cb of _onClearCallbacks) { try { cb(); } catch (e) { console.warn("[auth] onAuthCleared callback threw:", e); } @@ -317,8 +343,8 @@ export function clearAuth(): void { } } -/** Clear only the auth token. Preserves all user data (config, view state, dashboard - * cache) so the same user's preferences and cached data survive re-authentication. +/** Clear the auth token and dashboard snapshot/cache while preserving user + * preferences (config and view state) so they survive re-authentication. * Used when a token becomes invalid (expired PAT, revoked OAuth) — NOT for explicit * logout. Full data wipe (cross-user data isolation) is handled by clearAuth() * which is reserved for explicit user actions (Sign out, Reset all). @@ -333,6 +359,8 @@ export function expireToken(): void { localStorage.removeItem(DASHBOARD_BODIES_STORAGE_KEY); _setToken(null); setUser(null); + _identityEpoch++; + void clearSnapshotLogged(); _onClearCallbacks.forEach((cb) => { try { cb(); } catch (e) { console.warn("[auth] callback failed during expireToken:", e); } }); console.info("[auth] token expired (dashboard cache cleared)"); } @@ -485,6 +513,10 @@ function handleAuthStorage(e: StorageEvent): void { // while still rendering a previous identity's config/view/cached // data. Reload instead so it re-initializes cleanly from what the // active tab already persisted. + _identityEpoch++; + // Drop any snapshot write still pending for the previous identity + // so the reload's visibilitychange flush cannot persist it. + void clearSnapshotLogged(); crossTabReload.reloadForIdentitySwitch(); } } @@ -511,15 +543,3 @@ function handleAuthStorage(e: StorageEvent): void { if (typeof window !== "undefined") { window.addEventListener("storage", handleAuthStorage); } - -/** - * Test-only: detaches the module-scope `storage` listener registered above. - * Suites re-import this module via `vi.resetModules()`, which attaches a fresh - * listener to the shared window on each import; calling this in teardown removes - * the current instance's listener so they don't accumulate across tests. - */ -export function _resetAuthStorageListenerForTests(): void { - if (typeof window !== "undefined") { - window.removeEventListener("storage", handleAuthStorage); - } -} diff --git a/tests/acceptance/steps/dependency-exclusions.steps.tsx b/tests/acceptance/steps/dependency-exclusions.steps.tsx index 48020398..b5130865 100644 --- a/tests/acceptance/steps/dependency-exclusions.steps.tsx +++ b/tests/acceptance/steps/dependency-exclusions.steps.tsx @@ -50,6 +50,14 @@ vi.mock("../../../src/app/stores/auth", () => ({ clearJiraConfigFull: vi.fn(), ensureJiraTokenValid: vi.fn().mockResolvedValue(false), setAuthFromPat: vi.fn(), + identityEpoch: () => 0, +})); + +vi.mock("../../../src/app/lib/dashboard-snapshot", () => ({ + loadSnapshot: vi.fn().mockResolvedValue(null), + writeSnapshot: vi.fn(), + flushSnapshot: vi.fn().mockResolvedValue(undefined), + clearSnapshot: vi.fn().mockResolvedValue(undefined), })); vi.mock("@sentry/solid", () => ({ @@ -101,6 +109,12 @@ vi.mock("../../../src/app/services/poll", () => ({ seedHotSetsFromTargeted: vi.fn(), clearHotSets: vi.fn(), getHotPollGeneration: vi.fn().mockReturnValue(0), + StaleIdentityError: class StaleIdentityError extends Error { + constructor() { + super("Stale identity"); + this.name = "StaleIdentityError"; + } + }, })); vi.mock("../../../src/app/lib/notifications", () => ({ diff --git a/tests/components/DashboardPage.test.tsx b/tests/components/DashboardPage.test.tsx index d5e246c8..b1818bc3 100644 --- a/tests/components/DashboardPage.test.tsx +++ b/tests/components/DashboardPage.test.tsx @@ -25,14 +25,25 @@ vi.mock("@solidjs/router", () => ({ })); // Mock auth store — capture onAuthCleared callbacks so qa-4 can invoke them +let mockIdentityEpochValue = 0; +let mockUserValue: { login: string; avatar_url: string; name: string | null } | null = { + login: "testuser", + avatar_url: "", + name: "Test User", +}; const authClearCallbacks: (() => void)[] = []; vi.mock("../../src/app/stores/auth", () => ({ clearAuth: vi.fn(), expireToken: vi.fn(), token: () => "fake-token", - user: () => ({ login: "testuser", avatar_url: "", name: "Test User" }), + user: () => mockUserValue, isAuthenticated: () => true, - onAuthCleared: vi.fn((cb: () => void) => { authClearCallbacks.push(cb); }), + onAuthCleared: vi.fn((cb: () => void) => { + authClearCallbacks.push(() => { + mockIdentityEpochValue++; + cb(); + }); + }), DASHBOARD_STORAGE_KEY: "github-tracker:dashboard", DEP_META_STORAGE_KEY: "github-tracker:dep-meta", jiraAuth: vi.fn(() => null), @@ -40,6 +51,14 @@ vi.mock("../../src/app/stores/auth", () => ({ setJiraAuth: vi.fn(), clearJiraAuth: vi.fn(), ensureJiraTokenValid: vi.fn().mockResolvedValue(false), + identityEpoch: () => mockIdentityEpochValue, +})); + +vi.mock("../../src/app/lib/dashboard-snapshot", () => ({ + loadSnapshot: vi.fn().mockResolvedValue(null), + writeSnapshot: vi.fn(), + flushSnapshot: vi.fn().mockResolvedValue(undefined), + clearSnapshot: vi.fn().mockResolvedValue(undefined), })); vi.mock("../../src/app/services/jira-client", () => ({ @@ -100,9 +119,10 @@ let capturedOnHotData: (( prUpdates: Map, runUpdates: Map, generation: number, + epoch: number, ) => void) | null = null; // capturedOnTargetedData is populated by the createEventsPollCoordinator mock -let capturedOnTargetedData: ((data: DashboardData, affectedRepos: string[]) => void) | null = null; +let capturedOnTargetedData: ((data: DashboardData, affectedRepos: string[], epoch: number) => void) | null = null; // DashboardPage and pollService are imported dynamically after each vi.resetModules() // so the module-level _coordinator variable is always fresh (null) per test. @@ -115,10 +135,13 @@ let viewStore: typeof import("../../src/app/stores/view"); let configStore: typeof import("../../src/app/stores/config"); let githubService: typeof import("../../src/app/services/github"); let errorsLib: typeof import("../../src/app/lib/errors"); +let snapshotLib: typeof import("../../src/app/lib/dashboard-snapshot"); +let notificationsLib: typeof import("../../src/app/lib/notifications"); beforeEach(async () => { - // Clear localStorage so loadCachedDashboard doesn't pick up stale data from prior tests localStorage.clear?.(); + mockIdentityEpochValue = 0; + mockUserValue = { login: "testuser", avatar_url: "", name: "Test User" }; // Reset module registry so DashboardPage's module-level _coordinator starts as null vi.resetModules(); // Mutate in place (not reassign) to preserve the reference captured by vi.mock @@ -128,6 +151,28 @@ beforeEach(async () => { // vi.doMock (not vi.mock) is the correct API for dynamic re-registration // after vi.resetModules(). vi.mock inside beforeEach is hoisted and will // become a hard error in a future Vitest version. + vi.doMock("../../src/app/stores/auth", () => ({ + clearAuth: vi.fn(), + expireToken: vi.fn(), + token: () => "fake-token", + user: () => mockUserValue, + isAuthenticated: () => true, + onAuthCleared: vi.fn((cb: () => void) => { + authClearCallbacks.push(() => { + mockIdentityEpochValue++; + cb(); + }); + }), + DASHBOARD_STORAGE_KEY: "github-tracker:dashboard", + DEP_META_STORAGE_KEY: "github-tracker:dep-meta", + jiraAuth: vi.fn(() => null), + isJiraAuthenticated: vi.fn(() => false), + setJiraAuth: vi.fn(), + clearJiraAuth: vi.fn(), + ensureJiraTokenValid: vi.fn().mockResolvedValue(false), + identityEpoch: () => mockIdentityEpochValue, + })); + vi.doMock("../../src/app/services/poll", () => ({ fetchAllData: vi.fn().mockResolvedValue({ issues: [], @@ -165,6 +210,12 @@ beforeEach(async () => { seedHotSetsFromTargeted: vi.fn(), clearHotSets: vi.fn(), getHotPollGeneration: vi.fn().mockReturnValue(0), + StaleIdentityError: class StaleIdentityError extends Error { + constructor() { + super("Stale identity"); + this.name = "StaleIdentityError"; + } + }, })); // Re-import with fresh module instances @@ -178,6 +229,8 @@ beforeEach(async () => { configStore = await import("../../src/app/stores/config"); githubService = await import("../../src/app/services/github"); errorsLib = await import("../../src/app/lib/errors"); + snapshotLib = await import("../../src/app/lib/dashboard-snapshot"); + notificationsLib = await import("../../src/app/lib/notifications"); mockLocationReplace.mockClear(); capturedFetchAll = null; @@ -191,6 +244,14 @@ beforeEach(async () => { vi.mocked(errorsLib.pushNotification).mockClear(); vi.mocked(errorsLib.dismissNotificationBySource).mockClear(); vi.mocked(errorsLib.getNotifications).mockReturnValue([]); + vi.mocked(snapshotLib.loadSnapshot).mockReset().mockResolvedValue(null); + vi.mocked(snapshotLib.writeSnapshot).mockReset(); + vi.mocked(snapshotLib.flushSnapshot).mockReset().mockResolvedValue(undefined); + vi.mocked(snapshotLib.clearSnapshot).mockReset().mockResolvedValue(undefined); + vi.mocked(notificationsLib.detectNewItems) + .mockReset() + .mockReturnValue({ issues: [], pullRequests: [], workflowRuns: [] }); + vi.mocked(notificationsLib.dispatchNotifications).mockReset(); vi.mocked(pollService.fetchAllData).mockResolvedValue({ issues: [], pullRequests: [], @@ -656,47 +717,94 @@ describe("DashboardPage — onAuthCleared integration", () => { }); describe("DashboardPage — scroll preservation on poll refresh", () => { - // MOCK INVARIANT: fetchAllData is mocked via vi.fn() and never calls its - // onLightData callback, so phaseOneFired is always false inside pollFetch(). - // This means every poll cycle takes the withScrollLock branch (not the - // fine-grained produce() path). If fetchAllData is ever changed to invoke - // onLightData in tests, phaseOneFired will become true and withScrollLock - // will NOT be called, silently breaking this test. - // - // window.scrollTo is the correct behavioral proxy for withScrollLock: - // withScrollLock captures scrollY then calls window.scrollTo(0, y) after - // the setter. Asserting scrollTo was called with the saved position is - // equivalent to asserting withScrollLock ran and completed successfully. - it("preserves scroll position when setDashboardData replaces arrays", async () => { - const issues = [makeIssue({ id: 1, title: "Scroll test issue" })]; - vi.mocked(pollService.fetchAllData).mockResolvedValue({ - issues, - pullRequests: [], - workflowRuns: [], - errors: [], + // With keyed reconcile(items, { key: "id", merge: true }) and repo groups keyed + // by repoFullName, store writes update items in place without replacing DOM + // nodes, so scroll position is preserved without compensation. These tests assert + // that repo group and row DOM nodes survive both full-poll and targeted refreshes. + it("preserves issue row and repo group DOM nodes when a poll refresh returns fresh arrays", async () => { + viewStore.setAllExpanded("issues", true); + let pollCount = 0; + vi.mocked(pollService.fetchAllData).mockImplementation(() => { + pollCount++; + return Promise.resolve({ + issues: [ + makeIssue({ + id: 1, + number: 1, + title: "Scroll test issue", + repoFullName: "owner/repo", + comments: pollCount, + }), + ], + pullRequests: [], + workflowRuns: [], + errors: [], + }); }); render(() => ); await waitFor(() => { - screen.getByText("owner/repo"); + screen.getByText("Scroll test issue"); }); - // Simulate user scrolled down - document.documentElement.scrollTop = 500; - vi.spyOn(window, "scrollTo"); + const groupNodeBefore = document.querySelector('[data-repo-group="owner/repo"]'); + const rowNodeBefore = screen.getByText("Scroll test issue").closest('[role="listitem"]'); + expect(groupNodeBefore).not.toBeNull(); + expect(rowNodeBefore).not.toBeNull(); - // Trigger a second poll (subsequent refresh — the path that uses withScrollLock). - // phaseOneFired is false (mock never calls onLightData), so withScrollLock - // wraps the full atomic setDashboardData replacement and restores scroll. if (capturedFetchAll) { await capturedFetchAll(); } - // window.scrollTo(0, 500) is the observable side-effect of withScrollLock - // saving and restoring the pre-update scroll position. - expect(window.scrollTo).toHaveBeenCalledWith(0, 500); - vi.restoreAllMocks(); - document.documentElement.scrollTop = 0; + const groupNodeAfter = document.querySelector('[data-repo-group="owner/repo"]'); + const rowNodeAfter = screen.getByText("Scroll test issue").closest('[role="listitem"]'); + expect(groupNodeAfter).toBe(groupNodeBefore); + expect(rowNodeAfter).toBe(rowNodeBefore); + }); + + it("preserves issue row and repo group DOM nodes across a targeted refresh", async () => { + viewStore.setAllExpanded("issues", true); + vi.mocked(pollService.fetchAllData).mockImplementation(() => + Promise.resolve({ + issues: [ + makeIssue({ id: 1, number: 1, title: "Targeted issue", repoFullName: "owner/repo", comments: 1 }), + makeIssue({ id: 2, number: 2, title: "Unaffected issue", repoFullName: "owner/other", comments: 1 }), + ], + pullRequests: [], + workflowRuns: [], + errors: [], + }), + ); + + render(() => ); + await waitFor(() => { + screen.getByText("Targeted issue"); + screen.getByText("Unaffected issue"); + }); + + const targetedRowBefore = screen.getByText("Targeted issue").closest('[role="listitem"]'); + const unaffectedRowBefore = screen.getByText("Unaffected issue").closest('[role="listitem"]'); + expect(targetedRowBefore).not.toBeNull(); + expect(unaffectedRowBefore).not.toBeNull(); + + expect(capturedOnTargetedData).not.toBeNull(); + capturedOnTargetedData!( + { + issues: [ + makeIssue({ id: 1, number: 1, title: "Targeted issue", repoFullName: "owner/repo", comments: 5 }), + ], + pullRequests: [], + workflowRuns: [], + errors: [], + }, + ["owner/repo"], + mockIdentityEpochValue, + ); + + const targetedRowAfter = screen.getByText("Targeted issue").closest('[role="listitem"]'); + const unaffectedRowAfter = screen.getByText("Unaffected issue").closest('[role="listitem"]'); + expect(targetedRowAfter).toBe(targetedRowBefore); + expect(unaffectedRowAfter).toBe(unaffectedRowBefore); }); }); @@ -733,7 +841,7 @@ describe("DashboardPage — onHotData integration", () => { mergeStateStatus: "CLEAN", reviewDecision: "APPROVED" as const, }]]); - capturedOnHotData!(prUpdates, new Map(), 0); + capturedOnHotData!(prUpdates, new Map(), 0, mockIdentityEpochValue); // Expand the repo to verify the StatusDot updated await user.click(screen.getByText("owner/repo")); @@ -794,7 +902,8 @@ describe("DashboardPage — onHotData integration", () => { viewerLatestReviewState: null, }]]), new Map(), - 0 + 0, + mockIdentityEpochValue ); await waitFor(() => { @@ -816,7 +925,8 @@ describe("DashboardPage — onHotData integration", () => { viewerLatestReviewState: "APPROVED" as const, }]]), new Map(), - 0 + 0, + mockIdentityEpochValue ); await waitFor(() => { @@ -861,7 +971,7 @@ describe("DashboardPage — onHotData integration", () => { mergeStateStatus: "CLEAN", reviewDecision: null, }]]); - capturedOnHotData!(prUpdates, new Map(), 999); + capturedOnHotData!(prUpdates, new Map(), 999, mockIdentityEpochValue); // PR should still show pending — stale update was discarded expect(screen.getByLabelText("Checks in progress")).toBeTruthy(); @@ -905,7 +1015,7 @@ describe("DashboardPage — onHotData integration", () => { updatedAt: "2026-03-29T12:00:00Z", completedAt: "2026-03-29T12:00:00Z", }]]); - capturedOnHotData!(new Map(), runUpdates, 0); + capturedOnHotData!(new Map(), runUpdates, 0, mockIdentityEpochValue); // The store was mutated — the collapsed summary still shows "1 workflow" // (the run count doesn't change, only the status), confirming the @@ -944,7 +1054,7 @@ describe("DashboardPage — onHotData integration", () => { mergeStateStatus: "CLEAN", reviewDecision: null, }]]); - capturedOnHotData!(prUpdates, new Map(), 0); + capturedOnHotData!(prUpdates, new Map(), 0, mockIdentityEpochValue); await waitFor(() => { expect(screen.queryByText("1 PR")).toBeNull(); @@ -1859,7 +1969,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); await waitFor(() => { screen.getByText("2 issues"); @@ -1885,7 +1995,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); await waitFor(() => { screen.getByText("1 issue"); @@ -1915,7 +2025,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); // handleTargetedData mutates data items in-place before merging into the store expect(targetedPR.surfacedBy).toEqual(expect.arrayContaining(["primary", "tracked-user"])); @@ -1943,7 +2053,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); expect(vi.mocked(notifLib.detectNewItems)).toHaveBeenCalledWith(targetedData); expect(vi.mocked(notifLib.dispatchNotifications)).toHaveBeenCalled(); @@ -1968,7 +2078,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); expect(vi.mocked(pollService.seedHotSetsFromTargeted)).toHaveBeenCalledWith(targetedData); }); @@ -1997,7 +2107,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); // seedHotSetsFromTargeted is called (additive), NOT rebuildHotSets (full replacement) expect(vi.mocked(pollService.rebuildHotSets)).not.toHaveBeenCalled(); @@ -2024,7 +2134,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); await waitFor(() => { screen.getByText("2 issues"); @@ -2050,7 +2160,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); await waitFor(() => { screen.getByText("1 issue"); @@ -2080,7 +2190,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); // handleTargetedData mutates data items in-place before merging into the store expect(targetedPR.surfacedBy).toEqual(expect.arrayContaining(["primary", "tracked-user"])); @@ -2108,7 +2218,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); expect(vi.mocked(notifLib.detectNewItems)).toHaveBeenCalledWith(targetedData); expect(vi.mocked(notifLib.dispatchNotifications)).toHaveBeenCalled(); @@ -2133,7 +2243,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); expect(vi.mocked(pollService.seedHotSetsFromTargeted)).toHaveBeenCalledWith(targetedData); }); @@ -2162,7 +2272,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); // seedHotSetsFromTargeted is called (additive), NOT rebuildHotSets (full replacement) expect(vi.mocked(pollService.rebuildHotSets)).not.toHaveBeenCalled(); @@ -2206,7 +2316,7 @@ describe("DashboardPage — events poll targeted merge", () => { workflowRuns: [], errors: [], }; - capturedOnTargetedData?.(targetedData, ["org/repo"]); + capturedOnTargetedData?.(targetedData, ["org/repo"], mockIdentityEpochValue); expect(unenrichedTargetedPR.enriched).toBe(true); expect(unenrichedTargetedPR.autoMergeMethod).toBe("SQUASH"); @@ -2754,6 +2864,7 @@ describe("DashboardPage — pruneJiraCustomOrder on refresh", () => { setJiraAuth: vi.fn(), clearJiraAuth: vi.fn(), ensureJiraTokenValid: vi.fn().mockResolvedValue(true), + identityEpoch: () => 0, })); const MockJiraClient = vi.fn(function (this: Record) { @@ -3311,6 +3422,7 @@ describe("DashboardPage — Jira expiration UX and reactive re-authentication", setJiraAuth: vi.fn(), clearJiraAuth: mockClearJiraAuth, ensureJiraTokenValid: vi.fn().mockResolvedValue(true), + identityEpoch: () => 0, })); class MockJiraApiError extends Error { @@ -3418,6 +3530,7 @@ describe("DashboardPage — Jira expiration UX and reactive re-authentication", setJiraAuth: vi.fn(), clearJiraAuth: mockClearJiraAuth, ensureJiraTokenValid: vi.fn().mockResolvedValue(false), + identityEpoch: () => 0, })); const mockSearchJql = vi.fn().mockRejectedValue(new Error("Jira auth cleared during token refresh")); @@ -3540,6 +3653,7 @@ describe("DashboardPage — Jira expiration UX and reactive re-authentication", setJiraAuth: vi.fn(), clearJiraAuth: vi.fn(), ensureJiraTokenValid: vi.fn().mockResolvedValue(true), + identityEpoch: () => 0, })); const mockSearchJql = vi.fn().mockResolvedValue({ @@ -3919,7 +4033,8 @@ describe("DashboardPage — PR approval integration", () => { viewerReviewRequested: true, }]]), new Map(), - 0 + 0, + mockIdentityEpochValue ); expect(screen.getByText("Approved")).toBeTruthy(); expect(screen.queryByLabelText("Approve #42 Fix race condition")).toBeNull(); @@ -3927,7 +4042,8 @@ describe("DashboardPage — PR approval integration", () => { // 2. Subsequent targeted poll with stale review state on the same headSha capturedOnTargetedData!( { issues: [], pullRequests: [makeStalePR()], workflowRuns: [], errors: [] }, - ["owner/repo"] + ["owner/repo"], + mockIdentityEpochValue ); expect(screen.getByText("Approved")).toBeTruthy(); expect(screen.queryByLabelText("Approve #42 Fix race condition")).toBeNull(); @@ -4089,7 +4205,8 @@ describe("DashboardPage — PR approval integration", () => { viewerReviewRequested: true, }]]), new Map(), - 0 + 0, + mockIdentityEpochValue ); resolveMutation({ @@ -4492,7 +4609,8 @@ describe("DashboardPage — PR approval integration", () => { ], ]), new Map(), - 0 + 0, + mockIdentityEpochValue ); // Retry approval after checkStatusStale is cleared -> succeeds @@ -4602,7 +4720,8 @@ describe("DashboardPage — Mark PR ready integration", () => { reviewDecision: null, }]]), new Map(), - 0 + 0, + mockIdentityEpochValue ); expect(screen.queryByTestId("draft-badge")).toBeNull(); expect(screen.getByText("Review needed")).toBeTruthy(); @@ -4611,7 +4730,8 @@ describe("DashboardPage — Mark PR ready integration", () => { vi.mocked(pollService.seedHotSetsFromTargeted).mockClear(); capturedOnTargetedData!( { issues: [], pullRequests: [makeStaleDraftPR()], workflowRuns: [], errors: [] }, - ["owner/repo"] + ["owner/repo"], + mockIdentityEpochValue ); expect(screen.queryByTestId("draft-badge")).toBeNull(); expect(screen.getByText("Review needed")).toBeTruthy(); @@ -4667,7 +4787,8 @@ describe("DashboardPage — Mark PR ready integration", () => { reviewDecision: "APPROVED", }]]), new Map(), - 0 + 0, + mockIdentityEpochValue ); expect(screen.queryByTestId("draft-badge")).toBeNull(); expect(screen.getByText("Approved")).toBeTruthy(); @@ -4969,19 +5090,67 @@ describe("DashboardPage — Mark PR ready integration", () => { // ── Fallback PR Enrichment Propagation ─────────────────────────────────────── -describe("DashboardPage — PR mutation enrichment cache eviction", () => { - const validSha = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; - const approveResponse = { - addPullRequestReview: { - pullRequestReview: { state: "APPROVED", pullRequest: { reviewDecision: "APPROVED" } }, - }, - rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-04-20T00:00:00Z" }, - }; - const markReadyResponse = { - markPullRequestReadyForReview: { pullRequest: { isDraft: false, reviewDecision: "REVIEW_REQUIRED" } }, - rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-04-20T00:00:00Z" }, - }; +// ── Shared PR mutation fixtures ────────────────────────────────────────────── + +const mutationValidSha = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; +const approveResponse = { + addPullRequestReview: { + pullRequestReview: { state: "APPROVED", pullRequest: { reviewDecision: "APPROVED" } }, + }, + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-04-20T00:00:00Z" }, +}; +const markReadyResponse = { + markPullRequestReadyForReview: { pullRequest: { isDraft: false, reviewDecision: "REVIEW_REQUIRED" } }, + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2026-04-20T00:00:00Z" }, +}; + +async function renderWithPR(pr: ReturnType) { + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [], + pullRequests: [pr], + workflowRuns: [], + errors: [], + }); + const user = userEvent.setup(); + render(() => ); + await user.click(screen.getByText("Pull Requests")); + await waitFor(() => screen.getByText("owner/repo")); + await user.click(screen.getByText("owner/repo")); + return user; +} + +const makeApprovablePR = () => + makePullRequest({ + id: 42, + number: 42, + title: "Fix race condition", + repoFullName: "owner/repo", + userLogin: "otheruser", + state: "OPEN", + draft: false, + enriched: true, + nodeId: "PR_kwDOtest0042", + headSha: mutationValidSha, + reviewDecision: "REVIEW_REQUIRED", + checkStatus: "success", + }); + +const makeDraftPR = () => + makePullRequest({ + id: 77, + number: 77, + title: "Ship draft feature", + repoFullName: "owner/repo", + userLogin: "testuser", + state: "OPEN", + draft: true, + enriched: true, + nodeId: "PR_kwDOtest0077", + reviewDecision: null, + checkStatus: "success", + }); +describe("DashboardPage — PR mutation enrichment cache eviction", () => { // DashboardPage imports the real api module (fresh per vi.resetModules in beforeEach); // fetchAllData is mocked, so the cache itself is exercised in api.test.ts. async function spyOnEvict() { @@ -4989,52 +5158,6 @@ describe("DashboardPage — PR mutation enrichment cache eviction", () => { return vi.spyOn(apiModule, "evictPREnrichmentCacheNode"); } - async function renderWithPR(pr: ReturnType) { - vi.mocked(pollService.fetchAllData).mockResolvedValue({ - issues: [], - pullRequests: [pr], - workflowRuns: [], - errors: [], - }); - const user = userEvent.setup(); - render(() => ); - await user.click(screen.getByText("Pull Requests")); - await waitFor(() => screen.getByText("owner/repo")); - await user.click(screen.getByText("owner/repo")); - return user; - } - - const makeApprovablePR = () => - makePullRequest({ - id: 42, - number: 42, - title: "Fix race condition", - repoFullName: "owner/repo", - userLogin: "otheruser", - state: "OPEN", - draft: false, - enriched: true, - nodeId: "PR_kwDOtest0042", - headSha: validSha, - reviewDecision: "REVIEW_REQUIRED", - checkStatus: "success", - }); - - const makeDraftPR = () => - makePullRequest({ - id: 77, - number: 77, - title: "Ship draft feature", - repoFullName: "owner/repo", - userLogin: "testuser", - state: "OPEN", - draft: true, - enriched: true, - nodeId: "PR_kwDOtest0077", - reviewDecision: null, - checkStatus: "success", - }); - it("evicts the PR's enrichment cache entry after a successful approval", async () => { const evictSpy = await spyOnEvict(); const graphqlMock = vi.fn().mockResolvedValue(approveResponse); @@ -5146,8 +5269,8 @@ describe("DashboardPage — PR mutation enrichment cache eviction", () => { }); }); -describe("DashboardPage — fallbackToPreviousEnrichment propagation to hot sets and localStorage", () => { - it("retains fallback-merged enrichment in rebuildHotSets and localStorage when a second poll cycle returns enriched: false on a previously-enriched PR", async () => { +describe("DashboardPage — fallbackToPreviousEnrichment propagation to hot sets and the dashboard snapshot", () => { + it("retains fallback-merged enrichment in rebuildHotSets and the immediate writeSnapshot when a second poll cycle returns enriched: false on a previously-enriched PR", async () => { const enrichedPR = makePullRequest({ id: 501, number: 501, @@ -5179,11 +5302,11 @@ describe("DashboardPage — fallbackToPreviousEnrichment propagation to hot sets expect(pollService.rebuildHotSets).toHaveBeenCalledTimes(1); }); await waitFor(() => { - expect(localStorage.getItem("github-tracker:dashboard")).not.toBeNull(); + expect(snapshotLib.writeSnapshot).toHaveBeenCalledTimes(1); }); vi.mocked(pollService.rebuildHotSets).mockClear(); - localStorage.removeItem("github-tracker:dashboard"); + vi.mocked(snapshotLib.writeSnapshot).mockClear(); // Second poll cycle: prEnrichment batch fails and returns enriched: false const unenrichedPR = makePullRequest({ @@ -5230,19 +5353,23 @@ describe("DashboardPage — fallbackToPreviousEnrichment propagation to hot sets ); await waitFor(() => { - const raw = localStorage.getItem("github-tracker:dashboard"); - expect(raw).not.toBeNull(); - const parsed = JSON.parse(raw!) as { pullRequests: Array> }; - expect(parsed.pullRequests).toHaveLength(1); - expect(parsed.pullRequests[0]).toMatchObject({ - id: 501, - nodeId: "PR_kwDOtest0501", - enriched: true, - checkStatus: "running", - autoMergeMethod: "SQUASH", - assigneeLogins: ["alice"], - reviewerLogins: ["bob"], - }); + expect(snapshotLib.writeSnapshot).toHaveBeenCalledWith( + expect.objectContaining({ + pullRequests: [ + expect.objectContaining({ + id: 501, + nodeId: "PR_kwDOtest0501", + enriched: true, + checkStatus: "running", + autoMergeMethod: "SQUASH", + assigneeLogins: ["alice"], + reviewerLogins: ["bob"], + }), + ], + }), + "testuser", + "immediate", + ); }); }); }); @@ -5759,8 +5886,15 @@ describe("DashboardPage — dashboardBodies caching and cooldown", () => { const { unmount } = render(() => ); await waitFor(() => expect(graphqlSpy1).toHaveBeenCalledTimes(1)); await waitFor(() => { - // Wait for both the deferred setTimeout write of github-tracker:dashboard and github-tracker:dashboard-bodies - expect(localStorage.getItem("github-tracker:dashboard")).not.toBeNull(); + // Wait for both writeSnapshot and github-tracker:dashboard-bodies persistence + expect(snapshotLib.writeSnapshot).toHaveBeenCalledWith( + expect.objectContaining({ + issues: [expect.objectContaining({ nodeId: "DASH_PERSIST_1" })], + pullRequests: [expect.objectContaining({ id: 901 })], + }), + "testuser", + "immediate", + ); const storedRaw = localStorage.getItem("github-tracker:dashboard-bodies"); expect(storedRaw).not.toBeNull(); const stored = JSON.parse(storedRaw!) as Record }>; @@ -5771,7 +5905,7 @@ describe("DashboardPage — dashboardBodies caching and cooldown", () => { }); unmount(); - // Simulate reload: reset modules while keeping localStorage intact (both dashboard cache and dashboard-bodies cache) + // Simulate reload: reset modules while keeping localStorage intact and seeding loadSnapshot vi.resetModules(); authClearCallbacks.length = 0; @@ -5806,8 +5940,19 @@ describe("DashboardPage — dashboardBodies caching and cooldown", () => { seedHotSetsFromTargeted: vi.fn(), clearHotSets: vi.fn(), getHotPollGeneration: vi.fn().mockReturnValue(0), + StaleIdentityError: class StaleIdentityError extends Error {}, })); + const freshSnapshot = await import("../../src/app/lib/dashboard-snapshot"); + vi.mocked(freshSnapshot.loadSnapshot).mockResolvedValue({ + _v: 1, + login: "testuser", + savedAt: Date.now(), + issues: [dashIssue], + pullRequests: [depPRInRepo()], + workflowRuns: [], + lastRefreshedAt: new Date("2026-04-01T00:00:00Z"), + }); const freshGithub = await import("../../src/app/services/github"); vi.mocked(freshGithub.getClient).mockReturnValue( { graphql: graphqlSpy2 } as unknown as ReturnType @@ -5815,9 +5960,10 @@ describe("DashboardPage — dashboardBodies caching and cooldown", () => { const freshDashMod = await import("../../src/app/components/dashboard/DashboardPage"); render(() => ); + await waitFor(() => expect(screen.getByRole("tab", { name: /Dependencies/ })).toBeDefined()); await user.click(screen.getByRole("tab", { name: /Dependencies/ })); // Pre-refresh assertion: before fetchAllData resolves (and before the { defer: true } dashboardBodies effect runs), - // seedCachedDashboardBodies() has already hydrated abandonedDepsMap and dashboardIssueUrls at module load + // seedCachedDashboardBodies() has already hydrated abandonedDepsMap and dashboardIssueUrls when loadSnapshot resolved in onMount await waitFor(() => { expect(screen.getByText(/abandoned/i)).toBeDefined(); }); @@ -6146,28 +6292,6 @@ describe("DashboardPage — dashboardBodies caching and cooldown", () => { it("suppresses the Dependencies 'Checking status' badge while the dashboard is still loading from a cached snapshot", async () => { const user = userEvent.setup(); - // Snapshot without lastRefreshedAt: pollFetch keeps `loading: true` until the first fetch resolves - localStorage.setItem( - "github-tracker:dashboard", - JSON.stringify({ - _v: 2, - issues: [], - pullRequests: [ - makePullRequest({ - id: 970, - nodeId: "PR_DEP_970", - repoFullName: "owner/repo", - title: "Bump lodash from 4.17.20 to 4.17.21", - userLogin: "dependabot[bot]", - headRef: "dependabot/npm_and_yarn/lodash-4.17.21", - enriched: false, - checkStatus: null, - updatedAt: new Date().toISOString(), - }), - ], - workflowRuns: [], - }) - ); vi.resetModules(); vi.doMock("../../src/app/services/poll", () => ({ @@ -6175,7 +6299,7 @@ describe("DashboardPage — dashboardBodies caching and cooldown", () => { createPollCoordinator: vi.fn().mockImplementation((_getInterval: unknown, fetchAll: () => Promise) => { void fetchAll().catch(() => {}); return { - isRefreshing: () => false, + isRefreshing: () => true, lastRefreshAt: () => null, manualRefresh: vi.fn(), destroy: vi.fn(), @@ -6187,10 +6311,34 @@ describe("DashboardPage — dashboardBodies caching and cooldown", () => { seedHotSetsFromTargeted: vi.fn(), clearHotSets: vi.fn(), getHotPollGeneration: vi.fn().mockReturnValue(0), + StaleIdentityError: class StaleIdentityError extends Error {}, })); + const freshSnapshot = await import("../../src/app/lib/dashboard-snapshot"); + vi.mocked(freshSnapshot.loadSnapshot).mockResolvedValue({ + _v: 1, + login: "testuser", + savedAt: Date.now(), + issues: [], + pullRequests: [ + makePullRequest({ + id: 970, + nodeId: "PR_DEP_970", + repoFullName: "owner/repo", + title: "Bump lodash from 4.17.20 to 4.17.21", + userLogin: "dependabot[bot]", + headRef: "dependabot/npm_and_yarn/lodash-4.17.21", + enriched: false, + checkStatus: null, + updatedAt: new Date().toISOString(), + }), + ], + workflowRuns: [], + lastRefreshedAt: null, + }); const freshDashMod = await import("../../src/app/components/dashboard/DashboardPage"); render(() => ); + await waitFor(() => expect(screen.getByRole("tab", { name: /Dependencies/ })).toBeDefined()); await user.click(screen.getByRole("tab", { name: /Dependencies/ })); await waitFor(() => expect(document.getElementById("dep-group-needs-action")).not.toBeNull()); expect(screen.queryByText("Checking status")).toBeNull(); @@ -6198,7 +6346,7 @@ describe("DashboardPage — dashboardBodies caching and cooldown", () => { }); describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale restoration, and partial-sync indicator", () => { - it("returns resolvedPullRequests with enriched: true from pollFetch on cached backfill failure and suppresses Tier 0 targeted backfill errors while escalating Tier 1 with silentToast: true", async () => { + it("returns fallback-merged pullRequests with enriched: true from pollFetch on cached backfill failure and suppresses Tier 0 targeted backfill errors while escalating Tier 1 with silentToast: true", async () => { const enrichedPR = makePullRequest({ id: 601, number: 601, @@ -6264,7 +6412,8 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r workflowRuns: [], errors: [backfillError], }, - ["owner/repo"] + ["owner/repo"], + mockIdentityEpochValue ); expect(errorsLib.pushError).not.toHaveBeenCalledWith( "backfill-batch-1/1", @@ -6296,7 +6445,8 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r workflowRuns: [], errors: [backfillError], }, - ["owner/repo"] + ["owner/repo"], + mockIdentityEpochValue ); expect(errorsLib.pushError).not.toHaveBeenCalledWith( "backfill-batch-1/1", @@ -6453,7 +6603,8 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r }, ], }, - ["owner/repo"] + ["owner/repo"], + mockIdentityEpochValue ); expect(staleTargetedPR.checkStatus).toBe("failure"); @@ -6494,7 +6645,8 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r }, ], }, - ["owner/repo"] + ["owner/repo"], + mockIdentityEpochValue ); expect(errorsLib.pushNotification).toHaveBeenCalledWith( "pr-enrichment", @@ -6524,7 +6676,8 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r ], ]), new Map(), - 0 + 0, + mockIdentityEpochValue ); // Subsequent clean targeted refresh dismisses 'pr-enrichment' @@ -6545,12 +6698,13 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r workflowRuns: [], errors: [], }, - ["owner/repo"] + ["owner/repo"], + mockIdentityEpochValue ); expect(errorsLib.dismissNotificationBySource).toHaveBeenCalledWith("pr-enrichment"); }); - it("clears stale checkStatus to null and keeps checkStatusStale: true when headSha changes across a degraded pollFetch (phaseOneFired branch) or handleTargetedData", async () => { + it("clears stale checkStatus to null and keeps checkStatusStale: true when headSha changes across a degraded pollFetch after a phase-1 light commit or across handleTargetedData", async () => { const oldSha = "1111111111111111111111111111111111111111"; const newSha = "2222222222222222222222222222222222222222"; const thirdSha = "3333333333333333333333333333333333333333"; @@ -6633,7 +6787,8 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r ], ]), new Map(), - 0 + 0, + mockIdentityEpochValue ); // Degraded handleTargetedData with a new commit (thirdSha) and checkStatusStale: true @@ -6657,7 +6812,8 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r workflowRuns: [], errors: [], }, - ["owner/repo"] + ["owner/repo"], + mockIdentityEpochValue ); expect(targetedStaleNewCommit.checkStatus).toBeNull(); @@ -6763,7 +6919,8 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r workflowRuns: [], errors: [], }, - ["owner/repo"] + ["owner/repo"], + mockIdentityEpochValue ); expect(targetedRegressedPR.enriched).toBe(true); @@ -6792,7 +6949,8 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r ], ]), new Map(), - 0 + 0, + mockIdentityEpochValue ); const reApproveBtn = await waitFor(() => @@ -6855,3 +7013,781 @@ describe("DashboardPage — 3-Tier backfill-batch-* handling, checkStatusStale r ); }); }); + +// ── Identity-scoped dashboard snapshot and identityEpoch guards ───────────── + +describe("DashboardPage — identity-scoped snapshot and identityEpoch guards", () => { + // Wires createPollCoordinator to run fetchAll once like the real doFetch: notify on + // success, swallow StaleIdentityError silently, and surface any other error. + function mockCoordinatorRunOnce(): { caughtError: () => unknown } { + let caughtError: unknown = null; + vi.mocked(pollService.createPollCoordinator).mockImplementation((_getInterval, fetchAll) => { + capturedFetchAll = fetchAll; + void (async () => { + try { + const data = await fetchAll(); + const newItems = notificationsLib.detectNewItems(data); + notificationsLib.dispatchNotifications(newItems, configStore.config); + } catch (err) { + caughtError = err; + if (err instanceof pollService.StaleIdentityError) return; + errorsLib.pushError("poll", err instanceof Error ? err.message : "Unknown error", true); + } + })(); + return { + isRefreshing: () => false, + lastRefreshAt: () => null, + manualRefresh: vi.fn(), + destroy: vi.fn(), + }; + }); + return { caughtError: () => caughtError }; + } + + it("does not apply a cached snapshot belonging to alice when current user() is bob", async () => { + mockUserValue = { login: "bob", avatar_url: "", name: "Bob" }; + // Simulate loadSnapshot's login check returning null when stored record is for 'alice' + const storedForAlice = { + _v: 1 as const, + login: "alice", + savedAt: Date.now(), + issues: [makeIssue({ id: 701, title: "Alice private issue", repoFullName: "alice/secret" })], + pullRequests: [], + workflowRuns: [], + lastRefreshedAt: new Date("2026-10-30T10:00:00Z"), + }; + vi.mocked(snapshotLib.loadSnapshot).mockImplementation(async (login: string) => + storedForAlice.login.toLowerCase() === login.toLowerCase() ? storedForAlice : null, + ); + // Keep initial poll pending so only snapshot hydration could populate the store + vi.mocked(pollService.fetchAllData).mockImplementation(() => new Promise(() => {})); + + render(() => ); + await waitFor(() => expect(snapshotLib.loadSnapshot).toHaveBeenCalledWith("bob")); + await new Promise((resolve) => setTimeout(resolve, 30)); + + expect(screen.queryByText("alice/secret")).toBeNull(); + const issuesTab = screen.getByRole("tab", { name: /^Issues/ }); + expect(issuesTab.textContent?.replace(/\D+/g, "")).toBe("0"); + }); + + it("discards snapshot result if it resolves after phase-1 onLightData has already committed real data", async () => { + let resolveSnapshot!: (snap: import("../../src/app/lib/dashboard-snapshot").DashboardSnapshot | null) => void; + vi.mocked(snapshotLib.loadSnapshot).mockImplementation( + () => + new Promise((resolve) => { + resolveSnapshot = resolve; + }), + ); + + vi.mocked(pollService.fetchAllData).mockImplementation((onLightData) => { + onLightData?.({ + issues: [makeIssue({ id: 702, title: "Fresh Phase 1 Issue", repoFullName: "owner/fresh-repo" })], + pullRequests: [], + workflowRuns: [], + errors: [], + }); + return new Promise(() => {}); + }); + + render(() => ); + await waitFor(() => expect(screen.getByText("owner/fresh-repo")).toBeDefined()); + + // Now resolve the slower snapshot read with stale data — must be ignored because realDataCommitted is true + resolveSnapshot({ + _v: 1, + login: "testuser", + savedAt: Date.now() - 60_000, + issues: [makeIssue({ id: 703, title: "Stale Snapshot Issue", repoFullName: "owner/stale-repo" })], + pullRequests: [], + workflowRuns: [], + lastRefreshedAt: new Date(Date.now() - 60_000), + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + + expect(screen.getByText("owner/fresh-repo")).toBeDefined(); + expect(screen.queryByText("owner/stale-repo")).toBeNull(); + }); + + it("does not call loadSnapshot on Settings -> Dashboard remount when data is already present", async () => { + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [makeIssue({ id: 704, title: "Committed Issue", repoFullName: "owner/committed" })], + pullRequests: [], + workflowRuns: [], + errors: [], + }); + + const { unmount } = render(() => ); + await waitFor(() => expect(screen.getByText("owner/committed")).toBeDefined()); + expect(snapshotLib.loadSnapshot).toHaveBeenCalledTimes(1); + + // Unmount (navigate to Settings) and remount DashboardPage without resetting module state + unmount(); + vi.mocked(snapshotLib.loadSnapshot).mockClear(); + + render(() => ); + expect(screen.getByText("owner/committed")).toBeDefined(); + expect(snapshotLib.loadSnapshot).not.toHaveBeenCalled(); + }); + + it("throws StaleIdentityError when a poll started before logout resolves after identity clear + new user, leaving store empty, skipping writeSnapshot/notifications/errors, and not touching loading while the new identity's poll is in flight", async () => { + const pollResolvers: Array<(data: DashboardData) => void> = []; + vi.mocked(pollService.fetchAllData).mockImplementation( + () => + new Promise((resolve) => { + pollResolvers.push(resolve); + }), + ); + + const coordinator = mockCoordinatorRunOnce(); + + render(() => ); + await waitFor(() => expect(pollResolvers).toHaveLength(1)); + + // Simulate clearIdentityData() + switching to a new user while poll is in flight + for (const cb of authClearCallbacks) cb(); + mockUserValue = { login: "newuser", avatar_url: "", name: "New User" }; + vi.mocked(snapshotLib.writeSnapshot).mockClear(); + + // The new identity's first poll starts and stays in flight + void capturedFetchAll!().catch(() => {}); + await waitFor(() => expect(pollResolvers).toHaveLength(2)); + + // Resolve the old user's in-flight poll + pollResolvers[0]({ + issues: [makeIssue({ id: 705, title: "Old user issue", repoFullName: "olduser/private" })], + pullRequests: [], + workflowRuns: [], + errors: [], + }); + await waitFor(() => expect(coordinator.caughtError()).not.toBeNull()); + + expect(coordinator.caughtError()).toBeInstanceOf(pollService.StaleIdentityError); + expect(screen.queryByText("olduser/private")).toBeNull(); + expect(snapshotLib.writeSnapshot).not.toHaveBeenCalled(); + expect(notificationsLib.dispatchNotifications).not.toHaveBeenCalled(); + expect(errorsLib.pushError).not.toHaveBeenCalled(); + expect(errorsLib.pushNotification).not.toHaveBeenCalled(); + // The skeleton stays up for the new identity's in-flight poll instead of the empty state + expect(screen.getByRole("status", { name: "Loading issues" })).toBeDefined(); + expect(screen.queryByText(/No open issues/i)).toBeNull(); + }); + + it("treats a 401 from a poll started under a previous identity as stale instead of expiring the current token, without touching loading", async () => { + const pollRejecters: Array<(err: unknown) => void> = []; + vi.mocked(pollService.fetchAllData).mockImplementation( + () => + new Promise((_resolve, reject) => { + pollRejecters.push(reject); + }), + ); + const coordinator = mockCoordinatorRunOnce(); + + render(() => ); + await waitFor(() => expect(pollRejecters).toHaveLength(1)); + + for (const cb of authClearCallbacks) cb(); + mockUserValue = { login: "newuser", avatar_url: "", name: "New User" }; + + // The new identity's first poll starts and stays in flight + void capturedFetchAll!().catch(() => {}); + await waitFor(() => expect(pollRejecters).toHaveLength(2)); + + pollRejecters[0](Object.assign(new Error("Bad credentials"), { status: 401 })); + await waitFor(() => expect(coordinator.caughtError()).not.toBeNull()); + + expect(coordinator.caughtError()).toBeInstanceOf(pollService.StaleIdentityError); + expect(authStore.expireToken).not.toHaveBeenCalled(); + expect(mockLocationReplace).not.toHaveBeenCalled(); + expect(errorsLib.pushError).not.toHaveBeenCalled(); + expect(screen.getByRole("status", { name: "Loading issues" })).toBeDefined(); + expect(screen.queryByText(/No open issues/i)).toBeNull(); + }); + + it("drops a targeted refresh whose events cycle started before clearAuth", async () => { + render(() => ); + await waitFor(() => expect(capturedOnTargetedData).not.toBeNull()); + vi.mocked(snapshotLib.writeSnapshot).mockClear(); + + // Events cycle captures epoch BEFORE clearAuth runs + const staleCycleEpoch = authStore.identityEpoch(); + + // Auth cleared before targeted data arrives + for (const cb of authClearCallbacks) cb(); + + capturedOnTargetedData!( + { + issues: [makeIssue({ id: 706, title: "Stale targeted issue", repoFullName: "owner/stale-targeted" })], + pullRequests: [], + workflowRuns: [], + errors: [], + }, + ["owner/stale-targeted"], + staleCycleEpoch, + ); + + expect(screen.queryByText("owner/stale-targeted")).toBeNull(); + expect(snapshotLib.writeSnapshot).not.toHaveBeenCalled(); + }); + + it("does not write snapshot or commit when PR approval started before logout resolves after logout", async () => { + const user = userEvent.setup(); + const validSha = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + let resolveApprove!: (val: unknown) => void; + const graphqlMock = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + resolveApprove = resolve; + }), + ); + vi.mocked(githubService.getClient).mockReturnValue( + { graphql: graphqlMock } as unknown as ReturnType, + ); + + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 707, + number: 707, + title: "PR to approve before logout", + repoFullName: "owner/repo", + userLogin: "otheruser", + state: "OPEN", + draft: false, + enriched: true, + nodeId: "PR_kwDO707", + headSha: validSha, + reviewDecision: "REVIEW_REQUIRED", + checkStatus: "success", + }), + ], + workflowRuns: [], + errors: [], + }); + + render(() => ); + await user.click(screen.getByText("Pull Requests")); + await waitFor(() => screen.getByText("owner/repo")); + await user.click(screen.getByText("owner/repo")); + + const approveBtn = await waitFor(() => + screen.getByLabelText("Approve #707 PR to approve before logout"), + ); + await user.click(approveBtn); + await user.click(approveBtn); + await waitFor(() => expect(graphqlMock).toHaveBeenCalledOnce()); + + vi.mocked(snapshotLib.writeSnapshot).mockClear(); + + // Logout occurs while approvePullRequest is in flight + for (const cb of authClearCallbacks) cb(); + + // Now resolve the in-flight approval + resolveApprove({ + addPullRequestReview: { + pullRequestReview: { + state: "APPROVED", + pullRequest: { reviewDecision: "APPROVED" }, + }, + }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + + expect(snapshotLib.writeSnapshot).not.toHaveBeenCalled(); + }); + + it("preserves optimistic approval across pollFetch when updatedAt advances on the same headSha while search index lags on reviewDecision", async () => { + const user = userEvent.setup(); + const validSha = "c1c2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + const graphqlMock = vi.fn().mockResolvedValue({ + addPullRequestReview: { + pullRequestReview: { + state: "APPROVED", + pullRequest: { reviewDecision: "APPROVED" }, + }, + }, + }); + vi.mocked(githubService.getClient).mockReturnValue( + { graphql: graphqlMock } as unknown as ReturnType, + ); + + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 712, + number: 712, + title: "PR with same-SHA updatedAt bump after approval", + repoFullName: "owner/repo", + userLogin: "otheruser", + state: "OPEN", + draft: false, + enriched: true, + nodeId: "PR_kwDO712", + headSha: validSha, + updatedAt: "2026-10-30T10:00:00Z", + reviewDecision: "REVIEW_REQUIRED", + checkStatus: "success", + }), + ], + workflowRuns: [], + errors: [], + }); + + render(() => ); + await user.click(screen.getByText("Pull Requests")); + await waitFor(() => screen.getByText("owner/repo")); + await user.click(screen.getByText("owner/repo")); + + const approveBtn = await waitFor(() => + screen.getByLabelText("Approve #712 PR with same-SHA updatedAt bump after approval"), + ); + await user.click(approveBtn); + await user.click(approveBtn); + await waitFor(() => expect(graphqlMock).toHaveBeenCalledOnce()); + + // Subsequent poll returns the same headSha with a newer updatedAt (from the approval itself) + // while GitHub's search index still lags at REVIEW_REQUIRED + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 712, + number: 712, + title: "PR with same-SHA updatedAt bump after approval", + repoFullName: "owner/repo", + userLogin: "otheruser", + state: "OPEN", + draft: false, + enriched: true, + nodeId: "PR_kwDO712", + headSha: validSha, + updatedAt: "2026-10-30T10:01:00Z", + reviewDecision: "REVIEW_REQUIRED", + checkStatus: "success", + }), + ], + workflowRuns: [], + errors: [], + }); + + await capturedFetchAll!(); + + // The Approve button must stay hidden because headSha did not change + expect( + screen.queryByLabelText("Approve #712 PR with same-SHA updatedAt bump after approval"), + ).toBeNull(); + }); + + it("does not write snapshot or commit when mark-PR-ready started before logout resolves after logout", async () => { + const user = userEvent.setup(); + let resolveMarkReady!: (val: unknown) => void; + const graphqlMock = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + resolveMarkReady = resolve; + }), + ); + vi.mocked(githubService.getClient).mockReturnValue( + { graphql: graphqlMock } as unknown as ReturnType, + ); + + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 711, + number: 711, + title: "Draft PR to mark ready before logout", + repoFullName: "owner/repo", + userLogin: "testuser", + state: "OPEN", + draft: true, + enriched: true, + nodeId: "PR_kwDO711", + headSha: "b1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + checkStatus: "success", + }), + ], + workflowRuns: [], + errors: [], + }); + + render(() => ); + await user.click(screen.getByText("Pull Requests")); + await waitFor(() => screen.getByText("owner/repo")); + await user.click(screen.getByText("owner/repo")); + + const draftBtn = await waitFor(() => + screen.getByLabelText("Mark #711 Draft PR to mark ready before logout ready for review"), + ); + await user.click(draftBtn); + await user.click(draftBtn); + await waitFor(() => expect(graphqlMock).toHaveBeenCalledOnce()); + + vi.mocked(snapshotLib.writeSnapshot).mockClear(); + + // Logout occurs while markPullRequestReady is in flight + for (const cb of authClearCallbacks) cb(); + + // Resolve the in-flight markPullRequestReady mutation + resolveMarkReady({ + markPullRequestReadyForReview: { + pullRequest: { isDraft: false }, + }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + + expect(snapshotLib.writeSnapshot).not.toHaveBeenCalled(); + }); + + it("writes a debounced snapshot for the current login after a targeted refresh from the current identity", async () => { + render(() => ); + await waitFor(() => expect(capturedOnTargetedData).not.toBeNull()); + await waitFor(() => + expect(snapshotLib.writeSnapshot).toHaveBeenCalledWith(expect.anything(), "testuser", "immediate"), + ); + vi.mocked(snapshotLib.writeSnapshot).mockClear(); + + capturedOnTargetedData!( + { + issues: [makeIssue({ id: 713, title: "Current targeted issue", repoFullName: "owner/targeted" })], + pullRequests: [], + workflowRuns: [], + errors: [], + }, + ["owner/targeted"], + authStore.identityEpoch(), + ); + + expect(snapshotLib.writeSnapshot).toHaveBeenCalledTimes(1); + expect(snapshotLib.writeSnapshot).toHaveBeenCalledWith( + expect.objectContaining({ issues: [expect.objectContaining({ id: 713 })] }), + "testuser", + "debounced", + ); + }); + + it("writes a debounced snapshot for the current login after a hot poll from the current identity", async () => { + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 714, + number: 714, + title: "Hot PR current identity", + repoFullName: "owner/repo", + state: "OPEN", + enriched: true, + nodeId: "PR_kwDO714", + checkStatus: "running", + reviewDecision: null, + }), + ], + workflowRuns: [], + errors: [], + }); + + render(() => ); + await waitFor(() => expect(capturedOnHotData).not.toBeNull()); + await waitFor(() => + expect(snapshotLib.writeSnapshot).toHaveBeenCalledWith(expect.anything(), "testuser", "immediate"), + ); + vi.mocked(snapshotLib.writeSnapshot).mockClear(); + + capturedOnHotData!( + new Map([[714, { + state: "OPEN" as const, + checkStatus: "success" as const, + mergeStateStatus: "CLEAN", + reviewDecision: null, + }]]), + new Map(), + 0, + authStore.identityEpoch(), + ); + + expect(snapshotLib.writeSnapshot).toHaveBeenCalledTimes(1); + expect(snapshotLib.writeSnapshot).toHaveBeenCalledWith( + expect.objectContaining({ + pullRequests: [expect.objectContaining({ id: 714, checkStatus: "success" })], + }), + "testuser", + "debounced", + ); + }); + + it("discards a hot poll whose cycle started under a previous identity epoch without mutating the store or writing a snapshot", async () => { + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 715, + number: 715, + title: "Hot PR stale epoch", + repoFullName: "owner/repo", + state: "OPEN", + enriched: true, + nodeId: "PR_kwDO715", + checkStatus: "running", + reviewDecision: null, + }), + ], + workflowRuns: [], + errors: [], + }); + + render(() => ); + await waitFor(() => expect(capturedOnHotData).not.toBeNull()); + + const user = userEvent.setup(); + await user.click(screen.getByText("Pull Requests")); + await waitFor(() => screen.getByText("owner/repo")); + await user.click(screen.getByText("owner/repo")); + await waitFor(() => expect(screen.getByLabelText("Checks in progress")).toBeTruthy()); + + vi.mocked(snapshotLib.writeSnapshot).mockClear(); + + // Hot cycle captures the epoch, then the identity epoch advances before results arrive + const staleCycleEpoch = authStore.identityEpoch(); + mockIdentityEpochValue++; + + capturedOnHotData!( + new Map([[715, { + state: "OPEN" as const, + checkStatus: "success" as const, + mergeStateStatus: "CLEAN", + reviewDecision: null, + }]]), + new Map(), + 0, + staleCycleEpoch, + ); + + expect(screen.getByLabelText("Checks in progress")).toBeTruthy(); + expect(screen.queryByLabelText("All checks passed")).toBeNull(); + expect(snapshotLib.writeSnapshot).not.toHaveBeenCalled(); + }); + + it("writes a debounced snapshot for the current login after a successful PR approval", async () => { + vi.mocked(githubService.getClient).mockReturnValue( + { graphql: vi.fn().mockResolvedValue(approveResponse) } as unknown as ReturnType, + ); + const user = await renderWithPR(makeApprovablePR()); + + const approveBtn = await waitFor(() => screen.getByLabelText("Approve #42 Fix race condition")); + vi.mocked(snapshotLib.writeSnapshot).mockClear(); + await user.click(approveBtn); + await user.click(approveBtn); + + await waitFor(() => + expect(snapshotLib.writeSnapshot).toHaveBeenCalledWith( + expect.objectContaining({ + pullRequests: [expect.objectContaining({ id: 42, reviewDecision: "APPROVED" })], + }), + "testuser", + "debounced", + ), + ); + }); + + it("writes a debounced snapshot for the current login after a successful mark-PR-ready", async () => { + vi.mocked(githubService.getClient).mockReturnValue( + { graphql: vi.fn().mockResolvedValue(markReadyResponse) } as unknown as ReturnType, + ); + const user = await renderWithPR(makeDraftPR()); + + const draftBtn = await waitFor(() => + screen.getByLabelText("Mark #77 Ship draft feature ready for review"), + ); + vi.mocked(snapshotLib.writeSnapshot).mockClear(); + await user.click(draftBtn); + await user.click(draftBtn); + + await waitFor(() => + expect(snapshotLib.writeSnapshot).toHaveBeenCalledWith( + expect.objectContaining({ + pullRequests: [expect.objectContaining({ id: 77, draft: false })], + }), + "testuser", + "debounced", + ), + ); + }); + + it("discards an in-flight snapshot load if clearAuth() runs before loadSnapshot resolves", async () => { + let resolveSnapshot!: (snap: import("../../src/app/lib/dashboard-snapshot").DashboardSnapshot | null) => void; + vi.mocked(snapshotLib.loadSnapshot).mockImplementation( + () => + new Promise((resolve) => { + resolveSnapshot = resolve; + }), + ); + vi.mocked(pollService.fetchAllData).mockImplementation(() => new Promise(() => {})); + + render(() => ); + await waitFor(() => expect(snapshotLib.loadSnapshot).toHaveBeenCalledWith("testuser")); + + // Logout while loadSnapshot is still in flight + for (const cb of authClearCallbacks) cb(); + + resolveSnapshot({ + _v: 1, + login: "testuser", + savedAt: Date.now(), + issues: [makeIssue({ id: 708, title: "Pre-logout cached issue", repoFullName: "owner/pre-logout" })], + pullRequests: [], + workflowRuns: [], + lastRefreshedAt: new Date(), + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + + expect(screen.queryByText("owner/pre-logout")).toBeNull(); + const issuesTab = screen.getByRole("tab", { name: /^Issues/ }); + expect(issuesTab.textContent?.replace(/\D+/g, "")).toBe("0"); + }); + + it("resets _fetchingDashboardBodies and _fetchingDepBodies after an in-flight fetch when same-user token rotation does not bump identityEpoch", async () => { + const [lastRefreshAtSig, setLastRefreshAtSig] = createSignal(null); + vi.mocked(pollService.createPollCoordinator).mockImplementation((_getInterval, fetchAll) => { + const wrappedFetchAll = async () => { + const res = await fetchAll(); + setLastRefreshAtSig(new Date()); + return res; + }; + capturedFetchAll = wrappedFetchAll; + void wrappedFetchAll().catch(() => {}); + return { + isRefreshing: () => false, + lastRefreshAt: lastRefreshAtSig, + manualRefresh: vi.fn(), + destroy: vi.fn(), + }; + }); + + let resolveDashBody1!: (val: unknown) => void; + let resolveDepBody1!: (val: unknown) => void; + let dashCallCount = 0; + let depCallCount = 0; + + const client1 = { + graphql: vi.fn().mockImplementation((query: string) => { + if (query.includes("databaseId")) { + depCallCount++; + if (depCallCount === 1) { + return new Promise((resolve) => { + resolveDepBody1 = resolve; + }); + } + return Promise.resolve({ + nodes: [ + { + id: "PR_ROTATE_2", + databaseId: 710, + body: "| Package | Change |\n| --- | --- |\n| pkg-b | `1.0.0` -> `1.1.0` |", + }, + ], + rateLimit: null, + }); + } + dashCallCount++; + if (dashCallCount === 1) { + return new Promise((resolve) => { + resolveDashBody1 = resolve; + }); + } + return Promise.resolve({ + nodes: [{ id: "DASH_ROTATE_2", body: "" }], + rateLimit: null, + }); + }), + }; + const client2 = { graphql: client1.graphql }; + vi.mocked(githubService.getClient).mockReturnValue( + client1 as unknown as ReturnType, + ); + + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [ + makeIssue({ + id: 709, + number: 709, + title: "Dependency Dashboard", + userLogin: "renovate[bot]", + nodeId: "DASH_ROTATE_1", + repoFullName: "owner/repo", + state: "OPEN", + updatedAt: "2026-04-01T00:00:00Z", + }), + ], + pullRequests: [ + makePullRequest({ + id: 709, + nodeId: "PR_ROTATE_1", + repoFullName: "owner/repo", + title: "Update dependency pkg-a", + userLogin: "dependabot[bot]", + headRef: "dependabot/npm_and_yarn/pkg-a", + enriched: true, + }), + ], + workflowRuns: [], + errors: [], + }); + + render(() => ); + await waitFor(() => { + expect(dashCallCount).toBe(1); + expect(depCallCount).toBe(1); + }); + + // Simulate same-user token rotation while both body fetches are in flight: + // getClient() swaps to client2, but identityEpoch() is NOT incremented! + vi.mocked(githubService.getClient).mockReturnValue( + client2 as unknown as ReturnType, + ); + + // Resolve the in-flight fetches — they bail on `getClient() !== octokit` but their + // `finally` blocks still reset `_fetchingDashboardBodies` and `_fetchingDepBodies` + // because `authGen === identityEpoch()`. + resolveDashBody1({ nodes: [], rateLimit: null }); + resolveDepBody1({ nodes: [], rateLimit: null }); + await new Promise((resolve) => setTimeout(resolve, 30)); + + // Next poll cycle with new items must be able to trigger both body fetches again + vi.mocked(pollService.fetchAllData).mockResolvedValue({ + issues: [ + makeIssue({ + id: 710, + number: 710, + title: "Dependency Dashboard", + userLogin: "renovate[bot]", + nodeId: "DASH_ROTATE_2", + repoFullName: "owner/repo", + state: "OPEN", + updatedAt: "2026-04-02T00:00:00Z", + }), + ], + pullRequests: [ + makePullRequest({ + id: 710, + nodeId: "PR_ROTATE_2", + repoFullName: "owner/repo", + title: "Update dependency pkg-b", + userLogin: "dependabot[bot]", + headRef: "dependabot/npm_and_yarn/pkg-b", + enriched: true, + }), + ], + workflowRuns: [], + errors: [], + }); + + await capturedFetchAll!(); + await waitFor(() => { + expect(dashCallCount).toBe(2); + expect(depCallCount).toBe(2); + }); + }); +}); diff --git a/tests/components/dashboard/render-stability.test.tsx b/tests/components/dashboard/render-stability.test.tsx new file mode 100644 index 00000000..b7fd3e3f --- /dev/null +++ b/tests/components/dashboard/render-stability.test.tsx @@ -0,0 +1,628 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { createStore, reconcile, unwrap } from "solid-js/store"; +import { render, screen, waitFor } from "@solidjs/testing-library"; +import { makeIssue, makePullRequest, makeWorkflowRun } from "../../helpers/index"; +import { fallbackToPreviousEnrichment } from "../../../src/app/services/api"; +import type { DashboardData } from "../../../src/app/services/poll"; +import type { HotPRStatusUpdate, HotWorkflowRunUpdate, PullRequest } from "../../../src/app/services/api"; + +const mockLocationReplace = vi.fn(); + +Object.defineProperty(window, "location", { + configurable: true, + writable: true, + value: { replace: mockLocationReplace, href: "" }, +}); + +vi.mock("@solidjs/router", () => ({ + useNavigate: () => vi.fn(), +})); + +const authClearCallbacks: (() => void)[] = []; +vi.mock("../../../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(cb); + }), + DASHBOARD_STORAGE_KEY: "github-tracker:dashboard", + DEP_META_STORAGE_KEY: "github-tracker:dep-meta", + jiraAuth: vi.fn(() => null), + isJiraAuthenticated: vi.fn(() => false), + setJiraAuth: vi.fn(), + clearJiraAuth: vi.fn(), + ensureJiraTokenValid: vi.fn().mockResolvedValue(false), + identityEpoch: () => 0, +})); + +vi.mock("../../../src/app/lib/dashboard-snapshot", () => ({ + loadSnapshot: vi.fn().mockResolvedValue(null), + writeSnapshot: vi.fn(), + flushSnapshot: vi.fn().mockResolvedValue(undefined), + clearSnapshot: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../src/app/services/jira-client", () => ({ + JiraClient: vi.fn(), + JiraProxyClient: vi.fn(), + 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.mock("../../../src/app/services/jira-keys", () => ({ + detectAndLookupJiraKeys: vi.fn().mockResolvedValue(new Map()), + clearJiraKeyCache: vi.fn(), +})); + +vi.mock("../../../src/app/services/github", () => ({ + getCoreRateLimit: () => null, + getGraphqlRateLimit: () => null, + getClient: vi.fn(() => null), + updateGraphqlRateLimit: vi.fn(), +})); + +vi.mock("../../../src/app/lib/notifications", () => ({ + detectNewItems: vi.fn(() => []), + dispatchNotifications: vi.fn(), + _resetNotificationState: vi.fn(), +})); + +vi.mock("../../../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(), +})); + +let capturedFetchAll: (() => Promise) | null = null; +let capturedOnHotData: + | (( + prUpdates: Map, + runUpdates: Map, + generation: number, + epoch: number, + ) => void) + | null = null; +let capturedOnTargetedData: ((data: DashboardData, affectedRepos: string[], epoch: number) => void) | null = null; + +let DashboardPage: typeof import("../../../src/app/components/dashboard/DashboardPage").default; +let pollService: typeof import("../../../src/app/services/poll"); +let viewStore: typeof import("../../../src/app/stores/view"); +let configStore: typeof import("../../../src/app/stores/config"); + +beforeEach(async () => { + localStorage.clear?.(); + vi.resetModules(); + authClearCallbacks.length = 0; + + vi.doMock("../../../src/app/services/poll", () => ({ + fetchAllData: vi.fn().mockResolvedValue({ + issues: [], + pullRequests: [], + workflowRuns: [], + errors: [], + }), + createPollCoordinator: vi.fn().mockImplementation( + (_getInterval: unknown, fetchAll: () => Promise) => { + capturedFetchAll = fetchAll; + void fetchAll().catch(() => {}); + return { + isRefreshing: () => false, + lastRefreshAt: () => null, + manualRefresh: vi.fn(), + destroy: vi.fn(), + }; + }, + ), + createHotPollCoordinator: vi.fn().mockImplementation( + (_getInterval: unknown, onHotData: typeof capturedOnHotData) => { + capturedOnHotData = onHotData; + return { destroy: vi.fn() }; + }, + ), + createEventsPollCoordinator: vi.fn().mockImplementation( + ( + _getUsername: unknown, + _trackedRepoNames: unknown, + _isFullRefreshing: unknown, + onTargetedData: typeof capturedOnTargetedData, + ) => { + capturedOnTargetedData = onTargetedData; + return { destroy: vi.fn() }; + }, + ), + rebuildHotSets: vi.fn(), + seedHotSetsFromTargeted: vi.fn(), + clearHotSets: vi.fn(), + getHotPollGeneration: vi.fn().mockReturnValue(0), + StaleIdentityError: class StaleIdentityError extends Error { + constructor() { + super("Stale identity"); + this.name = "StaleIdentityError"; + } + }, + })); + + const dashboardModule = await import("../../../src/app/components/dashboard/DashboardPage"); + DashboardPage = dashboardModule.default; + pollService = await import("../../../src/app/services/poll"); + viewStore = await import("../../../src/app/stores/view"); + configStore = await import("../../../src/app/stores/config"); + + capturedFetchAll = null; + capturedOnHotData = null; + capturedOnTargetedData = null; + viewStore.resetViewState(); + configStore.resetConfig(); +}, 30_000); + +describe("render stability — keyed reconcile store writes and stable group identity", () => { + it("preserves PR row and repo group DOM nodes across a full poll when another PR's checkStatus changes", async () => { + viewStore.updateViewState({ lastActiveTab: "pullRequests" }); + viewStore.setAllExpanded("pullRequests", true); + + let cycle = 0; + vi.mocked(pollService.fetchAllData).mockImplementation(() => { + cycle++; + // Return freshly constructed plain arrays on every poll cycle + return Promise.resolve({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 101, + number: 101, + title: "Stable PR Alpha", + repoFullName: "owner/repo-a", + checkStatus: "success", + enriched: true, + }), + makePullRequest({ + id: 102, + number: 102, + title: "Changing PR Beta", + repoFullName: "owner/repo-a", + checkStatus: cycle === 1 ? "running" : "failure", + enriched: true, + }), + makePullRequest({ + id: 103, + number: 103, + title: "Other Repo PR Gamma", + repoFullName: "owner/repo-b", + checkStatus: "success", + enriched: true, + }), + ], + workflowRuns: [], + errors: [], + }); + }); + + render(() => ); + await waitFor(() => { + screen.getByText("Stable PR Alpha"); + screen.getByText("Changing PR Beta"); + screen.getByText("Other Repo PR Gamma"); + }); + + const groupANode = document.querySelector('[data-repo-group="owner/repo-a"]'); + const groupBNode = document.querySelector('[data-repo-group="owner/repo-b"]'); + const alphaRow = screen.getByText("Stable PR Alpha").closest('[role="listitem"]'); + const betaRow = screen.getByText("Changing PR Beta").closest('[role="listitem"]'); + const gammaRow = screen.getByText("Other Repo PR Gamma").closest('[role="listitem"]'); + + expect(groupANode).not.toBeNull(); + expect(groupBNode).not.toBeNull(); + expect(alphaRow).not.toBeNull(); + expect(betaRow).not.toBeNull(); + expect(gammaRow).not.toBeNull(); + expect(betaRow?.querySelector('[aria-label="Checks in progress"]')).not.toBeNull(); + + expect(capturedFetchAll).not.toBeNull(); + await capturedFetchAll!(); + + expect(document.querySelector('[data-repo-group="owner/repo-a"]')).toBe(groupANode); + expect(document.querySelector('[data-repo-group="owner/repo-b"]')).toBe(groupBNode); + expect(screen.getByText("Stable PR Alpha").closest('[role="listitem"]')).toBe(alphaRow); + expect(screen.getByText("Changing PR Beta").closest('[role="listitem"]')).toBe(betaRow); + expect(screen.getByText("Other Repo PR Gamma").closest('[role="listitem"]')).toBe(gammaRow); + expect(betaRow?.querySelector('[aria-label="Checks failing"]')).not.toBeNull(); + expect(betaRow?.querySelector('[aria-label="Checks in progress"]')).toBeNull(); + }); + + it("preserves PR row DOM nodes across the Phase 1 light-data to Phase 2 enriched transition", async () => { + viewStore.updateViewState({ lastActiveTab: "pullRequests" }); + viewStore.setAllExpanded("pullRequests", true); + + let resolvePhaseTwo: ((data: DashboardData) => void) | null = null; + vi.mocked(pollService.fetchAllData).mockImplementation((onLightData) => { + onLightData?.({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 150, + number: 150, + title: "Two-phase PR", + repoFullName: "owner/repo-a", + enriched: false, + checkStatus: null, + }), + ], + workflowRuns: [], + errors: [], + }); + return new Promise((resolve) => { + resolvePhaseTwo = resolve; + }); + }); + + render(() => ); + await waitFor(() => { + screen.getByText("Two-phase PR"); + }); + + const rowAfterPhaseOne = screen.getByText("Two-phase PR").closest('[role="listitem"]'); + expect(rowAfterPhaseOne).not.toBeNull(); + expect(rowAfterPhaseOne?.querySelector('[aria-label="All checks passed"]')).toBeNull(); + + resolvePhaseTwo!({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 150, + number: 150, + title: "Two-phase PR", + repoFullName: "owner/repo-a", + enriched: true, + checkStatus: "success", + }), + ], + workflowRuns: [], + errors: [], + }); + + await waitFor(() => { + expect(rowAfterPhaseOne?.querySelector('[aria-label="All checks passed"]')).not.toBeNull(); + }); + expect(screen.getByText("Two-phase PR").closest('[role="listitem"]')).toBe(rowAfterPhaseOne); + }); + + it("preserves PR row and repo group DOM nodes across a targeted-refresh (events) update", async () => { + viewStore.updateViewState({ lastActiveTab: "pullRequests" }); + viewStore.setAllExpanded("pullRequests", true); + + vi.mocked(pollService.fetchAllData).mockImplementation(() => + Promise.resolve({ + issues: [], + pullRequests: [ + makePullRequest({ + id: 101, + number: 101, + title: "Stable PR Alpha", + repoFullName: "owner/repo-a", + checkStatus: "success", + enriched: true, + }), + makePullRequest({ + id: 102, + number: 102, + title: "Targeted PR Beta", + repoFullName: "owner/repo-a", + checkStatus: "running", + enriched: true, + }), + makePullRequest({ + id: 103, + number: 103, + title: "Unaffected PR Gamma", + repoFullName: "owner/repo-b", + checkStatus: "success", + enriched: true, + }), + ], + workflowRuns: [], + errors: [], + }), + ); + + render(() => ); + await waitFor(() => { + screen.getByText("Stable PR Alpha"); + screen.getByText("Targeted PR Beta"); + screen.getByText("Unaffected PR Gamma"); + }); + + const groupANode = document.querySelector('[data-repo-group="owner/repo-a"]'); + const groupBNode = document.querySelector('[data-repo-group="owner/repo-b"]'); + const alphaRow = screen.getByText("Stable PR Alpha").closest('[role="listitem"]'); + const betaRow = screen.getByText("Targeted PR Beta").closest('[role="listitem"]'); + const gammaRow = screen.getByText("Unaffected PR Gamma").closest('[role="listitem"]'); + + expect(betaRow?.querySelector('[aria-label="Checks in progress"]')).not.toBeNull(); + expect(capturedOnTargetedData).not.toBeNull(); + + // Targeted refresh for owner/repo-a with freshly constructed plain objects + capturedOnTargetedData!( + { + issues: [], + pullRequests: [ + makePullRequest({ + id: 101, + number: 101, + title: "Stable PR Alpha", + repoFullName: "owner/repo-a", + checkStatus: "success", + enriched: true, + }), + makePullRequest({ + id: 102, + number: 102, + title: "Targeted PR Beta", + repoFullName: "owner/repo-a", + checkStatus: "success", + enriched: true, + }), + ], + workflowRuns: [], + errors: [], + }, + ["owner/repo-a"], + 0, + ); + + expect(document.querySelector('[data-repo-group="owner/repo-a"]')).toBe(groupANode); + expect(document.querySelector('[data-repo-group="owner/repo-b"]')).toBe(groupBNode); + expect(screen.getByText("Stable PR Alpha").closest('[role="listitem"]')).toBe(alphaRow); + expect(screen.getByText("Targeted PR Beta").closest('[role="listitem"]')).toBe(betaRow); + expect(screen.getByText("Unaffected PR Gamma").closest('[role="listitem"]')).toBe(gammaRow); + expect(betaRow?.querySelector('[aria-label="All checks passed"]')).not.toBeNull(); + }); + + it("preserves IssuesTab row and repo group DOM nodes across full poll and targeted refreshes", async () => { + viewStore.updateViewState({ lastActiveTab: "issues" }); + viewStore.setAllExpanded("issues", true); + + let cycle = 0; + vi.mocked(pollService.fetchAllData).mockImplementation(() => { + cycle++; + return Promise.resolve({ + issues: [ + makeIssue({ + id: 201, + number: 201, + title: "Stable Issue One", + repoFullName: "owner/repo-a", + comments: 2, + }), + makeIssue({ + id: 202, + number: 202, + title: "Updating Issue Two", + repoFullName: "owner/repo-a", + comments: cycle === 1 ? 3 : 9, + }), + ], + pullRequests: [], + workflowRuns: [], + errors: [], + }); + }); + + render(() => ); + await waitFor(() => { + screen.getByText("Stable Issue One"); + screen.getByText("Updating Issue Two"); + }); + + const repoGroupNode = document.querySelector('[data-repo-group="owner/repo-a"]'); + const issue1Row = screen.getByText("Stable Issue One").closest('[role="listitem"]'); + const issue2Row = screen.getByText("Updating Issue Two").closest('[role="listitem"]'); + + expect(repoGroupNode).not.toBeNull(); + expect(issue1Row).not.toBeNull(); + expect(issue2Row).not.toBeNull(); + + await capturedFetchAll!(); + + expect(document.querySelector('[data-repo-group="owner/repo-a"]')).toBe(repoGroupNode); + expect(screen.getByText("Stable Issue One").closest('[role="listitem"]')).toBe(issue1Row); + expect(screen.getByText("Updating Issue Two").closest('[role="listitem"]')).toBe(issue2Row); + expect(issue2Row?.textContent).toContain("9"); + + capturedOnTargetedData!( + { + issues: [ + makeIssue({ + id: 201, + number: 201, + title: "Stable Issue One", + repoFullName: "owner/repo-a", + comments: 7, + }), + makeIssue({ + id: 202, + number: 202, + title: "Updating Issue Two", + repoFullName: "owner/repo-a", + comments: 9, + }), + ], + pullRequests: [], + workflowRuns: [], + errors: [], + }, + ["owner/repo-a"], + 0, + ); + + expect(document.querySelector('[data-repo-group="owner/repo-a"]')).toBe(repoGroupNode); + expect(screen.getByText("Stable Issue One").closest('[role="listitem"]')).toBe(issue1Row); + expect(screen.getByText("Updating Issue Two").closest('[role="listitem"]')).toBe(issue2Row); + expect(issue1Row?.textContent).toContain("7"); + }); + + it("preserves ActionsTab workflow card and repo group DOM nodes across a run status change", async () => { + viewStore.updateViewState({ lastActiveTab: "actions" }); + viewStore.setAllExpanded("actions", true); + + let cycle = 0; + vi.mocked(pollService.fetchAllData).mockImplementation(() => { + cycle++; + return Promise.resolve({ + issues: [], + pullRequests: [], + workflowRuns: [ + makeWorkflowRun({ + id: 301, + workflowId: 10, + name: "CI Pipeline", + repoFullName: "owner/repo-a", + status: cycle === 1 ? "in_progress" : "completed", + conclusion: cycle === 1 ? null : "failure", + event: "push", + createdAt: "2026-03-01T12:00:00Z", + }), + makeWorkflowRun({ + id: 302, + workflowId: 20, + name: "Release Build", + repoFullName: "owner/repo-a", + status: "completed", + conclusion: "success", + event: "push", + createdAt: "2026-03-01T11:00:00Z", + }), + ], + errors: [], + }); + }); + + render(() => ); + await waitFor(() => { + screen.getByRole("button", { name: "CI Pipeline" }); + screen.getByRole("button", { name: "Release Build" }); + }); + + const repoGroupNode = document.querySelector('[data-repo-group="owner/repo-a"]'); + const ciCard = screen.getByRole("button", { name: "CI Pipeline" }); + const releaseCard = screen.getByRole("button", { name: "Release Build" }); + + expect(repoGroupNode).not.toBeNull(); + expect(ciCard.querySelector('[aria-label="1 running"]')).not.toBeNull(); + + await capturedFetchAll!(); + + expect(document.querySelector('[data-repo-group="owner/repo-a"]')).toBe(repoGroupNode); + expect(screen.getByRole("button", { name: "CI Pipeline" })).toBe(ciCard); + expect(screen.getByRole("button", { name: "Release Build" })).toBe(releaseCard); + expect(ciCard.querySelector('[aria-label="1 failed"]')).not.toBeNull(); + expect(ciCard.querySelector('[aria-label="1 running"]')).toBeNull(); + }); + + it("round-trips enrichment fields copied from a previous store proxy via fallbackToPreviousEnrichment through reconcile with identical values", async () => { + // 1. Direct SolidJS store proxy -> fallbackToPreviousEnrichment -> reconcile round-trip + const initialEnriched = makePullRequest({ + id: 401, + number: 401, + title: "Enriched PR", + repoFullName: "owner/repo-a", + updatedAt: "2026-03-01T00:00:00Z", + enriched: true, + additions: 42, + deletions: 7, + changedFiles: 3, + comments: 4, + reviewThreads: 2, + checkStatus: "success", + reviewDecision: "APPROVED", + headSha: "abc1234def5678", + autoMergeMethod: "SQUASH", + reviewerLogins: ["alice", "bob"], + totalReviewCount: 2, + viewerReviewRequested: true, + viewerLatestReviewState: "APPROVED", + }); + + const [store, setStore] = createStore<{ pullRequests: PullRequest[] }>({ + pullRequests: [initialEnriched], + }); + const proxyRef = store.pullRequests[0]; + + const unenrichedNext = makePullRequest({ + id: 401, + number: 401, + title: "Enriched PR", + repoFullName: "owner/repo-a", + updatedAt: "2026-03-01T00:00:00Z", + enriched: false, + additions: 0, + deletions: 0, + changedFiles: 0, + comments: 0, + reviewThreads: 0, + checkStatus: null, + reviewDecision: null, + headSha: undefined, + autoMergeMethod: null, + reviewerLogins: [], + totalReviewCount: 0, + viewerReviewRequested: false, + viewerLatestReviewState: null, + }); + + // Pass store proxies directly into fallbackToPreviousEnrichment, then reconcile back into the store + const merged = fallbackToPreviousEnrichment(store.pullRequests, [unenrichedNext]); + setStore("pullRequests", reconcile(merged, { key: "id", merge: true })); + + expect(store.pullRequests[0]).toBe(proxyRef); + expect(unwrap(store.pullRequests[0])).toEqual(initialEnriched); + + // 2. End-to-end through DashboardPage pollFetch + viewStore.updateViewState({ lastActiveTab: "pullRequests" }); + viewStore.setAllExpanded("pullRequests", true); + + let cycle = 0; + vi.mocked(pollService.fetchAllData).mockImplementation(() => { + cycle++; + return Promise.resolve({ + issues: [], + pullRequests: [cycle === 1 ? { ...initialEnriched, reviewerLogins: [...initialEnriched.reviewerLogins] } : { ...unenrichedNext }], + workflowRuns: [], + errors: [], + }); + }); + + render(() => ); + await waitFor(() => { + screen.getByText("Enriched PR"); + }); + + const rowBefore = screen.getByText("Enriched PR").closest('[role="listitem"]'); + expect(rowBefore).not.toBeNull(); + expect(rowBefore?.querySelector('[aria-label="All checks passed"]')).not.toBeNull(); + + const poll2Result = await capturedFetchAll!(); + + expect(screen.getByText("Enriched PR").closest('[role="listitem"]')).toBe(rowBefore); + expect(rowBefore?.querySelector('[aria-label="All checks passed"]')).not.toBeNull(); + expect(poll2Result.pullRequests[0]).toEqual(initialEnriched); + }); +}); diff --git a/tests/lib/dashboard-snapshot.test.ts b/tests/lib/dashboard-snapshot.test.ts new file mode 100644 index 00000000..1b7e024b --- /dev/null +++ b/tests/lib/dashboard-snapshot.test.ts @@ -0,0 +1,521 @@ +import "fake-indexeddb/auto"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { openDB } from "idb"; +import { makeIssue, makePullRequest, makeWorkflowRun } from "../helpers"; +import type { DashboardSnapshotState } from "../../src/app/lib/dashboard-snapshot"; + +const idbHoisted = vi.hoisted(() => ({ + realOpenDB: null as unknown as typeof openDB, +})); +vi.mock("idb", async (importOriginal) => { + const actual = await importOriginal(); + idbHoisted.realOpenDB = actual.openDB; + return { + ...actual, + openDB: vi.fn((...args: Parameters) => actual.openDB(...args)), + }; +}); + +const mockPushNotification = vi.fn(); +vi.mock("../../src/app/lib/errors", () => ({ + pushNotification: (...args: unknown[]) => mockPushNotification(...args), +})); + +const DB_NAME = "github-tracker-snapshot"; +const STORE_NAME = "snapshot"; +const RECORD_KEY = "current"; + +function makeSampleState(overrides?: Partial): DashboardSnapshotState { + return { + issues: [makeIssue({ id: 1, title: "Issue 1" })], + pullRequests: [makePullRequest({ id: 10, title: "PR 10" })], + workflowRuns: [makeWorkflowRun({ id: 100, name: "CI" })], + lastRefreshedAt: new Date("2026-10-30T12:00:00.000Z"), + ...overrides, + }; +} + +// Re-imports the module with a fresh DB connection whose `put` calls are counted +// (and, when `gate` is given, held until it resolves). +async function installPutCounter(gate?: Promise): Promise<() => number> { + vi.resetModules(); + const idb = await import("idb"); + let putCount = 0; + vi.mocked(idb.openDB).mockImplementation(async (...args: Parameters) => { + const db = await idbHoisted.realOpenDB(...args); + const origPut = db.put.bind(db); + vi.spyOn(db, "put").mockImplementation((async (...putArgs: Parameters) => { + putCount++; + if (gate) await gate; + return origPut(...putArgs); + }) as typeof db.put); + return db; + }); + return () => putCount; +} + +describe("dashboard-snapshot", () => { + beforeEach(async () => { + vi.useRealTimers(); + mockPushNotification.mockClear(); + vi.mocked(openDB).mockImplementation((...args: Parameters) => + idbHoisted.realOpenDB(...args), + ); + const mod = await import("../../src/app/lib/dashboard-snapshot"); + await mod.clearSnapshot(); + }); + + afterEach(async () => { + vi.useRealTimers(); + vi.mocked(openDB).mockImplementation((...args: Parameters) => + idbHoisted.realOpenDB(...args), + ); + const mod = await import("../../src/app/lib/dashboard-snapshot"); + await mod.clearSnapshot(); + vi.restoreAllMocks(); + }); + + it("removes legacy localStorage key 'github-tracker:dashboard' on module import", async () => { + localStorage.setItem("github-tracker:dashboard", '{"legacy":true}'); + vi.resetModules(); + await import("../../src/app/lib/dashboard-snapshot"); + expect(localStorage.getItem("github-tracker:dashboard")).toBeNull(); + }); + + it("round-trips a snapshot through writeSnapshot (immediate) and loadSnapshot (case-insensitive login)", async () => { + const { writeSnapshot, loadSnapshot } = await import("../../src/app/lib/dashboard-snapshot"); + const state = makeSampleState(); + + writeSnapshot(state, "Alice", "immediate"); + await vi.waitFor(async () => { + expect(await loadSnapshot("alice")).not.toBeNull(); + }); + + const loaded = await loadSnapshot("alice"); + expect(loaded).not.toBeNull(); + expect(loaded!._v).toBe(1); + expect(loaded!.login).toBe("Alice"); + expect(loaded!.issues).toEqual(state.issues); + expect(loaded!.pullRequests).toEqual(state.pullRequests); + expect(loaded!.workflowRuns).toEqual(state.workflowRuns); + expect(loaded!.lastRefreshedAt).toEqual(new Date("2026-10-30T12:00:00.000Z")); + }); + + it("returns null on login mismatch", async () => { + const { writeSnapshot, loadSnapshot } = await import("../../src/app/lib/dashboard-snapshot"); + writeSnapshot(makeSampleState(), "alice", "immediate"); + await vi.waitFor(async () => { + expect(await loadSnapshot("alice")).not.toBeNull(); + }); + + const loaded = await loadSnapshot("bob"); + expect(loaded).toBeNull(); + }); + + it("returns null on version or shape mismatch", async () => { + const { loadSnapshot } = await import("../../src/app/lib/dashboard-snapshot"); + const db = await idbHoisted.realOpenDB(DB_NAME, 1, { + upgrade(d) { + if (!d.objectStoreNames.contains(STORE_NAME)) { + d.createObjectStore(STORE_NAME); + } + }, + }); + + // Version mismatch + await db.put( + STORE_NAME, + { + _v: 2, + login: "alice", + savedAt: Date.now(), + issues: [], + pullRequests: [], + workflowRuns: [], + lastRefreshedAt: null, + }, + RECORD_KEY, + ); + expect(await loadSnapshot("alice")).toBeNull(); + + // Non-array issues + await db.put( + STORE_NAME, + { + _v: 1, + login: "alice", + savedAt: Date.now(), + issues: "not-an-array", + pullRequests: [], + workflowRuns: [], + lastRefreshedAt: null, + }, + RECORD_KEY, + ); + expect(await loadSnapshot("alice")).toBeNull(); + + // Invalid lastRefreshedAt type + await db.put( + STORE_NAME, + { + _v: 1, + login: "alice", + savedAt: Date.now(), + issues: [], + pullRequests: [], + workflowRuns: [], + lastRefreshedAt: 12345, + }, + RECORD_KEY, + ); + expect(await loadSnapshot("alice")).toBeNull(); + + // Corrupt array element (null or missing numeric id) + await db.put( + STORE_NAME, + { + _v: 1, + login: "alice", + savedAt: Date.now(), + issues: [null], + pullRequests: [], + workflowRuns: [], + lastRefreshedAt: null, + }, + RECORD_KEY, + ); + expect(await loadSnapshot("alice")).toBeNull(); + + db.close(); + }); + + it("coalesces 5 debounced writes into 1 IDB put", async () => { + const putCount = await installPutCounter(); + + const { writeSnapshot, loadSnapshot, DEBOUNCE_MS } = await import( + "../../src/app/lib/dashboard-snapshot" + ); + + vi.useFakeTimers({ shouldAdvanceTime: true }); + + for (let i = 1; i <= 5; i++) { + writeSnapshot( + makeSampleState({ issues: [makeIssue({ id: i, title: `Issue ${i}` })] }), + "alice", + "debounced", + ); + await vi.advanceTimersByTimeAsync(1_000); + } + + // 5 * 1000ms elapsed, last write still needs ~4000ms to fire + expect(putCount()).toBe(0); + + await vi.advanceTimersByTimeAsync(DEBOUNCE_MS); + vi.useRealTimers(); + await vi.waitFor(() => { + expect(putCount()).toBe(1); + }); + + const snap = await loadSnapshot("alice"); + expect(snap?.issues.map((i) => i.id)).toEqual([5]); + }); + + it("commits an immediate write on the next macrotask without waiting for the 5s debounce", async () => { + const { writeSnapshot, loadSnapshot } = await import("../../src/app/lib/dashboard-snapshot"); + vi.useFakeTimers({ shouldAdvanceTime: true }); + + writeSnapshot( + makeSampleState({ issues: [makeIssue({ id: 42, title: "Immediate" })] }), + "alice", + "immediate", + ); + + // Advance only past setTimeout(0), not 5000ms + await vi.advanceTimersByTimeAsync(10); + vi.useRealTimers(); + await vi.waitFor(async () => { + expect(await loadSnapshot("alice")).not.toBeNull(); + }); + + const snap = await loadSnapshot("alice"); + expect(snap).not.toBeNull(); + expect(snap!.issues[0]?.id).toBe(42); + }); + + it("keeps a pending immediate timer when a debounced write arrives before it fires, committing the latest payload once", async () => { + const putCount = await installPutCounter(); + const { writeSnapshot, loadSnapshot } = await import("../../src/app/lib/dashboard-snapshot"); + vi.useFakeTimers({ shouldAdvanceTime: true }); + + writeSnapshot( + makeSampleState({ issues: [makeIssue({ id: 1, title: "Immediate" })] }), + "alice", + "immediate", + ); + writeSnapshot( + makeSampleState({ issues: [makeIssue({ id: 2, title: "Debounced" })] }), + "alice", + "debounced", + ); + + // The debounced call reuses the pending 0ms timer instead of rescheduling at DEBOUNCE_MS + expect(vi.getTimerCount()).toBe(1); + + await vi.advanceTimersByTimeAsync(10); + vi.useRealTimers(); + await vi.waitFor(() => { + expect(putCount()).toBe(1); + }); + + const snap = await loadSnapshot("alice"); + expect(snap?.issues.map((i) => i.id)).toEqual([2]); + }); + + it("replaces a pending debounced timer when an immediate write arrives, committing on the next macrotask once", async () => { + const putCount = await installPutCounter(); + const { writeSnapshot, loadSnapshot } = await import("../../src/app/lib/dashboard-snapshot"); + vi.useFakeTimers({ shouldAdvanceTime: true }); + + writeSnapshot( + makeSampleState({ issues: [makeIssue({ id: 1, title: "Debounced" })] }), + "alice", + "debounced", + ); + writeSnapshot( + makeSampleState({ issues: [makeIssue({ id: 2, title: "Immediate" })] }), + "alice", + "immediate", + ); + + // The DEBOUNCE_MS timer is cleared; only the 0ms timer remains + expect(vi.getTimerCount()).toBe(1); + + // Advance only past setTimeout(0), not DEBOUNCE_MS + await vi.advanceTimersByTimeAsync(10); + vi.useRealTimers(); + await vi.waitFor(() => { + expect(putCount()).toBe(1); + }); + + const snap = await loadSnapshot("alice"); + expect(snap?.issues.map((i) => i.id)).toEqual([2]); + }); + + it("cancels a pending debounced write when clearSnapshot() runs before debounce elapses", async () => { + const { writeSnapshot, clearSnapshot, loadSnapshot, DEBOUNCE_MS } = await import( + "../../src/app/lib/dashboard-snapshot" + ); + vi.useFakeTimers({ shouldAdvanceTime: true }); + + writeSnapshot(makeSampleState(), "alice", "debounced"); + const clearPromise = clearSnapshot(); + await vi.advanceTimersByTimeAsync(DEBOUNCE_MS + 1_000); + vi.useRealTimers(); + await clearPromise; + + expect(await loadSnapshot("alice")).toBeNull(); + }); + + it("drops an in-flight flush awaiting getDb() when clearSnapshot() runs concurrently", async () => { + vi.resetModules(); + const idb = await import("idb"); + + let resolveOpenDb!: () => void; + const openGate = new Promise((resolve) => { + resolveOpenDb = resolve; + }); + let putCalls = 0; + + vi.mocked(idb.openDB).mockImplementation(async (...args: Parameters) => { + await openGate; + const db = await idbHoisted.realOpenDB(...args); + const origPut = db.put.bind(db); + vi.spyOn(db, "put").mockImplementation((async (...putArgs: Parameters) => { + putCalls++; + return origPut(...putArgs); + }) as typeof db.put); + return db; + }); + + const { writeSnapshot, flushSnapshot, clearSnapshot, loadSnapshot } = await import( + "../../src/app/lib/dashboard-snapshot" + ); + + writeSnapshot(makeSampleState(), "alice", "debounced"); + const flushPromise = flushSnapshot(); + + // While flushSnapshot is suspended awaiting getDb(), clearSnapshot runs synchronously + const clearPromise = clearSnapshot(); + + // Release getDb() + resolveOpenDb(); + await Promise.all([flushPromise, clearPromise]); + + expect(putCalls).toBe(0); + expect(await loadSnapshot("alice")).toBeNull(); + }); + + it("skips writing and logs console.warn once when total items exceed MAX_SNAPSHOT_ITEMS (5000)", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const { writeSnapshot, loadSnapshot, MAX_SNAPSHOT_ITEMS } = await import( + "../../src/app/lib/dashboard-snapshot" + ); + + const oversizedIssues = Array.from({ length: MAX_SNAPSHOT_ITEMS + 1 }, (_, i) => + makeIssue({ id: i + 1 }), + ); + + writeSnapshot(makeSampleState({ issues: oversizedIssues }), "alice", "immediate"); + writeSnapshot(makeSampleState({ issues: oversizedIssues }), "alice", "immediate"); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("exceeds 5000 items")); + expect(await loadSnapshot("alice")).toBeNull(); + }); + + it("queues one stored-snapshot delete per over-cap streak, re-arming after an under-cap write", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.resetModules(); + const idb = await import("idb"); + let deleteCount = 0; + vi.mocked(idb.openDB).mockImplementation(async (...args: Parameters) => { + const db = await idbHoisted.realOpenDB(...args); + const origDelete = db.delete.bind(db); + vi.spyOn(db, "delete").mockImplementation(async (...deleteArgs: Parameters) => { + deleteCount++; + return origDelete(...deleteArgs); + }); + return db; + }); + const { writeSnapshot, flushSnapshot, MAX_SNAPSHOT_ITEMS } = await import( + "../../src/app/lib/dashboard-snapshot" + ); + const oversized = makeSampleState({ + issues: Array.from({ length: MAX_SNAPSHOT_ITEMS + 1 }, (_, i) => makeIssue({ id: i + 1 })), + }); + + writeSnapshot(oversized, "alice", "immediate"); + writeSnapshot(oversized, "alice", "immediate"); + writeSnapshot(oversized, "alice", "debounced"); + await flushSnapshot(); + expect(deleteCount).toBe(1); + + writeSnapshot(makeSampleState(), "alice", "immediate"); + await flushSnapshot(); + writeSnapshot(oversized, "alice", "immediate"); + await flushSnapshot(); + expect(deleteCount).toBe(2); + }); + + it("orders the over-cap discard after an under-cap put that is already in flight", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + let releasePut!: () => void; + const putGate = new Promise((resolve) => { + releasePut = resolve; + }); + const putCount = await installPutCounter(putGate); + const { writeSnapshot, flushSnapshot, loadSnapshot, MAX_SNAPSHOT_ITEMS } = await import( + "../../src/app/lib/dashboard-snapshot" + ); + + writeSnapshot(makeSampleState(), "alice", "debounced"); + const inFlight = flushSnapshot(); + await vi.waitFor(() => expect(putCount()).toBe(1)); + + const oversizedIssues = Array.from({ length: MAX_SNAPSHOT_ITEMS + 1 }, (_, i) => + makeIssue({ id: i + 1 }), + ); + writeSnapshot(makeSampleState({ issues: oversizedIssues }), "alice", "immediate"); + + releasePut(); + await inFlight; + await flushSnapshot(); + + expect(await loadSnapshot("alice")).toBeNull(); + }); + + it("discards the stored snapshot and any pending under-cap write when a later write exceeds MAX_SNAPSHOT_ITEMS", async () => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + const { writeSnapshot, flushSnapshot, loadSnapshot, MAX_SNAPSHOT_ITEMS, DEBOUNCE_MS } = + await import("../../src/app/lib/dashboard-snapshot"); + + writeSnapshot(makeSampleState(), "alice", "immediate"); + await flushSnapshot(); + expect(await loadSnapshot("alice")).not.toBeNull(); + + const oversizedIssues = Array.from({ length: MAX_SNAPSHOT_ITEMS + 1 }, (_, i) => + makeIssue({ id: i + 1 }), + ); + + vi.useFakeTimers({ shouldAdvanceTime: true }); + writeSnapshot( + makeSampleState({ issues: [makeIssue({ id: 2, title: "Pending" })] }), + "alice", + "debounced", + ); + writeSnapshot(makeSampleState({ issues: oversizedIssues }), "alice", "immediate"); + + // The pending debounced timer is cancelled and no new timer is scheduled + expect(vi.getTimerCount()).toBe(0); + + await vi.advanceTimersByTimeAsync(DEBOUNCE_MS + 1_000); + vi.useRealTimers(); + await flushSnapshot(); + + expect(await loadSnapshot("alice")).toBeNull(); + }); + + it("flushes a pending debounced snapshot when document visibilitychange transitions to hidden (and ignores visible)", async () => { + const { writeSnapshot, loadSnapshot } = await import("../../src/app/lib/dashboard-snapshot"); + + writeSnapshot( + makeSampleState({ issues: [makeIssue({ id: 99, title: "Hidden flush" })] }), + "alice", + "debounced", + ); + + Object.defineProperty(document, "visibilityState", { + value: "visible", + configurable: true, + }); + document.dispatchEvent(new Event("visibilitychange")); + expect(await loadSnapshot("alice")).toBeNull(); + + Object.defineProperty(document, "visibilityState", { + value: "hidden", + configurable: true, + }); + document.dispatchEvent(new Event("visibilitychange")); + + await vi.waitFor(async () => { + const snap = await loadSnapshot("alice"); + expect(snap).not.toBeNull(); + expect(snap!.issues[0]?.id).toBe(99); + }); + }); + + it("pushes a single warning notification and does not throw on IDB quota error", async () => { + vi.resetModules(); + const idb = await import("idb"); + vi.spyOn(console, "warn").mockImplementation(() => {}); + + vi.mocked(idb.openDB).mockImplementation(async (...args: Parameters) => { + const db = await idbHoisted.realOpenDB(...args); + vi.spyOn(db, "put").mockRejectedValue( + new DOMException("QuotaExceededError", "QuotaExceededError"), + ); + return db; + }); + + const { writeSnapshot, flushSnapshot } = await import("../../src/app/lib/dashboard-snapshot"); + writeSnapshot(makeSampleState(), "alice", "immediate"); + + await expect(flushSnapshot()).resolves.toBeUndefined(); + expect(mockPushNotification).toHaveBeenCalledTimes(1); + expect(mockPushNotification).toHaveBeenCalledWith( + "snapshot:write", + expect.stringContaining("storage may be full"), + "warning", + ); + }); +}); diff --git a/tests/lib/grouping.test.ts b/tests/lib/grouping.test.ts index 5f84bd05..97904f7e 100644 --- a/tests/lib/grouping.test.ts +++ b/tests/lib/grouping.test.ts @@ -1,5 +1,16 @@ import { describe, it, expect } from "vitest"; -import { groupByRepo, computePageLayout, slicePageGroups, isUserInvolved, ensureLockedRepoGroups, applyCustomOrder, type RepoGroup } from "../../src/app/lib/grouping"; +import { + groupByRepo, + computePageLayout, + slicePageGroups, + isUserInvolved, + ensureLockedRepoGroups, + applyCustomOrder, + groupKeys, + indexGroups, + arraysShallowEqual, + type RepoGroup, +} from "../../src/app/lib/grouping"; interface Item { repoFullName: string; @@ -339,3 +350,56 @@ describe("applyCustomOrder", () => { expect(result[1]).toBe(item); }); }); + +describe("groupKeys", () => { + it("returns empty array for empty groups", () => { + expect(groupKeys([])).toEqual([]); + }); + + it("extracts repoFullName in group order", () => { + const groups = [makeGroup("org/alpha", 2), makeGroup("org/beta", 1), makeGroup("org/gamma", 3)]; + expect(groupKeys(groups)).toEqual(["org/alpha", "org/beta", "org/gamma"]); + }); +}); + +describe("indexGroups", () => { + it("returns empty Map for empty groups", () => { + const map = indexGroups([]); + expect(map.size).toBe(0); + }); + + it("indexes groups by repoFullName preserving group references", () => { + const g1 = makeGroup("org/alpha", 2); + const g2 = makeGroup("org/beta", 1); + const map = indexGroups([g1, g2]); + expect(map.size).toBe(2); + expect(map.get("org/alpha")).toBe(g1); + expect(map.get("org/beta")).toBe(g2); + expect(map.get("org/missing")).toBeUndefined(); + }); +}); + +describe("arraysShallowEqual", () => { + it("returns true for the same array reference", () => { + const arr = ["org/a", "org/b"]; + expect(arraysShallowEqual(arr, arr)).toBe(true); + }); + + it("returns true for two empty arrays", () => { + expect(arraysShallowEqual([], [])).toBe(true); + }); + + it("returns true for distinct arrays with identical elements in order", () => { + expect(arraysShallowEqual(["org/a", "org/b"], ["org/a", "org/b"])).toBe(true); + }); + + it("returns false when lengths differ", () => { + expect(arraysShallowEqual(["org/a"], ["org/a", "org/b"])).toBe(false); + expect(arraysShallowEqual(["org/a", "org/b"], ["org/a"])).toBe(false); + }); + + it("returns false when elements differ or are in different order", () => { + expect(arraysShallowEqual(["org/a", "org/b"], ["org/a", "org/c"])).toBe(false); + expect(arraysShallowEqual(["org/a", "org/b"], ["org/b", "org/a"])).toBe(false); + }); +}); diff --git a/tests/lib/settings-transfer.test.ts b/tests/lib/settings-transfer.test.ts index 6aac7cbc..f8ad5548 100644 --- a/tests/lib/settings-transfer.test.ts +++ b/tests/lib/settings-transfer.test.ts @@ -6,6 +6,12 @@ import type { z } from "zod"; vi.mock("../../src/app/stores/cache", () => ({ clearCache: vi.fn().mockResolvedValue(undefined), })); +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), +})); vi.mock("@sentry/solid", () => ({ captureException: vi.fn(), captureMessage: vi.fn(), diff --git a/tests/services/events-poll.test.ts b/tests/services/events-poll.test.ts index 70b5ced3..ff3f57af 100644 --- a/tests/services/events-poll.test.ts +++ b/tests/services/events-poll.test.ts @@ -79,9 +79,11 @@ vi.mock("../../src/app/stores/config", () => ({ }, })); +const mockIdentityEpoch = vi.fn(() => 0); vi.mock("../../src/app/stores/auth", () => ({ user: vi.fn(() => null), onAuthCleared: vi.fn(), + identityEpoch: () => mockIdentityEpoch(), })); vi.mock("../../src/app/services/api-usage", () => ({ @@ -488,6 +490,11 @@ describe("createEventsPollCoordinator", () => { coordinator!.destroy(); expect(onTargetedData).toHaveBeenCalledTimes(1); + expect(onTargetedData).toHaveBeenCalledWith( + expect.objectContaining({ issues: [], pullRequests: [], workflowRuns: [], errors: [] }), + ["owner/repo"], + 0, + ); }); it("does NOT call onTargetedData when changed=false", async () => { diff --git a/tests/services/github.test.ts b/tests/services/github.test.ts index 1af52f9f..f64bafee 100644 --- a/tests/services/github.test.ts +++ b/tests/services/github.test.ts @@ -1,5 +1,12 @@ import "fake-indexeddb/auto"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +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), +})); import { createRoot } from "solid-js"; import { createGitHubClient, cachedRequest, getClient, initClientWatcher, getGraphqlRateLimit, updateGraphqlRateLimit, updateRateLimitFromHeaders, getCoreRateLimit, fetchRateLimitDetails, onApiRequest, APPROVE_PR_MUTATION, MARK_PR_READY_MUTATION, GITHUB_NODE_ID_RE, GIT_SHA_RE, REQUEST_TIMEOUT_MS, MAX_INLINE_RATE_LIMIT_RETRY_AFTER_SEC, type ApiRequestInfo } from "../../src/app/services/github"; import { clearCache } from "../../src/app/stores/cache"; diff --git a/tests/services/hot-poll.test.ts b/tests/services/hot-poll.test.ts index 37bd818b..6e6bbd56 100644 --- a/tests/services/hot-poll.test.ts +++ b/tests/services/hot-poll.test.ts @@ -60,10 +60,12 @@ vi.mock("../../src/app/stores/config", () => ({ }, })); +const mockIdentityEpoch = vi.fn(() => 0); vi.mock("../../src/app/stores/auth", () => ({ user: vi.fn(() => null), onAuthCleared: vi.fn(), expireToken: vi.fn(), + identityEpoch: () => mockIdentityEpoch(), })); // Import AFTER mocks are set up @@ -795,7 +797,12 @@ describe("createHotPollCoordinator", () => { createHotPollCoordinator(() => 30, onHotData); // First cycle fires after 30s await vi.advanceTimersByTimeAsync(30_000); - expect(onHotData).toHaveBeenCalled(); + expect(onHotData).toHaveBeenCalledWith( + expect.any(Map), + expect.any(Map), + expect.any(Number), + 0, + ); dispose(); }); }); @@ -1198,6 +1205,48 @@ describe("createHotPollCoordinator", () => { }); }); + it("ignores a 401 from a cycle started under a previous identity epoch instead of expiring the current token", async () => { + const onHotData = vi.fn(); + let rejectFetch!: (err: unknown) => void; + const octokit = makeOctokit(undefined, () => + new Promise((_resolve, reject) => { + rejectFetch = reject; + })); + mockGetClient.mockReturnValue(octokit); + + rebuildHotSets({ + ...emptyData, + pullRequests: [makePullRequest({ id: 1, checkStatus: "pending", enriched: true, nodeId: "PR_a" })], + }); + + const { expireToken } = await import("../../src/app/stores/auth"); + const { pushError } = await import("../../src/app/lib/errors"); + (expireToken as ReturnType).mockClear(); + (pushError as ReturnType).mockClear(); + mockLocationReplace.mockClear(); + + try { + await createRoot(async (dispose) => { + createHotPollCoordinator(() => 10, onHotData); + await vi.advanceTimersByTimeAsync(10_000); + expect(rejectFetch).toBeDefined(); + + // Identity changes while the cycle's fetch is in flight + mockIdentityEpoch.mockImplementation(() => 1); + rejectFetch(Object.assign(new Error("Bad credentials"), { status: 401 })); + await vi.advanceTimersByTimeAsync(0); + + expect(expireToken).not.toHaveBeenCalled(); + expect(mockLocationReplace).not.toHaveBeenCalled(); + expect(pushError).not.toHaveBeenCalledWith("hot-poll", expect.any(String), true); + expect(onHotData).not.toHaveBeenCalled(); + dispose(); + }); + } finally { + mockIdentityEpoch.mockImplementation(() => 0); + } + }); + it("does not schedule another cycle after a 401 redirect", async () => { const onHotData = vi.fn(); const octokit = makeOctokit(undefined, () => diff --git a/tests/services/poll-fetchAllData.test.ts b/tests/services/poll-fetchAllData.test.ts index 4c1b46d8..37b7fd56 100644 --- a/tests/services/poll-fetchAllData.test.ts +++ b/tests/services/poll-fetchAllData.test.ts @@ -23,6 +23,7 @@ vi.mock("../../src/app/stores/config", () => ({ vi.mock("../../src/app/stores/auth", () => ({ user: vi.fn(() => ({ login: "octocat", avatar_url: "https://github.com/images/error/octocat_happy.gif", name: "Octocat" })), onAuthCleared: vi.fn(), + identityEpoch: vi.fn(() => 0), })); // Mock the fetch functions (combined issues+PRs and workflow runs) @@ -464,66 +465,6 @@ describe("fetchAllData — upstream repos and tracked users", () => { }); }); -// ── DashboardPage fine-grained merge: surfacedBy preserved ──────────────────── - -describe("DashboardPage pollFetch — fine-grained merge preserves surfacedBy", () => { - it("surfacedBy is copied into the store during the canMerge path", () => { - // Test the canMerge loop logic directly (same code as DashboardPage.tsx pollFetch). - // Using Record to avoid the 'never' collapse from conflicting - // enriched: false vs enriched: true literal types. - type MutablePR = Record & { id: number }; - - const pr: MutablePR = { - id: 5001, - surfacedBy: ["mainuser", "trackeduser"], - enriched: false, - }; - - const enriched: MutablePR = { - id: 5001, - headSha: "abc123", - assigneeLogins: [], - reviewerLogins: [], - checkStatus: "success", - additions: 5, - deletions: 2, - changedFiles: 1, - comments: 0, - reviewThreads: 0, - totalReviewCount: 0, - enriched: true, - nodeId: "PR_node_5001", - surfacedBy: ["mainuser", "trackeduser"], - }; - - const state = { pullRequests: [pr] }; - const enrichedMap = new Map([[5001, enriched]]); - - // Simulate the canMerge loop from DashboardPage.tsx - for (let i = 0; i < state.pullRequests.length; i++) { - const e = enrichedMap.get(state.pullRequests[i].id)!; - const p = state.pullRequests[i]; - p["headSha"] = e["headSha"]; - p["assigneeLogins"] = e["assigneeLogins"]; - p["reviewerLogins"] = e["reviewerLogins"]; - p["checkStatus"] = e["checkStatus"]; - p["additions"] = e["additions"]; - p["deletions"] = e["deletions"]; - p["changedFiles"] = e["changedFiles"]; - p["comments"] = e["comments"]; - p["reviewThreads"] = e["reviewThreads"]; - p["totalReviewCount"] = e["totalReviewCount"]; - p["enriched"] = e["enriched"]; - p["nodeId"] = e["nodeId"]; - p["surfacedBy"] = e["surfacedBy"]; - } - - expect(state.pullRequests[0]["surfacedBy"]).toEqual(["mainuser", "trackeduser"]); - expect(state.pullRequests[0]["enriched"]).toBe(true); - expect(state.pullRequests[0]["checkStatus"]).toBe("success"); - }); -}); - // ── 401 propagation from Promise.allSettled ─────────────────────────────────── describe("fetchAllData — 401 propagation from allSettled", () => { diff --git a/tests/services/poll.test.ts b/tests/services/poll.test.ts index c304cf3b..2158bac8 100644 --- a/tests/services/poll.test.ts +++ b/tests/services/poll.test.ts @@ -5,6 +5,7 @@ import { createPollCoordinator, processBackfillErrors, resetPollState, + StaleIdentityError, WAKE_GRACE_MS, ONLINE_RECOVERY_GRACE_MS, type DashboardData, @@ -1076,6 +1077,31 @@ describe("createPollCoordinator", () => { dispose(); }); }); + + it("treats StaleIdentityError from fetchAll as a silent no-op without pushing errors or dispatching notifications", async () => { + const notificationsMod = await import("../../src/app/lib/notifications"); + vi.mocked(notificationsMod.detectNewItems).mockClear(); + vi.mocked(notificationsMod.dispatchNotifications).mockClear(); + mockPushError.mockClear(); + mockPushNotification.mockClear(); + + const fetchAll = vi.fn().mockRejectedValue(new StaleIdentityError()); + + await createRoot(async (dispose) => { + const coordinator = createPollCoordinator(makeGetInterval(0), fetchAll); + await flushPromises(); + + expect(fetchAll).toHaveBeenCalledTimes(1); + expect(mockPushError).not.toHaveBeenCalled(); + expect(mockPushNotification).not.toHaveBeenCalled(); + expect(notificationsMod.detectNewItems).not.toHaveBeenCalled(); + expect(notificationsMod.dispatchNotifications).not.toHaveBeenCalled(); + expect(coordinator.isRefreshing()).toBe(false); + + coordinator.destroy(); + dispose(); + }); + }); }); describe("processBackfillErrors", () => { diff --git a/tests/stores/auth.test.ts b/tests/stores/auth.test.ts index 641f69d6..8aa744a0 100644 --- a/tests/stores/auth.test.ts +++ b/tests/stores/auth.test.ts @@ -1,8 +1,14 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; // Mock cache.ts so clearAuth() doesn't try to open IndexedDB +const mockClearCache = vi.fn().mockResolvedValue(undefined); vi.mock("../../src/app/stores/cache", () => ({ - clearCache: vi.fn().mockResolvedValue(undefined), + clearCache: (...args: unknown[]) => mockClearCache(...args), +})); + +const mockClearSnapshot = vi.fn().mockResolvedValue(undefined); +vi.mock("../../src/app/lib/dashboard-snapshot", () => ({ + clearSnapshot: (...args: unknown[]) => mockClearSnapshot(...args), })); const mockPushNotification = vi.fn(); @@ -37,6 +43,25 @@ Object.defineProperty(globalThis, "localStorage", { // Each describe block uses vi.resetModules() + dynamic import to get clean signal state. // Tests that seed localStorage must do so BEFORE the dynamic import. +// auth.ts registers a module-scope "storage" listener on every (re-)import, and +// some tests import more than one instance. Track them all and detach after each +// test so listeners from earlier instances don't react to later cross-tab events. +const storageListeners = new Set(); +const originalAddEventListener = window.addEventListener.bind(window); +window.addEventListener = (( + type: string, + listener: EventListenerOrEventListenerObject, + options?: boolean | AddEventListenerOptions, +) => { + if (type === "storage") storageListeners.add(listener); + originalAddEventListener(type, listener, options); +}) as typeof window.addEventListener; + +afterEach(() => { + for (const listener of storageListeners) window.removeEventListener("storage", listener); + storageListeners.clear(); +}); + describe("setAuth / token signal", () => { let mod: typeof import("../../src/app/stores/auth"); @@ -473,10 +498,8 @@ describe("cross-tab auth sync", () => { vi.resetModules(); mod = await import("../../src/app/stores/auth"); // Safe default fetch stub for every test in this describe block. auth.ts - // registers its "storage" listener as a module-level side effect; the - // afterEach below detaches it via `_resetAuthStorageListenerForTests()`, so - // within this describe each `vi.resetModules()` + re-import swaps the listener - // rather than piling new ones onto the shared happy-dom `window`. The stub still guards the + // registers its "storage" listener as a module-level side effect (detached + // after each test by the file-level listener tracker). The stub guards the // current test's own listener: its cross-tab branch fires an un-awaited // `/user` fetch that, without a stub, would hit the real network and abort // on teardown (throwing unhandled AbortError/ERR_INVALID_STATE). Individual @@ -491,7 +514,6 @@ describe("cross-tab auth sync", () => { // still-active mock rather than racing `vi.unstubAllGlobals()` below and // falling through to the real fetch. await new Promise((r) => setTimeout(r, 50)); - mod._resetAuthStorageListenerForTests(); vi.restoreAllMocks(); vi.unstubAllGlobals(); }); @@ -1271,10 +1293,6 @@ describe("cross-tab Jira auth sync", () => { }); afterEach(() => { - // Detach this test's module-scope "storage" listener so re-imports don't - // accumulate listeners on the shared window (same reason as the GitHub - // cross-tab describe above). - mod._resetAuthStorageListenerForTests(); vi.restoreAllMocks(); vi.unstubAllGlobals(); }); @@ -1511,3 +1529,167 @@ describe("setAuthFromPat — identity switch", () => { ); }); }); + +describe("identityEpoch and clearSnapshot integration", () => { + let mod: typeof import("../../src/app/stores/auth"); + + beforeEach(async () => { + localStorageMock.clear(); + mockClearCache.mockReset().mockResolvedValue(undefined); + mockClearSnapshot.mockReset().mockResolvedValue(undefined); + vi.resetModules(); + mod = await import("../../src/app/stores/auth"); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: false, status: 401, json: () => Promise.resolve({}) })); + }); + + afterEach(async () => { + await new Promise((r) => setTimeout(r, 50)); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + }); + + it("clearAuth() calls clearSnapshot() and increments identityEpoch()", () => { + const beforeEpoch = mod.identityEpoch(); + mod.clearAuth(); + expect(mod.identityEpoch()).toBe(beforeEpoch + 1); + expect(mockClearSnapshot).toHaveBeenCalledTimes(1); + }); + + it("expireToken() calls clearSnapshot() and increments identityEpoch()", () => { + const beforeEpoch = mod.identityEpoch(); + mod.expireToken(); + expect(mod.identityEpoch()).toBe(beforeEpoch + 1); + expect(mockClearSnapshot).toHaveBeenCalledTimes(1); + }); + + it("cross-tab auth-key removal calls clearSnapshot() and increments identityEpoch()", () => { + const replaceMock = vi.fn(); + vi.stubGlobal("location", { href: window.location.href, replace: replaceMock }); + mod.setAuth({ access_token: "ghs_abc" }); + localStorageMock.removeItem("github-tracker:auth-token"); + + const beforeEpoch = mod.identityEpoch(); + window.dispatchEvent(new StorageEvent("storage", { + key: "github-tracker:auth-token", + newValue: null, + })); + + expect(mod.identityEpoch()).toBe(beforeEpoch + 1); + expect(mockClearSnapshot).toHaveBeenCalled(); + expect(replaceMock).toHaveBeenCalledWith("/login"); + }); + + it("cross-tab token change to a different identity increments identityEpoch() and clears the snapshot before reload", async () => { + mod.setAuthFromPat("ghs_old", { login: "alice", avatar_url: "https://avatars.githubusercontent.com/u/1", name: "Alice" }); + mockClearSnapshot.mockClear(); + const beforeEpoch = mod.identityEpoch(); + let epochAtReload = -1; + let snapshotClearsAtReload = -1; + + const reloadSpy = vi.spyOn(mod.crossTabReload, "reloadForIdentitySwitch").mockImplementation(() => { + epochAtReload = mod.identityEpoch(); + snapshotClearsAtReload = mockClearSnapshot.mock.calls.length; + }); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: () => Promise.resolve({ login: "bob", avatar_url: "https://avatars.githubusercontent.com/u/2", name: "Bob" }), + })); + + window.dispatchEvent(new StorageEvent("storage", { + key: "github-tracker:auth-token", + newValue: "ghs_bob_token", + })); + + await new Promise((r) => setTimeout(r, 50)); + + expect(reloadSpy).toHaveBeenCalledOnce(); + expect(epochAtReload).toBe(beforeEpoch + 1); + expect(mod.identityEpoch()).toBe(beforeEpoch + 1); + expect(snapshotClearsAtReload).toBe(1); + expect(mockClearSnapshot).toHaveBeenCalledOnce(); + }); + + it("same-user token rotation does NOT increment identityEpoch() or clear the snapshot", async () => { + mod.setAuthFromPat("ghs_old", { login: "alice", avatar_url: "https://avatars.githubusercontent.com/u/1", name: "Alice" }); + mockClearSnapshot.mockClear(); + const beforeEpoch = mod.identityEpoch(); + const reloadSpy = vi.spyOn(mod.crossTabReload, "reloadForIdentitySwitch").mockImplementation(() => {}); + vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ + ok: true, + status: 200, + json: () => Promise.resolve({ login: "ALICE", avatar_url: "https://avatars.githubusercontent.com/u/1", name: "Alice" }), + })); + + window.dispatchEvent(new StorageEvent("storage", { + key: "github-tracker:auth-token", + newValue: "ghs_alice_rotated", + })); + + await new Promise((r) => setTimeout(r, 50)); + + expect(reloadSpy).not.toHaveBeenCalled(); + expect(mod.identityEpoch()).toBe(beforeEpoch); + expect(mockClearSnapshot).not.toHaveBeenCalled(); + }); + + it("clearIdentityData() invokes clearSnapshot() synchronously before clearCache() resolves, awaits both, and survives either rejecting", async () => { + const order: string[] = []; + let resolveCache!: () => void; + let resolveSnapshot!: () => void; + + mockClearCache.mockImplementation(() => { + order.push("clearCache:called"); + return new Promise((resolve) => { + resolveCache = () => { + order.push("clearCache:resolved"); + resolve(); + }; + }); + }); + + mockClearSnapshot.mockImplementation(() => { + order.push("clearSnapshot:called"); + return new Promise((resolve) => { + resolveSnapshot = () => { + order.push("clearSnapshot:resolved"); + resolve(); + }; + }); + }); + + const beforeEpoch = mod.identityEpoch(); + let settled = false; + const clearPromise = mod.clearIdentityData().then(() => { + settled = true; + return undefined; + }); + + // Both clearSnapshot and clearCache are invoked synchronously before the first await resolves + expect(mod.identityEpoch()).toBe(beforeEpoch + 1); + expect(order).toEqual(["clearSnapshot:called", "clearCache:called"]); + expect(settled).toBe(false); + + resolveCache(); + await Promise.resolve(); + await Promise.resolve(); + // Still waiting on snapshotClear + expect(settled).toBe(false); + + resolveSnapshot(); + await clearPromise; + expect(settled).toBe(true); + expect(order).toEqual([ + "clearSnapshot:called", + "clearCache:called", + "clearCache:resolved", + "clearSnapshot:resolved", + ]); + + // Survives clearCache rejecting and still awaits clearSnapshot + vi.spyOn(console, "warn").mockImplementation(() => {}); + mockClearCache.mockRejectedValueOnce(new Error("cache error")); + mockClearSnapshot.mockRejectedValueOnce(new Error("snapshot error")); + await expect(mod.clearIdentityData()).resolves.toBeUndefined(); + }); +}); diff --git a/tests/stores/config.test.ts b/tests/stores/config.test.ts index 156e6040..ad41765d 100644 --- a/tests/stores/config.test.ts +++ b/tests/stores/config.test.ts @@ -1,4 +1,11 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, vi } from "vitest"; + +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), +})); import { ConfigSchema, TrackedUserSchema, loadConfig, config, updateConfig, resetConfig, setMonitoredRepo, addCustomTab, updateCustomTab, removeCustomTab, reorderCustomTab, getCustomTab, isBuiltinTab,