feat: Enhance Linux support for screen sharing and dependencies#3162
feat: Enhance Linux support for screen sharing and dependencies#3162jeanfbrito merged 33 commits intomasterfrom
Conversation
- Added `xdg-desktop-portal` and `xdg-desktop-portal-gtk` as recommended dependencies in the Electron builder configuration for improved screen sharing functionality on Linux. - Removed the ozone platform switch for `x11` in the Electron startup process and enabled PipeWire screen capture support for Wayland environments. - Updated the display media request handler to utilize system picker for better user experience during screen sharing.
WalkthroughAdds Wayland/portal and internal screen-picker framework with guarded concurrent screen-sharing, automatic GPU-crash detection and relaunch (including AppImage handling), Linux packaging metadata updates, AppImage in CI artifacts, new Linux test scripts and Volta installer, and new Linux display-server documentation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Startup as App Startup
participant GPUHandler as GPU Crash Handler
participant Electron as Electron Process
participant Relaunch as Relaunch Logic
participant Sentinel as Crash Sentinel
Startup->>GPUHandler: setupGpuCrashHandler()
GPUHandler->>Sentinel: read/validate crash timestamps
alt Crashes exceed threshold
GPUHandler->>Relaunch: prepare relaunch args (--disable-gpu, --ozone-platform=x11, ...)
Relaunch->>Electron: spawn detached (AppImage path if APPIMAGE)
Electron-->>Startup: exit current process
else Healthy or within threshold
GPUHandler->>Sentinel: update timestamp/state
end
Startup->>Electron: app.whenReady()
Electron->>Startup: root window shown
Startup->>GPUHandler: markMainWindowStable()
GPUHandler->>Sentinel: adjust stable behavior
sequenceDiagram
autonumber
participant Renderer as Renderer
participant IPC as ScreenSharing IPC Guard
participant Factory as Picker Factory
participant Provider as ScreenPicker Provider
participant Desktop as desktopCapturer/OS Portal
Renderer->>IPC: request getDisplayMedia()
IPC->>IPC: if pending -> return { video: false }
IPC->>IPC: create requestId, start 60s timeout
IPC->>Factory: createScreenPicker()
Factory->>Provider: return cached provider (portal/internal)
IPC->>Provider: handleDisplayMediaRequest(callback)
Provider->>Desktop: trigger source selection (portal or internal UI)
alt source selected
Provider->>IPC: callback with source
IPC->>IPC: validate source, clear timeout, cleanup listeners
IPC->>Renderer: resolve with source
else canceled / error / timeout
IPC->>IPC: cleanup, mark complete
IPC->>Renderer: resolve { video: false }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
src/videoCallWindow/ipc.ts (1)
269-283: Source validation adds defensive handling but introduces a secondgetSourcescall.The validation logic correctly handles the edge case where a source disappears between picker selection and callback. However, this requires a second call to
desktopCapturer.getSources, adding latency.Consider whether the existing
desktopCapturerCache(used elsewhere in this file) could be leveraged here for the validation lookup, with a fallback to fresh fetch only if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
electron-builder.json(2 hunks)package.json(1 hunks)src/app/main/app.ts(1 hunks)src/videoCallWindow/ipc.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/app/main/app.tssrc/videoCallWindow/ipc.ts
{rollup.config.mjs,electron-builder.json,tsconfig.json,.eslintrc.json,jest.config.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain and configure the project via the designated root config files for build, packaging, TypeScript, ESLint, and Jest
Files:
electron-builder.json
🧠 Learnings (3)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.ts
🧬 Code graph analysis (1)
src/videoCallWindow/ipc.ts (1)
src/jitsi/preload.ts (1)
desktopCapturer(11-17)
⏰ 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). (4)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
🔇 Additional comments (6)
package.json (1)
9-9: LGTM!Version bump to 4.10.2 is appropriate for the Linux screen sharing enhancements introduced in this PR.
electron-builder.json (3)
133-140: LGTM!Adding
xdg-desktop-portalandxdg-desktop-portal-gtkas recommended dependencies is the correct approach for enabling PipeWire-based screen sharing. Usingrecommends(soft dependency) ensures the package can still be installed on systems without these portals.
142-147: RPM Recommends configuration looks correct.The
--rpm-tag=Recommends:syntax via fpm is the proper way to add soft dependencies for RPM packages.
149-167: Snap plugs updated correctly for PipeWire support.Adding the
pipewireplug enables the snap to access PipeWire for screen capture, which is essential for Wayland screen sharing.src/app/main/app.ts (1)
141-145: Correct Chromium version for Electron 39.x in verification statement.Electron 39 uses Chromium 142.0.7444.52, not Chromium 134. The
WebRTCPipeWireCapturerfeature flag is valid and correctly implemented for Linux Wayland screen capture support. The code properly applies the flag only for Linux and aligns with the packaging changes for PipeWire and desktop portal support.Likely an incorrect or invalid review comment.
src/videoCallWindow/ipc.ts (1)
256-259: The code correctly handles platform differences—no conflict exists.The
useSystemPickeroption allows use of the system picker and is experimental, available for MacOS 15+ only. The original concern about Linux/PipeWire conflicts with xdg-desktop-portal is unfounded. When the system picker is available and useSystemPicker is set to true, the handler will not be invoked—meaning on macOS 15+, the system picker takes precedence. On Linux and other platforms where system picker is unavailable, the handler is invoked and sends the custom'video-call-window/open-screen-picker'IPC as intended.Remove all references to xdg-desktop-portal and Linux-specific concerns; the code is correct.
Likely an incorrect or invalid review comment.
…p logic - Added a timeout mechanism for screen sharing requests to prevent orphaned listeners. - Introduced cleanup functions for screen sharing listeners and request states to enhance resource management. - Improved IPC handling for screen sharing source responses, ensuring proper request tracking and error handling. - Enhanced user experience by preventing concurrent screen sharing requests and managing listener cleanup effectively.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/videoCallWindow/ipc.ts (1)
307-307: Usesubstringinstead of deprecatedsubstr.
String.prototype.substris deprecated. Usesubstringorslicefor the same functionality.- const requestId = `${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + const requestId = `${Date.now()}-${Math.random().toString(36).substring(2, 11)}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/videoCallWindow/ipc.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/ipc.ts
🧠 Learnings (1)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.ts
🧬 Code graph analysis (1)
src/videoCallWindow/ipc.ts (1)
src/jitsi/preload.ts (1)
desktopCapturer(11-17)
🔇 Additional comments (6)
src/videoCallWindow/ipc.ts (6)
35-47: LGTM! Well-structured state management for screen sharing requests.The module-level state variables for tracking screen sharing requests are properly typed and the 60-second timeout is reasonable for user interaction with the screen picker.
261-277: LGTM! Comprehensive cleanup function addresses the previously flagged resource leak.The
cleanupScreenSharingListenerfunction properly removes the IPC listener using the stored reference, clears the timeout, and resets all state variables. This effectively addresses the concern from the past review about orphaned listeners.
312-356: LGTM! Robust listener implementation with proper validation.The listener correctly validates the
requestIdto prevent stale responses, handles the nullsourceIdcase, and validates the selected source still exists before passing it to the callback. Good defensive programming.
221-223: LGTM! Cleanup integrated into the window cleanup path.Calling
cleanupScreenSharingListener()before removing window listeners ensures screen sharing state is properly reset during window cleanup.
601-603: LGTM! Cleanup integrated into bothclosedandcloseevent handlers.Adding cleanup calls to both the
closedandcloseevent handlers ensures screen sharing resources are released regardless of how the window is closed. The cleanup function is idempotent, so multiple calls are safe.Also applies to: 629-631
284-301: LGTM! Proper guard conditions prevent concurrent requests and handle window unavailability.The guards ensure only one screen sharing request is processed at a time and gracefully handle cases where the video call window is unavailable, both correctly completing the callback to avoid hanging requests.
…cleanup - Added validation to ensure the screen sharing timeout is only processed for the current request, preventing double-invocation. - Enhanced cleanup logic to clear timeout references and invoke callbacks appropriately, improving resource management and user experience during screen sharing sessions.
…completion handling - Introduced `removeScreenSharingListenerOnly` to streamline listener removal and timeout clearing without altering the pending request state. - Added `markScreenSharingComplete` to reset request states, allowing new screen sharing requests after completion. - Enhanced existing cleanup logic to improve resource management and prevent concurrent screen sharing requests.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/videoCallWindow/ipc.ts (1)
307-307: Consider usingsubstringinstead of deprecatedsubstr.The
substrmethod is deprecated. Usesubstringorslicefor future compatibility.- const requestId = `${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + const requestId = `${Date.now()}-${Math.random().toString(36).substring(2, 11)}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/videoCallWindow/ipc.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/ipc.ts
🧠 Learnings (1)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.ts
🧬 Code graph analysis (1)
src/videoCallWindow/ipc.ts (1)
src/jitsi/preload.ts (1)
desktopCapturer(11-17)
⏰ 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: build (windows-latest, windows)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
src/videoCallWindow/ipc.ts (8)
35-47: LGTM! Well-structured screen sharing state management.The new constant and state variables provide comprehensive tracking for the screen sharing request lifecycle. The 60-second timeout is reasonable for user interaction, and the naming clearly conveys the purpose of each variable.
261-277: LGTM! Robust cleanup implementation.The cleanup function properly encapsulates all screen sharing resource cleanup with appropriate null checks. This addresses the resource leak concerns from previous reviews by ensuring listeners, timeouts, and state are always cleaned up consistently.
287-293: LGTM! Effective concurrent request prevention.The guard properly rejects new screen sharing requests while one is pending, preventing race conditions and request interference.
295-301: LGTM! Proper window availability validation.The early return when the video call window is unavailable prevents errors and ensures screen sharing requests are only processed when the window is ready.
312-356: LGTM! Robust listener implementation with proper validation.The listener correctly:
- Validates responses against the current requestId to prevent cross-request interference
- Clears the timeout to prevent double-invocation
- Validates the selected source via
desktopCapturer.getSourcesbefore responding- Handles errors gracefully with fallback responses
This addresses the double-callback concerns from previous reviews.
362-384: LGTM! Timeout handler properly mirrors listener cleanup path.The timeout implementation correctly:
- Validates the requestId before proceeding (lines 365-370) to prevent double-invocation
- Nullifies the timeout reference before cleanup (line 377) to prevent racing callbacks
- Mirrors the listener's cleanup and callback pattern for consistency
This fully addresses the double-callback concerns raised in previous reviews.
284-398: Excellent implementation that addresses all previous review concerns.The refactored screen sharing handler introduces:
- Unique request IDs to prevent cross-request interference
- Guards against concurrent requests
- Proper timeout management to prevent orphaned listeners
- RequestId validation in both listener and timeout paths to prevent double-callbacks
- Comprehensive cleanup at all exit points
The implementation successfully resolves the resource leak and double-callback issues identified in prior reviews.
222-222: LGTM! Comprehensive cleanup integration across window lifecycle.The
cleanupScreenSharingListenercalls are strategically placed at all critical window destruction points:
- In
cleanupVideoCallWindow(line 222) before removing window listeners- In the
closedevent handler (line 618)- In the
closeevent handler (line 646)This ensures no orphaned listeners or dangling state, fully addressing the resource leak concerns from previous reviews.
Also applies to: 617-619, 645-647
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/videoCallWindow/ipc.ts (1)
304-421: Request handler structure is sound, but type safety can be improved.The request handling flow is well-structured with proper guards against concurrent requests, missing windows, and race conditions. The requestId validation prevents double-invocation, and cleanup is properly integrated.
The multiple
as anytype assertions on the callback invocations bypass TypeScript's type checking. Electron bundles TypeScript definitions in its npm package, but the callback signature may require more specific typing. Define a proper interface for the display media result instead of usingany.Consider using UUID for request IDs (line 327) rather than timestamp + random string for stronger uniqueness guarantees, though the current approach is functional for practical purposes.
The code uses
ipcMain.oncebut also manually removes the listener. While safe, this creates redundancy sinceonce()auto-removes on first invocation. Simplify by usingipcMain.on()with explicit manual cleanup only.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/videoCallWindow/ipc.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/ipc.ts
🧠 Learnings (1)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.ts
🧬 Code graph analysis (1)
src/videoCallWindow/ipc.ts (1)
src/jitsi/preload.ts (1)
desktopCapturer(11-17)
⏰ 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). (4)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (macos-latest, mac)
🔇 Additional comments (3)
src/videoCallWindow/ipc.ts (3)
35-47: LGTM: Screen sharing state tracking is well-structured.The timeout constant and state variables provide comprehensive tracking for the screen sharing request lifecycle. The 60-second timeout is reasonable for user interaction with the system picker.
261-297: LGTM: Cleanup functions are well-designed for race condition prevention.The three-tier cleanup approach provides precise control over the screen sharing lifecycle:
cleanupScreenSharingListener: Full cleanup for terminal statesremoveScreenSharingListenerOnly: Prevents re-firing while keeping request activemarkScreenSharingComplete: Resets state to allow new requestsThis separation effectively prevents double-invocation and concurrent request issues.
222-222: LGTM: Screen sharing cleanup is properly integrated into window lifecycle.The cleanup calls are strategically placed in:
cleanupVideoCallWindow(before removing window listeners)- Window
closedevent handler- Window
closeevent handlerThis ensures screen sharing resources are released regardless of how the window is destroyed, preventing the resource leak identified in previous reviews.
Also applies to: 641-641, 669-669
…r Linux - Added a GPU crash handler to automatically switch to X11 mode if the GPU process crashes during startup. - Introduced a persistent fallback mode setting to remember the user's choice across sessions. - Enhanced the settings UI to allow users to control the display server mode, including options for auto-detect, force X11, or disable GPU. - Updated the application to handle GPU fallback mode changes dynamically, improving user experience and stability on Linux systems.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/linux-display-server.md (1)
44-46: Add language specifier to fenced code block.The error message code block should have a language specifier for consistency and to satisfy linting rules. Since this is a log/error output, use
textorlog.-``` +```text ERROR:viz_main_impl.cc:189] Exiting GPU process due to errors during initialization</blockquote></details> <details> <summary>src/app/main/app.ts (2)</summary><blockquote> `66-73`: **Add validation for parsed sentinel data.** The `JSON.parse` result is cast directly to `IStartupSentinel` without validation. If the file is corrupted or has an unexpected structure, this could cause runtime issues. ```diff const readStartupSentinel = (): IStartupSentinel | null => { try { const content = fs.readFileSync(getStartupSentinelPath(), 'utf8'); - return JSON.parse(content) as IStartupSentinel; + const parsed = JSON.parse(content); + if ( + typeof parsed === 'object' && + parsed !== null && + typeof parsed.crashCount === 'number' && + typeof parsed.lastCrashTime === 'number' + ) { + return parsed as IStartupSentinel; + } + return null; } catch { return null; } };
294-301: ReuserelaunchApphelper instead of duplicating logic.The relaunch logic here duplicates
relaunchApp()defined at lines 108-112. Consider using the existing helper for consistency and maintainability.// Save fallback mode and relaunch saveGpuFallbackMode('x11'); console.log('GPU fallback mode set to x11, relaunching...'); - - const command = process.argv.slice(1, app.isPackaged ? 1 : 2); - app.relaunch({ args: command }); - app.exit(0); + relaunchApp(); }); };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
docs/linux-display-server.md(1 hunks)electron-builder.json(2 hunks)src/app/PersistableValues.ts(2 hunks)src/app/main/app.ts(6 hunks)src/app/selectors.ts(1 hunks)src/i18n/en.i18n.json(1 hunks)src/main.ts(3 hunks)src/store/readSetting.ts(2 hunks)src/store/rootReducer.ts(2 hunks)src/ui/actions.ts(3 hunks)src/ui/components/SettingsView/GeneralTab.tsx(2 hunks)src/ui/components/SettingsView/features/GpuFallbackMode.tsx(1 hunks)src/ui/reducers/gpuFallbackMode.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/ui/reducers/gpuFallbackMode.tssrc/store/readSetting.tssrc/ui/components/SettingsView/features/GpuFallbackMode.tsxsrc/app/PersistableValues.tssrc/app/selectors.tssrc/ui/actions.tssrc/store/rootReducer.tssrc/ui/components/SettingsView/GeneralTab.tsxsrc/app/main/app.tssrc/main.ts
src/store/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Write Redux actions following the Flux Standard Action (FSA) convention
Files:
src/store/readSetting.tssrc/store/rootReducer.ts
src/ui/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/ui/**/*.tsx: Implement React UI with functional components and hooks
Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from@rocket.chat/fuselageinstead of raw HTML elements
For Fuselage theming, validate tokens againstTheme.d.tsand only use documented values
Name React component files in PascalCase; non-component files should follow camelCase naming
Files:
src/ui/components/SettingsView/features/GpuFallbackMode.tsxsrc/ui/components/SettingsView/GeneralTab.tsx
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Place all documentation in thedocs/directory using Markdown with clear, descriptive filenames and simple language
Before adding features, check and update existing docs; create/extend docs for new or changed features
Files:
docs/linux-display-server.md
src/main.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep
src/main.tsas the Electron main process entry point compiled by Rollup
Files:
src/main.ts
{rollup.config.mjs,electron-builder.json,tsconfig.json,.eslintrc.json,jest.config.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain and configure the project via the designated root config files for build, packaging, TypeScript, ESLint, and Jest
Files:
electron-builder.json
🧠 Learnings (5)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ui/**/*.tsx : Use Fuselage components (Box, Button, TextInput, Modal, etc.) and import from `rocket.chat/fuselage` instead of raw HTML elements
Applied to files:
src/ui/components/SettingsView/features/GpuFallbackMode.tsx
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/ui/actions.tssrc/main.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/ui/actions.tssrc/app/main/app.tssrc/main.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/app/main/app.tssrc/main.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/main.ts
🧬 Code graph analysis (8)
src/ui/reducers/gpuFallbackMode.ts (5)
src/store/actions.ts (1)
ActionOf(42-42)src/ui/actions.ts (1)
SETTINGS_GPU_FALLBACK_MODE_CHANGED(120-121)src/app/PersistableValues.ts (1)
GpuFallbackMode(95-95)src/ui/components/SettingsView/features/GpuFallbackMode.tsx (1)
GpuFallbackMode(23-78)src/app/actions.ts (1)
APP_SETTINGS_LOADED(6-6)
src/store/readSetting.ts (2)
src/app/PersistableValues.ts (1)
GpuFallbackMode(95-95)src/ui/components/SettingsView/features/GpuFallbackMode.tsx (1)
GpuFallbackMode(23-78)
src/app/PersistableValues.ts (1)
src/ui/components/SettingsView/features/GpuFallbackMode.tsx (1)
GpuFallbackMode(23-78)
src/app/selectors.ts (2)
src/store/rootReducer.ts (1)
RootState(111-111)src/ui/reducers/gpuFallbackMode.ts (1)
gpuFallbackMode(15-30)
src/ui/actions.ts (2)
src/app/PersistableValues.ts (1)
GpuFallbackMode(95-95)src/ui/components/SettingsView/features/GpuFallbackMode.tsx (1)
GpuFallbackMode(23-78)
src/ui/components/SettingsView/GeneralTab.tsx (2)
src/app/PersistableValues.ts (1)
GpuFallbackMode(95-95)src/ui/components/SettingsView/features/GpuFallbackMode.tsx (1)
GpuFallbackMode(23-78)
src/app/main/app.ts (3)
src/ui/reducers/gpuFallbackMode.ts (1)
gpuFallbackMode(15-30)src/store/readSetting.ts (2)
readGpuFallbackMode(20-26)saveGpuFallbackMode(28-45)src/ui/actions.ts (1)
SETTINGS_GPU_FALLBACK_MODE_CHANGED(120-121)
src/main.ts (1)
src/app/main/app.ts (2)
setupGpuCrashHandler(264-302)markMainWindowStable(255-262)
🪛 LanguageTool
docs/linux-display-server.md
[style] ~17-~17: This wording can make your sentence hard to follow. Try rephrasing for improved clarity.
Context: ...GPU process crashes during startup (e.g., due to incompatible drivers), the app detects ...
(DUE_TO_BECAUSE)
🪛 markdownlint-cli2 (0.18.1)
docs/linux-display-server.md
44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (6)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (macos-latest, mac)
🔇 Additional comments (20)
electron-builder.json (3)
137-140: LGTM: Debian recommended dependencies added.The addition of xdg-desktop-portal packages as recommended dependencies correctly supports modern Linux screen sharing. Using "recommends" rather than hard dependencies is the right approach for optional portal support.
166-166: LGTM: PipeWire plug added for Wayland screen capture.Adding the
pipewireplug correctly enables PipeWire-based screen sharing support for snap packages, which is essential for modern Wayland environments.
143-147: Package names are correct across RPM distributions.Both
xdg-desktop-portalandxdg-desktop-portal-gtkare available as canonical packages in Fedora, CentOS, and openSUSE repositories. The RPM Recommends tags use correct fpm syntax and match the actual package names used across major RPM-based distributions.docs/linux-display-server.md (1)
1-146: Well-structured documentation for the new GPU fallback feature.The documentation clearly explains the automatic crash recovery mechanism, Wayland requirements, troubleshooting steps, and manual overrides. This aligns well with the code changes in this PR.
src/ui/components/SettingsView/GeneralTab.tsx (1)
6-6: LGTM!The
GpuFallbackModecomponent is correctly imported and conditionally rendered for Linux only. The placement afterHardwareAccelerationis logical since both relate to GPU/display settings.Also applies to: 25-25
src/store/rootReducer.ts (1)
21-21: LGTM!The
gpuFallbackModereducer is correctly imported and wired into the root reducer. TheRootStatetype will automatically include this new state slice viaReturnType<typeof rootReducer>.Also applies to: 69-69
src/i18n/en.i18n.json (1)
242-250: LGTM!The translation entries are well-structured with clear descriptions. The option labels appropriately map to the underlying values (
auto→none,x11→x11,disabled→disabled).src/app/selectors.ts (1)
74-74: LGTM!The
gpuFallbackModeis correctly added toselectPersistableValues, enabling it to be persisted alongside other settings.src/main.ts (2)
7-8: LGTM!Setting up the GPU crash handler early (after
app.whenReady()but before store creation) is the correct approach. This ensures GPU crashes during startup are caught. The handler reads directly fromconfig.jsonviareadGpuFallbackMode, so it doesn't depend on the Redux store.Also applies to: 59-60
86-87: LGTM!Marking the main window as stable after
showRootWindow()is appropriate. This prevents GPU crashes that occur during normal operation (after successful startup) from triggering the fallback mechanism.src/store/readSetting.ts (2)
20-26: LGTM!The
readGpuFallbackModefunction correctly validates the persisted value and defaults to'none'for unknown or missing values. This is a safe approach.
28-45: LGTM!The
saveGpuFallbackModefunction handles edge cases well:
- Gracefully handles missing or invalid existing config files
- Merges with existing settings rather than overwriting
- Logs errors without throwing, which is appropriate for a non-critical persistence operation
The synchronous file I/O is acceptable here since this runs during startup/crash recovery when blocking is not a concern.
src/ui/actions.ts (1)
3-3: LGTM!The new action constant and type mapping follow existing patterns. The import is correctly sourced from
PersistableValues.ts, and the action naming is consistent with other settings actions in this file.Also applies to: 120-121, 261-261
src/ui/reducers/gpuFallbackMode.ts (1)
1-30: LGTM!The reducer is well-structured with proper type guards for loaded settings. The
isValidFallbackModetype guard provides good validation for potentially corrupted persisted data, and the reducer correctly handles both initialization and user-triggered changes.src/ui/components/SettingsView/features/GpuFallbackMode.tsx (1)
1-78: LGTM!The component is well-implemented following React best practices:
- Correctly uses Fuselage components as per coding guidelines
- Proper memoization of options and callback
- Platform check ensures Linux-only rendering
- Type-safe dispatch with validated payload
src/app/PersistableValues.ts (1)
95-99: LGTM!The new
GpuFallbackModetype and migration follow the established versioning pattern. The migration correctly initializes the new field with a safe default of'none'for existing installations.Also applies to: 182-185
src/app/main/app.ts (4)
199-234: Verify crash detection threshold behavior.The current logic writes
crashCount: 1on every startup and increments on subsequent startups within the crash window. WithMAX_CRASH_COUNT = 2, this triggers fallback after the second consecutive startup attempt (i.e., after only one crash). Please verify this is the intended threshold—typically crash detection uses 3+ failures before fallback.
191-245: LGTM for Linux startup enhancements.The PipeWire screen capture enablement and GPU fallback mode application logic are well-structured. The conditional switches for X11 and disabled modes are correctly applied based on the persisted or detected fallback state.
255-262: LGTM for GPU crash handler and stability marker.The
markMainWindowStablefunction correctly clears the startup sentinel on successful initialization. ThesetupGpuCrashHandlerappropriately guards against non-Linux platforms, existing fallback modes, and post-stability crashes before triggering a fallback.Also applies to: 264-302
348-355: LGTM for settings change listener.The listener correctly saves the new GPU fallback mode and triggers a relaunch on Linux to apply the changes.
…ments on relaunch - Updated the crash detection logic to trigger fallback mode after exceeding the maximum crash count. - Enhanced the relaunch functionality to preserve command-line arguments, ensuring user inputs are retained during application restarts.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/app/main/app.ts(6 hunks)src/videoCallWindow/ipc.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/app/main/app.tssrc/videoCallWindow/ipc.ts
🧠 Learnings (3)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.ts
🧬 Code graph analysis (2)
src/app/main/app.ts (4)
src/ui/reducers/gpuFallbackMode.ts (1)
gpuFallbackMode(15-30)src/store/readSetting.ts (2)
readGpuFallbackMode(20-26)saveGpuFallbackMode(28-45)src/store/index.ts (2)
listen(88-114)listen(144-157)src/ui/actions.ts (1)
SETTINGS_GPU_FALLBACK_MODE_CHANGED(120-121)
src/videoCallWindow/ipc.ts (1)
src/jitsi/preload.ts (1)
desktopCapturer(11-17)
⏰ 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). (6)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: check (macos-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
🔇 Additional comments (10)
src/videoCallWindow/ipc.ts (4)
35-47: LGTM: Screen sharing state tracking is well-structured.The timeout constant and state tracking variables are clearly named and properly typed for managing the screen sharing lifecycle.
261-297: LGTM: Cleanup functions provide appropriate lifecycle management.The three-tier cleanup approach correctly handles different stages of the screen sharing request lifecycle, properly preventing race conditions and concurrent requests.
304-444: Screen sharing handler correctly implements guarded lifecycle management.The implementation properly addresses the concerns from previous reviews:
- Concurrent requests are blocked via
isScreenSharingRequestPendingflag- Resource leaks are prevented with cleanup in window lifecycle events
- Double-invocation is prevented via
callbackInvokedguard- Request ID validation ensures responses match current request
- Timeout handling ensures no orphaned listeners
The use of
ipcMain.once()combined with manual cleanup viaremoveScreenSharingListenerOnly()is redundant (.once()auto-removes after firing), but harmless and provides belt-and-suspenders safety.
221-223: LGTM: Comprehensive cleanup integration in window lifecycle.The screen sharing cleanup is properly integrated at key lifecycle points, ensuring listeners are removed before window destruction and preventing resource leaks.
Also applies to: 663-665, 691-693
src/app/main/app.ts (6)
1-3: LGTM: Imports are appropriate for the new functionality.The added imports for file system operations, GPU fallback mode utilities, and the settings action are correctly included for the startup sentinel and GPU fallback features.
Also applies to: 17-21, 24-24
63-93: LGTM: Sentinel file helpers are well-implemented.The helper functions appropriately handle file I/O errors with try-catch blocks. Silent failure handling is acceptable for sentinel files since missing or corrupted files can be treated as "no previous crash."
255-262: Verify the call site for markMainWindowStable.The function correctly clears the startup sentinel to complete the crash detection flow. Ensure this is called after the main window has fully loaded and stabilized (not just created), to avoid clearing the sentinel too early during startup crashes.
The call site verification is covered by the script in the previous comment.
350-357: LGTM: GPU fallback mode change handler is consistent with other settings.The listener correctly saves the mode and triggers a relaunch on Linux, following the same pattern as other settings that require an app restart (hardware acceleration, transparent window).
264-304: GPU crash handler integration is correct and properly timed.The
setupGpuCrashHandler()is registered early at startup (before the main window is created) and the listener only triggers during early startup whenisMainWindowStableis false. AftermarkMainWindowStable()is called following window display, subsequent GPU crashes are ignored, preventing fallback mode activation after the app has stabilized. The implementation correctly handles GPU process crashes only on Linux and preserves command-line arguments on relaunch.
192-245: The Linux crash detection and GPU fallback logic is correctly implemented. The WebRTCPipeWireCapturer feature requires the --enable-features flag to enable PipeWire in Chromium/Electron, which the code applies correctly at startup. The markMainWindowStable() function is appropriately called after the main window has fully loaded and the app initialization completes, ensuring the startup sentinel is cleared only when the application has successfully stabilized. The "dead man's switch" pattern—writing a sentinel on startup, incrementing crash count on repeated failures within the time window, and clearing on success—correctly prevents false positives while detecting genuine crash loops.
- Added Wayland support to the Electron builder configuration, allowing for improved compatibility with modern Linux environments. - Updated documentation to reflect auto-detection of display servers for Snap packages, enhancing user clarity. - Refactored crash detection logic to use a more descriptive variable name and improved logging for better debugging during GPU fallback scenarios.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/app/main/app.ts (1)
55-55: Clarify whether 2 or 3 crashes should trigger fallback.The constant
CRASH_THRESHOLD = 2combined withnewCrashCount > CRASH_THRESHOLDat line 211 means fallback triggers on the 3rd crash, not the 2nd. If allowing up to 2 crashes before fallback is intentional, consider renaming toMAX_ALLOWED_CRASHESor adjusting the condition to>=if 2 crashes should trigger the fallback.Also applies to: 211-211
🧹 Nitpick comments (1)
src/app/main/app.ts (1)
66-73: Consider validating the parsed sentinel structure.The function parses JSON and casts to
IStartupSentinelwithout validating thatcrashCountandlastCrashTimeexist and are numbers. If the file is corrupted or manually edited, this could lead to undefined behavior downstream.Consider adding validation:
const readStartupSentinel = (): IStartupSentinel | null => { try { const content = fs.readFileSync(getStartupSentinelPath(), 'utf8'); - return JSON.parse(content) as IStartupSentinel; + const data = JSON.parse(content); + if ( + typeof data === 'object' && + typeof data.crashCount === 'number' && + typeof data.lastCrashTime === 'number' + ) { + return data as IStartupSentinel; + } + return null; } catch { return null; } };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
docs/linux-display-server.md(1 hunks)electron-builder.json(2 hunks)src/app/main/app.ts(6 hunks)src/videoCallWindow/ipc.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Place all documentation in thedocs/directory using Markdown with clear, descriptive filenames and simple language
Before adding features, check and update existing docs; create/extend docs for new or changed features
Files:
docs/linux-display-server.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/app/main/app.tssrc/videoCallWindow/ipc.ts
{rollup.config.mjs,electron-builder.json,tsconfig.json,.eslintrc.json,jest.config.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain and configure the project via the designated root config files for build, packaging, TypeScript, ESLint, and Jest
Files:
electron-builder.json
🧠 Learnings (3)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.ts
🧬 Code graph analysis (2)
src/app/main/app.ts (3)
src/ui/reducers/gpuFallbackMode.ts (1)
gpuFallbackMode(15-30)src/store/readSetting.ts (2)
readGpuFallbackMode(20-26)saveGpuFallbackMode(28-45)src/ui/actions.ts (1)
SETTINGS_GPU_FALLBACK_MODE_CHANGED(120-121)
src/videoCallWindow/ipc.ts (1)
src/jitsi/preload.ts (1)
desktopCapturer(11-17)
🪛 LanguageTool
docs/linux-display-server.md
[style] ~17-~17: This wording can make your sentence hard to follow. Try rephrasing for improved clarity.
Context: ...GPU process crashes during startup (e.g., due to incompatible drivers), the app detects ...
(DUE_TO_BECAUSE)
🪛 markdownlint-cli2 (0.18.1)
docs/linux-display-server.md
44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (6)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: check (macos-latest)
- GitHub Check: build (ubuntu-latest, linux)
🔇 Additional comments (17)
electron-builder.json (4)
137-140: LGTM! Correctly adds portal dependencies for screen sharing.The
recommendsfield is the appropriate choice for these packages, as they enhance functionality (screen sharing) without being strictly required for the application to run.
167-167: LGTM! PipeWire plug enables Wayland screen capture.Adding the
pipewireplug is essential for screen sharing functionality on Wayland environments, aligning with the PR objectives.
149-169: No action needed—ozone-platform flag has been removed.The
executableArgsis not present in the snap section, confirming the hardcoded--ozone-platform=x11flag has been successfully removed.
143-147: The fpm syntax and package names are correct for RPM-based distributions.Both xdg-desktop-portal and xdg-desktop-portal-gtk are available in major RPM repositories (Fedora, CentOS/EPEL, and openSUSE). The --rpm-tag option adds custom tags in the spec file, and the Recommends syntax is valid for RPM package metadata. The array format is correct for passing multiple fpm arguments.
docs/linux-display-server.md (1)
23-24: UI implementation confirmed to match documentation.The settings path "Settings > General > Display Server Mode" exists in the implementation. The
GpuFallbackModecomponent renders correctly on Linux with the exact UI labels documented: "Auto-detect (Wayland/X11)" and "Force X11". The GPU crash recovery mechanism is implemented insrc/app/main/app.ts. All documented UI elements, component names, and label text are present and match the documentation precisely.src/videoCallWindow/ipc.ts (7)
35-35: LGTM!The 60-second timeout constant is appropriately named and provides reasonable time for user interaction with the screen picker.
41-48: LGTM!State tracking variables are well-structured with appropriate nullable types for managing the screen sharing request lifecycle.
261-297: Well-designed two-phase cleanup pattern.The separation between
removeScreenSharingListenerOnly(removes listener/timeout but keeps pending flag) andmarkScreenSharingComplete(clears request state) correctly prevents race conditions where a new request could start while async validation is still in progress.
221-223: LGTM!Correct placement of screen sharing cleanup before removing window listeners ensures no orphaned IPC listeners remain.
663-665: LGTM!Defensive cleanup in the
closedhandler provides an additional safety net in case the window is destroyed without thecloseevent firing first.
691-693: LGTM!Screen sharing cleanup in the
closehandler ensures resources are released promptly when the user initiates window closure.
304-444: Implementation correctly handles concurrent screen sharing requests with proper cleanup.The screen sharing flow is well-designed:
- Prevents concurrent requests with
isScreenSharingRequestPendingflag- Uses per-request unique IDs to prevent cross-request interference
- Guards against double-invocation with
callbackInvokedflag in both listener and timeout paths- Cleans up resources on window lifecycle events
The pattern of registering a listener with
ipcMain.once()and storing the reference for early removal viaremoveListener()is valid—both methods work with the same function reference on the EventEmitter API.src/app/main/app.ts (5)
1-3: LGTM!The new imports for file system operations and GPU fallback mode helpers are appropriate for the added Linux crash detection and fallback functionality.
Also applies to: 17-21, 24-24
192-194: LGTM!Enabling
WebRTCPipeWireCapturerfor Linux correctly adds PipeWire support for screen sharing on Wayland, aligning with the PR objectives.
236-244: LGTM!The GPU fallback mode application correctly handles both
'x11'(force X11 display server) and'disabled'(disable GPU entirely) modes based on the persisted setting.
255-262: LGTM!The function correctly marks successful startup and clears the crash sentinel, completing the crash detection lifecycle.
350-357: LGTM!The listener correctly handles user-initiated GPU fallback mode changes on Linux by persisting the setting and relaunching the app.
- Simplified the GPU crash detection process by consolidating logic for tracking crash counts and managing fallback mode. - Improved logging for better visibility during relaunch scenarios. - Ensured that the application preserves command-line arguments when relaunching after a GPU crash.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/main/app.ts (2)
66-73: Consider validating parsed sentinel data.The parsed JSON is cast to
IStartupSentinelwithout validation. If the file is corrupted or manually edited, invalid data could cause issues downstream.Add basic validation:
const readStartupSentinel = (): IStartupSentinel | null => { try { const content = fs.readFileSync(getStartupSentinelPath(), 'utf8'); - return JSON.parse(content) as IStartupSentinel; + const parsed = JSON.parse(content); + if (typeof parsed.crashCount === 'number' && typeof parsed.lastCrashTime === 'number') { + return parsed as IStartupSentinel; + } + return null; } catch { return null; } };
203-207: Potential duplicate GPU disabling logic.When
gpuFallbackMode === 'disabled', this code disables hardware acceleration and GPU. However, lines 154-162 also handle GPU disabling based on--disable-gpuarg orisHardwareAccelerationEnabledsetting.If both conditions are true,
app.disableHardwareAcceleration()would be called twice. While likely harmless, consider consolidating the logic or adding a guard:+ let isGpuDisabled = false; + if ( args.includes('--disable-gpu') || isHardwareAccelerationEnabled === false ) { console.log('Disabling Hardware acceleration'); app.disableHardwareAcceleration(); app.commandLine.appendSwitch('--disable-2d-canvas-image-chromium'); app.commandLine.appendSwitch('--disable-accelerated-2d-canvas'); app.commandLine.appendSwitch('--disable-gpu'); + isGpuDisabled = true; } // ... later in Linux section ... } else if (gpuFallbackMode === 'disabled') { - console.log('GPU fallback mode: GPU disabled'); - app.disableHardwareAcceleration(); - app.commandLine.appendSwitch('disable-gpu'); + if (!isGpuDisabled) { + console.log('GPU fallback mode: GPU disabled'); + app.disableHardwareAcceleration(); + app.commandLine.appendSwitch('disable-gpu'); + } }Also applies to: 154-162
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/app/main/app.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/app/main/app.ts
🧠 Learnings (2)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/app/main/app.ts
⏰ 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). (6)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
🔇 Additional comments (5)
src/app/main/app.ts (5)
1-2: LGTM: Imports support the new GPU crash handling features.The new imports for
fs,path, GPU fallback utilities, and the GPU fallback mode change action are necessary for the startup sentinel and GPU crash handling functionality.Also applies to: 17-21, 24-24
54-61: Verify crash threshold intent: triggers on 3rd crash, not 2nd.
CRASH_THRESHOLD = 2combined withnewCrashCount > CRASH_THRESHOLDat line 266 means the fallback triggers after the 3rd crash, not the 2nd. If the intent is to allow only 2 crashes before fallback:Apply this diff:
- if (newCrashCount > CRASH_THRESHOLD) { + if (newCrashCount >= CRASH_THRESHOLD) {Or if 3 crashes is intentional, add a clarifying comment:
-const CRASH_THRESHOLD = 2; +const CRASH_THRESHOLD = 2; // Triggers fallback after exceeding this count (on 3rd crash)Also applies to: 266-266
218-225: LGTM: Proper lifecycle management for startup sentinel.The function correctly sets the stability flag and clears the sentinel only on Linux. This prevents the GPU crash handler from triggering after the main window is stable.
329-336: LGTM: Proper handling of GPU fallback mode changes.The listener correctly guards Linux-specific functionality, persists the new mode, and uses the
relaunchApp()helper to apply changes.
227-283: Startup sentinel is correctly scoped to GPU crash handler only.Verification confirms
writeStartupSentinelis called only withinsetupGpuCrashHandlerat line 273. The sentinel tracking is properly isolated to GPU crash events and is not invoked elsewhere during the startup flow, as intended.
… recovery details - Clarified the automatic GPU crash detection and recovery process in the documentation. - Removed version-specific language for GPU crash recovery to streamline information. - Enhanced the logging section with standard Chromium flags for better debugging.
- Introduced `install-volta.sh` to automate the installation of Volta, ensuring node.js and npm are available for building the project. - Added `linux-test-deb.sh` for building, installing, and running the Rocket.Chat Desktop .deb package, with options to skip build, install, or run steps. - Created a README.md to document the usage and functionality of the new scripts, enhancing developer experience and automation for testing on Linux.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
rollup.config.mjs (1)
37-44: Fix formatting issues flagged by Prettier.Static analysis indicates formatting violations. The empty line 38 has trailing whitespace, and the arguments should be on separate lines.
🔎 Apply this diff to fix formatting:
const electronArgs = ['.']; - + // Linux-specific flags for development if (process.platform === 'linux') { - electronArgs.push('--no-sandbox', '--ozone-platform=x11', '--disable-gpu'); + electronArgs.push( + '--no-sandbox', + '--ozone-platform=x11', + '--disable-gpu' + ); }Consider Wayland testing implications: Forcing
--ozone-platform=x11in development means Wayland-specific features (like PipeWire screen capture) cannot be tested during local dev. If Wayland testing is needed, consider making this configurable via an environment variable.scripts/linux-test-deb.sh (3)
138-141: Hardcoded yarn version is fragile.The path
.yarn/releases/yarn-4.6.0.cjswill break when yarn is updated.🔎 Apply this diff to use dynamic discovery:
- elif [ -f "$PROJECT_DIR/.yarn/releases/yarn-4.6.0.cjs" ]; then - # Use local yarn binary if available - if command -v node &> /dev/null; then - YARN_CMD="node $PROJECT_DIR/.yarn/releases/yarn-4.6.0.cjs" + else + # Try to find local yarn binary + LOCAL_YARN=$(find "$PROJECT_DIR/.yarn/releases" -name "yarn-*.cjs" -type f 2>/dev/null | head -n 1) + if [ -n "$LOCAL_YARN" ] && command -v node &> /dev/null; then + YARN_CMD="node $LOCAL_YARN"
34-48: Consider extracting shared utility functions.The color functions (
info,success,warning,error) are duplicated frominstall-volta.sh. Consider creating a sharedscripts/common.shthat both scripts can source.
226-231: Use SIGTERM before SIGKILL.
kill -9(SIGKILL) doesn't allow processes to clean up. Consider using SIGTERM first.🔎 Apply this diff:
EXISTING_PIDS=$(pgrep -f "rocketchat-desktop" 2>/dev/null || true) if [ -n "$EXISTING_PIDS" ]; then warning "Found existing Rocket.Chat processes, stopping them..." - echo "$EXISTING_PIDS" | xargs kill -9 2>/dev/null || true - sleep 1 + echo "$EXISTING_PIDS" | xargs kill 2>/dev/null || true + sleep 2 + # Force kill any remaining processes + echo "$EXISTING_PIDS" | xargs kill -9 2>/dev/null || true fiscripts/install-volta.sh (1)
129-135: Consider supporting additional shell profiles.The script only appends to
~/.bashrc. Users with zsh, fish, or other shells won't have Volta auto-configured. The Volta installer itself handles multiple shells, but this manual addition doesn't.🔎 Example extension:
# Add to shell profile if not already there - if [ -f "$HOME/.bashrc" ] && ! grep -q "VOLTA_HOME" "$HOME/.bashrc"; then - info "Adding Volta to ~/.bashrc for future sessions..." - echo '' >> "$HOME/.bashrc" - echo '# Volta' >> "$HOME/.bashrc" - echo 'export VOLTA_HOME="$HOME/.volta"' >> "$HOME/.bashrc" - echo 'export PATH="$VOLTA_HOME/bin:$PATH"' >> "$HOME/.bashrc" - fi + add_volta_to_profile() { + local profile="$1" + if [ -f "$profile" ] && ! grep -q "VOLTA_HOME" "$profile"; then + info "Adding Volta to $profile for future sessions..." + echo '' >> "$profile" + echo '# Volta' >> "$profile" + echo 'export VOLTA_HOME="$HOME/.volta"' >> "$profile" + echo 'export PATH="$VOLTA_HOME/bin:$PATH"' >> "$profile" + fi + } + add_volta_to_profile "$HOME/.bashrc" + add_volta_to_profile "$HOME/.zshrc"scripts/README.md (2)
37-42: Minor: Improve bullet point grammar for consistency.Line 39 uses a sentence fragment. While acceptable in bullet lists, consider adding a subject to make it grammatically complete for consistency with other items.
🔎 Suggested improvement
- Can be sourced by other scripts to use its functions + It can be sourced by other scripts to use its functionsAlternatively, make it more concise:
- Can be sourced by other scripts to use its functions + Sourceable by other scripts
110-139: Minor: Reduce redundant use of "automatically" across sections.The word "automatically" appears in both line 112 ("The script automatically installs") and line 136 ("automatically resolved"). Since these convey overlapping meaning, consider consolidating or rephrasing one instance to improve clarity.
🔎 Suggested improvement
Modify line 136 to remove the redundant adverb:
- Missing dependencies are automatically resolved during installation + Missing dependencies are resolved during installationOr rephrase to emphasize a different aspect:
- Missing dependencies are automatically resolved during installation + Missing package dependencies are handled transparently during installation
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
docs/linux-display-server.md(1 hunks)rollup.config.mjs(1 hunks)scripts/README.md(1 hunks)scripts/install-volta.sh(1 hunks)scripts/linux-test-deb.sh(1 hunks)src/app/main/app.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{rollup.config.mjs,electron-builder.json,tsconfig.json,.eslintrc.json,jest.config.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain and configure the project via the designated root config files for build, packaging, TypeScript, ESLint, and Jest
Files:
rollup.config.mjs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/app/main/app.ts
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.md: Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Place all documentation in thedocs/directory using Markdown with clear, descriptive filenames and simple language
Before adding features, check and update existing docs; create/extend docs for new or changed features
Files:
docs/linux-display-server.md
🧠 Learnings (3)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
rollup.config.mjs
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to {rollup.config.mjs,electron-builder.json,tsconfig.json,.eslintrc.json,jest.config.js} : Maintain and configure the project via the designated root config files for build, packaging, TypeScript, ESLint, and Jest
Applied to files:
rollup.config.mjs
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/app/main/app.ts
🧬 Code graph analysis (2)
scripts/linux-test-deb.sh (1)
scripts/install-volta.sh (4)
error(25-27)info(13-15)warning(21-23)success(17-19)
src/app/main/app.ts (4)
src/ui/reducers/gpuFallbackMode.ts (1)
gpuFallbackMode(15-30)src/store/readSetting.ts (2)
readGpuFallbackMode(20-26)saveGpuFallbackMode(28-45)src/store/index.ts (2)
listen(88-114)listen(144-157)src/ui/actions.ts (1)
SETTINGS_GPU_FALLBACK_MODE_CHANGED(120-121)
🪛 GitHub Check: build (ubuntu-latest, linux)
rollup.config.mjs
[failure] 41-41:
Replace '--no-sandbox',·'--ozone-platform=x11',·'--disable-gpu' with ⏎··········'--no-sandbox',⏎··········'--ozone-platform=x11',⏎··········'--disable-gpu'⏎········
[failure] 38-38:
Delete ······
🪛 LanguageTool
scripts/README.md
[style] ~39-~39: To form a complete sentence, be sure to include a subject.
Context: ...isplays Volta version #### Features - Can be sourced by other scripts to use its ...
(MISSING_IT_THERE)
[style] ~136-~136: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...ous versions - Missing dependencies are automatically resolved during installation - The app ...
(ADVERB_REPETITION_PREMIUM)
docs/linux-display-server.md
[style] ~17-~17: This wording can make your sentence hard to follow. Try rephrasing for improved clarity.
Context: ...GPU process crashes during startup (e.g., due to incompatible drivers), the app detects ...
(DUE_TO_BECAUSE)
⏰ 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). (5)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (macos-latest, mac)
🔇 Additional comments (8)
src/app/main/app.ts (4)
54-93: LGTM! Startup sentinel implementation is well-structured.The sentinel-based crash tracking with time window (
CRASH_WINDOW_MS) prevents stale crash counts from triggering unnecessary fallbacks. Good defensive error handling inreadStartupSentinelandwriteStartupSentinel.
191-208: LGTM! Linux-specific GPU fallback logic looks correct.The PipeWire screen capture enablement and fallback mode application are properly guarded by platform checks. The
ozone-platformswitch syntax matches Electron'sapp.commandLine.appendSwitchexpectations (without the leading--).
227-281: GPU crash handler now properly respects the threshold.The implementation addresses previous concerns:
- Uses
newCrashCount > CRASH_THRESHOLDconsistently with the sentinel logic- Only tracks GPU process crashes (
details.type !== 'GPU'filter)- Only triggers during startup (
isMainWindowStableguard)- Uses
relaunchApp()helper for consistent argument handling (line 279)
327-334: LGTM! GPU fallback mode change listener is properly guarded.Platform check ensures this only runs on Linux, and it correctly persists the mode before relaunching.
docs/linux-display-server.md (2)
1-149: Documentation is well-structured and comprehensive.The documentation covers:
- Default behavior by package type
- Automatic GPU crash recovery workflow
- Wayland requirements and known issues
- Manual overrides and troubleshooting steps
- Clear best practices
Previous review concerns about version claims have been addressed.
109-115: Documentation is correct as-is.The
--enable-logging --v=1flags documented in lines 111-115 work exactly as shown. These are Chromium flags that can be passed on the command line, where --v controls verbosity when used together with --enable-logging. While --enable-logging cannot be used in app.commandLine.appendSwitch since it is parsed earlier than user's app is loaded, the documentation correctly shows passing them directly as CLI arguments to the rocketchat-desktop executable, which is the appropriate usage pattern. No code changes or documentation clarification needed.scripts/install-volta.sh (1)
143-152: LGTM! Script correctly handles both execution and sourcing.The
BASH_SOURCEcheck properly differentiates between direct execution and sourcing. Exporting functions when sourced enables reuse in other scripts likelinux-test-deb.sh.scripts/README.md (1)
1-139: Comprehensive documentation for Linux test scripts.The README provides clear, detailed documentation for both scripts with good coverage of usage, behavior, requirements, and error handling. The structure is logical, examples are practical, and the explanations are well-organized. This will help developers quickly understand how to use these scripts for Linux testing workflows.
…ncy management - Updated `install-volta.sh` to handle installation errors more gracefully by checking the success of the Volta installation command. - Enhanced `linux-test-deb.sh` to streamline dependency installation checks and provide clearer error messages if installation fails. - Refactored the package installation logic to handle dependency issues more effectively, ensuring smoother installation processes.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
scripts/linux-test-deb.sh (4)
138-138: Consider making the yarn version detection more robust.The hardcoded path
.yarn/releases/yarn-4.6.0.cjswill break if the yarn version is updated. Consider using a glob pattern instead.🔎 Suggested improvement
- elif [ -f "$PROJECT_DIR/.yarn/releases/yarn-4.6.0.cjs" ]; then + else + # Use local yarn binary if available (find latest version) + YARN_BINARY=$(find "$PROJECT_DIR/.yarn/releases" -name "yarn-*.cjs" 2>/dev/null | sort -V | tail -1) + if [ -n "$YARN_BINARY" ] && command -v node &> /dev/null; then + YARN_CMD="node $YARN_BINARY" + fi + fi + + if [ -z "$YARN_CMD" ]; then - # Use local yarn binary if available - if command -v node &> /dev/null; then - YARN_CMD="node $PROJECT_DIR/.yarn/releases/yarn-4.6.0.cjs" - else - error "Neither 'yarn' nor 'node' is available. Please install yarn or node." - exit 1 - fi - else error "yarn is not installed and local yarn binary not found." error "Please install yarn or ensure .yarn/releases/yarn-*.cjs exists." exit 1 fi
173-173: Consider warning if multiple .deb files exist.If multiple
.debfiles exist indist/,head -n 1picks the first arbitrarily. This could lead to testing the wrong package if old builds weren't cleaned up.🔎 Optional enhancement
DEB_FILE=$(find dist -name "rocketchat-*-linux-*.deb" -type f | head -n 1) +DEB_COUNT=$(find dist -name "rocketchat-*-linux-*.deb" -type f | wc -l) + +if [ "$DEB_COUNT" -gt 1 ]; then + warning "Multiple .deb files found in dist/. Using: $DEB_FILE" +fi
231-236: Overly broad process matching pattern.Using
pgrep -f "rocketchat-desktop"matches any process with "rocketchat-desktop" anywhere in its command line, which could inadvertently kill unrelated processes (e.g., editor with that filename open, grep commands, etc.).🔎 More precise process matching
- EXISTING_PIDS=$(pgrep -f "rocketchat-desktop" 2>/dev/null || true) + # Match only the actual binary, not just any command line containing the name + EXISTING_PIDS=$(pgrep -x "rocketchat-desktop" 2>/dev/null || true)Note: Use
-xfor exact executable name matching rather than-ffor full command-line matching.
306-311: Consolidate redundant sleep calls.The script sleeps for 5 seconds at line 308, then immediately sleeps for another 3 seconds at line 311 (total 8 seconds). These sequential sleeps serve the same purpose and can be consolidated.
🔎 Consolidate sleeps
- # Wait for window to appear... - info "Waiting for window to appear..." - sleep 5 - - # Give the app a moment to start and show the window - sleep 3 + # Wait for window to appear and app to initialize + info "Waiting for window to appear..." + sleep 8
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
scripts/install-volta.sh(1 hunks)scripts/linux-test-deb.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/install-volta.sh
⏰ 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). (4)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
🔇 Additional comments (2)
scripts/linux-test-deb.sh (2)
117-129: Previous review concern addressed.The unreachable
$?check issue flagged in the previous review has been correctly fixed. The code now usesif ! sudo apt-get update || ! sudo apt-get install -y "${MISSING_DEPS[@]}"; thenpattern, which properly captures command failures withset -eenabled.
200-219: Correct handling of dpkg dependency issues.The use of
set +e/set -epattern to capture thedpkgexit code and conditionally runapt-get install -fis the correct approach for handling missing dependencies during package installation.
- Implemented auto-detection of Wayland sessions and added logic to relaunch the app with X11 fallback for stability. - Updated GPU fallback mode to include 'wayland' as a valid option, allowing users to specify their preferred display server. - Improved logging for GPU crash handling and display server mode selection, enhancing debugging and user experience on Linux. - Adjusted screen sharing request handling to utilize XDG portal on Wayland sessions, improving compatibility with modern desktop environments.
…creen sharing - Implemented a new screen picker system to handle display media requests, improving user experience during screen sharing. - Created internal and portal picker providers to support different environments, including Linux Wayland and X11. - Enhanced IPC handling for screen sharing requests, ensuring proper state management and preventing concurrent requests. - Added initialization and cleanup logic for the screen picker, streamlining resource management and improving performance.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main.ts (1)
54-100: Duplicated display server logic betweenmain.tsandapp.ts.This function duplicates the ozone-platform determination logic that also exists in
performElectronStartup(lines 204-221 inapp.ts). After a relaunch with--ozone-platform=x11:
handleLinuxDisplayServerreturns early (flag already set)performElectronStartupstill executes its own ozone-platform logic viaappendSwitchWhile not a bug (command-line flags take precedence), this creates maintenance overhead. Consider consolidating the display server logic into a single location.
🔎 Suggested consolidation approach
Move the decision logic to
app.tsand havemain.tsonly handle the relaunch mechanism:// In app.ts - export a function that determines if relaunch is needed export const shouldRelaunchForX11 = (): boolean => { if (process.platform !== 'linux' || !app.isPackaged) return false; const hasOzonePlatformFlag = process.argv.some(arg => arg.startsWith('--ozone-platform=')); if (hasOzonePlatformFlag) return false; const gpuFallbackMode = readGpuFallbackMode(); const isWaylandSession = process.env.XDG_SESSION_TYPE === 'wayland'; return gpuFallbackMode === 'x11' || (gpuFallbackMode === 'none' && isWaylandSession); };Then in
main.ts:if (shouldRelaunchForX11()) { relaunchApp('--ozone-platform=x11'); }src/app/main/app.ts (1)
271-279: Development mode GPU disable persists across runs—consider a session-only approach.When a GPU crash occurs in development,
saveGpuFallbackMode('disabled')persists toconfig.jsonwithout relaunching. This means the GPU remains disabled on subsequent dev runs until the developer manually edits the config or changes the setting in the UI.Consider either:
- Adding a log message informing developers how to re-enable GPU
- Using a session-only flag instead of persisting for dev mode
🔎 Suggested improvement
if (!app.isPackaged) { console.log( - 'Development mode: Disabling GPU immediately after crash' + 'Development mode: Disabling GPU for this session. To re-enable, set gpuFallbackMode to "none" in config.json or via Settings.' ); saveGpuFallbackMode('disabled'); // Don't relaunch - let the app continue without GPU return; }src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts (1)
70-73: Misleading comment in cleanup method.The comment states "Clear caches, reset state" but the implementation only resets the
isInitializedflag. The actual cache and UI resources are managed externally byvideo-call-window.ts, which is consistent with this provider's delegation pattern. Consider updating the comment to reflect what actually happens.🔎 Proposed fix
cleanup(): void { - // Clear caches, reset state + // Reset initialization state (actual resources are managed externally) this.isInitialized = false; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
rollup.config.mjs(1 hunks)src/app/PersistableValues.ts(2 hunks)src/app/main/app.ts(8 hunks)src/main.ts(4 hunks)src/store/readSetting.ts(2 hunks)src/videoCallWindow/ipc.ts(5 hunks)src/videoCallWindow/screenPicker/createScreenPicker.ts(1 hunks)src/videoCallWindow/screenPicker/index.ts(1 hunks)src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts(1 hunks)src/videoCallWindow/screenPicker/providers/PortalPickerProvider.ts(1 hunks)src/videoCallWindow/screenPicker/types.ts(1 hunks)src/videoCallWindow/video-call-window.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/videoCallWindow/screenPicker/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/store/readSetting.tssrc/videoCallWindow/screenPicker/types.tssrc/videoCallWindow/screenPicker/createScreenPicker.tssrc/videoCallWindow/screenPicker/providers/PortalPickerProvider.tssrc/videoCallWindow/video-call-window.tssrc/app/main/app.tssrc/videoCallWindow/ipc.tssrc/app/PersistableValues.tssrc/videoCallWindow/screenPicker/providers/InternalPickerProvider.tssrc/main.ts
src/store/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Write Redux actions following the Flux Standard Action (FSA) convention
Files:
src/store/readSetting.ts
{rollup.config.mjs,electron-builder.json,tsconfig.json,.eslintrc.json,jest.config.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain and configure the project via the designated root config files for build, packaging, TypeScript, ESLint, and Jest
Files:
rollup.config.mjs
src/main.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep
src/main.tsas the Electron main process entry point compiled by Rollup
Files:
src/main.ts
🧠 Learnings (5)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
rollup.config.mjssrc/app/main/app.tssrc/main.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to {rollup.config.mjs,electron-builder.json,tsconfig.json,.eslintrc.json,jest.config.js} : Maintain and configure the project via the designated root config files for build, packaging, TypeScript, ESLint, and Jest
Applied to files:
rollup.config.mjs
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/videoCallWindow/video-call-window.tssrc/main.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/app/main/app.tssrc/main.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.tssrc/videoCallWindow/screenPicker/providers/InternalPickerProvider.tssrc/main.ts
🧬 Code graph analysis (9)
src/store/readSetting.ts (2)
src/app/PersistableValues.ts (1)
GpuFallbackMode(95-95)src/ui/components/SettingsView/features/GpuFallbackMode.tsx (1)
GpuFallbackMode(23-78)
src/videoCallWindow/screenPicker/types.ts (1)
src/videoCallWindow/screenPicker/index.ts (3)
ScreenPickerType(2-2)DisplayMediaCallback(2-2)ScreenPickerProvider(2-2)
src/videoCallWindow/screenPicker/createScreenPicker.ts (3)
src/videoCallWindow/screenPicker/types.ts (2)
ScreenPickerType(3-3)ScreenPickerProvider(9-35)src/videoCallWindow/screenPicker/providers/PortalPickerProvider.ts (1)
PortalPickerProvider(3-23)src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts (1)
InternalPickerProvider(15-74)
src/videoCallWindow/screenPicker/providers/PortalPickerProvider.ts (2)
src/videoCallWindow/screenPicker/index.ts (3)
PortalPickerProvider(4-4)ScreenPickerProvider(2-2)DisplayMediaCallback(2-2)src/videoCallWindow/screenPicker/types.ts (2)
ScreenPickerProvider(9-35)DisplayMediaCallback(7-7)
src/videoCallWindow/video-call-window.ts (2)
src/videoCallWindow/screenPicker/createScreenPicker.ts (1)
createScreenPicker(29-45)src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts (1)
InternalPickerProvider(15-74)
src/app/main/app.ts (4)
src/ui/reducers/gpuFallbackMode.ts (1)
gpuFallbackMode(15-30)src/store/readSetting.ts (2)
readGpuFallbackMode(20-26)saveGpuFallbackMode(28-45)rollup.config.mjs (1)
isWaylandSession(45-45)src/ui/actions.ts (1)
SETTINGS_GPU_FALLBACK_MODE_CHANGED(120-121)
src/videoCallWindow/ipc.ts (5)
src/videoCallWindow/screenPicker/index.ts (3)
DisplayMediaCallback(2-2)createScreenPicker(1-1)InternalPickerProvider(3-3)src/videoCallWindow/screenPicker/types.ts (1)
DisplayMediaCallback(7-7)src/jitsi/preload.ts (1)
desktopCapturer(11-17)src/videoCallWindow/screenPicker/createScreenPicker.ts (1)
createScreenPicker(29-45)src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts (1)
InternalPickerProvider(15-74)
src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts (2)
src/videoCallWindow/screenPicker/index.ts (3)
InternalPickerProvider(3-3)ScreenPickerProvider(2-2)DisplayMediaCallback(2-2)src/videoCallWindow/screenPicker/types.ts (2)
ScreenPickerProvider(9-35)DisplayMediaCallback(7-7)
src/main.ts (2)
src/store/readSetting.ts (1)
readGpuFallbackMode(20-26)src/app/main/app.ts (2)
relaunchApp(108-112)markMainWindowStable(232-239)
🔇 Additional comments (31)
rollup.config.mjs (1)
37-57: LGTM! Well-structured Linux display server handling for development.The logic correctly prioritizes explicit user overrides (
FORCE_WAYLAND,FORCE_X11) and defaults to X11 on Wayland sessions for stability. The--no-sandboxflag is appropriate for development.src/store/readSetting.ts (1)
20-45: LGTM! Clean implementation of GPU fallback mode helpers.The validation logic correctly constrains to valid
GpuFallbackModevalues, and the save function properly handles the read-modify-write pattern for the config file.src/app/PersistableValues.ts (2)
95-104: LGTM! Standard migration pattern for the new GPU fallback mode setting.The type definition and versioned migration follow the established patterns in this file.
182-185: Appropriate default value for initial migration.Defaulting to
'none'allows the auto-detection logic to determine the optimal display server mode on first run.src/main.ts (1)
112-113: LGTM! Correct placement of GPU crash handler and stability marker.
setupGpuCrashHandleris called early (after app ready, before window creation) to catch startup GPU crashes, andmarkMainWindowStableis called after the root window is successfully shown, correctly scoping the crash-recovery window.Also applies to: 139-140
src/app/main/app.ts (4)
54-61: Clear sentinel interface and appropriate threshold values.The
IStartupSentinelinterface andCRASH_THRESHOLD = 2withCRASH_WINDOW_MS = 60000provide reasonable defaults—triggering fallback after 2 GPU crashes within a minute prevents premature fallback from transient issues.
286-306: Threshold logic correctly handles the crash-within-window scenario.The implementation properly:
- Resets the counter if outside the time window
- Uses
>=for threshold comparison (2 crashes triggers fallback)- Preserves user args on relaunch via
relaunchApp(...userArgs)- Writes sentinel for sub-threshold crashes to track progress
This addresses the previous review concerns about threshold consistency.
355-362: LGTM! Proper persistence and relaunch on setting change.The listener correctly persists the new mode and triggers a relaunch to apply the display server change.
191-222: Check ifappendSwitchhandles duplicate switches or if pre-existingozone-platformflag from command-line args causes conflicts.The method appends a switch to Chromium's command line, but the official documentation doesn't clarify behavior when the same switch already exists from startup arguments. Since the code doesn't remove or check for existing switches before appending, verify that calling
appendSwitch('ozone-platform')multiple times (or after it's already set) doesn't cause unexpected behavior or flag conflicts in Chromium.src/videoCallWindow/screenPicker/providers/PortalPickerProvider.ts (3)
8-12: LGTM: Clean delegation to OS picker.The implementation correctly delegates screen selection to the OS portal by passing an empty video object. The console logging provides useful debugging context.
14-17: LGTM: Appropriate no-op initialization.The portal picker correctly requires no initialization since it delegates entirely to the OS portal mechanism.
19-22: LGTM: Appropriate no-op cleanup.No cleanup is needed since the portal picker has no state or resources to manage.
src/videoCallWindow/video-call-window.ts (3)
11-11: LGTM: Clean import of screen picker modules.The imports are correctly structured and both exports are used appropriately in the initialization flow.
471-486: LGTM: Provider-based screen picker initialization.The initialization flow correctly:
- Creates/retrieves the cached screen picker provider
- Configures the internal picker's initialize handler before calling initialize()
- Handles errors gracefully
- Treats cache warming as an optional optimization
The InternalPickerProvider's isInitialized guard ensures the initialization logic runs only once, even if handleFinishLoad is called multiple times (e.g., on reload).
664-687: LGTM: Appropriate export for provider initialization.The function is correctly exported to support the provider-based architecture. The implementation maintains proper concurrent import protection and appropriate error handling.
src/videoCallWindow/screenPicker/types.ts (2)
5-7: LGTM: Well-documented type with intentional 'any'.The comment clearly explains the use of
anyfor flexibility, matching Electron's API pattern.
9-35: LGTM: Well-designed and documented interface.The interface provides a clean abstraction for screen picker providers with clear responsibilities and lifecycle methods. The JSDoc comments effectively explain when and where each method is called.
src/videoCallWindow/screenPicker/createScreenPicker.ts (4)
5-25: LGTM: Comprehensive portal detection.The detection logic correctly identifies portal-capable environments:
- Wayland sessions (via XDG_SESSION_TYPE)
- Major desktop environments with portal support (GNOME, KDE, etc.)
This enables system picker usage for both Wayland and X11 environments that support XDG Desktop Portal.
29-45: LGTM: Solid singleton factory pattern.The caching behavior is appropriate for this use case. The provider is created once and reused, with
resetScreenPicker()available for cleanup if needed.
47-54: LGTM: Appropriate getter with error handling.The function correctly guards against accessing an uninitialized provider with a clear error message.
56-59: LGTM: Clean lifecycle management.The reset function correctly cleans up the provider before clearing the cache.
src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts (4)
15-23: LGTM: Clean class definition and state management.The properties correctly identify this as the internal picker requiring UI and cache warming. The nullable handler functions support the delegation pattern.
25-41: LGTM: Simple handler injection pattern.The setter methods correctly support dependency injection by storing handlers provided by external modules. The lack of guards against multiple calls is appropriate since handlers can be updated.
43-52: LGTM: Robust error handling for missing handler.The method correctly handles the case where the handler hasn't been set, providing a clear error message and failing safely by denying the request.
54-68: LGTM: Proper initialization guard and warning.The initialize method correctly prevents re-initialization and provides appropriate warnings when the handler isn't set. The warning (rather than error) is appropriate since the provider might be used in contexts where UI preloading isn't necessary.
src/videoCallWindow/ipc.ts (6)
29-30: LGTM: Clean imports and reasonable timeout.The screen picker integration imports are appropriate, and the 60-second timeout provides a reasonable window for users to select a screen.
Also applies to: 37-37
43-49: LGTM: Comprehensive request tracking state.The tracking variables provide proper lifecycle management for screen sharing requests, enabling cleanup, request matching, and concurrency control.
263-299: LGTM: Well-separated cleanup helpers.The three helper functions provide appropriate granularity:
cleanupScreenSharingListener: Full cleanup for window destructionremoveScreenSharingListenerOnly: Remove handlers but preserve statemarkScreenSharingComplete: Mark request as doneThis separation enables correct cleanup in different scenarios.
301-422: LGTM: Comprehensive guarded request handler.The implementation effectively addresses all concurrency and resource management concerns:
- Prevents concurrent requests via
isScreenSharingRequestPendingguard- Matches requests and responses via unique request IDs
- Prevents double-callback with
callbackInvokedflag- Validates selected sources via
desktopCapturer- Handles timeout scenarios gracefully
- Ensures cleanup via helper functions
The request ID matching on both listener and timeout paths prevents race conditions, and the cleanup separation enables correct resource management.
224-224: LGTM: Comprehensive cleanup integration.Screen sharing cleanup is correctly integrated into all window destruction paths:
- Manual cleanup in
cleanupVideoCallWindow- Automatic cleanup on 'closed' event
- Automatic cleanup on 'close' event
The cleanup function is idempotent, making multiple calls safe.
Also applies to: 661-662, 689-690
437-442: Add clarifying comment about useSystemPicker behavior with provider types.The
useSystemPicker: trueoption works as designed: on macOS 15+, Electron uses the native system picker and the handler is bypassed. ForPortalPickerProvider, this is the intended behavior. However, forInternalPickerProvider, the custom UI is skipped on supported systems. Add a comment explaining this is intentional—the native picker provides better UX when available—and thatInternalPickerProviderserves as a fallback for unsupported systems.
src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts
Outdated
Show resolved
Hide resolved
…ener management - Refactored the internal picker handler to enhance state management during screen sharing requests, preventing concurrent requests and ensuring proper cleanup. - Improved error handling for screen sharing source validation, including better logging for unavailable sources and request timeouts. - Streamlined the listener setup and removal process, enhancing resource management and overall performance during screen sharing operations.
- Refactored the setup of webview handlers to use lazy loading for the screen picker module, improving performance and preventing blocking during webview initialization. - Improved error handling in the display media request handler, ensuring that errors are logged without disrupting the webview loading process. - Updated the internal picker provider to streamline the handling of display media requests, particularly for Linux environments.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/videoCallWindow/screenPicker/providers/PortalPickerProvider.ts (1)
10-36: Type assertion masks potential type mismatch.The
as anyassertions on lines 26 and 34 suggest that{ video: false }doesn't match the expected type for the callback parameter. According to the relevant code snippet,DisplayMediaCallbackis typed as(streams: any) => void, but the actual parameter structure appears to be{ video: source | false }.Consider either:
- Updating the
DisplayMediaCallbacktype definition intypes.tsto accurately reflect the expected structure:(streams: { video: DesktopCapturerSource | false }) => void- Or documenting why the type assertion is necessary with a brief comment
This would improve type safety and make the codebase more maintainable.
src/videoCallWindow/ipc.ts (2)
42-48: Module-level mutable state for request tracking.The screen sharing request state is managed via module-level variables. While this works for the current single-window scenario, it could become problematic if multiple video call windows are ever supported concurrently.
Consider encapsulating this state in a class or object if there's any future possibility of supporting multiple concurrent video call windows. For now, the implementation is acceptable given the single-window constraint enforced by the code.
361-376: Source validation adds safety but increases latency.The code validates the selected
sourceIdby fetching all sources again viadesktopCapturer.getSources()(lines 362-364) and checking if the source still exists (lines 366-376). This adds an extra async call to the screen sharing flow.If the validation is primarily to handle race conditions where a source disappears between selection and usage, consider whether:
- This validation is necessary in practice (sources typically remain stable during the selection flow)
- The error handling in the webview layer could handle invalid sources more gracefully
The current implementation prioritizes safety, which is reasonable for a "Chill" review setting. The latency impact is likely minimal (single-digit milliseconds).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/videoCallWindow/ipc.ts(6 hunks)src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts(1 hunks)src/videoCallWindow/screenPicker/providers/PortalPickerProvider.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/videoCallWindow/screenPicker/providers/PortalPickerProvider.tssrc/videoCallWindow/ipc.ts
🧠 Learnings (1)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/ipc/**/*.ts : Define and maintain IPC channels in `src/ipc/` for main/renderer state synchronization
Applied to files:
src/videoCallWindow/ipc.ts
🧬 Code graph analysis (2)
src/videoCallWindow/screenPicker/providers/PortalPickerProvider.ts (3)
src/videoCallWindow/screenPicker/index.ts (3)
PortalPickerProvider(4-4)ScreenPickerProvider(2-2)DisplayMediaCallback(2-2)src/videoCallWindow/screenPicker/types.ts (2)
ScreenPickerProvider(9-35)DisplayMediaCallback(7-7)src/jitsi/preload.ts (1)
desktopCapturer(11-17)
src/videoCallWindow/ipc.ts (5)
src/videoCallWindow/screenPicker/index.ts (3)
DisplayMediaCallback(2-2)createScreenPicker(1-1)InternalPickerProvider(3-3)src/videoCallWindow/screenPicker/types.ts (1)
DisplayMediaCallback(7-7)src/jitsi/preload.ts (1)
desktopCapturer(11-17)src/videoCallWindow/screenPicker/createScreenPicker.ts (1)
createScreenPicker(29-45)src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts (1)
InternalPickerProvider(15-74)
🔇 Additional comments (3)
src/videoCallWindow/ipc.ts (3)
422-469: LGTM! Well-designed lazy loading and error isolation.The async
setupWebviewHandlersfunction demonstrates several best practices:
- Lazy imports the screen picker module to avoid blocking webview initialization
- Properly wires the internal picker handler only when needed (line 432-434)
- Error handling ensures failures don't prevent webview loading (lines 447-453, 457-459, 465-468)
- Clears previous listeners before setting new ones (line 462)
This design reduces initialization blocking time and improves resilience.
300-420: Robust concurrency and timeout handling.The
createInternalPickerHandlerfunction implements comprehensive safeguards:
- Prevents concurrent requests with the pending flag (lines 306-312)
- Generates unique request IDs for validation (line 326)
- Uses
callbackInvokedflag to prevent double-invocation races (lines 331, 344-350, 399-404)- Both listener and timeout validate the request ID before proceeding (lines 336-341, 392-397)
- Proper cleanup of listeners and timeouts in all paths
The implementation addresses the concerns raised in previous review comments.
262-298: Clear separation of concerns in cleanup helpers.The three cleanup functions provide appropriate granularity:
cleanupScreenSharingListener: Full cleanup of all screen sharing stateremoveScreenSharingListenerOnly: Removes IPC listener and timeout but preserves request trackingmarkScreenSharingComplete: Marks request as complete without touching listenersThis separation allows the listener callback to remove the listener without clearing the request ID prematurely, while still validating responses.
…allback - Introduced a check for Wayland sessions in handleLinuxDisplayServer to default to XWayland for stability, addressing potential GPU issues on virtual machines. - Updated logging messages to clarify the use of XWayland when a Wayland session is detected, improving user guidance on display server settings. - Removed redundant Wayland session checks from performElectronStartup, streamlining the GPU fallback logic.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/main/app.ts (1)
281-287: Clarify that GPU disable only takes effect on next startup.The comment and log message state "Disabling GPU immediately after crash", but
saveGpuFallbackMode('disabled')only persists the setting for the next startup—the current process continues with GPU enabled.🔎 Suggested clarification
// In development, disable GPU immediately on first crash if (!app.isPackaged) { - console.log('Development mode: Disabling GPU immediately after crash'); + console.log('Development mode: Saving GPU-disabled mode for next startup'); saveGpuFallbackMode('disabled'); - // Don't relaunch - let the app continue without GPU + // Don't relaunch - let the app continue (GPU will be disabled on next startup) return; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/app/main/app.tssrc/main.ts
🧰 Additional context used
📓 Path-based instructions (2)
src/main.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep
src/main.tsas the Electron main process entry point compiled by Rollup
Files:
src/main.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/main.tssrc/app/main/app.ts
🧠 Learnings (3)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/main.tssrc/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/main.tssrc/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/main.tssrc/app/main/app.ts
🧬 Code graph analysis (2)
src/main.ts (4)
src/ui/reducers/gpuFallbackMode.ts (1)
gpuFallbackMode(18-33)src/store/readSetting.ts (1)
readGpuFallbackMode(20-26)rollup.config.mjs (1)
isWaylandSession(45-45)src/app/main/app.ts (4)
relaunchApp(109-131)performElectronStartup(133-233)setupGpuCrashHandler(251-317)markMainWindowStable(242-249)
src/app/main/app.ts (4)
src/ui/reducers/gpuFallbackMode.ts (1)
gpuFallbackMode(18-33)src/store/readSetting.ts (2)
readGpuFallbackMode(20-26)saveGpuFallbackMode(28-45)src/store/index.ts (2)
listen(88-114)listen(144-157)src/ui/actions.ts (1)
SETTINGS_GPU_FALLBACK_MODE_CHANGED(120-121)
⏰ 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). (6)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
🔇 Additional comments (11)
src/main.ts (4)
7-9: LGTM - imports are correctly structured.The new imports for
setupGpuCrashHandler,markMainWindowStable,relaunchApp, andreadGpuFallbackModeare properly organized and align with the functionality introduced in this PR.Also applies to: 33-33
54-104: LGTM - well-structured Linux display server handling.The function correctly:
- Short-circuits for non-Linux and development environments
- Respects existing command-line flags
- Defaults to XWayland on Wayland sessions for stability (documented behavior)
- Uses
relaunchAppwhich properly exits the process after initiating relaunch
109-115: LGTM - startup sequence is correctly ordered.The order ensures:
- Display server handling runs before Electron fully initializes (critical for ozone-platform)
- GPU crash handler is installed before
app.whenReady()to catch early failures- This aligns with the documented behavior in
docs/linux-display-server.md
143-144: LGTM - main window stability marker correctly placed.Calling
markMainWindowStable()aftershowRootWindow()ensures the startup sentinel is cleared only after successful window initialization, and subsequent GPU crashes won't trigger the fallback mechanism.src/app/main/app.ts (7)
1-4: LGTM - necessary imports for new functionality.The imports for
spawn,fs, andpathare correctly added to support AppImage relaunching and startup sentinel file operations.
55-94: LGTM - startup sentinel infrastructure is well-implemented.The threshold constant naming (
CRASH_THRESHOLD) addresses the previous review feedback. The file I/O operations are appropriately wrapped with try-catch blocks, and the 1-minute crash window prevents stale crash counts from accumulating across sessions.
109-131: LGTM - AppImage relaunch handling is correctly implemented.The AppImage-specific path using
spawnwithdetached: trueandunref()is the correct pattern for relaunching AppImages. This addresses the known issue whereapp.relaunch()doesn't work reliably in AppImage environments.
210-232: LGTM - Linux display server handling in performElectronStartup.The PipeWire capturer enablement and the display server mode application are correctly implemented:
WebRTCPipeWireCaptureris required for Wayland screen sharing- The mode-based branching correctly handles all four fallback modes
- The comment on line 230-231 correctly documents that 'none' mode on Wayland is handled by
handleLinuxDisplayServer()in main.ts before this function runs
242-249: LGTM - clean implementation.The function correctly marks the window as stable and clears the startup sentinel only on Linux where it's relevant.
251-317: LGTM - threshold-based GPU crash handling addresses previous review feedback.The implementation now:
- Uses consistent
relaunchApp(...userArgs)helper instead of directapp.relaunch()- Correctly implements threshold logic (
>=withCRASH_THRESHOLD = 2)- Only handles actual GPU process crashes (
details.type !== 'GPU'guard)- Gates fallback to early startup phase via
isMainWindowStable
363-370: LGTM - GPU fallback mode change handling.The listener correctly persists the new mode before relaunching, and is appropriately gated to Linux-only platforms.
Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
rollup.config.mjsscripts/linux-test-appimage.shsrc/main.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- rollup.config.mjs
- scripts/linux-test-appimage.sh
🧰 Additional context used
📓 Path-based instructions (2)
src/main.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep
src/main.tsas the Electron main process entry point compiled by Rollup
Files:
src/main.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/main.ts
🧠 Learnings (3)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/main.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/main.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/preload.ts : Keep `src/preload.ts` as the preload script bridging main and renderer
Applied to files:
src/main.ts
⏰ 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). (6)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
🔇 Additional comments (4)
src/main.ts (4)
7-9: LGTM!The new imports are correctly organized and all are used within the file for the Linux display server handling and GPU crash recovery features.
Also applies to: 33-33
109-110: LGTM!Correct placement - the display server handling must occur before
performElectronStartup()since ozone-platform flags cannot be set after Electron initializes. The dependency onsetUserDataDirectory()being called first is properly maintained.
114-115: LGTM!Setting up the GPU crash handler before
app.whenReady()ensures early GPU failures during initialization are captured. This is the correct timing for establishing crash monitoring.
143-144: LGTM!The timing is appropriate - marking the window as stable after
showRootWindow()completes establishes a clear boundary between the startup phase (where GPU crashes trigger fallback) and the runtime phase (where they don't). This prevents the app from entering a fallback loop due to transient GPU issues during normal operation.
- Remove X11 forcing on Wayland in linux-test-deb.sh (use native Wayland) - Fix console.log formatting with JSON.stringify in src/main.ts - Fix misleading success message in test scripts when app exits early - Remove unnecessary `as any` type assertion in InternalPickerProvider.ts
Always use X11 (XWayland) on Wayland sessions for stability. The app automatically relaunches with --ozone-platform=x11 when a Wayland session is detected.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts (1)
71-74: Consider clarifying cleanup scope.The comment mentions "Clear caches, reset state," but the implementation only resets the
isInitializedflag while preserving the handler functions. This might be intentional (handlers persist across cleanup/reinitialize cycles), but the comment could be more specific about what state is actually cleared.🔎 Suggested clarification
Update the comment to be more explicit about what's being reset:
- cleanup(): void { - // Clear caches, reset state + cleanup(): void { + // Reset initialization state (handlers are preserved for reuse) this.isInitialized = false; }Or, if handlers should also be cleared:
cleanup(): void { - // Clear caches, reset state + // Clear all state and handlers this.isInitialized = false; + this.handleRequestFn = null; + this.initializeFn = null; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
rollup.config.mjsscripts/linux-test-appimage.shscripts/linux-test-deb.shsrc/main.tssrc/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/linux-test-deb.sh
- rollup.config.mjs
- scripts/linux-test-appimage.sh
🧰 Additional context used
📓 Path-based instructions (2)
src/main.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Keep
src/main.tsas the Electron main process entry point compiled by Rollup
Files:
src/main.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/main.tssrc/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts
🧠 Learnings (2)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/main.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/main.ts
🧬 Code graph analysis (1)
src/main.ts (3)
rollup.config.mjs (1)
isWaylandSession(44-44)src/app/main/app.ts (4)
relaunchApp(109-131)performElectronStartup(133-233)setupGpuCrashHandler(251-317)markMainWindowStable(242-249)src/app/main/dev.ts (1)
setUserDataDirectory(7-19)
⏰ 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). (6)
- GitHub Check: check (macos-latest)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: check (windows-latest)
- GitHub Check: build (ubuntu-latest, linux)
🔇 Additional comments (11)
src/videoCallWindow/screenPicker/providers/InternalPickerProvider.ts (7)
1-1: Imports look clean.Good removal of unused imports from the earlier review. Only necessary types are imported now.
12-18: Class structure and constants are well-defined.The readonly properties with appropriate types clearly define this provider's characteristics. The
as constassertion ensures proper literal typing.
19-24: Dependency injection pattern via nullable handlers is reasonable.The approach of storing handlers that are set externally allows proper separation between the provider interface and implementation details from ipc.ts and video-call-window.ts.
30-34: Handler setter is appropriately straightforward.The method correctly stores the handler with clear documentation of its intended caller.
40-42: Initialize handler setter follows the same clean pattern.Consistent implementation with clear documentation about its usage from the renderer process.
55-69: Initialization logic is sound and idempotent.The guard against re-initialization and the graceful handling of missing initialize handlers both make sense for this provider's design. The warning log appropriately alerts developers if the handler wasn't set while still allowing the provider to proceed.
44-53: No action needed. TheDisplayMediaCallbacktype acceptsanyas its parameter, so the current callback shape{ video: false }is valid and matches the expected signature. Theas anycast removal is correct, and no additional properties are required.src/main.ts (4)
7-9: LGTM: New imports support Linux display server and GPU crash handling.The new imports are properly structured and used appropriately in the startup sequence for Linux Wayland detection and GPU crash management.
88-90: LGTM: Correct placement before performElectronStartup.The function is correctly invoked early in the startup sequence before
performElectronStartup(), as required for command-line ozone-platform flags to take effect. The comment clearly explains the ordering constraint.
93-95: LGTM: Early GPU crash handler setup catches failures before app readiness.Correctly positioned before
app.whenReady()to capture GPU crashes during early initialization. Theapp.on('child-process-gone')listener used bysetupGpuCrashHandlercan be registered before the app is ready, making this placement appropriate.
122-124: LGTM: Main window stability marker correctly placed after window display.Appropriately invoked after
showRootWindow()to mark the main window as stable, preventing GPU crash fallback logic from triggering after successful initialization. The comment clearly explains the behavior change.
Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/main/app.ts (1)
175-177: Consider enhancing the compatibility comment.While the no-op is appropriate for maintaining API compatibility, the comment could be more informative about what was removed and when this stub can be safely removed.
🔎 Suggested enhancement
-export const markMainWindowStable = (): void => { - // No-op: kept for API compatibility -}; +export const markMainWindowStable = (): void => { + // No-op: Previously used for GPU crash counting/sentinel logic (removed in PR #3162) + // Kept for API compatibility with src/main.ts; can be removed in a future cleanup +};
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/pull-request-build.ymlsrc/app/main/app.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pull-request-build.yml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/app/main/app.ts
🧠 Learnings (2)
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
src/app/main/app.ts
📚 Learning: 2025-09-26T19:24:21.364Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-26T19:24:21.364Z
Learning: Applies to src/rootWindow.ts : Keep `src/rootWindow.ts` as the renderer entry for the main window UI
Applied to files:
src/app/main/app.ts
🪛 GitHub Check: build (ubuntu-latest, linux)
src/app/main/app.ts
[failure] 195-195:
Replace ⏎······'GPU·process·crashed,·disabling·GPU·and·relaunching·with·X11'⏎···· with 'GPU·process·crashed,·disabling·GPU·and·relaunching·with·X11'
🪛 GitHub Check: check (macos-latest)
src/app/main/app.ts
[failure] 195-195:
Replace ⏎······'GPU·process·crashed,·disabling·GPU·and·relaunching·with·X11'⏎···· with 'GPU·process·crashed,·disabling·GPU·and·relaunching·with·X11'
🪛 GitHub Check: check (ubuntu-latest)
src/app/main/app.ts
[failure] 195-195:
Replace ⏎······'GPU·process·crashed,·disabling·GPU·and·relaunching·with·X11'⏎···· with 'GPU·process·crashed,·disabling·GPU·and·relaunching·with·X11'
⏰ 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: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: check (windows-latest)
🔇 Additional comments (6)
src/app/main/app.ts (6)
1-2: LGTM: Import required for AppImage support.The
spawnimport is correctly added to support the new AppImage relaunch logic.
60-82: LGTM: AppImage relaunch implemented correctly.The AppImage-specific relaunch logic properly handles the limitations of
app.relaunch()with AppImages by using a detached, unreferenced spawn process. The fallback to standardapp.relaunch()for non-AppImage environments maintains backward compatibility.
88-88: LGTM: Correct switch format for Electron API.The removal of the
--prefix is correct forcommandLine.appendSwitch, which expects switch names without prefixes.
129-131: LGTM: Comprehensive GPU disabling.The additional switches ensure GPU acceleration is fully disabled across canvas rendering and general GPU usage when hardware acceleration is turned off.
162-165: LGTM: Wayland screen sharing support enabled.Enabling
WebRTCPipeWireCapturercorrectly adds PipeWire support for screen capture on Linux, which is essential for Wayland environments as outlined in the PR objectives.
179-225: LGTM: GPU crash detection and fallback logic are sound.The implementation correctly handles GPU failures on Linux with two complementary approaches:
- Runtime crash detection: Catches GPU process crashes via
child-process-gone- Proactive feature validation: Checks GPU capabilities via
gpu-info-updateBoth paths appropriately relaunch with
--disable-gpuand--ozone-platform=x11as a fallback. The early return on line 185-187 prevents infinite relaunch loops. The simplified approach (no crash counting) aligns with the resolution of previous review feedback.
- Remove sentinel file system (crash counting, timing windows) - Add gpu-info-update listener to detect GPU issues early - Check gpu_compositing and webgl status for disabled/unavailable states - Relaunch with --disable-gpu --ozone-platform=x11 when GPU is broken - Simplify crash handler to immediately trigger fallback on GPU crash
- Fix case sensitivity: rocketchat-*.appimage → rocketchat-*.AppImage (AWS CLI on Linux is case-sensitive, file wasn't being uploaded) - Fix sticky comment header to use simple identifier instead of markdown - Remove redundant recreate/append flags (defaults work correctly)
jeanfbrito
force-pushed
the
wayland-screen-sharing-fix
branch
from
a4a78fa to
ca1b08f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/main/app.ts (1)
60-82: Consider adding error handling for the AppImage spawn.If the
spawncall fails (e.g., due to permissions or a corrupted binary), the app will exit without starting a new instance, leaving the user with no running application. While rare, this could degrade the user experience.🔎 Suggested error handling
// Spawn the AppImage as a detached process - spawn(process.env.APPIMAGE, args, { + const child = spawn(process.env.APPIMAGE, args, { detached: true, stdio: 'ignore', - }).unref(); + }); + + child.on('error', (err) => { + console.error('Failed to spawn AppImage:', err); + // Fallback to standard relaunch if spawn fails + const command = process.argv.slice(1, app.isPackaged ? 1 : 2); + app.relaunch({ args: [...command, ...args] }); + app.exit(); + return; + }); + + child.unref(); app.exit(); return;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/pull-request-build.ymlsrc/app/main/app.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pull-request-build.yml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow existing TypeScript patterns with strict mode enabled
All new code must pass ESLint and TypeScript checks
Avoid unnecessary comments; only add comments for complex logic or non-obvious decisions
Files:
src/app/main/app.ts
⏰ 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). (6)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (ubuntu-latest, linux)
🔇 Additional comments (6)
src/app/main/app.ts (6)
1-2: LGTM!The
spawnimport is correctly used for AppImage relaunch handling in therelaunchAppfunction.
88-88: LGTM!Correctly removed the
--prefix. Electron'sappendSwitchexpects switch names without the prefix.
129-131: LGTM!Correctly updated hardware acceleration switches to use the proper format without the
--prefix for Electron'sappendSwitchAPI.
161-165: LGTM!Correctly enables PipeWire screen capture support for Wayland environments on Linux, aligning with the PR objectives to enhance Linux screen sharing.
175-177: LGTM!The no-op function with a clear comment is an acceptable pattern for maintaining API compatibility during refactoring.
179-223: LGTM!The GPU crash handler is well-designed with proper safeguards:
- Early exit when
--disable-gpuis already present prevents infinite relaunch loops- Proactive detection using
gpu-info-updatecatches GPU issues before they cause crashes- The
isGpuBrokenhelper correctly identifies various GPU failure states including software fallback variants- Relaunching with both
--disable-gpuand--ozone-platform=x11provides a stable fallback for Linux environments
Linux installer download |
Windows installer download |
macOS installer download |
…etChat#3162) * feat: Enhance Linux support for screen sharing and dependencies - Added `xdg-desktop-portal` and `xdg-desktop-portal-gtk` as recommended dependencies in the Electron builder configuration for improved screen sharing functionality on Linux. - Removed the ozone platform switch for `x11` in the Electron startup process and enabled PipeWire screen capture support for Wayland environments. - Updated the display media request handler to utilize system picker for better user experience during screen sharing. * chore: Bump version to 4.10.2 in package.json * feat(videoCall): Implement screen sharing request handling and cleanup logic - Added a timeout mechanism for screen sharing requests to prevent orphaned listeners. - Introduced cleanup functions for screen sharing listeners and request states to enhance resource management. - Improved IPC handling for screen sharing source responses, ensuring proper request tracking and error handling. - Enhanced user experience by preventing concurrent screen sharing requests and managing listener cleanup effectively. * fix(videoCall): Improve screen sharing timeout handling and listener cleanup - Added validation to ensure the screen sharing timeout is only processed for the current request, preventing double-invocation. - Enhanced cleanup logic to clear timeout references and invoke callbacks appropriately, improving resource management and user experience during screen sharing sessions. * fix(videoCall): Refactor screen sharing listener cleanup and request completion handling - Introduced `removeScreenSharingListenerOnly` to streamline listener removal and timeout clearing without altering the pending request state. - Added `markScreenSharingComplete` to reset request states, allowing new screen sharing requests after completion. - Enhanced existing cleanup logic to improve resource management and prevent concurrent screen sharing requests. * feat(gpuFallback): Implement GPU crash detection and fallback mode for Linux - Added a GPU crash handler to automatically switch to X11 mode if the GPU process crashes during startup. - Introduced a persistent fallback mode setting to remember the user's choice across sessions. - Enhanced the settings UI to allow users to control the display server mode, including options for auto-detect, force X11, or disable GPU. - Updated the application to handle GPU fallback mode changes dynamically, improving user experience and stability on Linux systems. * fix(app): Adjust crash count condition and preserve command-line arguments on relaunch - Updated the crash detection logic to trigger fallback mode after exceeding the maximum crash count. - Enhanced the relaunch functionality to preserve command-line arguments, ensuring user inputs are retained during application restarts. * feat: Enhance Linux display server support and crash handling - Added Wayland support to the Electron builder configuration, allowing for improved compatibility with modern Linux environments. - Updated documentation to reflect auto-detection of display servers for Snap packages, enhancing user clarity. - Refactored crash detection logic to use a more descriptive variable name and improved logging for better debugging during GPU fallback scenarios. * refactor(app): Streamline GPU crash handling and fallback logic - Simplified the GPU crash detection process by consolidating logic for tracking crash counts and managing fallback mode. - Improved logging for better visibility during relaunch scenarios. - Ensured that the application preserves command-line arguments when relaunching after a GPU crash. * docs: Update Linux display server documentation and improve GPU crash recovery details - Clarified the automatic GPU crash detection and recovery process in the documentation. - Removed version-specific language for GPU crash recovery to streamline information. - Enhanced the logging section with standard Chromium flags for better debugging. * feat(scripts): Add installation and testing scripts for Linux - Introduced `install-volta.sh` to automate the installation of Volta, ensuring node.js and npm are available for building the project. - Added `linux-test-deb.sh` for building, installing, and running the Rocket.Chat Desktop .deb package, with options to skip build, install, or run steps. - Created a README.md to document the usage and functionality of the new scripts, enhancing developer experience and automation for testing on Linux. * refactor(scripts): Improve error handling in installation and dependency management - Updated `install-volta.sh` to handle installation errors more gracefully by checking the success of the Volta installation command. - Enhanced `linux-test-deb.sh` to streamline dependency installation checks and provide clearer error messages if installation fails. - Refactored the package installation logic to handle dependency issues more effectively, ensuring smoother installation processes. * feat(linux): Enhance Wayland and X11 support for GPU handling - Implemented auto-detection of Wayland sessions and added logic to relaunch the app with X11 fallback for stability. - Updated GPU fallback mode to include 'wayland' as a valid option, allowing users to specify their preferred display server. - Improved logging for GPU crash handling and display server mode selection, enhancing debugging and user experience on Linux. - Adjusted screen sharing request handling to utilize XDG portal on Wayland sessions, improving compatibility with modern desktop environments. * feat(videoCall): Introduce screen picker functionality for enhanced screen sharing - Implemented a new screen picker system to handle display media requests, improving user experience during screen sharing. - Created internal and portal picker providers to support different environments, including Linux Wayland and X11. - Enhanced IPC handling for screen sharing requests, ensuring proper state management and preventing concurrent requests. - Added initialization and cleanup logic for the screen picker, streamlining resource management and improving performance. * refactor(videoCall): Improve screen sharing request handling and listener management - Refactored the internal picker handler to enhance state management during screen sharing requests, preventing concurrent requests and ensuring proper cleanup. - Improved error handling for screen sharing source validation, including better logging for unavailable sources and request timeouts. - Streamlined the listener setup and removal process, enhancing resource management and overall performance during screen sharing operations. * refactor(videoCall): Enhance webview handler setup and error management - Refactored the setup of webview handlers to use lazy loading for the screen picker module, improving performance and preventing blocking during webview initialization. - Improved error handling in the display media request handler, ensuring that errors are logged without disrupting the webview loading process. - Updated the internal picker provider to streamline the handling of display media requests, particularly for Linux environments. * refactor(gpuFallback): Extend valid fallback modes and improve type safety - Updated the GPU fallback mode validation to include 'wayland' as a valid option, enhancing compatibility with modern Linux environments. - Improved type safety in the reducer by refining the type checks for fallback modes, ensuring better error handling and maintainability. * refactor(gpuFallback): Improve fallback mode handling in reducer - Updated the GPU fallback mode reducer to return the current state if the provided fallback mode is invalid, enhancing stability and preventing unintended state changes. - This change ensures that only valid fallback modes are accepted, improving overall type safety and error handling. * refactor(videoCall): Update display media request handling for improved platform compatibility - Enhanced comments in the IPC and PortalPickerProvider files to clarify the behavior of the display media request handler across different platforms, particularly focusing on macOS and Linux/Wayland. - Adjusted the handling of the XDG portal picker to ensure it returns a valid source or an empty array, improving robustness in source selection during screen sharing. * refactor(config): Clean up whitespace and improve logging consistency - Removed unnecessary whitespace in rollup configuration and video call window files for better readability. - Consolidated console log statements in the app setup to enhance clarity and maintain consistency in logging format. - Improved import organization in screen picker files to follow a more structured format. * fix(build): Update electron-builder command to include appimage target - Modified the build command in the pull request workflow to include 'appimage' as a target alongside 'snap' and 'deb', enhancing the packaging options for Linux distributions. * feat(build): Add AppImage support to pull request workflow - Updated the pull request build workflow to include the AppImage target in the S3 upload command, expanding the packaging options for Linux distributions. * fix(build): Correct AppImage file extension in pull request workflow - Updated the file extension for AppImage in the S3 upload command from '.AppImage' to '.appimage' to ensure proper handling of the file format during the build process. * fix(build): Add AppImage pattern to pull request workflow file matching - Updated the file matching patterns in the pull request build workflow to include the AppImage file extension, ensuring proper identification and handling of AppImage artifacts during the build process. * feat(scripts): Add Linux AppImage testing script - Introduced a new script for testing the Rocket.Chat Linux AppImage, which includes steps for building, making the AppImage executable, and running it. - The script provides options to skip build, installation, and execution, along with informative logging for each step. - Enhanced the relaunch functionality in the app to support AppImage, ensuring reliable relaunch behavior. * refactor(gpuFallback): Simplify GPU fallback handling and improve logging - Removed unnecessary session type checks for Wayland, streamlining the logic for determining X11 fallback needs. - Updated logging messages for clarity when relaunching the app with X11. - Adjusted the order of GPU crash handler setup to catch early failures more effectively. * refactor(gpuFallback): Enhance Wayland handling and logging for X11 fallback - Introduced a check for Wayland sessions in handleLinuxDisplayServer to default to XWayland for stability, addressing potential GPU issues on virtual machines. - Updated logging messages to clarify the use of XWayland when a Wayland session is detected, improving user guidance on display server settings. - Removed redundant Wayland session checks from performElectronStartup, streamlining the GPU fallback logic. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address PR review comments - Remove X11 forcing on Wayland in linux-test-deb.sh (use native Wayland) - Fix console.log formatting with JSON.stringify in src/main.ts - Fix misleading success message in test scripts when app exits early - Remove unnecessary `as any` type assertion in InternalPickerProvider.ts * fix(linux): Enforce X11 mode on Wayland sessions Always use X11 (XWayland) on Wayland sessions for stability. The app automatically relaunches with --ozone-platform=x11 when a Wayland session is detected. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(linux): Add proactive GPU detection with X11 fallback - Remove sentinel file system (crash counting, timing windows) - Add gpu-info-update listener to detect GPU issues early - Check gpu_compositing and webgl status for disabled/unavailable states - Relaunch with --disable-gpu --ozone-platform=x11 when GPU is broken - Simplify crash handler to immediately trigger fallback on GPU crash * fix(ci): Fix AppImage upload and PR comment updates - Fix case sensitivity: rocketchat-*.appimage → rocketchat-*.AppImage (AWS CLI on Linux is case-sensitive, file wasn't being uploaded) - Fix sticky comment header to use simple identifier instead of markdown - Remove redundant recreate/append flags (defaults work correctly) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…etChat#3162) * feat: Enhance Linux support for screen sharing and dependencies - Added `xdg-desktop-portal` and `xdg-desktop-portal-gtk` as recommended dependencies in the Electron builder configuration for improved screen sharing functionality on Linux. - Removed the ozone platform switch for `x11` in the Electron startup process and enabled PipeWire screen capture support for Wayland environments. - Updated the display media request handler to utilize system picker for better user experience during screen sharing. * chore: Bump version to 4.10.2 in package.json * feat(videoCall): Implement screen sharing request handling and cleanup logic - Added a timeout mechanism for screen sharing requests to prevent orphaned listeners. - Introduced cleanup functions for screen sharing listeners and request states to enhance resource management. - Improved IPC handling for screen sharing source responses, ensuring proper request tracking and error handling. - Enhanced user experience by preventing concurrent screen sharing requests and managing listener cleanup effectively. * fix(videoCall): Improve screen sharing timeout handling and listener cleanup - Added validation to ensure the screen sharing timeout is only processed for the current request, preventing double-invocation. - Enhanced cleanup logic to clear timeout references and invoke callbacks appropriately, improving resource management and user experience during screen sharing sessions. * fix(videoCall): Refactor screen sharing listener cleanup and request completion handling - Introduced `removeScreenSharingListenerOnly` to streamline listener removal and timeout clearing without altering the pending request state. - Added `markScreenSharingComplete` to reset request states, allowing new screen sharing requests after completion. - Enhanced existing cleanup logic to improve resource management and prevent concurrent screen sharing requests. * feat(gpuFallback): Implement GPU crash detection and fallback mode for Linux - Added a GPU crash handler to automatically switch to X11 mode if the GPU process crashes during startup. - Introduced a persistent fallback mode setting to remember the user's choice across sessions. - Enhanced the settings UI to allow users to control the display server mode, including options for auto-detect, force X11, or disable GPU. - Updated the application to handle GPU fallback mode changes dynamically, improving user experience and stability on Linux systems. * fix(app): Adjust crash count condition and preserve command-line arguments on relaunch - Updated the crash detection logic to trigger fallback mode after exceeding the maximum crash count. - Enhanced the relaunch functionality to preserve command-line arguments, ensuring user inputs are retained during application restarts. * feat: Enhance Linux display server support and crash handling - Added Wayland support to the Electron builder configuration, allowing for improved compatibility with modern Linux environments. - Updated documentation to reflect auto-detection of display servers for Snap packages, enhancing user clarity. - Refactored crash detection logic to use a more descriptive variable name and improved logging for better debugging during GPU fallback scenarios. * refactor(app): Streamline GPU crash handling and fallback logic - Simplified the GPU crash detection process by consolidating logic for tracking crash counts and managing fallback mode. - Improved logging for better visibility during relaunch scenarios. - Ensured that the application preserves command-line arguments when relaunching after a GPU crash. * docs: Update Linux display server documentation and improve GPU crash recovery details - Clarified the automatic GPU crash detection and recovery process in the documentation. - Removed version-specific language for GPU crash recovery to streamline information. - Enhanced the logging section with standard Chromium flags for better debugging. * feat(scripts): Add installation and testing scripts for Linux - Introduced `install-volta.sh` to automate the installation of Volta, ensuring node.js and npm are available for building the project. - Added `linux-test-deb.sh` for building, installing, and running the Rocket.Chat Desktop .deb package, with options to skip build, install, or run steps. - Created a README.md to document the usage and functionality of the new scripts, enhancing developer experience and automation for testing on Linux. * refactor(scripts): Improve error handling in installation and dependency management - Updated `install-volta.sh` to handle installation errors more gracefully by checking the success of the Volta installation command. - Enhanced `linux-test-deb.sh` to streamline dependency installation checks and provide clearer error messages if installation fails. - Refactored the package installation logic to handle dependency issues more effectively, ensuring smoother installation processes. * feat(linux): Enhance Wayland and X11 support for GPU handling - Implemented auto-detection of Wayland sessions and added logic to relaunch the app with X11 fallback for stability. - Updated GPU fallback mode to include 'wayland' as a valid option, allowing users to specify their preferred display server. - Improved logging for GPU crash handling and display server mode selection, enhancing debugging and user experience on Linux. - Adjusted screen sharing request handling to utilize XDG portal on Wayland sessions, improving compatibility with modern desktop environments. * feat(videoCall): Introduce screen picker functionality for enhanced screen sharing - Implemented a new screen picker system to handle display media requests, improving user experience during screen sharing. - Created internal and portal picker providers to support different environments, including Linux Wayland and X11. - Enhanced IPC handling for screen sharing requests, ensuring proper state management and preventing concurrent requests. - Added initialization and cleanup logic for the screen picker, streamlining resource management and improving performance. * refactor(videoCall): Improve screen sharing request handling and listener management - Refactored the internal picker handler to enhance state management during screen sharing requests, preventing concurrent requests and ensuring proper cleanup. - Improved error handling for screen sharing source validation, including better logging for unavailable sources and request timeouts. - Streamlined the listener setup and removal process, enhancing resource management and overall performance during screen sharing operations. * refactor(videoCall): Enhance webview handler setup and error management - Refactored the setup of webview handlers to use lazy loading for the screen picker module, improving performance and preventing blocking during webview initialization. - Improved error handling in the display media request handler, ensuring that errors are logged without disrupting the webview loading process. - Updated the internal picker provider to streamline the handling of display media requests, particularly for Linux environments. * refactor(gpuFallback): Extend valid fallback modes and improve type safety - Updated the GPU fallback mode validation to include 'wayland' as a valid option, enhancing compatibility with modern Linux environments. - Improved type safety in the reducer by refining the type checks for fallback modes, ensuring better error handling and maintainability. * refactor(gpuFallback): Improve fallback mode handling in reducer - Updated the GPU fallback mode reducer to return the current state if the provided fallback mode is invalid, enhancing stability and preventing unintended state changes. - This change ensures that only valid fallback modes are accepted, improving overall type safety and error handling. * refactor(videoCall): Update display media request handling for improved platform compatibility - Enhanced comments in the IPC and PortalPickerProvider files to clarify the behavior of the display media request handler across different platforms, particularly focusing on macOS and Linux/Wayland. - Adjusted the handling of the XDG portal picker to ensure it returns a valid source or an empty array, improving robustness in source selection during screen sharing. * refactor(config): Clean up whitespace and improve logging consistency - Removed unnecessary whitespace in rollup configuration and video call window files for better readability. - Consolidated console log statements in the app setup to enhance clarity and maintain consistency in logging format. - Improved import organization in screen picker files to follow a more structured format. * fix(build): Update electron-builder command to include appimage target - Modified the build command in the pull request workflow to include 'appimage' as a target alongside 'snap' and 'deb', enhancing the packaging options for Linux distributions. * feat(build): Add AppImage support to pull request workflow - Updated the pull request build workflow to include the AppImage target in the S3 upload command, expanding the packaging options for Linux distributions. * fix(build): Correct AppImage file extension in pull request workflow - Updated the file extension for AppImage in the S3 upload command from '.AppImage' to '.appimage' to ensure proper handling of the file format during the build process. * fix(build): Add AppImage pattern to pull request workflow file matching - Updated the file matching patterns in the pull request build workflow to include the AppImage file extension, ensuring proper identification and handling of AppImage artifacts during the build process. * feat(scripts): Add Linux AppImage testing script - Introduced a new script for testing the Rocket.Chat Linux AppImage, which includes steps for building, making the AppImage executable, and running it. - The script provides options to skip build, installation, and execution, along with informative logging for each step. - Enhanced the relaunch functionality in the app to support AppImage, ensuring reliable relaunch behavior. * refactor(gpuFallback): Simplify GPU fallback handling and improve logging - Removed unnecessary session type checks for Wayland, streamlining the logic for determining X11 fallback needs. - Updated logging messages for clarity when relaunching the app with X11. - Adjusted the order of GPU crash handler setup to catch early failures more effectively. * refactor(gpuFallback): Enhance Wayland handling and logging for X11 fallback - Introduced a check for Wayland sessions in handleLinuxDisplayServer to default to XWayland for stability, addressing potential GPU issues on virtual machines. - Updated logging messages to clarify the use of XWayland when a Wayland session is detected, improving user guidance on display server settings. - Removed redundant Wayland session checks from performElectronStartup, streamlining the GPU fallback logic. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address PR review comments - Remove X11 forcing on Wayland in linux-test-deb.sh (use native Wayland) - Fix console.log formatting with JSON.stringify in src/main.ts - Fix misleading success message in test scripts when app exits early - Remove unnecessary `as any` type assertion in InternalPickerProvider.ts * fix(linux): Enforce X11 mode on Wayland sessions Always use X11 (XWayland) on Wayland sessions for stability. The app automatically relaunches with --ozone-platform=x11 when a Wayland session is detected. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(linux): Add proactive GPU detection with X11 fallback - Remove sentinel file system (crash counting, timing windows) - Add gpu-info-update listener to detect GPU issues early - Check gpu_compositing and webgl status for disabled/unavailable states - Relaunch with --disable-gpu --ozone-platform=x11 when GPU is broken - Simplify crash handler to immediately trigger fallback on GPU crash * fix(ci): Fix AppImage upload and PR comment updates - Fix case sensitivity: rocketchat-*.appimage → rocketchat-*.AppImage (AWS CLI on Linux is case-sensitive, file wasn't being uploaded) - Fix sticky comment header to use simple identifier instead of markdown - Remove redundant recreate/append flags (defaults work correctly) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…etChat#3162) * feat: Enhance Linux support for screen sharing and dependencies - Added `xdg-desktop-portal` and `xdg-desktop-portal-gtk` as recommended dependencies in the Electron builder configuration for improved screen sharing functionality on Linux. - Removed the ozone platform switch for `x11` in the Electron startup process and enabled PipeWire screen capture support for Wayland environments. - Updated the display media request handler to utilize system picker for better user experience during screen sharing. * chore: Bump version to 4.10.2 in package.json * feat(videoCall): Implement screen sharing request handling and cleanup logic - Added a timeout mechanism for screen sharing requests to prevent orphaned listeners. - Introduced cleanup functions for screen sharing listeners and request states to enhance resource management. - Improved IPC handling for screen sharing source responses, ensuring proper request tracking and error handling. - Enhanced user experience by preventing concurrent screen sharing requests and managing listener cleanup effectively. * fix(videoCall): Improve screen sharing timeout handling and listener cleanup - Added validation to ensure the screen sharing timeout is only processed for the current request, preventing double-invocation. - Enhanced cleanup logic to clear timeout references and invoke callbacks appropriately, improving resource management and user experience during screen sharing sessions. * fix(videoCall): Refactor screen sharing listener cleanup and request completion handling - Introduced `removeScreenSharingListenerOnly` to streamline listener removal and timeout clearing without altering the pending request state. - Added `markScreenSharingComplete` to reset request states, allowing new screen sharing requests after completion. - Enhanced existing cleanup logic to improve resource management and prevent concurrent screen sharing requests. * feat(gpuFallback): Implement GPU crash detection and fallback mode for Linux - Added a GPU crash handler to automatically switch to X11 mode if the GPU process crashes during startup. - Introduced a persistent fallback mode setting to remember the user's choice across sessions. - Enhanced the settings UI to allow users to control the display server mode, including options for auto-detect, force X11, or disable GPU. - Updated the application to handle GPU fallback mode changes dynamically, improving user experience and stability on Linux systems. * fix(app): Adjust crash count condition and preserve command-line arguments on relaunch - Updated the crash detection logic to trigger fallback mode after exceeding the maximum crash count. - Enhanced the relaunch functionality to preserve command-line arguments, ensuring user inputs are retained during application restarts. * feat: Enhance Linux display server support and crash handling - Added Wayland support to the Electron builder configuration, allowing for improved compatibility with modern Linux environments. - Updated documentation to reflect auto-detection of display servers for Snap packages, enhancing user clarity. - Refactored crash detection logic to use a more descriptive variable name and improved logging for better debugging during GPU fallback scenarios. * refactor(app): Streamline GPU crash handling and fallback logic - Simplified the GPU crash detection process by consolidating logic for tracking crash counts and managing fallback mode. - Improved logging for better visibility during relaunch scenarios. - Ensured that the application preserves command-line arguments when relaunching after a GPU crash. * docs: Update Linux display server documentation and improve GPU crash recovery details - Clarified the automatic GPU crash detection and recovery process in the documentation. - Removed version-specific language for GPU crash recovery to streamline information. - Enhanced the logging section with standard Chromium flags for better debugging. * feat(scripts): Add installation and testing scripts for Linux - Introduced `install-volta.sh` to automate the installation of Volta, ensuring node.js and npm are available for building the project. - Added `linux-test-deb.sh` for building, installing, and running the Rocket.Chat Desktop .deb package, with options to skip build, install, or run steps. - Created a README.md to document the usage and functionality of the new scripts, enhancing developer experience and automation for testing on Linux. * refactor(scripts): Improve error handling in installation and dependency management - Updated `install-volta.sh` to handle installation errors more gracefully by checking the success of the Volta installation command. - Enhanced `linux-test-deb.sh` to streamline dependency installation checks and provide clearer error messages if installation fails. - Refactored the package installation logic to handle dependency issues more effectively, ensuring smoother installation processes. * feat(linux): Enhance Wayland and X11 support for GPU handling - Implemented auto-detection of Wayland sessions and added logic to relaunch the app with X11 fallback for stability. - Updated GPU fallback mode to include 'wayland' as a valid option, allowing users to specify their preferred display server. - Improved logging for GPU crash handling and display server mode selection, enhancing debugging and user experience on Linux. - Adjusted screen sharing request handling to utilize XDG portal on Wayland sessions, improving compatibility with modern desktop environments. * feat(videoCall): Introduce screen picker functionality for enhanced screen sharing - Implemented a new screen picker system to handle display media requests, improving user experience during screen sharing. - Created internal and portal picker providers to support different environments, including Linux Wayland and X11. - Enhanced IPC handling for screen sharing requests, ensuring proper state management and preventing concurrent requests. - Added initialization and cleanup logic for the screen picker, streamlining resource management and improving performance. * refactor(videoCall): Improve screen sharing request handling and listener management - Refactored the internal picker handler to enhance state management during screen sharing requests, preventing concurrent requests and ensuring proper cleanup. - Improved error handling for screen sharing source validation, including better logging for unavailable sources and request timeouts. - Streamlined the listener setup and removal process, enhancing resource management and overall performance during screen sharing operations. * refactor(videoCall): Enhance webview handler setup and error management - Refactored the setup of webview handlers to use lazy loading for the screen picker module, improving performance and preventing blocking during webview initialization. - Improved error handling in the display media request handler, ensuring that errors are logged without disrupting the webview loading process. - Updated the internal picker provider to streamline the handling of display media requests, particularly for Linux environments. * refactor(gpuFallback): Extend valid fallback modes and improve type safety - Updated the GPU fallback mode validation to include 'wayland' as a valid option, enhancing compatibility with modern Linux environments. - Improved type safety in the reducer by refining the type checks for fallback modes, ensuring better error handling and maintainability. * refactor(gpuFallback): Improve fallback mode handling in reducer - Updated the GPU fallback mode reducer to return the current state if the provided fallback mode is invalid, enhancing stability and preventing unintended state changes. - This change ensures that only valid fallback modes are accepted, improving overall type safety and error handling. * refactor(videoCall): Update display media request handling for improved platform compatibility - Enhanced comments in the IPC and PortalPickerProvider files to clarify the behavior of the display media request handler across different platforms, particularly focusing on macOS and Linux/Wayland. - Adjusted the handling of the XDG portal picker to ensure it returns a valid source or an empty array, improving robustness in source selection during screen sharing. * refactor(config): Clean up whitespace and improve logging consistency - Removed unnecessary whitespace in rollup configuration and video call window files for better readability. - Consolidated console log statements in the app setup to enhance clarity and maintain consistency in logging format. - Improved import organization in screen picker files to follow a more structured format. * fix(build): Update electron-builder command to include appimage target - Modified the build command in the pull request workflow to include 'appimage' as a target alongside 'snap' and 'deb', enhancing the packaging options for Linux distributions. * feat(build): Add AppImage support to pull request workflow - Updated the pull request build workflow to include the AppImage target in the S3 upload command, expanding the packaging options for Linux distributions. * fix(build): Correct AppImage file extension in pull request workflow - Updated the file extension for AppImage in the S3 upload command from '.AppImage' to '.appimage' to ensure proper handling of the file format during the build process. * fix(build): Add AppImage pattern to pull request workflow file matching - Updated the file matching patterns in the pull request build workflow to include the AppImage file extension, ensuring proper identification and handling of AppImage artifacts during the build process. * feat(scripts): Add Linux AppImage testing script - Introduced a new script for testing the Rocket.Chat Linux AppImage, which includes steps for building, making the AppImage executable, and running it. - The script provides options to skip build, installation, and execution, along with informative logging for each step. - Enhanced the relaunch functionality in the app to support AppImage, ensuring reliable relaunch behavior. * refactor(gpuFallback): Simplify GPU fallback handling and improve logging - Removed unnecessary session type checks for Wayland, streamlining the logic for determining X11 fallback needs. - Updated logging messages for clarity when relaunching the app with X11. - Adjusted the order of GPU crash handler setup to catch early failures more effectively. * refactor(gpuFallback): Enhance Wayland handling and logging for X11 fallback - Introduced a check for Wayland sessions in handleLinuxDisplayServer to default to XWayland for stability, addressing potential GPU issues on virtual machines. - Updated logging messages to clarify the use of XWayland when a Wayland session is detected, improving user guidance on display server settings. - Removed redundant Wayland session checks from performElectronStartup, streamlining the GPU fallback logic. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address PR review comments - Remove X11 forcing on Wayland in linux-test-deb.sh (use native Wayland) - Fix console.log formatting with JSON.stringify in src/main.ts - Fix misleading success message in test scripts when app exits early - Remove unnecessary `as any` type assertion in InternalPickerProvider.ts * fix(linux): Enforce X11 mode on Wayland sessions Always use X11 (XWayland) on Wayland sessions for stability. The app automatically relaunches with --ozone-platform=x11 when a Wayland session is detected. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(linux): Add proactive GPU detection with X11 fallback - Remove sentinel file system (crash counting, timing windows) - Add gpu-info-update listener to detect GPU issues early - Check gpu_compositing and webgl status for disabled/unavailable states - Relaunch with --disable-gpu --ozone-platform=x11 when GPU is broken - Simplify crash handler to immediately trigger fallback on GPU crash * fix(ci): Fix AppImage upload and PR comment updates - Fix case sensitivity: rocketchat-*.appimage → rocketchat-*.AppImage (AWS CLI on Linux is case-sensitive, file wasn't being uploaded) - Fix sticky comment header to use simple identifier instead of markdown - Remove redundant recreate/append flags (defaults work correctly) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…etChat#3162) * feat: Enhance Linux support for screen sharing and dependencies - Added `xdg-desktop-portal` and `xdg-desktop-portal-gtk` as recommended dependencies in the Electron builder configuration for improved screen sharing functionality on Linux. - Removed the ozone platform switch for `x11` in the Electron startup process and enabled PipeWire screen capture support for Wayland environments. - Updated the display media request handler to utilize system picker for better user experience during screen sharing. * chore: Bump version to 4.10.2 in package.json * feat(videoCall): Implement screen sharing request handling and cleanup logic - Added a timeout mechanism for screen sharing requests to prevent orphaned listeners. - Introduced cleanup functions for screen sharing listeners and request states to enhance resource management. - Improved IPC handling for screen sharing source responses, ensuring proper request tracking and error handling. - Enhanced user experience by preventing concurrent screen sharing requests and managing listener cleanup effectively. * fix(videoCall): Improve screen sharing timeout handling and listener cleanup - Added validation to ensure the screen sharing timeout is only processed for the current request, preventing double-invocation. - Enhanced cleanup logic to clear timeout references and invoke callbacks appropriately, improving resource management and user experience during screen sharing sessions. * fix(videoCall): Refactor screen sharing listener cleanup and request completion handling - Introduced `removeScreenSharingListenerOnly` to streamline listener removal and timeout clearing without altering the pending request state. - Added `markScreenSharingComplete` to reset request states, allowing new screen sharing requests after completion. - Enhanced existing cleanup logic to improve resource management and prevent concurrent screen sharing requests. * feat(gpuFallback): Implement GPU crash detection and fallback mode for Linux - Added a GPU crash handler to automatically switch to X11 mode if the GPU process crashes during startup. - Introduced a persistent fallback mode setting to remember the user's choice across sessions. - Enhanced the settings UI to allow users to control the display server mode, including options for auto-detect, force X11, or disable GPU. - Updated the application to handle GPU fallback mode changes dynamically, improving user experience and stability on Linux systems. * fix(app): Adjust crash count condition and preserve command-line arguments on relaunch - Updated the crash detection logic to trigger fallback mode after exceeding the maximum crash count. - Enhanced the relaunch functionality to preserve command-line arguments, ensuring user inputs are retained during application restarts. * feat: Enhance Linux display server support and crash handling - Added Wayland support to the Electron builder configuration, allowing for improved compatibility with modern Linux environments. - Updated documentation to reflect auto-detection of display servers for Snap packages, enhancing user clarity. - Refactored crash detection logic to use a more descriptive variable name and improved logging for better debugging during GPU fallback scenarios. * refactor(app): Streamline GPU crash handling and fallback logic - Simplified the GPU crash detection process by consolidating logic for tracking crash counts and managing fallback mode. - Improved logging for better visibility during relaunch scenarios. - Ensured that the application preserves command-line arguments when relaunching after a GPU crash. * docs: Update Linux display server documentation and improve GPU crash recovery details - Clarified the automatic GPU crash detection and recovery process in the documentation. - Removed version-specific language for GPU crash recovery to streamline information. - Enhanced the logging section with standard Chromium flags for better debugging. * feat(scripts): Add installation and testing scripts for Linux - Introduced `install-volta.sh` to automate the installation of Volta, ensuring node.js and npm are available for building the project. - Added `linux-test-deb.sh` for building, installing, and running the Rocket.Chat Desktop .deb package, with options to skip build, install, or run steps. - Created a README.md to document the usage and functionality of the new scripts, enhancing developer experience and automation for testing on Linux. * refactor(scripts): Improve error handling in installation and dependency management - Updated `install-volta.sh` to handle installation errors more gracefully by checking the success of the Volta installation command. - Enhanced `linux-test-deb.sh` to streamline dependency installation checks and provide clearer error messages if installation fails. - Refactored the package installation logic to handle dependency issues more effectively, ensuring smoother installation processes. * feat(linux): Enhance Wayland and X11 support for GPU handling - Implemented auto-detection of Wayland sessions and added logic to relaunch the app with X11 fallback for stability. - Updated GPU fallback mode to include 'wayland' as a valid option, allowing users to specify their preferred display server. - Improved logging for GPU crash handling and display server mode selection, enhancing debugging and user experience on Linux. - Adjusted screen sharing request handling to utilize XDG portal on Wayland sessions, improving compatibility with modern desktop environments. * feat(videoCall): Introduce screen picker functionality for enhanced screen sharing - Implemented a new screen picker system to handle display media requests, improving user experience during screen sharing. - Created internal and portal picker providers to support different environments, including Linux Wayland and X11. - Enhanced IPC handling for screen sharing requests, ensuring proper state management and preventing concurrent requests. - Added initialization and cleanup logic for the screen picker, streamlining resource management and improving performance. * refactor(videoCall): Improve screen sharing request handling and listener management - Refactored the internal picker handler to enhance state management during screen sharing requests, preventing concurrent requests and ensuring proper cleanup. - Improved error handling for screen sharing source validation, including better logging for unavailable sources and request timeouts. - Streamlined the listener setup and removal process, enhancing resource management and overall performance during screen sharing operations. * refactor(videoCall): Enhance webview handler setup and error management - Refactored the setup of webview handlers to use lazy loading for the screen picker module, improving performance and preventing blocking during webview initialization. - Improved error handling in the display media request handler, ensuring that errors are logged without disrupting the webview loading process. - Updated the internal picker provider to streamline the handling of display media requests, particularly for Linux environments. * refactor(gpuFallback): Extend valid fallback modes and improve type safety - Updated the GPU fallback mode validation to include 'wayland' as a valid option, enhancing compatibility with modern Linux environments. - Improved type safety in the reducer by refining the type checks for fallback modes, ensuring better error handling and maintainability. * refactor(gpuFallback): Improve fallback mode handling in reducer - Updated the GPU fallback mode reducer to return the current state if the provided fallback mode is invalid, enhancing stability and preventing unintended state changes. - This change ensures that only valid fallback modes are accepted, improving overall type safety and error handling. * refactor(videoCall): Update display media request handling for improved platform compatibility - Enhanced comments in the IPC and PortalPickerProvider files to clarify the behavior of the display media request handler across different platforms, particularly focusing on macOS and Linux/Wayland. - Adjusted the handling of the XDG portal picker to ensure it returns a valid source or an empty array, improving robustness in source selection during screen sharing. * refactor(config): Clean up whitespace and improve logging consistency - Removed unnecessary whitespace in rollup configuration and video call window files for better readability. - Consolidated console log statements in the app setup to enhance clarity and maintain consistency in logging format. - Improved import organization in screen picker files to follow a more structured format. * fix(build): Update electron-builder command to include appimage target - Modified the build command in the pull request workflow to include 'appimage' as a target alongside 'snap' and 'deb', enhancing the packaging options for Linux distributions. * feat(build): Add AppImage support to pull request workflow - Updated the pull request build workflow to include the AppImage target in the S3 upload command, expanding the packaging options for Linux distributions. * fix(build): Correct AppImage file extension in pull request workflow - Updated the file extension for AppImage in the S3 upload command from '.AppImage' to '.appimage' to ensure proper handling of the file format during the build process. * fix(build): Add AppImage pattern to pull request workflow file matching - Updated the file matching patterns in the pull request build workflow to include the AppImage file extension, ensuring proper identification and handling of AppImage artifacts during the build process. * feat(scripts): Add Linux AppImage testing script - Introduced a new script for testing the Rocket.Chat Linux AppImage, which includes steps for building, making the AppImage executable, and running it. - The script provides options to skip build, installation, and execution, along with informative logging for each step. - Enhanced the relaunch functionality in the app to support AppImage, ensuring reliable relaunch behavior. * refactor(gpuFallback): Simplify GPU fallback handling and improve logging - Removed unnecessary session type checks for Wayland, streamlining the logic for determining X11 fallback needs. - Updated logging messages for clarity when relaunching the app with X11. - Adjusted the order of GPU crash handler setup to catch early failures more effectively. * refactor(gpuFallback): Enhance Wayland handling and logging for X11 fallback - Introduced a check for Wayland sessions in handleLinuxDisplayServer to default to XWayland for stability, addressing potential GPU issues on virtual machines. - Updated logging messages to clarify the use of XWayland when a Wayland session is detected, improving user guidance on display server settings. - Removed redundant Wayland session checks from performElectronStartup, streamlining the GPU fallback logic. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address PR review comments - Remove X11 forcing on Wayland in linux-test-deb.sh (use native Wayland) - Fix console.log formatting with JSON.stringify in src/main.ts - Fix misleading success message in test scripts when app exits early - Remove unnecessary `as any` type assertion in InternalPickerProvider.ts * fix(linux): Enforce X11 mode on Wayland sessions Always use X11 (XWayland) on Wayland sessions for stability. The app automatically relaunches with --ozone-platform=x11 when a Wayland session is detected. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(linux): Add proactive GPU detection with X11 fallback - Remove sentinel file system (crash counting, timing windows) - Add gpu-info-update listener to detect GPU issues early - Check gpu_compositing and webgl status for disabled/unavailable states - Relaunch with --disable-gpu --ozone-platform=x11 when GPU is broken - Simplify crash handler to immediately trigger fallback on GPU crash * fix(ci): Fix AppImage upload and PR comment updates - Fix case sensitivity: rocketchat-*.appimage → rocketchat-*.AppImage (AWS CLI on Linux is case-sensitive, file wasn't being uploaded) - Fix sticky comment header to use simple identifier instead of markdown - Remove redundant recreate/append flags (defaults work correctly) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…etChat#3162) * feat: Enhance Linux support for screen sharing and dependencies - Added `xdg-desktop-portal` and `xdg-desktop-portal-gtk` as recommended dependencies in the Electron builder configuration for improved screen sharing functionality on Linux. - Removed the ozone platform switch for `x11` in the Electron startup process and enabled PipeWire screen capture support for Wayland environments. - Updated the display media request handler to utilize system picker for better user experience during screen sharing. * chore: Bump version to 4.10.2 in package.json * feat(videoCall): Implement screen sharing request handling and cleanup logic - Added a timeout mechanism for screen sharing requests to prevent orphaned listeners. - Introduced cleanup functions for screen sharing listeners and request states to enhance resource management. - Improved IPC handling for screen sharing source responses, ensuring proper request tracking and error handling. - Enhanced user experience by preventing concurrent screen sharing requests and managing listener cleanup effectively. * fix(videoCall): Improve screen sharing timeout handling and listener cleanup - Added validation to ensure the screen sharing timeout is only processed for the current request, preventing double-invocation. - Enhanced cleanup logic to clear timeout references and invoke callbacks appropriately, improving resource management and user experience during screen sharing sessions. * fix(videoCall): Refactor screen sharing listener cleanup and request completion handling - Introduced `removeScreenSharingListenerOnly` to streamline listener removal and timeout clearing without altering the pending request state. - Added `markScreenSharingComplete` to reset request states, allowing new screen sharing requests after completion. - Enhanced existing cleanup logic to improve resource management and prevent concurrent screen sharing requests. * feat(gpuFallback): Implement GPU crash detection and fallback mode for Linux - Added a GPU crash handler to automatically switch to X11 mode if the GPU process crashes during startup. - Introduced a persistent fallback mode setting to remember the user's choice across sessions. - Enhanced the settings UI to allow users to control the display server mode, including options for auto-detect, force X11, or disable GPU. - Updated the application to handle GPU fallback mode changes dynamically, improving user experience and stability on Linux systems. * fix(app): Adjust crash count condition and preserve command-line arguments on relaunch - Updated the crash detection logic to trigger fallback mode after exceeding the maximum crash count. - Enhanced the relaunch functionality to preserve command-line arguments, ensuring user inputs are retained during application restarts. * feat: Enhance Linux display server support and crash handling - Added Wayland support to the Electron builder configuration, allowing for improved compatibility with modern Linux environments. - Updated documentation to reflect auto-detection of display servers for Snap packages, enhancing user clarity. - Refactored crash detection logic to use a more descriptive variable name and improved logging for better debugging during GPU fallback scenarios. * refactor(app): Streamline GPU crash handling and fallback logic - Simplified the GPU crash detection process by consolidating logic for tracking crash counts and managing fallback mode. - Improved logging for better visibility during relaunch scenarios. - Ensured that the application preserves command-line arguments when relaunching after a GPU crash. * docs: Update Linux display server documentation and improve GPU crash recovery details - Clarified the automatic GPU crash detection and recovery process in the documentation. - Removed version-specific language for GPU crash recovery to streamline information. - Enhanced the logging section with standard Chromium flags for better debugging. * feat(scripts): Add installation and testing scripts for Linux - Introduced `install-volta.sh` to automate the installation of Volta, ensuring node.js and npm are available for building the project. - Added `linux-test-deb.sh` for building, installing, and running the Rocket.Chat Desktop .deb package, with options to skip build, install, or run steps. - Created a README.md to document the usage and functionality of the new scripts, enhancing developer experience and automation for testing on Linux. * refactor(scripts): Improve error handling in installation and dependency management - Updated `install-volta.sh` to handle installation errors more gracefully by checking the success of the Volta installation command. - Enhanced `linux-test-deb.sh` to streamline dependency installation checks and provide clearer error messages if installation fails. - Refactored the package installation logic to handle dependency issues more effectively, ensuring smoother installation processes. * feat(linux): Enhance Wayland and X11 support for GPU handling - Implemented auto-detection of Wayland sessions and added logic to relaunch the app with X11 fallback for stability. - Updated GPU fallback mode to include 'wayland' as a valid option, allowing users to specify their preferred display server. - Improved logging for GPU crash handling and display server mode selection, enhancing debugging and user experience on Linux. - Adjusted screen sharing request handling to utilize XDG portal on Wayland sessions, improving compatibility with modern desktop environments. * feat(videoCall): Introduce screen picker functionality for enhanced screen sharing - Implemented a new screen picker system to handle display media requests, improving user experience during screen sharing. - Created internal and portal picker providers to support different environments, including Linux Wayland and X11. - Enhanced IPC handling for screen sharing requests, ensuring proper state management and preventing concurrent requests. - Added initialization and cleanup logic for the screen picker, streamlining resource management and improving performance. * refactor(videoCall): Improve screen sharing request handling and listener management - Refactored the internal picker handler to enhance state management during screen sharing requests, preventing concurrent requests and ensuring proper cleanup. - Improved error handling for screen sharing source validation, including better logging for unavailable sources and request timeouts. - Streamlined the listener setup and removal process, enhancing resource management and overall performance during screen sharing operations. * refactor(videoCall): Enhance webview handler setup and error management - Refactored the setup of webview handlers to use lazy loading for the screen picker module, improving performance and preventing blocking during webview initialization. - Improved error handling in the display media request handler, ensuring that errors are logged without disrupting the webview loading process. - Updated the internal picker provider to streamline the handling of display media requests, particularly for Linux environments. * refactor(gpuFallback): Extend valid fallback modes and improve type safety - Updated the GPU fallback mode validation to include 'wayland' as a valid option, enhancing compatibility with modern Linux environments. - Improved type safety in the reducer by refining the type checks for fallback modes, ensuring better error handling and maintainability. * refactor(gpuFallback): Improve fallback mode handling in reducer - Updated the GPU fallback mode reducer to return the current state if the provided fallback mode is invalid, enhancing stability and preventing unintended state changes. - This change ensures that only valid fallback modes are accepted, improving overall type safety and error handling. * refactor(videoCall): Update display media request handling for improved platform compatibility - Enhanced comments in the IPC and PortalPickerProvider files to clarify the behavior of the display media request handler across different platforms, particularly focusing on macOS and Linux/Wayland. - Adjusted the handling of the XDG portal picker to ensure it returns a valid source or an empty array, improving robustness in source selection during screen sharing. * refactor(config): Clean up whitespace and improve logging consistency - Removed unnecessary whitespace in rollup configuration and video call window files for better readability. - Consolidated console log statements in the app setup to enhance clarity and maintain consistency in logging format. - Improved import organization in screen picker files to follow a more structured format. * fix(build): Update electron-builder command to include appimage target - Modified the build command in the pull request workflow to include 'appimage' as a target alongside 'snap' and 'deb', enhancing the packaging options for Linux distributions. * feat(build): Add AppImage support to pull request workflow - Updated the pull request build workflow to include the AppImage target in the S3 upload command, expanding the packaging options for Linux distributions. * fix(build): Correct AppImage file extension in pull request workflow - Updated the file extension for AppImage in the S3 upload command from '.AppImage' to '.appimage' to ensure proper handling of the file format during the build process. * fix(build): Add AppImage pattern to pull request workflow file matching - Updated the file matching patterns in the pull request build workflow to include the AppImage file extension, ensuring proper identification and handling of AppImage artifacts during the build process. * feat(scripts): Add Linux AppImage testing script - Introduced a new script for testing the Rocket.Chat Linux AppImage, which includes steps for building, making the AppImage executable, and running it. - The script provides options to skip build, installation, and execution, along with informative logging for each step. - Enhanced the relaunch functionality in the app to support AppImage, ensuring reliable relaunch behavior. * refactor(gpuFallback): Simplify GPU fallback handling and improve logging - Removed unnecessary session type checks for Wayland, streamlining the logic for determining X11 fallback needs. - Updated logging messages for clarity when relaunching the app with X11. - Adjusted the order of GPU crash handler setup to catch early failures more effectively. * refactor(gpuFallback): Enhance Wayland handling and logging for X11 fallback - Introduced a check for Wayland sessions in handleLinuxDisplayServer to default to XWayland for stability, addressing potential GPU issues on virtual machines. - Updated logging messages to clarify the use of XWayland when a Wayland session is detected, improving user guidance on display server settings. - Removed redundant Wayland session checks from performElectronStartup, streamlining the GPU fallback logic. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address PR review comments - Remove X11 forcing on Wayland in linux-test-deb.sh (use native Wayland) - Fix console.log formatting with JSON.stringify in src/main.ts - Fix misleading success message in test scripts when app exits early - Remove unnecessary `as any` type assertion in InternalPickerProvider.ts * fix(linux): Enforce X11 mode on Wayland sessions Always use X11 (XWayland) on Wayland sessions for stability. The app automatically relaunches with --ozone-platform=x11 when a Wayland session is detected. * feat(wayland): Enable native Wayland support by default Use native Wayland when available instead of forcing X11 fallback. The GPU crash handler will automatically switch to X11 if issues occur. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(linux): Add proactive GPU detection with X11 fallback - Remove sentinel file system (crash counting, timing windows) - Add gpu-info-update listener to detect GPU issues early - Check gpu_compositing and webgl status for disabled/unavailable states - Relaunch with --disable-gpu --ozone-platform=x11 when GPU is broken - Simplify crash handler to immediately trigger fallback on GPU crash * fix(ci): Fix AppImage upload and PR comment updates - Fix case sensitivity: rocketchat-*.appimage → rocketchat-*.AppImage (AWS CLI on Linux is case-sensitive, file wasn't being uploaded) - Fix sticky comment header to use simple identifier instead of markdown - Remove redundant recreate/append flags (defaults work correctly) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Enable Native Wayland Support with Automatic GPU Crash Recovery
Summary
This PR enables native Wayland support for Rocket.Chat Desktop on Linux while implementing automatic GPU crash detection and recovery. The implementation addresses platform-specific challenges with GPU acceleration and screen sharing across different Linux configurations, particularly in virtual machine environments.
Context
Linux desktop environments present multiple display server options (X11, Wayland) with varying GPU acceleration capabilities. Previous versions forced X11 mode for all Linux sessions, preventing users from benefiting from native Wayland features like PipeWire screen capture. Additionally, GPU crashes on incompatible hardware (particularly in VMs) required manual intervention to recover.
This work introduces:
Changes Made
GPU Crash Detection and Recovery
Implemented proactive GPU failure detection that monitors GPU feature status during application startup:
gpu-info-updateevent to check GPU compositing and WebGL status earlychild-process-goneevents for GPU process crashes--disable-gpu --ozone-platform=x11when failures detecteddisabled_*andunavailable_*GPU feature statesLocation:
src/app/main/app.ts:175-223Technical Implementation:
Native Wayland Support
Removed X11 forcing on Linux to enable native Wayland when available:
app.commandLine.appendSwitch('ozone-platform', 'x11')from startupWebRTCPipeWireCapturer)xdg-desktop-portalandxdg-desktop-portal-gtkas recommended packagesLocation:
src/app/main/app.ts:85-165Screen Sharing Architecture
Implemented a pluggable screen picker system with platform-specific providers:
Provider Architecture:
ScreenPickerProviderinterface for platform abstractionInternalPickerProvider: Uses Electron's built-in desktop capturer with internal UIPortalPickerProvider: Uses XDG Desktop Portal for Wayland environmentsRequest Management:
Location:
src/videoCallWindow/screenPicker/Key Implementation Details:
desktopCapturer.getSources()on WaylandAppImage Support
Enhanced AppImage compatibility with proper relaunch handling:
process.env.APPIMAGEto identify AppImage executionapp.relaunch()app.relaunch()doesn't work reliably with AppImagesLocation:
src/app/main/app.ts:60-82Documentation
Created comprehensive Linux display server documentation:
Location:
docs/linux-display-server.mdTechnical Highlights
Display Server Detection Flow
gpu-info-updateevent--disable-gpu --ozone-platform=x11child-process-gonecatches runtime GPU crashesScreen Sharing Provider Selection
Command-Line Flag Handling
--disable-gpu,--ozone-platform) prependedReferences
Closes #3161
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.