Skip to content

fix: stop dashboard refresh loops#1621

Open
gsxdsm wants to merge 1 commit into
mainfrom
fix/dashboard-refresh-loop-pr
Open

fix: stop dashboard refresh loops#1621
gsxdsm wants to merge 1 commit into
mainfrom
fix/dashboard-refresh-loop-pr

Conversation

@gsxdsm

@gsxdsm gsxdsm commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Returning to an existing dashboard tab no longer risks a blank page or a repeated refresh loop after a rebuild. Built assets now revalidate against the running server before falling back to service-worker cache, so stale JS/CSS cannot keep an old app shell alive.

The version checker also remembers which remote build it already auto-reloaded for. If focus or visibility events keep seeing the same remote build from an old bundle, Fusion suppresses repeat reloads instead of refreshing every time the browser regains focus.

Verification

  • pnpm --filter @fusion/dashboard exec vitest run --project dashboard-app app/__tests__/versionCheck.test.ts app/__tests__/pwa.test.ts
  • pnpm --filter @fusion/dashboard build

Compound Engineering
Codex

Summary by CodeRabbit

  • New Features

    • Enhanced service-worker asset revalidation to ensure rebuilt applications use fresh bundles instead of stale cached versions, preventing blank page display.
  • Bug Fixes

    • Fixed redundant page reloads when version mismatches are detected during tab reuse.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Service worker cache-first strategy replaced with network-first for built assets (under /assets/ with script/style/font destinations) to prevent stale bundles; version-check logic now suppresses redundant reloads by tracking already-reloaded remote versions in sessionStorage across navigation.

Changes

Blank Page Prevention via Asset Revalidation and Reload Suppression

Layer / File(s) Summary
Service worker built-asset revalidation
packages/dashboard/app/public/sw.js, packages/dashboard/app/__tests__/pwa.test.ts
Cache version bumped to fusion-cache-v3. Built assets under /assets/ with script/style/font destinations now use network-first strategy with cache fallback. Test validates the detection and caching behavior.
Version-check reload suppression
packages/dashboard/app/versionCheck.ts, packages/dashboard/app/__tests__/versionCheck.test.ts
SessionStorage flag added to track reloaded remote versions. Helpers manage state reads, writes, and clears. Mismatch handler suppresses reloads for versions already reloaded in the same session; first mismatch sets the flag and stores the remote version. Test verifies suppression blocks redundant reload calls.
Release notes and version bump
.changeset/fn-blank-page-service-worker-assets.md
Patch bump documented with guidance that service-worker assets must revalidate before fallback to prevent blank pages from stale bundles.

🎯 3 (Moderate) | ⏱️ ~22 minutes

A rabbit hops through the cache,
Network-first on assets, never a miss,
Reloads suppressed, no more thrash,
Blank pages gone in PWA bliss! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: stop dashboard refresh loops' directly and concisely summarizes the main objective of preventing repeated refresh loops, which is the core problem addressed by the changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dashboard-refresh-loop-pr

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/dashboard/app/__tests__/pwa.test.ts (1)

121-124: ⚡ Quick win

Cover the font destination in this built-asset contract test.

The production branch includes request.destination === "font", but this regression guard currently validates only script/style.

Suggested patch
   expect(swSource).toContain('request.destination === "script"');
   expect(swSource).toContain('request.destination === "style"');
+  expect(swSource).toContain('request.destination === "font"');
   expect(swSource).toContain('if (isBuiltAssetRequest) {');
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/dashboard/app/__tests__/pwa.test.ts` around lines 121 - 124, The
test in packages/dashboard/app/__tests__/pwa.test.ts is asserting the
built-asset contract against swSource but only checks for script/style
destinations; update the assertions to also require the font destination by
adding an expectation that swSource contains the string 'request.destination ===
"font"'. Keep the other checks (e.g., 'url.pathname.startsWith("/assets/")' and
'if (isBuiltAssetRequest) {') unchanged so the test still validates the
built-asset request handling logic in the service worker.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/dashboard/app/__tests__/pwa.test.ts`:
- Around line 121-124: The test in packages/dashboard/app/__tests__/pwa.test.ts
is asserting the built-asset contract against swSource but only checks for
script/style destinations; update the assertions to also require the font
destination by adding an expectation that swSource contains the string
'request.destination === "font"'. Keep the other checks (e.g.,
'url.pathname.startsWith("/assets/")' and 'if (isBuiltAssetRequest) {')
unchanged so the test still validates the built-asset request handling logic in
the service worker.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ac41d34d-5589-4b38-ad9f-d79bc0305189

📥 Commits

Reviewing files that changed from the base of the PR and between c27c321 and cfc580b.

📒 Files selected for processing (5)
  • .changeset/fn-blank-page-service-worker-assets.md
  • packages/dashboard/app/__tests__/pwa.test.ts
  • packages/dashboard/app/__tests__/versionCheck.test.ts
  • packages/dashboard/app/public/sw.js
  • packages/dashboard/app/versionCheck.ts

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR addresses two related problems: a blank-page / reload-loop scenario when returning to a dashboard tab after a rebuild. It adds a network-first caching strategy in the service worker for built assets and introduces a per-tab sessionStorage flag that remembers which remote build version already triggered an auto-reload, suppressing further reloads for that same version.

  • sw.js: Cache name bumped to v3 (forcing old entries to be evicted on activate), and a new isBuiltAssetRequest branch applies network-first / cache-fallback to all /assets/ paths, scripts, styles, and fonts — the same pattern already used for navigation and API requests.
  • versionCheck.ts: A new fusion:version-reloaded-remote sessionStorage key records the remote version that was reloaded for; subsequent mismatch-confirmed checks for the same remote version emit a reload-suppressed trace and return without calling reloadOnce. The flag is cleared when versions match again.
  • .changeset/fn-blank-page-service-worker-assets.md: Missing --- frontmatter fences will cause the changeset CLI to skip this entry entirely during release.

Confidence Score: 3/5

Safe to merge after fixing the changeset file — the SW and versionCheck logic is correct, but the malformed changeset will silently drop this fix from the next release changelog and version bump.

The service-worker and version-check changes are well-reasoned and well-tested. The one hard blocker is the changeset file: it is missing the --- frontmatter fences that the @changesets/cli parser requires, so pnpm release will not pick it up — this patch fix would ship without a version entry or changelog line.

.changeset/fn-blank-page-service-worker-assets.md needs the --- fences added before merge. The versionCheck.ts ordering of setReloadedRemoteVersion vs reloadOnce is worth a follow-up but is not blocking.

Important Files Changed

Filename Overview
.changeset/fn-blank-page-service-worker-assets.md New changeset entry is missing required --- frontmatter fences, causing the changeset CLI to ignore it entirely during release.
packages/dashboard/app/versionCheck.ts Adds RELOADED_REMOTE_VERSION_FLAG to suppress repeat reloads for the same remote version. Logic is sound; minor concern that setReloadedRemoteVersion is called before confirming reloadOnce will proceed.
packages/dashboard/app/public/sw.js Bumps cache name to v3 and adds network-first strategy for built assets (/assets/, scripts, styles, fonts) to prevent stale bundles from persisting across rebuilds. Pattern mirrors the existing navigation and API handlers.
packages/dashboard/app/tests/versionCheck.test.ts Adds a well-structured test for the repeat-reload suppression, correctly simulating post-reload state by manually removing RELOAD_FLAG while keeping RELOADED_REMOTE_VERSION_FLAG in sessionStorage.
packages/dashboard/app/tests/pwa.test.ts Adds string-presence assertions for the new isBuiltAssetRequest branch in sw.js, confirming the key identifiers exist in the compiled service worker source.

Sequence Diagram

sequenceDiagram
    participant Tab as Browser Tab
    participant VC as versionCheck.ts
    participant SS as sessionStorage
    participant SW as sw.js (network-first)
    participant Server as Server

    Tab->>VC: checkVersion("initial")
    VC->>Server: fetch /version.json
    Server-->>VC: "remote="v2" ≠ local="v1""
    VC->>VC: mismatch-PENDING

    Note over Tab: 60s+ passes, user returns to tab

    Tab->>VC: checkVersion("focus")
    VC->>Server: fetch /version.json
    Server-->>VC: "remote="v2" (same)"
    VC->>VC: mismatch-CONFIRMED
    VC->>SS: "set fusion:version-reloaded-remote="v2""
    VC->>Tab: window.location.reload()

    Note over Tab: Page reloads — SW serves fresh assets

    Tab->>SW: fetch /assets/app-xyz.js
    SW->>Server: fetch (network-first)
    Server-->>SW: fresh bundle
    SW-->>Tab: fresh JS

    Tab->>VC: checkVersion("initial")
    VC->>Server: fetch /version.json
    Server-->>VC: "remote="v2" = local="v2""
    VC->>SS: clearReloadedRemoteVersion()

    Note over Tab: User switches away and returns (stale bundle scenario)

    Tab->>VC: checkVersion("focus")
    VC->>VC: mismatch-CONFIRMED
    VC->>SS: "getReloadedRemoteVersion() === "v2"?"
    SS-->>VC: yes
    VC->>VC: emit reload-suppressed, return
Loading

Reviews (1): Last reviewed commit: "fix: stop dashboard refresh loops" | Re-trigger Greptile

Comment on lines +1 to +3
"@runfusion/fusion": patch

Revalidate dashboard service-worker assets before falling back to cache so rebuilt tabs cannot stay on stale bundles and render a blank page.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Missing --- frontmatter fences — changeset will be silently ignored

The changeset file is missing the required --- delimiters. Without them the @changesets/cli parser does not recognize this file as a valid changeset, so pnpm release will not include this entry in the version bump or CHANGELOG.md. Every other changeset in the repo (e.g., fix-frozen-dashboard-animations.md) uses the three-line frontmatter pattern.

Comment on lines +243 to 246
if (autoReloadEnabled) {
setReloadedRemoteVersion(remote);
}
reloadOnce(`build version changed: ${__BUILD_VERSION__} -> ${remote}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 setReloadedRemoteVersion is stored before reloadOnce is called. If reloadOnce is suppressed because RELOAD_FLAG is already in sessionStorage, the remote version gets permanently marked as "already reloaded" even though no version-mismatch reload occurred — silently blocking future reload attempts for that version. Moving the call after the flag check in reloadOnce (or reading RELOAD_FLAG state here first) would ensure the marker is only set when a reload actually proceeds.

Suggested change
if (autoReloadEnabled) {
setReloadedRemoteVersion(remote);
}
reloadOnce(`build version changed: ${__BUILD_VERSION__} -> ${remote}`);
const alreadyAttempted = Boolean((() => { try { return sessionStorage.getItem(RELOAD_FLAG); } catch { return null; } })());
if (autoReloadEnabled && !alreadyAttempted) {
setReloadedRemoteVersion(remote);
}
reloadOnce(`build version changed: ${__BUILD_VERSION__} -> ${remote}`);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant