Click-to-Call: Dial Phone Numbers Directly from Any App#3325
Click-to-Call: Dial Phone Numbers Directly from Any App#3325jeanfbrito wants to merge 19 commits into
Conversation
Register Rocket.Chat as OS handler for callto: and tel: URL schemes on Windows, macOS, and Linux. When a telephony link is clicked in any app, RC launches or focuses and dispatches a typed IPC event to the server webview with the parsed phone number. - Register callto/tel schemes in electron-builder.json (all platforms) - Add parseTelephonyLink() with number normalization and callto:// support - Add performTelephonyCall() with multi-server dialog + remember choice - Expose onTelephonyCallRequested callback on RocketChatDesktop API - Persist telephonyPreferredServer via selectPersistableValues - IPC listener registered before onReady to avoid cold-start race
Tests cover parseTelephonyLink (tel:/callto: protocols, number normalization, callto:// double-slash format, extension syntax, edge cases) and performTelephonyCall (0/1/2+ servers, preferred server persistence, dialog remember checkbox).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds tel:/callto: deep-link handling: builder protocol config, multi-scheme registration, telephony URI parsing, server-selection and preference persistence, preload IPC plumbing to deliver telephony requests to renderers, UI settings and i18n, supported-version git-hash handling, and Jest tests. ChangesTelephony Deep Links Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labelstype: feature 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/telephony/preload.ts (1)
18-29: ⚡ Quick winGuard
listenToTelephonyRequestsagainst duplicate registration.Calling
listenToTelephonyRequests()more than once (e.g., in a hot-reload dev cycle or defensive initialization) stacks multipleipcRenderer.onhandlers. Each subsequenttelephony/call-requestedevent would fire all of them, invokingtelephonyCallbackmultiple times or repeatedly overwritingpendingPayload.♻️ Proposed fix — idempotency guard
+let isListening = false; + export const listenToTelephonyRequests = (): void => { + if (isListening) return; + isListening = true; ipcRenderer.on( 'telephony/call-requested', (_event, payload: TelephonyPayload) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/telephony/preload.ts` around lines 18 - 29, listenToTelephonyRequests currently registers an ipcRenderer.on handler every time it's called, causing duplicate handlers; make it idempotent by guarding registration: add a module-level flag (e.g., isTelephonyListenerRegistered) or remove existing listeners for 'telephony/call-requested' before adding, then only call ipcRenderer.on if not already registered; keep the handler logic using telephonyCallback and pendingPayload unchanged and set the flag to true after successful registration (or rely on removeAllListeners + add to ensure a single handler).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/deepLinks/main.ts`:
- Around line 110-117: The dialog.showMessageBox call is missing the parent
window argument so the prompt isn't attached as a modal; update the call in
src/deepLinks/main.ts to pass the app's root window as the first argument (same
pattern used by askForServerAddition and warnAboutInvalidServerUrl) so the
dialog becomes a sheet/modal attached to the main window, keeping the existing
options ({ type, title, message, buttons, checkboxLabel, checkboxChecked }) as
the second parameter.
---
Nitpick comments:
In `@src/telephony/preload.ts`:
- Around line 18-29: listenToTelephonyRequests currently registers an
ipcRenderer.on handler every time it's called, causing duplicate handlers; make
it idempotent by guarding registration: add a module-level flag (e.g.,
isTelephonyListenerRegistered) or remove existing listeners for
'telephony/call-requested' before adding, then only call ipcRenderer.on if not
already registered; keep the handler logic using telephonyCallback and
pendingPayload unchanged and set the flag to true after successful registration
(or rely on removeAllListeners + add to ensure a single handler).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 705c516b-f939-4295-87b5-cd4aab10137e
📒 Files selected for processing (12)
electron-builder.jsonsrc/app/main/app.tssrc/app/selectors.tssrc/deepLinks/main.spec.tssrc/deepLinks/main.tssrc/preload.tssrc/servers/preload/api.tssrc/store/actions.tssrc/store/rootReducer.tssrc/telephony/actions.tssrc/telephony/preload.tssrc/telephony/reducers.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
- GitHub Check: check (macos-latest)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript for all new code in this codebase unless explicitly told otherwise
Use Fuselage components from@rocket.chat/fuselagefor all UI work — only create custom components when Fuselage doesn't provide the needed functionality
Check Theme.d.ts for valid color tokens when working with Fuselage components
Use optional chaining with fallbacks for platform-specific APIs instead of mocks (e.g., process.getuid?.() ?? 1000) to ensure code works across all platforms without requiring mocks
TypeScript code must use strict mode
Use React functional components with hooks instead of class components
Redux actions must follow the FSA (Flux Standard Action) pattern
Use camelCase for file naming
Use PascalCase for component file names (React components)
Write self-documenting code through clear naming — avoid unnecessary comments
Files:
src/store/actions.tssrc/telephony/reducers.tssrc/app/selectors.tssrc/telephony/preload.tssrc/preload.tssrc/store/rootReducer.tssrc/telephony/actions.tssrc/app/main/app.tssrc/servers/preload/api.tssrc/deepLinks/main.spec.tssrc/deepLinks/main.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use *.spec.ts file naming convention for Renderer process tests
Files:
src/deepLinks/main.spec.ts
**/*.{spec,main.spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{spec,main.spec}.ts: Tests must run and pass on Windows, macOS, and Linux CI environments — always verify cross-platform compatibility
Only mock Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) when defensive coding with optional chaining isn't possible
Files:
src/deepLinks/main.spec.ts
🔇 Additional comments (10)
src/telephony/actions.ts (1)
1-5: Clean action typing for preferred server state.The constant and payload map are consistent and correctly model set/clear (
string | null) behavior.src/store/actions.ts (1)
12-12: Root action map integration is correct.Including the telephony payload map in the global action type composition is done cleanly and keeps
RootActiontyping coherent.Also applies to: 30-31
src/telephony/reducers.ts (1)
10-21: Reducer logic is straightforward and correct.Initial state, set behavior, and reset support via
nullpayload are all implemented as expected.src/store/rootReducer.ts (1)
21-21: Root reducer wiring for telephony state looks good.The new slice is properly registered and will be reflected in
RootState.Also applies to: 122-122
src/app/selectors.ts (1)
86-87: Persistable selector update is aligned with state changes.Including
telephonyPreferredServerhere keeps persisted state in sync with the new reducer slice.electron-builder.json (1)
5-8: Protocol packaging metadata update is consistent.The new telephony schemes are correctly declared both in app protocol config and Linux MIME handlers.
Also applies to: 126-126
src/preload.ts (1)
11-11: Preload telephony listener wiring is solid.The listener is integrated at an appropriate point in the preload startup sequence.
Also applies to: 68-68
src/app/main/app.ts (1)
43-47: Multi-scheme protocol registration is implemented correctly.Flattening all configured schemes and registering each one at startup matches the new
electron-builder.jsonprotocol format.Also applies to: 90-92
src/servers/preload/api.ts (1)
17-17: LGTM!Clean extension of
ExtendedIRocketChatDesktopfollowing the established pattern. The type signature correctly matchessrc/telephony/preload.ts's export.Also applies to: 53-55, 102-102
src/deepLinks/main.spec.ts (1)
131-335: LGTM! Thorough coverage across all server-selection branches.The
performTelephonyCalltests correctly sequencemockReturnValueOncecalls to separately control theserversandtelephonyPreferredServerselectors, and the polling test placesjest.useRealTimers()before the assertions so timers are always restored.
…duplicate IPC listener - Pass getRootWindow() as first argument to dialog.showMessageBox so the server selection prompt appears as a modal sheet attached to the main window (consistent with all other dialogs in the codebase) - Add idempotency guard to listenToTelephonyRequests to prevent duplicate IPC handler registration during hot-reload dev cycles
Add GitNexus section with impact analysis, query, and context tools. Gitignore .gitnexus index directory.
Dispatch WEBVIEW_GIT_COMMIT_HASH_CHANGED from server info response. Match supportedVersions exceptions using sha:<hash> prefix against the server's git commit hash for per-build version overrides.
Add TelephonyServer component to Settings > General tab with a Select dropdown to choose which server handles tel:/callto: links. Hidden when only one server exists. "Auto (ask each time)" option clears the preference and reverts to dialog behavior.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/servers/supportedVersions/main.ts (2)
237-238: 💤 Low valueRemove redundant trim() call.
The
gitCommitHashvariable is already trimmed on line 232, so the.trim()call on line 238 is redundant.♻️ Proposed cleanup
const normalizedGitCommitHash = gitCommitHash - .trim() .replace(/^sha-/, '') .toLowerCase();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/servers/supportedVersions/main.ts` around lines 237 - 238, The assignment to normalizedGitCommitHash unnecessarily calls .trim() again; remove the redundant .trim() so normalizedGitCommitHash is set directly from the already-trimmed gitCommitHash (update the line that defines normalizedGitCommitHash in main.ts to use gitCommitHash without calling .trim()).
237-240: ⚡ Quick winConsider case-insensitive prefix removal for robustness.
While
server.gitCommitHashis unlikely to have an uppercase "SHA-" prefix in practice, using a case-insensitive regex makes the code more defensive and consistent with Git's case-insensitive treatment of commit hashes.♻️ Proposed improvement
const normalizedGitCommitHash = gitCommitHash .trim() - .replace(/^sha-/, '') + .replace(/^sha-/i, '') .toLowerCase();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/servers/supportedVersions/main.ts` around lines 237 - 240, Update the normalization of gitCommitHash in normalizedGitCommitHash to remove a leading "sha-" prefix case-insensitively: change the regex used on gitCommitHash.trim().replace(/^sha-/, '') to a case-insensitive variant (e.g., use the /i flag) so any "SHA-", "Sha-", etc. prefixes are stripped consistently before toLowerCase(); keep the rest of the flow intact.src/servers/supportedVersions/main.main.spec.ts (1)
838-908: ⚡ Quick winConsider adding test coverage for case variations.
While the current tests cover the core sha-prefix functionality, adding tests for case variations (e.g., "SHA-bb83777" or uppercase commit hashes) would improve robustness—especially if the case-sensitivity issues in
main.tsare addressed.📋 Suggested test cases
it('should support uppercase SHA- prefix in exception versions', async () => { const futureDate = new Date(Date.now() + 86400000); const supportedVersions: SupportedVersions = { enforcementStartDate: new Date(Date.now() - 86400000).toISOString(), timestamp: new Date().toISOString(), versions: [{ version: '8.4.0', expiration: futureDate }], exceptions: { domain: 'open.rocket.chat', uniqueId: 'test-unique-id', versions: [{ version: 'SHA-bb83777', expiration: futureDate }], }, }; const result = await isServerVersionSupported( { url: 'https://open.rocket.chat/', version: '8.5', title: 'Rocket.Chat Open', gitCommitHash: 'bb83777b51a42d', } as any, supportedVersions ); expect(result.supported).toBe(true); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/servers/supportedVersions/main.main.spec.ts` around lines 838 - 908, Add tests that cover case variations for SHA-prefixed exception matching: create additional it(...) cases in the same spec that pass exception versions like 'SHA-bb83777' and commit hashes in uppercase (e.g., 'BB83777B51A42D') to ensure isServerVersionSupported correctly normalizes/matches case; reference the existing test setup (the SupportedVersions object and the isServerVersionSupported call) and duplicate the pattern used in the two existing tests but change the exception version prefix casing and commit-hash casing, asserting supported === true for matching variations and supported === false for malformed/non-matching variations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/servers/supportedVersions/main.ts`:
- Line 221: The current if check using
trimmedExceptionVersion.startsWith('sha-') is case-sensitive and will miss
prefixes like "SHA-"; update the condition in the if that references
trimmedExceptionVersion to perform a case-insensitive check (e.g., compare
trimmedExceptionVersion.toLowerCase().startsWith('sha-') or use a
case-insensitive regex) so any "sha-" prefix in any case is recognized as a
git-hash exception.
In `@src/ui/components/SettingsView/features/TelephonyServer.tsx`:
- Around line 37-43: The options useMemo is calling new URL(s.url).hostname
which can throw on malformed URLs and crash rendering; update the mapping inside
useMemo (or compute options after the component's early return) to safely
extract hostname by catching URL parsing errors or validating s.url first—e.g.,
wrap the hostname extraction in a try/catch (or use a small helper like
safeHostname(s.url)) and fall back to s.url or an empty string if parsing fails;
ensure this change references the existing useMemo/options mapping and s.url to
avoid crashing the component.
---
Nitpick comments:
In `@src/servers/supportedVersions/main.main.spec.ts`:
- Around line 838-908: Add tests that cover case variations for SHA-prefixed
exception matching: create additional it(...) cases in the same spec that pass
exception versions like 'SHA-bb83777' and commit hashes in uppercase (e.g.,
'BB83777B51A42D') to ensure isServerVersionSupported correctly
normalizes/matches case; reference the existing test setup (the
SupportedVersions object and the isServerVersionSupported call) and duplicate
the pattern used in the two existing tests but change the exception version
prefix casing and commit-hash casing, asserting supported === true for matching
variations and supported === false for malformed/non-matching variations.
In `@src/servers/supportedVersions/main.ts`:
- Around line 237-238: The assignment to normalizedGitCommitHash unnecessarily
calls .trim() again; remove the redundant .trim() so normalizedGitCommitHash is
set directly from the already-trimmed gitCommitHash (update the line that
defines normalizedGitCommitHash in main.ts to use gitCommitHash without calling
.trim()).
- Around line 237-240: Update the normalization of gitCommitHash in
normalizedGitCommitHash to remove a leading "sha-" prefix case-insensitively:
change the regex used on gitCommitHash.trim().replace(/^sha-/, '') to a
case-insensitive variant (e.g., use the /i flag) so any "SHA-", "Sha-", etc.
prefixes are stripped consistently before toLowerCase(); keep the rest of the
flow intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3c2f774-98af-42e8-bd5b-bfd1d5a03ff7
📒 Files selected for processing (7)
.gitignoreCLAUDE.mdsrc/i18n/en.i18n.jsonsrc/servers/supportedVersions/main.main.spec.tssrc/servers/supportedVersions/main.tssrc/ui/components/SettingsView/GeneralTab.tsxsrc/ui/components/SettingsView/features/TelephonyServer.tsx
✅ Files skipped from review due to trivial changes (3)
- src/ui/components/SettingsView/GeneralTab.tsx
- .gitignore
- src/i18n/en.i18n.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check (macos-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: check (windows-latest)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Fuselage components for all UI work; only create custom components when Fuselage doesn't provide what's needed. Import from
@rocket.chat/fuselageCheck Theme.d.ts for valid color tokens when using Fuselage
Use optional chaining with fallbacks for platform-specific APIs instead of mocking (e.g., const uid = process.getuid?.() ?? 1000)
Use TypeScript strict mode
Use React functional components with hooks
Use camelCase for file naming
No unnecessary comments — self-documenting code through clear naming
Files:
src/ui/components/SettingsView/features/TelephonyServer.tsxsrc/servers/supportedVersions/main.tssrc/servers/supportedVersions/main.main.spec.ts
**/*[A-Z]*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use PascalCase for component files
Files:
src/ui/components/SettingsView/features/TelephonyServer.tsx
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript for all new code unless explicitly told otherwise
Files:
src/servers/supportedVersions/main.tssrc/servers/supportedVersions/main.main.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Name test files with *.spec.ts for renderer process tests
Files:
src/servers/supportedVersions/main.main.spec.ts
**/*.main.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Name test files with *.main.spec.ts for main process tests
Files:
src/servers/supportedVersions/main.main.spec.ts
**/*.{spec,main.spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Tests run on Windows, macOS, AND Linux CI — always verify cross-platform compatibility
Files:
src/servers/supportedVersions/main.main.spec.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: NEVER run `yarn build` directly in workspace directories — always use root commands
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: After building desktop-release-action, remove nested dist with: rm -rf workspaces/desktop-release-action/dist/dist
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: Code signing for Windows builds uses Google Cloud KMS in two phases: build packages without signing first, then sign built packages using jsign
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: Prefer editing existing files over creating new ones
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: NEVER commit or push without explicit user permission — 'fix this' does NOT mean 'commit it'
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: NEVER commit directly to master or dev — create a branch, test, open a PR
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: Use worktrees to avoid disrupting the user's working directory when making changes
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: Understand WHY code is written that way before changing it — working code is correct until proven otherwise
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: Verify your work by running tests, checking types with npx tsc --noEmit, and demonstrating correctness
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: Diagnose before iterating — analyze WHY approaches fail before trying the next one
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: Always verify libraries by checking official docs and .d.ts files in node_modules/ — never assume props, tokens, or APIs
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: Avoid subjective descriptors like 'smart', 'excellent', 'dumb' — use measurable descriptions in writing
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: Never invent metrics in PR descriptions or documentation — only include numbers from actual logs, error messages, or documented sources
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: PR descriptions should use straightforward language and focus on what changed and why
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: MUST run impact analysis with gitnexus_impact before editing any symbol
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: MUST run gitnexus_detect_changes before committing to verify changes only affect expected symbols
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: MUST warn the user if impact analysis returns HIGH or CRITICAL risk before proceeding with edits
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: NEVER edit a function, class, or method without first running gitnexus_impact on it
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: NEVER ignore HIGH or CRITICAL risk warnings from impact analysis
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-09T02:27:13.470Z
Learning: NEVER rename symbols with find-and-replace — use gitnexus_rename which understands the call graph
🪛 LanguageTool
CLAUDE.md
[style] ~118-~118: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...pected symbols and execution flows. - MUST warn the user if impact analysis retu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~126-~126: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...L risk warnings from impact analysis. - NEVER rename symbols with find-and-replace — ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~127-~127: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...mewhich understands the call graph. - NEVER commit changes without runninggitnexu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (9)
CLAUDE.md (1)
107-149: Solid addition: clear operational guardrails for GitNexus usage.This section is actionable and well-structured (required steps, anti-patterns, and quick links), and it aligns with the workflow expectations for safe symbol changes and commit validation.
src/servers/supportedVersions/main.ts (3)
22-22: LGTM!Import of
WEBVIEW_GIT_COMMIT_HASH_CHANGEDis properly used in the dispatch call at line 392.
390-398: LGTM!The conditional dispatch of
WEBVIEW_GIT_COMMIT_HASH_CHANGEDcorrectly uses optional chaining to check forinfo.commit?.hashbefore dispatching, and the payload structure is consistent with Redux action patterns.
312-312: LGTM!The refactoring to use
isVersionExceptionForServerhelper improves code organization and enables the new git-hash exception matching functionality.src/servers/supportedVersions/main.main.spec.ts (5)
14-14: LGTM!Import is correctly used in the test assertion at line 197.
179-203: LGTM!Comprehensive test coverage for the git commit hash dispatch behavior. The test properly mocks server info with a commit hash and verifies the action payload structure.
796-796: LGTM!Changing
expirationto aDateinstance improves type consistency with the expectedSupportedVersionsinterface.
838-872: LGTM!Excellent test coverage for the sha-prefixed exception matching feature. The test correctly verifies that a server with matching git commit hash is recognized as supported when an exception entry with
sha-prefix exists.
874-908: LGTM!Good negative test case verifying that malformed (empty) exception versions don't incorrectly match.
electron-builder v26 rejects MimeType as a direct child of linux.desktop — only desktopActions and entry are valid properties. Move it inside desktop.entry where it belongs.
Replace hardcoded English strings in the telephony dialog with i18n t() calls and add telephonySelectServer translation keys to all 22 locale files.
jeanfbrito
changed the title
jeanfbrito
changed the title
mOSdat PR Verification — #3325 feat/telephony-deeplinkScope: 39 files, +1023 lines — new protocol handler ( Build Results
Smoke Tests (Display Scenarios)Both ubuntu2404 (GNOME Wayland) and fedora42 (GNOME Wayland) — identical results:
3 PASS, 0 FAIL, 2 EXPECTED — identical on both distros. SummaryWindows smoke test blocked by missing cross-compile toolchain (wine). No regressions detected on Linux. |
…selection Replace dialog.showMessageBox with an in-app modal that shows server favicons, names and hostnames — matching the sidebar appearance. Scales to many servers via a scrollable list and includes a "remember this choice" checkbox. Also hardens the telephony flow: - Mutex prevents concurrent tel: links from opening duplicate modals - 120s timeout on modal promise prevents hanging if renderer crashes - 10s timeout on webContents polling prevents infinite loop if server is removed between selection and view creation
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/deepLinks/main.ts`:
- Around line 83-85: The code currently normalizes the telephony URI using the
raw value from url.pathname/url.href but doesn't decode percent-encoded
characters; update the logic that builds raw/phoneNumber so that you first run
decodeURIComponent on the raw input (derived from url.pathname or
url.href.slice(url.protocol.length)) before applying replace(/^\/+/, '') and
.replace(/[\s\-().]/g, '') so percent-encoded characters like %2B become "+";
adjust variables raw and phoneNumber in src/deepLinks/main.ts accordingly.
In `@src/ui/components/TelephonyServerSelectModal/index.tsx`:
- Around line 65-75: The checkbox label isn't programmatically associated with
the control: add an id to the CheckBox (e.g., "telephony-server-remember") and
set that id as htmlFor on the Box (which is acting as the label) so assistive
tech recognizes the relationship; then remove the Box onClick toggle (leave the
CheckBox onChange using setRememberChoice and keep the checked={rememberChoice})
to avoid double-toggling while preserving the existing rememberChoice and
setRememberChoice state handlers.
In `@src/ui/components/TelephonyServerSelectModal/ServerItem.tsx`:
- Around line 39-55: The interactive row in ServerItem is not
keyboard-accessible; update the Tile element (in ServerItem) to render as a
native button by adding is='button' (and type='button') so it receives keyboard
focus and activates with Enter/Space, keep the existing onClick={handleClick}
and hover handlers (setIsHovered/isHovered) intact, and ensure any custom styles
don't remove the native focus outline so keyboard users can see focus.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d79ae606-5abc-4b78-babf-4f36179624b2
📒 Files selected for processing (7)
src/deepLinks/main.spec.tssrc/deepLinks/main.tssrc/ui/actions.tssrc/ui/components/Shell/index.tsxsrc/ui/components/TelephonyServerSelectModal/ServerItem.tsxsrc/ui/components/TelephonyServerSelectModal/index.tsxsrc/ui/reducers/dialogs.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/deepLinks/main.spec.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
MANDATORY: Use Fuselage components from
@rocket.chat/fuselagefor all UI work. Only create custom components when Fuselage doesn't provide what's needed.Check Theme.d.ts for valid color tokens when using Fuselage components.
Files:
src/ui/components/TelephonyServerSelectModal/index.tsxsrc/ui/components/Shell/index.tsxsrc/ui/actions.tssrc/ui/components/TelephonyServerSelectModal/ServerItem.tsxsrc/ui/reducers/dialogs.tssrc/deepLinks/main.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use optional chaining with fallbacks for platform-specific APIs to ensure cross-platform compatibility without mocks: e.g.,
process.getuid?.() ?? 1000Use TypeScript strict mode.
Use camelCase for file naming.
No unnecessary comments — self-documenting code through clear naming.
Always verify libraries — check official docs and
.d.tsfiles innode_modules/. Never assume props, tokens, or APIs work without verification.Avoid subjective descriptors ('smart', 'excellent', 'dumb') in code comments and documentation.
Use measurable descriptions in documentation and comments: 'reduced memory usage', 'improved by X%' instead of subjective terms.
Never invent metrics in code comments or documentation — no estimated time spent, no speculated user counts. Only include numbers from actual logs, error messages, or documented sources.
Files:
src/ui/components/TelephonyServerSelectModal/index.tsxsrc/ui/components/Shell/index.tsxsrc/ui/actions.tssrc/ui/components/TelephonyServerSelectModal/ServerItem.tsxsrc/ui/reducers/dialogs.tssrc/deepLinks/main.ts
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use React functional components with hooks.
Use PascalCase for component files.
Files:
src/ui/components/TelephonyServerSelectModal/index.tsxsrc/ui/components/Shell/index.tsxsrc/ui/components/TelephonyServerSelectModal/ServerItem.tsx
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
TypeScript codebase. Use TypeScript for all new code unless explicitly told otherwise.
Files:
src/ui/actions.tssrc/ui/reducers/dialogs.tssrc/deepLinks/main.ts
**/*action*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Redux actions must follow FSA (Flux Standard Action) pattern.
Files:
src/ui/actions.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: NEVER run `yarn build` directly in workspace directories — always use root commands to avoid creating incorrect output structures.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: After building `desktop-release-action`, remove the nested dist: `rm -rf workspaces/desktop-release-action/dist/dist` — the action only needs `workspaces/desktop-release-action/dist/index.js`.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: NEVER add `ewsjs/xhr` patches to `patches/` directory — use `.yarn/patches/` instead as configured in package.json to avoid CI failures due to conflicts.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Use patch-package for patching `kayahr/jest-electron-runner` with patches stored in `patches/` directory.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Always include all architectures for Windows builds: x64, ia32, arm64.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Code signing uses Google Cloud KMS in two phases: 1) Build packages without signing (empty env vars), 2) Sign built packages using jsign with Google Cloud KMS to prevent MSI build failures.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Prefer editing existing files over creating new ones.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: NEVER commit or push without explicit user permission — 'fix this' does NOT mean 'commit it'.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: NEVER commit directly to master or dev — create a branch, test, open a PR.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Read operations on git (status, diff, log) are always fine.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Show what will be committed before committing.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Use git worktrees to avoid disrupting the user's working directory when creating feature branches.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Understand before changing — understand WHY code is written that way. Working code is correct until proven otherwise. If unsure, ASK.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Verify your work — run tests, check types (`npx tsc --noEmit`), demonstrate correctness. Never mark a task done without proving it works.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Diagnose before iterating — when approaches fail, analyze WHY before trying the next one. Don't cycle through 3+ approaches blindly.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: PR descriptions should use straightforward language and focus on what changed and why.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: MUST run impact analysis before editing any symbol using gitnexus_impact to understand blast radius.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: MUST run `gitnexus_detect_changes()` before committing to verify changes only affect expected symbols.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: MUST warn the user if impact analysis returns HIGH or CRITICAL risk before proceeding with edits.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: Use gitnexus_query to find execution flows instead of grepping when exploring unfamiliar code.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron
Timestamp: 2026-05-12T00:50:52.404Z
Learning: NEVER rename symbols with find-and-replace — use `gitnexus_rename` which understands the call graph.
🔇 Additional comments (3)
src/ui/actions.ts (1)
159-160: LGTM!Also applies to: 312-319
src/ui/components/Shell/index.tsx (1)
20-20: LGTM!Also applies to: 106-106
src/ui/reducers/dialogs.ts (1)
4-9: LGTM!Also applies to: 24-33, 35-40, 65-83, 100-108
Drop Margins wrapper from the modal and the Tile container from rows. Title→message margin x8→x4, message→list x16→x12, rows now use paddingBlock x6 / paddingInline x8 instead of Tile padding x12.
…ver modal Adds 20 tests across three new spec files covering the telephony deep-link runtime path that was previously only validated by deepLinks/main.spec.ts. - src/telephony/renderer/preload.spec.ts (6 tests): IPC bridge state machine — listenToTelephonyRequests guard, pendingPayload buffer/replay, callback replacement, ipcRenderer.on registration. - src/ui/components/SettingsView/features/TelephonyServer.spec.tsx (8 tests): Settings dropdown — hide when servers.length <= 1, option generation (auto + per-server), value binding to telephonyPreferredServer, dispatch of TELEPHONY_PREFERRED_SERVER_SET (null for auto, URL string otherwise), hostname fallback when server title missing. - src/ui/components/TelephonyServerSelectModal/index.spec.tsx (6 tests): Modal flow — visibility gating on dialogs.telephonyServerSelect.isOpen, ServerItem rendering per server, dispatch payload shape on click with rememberChoice on/off, close dispatch with null payload, rememberChoice reset after close. Adds @testing-library/react, @testing-library/jest-dom, and @testing-library/dom (peer) as devDependencies. Fuselage Select and Dialog are mocked at module level since they rely on React-Aria and native <dialog>.showModal() respectively, which don't drive cleanly in @kayahr/jest-electron-runner's renderer environment. Spec paths follow the existing renderer testMatch convention: src/<module>/<subdir>/<file>.spec.tsx — a flat src/telephony/preload.spec.ts would be silently dropped by jest's testMatch globs.
tel:%2B15551234 left %2B encoded, producing phoneNumber '%2B15551234' instead of '+15551234'. decodeURIComponent runs before strip pass; malformed escapes return null (treated same as other invalid input).
…tive Git commit hashes are conventionally case-insensitive. SHA-bb83777 should match same as sha-bb83777.
- TelephonyServer: extract hostname via safeHostname helper to prevent settings page crash on malformed server URLs (new URL() throws). - TelephonyServerSelectModal: associate 'Remember this choice' label with checkbox via htmlFor/id for assistive tech. - ServerItem: render Tile as native button (is='button' type='button') so keyboard users get Tab focus and Enter/Space activation.
* feat(telephony): add global shortcut to dial clipboard number * fix(telephony): harden global shortcut handling * refactor(telephony): share dialpad opener * test(telephony): stabilize shortcut notification click
Linux installer download |
macOS installer download |
2 participants
Summary
You can now dial phone numbers from outside Rocket.Chat Desktop in two ways: click a phone link (
tel:orcallto:), or copy text that looks like a phone number and press a configurable global keyboard shortcut. Rocket.Chat opens or focuses, routes the request to the selected workspace, and opens the dial pad with the number ready to dial. If you have multiple workspaces, the app can remember which workspace handles telephony, and Settings lets you update or clear that preference at any time.What's New
Added
tel:orcallto:) in any application opens Rocket.Chat with the number pre-populated in the call widgetsha:<hash>prefix to match against a server's git commit hash, enabling per-build overridesImproved
rocketchat://protocol on all platformsPlatform Notes
tel:links; global shortcut registration uses Electron's platform supportxdg-openon both X11 and Wayland; global shortcut registration uses Electron's platform supportHow to Test
Click-to-Call (App Running)
tel:+1234567890in the address bar, or use one of these direct examples:Click-to-Call (App Closed)
tel:orcallto:phone link in any applicationGlobal Shortcut
+1 (800) 555-0199Multi-Workspace Selection
Settings UI
tel:link or trigger the telephony shortcut — verify it routes directly without dialogtel:link or trigger the telephony shortcut — verify the dialog appears againVarious Link Formats
Related
Summary by CodeRabbit
New Features
Localization
Tests