add Outlook Sync debbuging logs

[jeanfbrito]

Introducing the Log Viewer

To open it, go in the menu Help -> Open Log Viewer

The logs for Outlook syncronization debug will be filtered by selecting it on the context box:

Buttons

Open Log File will make it possible to open a pre-saved log, so you can visualize a log of another machine or user if needed.
Refresh will update the logs on the click.
Auto Refresh will enable to the logs be auto updated as it are created, it is not enabled by default to be heavy to load the window for the first time.
Copy will copy all the text shown in the list to the clipboard.
Save will create a zip file with the text shown in the list.
Clear will clear the log file totally. There is no rollback for this.
Close will close the window.

CORE-1280

Summary by CodeRabbit

• New Features

  • Added Log Viewer window accessible from Help menu for inspecting application logs with search and filtering capabilities
  • Added system-wide theme appearance preference selector (auto/light/dark) in settings
  • Enhanced Outlook Calendar integration with improved error reporting and NTLM authentication support

• Chores

  • Version bump to 4.12.0-alpha.2
  • Added Windows arm64 architecture support
  • Dependency updates including Electron 40.0.0, Node.js 24.11.1

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/outlookCalendar/ipc.ts (1)

1008-1017: Incorrect double-wrapped Promise return type.

Promise<Promise<boolean>> is likely unintentional — async functions already return a Promise, so this declares a Promise that resolves to another Promise. This could cause issues if callers don't double-await.

🐛 Proposed fix

   handle(
     'outlook-calendar/has-credentials',
-    async (event): Promise<Promise<boolean>> => {
+    async (event): Promise<boolean> => {
       const server = getServerInformationByWebContentsId(event.id);
       if (!server) return false;

🤖 Fix all issues with AI agents

In @.sisyphus/boulder.json:
- Around line 2-6: The file currently commits user-specific data: the
"active_plan" value is an absolute path exposing a local username and
"session_ids" contains per-session identifiers; update the file to remove or
neutralize those values by replacing "active_plan" with a repository-relative or
templated path (e.g., a relative path or a placeholder like
"plans/log-viewer-professional-review.md") and remove or blank out "session_ids"
(or move session runtime data to a local-only file and add it to .gitignore);
ensure any code that reads "active_plan" or "session_ids" can handle the
template/empty value or fallback to generating them at runtime (check the reader
that references active_plan/session_ids).

In @.sisyphus/plans/logging-system-professional-upgrade.md:
- Line 788: The line that currently uses bold emphasis "**Total: 9 tasks across
3 waves**" should be converted to a markdown heading to satisfy markdownlint
MD036; locate the exact text "Total: 9 tasks across 3 waves" and replace the
bold markup with an appropriate heading level (for example prefix with "## " or
"### " as consistent with surrounding headings) so the line becomes a proper
heading instead of bold text.
- Around line 118-133: The fenced code block that begins with the "Wave 1
(Critical - Privacy & Memory):" list is missing a language identifier which
triggers markdownlint MD040; add a language tag (e.g., text) to the opening
triple-backtick so the block becomes ```text to silence MD040 and preserve
formatting of the Wave/Task list.

In `@src/logging/cleanup.ts`:
- Around line 12-17: Validate and clamp the retentionDays parameter at the start
of cleanupOldLogs to prevent negative or non-finite values from creating a
future cutoffTime; if retentionDays is NaN, infinite, or <= 0, fall back to
DEFAULT_RETENTION_DAYS (or clamp to a sensible minimum) before computing
cutoffTime, so the cutoffTime calculation and subsequent deletions using
app.getPath('logs') are safe.

In `@src/logging/context.ts`:
- Around line 86-95: The fallback to MAX_SERVER_ID when
availableServerIds.shift() is undefined can produce duplicate IDs; change the
allocation in the webview branch so that when availableServerIds is empty you
generate a unique serverId instead of using MAX_SERVER_ID (e.g.,
maintain/increment a lastAllocatedServerId counter or compute
Math.max(...usedServerIds values)+1), then set usedServerIds.set(webContentsId,
serverId) and continue building serverName and serverContextMap as before
(update any initialization for lastAllocatedServerId near the top of the
module). Ensure the logic references webContents.getType(), availableServerIds,
usedServerIds, webContentsId, serverContextMap, and MAX_SERVER_ID.

In `@src/logging/index.ts`:
- Around line 252-254: The injected renderer script currently calls
logLoggingFailure(error, ...) which is undefined in that context; instead,
update the injection inside setupWebContentsLogging (the "console override
injection" block) to send the error to the main process (for example via
window.postMessage or window.electron.ipcRenderer.invoke) and have the main
process handler call logLoggingFailure; ensure you add a fallback to
console.error in the injected script so a ReferenceError cannot be thrown if IPC
is unavailable, and wire a corresponding IPC/message listener in the main
process that invokes logLoggingFailure with the received error details.

In `@src/logging/privacy.ts`:
- Around line 56-72: The createPrivacyHook currently maps message.data without
guarding type and on catch re-attaches the original data, potentially leaking
secrets; update the hook so it first verifies Array.isArray(message?.data)
before mapping (use redactSensitiveData and redactObject as you already do) and
if data is not an array return the message with a safe placeholder data array
like ['[Privacy redaction failed]'] or an empty array; additionally, in the
catch block do NOT include message.data—return {...message, data: ['[Privacy
redaction failed]']} so unredacted payloads are never returned.

In `@src/logViewerWindow/ipc.ts`:
- Around line 140-161: The dialog presents an "All Files" option but
validateLogFilePath only allows .log/.txt, causing confusing rejections; either
remove the `{ name: 'All Files', extensions: ['*'] }` entry from the options
passed to dialog.showOpenDialog in ipc.ts (so users can only pick .log/.txt), or
update the validateLogFilePath function to accept any extension (or to accept
whatever extensions are allowed by the dialog) so the selection and validation
match; locate dialog.showOpenDialog and validateLogFilePath in
src/logViewerWindow/ipc.ts and apply one of these two fixes consistently.

In `@src/logViewerWindow/logViewerWindow.tsx`:
- Line 16: The file currently imports ipcRenderer directly (import { ipcRenderer
} from 'electron') which bypasses the preload security layer; remove that import
and replace all ipcRenderer.invoke(...) usages in logViewerWindow.tsx with the
safe methods exposed on window.logViewerAPI (e.g., call
window.logViewerAPI.<methodName> for each IPC action), and ensure the preload.ts
contextBridge exposes corresponding method names that return the same
data/Promise shapes expected by the component (update any call sites in
functions or event handlers inside logViewerWindow.tsx such as the routines that
previously used ipcRenderer.invoke to use window.logViewerAPI instead).
- Line 134: Replace the hardcoded label ['outlook', '📅 Outlook Calendar'] with
a translated string using the project's i18n function (e.g.,
t('filters.outlookCalendar')) in the same array/constant (where the filter
options are defined in logViewerWindow.tsx), ensure the component imports/uses
the translation hook or helper (useTranslation/t), and add the key
"filters.outlookCalendar": "📅 Outlook Calendar" to the locale files (and
counterparts for other locales) so the option is consistent with other filter
options.
- Around line 820-823: Replace the hardcoded "entries" string in the JSX that
renders "{filteredLogs.length} entries" with the app's i18n translation function
(e.g., use t or i18n.t) and pass the count for pluralization/formatting; update
the render in the component (the div that currently shows filteredLogs.length
entries) to call the translation helper with filteredLogs.length so the label is
localized and correctly pluralized.

In `@src/logViewerWindow/types.ts`:
- Around line 56-60: The isLogLevel type guard currently validates
value.toLowerCase() but narrows the original value as LogLevel, which is unsafe;
change isLogLevel's signature from "value is LogLevel" to a plain boolean
"(value: string) => boolean" and update any callers to either call parseLogLevel
(or explicitly lowercase before treating as LogLevel) instead of relying on the
guard to produce a correctly-cased LogLevel; reference isLogLevel, LogLevel and
parseLogLevel when making these changes.

🧹 Nitpick comments (8)

src/injected.ts (1)

527-535: Maintain consistent logging approach for userId across this feature.

The pre-check log at lines 511-516 uses !!userId to mask the actual value, but line 529 logs the full userId. For consistency with the privacy-conscious approach established in the earlier log, consider using the same masking pattern here as well.

The setOutlookExchangeUrl function signature correctly accepts userId as the second parameter, so the API call is correct.

💡 Optional: Consistent logging approach

         console.log(
           '[Rocket.Chat Desktop] Setting Outlook token for user:',
-          userId
+          !!userId
         );

src/outlookCalendar/ipc.ts (2)

31-37: Unsafe type cast may cause runtime errors downstream.

Returning {} as Server when no server is found bypasses TypeScript's type safety. Callers accessing properties like server.url or server.outlookCredentials will get undefined without type errors, potentially causing runtime failures.

Consider returning undefined and updating callers to handle it:

🛡️ Proposed fix

-const getServerInformationByWebContentsId = (webContentsId: number): Server => {
+const getServerInformationByWebContentsId = (webContentsId: number): Server | undefined => {
   const { servers } = select(selectPersistableValues);
   const server = servers.find(
     (server) => server.webContentsId === webContentsId
   );
-  return server || ({} as Server);
+  return server;
 };

---

53-54: Inconsistent use of scoped logger vs console.

The file imports and uses loggers.outlook in a few places (lines 53, 69, 90, 106) but predominantly uses console.log/console.error with [OutlookCalendar] prefix. For the Log Viewer's context filtering to work effectively, consider migrating all logging to the scoped logger:

-    console.log('[OutlookCalendar] Fetching events from Rocket.Chat server:', {
+    loggers.outlook.info('Fetching events from Rocket.Chat server:', {
       url,
       userId,
       hasToken: !!token,
     });

This would allow users to filter specifically for Outlook-related logs in the new Log Viewer.

Also applies to: 130-134

src/logViewerWindow/constants.ts (1)

1-21: Consider removing redundant comments.

The constants are self-descriptive; the inline comments add noise without extra context.

♻️ Proposed cleanup

-/**
- * Constants for Log Viewer window
- */
-
-/** Auto-refresh interval in milliseconds */
 export const AUTO_REFRESH_INTERVAL_MS = 2000;
-
-/** Delay before scrolling to ensure Virtuoso is ready */
 export const SCROLL_DELAY_MS = 100;
-
-/** Window size as multiplier of screen size */
 export const WINDOW_SIZE_MULTIPLIER = 0.8;
-
-/** Debounce delay for search filter in milliseconds */
 export const SEARCH_DEBOUNCE_MS = 300;
-
-/** Maximum log file size in bytes (10MB) */
 export const MAX_LOG_FILE_SIZE = 10 * 1024 * 1024;
-
-/** Virtuoso overscan count for smooth scrolling */
 export const VIRTUOSO_OVERSCAN = 50;

As per coding guidelines, "No unnecessary comments - code must be self-documenting through clear naming and structure; only add comments for complex business logic or non-obvious decisions".

src/logViewerWindow/ipc.ts (1)

200-211: Tail reading loads entire file into memory.

For the "last N lines" case, the entire file is read into memory before slicing. For large log files (up to 10MB per the file transport config), this is inefficient.

Consider using a streaming approach or reading from the end of the file for better performance with large logs.

src/logViewerWindow/logViewerWindow.tsx (1)

722-727: Magic number 100 for scroll timeout.

The setTimeout delay should use the existing SCROLL_DELAY_MS constant for consistency.

♻️ Suggested fix

                   if (filteredLogs.length > 0 && virtuosoRef.current) {
                     setTimeout(() => {
                       virtuosoRef.current?.scrollToIndex({
                         index: 0,
                         behavior: 'smooth',
                       });
-                    }, 100);
+                    }, SCROLL_DELAY_MS);
                   }

src/logging/index.ts (1)

319-333: Validate log level against an allowlist.

The level parameter from the renderer is used directly in the switch statement without validation. A malformed or unexpected level would fall through silently. Clamp to an allowlist.

🛡️ Suggested fix

+          // Validate level against allowlist
+          const allowedLevels = new Set(['debug', 'info', 'warn', 'error']);
+          const safeLevel = allowedLevels.has(level) ? level : 'info';
+
           // Log with enhanced context
-          switch (level) {
+          switch (safeLevel) {
             case 'debug':

src/i18n/en.i18n.json (1)

447-453: The logging.context.components section uses unused keys.

Line 452 uses "videoCall", but this entire section appears to be unused—no code in the codebase accesses logging.context.components. The active logging context filtering uses logging.context.filters.context, which already correctly uses lowercase "videocall" to align with getComponentContext(). Consider either aligning the unused components section for consistency or removing it if it's no longer needed.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Avoid committing user-specific absolute paths and session IDs.

active_plan contains a local absolute path (leaks username, breaks portability), and session_ids looks per-user/session state. This should be generated locally or stored in a template with relative paths. If the file must live in repo, make it environment-agnostic and avoid storing session identifiers.

🔧 Proposed adjustment (if keeping the file in VCS)

 {
-  "active_plan": "/Users/jean/Github/Rocket.Chat.Electron/.sisyphus/plans/log-viewer-professional-review.md",
+  "active_plan": ".sisyphus/plans/log-viewer-professional-review.md",
   "started_at": "2026-01-29T14:31:04.420Z",
-  "session_ids": [
-    "ses_3f5e67dc9ffeRCoFXMk5DAQmSe"
-  ],
+  "session_ids": [],
   "plan_name": "log-viewer-professional-review"
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"active_plan": "/Users/jean/Github/Rocket.Chat.Electron/.sisyphus/plans/log-viewer-professional-review.md",
	"started_at": "2026-01-29T14:31:04.420Z",
	"session_ids": [
	"ses_3f5e67dc9ffeRCoFXMk5DAQmSe"
	],
	"active_plan": ".sisyphus/plans/log-viewer-professional-review.md",
	"started_at": "2026-01-29T14:31:04.420Z",
	"session_ids": [],

🤖 Prompt for AI Agents

In @.sisyphus/boulder.json around lines 2 - 6, The file currently commits
user-specific data: the "active_plan" value is an absolute path exposing a local
username and "session_ids" contains per-session identifiers; update the file to
remove or neutralize those values by replacing "active_plan" with a
repository-relative or templated path (e.g., a relative path or a placeholder
like "plans/log-viewer-professional-review.md") and remove or blank out
"session_ids" (or move session runtime data to a local-only file and add it to
.gitignore); ensure any code that reads "active_plan" or "session_ids" can
handle the template/empty value or fallback to generating them at runtime (check
the reader that references active_plan/session_ids).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add a language identifier to the fenced block.

markdownlint MD040 flags the unlabeled fence.

✅ Fix

-```
+```text

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	```
	Wave 1 (Critical - Privacy & Memory):
	├── Task 1: Add privacy hooks for sensitive data filtering
	├── Task 2: Fix server counter memory leak
	└── Task 3: Add logging failure fallback
	Wave 2 (Architecture - Scoped Loggers):
	├── Task 4: Create scoped logger factory
	├── Task 5: Migrate from stack-trace to explicit scopes
	└── Task 6: Add structured logging option
	Wave 3 (Polish - Cleanup & Docs):
	├── Task 7: Add log file cleanup on startup
	├── Task 8: Add retention policy configuration
	└── Task 9: Add runtime log level control
	```

🧰 Tools

🪛 markdownlint-cli2 (0.20.0)

118-118: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In @.sisyphus/plans/logging-system-professional-upgrade.md around lines 118 -
133, The fenced code block that begins with the "Wave 1 (Critical - Privacy &
Memory):" list is missing a language identifier which triggers markdownlint
MD040; add a language tag (e.g., text) to the opening triple-backtick so the
block becomes ```text to silence MD040 and preserve formatting of the Wave/Task
list.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use a heading instead of bold text.

markdownlint MD036 flags emphasis used as a heading.

✅ Fix

-**Total: 9 tasks across 3 waves**
+### Total: 9 tasks across 3 waves

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	**Total: 9 tasks across 3 waves**
	### Total: 9 tasks across 3 waves

🧰 Tools

🪛 markdownlint-cli2 (0.20.0)

788-788: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🤖 Prompt for AI Agents

In @.sisyphus/plans/logging-system-professional-upgrade.md at line 788, The line
that currently uses bold emphasis "**Total: 9 tasks across 3 waves**" should be
converted to a markdown heading to satisfy markdownlint MD036; locate the exact
text "Total: 9 tasks across 3 waves" and replace the bold markup with an
appropriate heading level (for example prefix with "## " or "### " as consistent
with surrounding headings) so the line becomes a proper heading instead of bold
text.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Avoid duplicate server IDs when the pool is exhausted.

Line 88 falls back to MAX_SERVER_ID, which can collide when more than 100 webviews exist and muddle context mapping.

🔧 Proposed fix

-    const serverId = availableServerIds.shift() || MAX_SERVER_ID;
-    usedServerIds.set(webContentsId, serverId);
-    const serverName = `server-${serverId}`;
+    const serverId = availableServerIds.shift();
+    const serverName = serverId ? `server-${serverId}` : 'server-overflow';
+    if (serverId !== undefined) {
+      usedServerIds.set(webContentsId, serverId);
+    }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// For webviews, assign anonymous server ID from pool
	if (webContents.getType() === 'webview') {
	const serverId = availableServerIds.shift() || MAX_SERVER_ID;
	usedServerIds.set(webContentsId, serverId);
	const serverName = `server-${serverId}`;
	serverContextMap.set(webContentsId, {
	serverUrl: 'unknown',
	serverName,
	});
	return serverName;
	// For webviews, assign anonymous server ID from pool
	if (webContents.getType() === 'webview') {
	const serverId = availableServerIds.shift();
	const serverName = serverId ? `server-${serverId}` : 'server-overflow';
	if (serverId !== undefined) {
	usedServerIds.set(webContentsId, serverId);
	}
	serverContextMap.set(webContentsId, {
	serverUrl: 'unknown',
	serverName,
	});
	return serverName;

🤖 Prompt for AI Agents

In `@src/logging/context.ts` around lines 86 - 95, The fallback to MAX_SERVER_ID
when availableServerIds.shift() is undefined can produce duplicate IDs; change
the allocation in the webview branch so that when availableServerIds is empty
you generate a unique serverId instead of using MAX_SERVER_ID (e.g.,
maintain/increment a lastAllocatedServerId counter or compute
Math.max(...usedServerIds values)+1), then set usedServerIds.set(webContentsId,
serverId) and continue building serverName and serverContextMap as before
(update any initialization for lastAllocatedServerId near the top of the
module). Ensure the logic references webContents.getType(), availableServerIds,
usedServerIds, webContentsId, serverContextMap, and MAX_SERVER_ID.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Direct ipcRenderer import bypasses the preload security layer.

The component imports ipcRenderer directly from 'electron', but the preload script (preload.ts) exposes a safe window.logViewerAPI via contextBridge. With contextIsolation: true and nodeIntegration: false in the BrowserWindow config, this direct import should fail at runtime.

Use window.logViewerAPI instead:

🔧 Suggested fix

-import { ipcRenderer } from 'electron';

Then replace all ipcRenderer.invoke(...) calls with the corresponding window.logViewerAPI methods:

-      const response = (await ipcRenderer.invoke(
-        'log-viewer-window/read-logs',
-        { ... }
-      )) as ReadLogsResponse;
+      const response = await window.logViewerAPI.readLogs({ ... });

-      const response = (await ipcRenderer.invoke(
-        'log-viewer-window/select-log-file'
-      )) as SelectFileResponse;
+      const response = await window.logViewerAPI.selectLogFile();

-      const response = (await ipcRenderer.invoke(
-        'log-viewer-window/clear-logs'
-      )) as ClearLogsResponse;
+      const response = await window.logViewerAPI.clearLogs();

-    ipcRenderer.invoke('log-viewer-window/close-requested');
+    window.logViewerAPI.closeWindow();

🤖 Prompt for AI Agents

In `@src/logViewerWindow/logViewerWindow.tsx` at line 16, The file currently
imports ipcRenderer directly (import { ipcRenderer } from 'electron') which
bypasses the preload security layer; remove that import and replace all
ipcRenderer.invoke(...) usages in logViewerWindow.tsx with the safe methods
exposed on window.logViewerAPI (e.g., call window.logViewerAPI.<methodName> for
each IPC action), and ensure the preload.ts contextBridge exposes corresponding
method names that return the same data/Promise shapes expected by the component
(update any call sites in functions or event handlers inside logViewerWindow.tsx
such as the routines that previously used ipcRenderer.invoke to use
window.logViewerAPI instead).

[coderabbitai]

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

.github/workflows/validate-pr.yml (1)

52-54: Windows validation build is missing arm64 architecture.

The electron-builder.json changes in this PR add arm64 to Windows targets (nsis, msi, zip), but the Windows validation build here only includes --x64 --ia32. This means arm64 builds won't be validated during PR checks.

Based on learnings: "When modifying Windows build commands, ensure all architectures are included: x64, ia32, and arm64 with electron-builder."

🔧 Proposed fix

       - name: Build uncompressed on dist/
         if: ${{ matrix.os == 'windows-latest' }}
-        run: yarn electron-builder --x64 --ia32 --dir
+        run: yarn electron-builder --x64 --ia32 --arm64 --dir

src/injected.ts (1)

518-526: Avoid logging raw user IDs to disk.

Logging the full userId risks leaking identifiers when logs are shared. Prefer masking or omit the value.

🔒 Suggested masking

-        console.log(
-          '[Rocket.Chat Desktop] Setting Outlook token for user:',
-          userId
-        );
+        const maskedUserId =
+          typeof userId === 'string'
+            ? `${userId.slice(0, 4)}…${userId.slice(-4)}`
+            : String(userId);
+        console.log(
+          '[Rocket.Chat Desktop] Setting Outlook token for user:',
+          maskedUserId
+        );

🤖 Fix all issues with AI agents

In `@docs/qa-alpha-update-testing.md`:
- Line 7: Replace the bare URL string
"https://github.com/RocketChat/Rocket.Chat.Electron/releases" with a Markdown
link (e.g., [GitHub
releases](https://github.com/RocketChat/Rocket.Chat.Electron/releases)) so the
docs line uses inline Markdown link syntax instead of a raw URL; update the line
that currently reads "Access to the GitHub releases page:
https://github.com/RocketChat/Rocket.Chat.Electron/releases" to use the
bracketed link form for markdownlint compliance and improved readability.
- Around line 1-4: The doc "QA Testing: Alpha Channel Updates" is missing a flow
diagram and an example report; add a short mermaid flowchart showing steps
A[Install 4.12.0-alpha.1] -> B[Enable Alpha channel] -> C[Check for updates] ->
D[Download + install 4.12.0-alpha.2] -> E[Verify version], and append a brief
"Example report" block that lists OS, From, To, Result, and Notes (e.g., OS:
Windows 11 23H2 / From: 4.12.0-alpha.1 / To: 4.12.0-alpha.2 / Result: Update
found, download ok, restart ok / Notes: No errors in About dialog) so the
document meets the flow-diagram and usage-example guideline.
- Around line 56-58: The Markdown table header row is missing spaces around the
pipe separators; update the header line "| Step | Expected Behavior |" and all
table rows to ensure there's a single space before and after each pipe (e.g., "
| Step | Expected Behavior | " style) so the header and separators conform to
markdownlint spacing rules and render correctly; verify the separator row
"|------|-------------------|" also has spaces around pipes and adjust other
rows like "| Install Alpha 1 | App installs and runs without errors |" to match.

In `@package.json`:
- Line 38: Update the "build-win-unsigned" npm script so the electron-builder
invocation includes all Windows architectures; modify the command referenced by
the "build-win-unsigned" script to add the flags --x64 --ia32 --arm64 to the
existing electron-builder call (keeping the existing cross-env and publish
settings) so the unsigned Windows build produces x64, ia32, and arm64 artifacts.
- Line 9: Update package.json to account for the dependency breaking changes and
to make Windows builds explicit: audit and fix any Buffer.allocUnsafe usages in
your codebase to avoid relying on zero-filled memory; replace direct
renderer-process clipboard calls with a preload + contextBridge implementation
(move clipboard access behind preload script referenced by your Electron
BrowserWindow options); update your electron build config to use the new macOS
debug symbol format (.dsym.tar.xz) instead of .dsym.zip; run tests and adapt
code to axios 1.13.3 changes (handle AxiosError as a native Error and ensure
JSON parsing errors are caught when silentJSONParsing is false); and modify the
package.json "scripts" and/or "build" config entries (the "scripts" keys that
perform Windows builds and the "build" object used by electron-builder) to
explicitly specify architectures x64, ia32 and arm64 for Windows targets so
builds include all three archs; also review the "version" field and dependency
pins in "dependencies"/"devDependencies" to decide whether to pin or upgrade
versions after verifying compatibility.

In `@src/i18n/it-IT.i18n.json`:
- Around line 8-13: Update the Italian translation for the themeAppearance.auto
label to use the natural phrasing "Segui il sistema" instead of "Segui sistema";
locate the themeAppearance object and change the value of the "auto" key
accordingly so the UI reads the standard Italian label.

In `@src/i18n/ja.i18n.json`:
- Around line 226-240: There are duplicate top-level "settings" objects causing
invalid/ambiguous JSON; remove the duplicate and merge "themeAppearance" into
the existing "settings.options" block so both "transparentWindow" and
"themeAppearance" live under a single "settings.options"; ensure you keep all
keys ("title", "description", "auto", "light", "dark") and remove the redundant
"settings" wrapper that contains "themeAppearance", updating any consumers if
they relied on the removed duplicate structure.

In `@src/main.ts`:
- Around line 57-62: The startup logging is initialized before Electron is
ready; either defer configureLogging() and cleanupOldLogs() until after
app.whenReady() or make them defensive: update configureLogging() (and
cleanupOldLogs()) to call app.getPath('logs') via optional chaining with a
fallback (e.g., app.getPath?.('logs') ?? fallbackPath) and ensure
setupWebContentsLogging() is registered after app.whenReady() (or guarded so it
uses app.on only if available); modify references to configureLogging,
cleanupOldLogs, and setupWebContentsLogging so they are invoked post-app
readiness or made safe for pre-ready import.

In `@workspaces/desktop-release-action/package.json`:
- Around line 32-35: Remove the deprecated dependency entry for "@types/glob"
from package.json because glob v11.x includes its own TypeScript types; locate
the dependency line containing "@types/glob": "~9.0.0" and delete it, leaving
the "glob": "~11.1.0" (and any related entries like "fast-glob") intact, then
run your package manager (npm/yarn/pnpm) to update lockfile and ensure
TypeScript still compiles.

🧹 Nitpick comments (6)

.github/workflows/validate-pr.yml (1)

70-82: Windows executable test is missing arm64.

The Windows executable validation tests x64 (win-unpacked) and ia32 (win-ia32-unpacked) but not arm64 (win-arm64-unpacked). If arm64 is added to the build step, consider adding validation for it as well.

docs/qa-alpha-update-testing.md (1)

88-99: Add Log Viewer capture instructions to issue reports.

Given the new Log Viewer feature, include a short step to export logs from the viewer when reporting issues.

✍️ Suggested addition

 4. Any error messages displayed
 5. App logs if available:
    - **Windows**: `%APPDATA%\Rocket.Chat\logs`
    - **macOS**: `~/Library/Logs/Rocket.Chat`
    - **Linux**: `~/.config/Rocket.Chat/logs`
+6. If available, open **Help → Open Log Viewer** and use **Copy** or **Save** to attach the output.

docs/pre-release-process.md (2)

138-144: Inconsistent terminology: "prerelease" vs "pre-release".

The document uses "Pre-release" on line 65 but "prerelease" on lines 138 and 144. For consistency, use the same form throughout the document.

🔧 Suggested fix

-### Switching to a prerelease channel (stable → alpha/beta)
+### Switching to a pre-release channel (stable → alpha/beta)

 1. Open Settings > Enable Developer Mode
 2. Open About dialog
 3. Select the desired channel from dropdown
 4. Click "Check for Updates"
-5. The next prerelease version will be offered
+5. The next pre-release version will be offered

---

174-176: Add language identifier to code fence.

The code fence is missing a language specifier. Since this represents a version progression, consider using text or plaintext as the language identifier.

🔧 Suggested fix

-```
+```text
 4.12.0-alpha.1 → 4.12.0-alpha.2 → 4.12.0-beta.1 → 4.12.0-beta.2 → 4.12.0

</details>

</blockquote></details>
<details>
<summary>src/ui/components/ServersView/DocumentViewer.tsx (1)</summary><blockquote>

`77-85`: **Consider using Fuselage Box for the heading.**

Line 84 uses a raw `<h2>` element. Per coding guidelines, Fuselage components should be preferred over HTML elements.


<details>
<summary>💡 Suggested change</summary>

```diff
         <Box
           content='center'
           alignItems='center'
           display='flex'
           color='default'
         >
           <IconButton icon='arrow-back' onClick={closeDocumentViewer} mi='x8' />
-          <h2>PDF Viewer</h2>
+          <Box is='h2' fontScale='h2'>PDF Viewer</Box>
         </Box>

scripts/release-tag.ts (1)

54-62: Consider using semver.compare for cleaner sorting.

The current sort using gt() works but is less idiomatic. The semver library provides a compare function designed for sorting.

💡 Suggested change

+import { parse, gt, compare, prerelease, SemVer } from 'semver';
...
 const getLatestTagForChannel = (tags: string[], channel: string): SemVer | null => {
   const channelTags = tags
     .map((tag) => parse(tag))
     .filter((v): v is SemVer => v !== null)
     .filter((v) => getChannel(v) === channel)
-    .sort((a, b) => (gt(a, b) ? -1 : 1));
+    .sort((a, b) => compare(b, a)); // Descending order

   return channelTags[0] || null;
 };

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Add a short flow diagram + example per docs guideline.

This is a new doc but it doesn’t include a flow diagram or a usage example. Please add a simple update-flow diagram and a short example report so the doc meets the docs requirement. As per coding guidelines, create documentation for new features with flow diagrams, architecture explanations, and usage examples.

✍️ Suggested addition (simple and short)

 This guide explains how to test the alpha update flow from `4.12.0-alpha.1` to `4.12.0-alpha.2`.

+## Update flow overview
+
+```mermaid
+flowchart TD
+  A[Install 4.12.0-alpha.1] --> B[Enable Alpha channel]
+  B --> C[Check for updates]
+  C --> D[Download + install 4.12.0-alpha.2]
+  D --> E[Verify version]
+```
+
+## Example report
+
+```
+OS: Windows 11 23H2
+From: 4.12.0-alpha.1
+To: 4.12.0-alpha.2
+Result: Update found, download ok, restart ok
+Notes: No errors in About dialog
+```

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	# QA Testing: Alpha Channel Updates
	This guide explains how to test the alpha update flow from `4.12.0-alpha.1` to `4.12.0-alpha.2`.
	# QA Testing: Alpha Channel Updates
	This guide explains how to test the alpha update flow from `4.12.0-alpha.1` to `4.12.0-alpha.2`.
	## Update flow overview

🤖 Prompt for AI Agents

In `@docs/qa-alpha-update-testing.md` around lines 1 - 4, The doc "QA Testing:
Alpha Channel Updates" is missing a flow diagram and an example report; add a
short mermaid flowchart showing steps A[Install 4.12.0-alpha.1] -> B[Enable
Alpha channel] -> C[Check for updates] -> D[Download + install 4.12.0-alpha.2]
-> E[Verify version], and append a brief "Example report" block that lists OS,
From, To, Result, and Notes (e.g., OS: Windows 11 23H2 / From: 4.12.0-alpha.1 /
To: 4.12.0-alpha.2 / Result: Update found, download ok, restart ok / Notes: No
errors in About dialog) so the document meets the flow-diagram and usage-example
guideline.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use a Markdown link instead of a bare URL.

The bare URL trips markdownlint and is harder to read.

✅ Suggested fix

-- Access to the GitHub releases page: https://github.com/RocketChat/Rocket.Chat.Electron/releases
+- Access to the GitHub releases page: [GitHub Releases](https://github.com/RocketChat/Rocket.Chat.Electron/releases)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	- Access to the GitHub releases page: https://github.com/RocketChat/Rocket.Chat.Electron/releases
	- Access to the GitHub releases page: [GitHub Releases](https://github.com/RocketChat/Rocket.Chat.Electron/releases)

🧰 Tools

🪛 markdownlint-cli2 (0.20.0)

7-7: Bare URL used

(MD034, no-bare-urls)

🤖 Prompt for AI Agents

In `@docs/qa-alpha-update-testing.md` at line 7, Replace the bare URL string
"https://github.com/RocketChat/Rocket.Chat.Electron/releases" with a Markdown
link (e.g., [GitHub
releases](https://github.com/RocketChat/Rocket.Chat.Electron/releases)) so the
docs line uses inline Markdown link syntax instead of a raw URL; update the line
that currently reads "Access to the GitHub releases page:
https://github.com/RocketChat/Rocket.Chat.Electron/releases" to use the
bracketed link form for markdownlint compliance and improved readability.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix the table spacing to satisfy markdownlint.

The header row is missing spaces around pipes.

✅ Suggested fix

-| Step | Expected Behavior |
-|------|-------------------|
+| Step | Expected Behavior |
+| --- | --- |

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	| Step | Expected Behavior |
	|------|-------------------|
	| Install Alpha 1 | App installs and runs without errors |
	| Step | Expected Behavior |
	| --- | --- |
	| Install Alpha 1 | App installs and runs without errors |

🧰 Tools

🪛 markdownlint-cli2 (0.20.0)

57-57: Table column style
Table pipe is missing space to the right for style "compact"

(MD060, table-column-style)

---

57-57: Table column style
Table pipe is missing space to the left for style "compact"

(MD060, table-column-style)

---

57-57: Table column style
Table pipe is missing space to the right for style "compact"

(MD060, table-column-style)

---

57-57: Table column style
Table pipe is missing space to the left for style "compact"

(MD060, table-column-style)

🤖 Prompt for AI Agents

In `@docs/qa-alpha-update-testing.md` around lines 56 - 58, The Markdown table
header row is missing spaces around the pipe separators; update the header line
"| Step | Expected Behavior |" and all table rows to ensure there's a single
space before and after each pipe (e.g., " | Step | Expected Behavior | " style)
so the header and separators conform to markdownlint spacing rules and render
correctly; verify the separator row "|------|-------------------|" also has
spaces around pipes and adjust other rows like "| Install Alpha 1 | App installs
and runs without errors |" to match.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n package.json | head -200

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 7282

---

🏁 Script executed:

# Check Windows build command configuration
sed -n '37,38p' package.json

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 330

---

🌐 Web query:

Electron 40.0.0 breaking changes release notes

💡 Result:

Electron 40.0.0 — Breaking changes (release notes)

• Deprecated clipboard API access from renderer processes (clipboard in renderer is now deprecated). Use the clipboard API from a preload script and expose it via contextBridge instead. [1][2]
• macOS debug symbols packaging changed: dsym.zip is now dsym.tar.xz (xz compression) to support larger files; update any tooling that expects ZIP. [1][2]

(Associated stack upgrades in 40.0.0: Chromium 144.0.7559.60, Node 24.11.1, V8 14.4.) [1][2]

Sources: Electron v40.0.0 release page [1], Electron “Electron 40.0.0” blog post [2].

---

🌐 Web query:

electron-updater 6.3.9 breaking changes release notes

💡 Result:

electron-updater@6.3.9 is a patch release and its published notes list no breaking changes—only fixes/maintenance items. Release notes for 6.3.9 include:

• Fix: “Unable to copy file for caching: ENOENT” (PR #8541)
• Chore: devDependencies update (including TypeScript) (PR #8545)
• Deps: bump builder-util-runtime to 9.2.10 [1]

---

🌐 Web query:

Node.js 24.11.1 breaking changes release notes

💡 Result:

Node.js v24.11.1 (LTS) (released 2025-11-11) does not list broad “breaking changes” in its official release notes, but it does include one behavior change that can break apps:

• Buffer.allocUnsafe() no longer (incorrectly) zero-fills memory. A prior known issue caused Buffer.allocUnsafe to return zeroed buffers; in 24.11.1 this is fixed so it again returns uninitialized memory as documented. Any code that (incorrectly) relied on the buffer being zero-filled may now behave differently and must explicitly initialize (e.g., buf.fill(0) or use Buffer.alloc). [1][2]

Sources: Node.js release blog / GitHub release notes. [1][2]

---

🌐 Web query:

axios 1.13.x breaking changes

💡 Result:

As of January 27, 2026, the latest 1.13.x release is axios v1.13.4. In the 1.13.x line, axios does not call out any explicit “Breaking Changes” in the release notes; for example, v1.13.4 explicitly says “Breaking Changes: None”. [1]

That said, there are a couple of notable behavior/type changes introduced within 1.13.x that can function like breaking changes depending on your app:

• v1.13.3
  • “turn AxiosError into a native error” (may affect instanceof Error, enumerability/shape, stack/cause handling, etc.). [1]
  • silentJSONParsing=false should throw on invalid JSON (if you rely on non-throwing behavior for invalid JSON, this can break flows). [1]

Other 1.13.x releases look like features/fixes (no breaking changes indicated in the notes):

• v1.13.0: adds HTTP/2 support. [2]
• v1.13.1 / v1.13.2 / v1.13.4: bugfixes/perf/infra; v1.13.4 explicitly says no breaking changes. [1][2]

If you’re upgrading from 0.x → 1.x, the real breaking changes are covered in the v0.x to v1.x upgrade guide (imports, headers shape, interceptor typing, etc.). [3]

Sources: axios GitHub releases (v1.13.0–v1.13.4) [1][2], axios upgrade guide [3].

---

🌐 Web query:

archiver 7.0.1 breaking changes release notes

💡 Result:

For the npm package archiver (archiverjs/node-archiver):

• v7.0.1 (March 9–10, 2024) release notes: contains maintenance + dependency updates only (e.g., added a test for “stream-like sources”, bumped zip-stream and archiver-utils, plus docs-site deps). No breaking changes are listed for 7.0.1. [1]
• The breaking change in the v7 line is in v7.0.0 (Feb 28, 2024): “Drop support for Node v12.” [2]

Sources: [1], [2]

---

Verify and resolve breaking changes from dependency updates.

Electron 40.0.0 introduces breaking changes: renderer process clipboard API is deprecated (use preload + contextBridge instead), and macOS debug symbols format changed from .dsym.zip to .dsym.tar.xz. Node 24.11.1 changes Buffer.allocUnsafe() behavior—it no longer zero-fills memory, which may break code relying on the previous (incorrect) behavior. axios 1.13.3 introduced behavior changes (AxiosError is now a native error; silentJSONParsing=false now throws on invalid JSON). electron-updater 6.3.9 and archiver 7.0.1 are patch/maintenance releases with no breaking changes.

Additionally, the Windows build commands (lines 37–38) do not specify architectures. Ensure they include x64, ia32, and arm64 with electron-builder.

Also applies to: 66-73, 108-113, 121-121, 139-140, 149-170

🤖 Prompt for AI Agents

In `@package.json` at line 9, Update package.json to account for the dependency
breaking changes and to make Windows builds explicit: audit and fix any
Buffer.allocUnsafe usages in your codebase to avoid relying on zero-filled
memory; replace direct renderer-process clipboard calls with a preload +
contextBridge implementation (move clipboard access behind preload script
referenced by your Electron BrowserWindow options); update your electron build
config to use the new macOS debug symbol format (.dsym.tar.xz) instead of
.dsym.zip; run tests and adapt code to axios 1.13.3 changes (handle AxiosError
as a native Error and ensure JSON parsing errors are caught when
silentJSONParsing is false); and modify the package.json "scripts" and/or
"build" config entries (the "scripts" keys that perform Windows builds and the
"build" object used by electron-builder) to explicitly specify architectures
x64, ia32 and arm64 for Windows targets so builds include all three archs; also
review the "version" field and dependency pins in
"dependencies"/"devDependencies" to decide whether to pin or upgrade versions
after verifying compatibility.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

electron-builder Windows build command line options x64 ia32 arm64 nsis

💡 Result:

electron-builder Windows CLI options (x64 / ia32 / arm64 + NSIS)

Platform + target selection

• --win / -w / --windows — build for Windows (can take a target list, e.g. nsis, nsis-web, portable) [1]
• Targets can be passed like:
  • electron-builder --win nsis [1]
  • electron-builder --win portable [2]
  • Web installer: electron-builder --win nsis-web [2]

Architecture flags

• --x64 — build x64 [1]
• --ia32 — build 32-bit (ia32) [1]
• --arm64 — build ARM64 [1]

Common Windows+NSIS examples

• NSIS x64 only:
  • electron-builder --win nsis --x64 [1]
• NSIS ia32 only (also supported as target shorthand):
  • electron-builder --win nsis:ia32 [1]
• NSIS for both x64 + ia32 (combined installer behavior):
  • electron-builder --win nsis --x64 --ia32
  • If you build both ia32 and x64, electron-builder produces one installer that installs the appropriate arch automatically (same idea applies to nsis-web) [2]
• NSIS-web (downloads correct arch at install time):
  • electron-builder --win nsis-web --x64 --arm64 (builds packages; installer downloads correct one) [2]

Passing NSIS options from the command line

• You can override config values via -c.<path>=<value>, for example:
  • electron-builder --win -c.nsis.unicode=false [1]
  • (Docs show oneClick / perMachine as NSIS config keys you set under build.nsis in config files.) [2]

Sources: electron-builder CLI docs [1], NSIS docs [2].

---

Include all Windows architectures in the unsigned build.

The new command builds only the default arch; add --x64, --ia32, and --arm64 flags so the unsigned build matches the supported Windows matrix.

💡 Suggested update

-    "build-win-unsigned": "yarn patch-package && yarn build && cross-env CSC_IDENTITY_AUTO_DISCOVERY=false CSC_KEY_PASSWORD='' yarn electron-builder --publish never --win nsis",
+    "build-win-unsigned": "yarn patch-package && yarn build && cross-env CSC_IDENTITY_AUTO_DISCOVERY=false CSC_KEY_PASSWORD='' yarn electron-builder --publish never --win nsis --x64 --ia32 --arm64",

Per coding guidelines: When modifying Windows build commands, ensure all architectures are included: x64, ia32, and arm64 with electron-builder.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"build-win-unsigned": "yarn patch-package && yarn build && cross-env CSC_IDENTITY_AUTO_DISCOVERY=false CSC_KEY_PASSWORD='' yarn electron-builder --publish never --win nsis",
	"build-win-unsigned": "yarn patch-package && yarn build && cross-env CSC_IDENTITY_AUTO_DISCOVERY=false CSC_KEY_PASSWORD='' yarn electron-builder --publish never --win nsis --x64 --ia32 --arm64",

🤖 Prompt for AI Agents

In `@package.json` at line 38, Update the "build-win-unsigned" npm script so the
electron-builder invocation includes all Windows architectures; modify the
command referenced by the "build-win-unsigned" script to add the flags --x64
--ia32 --arm64 to the existing electron-builder call (keeping the existing
cross-env and publish settings) so the unsigned Windows build produces x64,
ia32, and arm64 artifacts.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Prefer “Segui il sistema” for natural Italian.

“Segui sistema” sounds clipped; “Segui il sistema” is the standard phrasing for this UI label.

💬 Suggested wording

-        "auto": "Segui sistema",
+        "auto": "Segui il sistema",

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"themeAppearance": {
	"title": "Tema",
	"description": "Scegli il tema colore per l'applicazione.",
	"auto": "Segui sistema",
	"light": "Chiaro",
	"dark": "Scuro"
	"themeAppearance": {
	"title": "Tema",
	"description": "Scegli il tema colore per l'applicazione.",
	"auto": "Segui il sistema",
	"light": "Chiaro",
	"dark": "Scuro"

🤖 Prompt for AI Agents

In `@src/i18n/it-IT.i18n.json` around lines 8 - 13, Update the Italian translation
for the themeAppearance.auto label to use the natural phrasing "Segui il
sistema" instead of "Segui sistema"; locate the themeAppearance object and
change the value of the "auto" key accordingly so the UI reads the standard
Italian label.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Avoid duplicate settings keys (invalid/ambiguous JSON).

This file now has two top-level "settings" objects; JSON parsers may overwrite the earlier one or fail. Please merge themeAppearance into the existing settings.options block and remove this duplicate.

🧩 Suggested fix (merge into existing block)

-  "settings": {
-    "options": {
-      "transparentWindow": {
-        "title": "透明ウィンドウ効果",
-        "description": "ウィンドウのネイティブなビブランシー/透明効果を有効にします。適用するには再起動が必要です。"
-      },
-      "themeAppearance": {
-        "title": "テーマ",
-        "description": "アプリケーションのカラーテーマを選択します。",
-        "auto": "システムに従う",
-        "light": "ライト",
-        "dark": "ダーク"
-      }
-    }
-  },

   "settings": {
     "options": {
       "transparentWindow": {
         "title": "透明ウィンドウ効果",
         "description": "ウィンドウのネイティブなビブランシー/透明効果を有効にします。適用するには再起動が必要です。"
+      },
+      "themeAppearance": {
+        "title": "テーマ",
+        "description": "アプリケーションのカラーテーマを選択します。",
+        "auto": "システムに従う",
+        "light": "ライト",
+        "dark": "ダーク"
       }
     }
   },

🤖 Prompt for AI Agents

In `@src/i18n/ja.i18n.json` around lines 226 - 240, There are duplicate top-level
"settings" objects causing invalid/ambiguous JSON; remove the duplicate and
merge "themeAppearance" into the existing "settings.options" block so both
"transparentWindow" and "themeAppearance" live under a single
"settings.options"; ensure you keep all keys ("title", "description", "auto",
"light", "dark") and remove the redundant "settings" wrapper that contains
"themeAppearance", updating any consumers if they relied on the removed
duplicate structure.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect logging initialization for Electron API usage at module init time
echo "=== Checking logging files and main.ts for Electron API usage ===" 
rg -n "configureLogging|setupWebContentsLogging|app\.getPath|logger|ipcMain" src/logging/ src/main.ts 2>/dev/null | head -100

echo -e "\n=== Checking src/logging/index.ts structure ===" 
if [ -f src/logging/index.ts ]; then
  wc -l src/logging/index.ts
  echo "--- First 80 lines ---"
  head -80 src/logging/index.ts
fi

echo -e "\n=== Checking src/logging/cleanup.ts structure ===" 
if [ -f src/logging/cleanup.ts ]; then
  wc -l src/logging/cleanup.ts
  echo "--- Content ---"
  cat src/logging/cleanup.ts
fi

echo -e "\n=== Checking setupWebContentsLogging location and implementation ===" 
rg -l "setupWebContentsLogging" src/ --type ts

echo -e "\n=== Checking main.ts lines 50-80 for context ===" 
if [ -f src/main.ts ]; then
  sed -n '50,80p' src/main.ts
fi

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 6353

---

🏁 Script executed:

#!/bin/bash
# Get full configureLogging function (lines 100-157)
echo "=== configureLogging function (lines 100-157) ===" 
sed -n '100,157p' src/logging/index.ts

echo -e "\n=== setupWebContentsLogging function (lines 161-270) ===" 
sed -n '161,270p' src/logging/index.ts

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 6304

---

Move logging initialization to after app.whenReady() or use defensive coding with fallback paths.

The code calls app.getPath('logs') (line 124 of src/logging/index.ts) at module load time via configureLogging(), which is invoked when logger is imported in src/main.ts (line 23)—before app.whenReady() (line 65). Similarly, setupWebContentsLogging() registers listeners on app.on('web-contents-created') before app readiness.

While error handling catches failures, this violates Electron initialization best practices. Either:

1. Defer configureLogging() to after app.whenReady(), or
2. Use defensive coding per project guidelines: wrap app.getPath('logs') with optional chaining and provide a fallback, e.g., app.getPath?.('logs') ?? '/tmp/logs' to maintain cross-platform test compatibility.

The same pattern should apply to cleanupOldLogs() in src/logging/cleanup.ts (line 16).

🤖 Prompt for AI Agents

In `@src/main.ts` around lines 57 - 62, The startup logging is initialized before
Electron is ready; either defer configureLogging() and cleanupOldLogs() until
after app.whenReady() or make them defensive: update configureLogging() (and
cleanupOldLogs()) to call app.getPath('logs') via optional chaining with a
fallback (e.g., app.getPath?.('logs') ?? fallbackPath) and ensure
setupWebContentsLogging() is registered after app.whenReady() (or guarded so it
uses app.on only if available); modify references to configureLogging,
cleanupOldLogs, and setupWebContentsLogging so they are invoked post-app
readiness or made safe for pre-ready import.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

glob npm package version 11 TypeScript types built-in

💡 Result:

Yes—glob v11 ships with built-in TypeScript types (you should import from glob directly, and it exposes types like the GlobOptions TypeScript interface). [2]

Because of that, you do not need @types/glob—it’s explicitly deprecated as a stub that says glob provides its own type definitions. [1]

What to do

• Install/use: npm i glob@^11 (current v11 line is 11.0.3). [3]
• If you previously installed @types/glob, remove it: npm rm @types/glob``. [1]

Sources: [1] [2] [3]

---

Remove the deprecated @types/glob package as glob 11.x includes built-in TypeScript types.

The glob package v11 ships with built-in TypeScript definitions. The @types/glob package is explicitly deprecated as a stub for versions that provide their own types. Since the codebase uses glob ~11.1.0, @types/glob should be removed.

🤖 Prompt for AI Agents

In `@workspaces/desktop-release-action/package.json` around lines 32 - 35, Remove
the deprecated dependency entry for "@types/glob" from package.json because glob
v11.x includes its own TypeScript types; locate the dependency line containing
"@types/glob": "~9.0.0" and delete it, leaving the "glob": "~11.1.0" (and any
related entries like "fast-glob") intact, then run your package manager
(npm/yarn/pnpm) to update lockfile and ensure TypeScript still compiles.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@docs/pre-release-process.md`:
- Around line 138-144: The document uses both "pre-release" and "prerelease";
pick a single spelling and make it consistent across the file—e.g., replace
occurrences in the header "### Switching to a prerelease channel (stable →
alpha/beta)" and all list items/mentions (search for "prerelease",
"pre-release", and "Prerelease") so every instance matches the chosen form;
update the header text and the step list accordingly.

In `@src/i18n/pt-BR.i18n.json`:
- Line 503: The Portuguese translation for the key "outlook" contains a missing
accent; update the value for the "outlook" key in src/i18n/pt-BR.i18n.json from
"Calendario Outlook" to "Calendário Outlook" (preserve spacing and
capitalization) so the string uses the correct accented character.
- Line 450: Update the value for the "logIcon" key in src/i18n/pt-BR.i18n.json
to correct the Portuguese accent: change "Icone do visualizador de logs" to
"Ícone do visualizador de logs" (preserve the key "logIcon" exactly).

🧹 Nitpick comments (3)

src/injected.ts (1)

502-521: Consider redacting the userId in the debug log for consistency.

The first log at line 505 correctly redacts the userId using hasUserId: !!userId, but the second log at line 520 exposes the actual userId value. For consistency and to avoid logging user identifiers (which could be a privacy concern in shared log files), consider redacting it here as well.

♻️ Suggested change

         console.log(
-          '[Rocket.Chat Desktop] Setting Outlook token for user:',
-          userId
+          '[Rocket.Chat Desktop] Setting Outlook token for user (userId present):',
+          !!userId
         );

docs/pre-release-process.md (1)

152-161: Clarify downgrade behavior.

The explanation is helpful, but the semver comparison statement could be clearer. Semver considers 4.12.0 greater than 4.12.0-alpha.2 because pre-release versions have lower precedence than their associated normal version. Consider adding a brief note for users unfamiliar with semver:

Per semantic versioning rules, a release version (4.12.0) is considered newer than its pre-release counterparts (4.12.0-alpha.x, 4.12.0-beta.x).

README.md (1)

246-249: Consider rewording for stylistic variety.

The static analysis tool flagged three consecutive sentences starting with "When". While the text is functionally clear, consider a slight reword for better flow.

💡 Suggested improvement

-When `isTrayIconEnabled` is enabled, the app will be hidden on close.
-When `isMinimizeOnCloseEnabled` is enabled, the app will be minimized on close.
-When both are disabled, the app will quit on close.
+If `isTrayIconEnabled` is enabled, the app will be hidden on close.
+If `isMinimizeOnCloseEnabled` is enabled, the app will be minimized on close.
+With both disabled, the app will quit on close.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Minor: Inconsistent spelling of "prerelease" vs "pre-release".

The document uses "Pre-release" (line 65) but "prerelease" here (lines 138, 144). Consider using consistent spelling throughout—either "pre-release" (hyphenated) or "prerelease" (one word).

📝 Suggested fix

-### Switching to a prerelease channel (stable → alpha/beta)
+### Switching to a pre-release channel (stable → alpha/beta)

 1. Open Settings > Enable Developer Mode
 2. Open About dialog
 3. Select the desired channel from dropdown
 4. Click "Check for Updates"
-5. The next prerelease version will be offered
+5. The next pre-release version will be offered

🧰 Tools

🪛 LanguageTool

[uncategorized] ~138-~138: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: ... Switching Channels ### Switching to a prerelease channel (stable → alpha/beta) 1. Open ...

(EN_WORD_COHERENCY)

---

[uncategorized] ~144-~144: Do not mix variants of the same word (‘prerelease’ and ‘pre-release’) within a single text.
Context: .... Click "Check for Updates" 5. The next prerelease version will be offered ### Switching ...

(EN_WORD_COHERENCY)

🤖 Prompt for AI Agents

In `@docs/pre-release-process.md` around lines 138 - 144, The document uses both
"pre-release" and "prerelease"; pick a single spelling and make it consistent
across the file—e.g., replace occurrences in the header "### Switching to a
prerelease channel (stable → alpha/beta)" and all list items/mentions (search
for "prerelease", "pre-release", and "Prerelease") so every instance matches the
chosen form; update the header text and the step list accordingly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Typo: Missing accent in Portuguese word.

"Calendario" should be "Calendário" (with accent mark).

📝 Proposed fix

-        "outlook": "Calendario Outlook",
+        "outlook": "Calendário Outlook",

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"outlook": "Calendario Outlook",
	"outlook": "Calendário Outlook",

🤖 Prompt for AI Agents

In `@src/i18n/pt-BR.i18n.json` at line 503, The Portuguese translation for the key
"outlook" contains a missing accent; update the value for the "outlook" key in
src/i18n/pt-BR.i18n.json from "Calendario Outlook" to "Calendário Outlook"
(preserve spacing and capitalization) so the string uses the correct accented
character.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/preload.ts (1)

35-51: Reset startInProgress on invoke failures to avoid a stuck preload.

If invoke('server-view/get-url') throws, the flag never resets and startup won’t retry.

🛠️ Suggested fix

-  const serverUrl = await invoke('server-view/get-url');
+  let serverUrl: string | undefined;
+  try {
+    serverUrl = await invoke('server-view/get-url');
+  } catch (error) {
+    console.error('[Rocket.Chat Desktop] Preload start failed:', error);
+    startInProgress = false;
+    setTimeout(start, 1000);
+    retryCount += 1;
+    return;
+  }

-  if (retryCount > 5) return;
+  if (retryCount > 5) {
+    startInProgress = false;
+    return;
+  }

🤖 Fix all issues with AI agents

In `@patches-src/`@ewsjs/xhr/src/ntlmProvider.ts:
- Around line 19-27: The constructor currently calls username.indexOf("\\") and
can throw if username is undefined; change the guard to use the sanitized value
(this.username) or test username for truthiness before calling indexOf, e.g.
check if (this.username && this.username.indexOf("\\") > 0) then split that
sanitized string to set this.username and this.domain (ensure you call split on
the same sanitized value rather than the original possibly undefined username).

In `@patches-src/`@ewsjs/xhr/src/xhrApi.ts:
- Around line 213-231: The current success check in the XhrApi response handler
only treats status === 200 as success and throws for other 2xx codes; update the
conditional around xhrResponse (the block that currently checks
xhrResponse.status === 200) to treat any 2xx status as success (e.g., status >=
200 && status < 300 or status.toString().startsWith('2')), returning
setupXhrResponse(xhrResponse) for 2xx and throwing setupXhrResponse(xhrResponse)
otherwise; keep existing logging about allowUntrustedCertificate and
getProxyOption() but ensure they run for all 2xx successes and that the failure
branch only runs for non-2xx statuses.

In `@src/logging/index.ts`:
- Around line 220-223: The injected script currently builds serverUrl by
manually escaping single quotes, which is unsafe; update the construction in
src/logging/index.ts to embed the dynamic serverUrl using
JSON.stringify(serverUrl) (and keep webContents.id reference like webContentsId)
so the string is safely escaped for quotes, backslashes, newlines and backticks;
replace the current serverUrl = '...'.replace(...) line with one that assigns
serverUrl via JSON.stringify(serverUrl) in the injected template, ensuring the
injected code uses the JSON-serialized value.

In `@src/logViewerWindow/log-viewer-window.tsx`:
- Around line 8-59: The local setupI18n function hardcodes fallbackLng and
causes the log viewer to ignore user locale; remove this local setupI18n and
instead import and call the shared renderer setupI18n (the shared function named
setupI18n) before rendering ThemedLogViewerWindow so the window uses the
app-wide locale; also delete the i18next.init block in this file to avoid double
initialization and ensure ThemedLogViewerWindow and any i18n-dependent
components rely on the imported setupI18n being awaited.

In `@src/public/log-viewer-window.html`:
- Line 5: The CSP meta tag (<meta http-equiv="Content-Security-Policy"
content="script-src 'self'" />) is too permissive because it only constrains
scripts; replace it with a stricter policy that starts from a safe baseline
(e.g., default-src 'none') and then explicitly allow only needed resource types:
script-src 'self'; style-src 'self' (and 'unsafe-inline' only if inline styles
are required); img-src 'self' data:; font-src 'self'; connect-src 'self' (or
none if not needed); frame-ancestors 'none'; and form-action 'none' — update the
meta element in log-viewer-window.html accordingly so all resource types are
explicitly restricted instead of relying solely on script-src.

🧹 Nitpick comments (3)

src/logViewerWindow/logViewerWindow.tsx (1)

498-501: Track the TODO for real-time streaming.

Happy to help draft the streaming implementation or open a follow-up issue.

src/logViewerWindow/ipc.ts (1)

237-268: Replace sync fs calls with async equivalents in the IPC handler to prevent UI stalls.

The handler uses fs.existsSync(), fs.mkdirSync(), and fs.statSync() which block the main thread. Since the function is already async, convert these to fs.promises methods:

⚡ Suggested async fs usage

-        if (!fs.existsSync(logPath)) {
+        const logExists = await fs.promises
+          .access(logPath, fs.constants.F_OK)
+          .then(() => true)
+          .catch(() => false);
+        if (!logExists) {
           if (!options?.filePath) {
             // Only create default log file if it doesn't exist
             const logDir = path.dirname(logPath);
-            if (!fs.existsSync(logDir)) {
-              fs.mkdirSync(logDir, { recursive: true });
-            }
+            await fs.promises.mkdir(logDir, { recursive: true });
             await writeFile(logPath, '');
           } else {
             return {
               success: false,
               error: 'Selected log file does not exist',
             };
           }
         }
@@
-        const stats = fs.statSync(logPath);
+        const stats = await fs.promises.stat(logPath);

src/logViewerWindow/log-viewer-window.tsx (1)

1-12: Align renderer entrypoint location and filename with project conventions.

This renderer entrypoint lives under src/logViewerWindow and uses a kebab-case filename. Consider moving it under src/ui/ and renaming to a camelCase filename (e.g., logViewerWindowEntry.tsx) to match project conventions and reduce future friction in tooling and discovery.

Based on learnings: Applies to src/ui/**/*.{tsx,ts} : For renderer process code, use React with functional components and hooks; place renderer code in src/ui/ directory.
As per coding guidelines: Use camelCase for file names and PascalCase for component file names.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Guard against undefined usernames before indexOf.

username can be null/undefined at runtime and would throw; use the sanitized value or guard first.

🛠️ Proposed fix

-    if (username.indexOf("\\") > 0) {
-      this.username = username.split("\\")[1];
-      this.domain = username.split("\\")[0].toUpperCase();
-    }
+    const rawUsername = this.username;
+    if (rawUsername.includes("\\")) {
+      const [domain, user] = rawUsername.split("\\");
+      this.username = user ?? '';
+      this.domain = (domain ?? '').toUpperCase();
+    }

🤖 Prompt for AI Agents

In `@patches-src/`@ewsjs/xhr/src/ntlmProvider.ts around lines 19 - 27, The
constructor currently calls username.indexOf("\\") and can throw if username is
undefined; change the guard to use the sanitized value (this.username) or test
username for truthiness before calling indexOf, e.g. check if (this.username &&
this.username.indexOf("\\") > 0) then split that sanitized string to set
this.username and this.domain (ensure you call split on the same sanitized value
rather than the original possibly undefined username).

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Treat all 2xx responses as success.

Non-200 success codes (e.g., 201/204) are currently treated as failures.

✅ Proposed fix

-      if (xhrResponse.status === 200) {
+      if (xhrResponse.status >= 200 && xhrResponse.status < 300) {

🤖 Prompt for AI Agents

In `@patches-src/`@ewsjs/xhr/src/xhrApi.ts around lines 213 - 231, The current
success check in the XhrApi response handler only treats status === 200 as
success and throws for other 2xx codes; update the conditional around
xhrResponse (the block that currently checks xhrResponse.status === 200) to
treat any 2xx status as success (e.g., status >= 200 && status < 300 or
status.toString().startsWith('2')), returning setupXhrResponse(xhrResponse) for
2xx and throwing setupXhrResponse(xhrResponse) otherwise; keep existing logging
about allowUntrustedCertificate and getProxyOption() but ensure they run for all
2xx successes and that the failure branch only runs for non-2xx statuses.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd /tmp && find . -name "src" -type d 2>/dev/null | head -5

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 57

---

🏁 Script executed:

find . -name "index.ts" -path "*/logging/*" 2>/dev/null

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 97

---

🏁 Script executed:

cat -n src/logging/index.ts | sed -n '210,235p'

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1347

---

🏁 Script executed:

cat -n src/logging/index.ts | sed -n '190,230p'

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1939

---

🏁 Script executed:

cat -n src/logging/index.ts | sed -n '200,210p'

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 494

---

🏁 Script executed:

cat -n src/logging/index.ts | sed -n '1,50p'

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1916

---

🏁 Script executed:

cat -n src/logging/index.ts | sed -n '250,280p'

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1328

---

🏁 Script executed:

cat -n src/logging/index.ts | sed -n '150,180p'

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1205

---

🏁 Script executed:

grep -n "executeJavaScript\|serverUrl" src/logging/index.ts | head -20

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 873

---

Escape serverUrl safely when embedding into the injected script.

Only escaping ' can still break the string on backslashes, newlines, or backticks. Use JSON.stringify() to safely embed dynamic strings in template literals.

🛡️ Suggested fix

-                const serverUrl = '${serverUrl.replace(/'/g, "\\'")}';
+                const serverUrl = ${JSON.stringify(serverUrl)};

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Get webContents ID and server URL for context
	const webContentsId = ${webContents.id};
	const serverUrl = '${serverUrl.replace(/'/g, "\\'")}';
	// Get webContents ID and server URL for context
	const webContentsId = ${webContents.id};
	const serverUrl = ${JSON.stringify(serverUrl)};

🧰 Tools

🪛 GitHub Check: CodeQL

[failure] 222-222: Incomplete string escaping or encoding
This does not escape backslash characters in the input.

🤖 Prompt for AI Agents

In `@src/logging/index.ts` around lines 220 - 223, The injected script currently
builds serverUrl by manually escaping single quotes, which is unsafe; update the
construction in src/logging/index.ts to embed the dynamic serverUrl using
JSON.stringify(serverUrl) (and keep webContents.id reference like webContentsId)
so the string is safely escaped for quotes, backslashes, newlines and backticks;
replace the current serverUrl = '...'.replace(...) line with one that assigns
serverUrl via JSON.stringify(serverUrl) in the injected template, ensuring the
injected code uses the JSON-serialized value.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Log viewer always renders in fallback language.

At Line 45, the local setupI18n hardcodes fallbackLng, so this window ignores the user’s configured language. Reuse the shared renderer setupI18n to keep locale behavior consistent across windows.

🔧 Suggested fix (reuse shared renderer setupI18n)

-import { fallbackLng, interpolation } from '../i18n/common';
-// Import and apply console override for the log viewer window renderer
-import '../logging/preload';
-import resources from '../i18n/resources';
+// Import and apply console override for the log viewer window renderer
+import '../logging/preload';
+import { setupI18n } from '../i18n/renderer';

-// Initialize i18n for this window
-const setupI18n = async () => {
-  // For now we'll use the fallback language (en)
-  const lng = fallbackLng;
-
-  await i18next.use(initReactI18next).init({
-    lng,
-    fallbackLng,
-    resources: {
-      [fallbackLng]: {
-        translation: await resources[fallbackLng](),
-      },
-    },
-    interpolation,
-    initImmediate: true,
-  });
-};
-
 const start = async () => {
   // Initialize i18n before rendering
   await setupI18n();

🤖 Prompt for AI Agents

In `@src/logViewerWindow/log-viewer-window.tsx` around lines 8 - 59, The local
setupI18n function hardcodes fallbackLng and causes the log viewer to ignore
user locale; remove this local setupI18n and instead import and call the shared
renderer setupI18n (the shared function named setupI18n) before rendering
ThemedLogViewerWindow so the window uses the app-wide locale; also delete the
i18next.init block in this file to avoid double initialization and ensure
ThemedLogViewerWindow and any i18n-dependent components rely on the imported
setupI18n being awaited.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Tighten CSP beyond script-src to restrict other resource types.

With only script-src, other resource types can still load from anywhere. A stricter CSP reduces exposure.

🔒 Suggested CSP hardening

-    <meta http-equiv="Content-Security-Policy" content="script-src 'self'" />
+    <meta
+      http-equiv="Content-Security-Policy"
+      content="default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; font-src 'self'; object-src 'none'; base-uri 'self'"
+    />

🤖 Prompt for AI Agents

In `@src/public/log-viewer-window.html` at line 5, The CSP meta tag (<meta
http-equiv="Content-Security-Policy" content="script-src 'self'" />) is too
permissive because it only constrains scripts; replace it with a stricter policy
that starts from a safe baseline (e.g., default-src 'none') and then explicitly
allow only needed resource types: script-src 'self'; style-src 'self' (and
'unsafe-inline' only if inline styles are required); img-src 'self' data:;
font-src 'self'; connect-src 'self' (or none if not needed); frame-ancestors
'none'; and form-action 'none' — update the meta element in
log-viewer-window.html accordingly so all resource types are explicitly
restricted instead of relying solely on script-src.

[github-actions]

Linux installer download

• rocketchat-4.12.0-alpha.2-linux-amd64.deb
• rocketchat-4.12.0-alpha.2-linux-amd64.snap
• rocketchat-4.12.0-alpha.2-linux-x86_64.AppImage

[github-actions]

Windows installer download

• rocketchat-4.12.0-alpha.2-win-x64.exe

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@patches-src/`@ewsjs/xhr/src/xhrApi.ts:
- Around line 407-441: The async handler passed to _promise.then can throw if
client(opt || options) rejects and that error isn't forwarded to the outer
reject; wrap the body of the async callback in a try-catch, catch any error from
client(...) (or other await operations) and call rejectWithError(reject, error)
(or reject(setupXhrResponse(error)) if consistent with surrounding logic) so the
outer promise is rejected; update the async callback used with _promise.then
(the function referencing client, this.stream, progressDelegate,
this.disconnect) to ensure all thrown errors are caught and forwarded.

In `@src/ipc/channels.ts`:
- Around line 61-64: The declared return type for the
'outlook-calendar/has-credentials' IPC channel is incorrect (currently boolean)
while its handler is async and returns Promise<boolean>; update the channel map
entry for 'outlook-calendar/has-credentials' to use () => Promise<boolean> so
the declaration matches the handler's return type and fixes the TypeScript
strict-mode error.

In `@src/outlookCalendar/ipc.ts`:
- Around line 1084-1117: When retrieving a token via
webContents.executeJavaScript before calling syncEventsWithRocketChatServer,
also fetch the current Meteor.userId() inside the same executeJavaScript call
and compare it to outlookCredentials.userId; if they differ or userId is falsy,
log a warning and abort the sync instead of calling
syncEventsWithRocketChatServer. Ensure the executeJavaScript call safely
accesses both Meteor._localStorage and Meteor.userId() (handle the case where
Meteor.userId is not callable or returns null), catch any errors from
executeJavaScript, and only set userAPIToken and proceed with
syncEventsWithRocketChatServer when both token and matching userId are
confirmed.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "xhrApi.ts" -type f

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 113

---

🏁 Script executed:

cat -n patches-src/@ewsjs/xhr/src/xhrApi.ts | sed -n '350,450p'

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 3793

---

🌐 Web query:

JavaScript Promise async function rejection handling in .then() callback

💡 Result:

If you pass an async function to .then(...), that handler always returns a promise. So:

• If the async handler returns a value, the promise returned by .then() fulfills with that value.
• If the async handler throws or awaits a rejected promise, the promise returned by .then() rejects with that error/reason. This is exactly the same rule as “a .then() handler that throws or returns a rejected promise makes the next promise reject”. [1]

So you handle the rejection on the promise returned by .then(), typically with .catch() (or the second argument to .then()):

doSomething()
  .then(async (value) => {
    const r = await mayReject(value); // if this rejects, the chain rejects
    return r;
  })
  .catch((err) => {
    // handles rejections from doSomething() OR from the async then-handler
    console.error(err);
  });

Or equivalently:

doSomething().then(
  async (value) => {
    const r = await mayReject(value);
    return r;
  },
  (err) => {
    // handles only doSomething() rejecting
    throw err; // rethrow if you want the chain to stay rejected
  }
);

Key detail: .then() returns a new promise representing both the original promise and completion of your handler, so any rejection inside the async handler propagates to the next link in the chain. [2][1] Using .catch() is just the standard way to attach a rejection handler to that chain. [3]

Sources: MDN on Promise.prototype.then() behavior and chaining [1], MDN guide “Using promises” (promise chains) [2], MDN on Promise.prototype.catch() [3].

---

Ensure xhrStream rejects when the axios call fails before stream setup.
If client(opt) rejects on line 409, the async handler throws but the rejection is not wired to the outer reject() function. The second parameter to .then() only handles rejections from _promise itself, not from the async handler. Without a catch handler, this creates an unhandled rejection and callers hang indefinitely.

Wrap the async logic in a try-catch block to forward errors:

Fix

-        async (opt) => {
-          const response = await client(opt || options);
-          this.stream = response.data;
+        async (opt) => {
+          try {
+            const response = await client(opt || options);
+            this.stream = response.data;
 
-          this.stream.on('error', (error) => {
-            progressDelegate({ type: 'error', error: error });
-            this.disconnect();
-            rejectWithError(reject, error);
-          });
+            this.stream.on('error', (error) => {
+              progressDelegate({ type: 'error', error: error });
+              this.disconnect();
+              rejectWithError(reject, error);
+            });
+          } catch (error) {
+            rejectWithError(reject, error);
+          }
         },

🤖 Prompt for AI Agents

In `@patches-src/`@ewsjs/xhr/src/xhrApi.ts around lines 407 - 441, The async
handler passed to _promise.then can throw if client(opt || options) rejects and
that error isn't forwarded to the outer reject; wrap the body of the async
callback in a try-catch, catch any error from client(...) (or other await
operations) and call rejectWithError(reject, error) (or
reject(setupXhrResponse(error)) if consistent with surrounding logic) so the
outer promise is rejected; update the async callback used with _promise.then
(the function referencing client, this.stream, progressDelegate,
this.disconnect) to ensure all thrown errors are caught and forwarded.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Search for the handler implementation for outlook-calendar/has-credentials
rg "outlook-calendar/has-credentials" -n -B 3 -A 3

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1525

---

🏁 Script executed:

# Find the handler registration in outlookCalendar/ipc.ts
fd "ipc.ts" | xargs grep -l "outlook-calendar" 2>/dev/null

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 101

---

🏁 Script executed:

# Look for handler registration patterns
rg "handle\(" -A 5 | grep -A 5 "has-credentials" 2>/dev/null || rg "ipcMain" -A 3 | grep -B 3 -A 3 "has-credentials"

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 439

---

Return type mismatch for outlook-calendar/has-credentials.

The handler is async and returns Promise<boolean> (see src/outlookCalendar/ipc.ts:1027), but the channel map declares the return type as boolean. This causes a TypeScript strict mode type error.

🔧 Fix

-'outlook-calendar/has-credentials': () => boolean;
+'outlook-calendar/has-credentials': () => Promise<boolean>;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	'outlook-calendar/get-events': (date: Date) => OutlookEventsResponse;
	'outlook-calendar/set-exchange-url': (url: string, userId: string) => void;
	'outlook-calendar/has-credentials': () => Promise<boolean>;
	'outlook-calendar/has-credentials': () => boolean;
	'outlook-calendar/clear-credentials': () => void;
	'outlook-calendar/get-events': (date: Date) => OutlookEventsResponse;
	'outlook-calendar/set-exchange-url': (url: string, userId: string) => void;
	'outlook-calendar/has-credentials': () => Promise<boolean>;
	'outlook-calendar/clear-credentials': () => void;

🤖 Prompt for AI Agents

In `@src/ipc/channels.ts` around lines 61 - 64, The declared return type for the
'outlook-calendar/has-credentials' IPC channel is incorrect (currently boolean)
while its handler is async and returns Promise<boolean>; update the channel map
entry for 'outlook-calendar/has-credentials' to use () => Promise<boolean> so
the declaration matches the handler's return type and fixes the TypeScript
strict-mode error.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd src/outlookCalendar && wc -l ipc.ts

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 86

---

🏁 Script executed:

# First, let's read the context around the specified lines with surrounding code
sed -n '1050,1150p' src/outlookCalendar/ipc.ts | cat -n

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 4282

---

🏁 Script executed:

# Also check the outlookCredentials structure definition
rg -n "outlookCredentials" src/outlookCalendar/ipc.ts | head -20

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1062

---

🏁 Script executed:

# Search for userId validation patterns in the file
rg -n "userId" src/outlookCalendar/ipc.ts -A 2 -B 2 | head -40

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1447

---

🏁 Script executed:

# Search for other executeJavaScript calls to see what APIs are used
rg -n "executeJavaScript" src/outlookCalendar/ipc.ts -A 3 -B 1

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 292

---

🏁 Script executed:

# Check the syncEventsWithRocketChatServer function to see if it validates userId
rg -n "syncEventsWithRocketChatServer" src/outlookCalendar/ipc.ts -A 15 | head -50

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1834

---

🏁 Script executed:

# Search for how Meteor APIs are accessed in webContents context elsewhere
rg -n "Meteor\." src/outlookCalendar/ --type ts

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 175

---

🏁 Script executed:

# Check if Meteor.userId is used anywhere in the project
rg -n "Meteor\.userId" . --type ts --type tsx 2>/dev/null | head -20

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 57

---

🏁 Script executed:

# Also check what Meteor APIs are accessible - search for Meteor usage patterns
rg -n "Meteor\._localStorage" . --type ts --type tsx 2>/dev/null

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 57

---

🏁 Script executed:

# Check if there's any documentation or comments about what Meteor APIs are available in webContents
rg -n "webContents" src/outlookCalendar/ipc.ts -B 5 -A 10 | grep -A 10 -B 10 "executeJavaScript"

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 997

---

🏁 Script executed:

# Check the OutlookCredentials type definition to understand the structure
rg -n "type OutlookCredentials\|interface OutlookCredentials" src/ --type ts

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 57

---

🏁 Script executed:

# Check if there's any session/user validation in the sync process
rg -n "sessionToken\|authorization\|Bearer" src/outlookCalendar/ipc.ts -B 2 -A 2

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 57

---

🏁 Script executed:

# Look for how the token is used in syncEventsWithRocketChatServer to see if there's implicit userId check
sed -n '425,550p' src/outlookCalendar/ipc.ts | cat -n

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 4460

---

🏁 Script executed:

# Let's check if there's any implicit validation through the API calls
# Look at listEventsFromRocketChatServer to see how it uses userId and token
rg -n "listEventsFromRocketChatServer" src/outlookCalendar/ipc.ts -A 30 | head -60

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1966

---

🏁 Script executed:

# Check if token format includes userId info (JWT or similar)
rg -n "loginToken\|authToken" src/outlookCalendar/ipc.ts -B 2 -A 2

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 57

---

Validate that the retrieved token belongs to the expected user before syncing.

Without an explicit userId check, a stale session in webContents could result in an authentication mismatch. While the API call includes both token and userId headers for server-side validation, adding a client-side check prevents unnecessary API calls and provides better error feedback.

Note: The suggested implementation should verify that Meteor.userId() is callable in the webContents.executeJavaScript() context, as this pattern is not currently used elsewhere in the codebase (only Meteor._localStorage is accessed via executeJavaScript).

🤖 Prompt for AI Agents

In `@src/outlookCalendar/ipc.ts` around lines 1084 - 1117, When retrieving a token
via webContents.executeJavaScript before calling syncEventsWithRocketChatServer,
also fetch the current Meteor.userId() inside the same executeJavaScript call
and compare it to outlookCredentials.userId; if they differ or userId is falsy,
log a warning and abort the sync instead of calling
syncEventsWithRocketChatServer. Ensure the executeJavaScript call safely
accesses both Meteor._localStorage and Meteor.userId() (handle the case where
Meteor.userId is not callable or returns null), catch any errors from
executeJavaScript, and only set userAPIToken and proceed with
syncEventsWithRocketChatServer when both token and matching userId are
confirmed.

[github-actions]

macOS installer download

• rocketchat-4.12.0-alpha.2-mac.dmg
• rocketchat-4.12.0-alpha.2-mac.pkg