feat: auto-clear stale cache on server version change#3298
feat: auto-clear stale cache on server version change#3298jeanfbrito wants to merge 3 commits intodevfrom
Conversation
Detects server upgrades pre-auth by reading commit.hash (or version) from /app/utils/rocketchat.info and the cache_version cookie, comparing against per-server persisted values. On change, clears only the Meteor bundle cache (IndexedDB + service workers + cachestorage + HTTP cache) — keeping cookies, localStorage drafts, login token, and UI preferences intact — then destroys and recreates the webview via a React key nonce. The recreate path also fixes a long-standing issue where the webview could get stuck and ignore reload() calls, previously only recoverable by removing the server or restarting the app. - Tighten clearWebviewStorageKeepingLoginData to the minimal storage set - Add lastServerBuildId / lastCacheVersion / webviewNonce fields to Server - Add WEBVIEW_SERVER_BUILD_CHECK pre-auth listener with first-observation recording and change-triggered clear - Add SERVER_WEBVIEW_RECREATE_REQUESTED action that bumps webviewNonce - Update ServersView key to include webviewNonce so React remounts the webview element on bump - Keep the existing post-auth WEBVIEW_GIT_COMMIT_HASH_CHECK as safety net
WalkthroughThe PR implements build and cache version tracking for webview servers. It detects server build or cache version changes from the preload layer, signals the main process, and automatically clears webview storage while preserving login data when changes occur. Changes
Sequence DiagramsequenceDiagram
participant Preload as Preload (WebView)
participant Main as Main Process
participant Redux as Redux State
participant Cache as Cache Manager
Preload->>Preload: setServerInfo() called<br/>with commit hash & version
Preload->>Preload: Compute buildId from commit.hash
Preload->>Preload: Extract cacheVersion from cookies
Preload->>Main: Dispatch WEBVIEW_SERVER_BUILD_CHECK<br/>{url, buildId, cacheVersion}
Main->>Main: Locate server by url
Main->>Main: Classify event:<br/>First observation,<br/>Build change, or<br/>Cache version change?
alt Build or Cache Change Detected
Main->>Main: Acquire buildCheckInFlight lock for url
Main->>Cache: clearWebviewStorageKeepingLoginData()
Cache->>Cache: Clear all but indexdb,<br/>serviceworkers, cachestorage
Main->>Redux: Dispatch WEBVIEW_SERVER_BUILD_UPDATED<br/>{url, buildId, cacheVersion}
Redux->>Redux: Update lastServerBuildId<br/>and lastCacheVersion
else First Observation or Initial Fill
Main->>Redux: Dispatch WEBVIEW_SERVER_BUILD_UPDATED<br/>without clearing storage
end
Main->>Main: Release buildCheckInFlight lock
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/servers/cache.ts`:
- Around line 3-15: Replace the direct use of select with the safeSelect helper
to avoid throwing when the Redux store isn't initialized: update the import line
to pull safeSelect from '../store' (instead of or in addition to select) and
change getServerUrlFromWebContents to call safeSelect(({ servers }) => servers)
so the servers lookup will return undefined safely; keep the rest of
getServerUrlFromWebContents (including the webContentsId match and ?.url)
unchanged.
In `@src/servers/main.ts`:
- Around line 209-263: The listener for WEBVIEW_SERVER_BUILD_CHECK can run
concurrent clear+recreate flows for the same url; add an in-flight deduplication
mechanism (e.g., a module-scoped Set<string> in the same file) keyed by url
inside the listen callback: before starting the clear path (the branch that
calls getWebContentsByServerUrl and await clearWebviewStorageKeepingLoginData),
check the Set and skip if the url is already in-progress, otherwise add the url
to the Set, perform the await, and remove the url from the Set in a finally
block so the flag is always cleared; apply this to the code paths that dispatch
WEBVIEW_SERVER_BUILD_UPDATED after clearWebviewStorageKeepingLoginData so
duplicate handlers won't run concurrently.
In `@src/servers/reducers.ts`:
- Around line 216-229: The reducer branches for WEBVIEW_SERVER_BUILD_UPDATED and
SERVER_WEBVIEW_RECREATE_REQUESTED currently call upsert(state, ...) which can
re-add a server removed during an in-flight async clear; change these to
update-only behavior by first locating the existing Server (using state.find((s)
=> s.url === url) or similar) and if none exists return state unchanged,
otherwise apply an update (not an upsert) to merge the partial patch into the
existing record (for WEBVIEW_SERVER_BUILD_UPDATED set
lastServerBuildId/lastCacheVersion on the found Server; for
SERVER_WEBVIEW_RECREATE_REQUESTED compute nextNonce from existing.webviewNonce
and update that server) so no new server entries are inserted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0066a86a-12e6-4cd0-bd39-5d609f2a5d49
📒 Files selected for processing (9)
src/servers/actions.tssrc/servers/cache.tssrc/servers/common.tssrc/servers/main.tssrc/servers/preload/api.tssrc/servers/preload/serverBuild.tssrc/servers/reducers.tssrc/ui/actions.tssrc/ui/components/ServersView/index.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript for all new code in this codebase unless explicitly told otherwise
Use Fuselage components from@rocket.chat/fuselagefor all UI work — only create custom components when Fuselage doesn't provide the needed functionality
Check Theme.d.ts for valid color tokens when working with Fuselage components
Use optional chaining with fallbacks for platform-specific APIs instead of mocks (e.g., process.getuid?.() ?? 1000) to ensure code works across all platforms without requiring mocks
TypeScript code must use strict mode
Use React functional components with hooks instead of class components
Redux actions must follow the FSA (Flux Standard Action) pattern
Use camelCase for file naming
Use PascalCase for component file names (React components)
Write self-documenting code through clear naming — avoid unnecessary comments
Files:
src/ui/components/ServersView/index.tsxsrc/servers/common.tssrc/servers/preload/serverBuild.tssrc/servers/reducers.tssrc/servers/actions.tssrc/servers/preload/api.tssrc/servers/main.tssrc/ui/actions.tssrc/servers/cache.ts
🔇 Additional comments (4)
src/ui/components/ServersView/index.tsx (1)
12-12: Good use of a nonce-backed key here.This gives
ServerPanea guaranteed remount path when the recreate action fires, which matches the cleanup behavior insrc/ui/components/ServersView/ServerPane.tsx:31-107.src/servers/preload/api.ts (1)
74-81: Nice best-effort signal collection.Using
commit.hashwith a version fallback, plus an optional cookie read, keeps the detection path resilient without makingsetServerInfodepend on either signal being present.src/servers/cache.ts (2)
18-23: Recreate action helper looks good.This keeps the dispatch sites tidy, and the
{ type, payload }shape is FSA-compliant.As per coding guidelines, Redux actions must follow the FSA (Flux Standard Action) pattern.
29-38: Targeted clear + remount flow matches the PR intent.Switching both paths to prefer webview recreation over reload should address the stuck-webview case, and the keep-login variant now limits storage clearing to the bundle-related stores instead of wiping auth/session data.
Also applies to: 47-52
- Use safeSelect in getServerUrlFromWebContents so the helper degrades to reloadIgnoringCache() instead of throwing if the Redux store isn't initialized when a clear is requested. - Add per-URL in-flight dedup around the auto cache-clear branch so a second WEBVIEW_SERVER_BUILD_CHECK for the same server during the async clear window can't trigger a duplicate clear+recreate. - Reducers for WEBVIEW_SERVER_BUILD_UPDATED and SERVER_WEBVIEW_RECREATE_REQUESTED now use update (with an early-return when the server is missing) instead of upsert, so a server removed mid-clear can't be re-inserted as a partial ghost record.
The webview recreate machinery introduced a gray-screen regression on reload: the <webview> remount timing via ReparentingContainer leaves the guest's src assignment silently dropped, leaving an empty surface that can't be recovered without removing and re-adding the server. Remove the Part C surface only: - SERVER_WEBVIEW_RECREATE_REQUESTED action and reducer case - webviewNonce field on Server - React key nonce suffix in ServersView - requestWebviewRecreate dispatches from clearWebviewStorage* paths Keep Parts A and B: the tighter storage clear (indexdb + service workers + cachestorage + HTTP cache, cookies/localStorage preserved) and the pre-auth build-change detection via rocketchat.info commit hash + cache_version cookie. Both the manual "Clear trusted content" dialog and the auto-detection path now fall back to reloadIgnoringCache() after clearing storage.
jeanfbrito
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/servers/reducers.ts (1)
211-218: Avoid no-op state churn when payload has no patch fields.If both
buildIdandcacheVersionare absent, this branch still reachesupdate()with{ url }, creating a needless object update. Early-returning keeps referential stability and avoids unnecessary rerenders.Suggested tweak
case WEBVIEW_SERVER_BUILD_UPDATED: { const { url, buildId, cacheVersion } = action.payload; + if (buildId === undefined && cacheVersion === undefined) return state; if (!state.some((s) => s.url === url)) return state; const patch: Partial<Server> & { url: string } = { url }; if (buildId !== undefined) patch.lastServerBuildId = buildId; if (cacheVersion !== undefined) patch.lastCacheVersion = cacheVersion; return update(state, patch as Server); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/servers/reducers.ts` around lines 211 - 218, The WEBVIEW_SERVER_BUILD_UPDATED branch currently calls update(state, patch as Server) even when payload contains neither buildId nor cacheVersion, causing no-op state churn; modify the handler in reducers.ts to check the extracted buildId and cacheVersion from action.payload and early-return state if both are undefined (i.e., only proceed to construct patch and call update(state, patch as Server) when at least one of buildId or cacheVersion is present), keeping the existing URL existence check (state.some((s) => s.url === url)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/servers/reducers.ts`:
- Around line 211-218: The WEBVIEW_SERVER_BUILD_UPDATED branch currently calls
update(state, patch as Server) even when payload contains neither buildId nor
cacheVersion, causing no-op state churn; modify the handler in reducers.ts to
check the extracted buildId and cacheVersion from action.payload and
early-return state if both are undefined (i.e., only proceed to construct patch
and call update(state, patch as Server) when at least one of buildId or
cacheVersion is present), keeping the existing URL existence check
(state.some((s) => s.url === url)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 64202316-d3ca-4bc0-90e8-0d46d407602b
📒 Files selected for processing (3)
src/servers/cache.tssrc/servers/common.tssrc/servers/reducers.ts
✅ Files skipped from review due to trivial changes (1)
- src/servers/common.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (ubuntu-latest)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript for all new code in this codebase unless explicitly told otherwise
Use Fuselage components from@rocket.chat/fuselagefor all UI work — only create custom components when Fuselage doesn't provide the needed functionality
Check Theme.d.ts for valid color tokens when working with Fuselage components
Use optional chaining with fallbacks for platform-specific APIs instead of mocks (e.g., process.getuid?.() ?? 1000) to ensure code works across all platforms without requiring mocks
TypeScript code must use strict mode
Use React functional components with hooks instead of class components
Redux actions must follow the FSA (Flux Standard Action) pattern
Use camelCase for file naming
Use PascalCase for component file names (React components)
Write self-documenting code through clear naming — avoid unnecessary comments
Files:
src/servers/reducers.tssrc/servers/cache.ts
🧠 Learnings (2)
📚 Learning: 2026-04-01T15:41:58.866Z
Learnt from: jeanfbrito
Repo: RocketChat/Rocket.Chat.Electron PR: 3284
File: src/logging/index.ts:423-445
Timestamp: 2026-04-01T15:41:58.866Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, Rocket.Chat server instances are always deployed at distinct origins (unique hostname or port combinations). Multiple servers sharing the same hostname but differing only by path (e.g., `https://host/team-a` vs `https://host/team-b`) is not a real-world deployment scenario. Therefore, comparing `new URL(s.url).origin` alone is sufficient for matching a webContents URL to its configured server in `src/logging/index.ts`, and path-based collision concerns do not apply.
Applied to files:
src/servers/reducers.tssrc/servers/cache.ts
📚 Learning: 2026-03-11T06:38:40.426Z
Learnt from: Ram-sah19
Repo: RocketChat/Rocket.Chat.Electron PR: 3254
File: .github/workflows/build-release.yml:80-94
Timestamp: 2026-03-11T06:38:40.426Z
Learning: In the RocketChat/Rocket.Chat.Electron repository, the issues flagged in `.github/workflows/build-release.yml` (e.g., `node12` runtime in the release action and missing `snapcraft_token` input), i18n files, and `electron-builder.json` are pre-existing in the `develop` branch and are pulled in during merge conflict resolution. Do not flag these as new issues introduced by PRs that only modify `src/injected.ts` and `src/ui/main/rootWindow.ts`.
Applied to files:
src/servers/reducers.ts
🔇 Additional comments (3)
src/servers/cache.ts (2)
15-17: Scoped storage clearing looks correct for “keep login data”.Limiting
clearStorageDatato IndexedDB, Service Workers, and CacheStorage while reloading ignoring cache matches the intended “preserve auth + user state” behavior.
26-26: Good consistency in post-clear recovery path.Using
guestWebContents.reloadIgnoringCache()here keeps the delete-login flow aligned with the keep-login flow after storage clear.src/servers/reducers.ts (1)
25-25: Good action wiring and reducer typing alignment.
WEBVIEW_SERVER_BUILD_UPDATEDis correctly imported and added toServersActionTypes, so the reducer stays typed for this new action path.Also applies to: 57-57
1 participant
Summary
Detects Rocket.Chat server upgrades and clears the stale client-side Meteor bundle cache that otherwise forces users to reload manually after an admin-side deploy.
What's in
/app/utils/rocketchat.info(Info.commit.hash+Info.version) and thecache_versioncookie, compared against per-server persisted values. On first observation, values are recorded; on subsequent change, the cache is cleared.WEBVIEW_GIT_COMMIT_HASH_CHECKpath kept in place.What's out
An earlier commit on this branch also introduced a
<webview>destroy+remount flow via a React key nonce, intended to rescue stuck webviews. That path produced a gray-screen regression on the second reload and has been reverted in `e69e71fba`. Both the manual "Clear trusted content" dialog and the auto-clear path now fall back to `reloadIgnoringCache()` after clearing storage — the pre-PR behavior on the reload side.Test plan
Summary by CodeRabbit
New Features
Improvements