fix: open conference deep links in the video call window - #3412
Conversation
Conference deep links (rocketchat://conference?...&path=conference/<id>?callUrl=...&callProvider=...) opened the call inside the server webview because performConference navigated the webview with loadURL, replacing the workspace UI underneath the call. Route the request to the standalone video call window instead — the same path used when a conference is joined from inside the app: - performConference reads callUrl/callProvider from the deep link and sends 'conference/open-call-requested' to the server webview (no navigation). Links without callUrl keep the previous loadURL fallback. - A new preload listener (listenToConferenceCallRequests) forwards the request to the existing openInternalVideoChatWindow. The listener is registered early and buffers a single request until the renderer store is ready (flushPendingConferenceCallRequest), so a deep link that cold-launches the app is not dropped before the listener exists. - Pexip credential enrichment is applied only when the callUrl origin matches the workspace origin, so a crafted deep link cannot cause the workspace session token to be handed to an unrelated URL loaded in the call window.
|
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:
WalkthroughConference deep links now dispatch call requests through IPC when needed, with preload buffering during cold start and same-origin provider handling. Pull request workflows also include ChangesConference call deep-link routing
Workflow and release metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant processDeepLink
participant webContents
participant preload
participant openInternalVideoChatWindow
User->>processDeepLink: open conference deep link
processDeepLink->>webContents: send conference/open-call-requested with callUrl
webContents->>preload: deliver callUrl and provider
preload->>openInternalVideoChatWindow: process call URL and provider options
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
Replace a real customer lab hostname in the conference deep-link test data with a generic example host.
![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
🤖 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/preload/internalVideoChatWindow.ts`:
- Around line 38-59: Update processConferenceCallRequest so the catch block for
malformed callUrl returns immediately, preventing the invalid URL from reaching
openInternalVideoChatWindow; preserve the existing same-origin provider
enrichment for valid URLs.
🪄 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: 23e30138-2bcb-4212-87c8-b5085c18119f
📒 Files selected for processing (5)
src/deepLinks/main.spec.tssrc/deepLinks/main.tssrc/preload.tssrc/servers/preload/__tests__/internalVideoChatWindow.spec.tssrc/servers/preload/internalVideoChatWindow.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from@rocket.chat/fuselagefor UI work unless the design requires something Fuselage does not provide.
CheckTheme.d.tsfor valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local.d.tsfiles instead of assuming.
Use React functional components with hooks.
Redux actions follow FSA shape.
Use camelCase for file names and PascalCase for components.
Prefer clear names over unnecessary comments.
Prefer editing existing files over creating new abstractions unless the new abstraction removes real complexity or matches an existing pattern.
**/*.{ts,tsx}: Use TypeScript for all new code unless explicitly told otherwise.
Use Fuselage components for all UI work; create custom components only when Fuselage lacks the required functionality.
Import Fuselage components from@rocket.chat/fuselage.
Use only valid color tokens documented byTheme.d.ts.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs such asprocess.getuid(),getgid(),geteuid(), andgetegid().
Use TypeScript strict mode.
Redux actions must follow the Flux Standard Action pattern.
Use camelCase for file names and PascalCase for component names.
Avoid unnecessary comments; prefer self-documenting code through clear naming.
Do not commit or push without explicit user permission.
Verify library APIs, props, tokens, and types against official documentation and.d.tsfiles instead of assuming they are valid.
Files:
src/servers/preload/__tests__/internalVideoChatWindow.spec.tssrc/deepLinks/main.tssrc/preload.tssrc/deepLinks/main.spec.tssrc/servers/preload/internalVideoChatWindow.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Renderer specs use
*.spec.ts/*.spec.tsx.
Files:
src/servers/preload/__tests__/internalVideoChatWindow.spec.tssrc/deepLinks/main.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
*.spec.tsfor renderer process tests.
Files:
src/servers/preload/__tests__/internalVideoChatWindow.spec.tssrc/deepLinks/main.spec.ts
src/**/*.{spec.ts,spec.tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Renderer test files should be placed in nested module paths such as
src/<module>/<subdir>/*.spec.ts(x)so Jest discovers them.
Files:
src/servers/preload/__tests__/internalVideoChatWindow.spec.tssrc/deepLinks/main.spec.ts
🔇 Additional comments (5)
src/deepLinks/main.ts (1)
213-233: LGTM!src/deepLinks/main.spec.ts (1)
651-726: LGTM!src/servers/preload/internalVideoChatWindow.ts (1)
64-70: LGTM!Also applies to: 72-108
src/servers/preload/__tests__/internalVideoChatWindow.spec.ts (1)
1-134: LGTM!src/preload.ts (1)
9-12: LGTM!Also applies to: 38-42, 80-80
The PR build and validate workflows only triggered for PRs targeting master or dev, so hotfix PRs got no CI or build artifacts. Add hotfix/** to the pull_request branch filters so any hotfix line gets the same CI and build-artifacts coverage.
3c0dc90 to
4759fde
Compare
Linux installer download |
macOS installer download |
Conference deep links come in two shapes. Links carrying an explicit callUrl already work: the workspace webview loads the conference page, which forwards the callUrl to the desktop video call window. Those are left untouched. Links without a callUrl (conference/<id>) instead loaded the conference page over the workspace webview, trapping the user on it with no way back. Route those to the standalone video call window so closing it returns the user to an untouched workspace.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 222-237: The conference deep-link routing contract is reversed. In
src/deepLinks/main.ts lines 222-237, dispatch conference/open-call-requested
when url.searchParams.has('callUrl'), and use webContents.loadURL as the
fallback when callUrl is absent. In src/deepLinks/main.spec.ts lines 679-719,
invert the test names and assertions to verify this routing behavior.
🪄 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: e20c4645-84f1-439d-9966-1891e1e2e57c
📒 Files selected for processing (3)
src/deepLinks/main.spec.tssrc/deepLinks/main.tssrc/servers/preload/__tests__/internalVideoChatWindow.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/servers/preload/tests/internalVideoChatWindow.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
- GitHub Check: build (macos-latest, mac)
- GitHub Check: check (macos-latest)
- GitHub Check: build (ubuntu-latest, linux)
- GitHub Check: build (windows-latest, windows)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from@rocket.chat/fuselagefor UI work unless the design requires something Fuselage does not provide.
CheckTheme.d.tsfor valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local.d.tsfiles instead of assuming.
Use React functional components with hooks.
Redux actions follow FSA shape.
Use camelCase for file names and PascalCase for components.
Prefer clear names over unnecessary comments.
Prefer editing existing files over creating new abstractions unless the new abstraction removes real complexity or matches an existing pattern.
**/*.{ts,tsx}: Use TypeScript for all new code unless explicitly told otherwise.
Use Fuselage components for all UI work; create custom components only when Fuselage lacks the required functionality.
Import Fuselage components from@rocket.chat/fuselage.
Use only valid color tokens documented byTheme.d.ts.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs such asprocess.getuid(),getgid(),geteuid(), andgetegid().
Use TypeScript strict mode.
Redux actions must follow the Flux Standard Action pattern.
Use camelCase for file names and PascalCase for component names.
Avoid unnecessary comments; prefer self-documenting code through clear naming.
Do not commit or push without explicit user permission.
Verify library APIs, props, tokens, and types against official documentation and.d.tsfiles instead of assuming they are valid.
Files:
src/deepLinks/main.spec.tssrc/deepLinks/main.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Renderer specs use
*.spec.ts/*.spec.tsx.
Files:
src/deepLinks/main.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
*.spec.tsfor renderer process tests.
Files:
src/deepLinks/main.spec.ts
src/**/*.{spec.ts,spec.tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Renderer test files should be placed in nested module paths such as
src/<module>/<subdir>/*.spec.ts(x)so Jest discovers them.
Files:
src/deepLinks/main.spec.ts
1 participant
What changed
Conference deep links (
rocketchat://conference?...&path=conference/<id>?callUrl=...&callProvider=...) opened the call inside the server webview, replacing the workspace UI.performConferencenavigated the webview withloadURL, so the conference page loaded over the whole workspace.They now open in the standalone video call window — the same window used when a conference is joined from inside the app.
How
performConferencereadscallUrl/callProviderfrom the deep link and sendsconference/open-call-requestedto the server webview instead of navigating it. Links withoutcallUrlkeep the previousloadURLfallback, so nothing regresses for older-style links.listenToConferenceCallRequests) forwards the request to the existingopenInternalVideoChatWindow. It registers early and buffers a single request until the renderer store is ready (flushPendingConferenceCallRequest), so a deep link that cold-launches the app isn't dropped before the listener exists.callUrlorigin matches the workspace origin, so a crafted deep link can't cause the workspace session token to be handed to an unrelated URL loaded in the call window.Testing
Requires the
build-artifactsinstaller build.1. Fix path — opens in call window (main case):
Use a real Pexip conference deep link from the QA workspace — one whose
pathcarries acallUrlandcallProvider=Pexip, i.e. of the shape:Expected: the call opens in the separate video call window; the workspace stays visible behind it (not replaced).
2. Fallback path — no
callUrl:Expected: previous behavior preserved (navigates the webview to the conference route).
3. Security check: a conference deep link whose
callUrlpoints to a host different from the workspace origin must not carry Pexip credentials into the call window.Automated checks
tsc --noEmit: 0 errorsyarn test(deepLinks/main.spec.ts + servers/preload/tests/internalVideoChatWindow.spec.ts): 42 passedFollow-up (webapp, out of scope here)
The web client's
/conference/:idroute ignores its:idparam and trusts the deep-linkcallUrloutright. A server-trustedvideoConference.joinCall(callId)endpoint already exists; wiringConferencePageto resolve the URL from:idvia that endpoint would retire deep-linkcallUrltrust entirely.Summary by CodeRabbit
rocketchat://conferencerouting to open calls directly when acallUrlis provided.callUrlno longer attempt in-view navigation; they use the standard call-opening flow instead.hotfix/**branches.4.15.4.