feat: auto-clear stale cache on server version change#3298
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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds build/cache signaling from webviews to the main process, records per-server last observed buildId/cacheVersion, and clears a narrowed set of webview storages (preserving login data) when a build/cache change is detected, then updates server state. Changes
Sequence DiagramsequenceDiagram
participant Preload as Preload (WebView)
participant Main as Main Process
participant Redux as Redux Store
participant Cache as Cache Manager
Preload->>Preload: setServerInfo(commit?, version)
Preload->>Preload: compute buildId (commit.hash || version)
Preload->>Preload: read cache_version from cookies
Preload->>Main: send WEBVIEW_SERVER_BUILD_CHECK {url, buildId?, cacheVersion?}
Main->>Main: lookup server by url
Main->>Main: classify event (first-observation / build/cache change / initial-fill)
alt Build or Cache Change Detected
Main->>Main: acquire buildCheckInFlight lock for url
Main->>Cache: clearWebviewStorageKeepingLoginData()
Cache->>Cache: clear indexdb, serviceworkers, cachestorage (keep login)
Main->>Redux: dispatch WEBVIEW_SERVER_BUILD_UPDATED {url, buildId?, cacheVersion?}
Redux->>Redux: update lastServerBuildId and/or lastCacheVersion
Main->>Main: release buildCheckInFlight lock
else First Observation or Initial Fill
Main->>Redux: dispatch WEBVIEW_SERVER_BUILD_UPDATED (no clearing)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
Use safeSelect in the WEBVIEW_SERVER_BUILD_CHECK listener so an early signal cannot throw when the store has not been hydrated yet, and drop the stale "webview recreate" wording from the log line now that the destroy+remount path is reverted.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/servers/main.ts (1)
240-259: Consider coalescing in-flight events instead of dropping them.Line 241 returns immediately when a check is already in flight. That avoids duplicate clears, but it also drops newer
buildId/cacheVersionsignals that arrive during the clear. A lightweight per-URL “latest pending payload” queue would make this more robust.♻️ Suggested approach
const buildCheckInFlight = new Set<Server['url']>(); +const pendingBuildChecks = new Map< + Server['url'], + { buildId?: string; cacheVersion?: string } +>(); ... - if (buildChanged || cacheVersionChanged) { - if (buildCheckInFlight.has(url)) return; + if (buildChanged || cacheVersionChanged) { + if (buildCheckInFlight.has(url)) { + pendingBuildChecks.set(url, { buildId, cacheVersion }); + return; + } const guestWebContents = getWebContentsByServerUrl(url); if (!guestWebContents) return; buildCheckInFlight.add(url); try { ... await clearWebviewStorageKeepingLoginData(guestWebContents); dispatch({ type: WEBVIEW_SERVER_BUILD_UPDATED, payload: { url, buildId, cacheVersion }, }); } finally { buildCheckInFlight.delete(url); + const pending = pendingBuildChecks.get(url); + if (pending) { + pendingBuildChecks.delete(url); + dispatch({ + type: WEBVIEW_SERVER_BUILD_CHECK, + payload: { url, ...pending }, + }); + } } return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/servers/main.ts` around lines 240 - 259, The current logic drops newer buildChanged/cacheVersion signals when buildCheckInFlight has the URL; instead implement a per-URL “latest pending payload” map (e.g., pendingBuildUpdates keyed by url) so incoming events update the latest {buildId, cacheVersion} rather than returning; in the handler around buildCheckInFlight.add(url) use getWebContentsByServerUrl and clearWebviewStorageKeepingLoginData as now, but in the finally block check pendingBuildUpdates for that url and, if present, loop to process the latest payload (dispatch WEBVIEW_SERVER_BUILD_UPDATED and clear storage) until no pending payload remains; ensure pendingBuildUpdates entries are cleared after processing to avoid leaks and keep using buildChanged/cacheVersion/server.lastServerBuildId values to build the reason string.
🤖 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/main.ts`:
- Around line 240-259: The current logic drops newer buildChanged/cacheVersion
signals when buildCheckInFlight has the URL; instead implement a per-URL “latest
pending payload” map (e.g., pendingBuildUpdates keyed by url) so incoming events
update the latest {buildId, cacheVersion} rather than returning; in the handler
around buildCheckInFlight.add(url) use getWebContentsByServerUrl and
clearWebviewStorageKeepingLoginData as now, but in the finally block check
pendingBuildUpdates for that url and, if present, loop to process the latest
payload (dispatch WEBVIEW_SERVER_BUILD_UPDATED and clear storage) until no
pending payload remains; ensure pendingBuildUpdates entries are cleared after
processing to avoid leaks and keep using
buildChanged/cacheVersion/server.lastServerBuildId values to build the reason
string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d4cb977d-4078-474a-8bf7-b3312304d71e
📒 Files selected for processing (1)
src/servers/main.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 (ubuntu-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-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/main.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/main.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/main.ts
🔇 Additional comments (2)
src/servers/main.ts (2)
215-218: Good guard for uninitialized store state.Using
safeSelecthere prevents a pre-hydration crash and keeps the listener no-op until state is available.
252-256: Cache-clear path aligns well with data-preservation intent.Nice use of
clearWebviewStorageKeepingLoginData(...)before updating server build metadata; this matches the “clear stale bundle cache but keep auth/user prefs” objective.
…deploys Issue 1 (serverBuild.ts): setServerBuildSignals() now queues the signal when the Redux store is not yet initialised instead of silently dropping it. flushPendingBuildSignal() is exported and called from preload.ts immediately after createRendererReduxStore() resolves, replaying any signal that arrived before the store was ready. Issue 2 (reducers.ts): WEBVIEW_SERVER_BUILD_UPDATED now writes gitCommitHash = buildId alongside lastServerBuildId. This keeps the legacy WEBVIEW_GIT_COMMIT_HASH_CHECK path in sync so it does not see a stale hash and trigger a second redundant cache-clear + reload for the same deploy. Tests added: - src/servers/__tests__/reducers.spec.ts — reducer cases for both the gitCommitHash sync (Issue 2) and existing UPDATE behaviour - src/servers/preload/serverBuild.spec.ts — queue/flush coverage for Issue 1 (pre-ready queuing, most-recent-wins, flush idempotency)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/preload/serverBuild.spec.ts`:
- Around line 14-19: In the beforeEach block, exempt the dynamic require by
adding the same ESLint disable comment used elsewhere (place a single-line
disable immediately above the require): add "// eslint-disable-next-line
global-require" (or the project's specific rule used elsewhere) directly above
"const storeMock = require('../../store');" so the require in beforeEach is
permitted; keep the rest of the block (jest.resetModules(),
storeMock.dispatch.mockClear()) unchanged.
- Around line 17-24: The test uses dynamic require() inside jest.isolateModules
(e.g. require('../../store') and require('./serverBuild') which expose
setServerBuildSignals and flushPendingBuildSignal) and CI fails due to
`@typescript-eslint/no-require-imports/Prettier`; fix by either converting the
test to use await import() with async/await and adjusting jest.isolateModules
accordingly, or add inline ESLint disable comments (// eslint-disable-next-line
`@typescript-eslint/no-require-imports`) immediately above each offending
require() call (apply the same change for the other require() occurrences
referenced in the comment).
🪄 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: bc695381-84be-4f9d-b60f-12502780cccd
📒 Files selected for processing (5)
src/preload.tssrc/servers/__tests__/reducers.spec.tssrc/servers/preload/serverBuild.spec.tssrc/servers/preload/serverBuild.tssrc/servers/reducers.ts
✅ Files skipped from review due to trivial changes (1)
- src/servers/tests/reducers.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/servers/reducers.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). (2)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/preload.tssrc/servers/preload/serverBuild.spec.tssrc/servers/preload/serverBuild.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use *.spec.ts file naming convention for Renderer process tests
Files:
src/servers/preload/serverBuild.spec.ts
**/*.{spec,main.spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{spec,main.spec}.ts: Tests must run and pass on Windows, macOS, and Linux CI environments — always verify cross-platform compatibility
Only mock Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) when defensive coding with optional chaining isn't possible
Files:
src/servers/preload/serverBuild.spec.ts
🧠 Learnings (3)
📓 Common learnings
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`.
📚 Learning: 2026-03-06T19:31:11.433Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.433Z
Learning: Applies to src/outlookCalendar/**/preload.ts : Keep preload.ts logging minimal since it cannot access the verbose logging toggle from Redux Store and all logs always appear
Applied to files:
src/preload.ts
📚 Learning: 2026-04-06T18:34:44.375Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-06T18:34:44.375Z
Learning: Applies to **/*.{spec,main.spec}.ts : Tests must run and pass on Windows, macOS, and Linux CI environments — always verify cross-platform compatibility
Applied to files:
src/servers/preload/serverBuild.spec.ts
🪛 GitHub Check: check (ubuntu-latest)
src/servers/preload/serverBuild.spec.ts
[failure] 70-70:
Require statement not part of import statement
[failure] 70-70:
Replace ·setServerBuildSignals,·flushPendingBuildSignal with ⏎········setServerBuildSignals,⏎········flushPendingBuildSignal,⏎·····
[failure] 69-69:
Require statement not part of import statement
[failure] 47-47:
Require statement not part of import statement
[failure] 47-47:
Replace ·setServerBuildSignals,·flushPendingBuildSignal with ⏎········setServerBuildSignals,⏎········flushPendingBuildSignal,⏎·····
[failure] 46-46:
Require statement not part of import statement
[failure] 24-24:
Require statement not part of import statement
[failure] 24-24:
Replace ·setServerBuildSignals,·flushPendingBuildSignal with ⏎········setServerBuildSignals,⏎········flushPendingBuildSignal,⏎·····
[failure] 23-23:
Require statement not part of import statement
[failure] 17-17:
Require statement not part of import statement
🔇 Additional comments (5)
src/preload.ts (1)
64-67: LGTM!The placement of
flushPendingBuildSignal()immediately afterawait createRendererReduxStore()is correct. This ensures:
- The Redux store is fully initialized before any queued build signals are dispatched
- Any signals that arrived during preload initialization are replayed before
server-view/readyis invokedThe sequencing aligns with the design documented in the context snippets.
src/servers/preload/serverBuild.spec.ts (1)
21-108: Test coverage is comprehensive and well-structured.The five test cases thoroughly cover:
- Immediate dispatch when store is ready
- Queuing behavior when store is not ready
- Most recent signal retention (important for race conditions)
- No-op behavior when flushing without pending signals
- Guard against dispatching empty signals
Good use of
jest.isolateModules()to ensure fresh module state per test.src/servers/preload/serverBuild.ts (3)
10-20: Consider the timing ofgetServerUrl()call.
doDispatch()callsgetServerUrl()at dispatch time. This works correctly because:
setServerUrl()is called inpreload.tsbeforecreateRendererReduxStore()flushPendingBuildSignal()is called after store creation- Any direct
setServerBuildSignals()calls (whenstoreReady=true) happen even laterThe current implementation is correct for the documented flow in
preload.ts.
22-32: LGTM!The queuing logic correctly:
- Guards against empty signals (line 24)
- Dispatches immediately when store is ready (line 26)
- Queues only the most recent signal when store is not ready (line 30), providing implicit deduplication for rapid successive calls
38-44: LGTM!The flush implementation correctly:
- Marks
storeReady = truefirst (line 39), so subsequentsetServerBuildSignalscalls dispatch immediately- Clears
pendingSignalbefore dispatching (line 42), preventing potential re-entrancy issues- Guards against no-op when there's no pending signal (line 40)
The JSDoc clearly documents the contract for callers.
| beforeEach(() => { | ||
| jest.resetModules(); | ||
| // Reset mock state | ||
| const storeMock = require('../../store'); | ||
| storeMock.dispatch.mockClear(); | ||
| }); |
There was a problem hiding this comment.
The beforeEach block also contains a flagged require() statement.
Line 17's require('../../store') needs the same ESLint disable treatment.
🔧 Proposed fix
beforeEach(() => {
jest.resetModules();
// Reset mock state
+ // eslint-disable-next-line `@typescript-eslint/no-require-imports`
const storeMock = require('../../store');
storeMock.dispatch.mockClear();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeEach(() => { | |
| jest.resetModules(); | |
| // Reset mock state | |
| const storeMock = require('../../store'); | |
| storeMock.dispatch.mockClear(); | |
| }); | |
| beforeEach(() => { | |
| jest.resetModules(); | |
| // Reset mock state | |
| // eslint-disable-next-line `@typescript-eslint/no-require-imports` | |
| const storeMock = require('../../store'); | |
| storeMock.dispatch.mockClear(); | |
| }); |
🧰 Tools
🪛 GitHub Check: check (ubuntu-latest)
[failure] 17-17:
Require statement not part of import statement
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/servers/preload/serverBuild.spec.ts` around lines 14 - 19, In the
beforeEach block, exempt the dynamic require by adding the same ESLint disable
comment used elsewhere (place a single-line disable immediately above the
require): add "// eslint-disable-next-line global-require" (or the project's
specific rule used elsewhere) directly above "const storeMock =
require('../../store');" so the require in beforeEach is permitted; keep the
rest of the block (jest.resetModules(), storeMock.dispatch.mockClear())
unchanged.
| const storeMock = require('../../store'); | ||
| storeMock.dispatch.mockClear(); | ||
| }); | ||
|
|
||
| it('dispatches immediately when store is already ready', () => { | ||
| jest.isolateModules(() => { | ||
| const storeMock = require('../../store'); | ||
| const { setServerBuildSignals, flushPendingBuildSignal } = require('./serverBuild'); |
There was a problem hiding this comment.
CI is failing due to ESLint/Prettier violations on require() statements.
The jest.isolateModules() pattern with dynamic require() is valid for testing module-level state, but ESLint's @typescript-eslint/no-require-imports rule and Prettier formatting are failing CI.
Options to resolve:
- Add inline disable comments for the ESLint rule
- Use
await import()with async test functions (may require adjustments tojest.isolateModules)
🔧 Proposed fix with ESLint disable comments
it('dispatches immediately when store is already ready', () => {
jest.isolateModules(() => {
+ // eslint-disable-next-line `@typescript-eslint/no-require-imports`
const storeMock = require('../../store');
+ // eslint-disable-next-line `@typescript-eslint/no-require-imports`
- const { setServerBuildSignals, flushPendingBuildSignal } = require('./serverBuild');
+ const {
+ setServerBuildSignals,
+ flushPendingBuildSignal,
+ } = require('./serverBuild');Apply similar changes to all other require() calls in lines 46-47, 69-70, 89-90, and 99-100.
🧰 Tools
🪛 GitHub Check: check (ubuntu-latest)
[failure] 24-24:
Require statement not part of import statement
[failure] 24-24:
Replace ·setServerBuildSignals,·flushPendingBuildSignal with ⏎········setServerBuildSignals,⏎········flushPendingBuildSignal,⏎·····
[failure] 23-23:
Require statement not part of import statement
[failure] 17-17:
Require statement not part of import statement
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/servers/preload/serverBuild.spec.ts` around lines 17 - 24, The test uses
dynamic require() inside jest.isolateModules (e.g. require('../../store') and
require('./serverBuild') which expose setServerBuildSignals and
flushPendingBuildSignal) and CI fails due to
`@typescript-eslint/no-require-imports/Prettier`; fix by either converting the
test to use await import() with async/await and adjusting jest.isolateModules
accordingly, or add inline ESLint disable comments (// eslint-disable-next-line
`@typescript-eslint/no-require-imports`) immediately above each offending
require() call (apply the same change for the other require() occurrences
referenced in the comment).
…ring poisoning
- Compute buildIdSource ('commit' | 'version') in api.ts based on whether
commit.hash is present; propagate through BuildSignals, action payloads,
and all WEBVIEW_SERVER_BUILD_UPDATED dispatch sites in main.ts.
- Reducer: only mirror buildId to gitCommitHash when buildIdSource === 'commit',
preventing version strings (e.g. "7.5.0") from overwriting the legacy hash
baseline and causing a spurious cache-clear on the next setGitCommitHash call.
- main.ts listener: replace naive firstObservation branch with explicit legacy
migration path — when new fields are empty but gitCommitHash exists, compare
incoming commit hash to the stored value; clear only when they differ (server
upgraded between client versions), adopt baseline silently when they match.
- Tests: reducer spec covers buildIdSource=commit mirrors, buildIdSource=version
and absent do not mirror; serverBuild spec asserts buildIdSource forwarded in
both immediate and queued dispatch paths.
…ures - Extract decideBuildCheck pure helper covering all migration branches: commit-source, version-source, cross-source (adopt), new baseline, partial baseline, and noop guard. - Extend WEBVIEW_SERVER_BUILD_CHECK listener to handle legacy server.version baseline (buildIdSource='version'), fixing the case where servers without commit.hash silently skipped cache-clear on upgrade. - Wrap clearWebviewStorageKeepingLoginData in inner try/catch in all clear paths so rejections are logged rather than bubbling as unhandled; baseline stays stale on failure so the next observation retries.
- Extend buildIdSource union with 'autoupdate' throughout actions, serverBuild, buildCheckDecision, reducers, and common Server type - Add lastBundleVersion field to Server type; autoupdate decisions operate on this separate field to avoid conflating with commit-hash/version baselines - Add notifyBundleAutoupdate API on window.RocketChatDesktop; injected.ts sets up a Tracker.autorun on Autoupdate.newClientAvailable() that calls it when Meteor pushes a new client bundle - Update decideBuildCheck to route autoupdate signals through lastBundleVersion path: adopt on first observation, noop on match, clear on change - Reducer writes lastBundleVersion for autoupdate, never touches lastServerBuildId or gitCommitHash — prevents cross-source double-clear with commit.hash path - Add test cases: autoupdate adopt/noop/clear, source isolation, reducer field routing, serverBuild autoupdate dispatch forwarding
The single `lastServerBuildId` field was shared across both 'commit' and
'version' build-id sources. This caused false cache-clears when the same
server was observed via different source paths (e.g. version string on
first boot, then commit hash on restart), because the two incomparable
values were measured against the same baseline.
Fix: replace `lastServerBuildId` with two independent fields:
- `lastCommitBuildId` — baseline for source='commit' observations
- `lastVersionBuildId` — baseline for source='version' observations
`buildCheckDecision.ts` now routes each source to its own baseline.
Cross-source observations (commit baseline + incoming version, or vice
versa) treat the new source as a first observation and adopt rather than
clearing. Same-source mismatches still trigger clear as before.
`reducers.ts` WEBVIEW_SERVER_BUILD_UPDATED handler:
- source='commit' → writes lastCommitBuildId + mirrors to gitCommitHash
- source='version' → writes lastVersionBuildId only (no gitCommitHash)
- source='autoupdate'→ writes lastBundleVersion (unchanged)
- source=undefined → writes neither commit nor version baseline to avoid
cross-source conflation; cacheVersion still recorded
Orphan data note: users who persisted `lastServerBuildId` via redux-persist
(only possible on the feat/smart-cache-clear branch, never shipped to v4.13)
will have an ignored orphan key in their store. No migration needed — the
field is internal, the orphan is harmless, and all paths now use the new
per-source fields.
- Legacy WEBVIEW_GIT_COMMIT_HASH_CHECK listener now short-circuits when server.lastCommitBuildId is already set, deferring to the canonical WEBVIEW_SERVER_BUILD_CHECK path to avoid double-clearing and double- reloading on the same deploy. - Swapped bare select() for safeSelect() in the legacy listener to guard against uninitialized-store throws. - Added comment to api.ts cache_version cookie read documenting it as an optional proxy-supplied signal absent on most stock Rocket.Chat deployments. - injected.ts autoupdate Tracker.autorun gating verified already correct (Autoupdate undefined guard + Meteor.connection._stores optional chaining + try/catch); no changes needed. - Legacy-listener race guard is reviewed-only, not unit-tested (listener lives in main.ts which lacks a unit harness). All 111 servers tests pass.
…aths Previously the commit and version source branches could return 'adopt' when no baseline existed, bypassing the inline cacheVersion comparison entirely. This meant a server that changed cacheVersion while having no stored commit/version baseline would never trigger a cache clear. Hoist the mismatch guard immediately after the empty-payload guard so it fires for all sources (commit, version, autoupdate, sourceless) before any source-specific logic runs. Remove the now-unreachable inline cacheVersion mismatch checks inside the commit and version same-baseline branches. The 'cacheVersion && !lastCacheVersion → adopt' first-adoption case is preserved in those branches. Tests: add 5 cases covering the hoisted guard across all source paths.
…H3/H2)
C2 — SERVERS_LOADED race: replace single pendingSignal with per-source
slot map (pendingByKey) and a server-record-ready gate. After
flushPendingBuildSignal(), if the server URL is not yet in the Redux
servers slice, start a 250 ms interval retry (hard cap 30 s) rather
than dropping the signal silently.
H3 — Multi-slot pending queue: key pending signals by source
('commit' | 'version' | 'autoupdate' | 'cacheVersion-only').
Most-recent-wins per slot; flush dispatches in deterministic order
(commit, version, autoupdate, cacheVersion-only).
H1+M3 — Strengthen legacy listener guard: WEBVIEW_GIT_COMMIT_HASH_CHECK
now short-circuits if ANY of lastCommitBuildId, lastVersionBuildId,
lastBundleVersion, or lastCacheVersion is set — not just
lastCommitBuildId — preventing double-clear for cacheVersion-only
deploys and version-first → commit-later transitions.
H2 — Semver-aware version compare: introduce sameVersion() using
semver.coerce so '7.5.0' and '7.5.0-rc.1' are treated as the same
release and do not false-clear. Commit and autoupdate sources retain
exact string equality (opaque identifiers).
Two high-priority bugs from round-6/auditor-2 reviews:
Bug 1 (in-flight signal drop): When flushAll dispatches multiple
WEBVIEW_SERVER_BUILD_CHECK signals synchronously, the second 'clear'
decision hit the buildCheckInFlight guard and was silently dropped.
Fix: queue any 'clear' payload that arrives while a clear is in-flight
(cap 8 per URL), then re-dispatch each queued entry from the finally
block after buildCheckInFlight is deleted. The listener re-evaluates
against the now-updated baseline — most resolve to noop, genuinely-new
signals queue-and-replay in the same way.
Bug 2 (sameVersion RC regression): semver coerce('7.5.0-rc.1') and
coerce('7.5.0-rc.2') both return '7.5.0', so RC bumps and RC→GA
promotions returned noop instead of clear. Fix: if either version
string contains '-', skip coerce and fall back to strict string
equality. Preserves the original H2 intent (handle 'v7.5.0' vs
'7.5.0' formatting differences) while correctly clearing on every
RC change.
Tests: updated H2 suite (removed two tests that asserted the now-fixed
incorrect behaviour, added v-prefix noop regression guard) and added
new 'RC version changes (H4 regression)' suite with 5 cases covering
rc→rc, rc→GA, GA→rc, same-GA noop, and legacy-rc migration.
Total: 130 tests (was 126).
… queue Bug 1: notifyBundleAutoupdate no longer early-returns when Meteor's private _stores lookup fails or yields no version string. A falsy bundleVersion is replaced with an autoupdate-<timestamp> sentinel so the main process always sees a new buildId → decideBuildCheck returns 'clear' → cache is cleared on every newClientAvailable event. Bug 2: pendingClears is now a single-slot Map<url, payload> instead of Map<url, Array<payload>>. When a clear is in-flight, incoming signals overwrite the slot (latest-wins) rather than appending to a capped queue. The finally block replays exactly one dispatch. This eliminates stale-first replay, wrong-baseline-at-end, and the redundant PENDING_CLEARS_CAP constant. Tests: new api.spec.ts covers sentinel synthesis for undefined, empty, and concrete bundleVersion values (4 cases); all 134 server tests pass.
When Meteor's private `_stores['meteor_autoupdate_clientVersions']`
cannot yield a version string, notifyBundleAutoupdate is called with no
bundleVersion. api.ts synthesizes a unique `autoupdate-${Date.now()}`
sentinel each time — which differed from the previously persisted
lastBundleVersion, triggering another cache-clear reload. After the
reload Tracker.autorun re-fired (newClientAvailable still true, store
still broken) producing another unique sentinel → infinite loop.
Fix: add a closure-scoped `unknownBundleSent` boolean inside the
autoupdateSetup block. On each autorun invocation:
- available === false: reset the flag (re-arm for the next true-edge).
- available === true, bundleVersion known: dispatch unconditionally
(each distinct concrete version is its own edge in the main process).
- available === true, no version: dispatch only once per true-edge
(unknownBundleSent gates the call). Subsequent re-fires while the
store is still broken are silently skipped.
The flag is acceptable as module-level-within-closure state because
injected.ts runs in a single-page renderer lifecycle — one autorun, one
flag, no cross-page contamination. The false→true reset ensures the
guard does not suppress a future genuine update after the page
eventually recovers.
No changes to api.ts or api.spec.ts — the sentinel synthesis at the
api layer remains correct; the gating now lives in injected.ts.
…process The edge-trigger guard in injected.ts (commit 07c4f9f) alone was insufficient: the page context that synthesizes sentinels is torn down on renderer reload, so each reload produces a fresh `autoupdate-${Date.now()}` sentinel that mismatches the persisted `lastBundleVersion`, triggering clear+reload in a loop. Fix lives in the main process (persistent across reloads): `decideBuildCheck` now classifies incoming and persisted `lastBundleVersion` values via `isSentinel()` and applies three rules: - Both synthetic → noop (same true-edge event, different timestamps) - Incoming synthetic, persisted concrete → noop (don't downgrade baseline) - Incoming concrete, persisted synthetic → clear (first real observation) `SENTINEL_PREFIX` is exported from `buildCheckDecision.ts` and imported by `api.ts` to prevent string drift between the synthesizer and the recognizer.
Hardens autoupdate sentinel detection with strict regex `/^autoupdate-\d{10,}$/`.
Prevents collisions with Meteor build labels (e.g. 'autoupdate-2026-04').
Treats non-numeric suffixes as concrete buildIds, not sentinels.
- Regex: 10+ digits required (Date.now() produces 13 digits)
- Added JSDoc on SENTINEL_PREFIX explaining pattern
- New test cases: label-like, empty suffix, short digits, full timestamp
- Updated sentinel tests to use 13-digit timestamps
Replace the single-slot Map<url, payload> with a two-level Map<url, Map<source, payload>> so concurrent signals from different sources (commit, version, autoupdate, cacheVersion-only) are each queued in their own slot and all drained on clear-finally. Previous behaviour silently dropped every source except the last one when a clear was already in-flight for that URL.
…stener (F3)
Route gitCommitHash through the new pipeline instead of a separate
listener that bypassed cooldown, per-source queuing, and the
decideBuildCheck gate.
Removed:
- listen(WEBVIEW_GIT_COMMIT_HASH_CHECK) block in src/servers/main.ts
- WEBVIEW_GIT_COMMIT_HASH_CHANGED action constant + payload type
- WEBVIEW_GIT_COMMIT_HASH_CHECK action constant + payload type
- WEBVIEW_GIT_COMMIT_HASH_CHANGED reducer case
- ActionOf<WEBVIEW_GIT_COMMIT_HASH_CHANGED> from ServersActionTypes union
- reducers.spec.ts describe block for WEBVIEW_GIT_COMMIT_HASH_CHANGED
Changed:
- src/servers/preload/gitCommitHash.ts now calls
setServerBuildSignals({ buildId: hash, buildIdSource: 'commit' })
so all legacy hash signals flow through queue, gate, decideBuildCheck,
cooldown, and per-source pendingClears
gitCommitHash field on Server type is retained; it is still written by
the WEBVIEW_SERVER_BUILD_UPDATED reducer when buildIdSource='commit'.
Meteor's Autoupdate.newClientAvailable() uses skipInitial: true on its DDP watch callback. It never fires on initial doc sync — only on changes. Therefore, any time the callback fires with a new buildId, the client's loaded bundle is provably stale relative to the server's published version. Previous behavior adopted the buildId on first observation (no lastBundleVersion baseline). This meant clients with no persisted state would not clear their cache, even though the autoupdate signal proves staleness. Updated logic: clear on first autoupdate observation, with a reason noting the signal's reliability. The cooldown in main.ts (60s) protects against repeated clears within the same session. Note: commit-source and version-source first-observation behavior unchanged — those rely on REST setServerInfo which fires once per page load with the CURRENT served version. Autoupdate's reactive semantics require clear-on-first.
Adds a 60s cooldown gate keyed by lastClearAt so the same server URL cannot trigger clearWebviewStorageKeepingLoginData more than once per minute. Within the cooldown window the listener adopts the incoming baseline via WEBVIEW_SERVER_BUILD_UPDATED instead of re-clearing. Refactors the in-flight queue around a typed BuildSignalPayload and extracts enqueuePendingClear / flushPendingClears helpers so each source (commit, version, autoupdate, sourceless-buildId, cacheVersion-only) keeps its own pending slot. Also tightens decideBuildCheck branching, adds cacheClear.spec.ts covering the cooldown-adopts and per-source-pending paths, and minor cleanup in injected.ts (destructured Autoupdate access) and serverBuild.ts / api.ts.
Preload setServerBuildSignals collapsed sourceless+buildId signals into the cacheVersion-only slot, silently overwriting a real cacheVersion-only signal queued for the same URL. Main's pendingKey already separated the two cases. Add 'sourceless-buildId' to the SlotKey union and FLUSH_ORDER and route signals with buildId-but-no-source through it.
jeanfbrito
force-pushed
the
feat/smart-cache-clear
branch
from
c3933d9 to
3be1cee
Compare
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
Stability
Tests