fix(linux): Prevent Wayland/X11 crashes#3171
Conversation
- Updated the Linux display server configuration to automatically detect Wayland sessions and fallback to X11 if necessary, preventing crashes during initialization. - Improved documentation to clarify the automatic detection process and added troubleshooting information for Ubuntu 22.04 LTS users experiencing segfaults. - Introduced unit tests for the platform detection logic to ensure correct behavior across various session types and manual overrides.
- Refactored logging tests in `app.main.spec.ts` to validate JSON structure of log messages for Wayland and X11 sessions. - Removed outdated tests for session type handling and improved assertions for log data properties. - Enhanced clarity in test descriptions to reflect the updated logging behavior during Electron startup.
…ection tests - Eliminated unnecessary assertions for JSON parsing in the logging tests for Wayland and X11 sessions in `app.main.spec.ts`. - Streamlined test cases to focus on validating the structure of log data without redundant error handling, enhancing clarity and maintainability.
WalkthroughAdds a pre-launch Linux wrapper and in-process startup detection to choose Wayland vs X11 before Electron initializes: inspects XDG_SESSION_TYPE, WAYLAND_DISPLAY, and Wayland socket presence, emits structured JSON decision logs, and forces Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User (launcher)
participant Wrapper as Linux wrapper (shell)
participant OS as Linux env
participant Binary as rocketchat-desktop.bin
participant Electron as Electron runtime
Note over Wrapper,OS: Wrapper runs before Electron process start
User->>Wrapper: launch app with args
Wrapper->>OS: read XDG_SESSION_TYPE, WAYLAND_DISPLAY, check XDG_RUNTIME_DIR or /run/user/<uid> socket
alt Valid Wayland session & socket
Wrapper->>Binary: exec binary (no ozone override)
Binary->>Electron: start (Electron may auto-detect Wayland)
else No valid Wayland / invalid session
Wrapper->>Binary: exec binary with --ozone-platform=x11
Binary->>Electron: start (Chromium forced to X11 ozone)
end
Electron->>OS: initialize display/GPU
Note right of Electron: Emit structured JSON log with decision reason/session/waylandDisplay
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
📜 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)
docs/linux-display-server.mdsrc/app/main/app.main.spec.tssrc/app/main/app.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.main.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Main-process tests must be named with the suffix
.main.spec.tsand run under the Electron runner
Files:
src/app/main/app.main.spec.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/app/main/app.main.spec.tssrc/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 (4)
📚 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.spec.ts : Main-process tests must be named with the suffix `.main.spec.ts` and run under the Electron runner
Applied to files:
src/app/main/app.main.spec.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).spec.ts : Renderer-process tests should be named `*.spec.ts` (without the `.main` qualifier)
Applied to files:
src/app/main/app.main.spec.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.main.spec.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/app/main/app.main.spec.ts
🧬 Code graph analysis (1)
src/app/main/app.main.spec.ts (1)
src/app/main/app.ts (1)
performElectronStartup(84-225)
⏰ 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 (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
🔇 Additional comments (13)
src/app/main/app.ts (1)
165-223: LGTM! Solid implementation of automatic display server detection.The Linux display server detection logic is well-implemented with:
- Proper override detection respecting manual configuration
- Comprehensive edge case handling (empty strings, whitespace, missing variables)
- Clear fallback reasons for troubleshooting
- Structured logging for debugging
The logic correctly prevents initialization crashes by forcing X11 when Wayland is unavailable, addressing the Ubuntu 22.04 LTS compatibility issue.
src/app/main/app.main.spec.ts (8)
78-132: Comprehensive manual override tests.The manual override detection tests properly verify that explicit
--ozone-platformflags take precedence over automatic detection across various scenarios (x11, wayland, auto).
134-205: Thorough Wayland detection test coverage.Tests cover the critical Wayland detection logic including:
- Valid Wayland sessions (with both environment variables set)
- Invalid scenarios (missing or empty WAYLAND_DISPLAY)
- Whitespace normalization
207-254: X11 fallback scenarios well-tested.Tests validate correct X11 fallback behavior for various session types and edge cases (unset, empty string, explicit x11).
256-303: Good coverage of invalid session types.Tests verify that invalid session types (tty, mir, unknown) properly fall back to X11 with appropriate reason codes.
305-363: Normalization tests validate edge case handling.Tests confirm that whitespace trimming works correctly for both
XDG_SESSION_TYPEandWAYLAND_DISPLAY, preventing common misconfiguration issues.
365-398: Logging validation ensures structured output.Tests verify that log messages follow a consistent structure with valid JSON, making debugging easier in production.
400-433: Platform-specific behavior correctly isolated.Tests confirm that the display server detection logic only runs on Linux, preventing unintended behavior on other platforms.
435-468: Integration tests verify end-to-end behavior.Tests confirm that platform detection works correctly alongside other Electron command-line switches like PipeWire support.
docs/linux-display-server.md (4)
1-23: Clear and accurate documentation of automatic detection behavior.The documentation properly describes the new automatic display server detection with X11 fallback, including:
- Updated default behavior summary
- Detailed detection logic explanation
- Logging output examples for troubleshooting
This aligns well with the implementation changes in
src/app/main/app.ts.
50-64: Well-documented Ubuntu 22.04 LTS fix.The new Ubuntu 22.04 LTS X11 Sessions section clearly explains:
- The symptoms users were experiencing
- The automatic fix implemented
- Manual override options if needed
This directly addresses the PR objective and provides good user guidance.
106-119: Manual override documentation is clear.The updated manual override section properly notes that manual overrides take precedence over automatic detection, which is important for users who need explicit control.
152-158: Best practices reflect the implementation changes.The updated best practices section accurately describes the automatic behavior for different scenarios (X11 sessions, Snap packages, Ubuntu 22.04 LTS).
Linux installer download |
Windows installer download |
macOS installer download |
jeanfbrito
changed the title
The app.commandLine.appendSwitch() approach doesn't work because Chromium initializes before Electron JavaScript runs. Fix by package type: - deb/rpm/tar.gz: Wrapper script runs BEFORE binary, detects display server - Snap: Set allowNativeWayland=false to force X11 via electron-builder - Flatpak/AppImage: Use electron-builder launcher with X11 fallback Wrapper script detection logic: - If XDG_SESSION_TYPE != wayland: force X11 - If WAYLAND_DISPLAY is empty: force X11 - If Wayland socket doesn't exist: force X11 - Otherwise: use native Wayland Validated on: - Fedora 42 physical (GTX 1660 Ti) - Wayland native - Ubuntu 22.04 physical (GTX 1660 Ti) - X11 session - Fedora 42 VM (no GPU) - Ubuntu 22.04 VM (no GPU) All 5 test scenarios pass with fix vs SEGFAULT without. Closes #3154
- Add fs.statSync check to verify Wayland socket actually exists - Add 'socket-not-found' reason when socket is missing - Remove placeholder dates from timeline in postmortem - Add tests for socket existence checking
jeanfbrito
force-pushed
the
fix-x11-ubuntu2204
branch
from
01c3111 to
e4881a3
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @docs/linux-display-server.md:
- Line 32: The table row for Snap incorrectly says "Auto-detect" for display
server; update that cell to reflect that Snap builds force X11 by setting
"allowNativeWayland": false in electron-builder.json (e.g., change to "X11
(Wayland disabled)" or similar) and add a brief note that Snap disables Wayland
via allowNativeWayland=false so readers know the explicit configuration.
In @docs/linux-wayland-bug-postmortem.md:
- Around line 9-11: Add a new top-level section titled "The Solution That
Actually Worked" immediately after the Executive Summary; include four
subsections: "Problem" (one-sentence summary: SEGFAULT when Wayland env vars
exist but no compositor because Chromium init runs before Electron JS),
"Solution" (one-sentence summary: add a shell wrapper script
build/linux/wrapper.sh that detects XDG_SESSION_TYPE, WAYLAND_DISPLAY and socket
existence and passes --ozone-platform=x11 when needed), "Result" (one-sentence
summary: all test scenarios pass on Ubuntu 22.04 and Fedora 42, wrapper allows
native Wayland when socket valid and forces X11 in failure cases preventing
segfaults), and "PR" with the link #[3171]; ensure this new section replaces or
precedes the longer solution narrative currently later in the doc so readers see
the concise Problem/Solution/Result/PR up front.
- Line 6: Update the date field that currently reads "- **Date**: January 2025"
to "- **Date**: January 2026" so the document reflects the actual last
modification date of January 8, 2026; locate and edit the Date line in
docs/linux-wayland-bug-postmortem.md (the "- **Date**: January 2025" entry) and
change the year to 2026.
🧹 Nitpick comments (4)
docs/linux-wayland-bug-postmortem.md (3)
24-35: Add language identifier to fenced code block.The error output code block should specify a language for proper syntax highlighting and accessibility.
✨ Suggested fix
-``` +```text ERROR:ui/ozone/platform/wayland/host/wayland_connection.cc:202 Failed to connect to Wayland display: No such file or directory (2)
147-169: Add language identifiers to timeline diagrams.The process startup timeline diagrams (lines 147-169 and 178-203) should specify a language identifier for proper rendering.
✨ Suggested fix
Change the opening backticks from
```to```textfor both timeline diagrams.Also applies to: 178-203
325-328: Format reference URLs as proper markdown links.The bare URLs should be formatted as proper markdown links for better rendering and accessibility.
✨ Suggested fix
-- Chromium Ozone Platform: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/ozone_overview.md -- Electron Command Line Switches: https://www.electronjs.org/docs/latest/api/command-line-switches -- Wayland Protocol: https://wayland.freedesktop.org/ -- XDG Base Directory Specification: https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html +- [Chromium Ozone Platform](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/ozone_overview.md) +- [Electron Command Line Switches](https://www.electronjs.org/docs/latest/api/command-line-switches) +- [Wayland Protocol](https://wayland.freedesktop.org/) +- [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html)docs/linux-display-server.md (1)
46-50: Clarify Wayland benefits vs. setup requirements.Lines 46–50 list native Wayland benefits (PipeWire screen capture, etc.), but the requirements section (lines 52–56) shows that PipeWire and xdg-desktop-portal packages must be installed separately. The flow could better clarify that these benefits require additional setup beyond the detector working correctly. Consider adding a note like "These features require the dependencies listed in 'Requirements for Wayland' to be installed."
📜 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 (7)
build/afterPack.jsbuild/linux/wrapper.shdocs/linux-display-server.mddocs/linux-wayland-bug-postmortem.mdelectron-builder.jsonsrc/app/main/app.main.spec.tssrc/app/main/app.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/main/app.main.spec.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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
{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
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.mddocs/linux-wayland-bug-postmortem.md
🧠 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/main.ts : Keep `src/main.ts` as the Electron main process entry point compiled by Rollup
Applied to files:
build/afterPack.jssrc/app/main/app.ts
🧬 Code graph analysis (1)
src/app/main/app.ts (2)
build/afterPack.js (1)
fs(1-1)workspaces/desktop-release-action/dist/index.js (2)
fs(475-475)fs(2861-2861)
🪛 markdownlint-cli2 (0.18.1)
docs/linux-wayland-bug-postmortem.md
24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
147-147: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
178-178: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
325-325: Bare URL used
(MD034, no-bare-urls)
326-326: Bare URL used
(MD034, no-bare-urls)
327-327: Bare URL used
(MD034, no-bare-urls)
328-328: Bare URL used
(MD034, no-bare-urls)
⏰ 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 (11)
electron-builder.json (1)
150-150: LGTM! Snap configuration correctly forces X11.The
allowNativeWayland: falsesetting properly configures Snap packages to use X11, preventing the Wayland-related segfaults. This aligns with the wrapper-based approach for other Linux package formats (deb/rpm/tar.gz).build/linux/wrapper.sh (1)
1-29: LGTM! Wrapper script implementation is correct.The wrapper script correctly implements the display server detection logic:
- Uses
set -euo pipefailfor robust error handling- Properly resolves the binary path using
readlink -f- Detection logic correctly checks session type, display variable, and socket existence
- Intentionally uses unquoted
$EXTRA_ARGSto avoid passing empty arguments- Uses
execto replace the shell process with the binaryThe logic aligns perfectly with the detection matrix documented in the post-mortem.
build/afterPack.js (2)
6-29: LGTM! Linux wrapper installation logic is solid.The
setupLinuxWrapperfunction correctly:
- Checks for idempotency (skips if already installed)
- Renames the binary to
.binbefore installing the wrapper- Copies and sets proper permissions (0o755) on the wrapper script
- Provides clear console logging for build visibility
Using synchronous fs operations is appropriate for build scripts where failures should stop the build process.
80-82: LGTM! Conditional wrapper installation is correctly placed.The Linux wrapper setup is properly invoked only for Linux platform builds, and is positioned after the fuses are applied. This ensures the wrapper is installed for all Linux package formats that use the afterPack hook.
src/app/main/app.ts (5)
2-2: LGTM! fs import added for Wayland socket detection.The fs module import is necessary for the new
fs.statSync()call used to verify Wayland socket existence in the display server detection logic.
167-172: LGTM! Override detection prevents conflicts.The logic correctly checks for existing ozone-platform overrides from both command-line arguments and the
ELECTRON_OZONE_PLATFORM_HINTenvironment variable. This ensures the auto-detection doesn't interfere with user or system-provided overrides.
175-180: LGTM! Environment variable normalization is defensive.Trimming whitespace and converting undefined values to empty strings prevents edge cases where environment variables have unexpected formatting. This defensive programming aligns with the documented failure scenarios.
188-204: LGTM! Socket existence check is robust.The
checkWaylandSocket()function correctly:
- Returns false early if session isn't Wayland or display is unset
- Uses try-catch to handle filesystem errors gracefully
- Verifies the path is actually a socket (not just a file)
- Falls back to
/run/user/$(id)whenXDG_RUNTIME_DIRis unsetNote:
process.getuid()is safe here since this code only runs on Linux (line 164 platform check).
207-244: LGTM! Display server selection logic is comprehensive.The conditional logic correctly implements the detection matrix from the post-mortem:
- Valid Wayland session → allow Electron auto-detection (native Wayland)
- All failure scenarios → force X11 with detailed reason logging
The reason codes (
socket-not-found,no-wayland-display,x11-session,no-session-type,invalid-session-type) provide excellent debugging information and cover all documented edge cases.docs/linux-wayland-bug-postmortem.md (1)
332-333: The completion date is correct and consistent with the document.The post-mortem completion date "2025-01-08" is accurate. The file metadata (line 6) indicates "Date: January 2025", and all timeline entries (lines 303-305) are dated 2025-01-07, making the completion date of 2025-01-08 verifiable and internally consistent. There is no indication in the document that this post-mortem was created in 2026.
Likely an incorrect or invalid review comment.
docs/linux-display-server.md (1)
1-145: Comprehensive and well-structured documentation.The document clearly explains the wrapper-based detection logic, package-specific behavior, practical scenarios, and troubleshooting steps. The detection table (lines 18–25) and command examples (lines 105–107, 112, 140–145) follow Linux conventions and are verifiable. No invented metrics or unsupported claims detected. Once the Snap behavior is clarified, this documentation aligns well with the PR objectives and coding guidelines.
- Add tests for ELECTRON_OZONE_PLATFORM_HINT env var override - Fix Snap docs: Auto-detect → Force X11 (allowNativeWayland: false) - Fix date: January 2025 → January 2026 - Add 'The Solution That Actually Worked' section to postmortem
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @docs/linux-wayland-bug-postmortem.md:
- Around line 318-325: The Timeline table in the "Timeline" section contains
dates labeled 2025 (rows showing 2025-01-07) that conflict with the PR creation
in January 2026; update those date entries in the timeline table (the rows under
"Date | Event") to the correct year 2026 so the timeline is consistent with the
PR and other references.
In @src/app/main/app.main.spec.ts:
- Around line 34-39: The test currently mocks ../actions with an
electronBuilderJsonInformation object, but performElectronStartup imports
electronBuilderJsonInformation from app.ts so this mock is incorrect and
ineffective; either remove the jest.mock('../actions', ...) block entirely, or
replace it with a mock that matches the real module you need to stub (e.g., mock
the module that actually exports electronBuilderJsonInformation such as app.ts)
and ensure the mocked symbol name is electronBuilderJsonInformation so
performElectronStartup sees the stubbed value.
🧹 Nitpick comments (1)
docs/linux-wayland-bug-postmortem.md (1)
344-347: Consider using Markdown link syntax for references.The bare URLs work but proper Markdown links are more consistent with the rest of the document.
📝 Suggested format
-- Chromium Ozone Platform: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/ozone_overview.md -- Electron Command Line Switches: https://www.electronjs.org/docs/latest/api/command-line-switches -- Wayland Protocol: https://wayland.freedesktop.org/ -- XDG Base Directory Specification: https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html +- [Chromium Ozone Platform](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/ozone_overview.md) +- [Electron Command Line Switches](https://www.electronjs.org/docs/latest/api/command-line-switches) +- [Wayland Protocol](https://wayland.freedesktop.org/) +- [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html)
📜 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 (8)
build/afterPack.jsbuild/linux/wrapper.shdocs/linux-display-server.mddocs/linux-wayland-bug-postmortem.mdelectron-builder.jsonpackage.jsonsrc/app/main/app.main.spec.tssrc/app/main/app.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- build/linux/wrapper.sh
- build/afterPack.js
- docs/linux-display-server.md
- electron-builder.json
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.main.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Main-process tests must be named with the suffix
.main.spec.tsand run under the Electron runner
Files:
src/app/main/app.main.spec.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/app/main/app.main.spec.tssrc/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-wayland-bug-postmortem.md
🧠 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.spec.ts : Main-process tests must be named with the suffix `.main.spec.ts` and run under the Electron runner
Applied to files:
src/app/main/app.main.spec.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).spec.ts : Renderer-process tests should be named `*.spec.ts` (without the `.main` qualifier)
Applied to files:
src/app/main/app.main.spec.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.main.spec.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/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 docs/**/*.md : Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Applied to files:
docs/linux-wayland-bug-postmortem.md
🧬 Code graph analysis (1)
src/app/main/app.main.spec.ts (1)
src/app/main/app.ts (1)
performElectronStartup(85-246)
🪛 markdownlint-cli2 (0.18.1)
docs/linux-wayland-bug-postmortem.md
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
166-166: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
197-197: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
344-344: Bare URL used
(MD034, no-bare-urls)
345-345: Bare URL used
(MD034, no-bare-urls)
346-346: Bare URL used
(MD034, no-bare-urls)
347-347: Bare URL used
(MD034, no-bare-urls)
⏰ 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 (8)
package.json (1)
9-9: LGTM!Patch version bump to 4.11.1 is appropriate for this bug fix release.
src/app/main/app.ts (2)
166-244: Well-structured Wayland detection logic.The implementation properly handles the critical timing issue by detecting display server before Electron auto-selects. The socket validation using
fs.statSyncwithisSocket()check is thorough, and the structured JSON logging with reason codes will aid debugging. The fallback to/run/user/${process.getuid()}whenXDG_RUNTIME_DIRis unset is a good defensive measure.
188-204: Socket validation is robust.The
checkWaylandSocket()helper correctly:
- Returns early if session type isn't
waylandor display is empty- Uses
fs.statSyncwith proper error handling- Validates that the path is actually a socket via
isSocket()This covers the crash scenarios where environment variables exist but the socket doesn't.
docs/linux-wayland-bug-postmortem.md (1)
15-31: Post-mortem structure follows guidelines correctly.The "The Solution That Actually Worked" section includes all required elements: Problem, Solution, Result, and PR reference. The technical explanation of why
app.commandLine.appendSwitch()fails and why the wrapper works is clear and valuable for future debugging.src/app/main/app.main.spec.ts (4)
1-6: LGTM! Test file follows naming conventions.The file is correctly named with
.main.spec.tssuffix as required for main-process tests per the coding guidelines.
46-84: Excellent test setup with proper state management.The test suite properly preserves and restores
process.platform,process.env,process.argv, andconsole.logbetween tests. ThebeforeEachcorrectly clears relevant environment variables to ensure test isolation.
173-280: Comprehensive Wayland detection test coverage.The tests thoroughly cover all Wayland detection scenarios:
- Valid Wayland session with socket ✓
- Missing WAYLAND_DISPLAY ✓
- Empty/whitespace WAYLAND_DISPLAY ✓
- Socket doesn't exist (ENOENT) ✓
- Path exists but is not a socket ✓
The assertions correctly validate both the
appendSwitchcalls and the structured JSON log output with appropriate reason codes.
510-543: Good integration tests verifying non-interference.The integration tests properly verify that:
- Wayland detection works alongside PipeWire feature enablement
- Other command-line switches (autoplay-policy) are not affected by the detection logic
This ensures the new feature doesn't break existing functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
electron-builder.json (1)
150-150: Consider migrating tocore24base for longer-term support and compatibility.
core24is now the recommended Snap base for new and actively-maintained Electron applications (updated 2024–2025). It provides more recent system libraries, longer-term support, and improved GPU/kernel handling compared tocore22. However, migration requires updatingsnapcraft.yaml, tooling (Snapcraft 8.x+), and CI/build scripts (e.g.,architectures→platforms). Plan thorough testing across target platforms before switching, or maintain acore22release channel during the transition.
📜 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)
electron-builder.json
🧰 Additional context used
📓 Path-based instructions (1)
{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
⏰ 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 (ubuntu-latest, linux)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
🔇 Additional comments (1)
electron-builder.json (1)
151-151: Remove the inconsistency concern—theallowNativeWayland: falsesetting for Snap is intentional and documented.The documentation in
docs/linux-display-server.mdanddocs/linux-wayland-bug-postmortem.mdclearly shows that Snap uses a deliberate strategy different from deb/rpm/AppImage:
- deb/rpm/AppImage: Use wrapper script with runtime detection (matches PR objectives: "Native Wayland when available, safe X11 fallback")
- Snap: Force X11 via
allowNativeWayland: falsedue to Snap confinement constraintsThis is not an inconsistency—it's a documented multi-format strategy where Snap's confinement model requires a different approach than the runtime-detection wrapper used for other packages.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/main/app.main.spec.ts (1)
89-174: LGTM: Thorough manual override testing.This suite correctly validates that:
- Command-line
--ozone-platformflags prevent automatic detectionELECTRON_OZONE_PLATFORM_HINTenvironment variable acts as an override- Empty or whitespace-only values are properly ignored (triggering detection)
The assertions align with the implementation logic in
app.ts.Optional: Extract repeated filtering logic
The pattern for filtering
ozone-platformcalls appears multiple times. Consider extracting to a helper:+const getOzonePlatformCalls = (mock: jest.Mock) => + mock.mock.calls.filter((call) => call[0] === 'ozone-platform'); + describe('Manual Override Detection', () => { it('should respect --ozone-platform=x11 flag', () => { // ... setup ... performElectronStartup(); - const ozonePlatformCalls = appendSwitchMock.mock.calls.filter( - (call) => call[0] === 'ozone-platform' - ); + const ozonePlatformCalls = getOzonePlatformCalls(appendSwitchMock); expect(ozonePlatformCalls).toHaveLength(0); });This would reduce duplication across the test file.
📜 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.main.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.main.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Main-process tests must be named with the suffix
.main.spec.tsand run under the Electron runner
Files:
src/app/main/app.main.spec.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/app/main/app.main.spec.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.spec.ts : Main-process tests must be named with the suffix `.main.spec.ts` and run under the Electron runner
Applied to files:
src/app/main/app.main.spec.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).spec.ts : Renderer-process tests should be named `*.spec.ts` (without the `.main` qualifier)
Applied to files:
src/app/main/app.main.spec.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.main.spec.ts
🧬 Code graph analysis (1)
src/app/main/app.main.spec.ts (1)
src/app/main/app.ts (1)
performElectronStartup(85-246)
⏰ 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 (ubuntu-latest, linux)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: check (macos-latest)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
🔇 Additional comments (9)
src/app/main/app.main.spec.ts (9)
1-44: LGTM: Comprehensive mock setup.The mock configuration is well-structured, covering all necessary dependencies (
fs,electron,readSetting,rimraf,actions). Preserving original process state (platform, env, argv, console.log) ensures proper test isolation.
51-76: LGTM: Proper test isolation and cross-platform fix.The setup correctly:
- Resets mocks and sets sensible defaults (
statSyncMockreturns socket by default)- Clears relevant environment variables for each test
- Sets
XDG_RUNTIME_DIRto avoidprocess.getuid()on Windows (as noted in PR objectives)This ensures consistent, platform-independent test execution.
176-283: LGTM: Comprehensive Wayland detection coverage.This suite thoroughly tests Wayland detection logic, including:
- Success path: session type, display, and socket all valid → uses Wayland
- Failure paths with correct reason codes:
no-wayland-display: whenWAYLAND_DISPLAYis unset/empty/whitespacesocket-not-found: when socket file doesn't exist or isn't a socketThe mock behavior correctly simulates different filesystem states, and assertions verify both the platform selection and structured logging output.
285-332: LGTM: X11 detection scenarios covered.Tests correctly validate X11 detection with proper reason codes:
x11-session: whenXDG_SESSION_TYPE=x11no-session-type: when session type is unset or emptyAssertions verify both platform forcing and structured log output.
334-381: LGTM: Invalid session type handling.Tests correctly validate that unsupported session types (
tty,mir, or any unknown value) fall back to X11 with theinvalid-session-typereason code, preventing crashes on non-standard display server configurations.
383-441: LGTM: Whitespace normalization coverage.Tests verify that leading/trailing whitespace in
XDG_SESSION_TYPEandWAYLAND_DISPLAYis properly trimmed, ensuring robust parsing of environment variables that may contain accidental whitespace.
443-476: LGTM: Structured logging validation.Tests confirm that platform detection logs emit valid JSON with the expected fields (
sessionType,waylandDisplayfor Wayland;reason,sessionTypefor X11), facilitating observability and debugging.
478-511: LGTM: Platform-specific behavior correctly isolated to Linux.Tests verify that display server detection only executes on Linux (not Darwin/Win32), preventing unnecessary logic on platforms where X11/Wayland detection is not applicable.
513-546: LGTM: Integration with other command-line switches.Tests confirm that:
- PipeWire screen capture feature (
WebRTCPipeWireCapturer) is enabled alongside platform detection- Other switches like
autoplay-policyare not affectedThis ensures the detection logic doesn't introduce conflicts or regressions.
jeanfbrito
force-pushed
the
fix-x11-ubuntu2204
branch
from
33ce297 to
1e0b83e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/main/app.main.spec.ts (1)
250-284: Good socket validation tests.The tests cover both
ENOENTerrors and non-socket paths. Consider adding a test for whenXDG_RUNTIME_DIRis unset to verify the fallback to/run/user/${process.getuid()}.Optional: Add test for XDG_RUNTIME_DIR fallback
it('should use getuid fallback when XDG_RUNTIME_DIR is unset', () => { delete process.env.XDG_RUNTIME_DIR; process.env.XDG_SESSION_TYPE = 'wayland'; process.env.WAYLAND_DISPLAY = 'wayland-0'; performElectronStartup(); // Verify statSync was called with the fallback path expect(statSyncMock).toHaveBeenCalledWith('/run/user/1000/wayland-0'); });
📜 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)
CLAUDE.mdsrc/app/main/app.main.spec.ts
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.main.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Main-process tests must be named with the suffix
.main.spec.tsand run under the Electron runner
Files:
src/app/main/app.main.spec.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/app/main/app.main.spec.ts
🧠 Learnings (16)
📚 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.spec.ts : Main-process tests must be named with the suffix `.main.spec.ts` and run under the Electron runner
Applied to files:
src/app/main/app.main.spec.tsCLAUDE.md
📚 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).spec.ts : Renderer-process tests should be named `*.spec.ts` (without the `.main` qualifier)
Applied to files:
src/app/main/app.main.spec.tsCLAUDE.md
📚 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.main.spec.tsCLAUDE.md
📚 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: Never commit directly to master/main; use branches and PR review flow
Applied to files:
CLAUDE.md
📚 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: Use two-phase Windows code signing with Google Cloud KMS (build unsigned, then sign with jsign)
Applied to files:
CLAUDE.md
📚 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: Windows builds must include x64, ia32, and arm64 architectures
Applied to files:
CLAUDE.md
📚 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 docs/**/*.md : Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Applied to files:
CLAUDE.md
📚 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 docs/**/*.md : Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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: Before testing, run `yarn install`, `yarn lint`, and `yarn test`
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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 : For Fuselage theming, validate tokens against `Theme.d.ts` and only use documented values
Applied to files:
CLAUDE.md
📚 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: Do not perform git write operations (commit, merge, rebase, push) without explicit user request; use worktrees for agent work
Applied to files:
CLAUDE.md
🧬 Code graph analysis (1)
src/app/main/app.main.spec.ts (1)
src/app/main/app.ts (1)
performElectronStartup(85-246)
⏰ 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 (macos-latest, mac)
- GitHub Check: build (windows-latest, windows)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (macos-latest)
- GitHub Check: check (windows-latest)
🔇 Additional comments (11)
CLAUDE.md (2)
367-407: Well-documented cross-platform test compatibility guidance.This new section provides clear, actionable guidance for writing tests that mock Linux-specific code paths. The examples demonstrate proper setup/teardown patterns, and the explicit list of Linux-only APIs (
process.getuid(),process.getgid(), environment variables) will help prevent CI failures when tests run on Windows/macOS.The accompanying test file
app.main.spec.tscorrectly implements this pattern at lines 74-76 and 88.
31-35: LGTM!Good addition of explicit step 4 for PR review workflow, reinforcing the branch-based development process from learnings.
src/app/main/app.main.spec.ts (9)
52-77: Good test setup following cross-platform compatibility guidelines.The setup correctly mocks Linux-specific APIs (
process.getuid,XDG_RUNTIME_DIR) as documented in CLAUDE.md, enabling these tests to run on Windows/macOS CI. ThestatSyncMockdefault allows individual tests to override socket behavior as needed.
91-176: Comprehensive manual override detection tests.The test suite covers all expected command-line flags and environment variable scenarios, including edge cases for empty and whitespace-only values. This ensures the auto-detection logic correctly defers to user-specified overrides.
287-334: LGTM!X11 detection tests cover the expected scenarios with proper reason code validation.
336-383: LGTM!Good coverage of edge cases like
ttyandmirsessions, ensuring safe X11 fallback.
385-443: LGTM!Good coverage of whitespace normalization for environment variables.
445-478: LGTM!Log structure validation ensures consistent output format for debugging.
480-513: LGTM!Platform isolation tests correctly verify that ozone-platform detection only runs on Linux.
515-548: LGTM!Good integration tests verifying that platform detection works alongside other Electron configuration switches like PipeWire and autoplay policy.
28-32: The rimraf mock structure is correct and matches the actual import and usage pattern. The code imports rimraf as a named import (import { rimraf } from 'rimraf') and uses it asrimraf.sync(), which aligns perfectly with the mock structure{ rimraf: { sync: jest.fn() } }. No changes are needed.
Use optional chaining with fallback for process.getuid() instead of mocking in tests. This is the pattern used by VSCode for cross-platform compatibility. Changes: - app.ts: process.getuid() → process.getuid?.() ?? 1000 - Remove process.getuid mocking from tests (no longer needed) - Update CLAUDE.md with VSCode pattern and source attribution
jeanfbrito
force-pushed
the
fix-x11-ubuntu2204
branch
from
1e0b83e to
b838127
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CLAUDE.md (1)
367-404: Cross-Platform Test Compatibility section is timely and relevant.This new section on defensive coding for Linux-only APIs is well-timed given the PR's Wayland/X11 socket detection work. The pattern guidance (optional chaining
?.with nullish coalescing??fallbacks) is sound and well-exemplified. However, fix the markdown formatting issues below.Lines 371 and 385 use bold (**) to mark pseudo-headings instead of proper markdown heading syntax (###). Markdownlint flags these as MD036 violations:
🔧 Proposed markdown formatting fixes
- **Primary Pattern: Defensive Coding (VSCode approach)** + ### Primary Pattern: Defensive Coding (VSCode approach) - **Fallback Pattern: Mocking (only when defensive coding isn't possible)** + ### Fallback Pattern: Mocking (only when defensive coding isn't possible)This ensures proper heading hierarchy for documentation processing tools and readers.
src/app/main/app.main.spec.ts (1)
72-72: Consider adding test coverage for the XDG_RUNTIME_DIR fallback.All tests explicitly set
XDG_RUNTIME_DIR = '/run/user/1000'in beforeEach. According to the implementation in app.ts, whenXDG_RUNTIME_DIRis unset, the code falls back to/run/user/${process.getuid()}. Adding a test that deletesXDG_RUNTIME_DIRand verifies the uid-based fallback path would improve coverage.📋 Example test case for uid fallback
Add this test within the "Wayland Detection" describe block:
it('should use uid-based runtime directory when XDG_RUNTIME_DIR is unset', () => { delete process.env.XDG_RUNTIME_DIR; process.env.XDG_SESSION_TYPE = 'wayland'; process.env.WAYLAND_DISPLAY = 'wayland-0'; performElectronStartup(); // Verify statSync was called with /run/user/<uid>/wayland-0 expect(statSyncMock).toHaveBeenCalledWith( expect.stringMatching(/^\/run\/user\/\d+\/wayland-0$/) ); });
📜 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)
CLAUDE.mdsrc/app/main/app.main.spec.tssrc/app/main/app.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/main/app.ts
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.main.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Main-process tests must be named with the suffix
.main.spec.tsand run under the Electron runner
Files:
src/app/main/app.main.spec.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/app/main/app.main.spec.ts
🧠 Learnings (16)
📚 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.spec.ts : Main-process tests must be named with the suffix `.main.spec.ts` and run under the Electron runner
Applied to files:
src/app/main/app.main.spec.tsCLAUDE.md
📚 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).spec.ts : Renderer-process tests should be named `*.spec.ts` (without the `.main` qualifier)
Applied to files:
src/app/main/app.main.spec.tsCLAUDE.md
📚 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.main.spec.tsCLAUDE.md
📚 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: Never commit directly to master/main; use branches and PR review flow
Applied to files:
CLAUDE.md
📚 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: Use two-phase Windows code signing with Google Cloud KMS (build unsigned, then sign with jsign)
Applied to files:
CLAUDE.md
📚 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: Windows builds must include x64, ia32, and arm64 architectures
Applied to files:
CLAUDE.md
📚 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 docs/**/*.md : Never invent or estimate metrics/time; only include verifiable numbers in documentation and reports
Applied to files:
CLAUDE.md
📚 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 docs/**/*.md : Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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: Before testing, run `yarn install`, `yarn lint`, and `yarn test`
Applied to files:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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:
CLAUDE.md
📚 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 : For Fuselage theming, validate tokens against `Theme.d.ts` and only use documented values
Applied to files:
CLAUDE.md
📚 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: Do not perform git write operations (commit, merge, rebase, push) without explicit user request; use worktrees for agent work
Applied to files:
CLAUDE.md
🧬 Code graph analysis (1)
src/app/main/app.main.spec.ts (1)
src/app/main/app.ts (1)
performElectronStartup(85-247)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
371-371: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
385-385: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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 (ubuntu-latest)
- GitHub Check: check (windows-latest)
🔇 Additional comments (4)
CLAUDE.md (3)
31-35: Process clarity improved with review step.Adding "Wait for user review and approval" to the committing workflow reinforces the collaborative PR process already documented in learnings. This is a good addition.
75-145: Comprehensive metrics guidance aligns with learnings.The extensive "Never Invent Metrics" section, examples, and "Verifiable Information" patterns directly implement the learning about not inventing estimates or metrics in documentation. The ✅/❌ examples make the distinction very clear for future authors.
150-320: Post-mortem structure well-organized and actionable.The 14-point post-mortem guidance with examples provides a strong framework aligned with the learning about structured technical retrospectives. The top-level "The Solution That Actually Worked" section and emphasis on verifiable metrics (rather than speculated impact) are particularly strong.
src/app/main/app.main.spec.ts (1)
1-544: Excellent test coverage and structure!The test suite is comprehensive, well-organized, and properly isolates each scenario. It covers manual overrides, Wayland/X11 detection logic, environment variable normalization, socket checks, logging structure, platform guards, and integration with other switches. The use of mocks is appropriate, and test isolation via beforeEach/afterEach is correctly implemented.
- Fix timeline dates in post-mortem from 2025 to 2026 - Remove incorrect mock of ../actions with electronBuilderJsonInformation (electronBuilderJsonInformation is exported from app.ts, not ../actions)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @docs/linux-wayland-bug-postmortem.md:
- Line 368: The post-mortem completion date is inconsistent (shows "2025-01-08")
— update the completion date string "_Post-mortem completed: 2025-01-08_" to the
correct year "2026-01-08" so it matches the PR creation and timeline entries,
and verify there are no other stray 2025 dates in the document (timeline
entries, PR references, or issue metadata) and correct them to 2026 where
appropriate.
🧹 Nitpick comments (1)
docs/linux-wayland-bug-postmortem.md (1)
361-364: Consider formatting URLs as proper markdown links.The bare URLs are functional but markdown link format improves readability and follows markdown best practices.
🔗 Proposed formatting
-- Chromium Ozone Platform: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/ozone_overview.md -- Electron Command Line Switches: https://www.electronjs.org/docs/latest/api/command-line-switches -- Wayland Protocol: https://wayland.freedesktop.org/ -- XDG Base Directory Specification: https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html +- [Chromium Ozone Platform](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/ozone_overview.md) +- [Electron Command Line Switches](https://www.electronjs.org/docs/latest/api/command-line-switches) +- [Wayland Protocol](https://wayland.freedesktop.org/) +- [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html)
📜 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)
docs/linux-wayland-bug-postmortem.mdsrc/app/main/app.main.spec.ts
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.main.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Main-process tests must be named with the suffix
.main.spec.tsand run under the Electron runner
Files:
src/app/main/app.main.spec.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/app/main/app.main.spec.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-wayland-bug-postmortem.md
🧠 Learnings (4)
📚 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.spec.ts : Main-process tests must be named with the suffix `.main.spec.ts` and run under the Electron runner
Applied to files:
src/app/main/app.main.spec.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).spec.ts : Renderer-process tests should be named `*.spec.ts` (without the `.main` qualifier)
Applied to files:
src/app/main/app.main.spec.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.main.spec.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 docs/**/*.md : Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Applied to files:
docs/linux-wayland-bug-postmortem.md
🧬 Code graph analysis (1)
src/app/main/app.main.spec.ts (1)
src/app/main/app.ts (1)
performElectronStartup(85-247)
🪛 markdownlint-cli2 (0.18.1)
docs/linux-wayland-bug-postmortem.md
52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
177-177: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
209-209: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
361-361: Bare URL used
(MD034, no-bare-urls)
362-362: Bare URL used
(MD034, no-bare-urls)
363-363: Bare URL used
(MD034, no-bare-urls)
364-364: Bare URL used
(MD034, no-bare-urls)
⏰ 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 (ubuntu-latest)
- GitHub Check: check (windows-latest)
🔇 Additional comments (2)
docs/linux-wayland-bug-postmortem.md (1)
16-37: Excellent structure following documentation guidelines.The "The Solution That Actually Worked" section is well-structured with Problem, Solution, Result, and PR link as required by the coding guidelines. This makes the post-mortem immediately actionable for readers.
Based on coding guidelines, technical post-mortems should include this exact structure.
src/app/main/app.main.spec.ts (1)
1-537: Excellent comprehensive test coverage!This test suite exemplifies thorough testing practices:
- Complete edge case coverage: Empty strings, whitespace, missing variables, invalid values
- Proper test isolation: BeforeEach/afterEach restore all modified global state (process.platform, process.env, process.argv, console.log)
- JSON logging validation: Ensures structured logging format is maintained
- Platform-specific guards: Verifies Linux-only behavior doesn't affect darwin/win32
- Integration testing: Confirms Wayland/X11 detection doesn't interfere with other switches (autoplay-policy, PipeWire)
The test structure aligns perfectly with the implementation described in the relevant code snippets and follows the
.main.spec.tsnaming convention required by coding guidelines.Based on coding guidelines, main-process tests must use the
.main.spec.tssuffix and this file correctly follows that pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/linux-wayland-bug-postmortem.md (2)
52-64: Add language specifiers to fenced code blocks.Code blocks should specify their language for proper syntax highlighting and linting compliance.
🔧 Proposed fixes for code block language specifiers
- ``` + ```text ERROR:ui/ozone/platform/wayland/host/wayland_connection.cc:202 Failed to connect to Wayland display: No such file or directory (2) ERROR:ui/ozone/platform/wayland/ozone_platform_wayland.cc:282 Failed to initialize Wayland platform ERROR:ui/aura/env.cc:257 The platform failed to initialize. Exiting. Segmentation fault (core dumped) - ``` + ```For line 177 (process timeline):
- ``` + ```text Process Startup Timeline: [Binary Start] ... - ``` + ```For line 209 (wrapper timeline):
- ``` + ```text Process Startup Timeline with Wrapper: [Wrapper Script Start] ... - ``` + ```Also applies to: 177-199, 209-234
361-364: Wrap bare URLs in markdown link syntax.Bare URLs should be formatted as markdown links for consistency and markdown linting compliance.
🔗 Proposed fixes for bare URLs
- Chromium Ozone Platform: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/ozone_overview.md - Electron Command Line Switches: https://www.electronjs.org/docs/latest/api/command-line-switches - Wayland Protocol: https://wayland.freedesktop.org/ - XDG Base Directory Specification: https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html + - [Chromium Ozone Platform](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/ozone_overview.md) + - [Electron Command Line Switches](https://www.electronjs.org/docs/latest/api/command-line-switches) + - [Wayland Protocol](https://wayland.freedesktop.org/) + - [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html)
📜 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)
docs/linux-wayland-bug-postmortem.md
🧰 Additional context used
📓 Path-based instructions (1)
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-wayland-bug-postmortem.md
🧠 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 docs/**/*.md : Structure technical post-mortems with a top section “The Solution That Actually Worked” including Problem, Solution, Result, and PR
Applied to files:
docs/linux-wayland-bug-postmortem.md
🪛 markdownlint-cli2 (0.18.1)
docs/linux-wayland-bug-postmortem.md
52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
177-177: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
209-209: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
361-361: Bare URL used
(MD034, no-bare-urls)
362-362: Bare URL used
(MD034, no-bare-urls)
363-363: Bare URL used
(MD034, no-bare-urls)
364-364: Bare URL used
(MD034, no-bare-urls)
⏰ 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 (ubuntu-latest)
- GitHub Check: check (windows-latest)
🔇 Additional comments (1)
docs/linux-wayland-bug-postmortem.md (1)
16-36: Excellent adherence to post-mortem documentation guidelines.The document correctly implements the structured post-mortem format with a prominent "The Solution That Actually Worked" section containing Problem, Solution, Result, and PR subsections. All presented metrics and timelines are verifiable and tied to actual testing outcomes rather than estimates or invented numbers. The comprehensive test matrices, validation environments, and technical analysis provide strong evidence for the solution's effectiveness across multiple configurations.
jeanfbrito
changed the title
| Example structure: | ||
| ```markdown | ||
|
|
||
| ````markdown |
There was a problem hiding this comment.
Never fully trust AI, it will mess up your code at some point. This is not correct markdown syntax.
There was a problem hiding this comment.
Good catch, I really dont care much about the md file the AI messes by itself.
* feat: Enhance Linux display server detection and fallback logic - Updated the Linux display server configuration to automatically detect Wayland sessions and fallback to X11 if necessary, preventing crashes during initialization. - Improved documentation to clarify the automatic detection process and added troubleshooting information for Ubuntu 22.04 LTS users experiencing segfaults. - Introduced unit tests for the platform detection logic to ensure correct behavior across various session types and manual overrides. * test: Update logging tests for platform detection - Refactored logging tests in `app.main.spec.ts` to validate JSON structure of log messages for Wayland and X11 sessions. - Removed outdated tests for session type handling and improved assertions for log data properties. - Enhanced clarity in test descriptions to reflect the updated logging behavior during Electron startup. * refactor(tests): Remove redundant JSON parsing checks in platform detection tests - Eliminated unnecessary assertions for JSON parsing in the logging tests for Wayland and X11 sessions in `app.main.spec.ts`. - Streamlined test cases to focus on validating the structure of log data without redundant error handling, enhancing clarity and maintainability. * fix(linux): add wrapper script to prevent Wayland/X11 crashes (RocketChat#3154) The app.commandLine.appendSwitch() approach doesn't work because Chromium initializes before Electron JavaScript runs. Fix by package type: - deb/rpm/tar.gz: Wrapper script runs BEFORE binary, detects display server - Snap: Set allowNativeWayland=false to force X11 via electron-builder - Flatpak/AppImage: Use electron-builder launcher with X11 fallback Wrapper script detection logic: - If XDG_SESSION_TYPE != wayland: force X11 - If WAYLAND_DISPLAY is empty: force X11 - If Wayland socket doesn't exist: force X11 - Otherwise: use native Wayland Validated on: - Fedora 42 physical (GTX 1660 Ti) - Wayland native - Ubuntu 22.04 physical (GTX 1660 Ti) - X11 session - Fedora 42 VM (no GPU) - Ubuntu 22.04 VM (no GPU) All 5 test scenarios pass with fix vs SEGFAULT without. Closes RocketChat#3154 * docs: add Fedora 42 test results to postmortem * fix: add Wayland socket existence check and address review feedback - Add fs.statSync check to verify Wayland socket actually exists - Add 'socket-not-found' reason when socket is missing - Remove placeholder dates from timeline in postmortem - Add tests for socket existence checking * fix: address review feedback - Add tests for ELECTRON_OZONE_PLATFORM_HINT env var override - Fix Snap docs: Auto-detect → Force X11 (allowNativeWayland: false) - Fix date: January 2025 → January 2026 - Add 'The Solution That Actually Worked' section to postmortem * chore: bump version to 4.11.1 in package.json * chore: update Snap configuration to use core22 base * fix: use defensive coding for Linux-only APIs (VSCode pattern) Use optional chaining with fallback for process.getuid() instead of mocking in tests. This is the pattern used by VSCode for cross-platform compatibility. Changes: - app.ts: process.getuid() → process.getuid?.() ?? 1000 - Remove process.getuid mocking from tests (no longer needed) - Update CLAUDE.md with VSCode pattern and source attribution * fix: address CodeRabbit review comments - Fix timeline dates in post-mortem from 2025 to 2026 - Remove incorrect mock of ../actions with electronBuilderJsonInformation (electronBuilderJsonInformation is exported from app.ts, not ../actions) * fix: correct post-mortem completion date from 2025 to 2026
* feat: Enhance Linux display server detection and fallback logic - Updated the Linux display server configuration to automatically detect Wayland sessions and fallback to X11 if necessary, preventing crashes during initialization. - Improved documentation to clarify the automatic detection process and added troubleshooting information for Ubuntu 22.04 LTS users experiencing segfaults. - Introduced unit tests for the platform detection logic to ensure correct behavior across various session types and manual overrides. * test: Update logging tests for platform detection - Refactored logging tests in `app.main.spec.ts` to validate JSON structure of log messages for Wayland and X11 sessions. - Removed outdated tests for session type handling and improved assertions for log data properties. - Enhanced clarity in test descriptions to reflect the updated logging behavior during Electron startup. * refactor(tests): Remove redundant JSON parsing checks in platform detection tests - Eliminated unnecessary assertions for JSON parsing in the logging tests for Wayland and X11 sessions in `app.main.spec.ts`. - Streamlined test cases to focus on validating the structure of log data without redundant error handling, enhancing clarity and maintainability. * fix(linux): add wrapper script to prevent Wayland/X11 crashes (RocketChat#3154) The app.commandLine.appendSwitch() approach doesn't work because Chromium initializes before Electron JavaScript runs. Fix by package type: - deb/rpm/tar.gz: Wrapper script runs BEFORE binary, detects display server - Snap: Set allowNativeWayland=false to force X11 via electron-builder - Flatpak/AppImage: Use electron-builder launcher with X11 fallback Wrapper script detection logic: - If XDG_SESSION_TYPE != wayland: force X11 - If WAYLAND_DISPLAY is empty: force X11 - If Wayland socket doesn't exist: force X11 - Otherwise: use native Wayland Validated on: - Fedora 42 physical (GTX 1660 Ti) - Wayland native - Ubuntu 22.04 physical (GTX 1660 Ti) - X11 session - Fedora 42 VM (no GPU) - Ubuntu 22.04 VM (no GPU) All 5 test scenarios pass with fix vs SEGFAULT without. Closes RocketChat#3154 * docs: add Fedora 42 test results to postmortem * fix: add Wayland socket existence check and address review feedback - Add fs.statSync check to verify Wayland socket actually exists - Add 'socket-not-found' reason when socket is missing - Remove placeholder dates from timeline in postmortem - Add tests for socket existence checking * fix: address review feedback - Add tests for ELECTRON_OZONE_PLATFORM_HINT env var override - Fix Snap docs: Auto-detect → Force X11 (allowNativeWayland: false) - Fix date: January 2025 → January 2026 - Add 'The Solution That Actually Worked' section to postmortem * chore: bump version to 4.11.1 in package.json * chore: update Snap configuration to use core22 base * fix: use defensive coding for Linux-only APIs (VSCode pattern) Use optional chaining with fallback for process.getuid() instead of mocking in tests. This is the pattern used by VSCode for cross-platform compatibility. Changes: - app.ts: process.getuid() → process.getuid?.() ?? 1000 - Remove process.getuid mocking from tests (no longer needed) - Update CLAUDE.md with VSCode pattern and source attribution * fix: address CodeRabbit review comments - Fix timeline dates in post-mortem from 2025 to 2026 - Remove incorrect mock of ../actions with electronBuilderJsonInformation (electronBuilderJsonInformation is exported from app.ts, not ../actions) * fix: correct post-mortem completion date from 2025 to 2026
* feat: Enhance Linux display server detection and fallback logic - Updated the Linux display server configuration to automatically detect Wayland sessions and fallback to X11 if necessary, preventing crashes during initialization. - Improved documentation to clarify the automatic detection process and added troubleshooting information for Ubuntu 22.04 LTS users experiencing segfaults. - Introduced unit tests for the platform detection logic to ensure correct behavior across various session types and manual overrides. * test: Update logging tests for platform detection - Refactored logging tests in `app.main.spec.ts` to validate JSON structure of log messages for Wayland and X11 sessions. - Removed outdated tests for session type handling and improved assertions for log data properties. - Enhanced clarity in test descriptions to reflect the updated logging behavior during Electron startup. * refactor(tests): Remove redundant JSON parsing checks in platform detection tests - Eliminated unnecessary assertions for JSON parsing in the logging tests for Wayland and X11 sessions in `app.main.spec.ts`. - Streamlined test cases to focus on validating the structure of log data without redundant error handling, enhancing clarity and maintainability. * fix(linux): add wrapper script to prevent Wayland/X11 crashes (RocketChat#3154) The app.commandLine.appendSwitch() approach doesn't work because Chromium initializes before Electron JavaScript runs. Fix by package type: - deb/rpm/tar.gz: Wrapper script runs BEFORE binary, detects display server - Snap: Set allowNativeWayland=false to force X11 via electron-builder - Flatpak/AppImage: Use electron-builder launcher with X11 fallback Wrapper script detection logic: - If XDG_SESSION_TYPE != wayland: force X11 - If WAYLAND_DISPLAY is empty: force X11 - If Wayland socket doesn't exist: force X11 - Otherwise: use native Wayland Validated on: - Fedora 42 physical (GTX 1660 Ti) - Wayland native - Ubuntu 22.04 physical (GTX 1660 Ti) - X11 session - Fedora 42 VM (no GPU) - Ubuntu 22.04 VM (no GPU) All 5 test scenarios pass with fix vs SEGFAULT without. Closes RocketChat#3154 * docs: add Fedora 42 test results to postmortem * fix: add Wayland socket existence check and address review feedback - Add fs.statSync check to verify Wayland socket actually exists - Add 'socket-not-found' reason when socket is missing - Remove placeholder dates from timeline in postmortem - Add tests for socket existence checking * fix: address review feedback - Add tests for ELECTRON_OZONE_PLATFORM_HINT env var override - Fix Snap docs: Auto-detect → Force X11 (allowNativeWayland: false) - Fix date: January 2025 → January 2026 - Add 'The Solution That Actually Worked' section to postmortem * chore: bump version to 4.11.1 in package.json * chore: update Snap configuration to use core22 base * fix: use defensive coding for Linux-only APIs (VSCode pattern) Use optional chaining with fallback for process.getuid() instead of mocking in tests. This is the pattern used by VSCode for cross-platform compatibility. Changes: - app.ts: process.getuid() → process.getuid?.() ?? 1000 - Remove process.getuid mocking from tests (no longer needed) - Update CLAUDE.md with VSCode pattern and source attribution * fix: address CodeRabbit review comments - Fix timeline dates in post-mortem from 2025 to 2026 - Remove incorrect mock of ../actions with electronBuilderJsonInformation (electronBuilderJsonInformation is exported from app.ts, not ../actions) * fix: correct post-mortem completion date from 2025 to 2026
* feat: Enhance Linux display server detection and fallback logic - Updated the Linux display server configuration to automatically detect Wayland sessions and fallback to X11 if necessary, preventing crashes during initialization. - Improved documentation to clarify the automatic detection process and added troubleshooting information for Ubuntu 22.04 LTS users experiencing segfaults. - Introduced unit tests for the platform detection logic to ensure correct behavior across various session types and manual overrides. * test: Update logging tests for platform detection - Refactored logging tests in `app.main.spec.ts` to validate JSON structure of log messages for Wayland and X11 sessions. - Removed outdated tests for session type handling and improved assertions for log data properties. - Enhanced clarity in test descriptions to reflect the updated logging behavior during Electron startup. * refactor(tests): Remove redundant JSON parsing checks in platform detection tests - Eliminated unnecessary assertions for JSON parsing in the logging tests for Wayland and X11 sessions in `app.main.spec.ts`. - Streamlined test cases to focus on validating the structure of log data without redundant error handling, enhancing clarity and maintainability. * fix(linux): add wrapper script to prevent Wayland/X11 crashes (RocketChat#3154) The app.commandLine.appendSwitch() approach doesn't work because Chromium initializes before Electron JavaScript runs. Fix by package type: - deb/rpm/tar.gz: Wrapper script runs BEFORE binary, detects display server - Snap: Set allowNativeWayland=false to force X11 via electron-builder - Flatpak/AppImage: Use electron-builder launcher with X11 fallback Wrapper script detection logic: - If XDG_SESSION_TYPE != wayland: force X11 - If WAYLAND_DISPLAY is empty: force X11 - If Wayland socket doesn't exist: force X11 - Otherwise: use native Wayland Validated on: - Fedora 42 physical (GTX 1660 Ti) - Wayland native - Ubuntu 22.04 physical (GTX 1660 Ti) - X11 session - Fedora 42 VM (no GPU) - Ubuntu 22.04 VM (no GPU) All 5 test scenarios pass with fix vs SEGFAULT without. Closes RocketChat#3154 * docs: add Fedora 42 test results to postmortem * fix: add Wayland socket existence check and address review feedback - Add fs.statSync check to verify Wayland socket actually exists - Add 'socket-not-found' reason when socket is missing - Remove placeholder dates from timeline in postmortem - Add tests for socket existence checking * fix: address review feedback - Add tests for ELECTRON_OZONE_PLATFORM_HINT env var override - Fix Snap docs: Auto-detect → Force X11 (allowNativeWayland: false) - Fix date: January 2025 → January 2026 - Add 'The Solution That Actually Worked' section to postmortem * chore: bump version to 4.11.1 in package.json * chore: update Snap configuration to use core22 base * fix: use defensive coding for Linux-only APIs (VSCode pattern) Use optional chaining with fallback for process.getuid() instead of mocking in tests. This is the pattern used by VSCode for cross-platform compatibility. Changes: - app.ts: process.getuid() → process.getuid?.() ?? 1000 - Remove process.getuid mocking from tests (no longer needed) - Update CLAUDE.md with VSCode pattern and source attribution * fix: address CodeRabbit review comments - Fix timeline dates in post-mortem from 2025 to 2026 - Remove incorrect mock of ../actions with electronBuilderJsonInformation (electronBuilderJsonInformation is exported from app.ts, not ../actions) * fix: correct post-mortem completion date from 2025 to 2026
* feat: Enhance Linux display server detection and fallback logic - Updated the Linux display server configuration to automatically detect Wayland sessions and fallback to X11 if necessary, preventing crashes during initialization. - Improved documentation to clarify the automatic detection process and added troubleshooting information for Ubuntu 22.04 LTS users experiencing segfaults. - Introduced unit tests for the platform detection logic to ensure correct behavior across various session types and manual overrides. * test: Update logging tests for platform detection - Refactored logging tests in `app.main.spec.ts` to validate JSON structure of log messages for Wayland and X11 sessions. - Removed outdated tests for session type handling and improved assertions for log data properties. - Enhanced clarity in test descriptions to reflect the updated logging behavior during Electron startup. * refactor(tests): Remove redundant JSON parsing checks in platform detection tests - Eliminated unnecessary assertions for JSON parsing in the logging tests for Wayland and X11 sessions in `app.main.spec.ts`. - Streamlined test cases to focus on validating the structure of log data without redundant error handling, enhancing clarity and maintainability. * fix(linux): add wrapper script to prevent Wayland/X11 crashes (RocketChat#3154) The app.commandLine.appendSwitch() approach doesn't work because Chromium initializes before Electron JavaScript runs. Fix by package type: - deb/rpm/tar.gz: Wrapper script runs BEFORE binary, detects display server - Snap: Set allowNativeWayland=false to force X11 via electron-builder - Flatpak/AppImage: Use electron-builder launcher with X11 fallback Wrapper script detection logic: - If XDG_SESSION_TYPE != wayland: force X11 - If WAYLAND_DISPLAY is empty: force X11 - If Wayland socket doesn't exist: force X11 - Otherwise: use native Wayland Validated on: - Fedora 42 physical (GTX 1660 Ti) - Wayland native - Ubuntu 22.04 physical (GTX 1660 Ti) - X11 session - Fedora 42 VM (no GPU) - Ubuntu 22.04 VM (no GPU) All 5 test scenarios pass with fix vs SEGFAULT without. Closes RocketChat#3154 * docs: add Fedora 42 test results to postmortem * fix: add Wayland socket existence check and address review feedback - Add fs.statSync check to verify Wayland socket actually exists - Add 'socket-not-found' reason when socket is missing - Remove placeholder dates from timeline in postmortem - Add tests for socket existence checking * fix: address review feedback - Add tests for ELECTRON_OZONE_PLATFORM_HINT env var override - Fix Snap docs: Auto-detect → Force X11 (allowNativeWayland: false) - Fix date: January 2025 → January 2026 - Add 'The Solution That Actually Worked' section to postmortem * chore: bump version to 4.11.1 in package.json * chore: update Snap configuration to use core22 base * fix: use defensive coding for Linux-only APIs (VSCode pattern) Use optional chaining with fallback for process.getuid() instead of mocking in tests. This is the pattern used by VSCode for cross-platform compatibility. Changes: - app.ts: process.getuid() → process.getuid?.() ?? 1000 - Remove process.getuid mocking from tests (no longer needed) - Update CLAUDE.md with VSCode pattern and source attribution * fix: address CodeRabbit review comments - Fix timeline dates in post-mortem from 2025 to 2026 - Remove incorrect mock of ../actions with electronBuilderJsonInformation (electronBuilderJsonInformation is exported from app.ts, not ../actions) * fix: correct post-mortem completion date from 2025 to 2026
2 participants
Summary
Resolves segmentation faults on Linux when environment variables indicate a Wayland session but no valid Wayland compositor is available. The fix uses a shell wrapper script that detects the display server before Chromium initializes, since
app.commandLine.appendSwitch()executes too late in the startup sequence.Root Cause Analysis
The Problem
Users on Ubuntu 22.04 LTS and other Linux distributions experienced crashes (SEGFAULT, exit code 139) when launching the application. The error manifested as:
Five Whys
Why did the app crash with SEGFAULT?
→ Chromium's Ozone layer attempted to connect to a Wayland display that didn't exist
Why did Chromium try to connect to non-existent Wayland?
→ Environment variable
WAYLAND_DISPLAYwas set (e.g.,wayland-0) but the socket didn't existWhy didn't the initial JavaScript fix work?
→
app.commandLine.appendSwitch('ozone-platform', 'x11')executes after Chromium has already initialized its display platformWhy does Chromium initialize before Electron JavaScript?
→ Chromium's platform initialization is part of the native startup sequence, before V8 and Electron's JavaScript environment are ready
Why were environment variables incorrect?
→ Various scenarios: SSH sessions inheriting desktop vars, systemd user sessions, misconfigured display managers, or stale environment from previous sessions
Root Cause
Chromium's Ozone platform layer initializes during native binary startup, before any Electron JavaScript executes. When
WAYLAND_DISPLAYis set, Chromium attempts Wayland connection and crashes on failure instead of falling back to X11.The Fix
Approach
Since JavaScript runs too late, the solution requires intervention before the binary starts. A shell wrapper script:
--ozone-platform=x11flag when Wayland isn't viableThis pattern is used by other Electron apps facing similar issues.
Implementation
1. Wrapper Script (
build/linux/wrapper.sh)Detection logic:
XDG_SESSION_TYPE≠wayland→ force X11WAYLAND_DISPLAYis empty → force X112. Build Integration (
build/afterPack.js:5-30)The
afterPackhook renames the binary and installs the wrapper:rocketchat-desktop→rocketchat-desktop.binrocketchat-desktop3. Snap Configuration (
electron-builder.json:150-151)Snap uses
allowNativeWayland: falseto force X11 mode, avoiding the detection complexity within the confined Snap environment.4. Runtime Detection (
src/app/main/app.ts:164-246)JavaScript-side detection provides:
--ozone-platform=*orELECTRON_OZONE_PLATFORM_HINT)Files Changed
build/linux/wrapper.shbuild/afterPack.jselectron-builder.jsoncore22base,allowNativeWayland: falsesrc/app/main/app.tssrc/app/main/app.main.spec.tsdocs/linux-wayland-bug-postmortem.mddocs/linux-display-server.mdVerification
Reproduction Steps (Before Fix)
Verification Steps (After Fix)
Test Matrix Results
Considerations
What This Doesn't Fix
Package-Specific Behavior
allowNativeWayland: false(forces X11)Manual Override
Users can override detection:
Cross-Platform Test Compatibility
Tests use defensive coding pattern (VSCode approach) for Linux-only APIs:
Architecture
References
docs/linux-wayland-bug-postmortem.mdSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.