Show RDP for Linux and FreeBSD peers and harden the browser RDP client - #710
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe RDP client adds Linux and FreeBSD entry points, OS-specific credentials, Windows display-control resizing, session recovery, structured IronRDP errors, and pinned v0.0.2 release artifacts. ChangesRDP client behavior
IronRDP release pinning
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RDPPage
participant useRemoteDesktop
participant IronRDPWASMBridge
participant RDPSession
RDPPage->>useRemoteDesktop: start RDP with dynamicResize
useRemoteDesktop->>IronRDPWASMBridge: connect(enableDisplayControl)
IronRDPWASMBridge->>RDPSession: start session
useRemoteDesktop->>IronRDPWASMBridge: resize(sessionId, width, height)
IronRDPWASMBridge->>RDPSession: resize(width, height)
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: one or more packages not found in the registry. 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: 2
🧹 Nitpick comments (2)
src/modules/remote-access/rdp/ironrdp-wasm-bridge.ts (2)
159-160: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVerify the
"display_control"extension identifier matches the pinned IronRDP wasm build.Devolutions/IronRDP's own architecture docs refer to this capability as
displayControl(camelCase) when describing the backend-extension mechanism, while this code registersnew this.ironrdp.Extension("display_control", true)(snake_case). If the pinnedironrdp-web/iron-remote-desktop-rdpv0.0.2 build expects a different literal string,Extensionconstruction/registration would silently no-op or throw, and Windows in-session resize (the feature this branch exists for) would quietly fall back to not resizing, with no error surfaced to the user.Please confirm the exact extension key accepted by the pinned wasm build (e.g., check
iron-remote-desktop-rdp's Extension factory source for v0.0.2).Also,
connect()now takes 10 positional parameters (hostname, port, username, password, domain, canvas, enableClipboard, netbirdClient, onSessionEnd, enableDisplayControl). This is fragile to reorder mistakes at the single call site. Consider bundling the trailing options into a single object parameter.Also applies to: 193-201
🤖 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/modules/remote-access/rdp/ironrdp-wasm-bridge.ts` around lines 159 - 160, Verify the extension key used by the pinned IronRDP wasm v0.0.2 build against its Extension factory, then update the registration in connect() to use the exact accepted identifier while preserving display-control resizing. Also replace connect()’s trailing positional options with a named options object, and update its sole call site to pass enableClipboard, netbirdClient, onSessionEnd, and enableDisplayControl by name.
83-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnum-index mapping to
kind()is fragile across wasm builds.
IRON_ERROR_KIND_NAMEShard-codes IronErrorKind ordering by array position. If the pinnedironrdp-webpackage (or a future bump of it) reorders/adds variants,kind()will silently map to the wrong human-readable message (or "Unknown" inlogIronError) with no compile-time signal. The pin to v0.0.2 mitigates near-term drift, but this coupling is worth guarding with a small unit test asserting array length/order against a known-good sample, so a future version bump surfaces the mismatch instead of silently mislabeling errors.Also applies to: 400-400
🤖 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/modules/remote-access/rdp/ironrdp-wasm-bridge.ts` around lines 83 - 106, The IRON_ERROR_KIND_NAMES array relies on fragile numeric ordering from IronRDP’s wasm kind() method. Add a focused unit test covering its expected length and variant order for the pinned ironrdp-web version, so future enum changes fail visibly; keep the existing mapping and IRON_ERROR_KIND_MESSAGES behavior unchanged.
🤖 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/modules/remote-access/rdp/ironrdp-wasm-bridge.ts`:
- Around line 251-268: Update the startSession flow and its consumer so
superseded sessions cannot reset active connection state: before invoking
onSessionEnd, verify the session/sessionId still matches the current session
tracked by useRemoteDesktop.ts, or suppress the callback when it does not.
Preserve cleanup and error reporting for every settled run while preventing
stale callbacks from calling resetState().
In `@src/modules/remote-access/rdp/useRemoteDesktop.ts`:
- Around line 193-197: Update the onSessionEnd callback in useRemoteDesktop to
capture the session id associated with the current run and compare it with the
active session id before calling resetState or setError. Ignore callbacks from
stale sessions, while preserving existing error handling for the active session.
---
Nitpick comments:
In `@src/modules/remote-access/rdp/ironrdp-wasm-bridge.ts`:
- Around line 159-160: Verify the extension key used by the pinned IronRDP wasm
v0.0.2 build against its Extension factory, then update the registration in
connect() to use the exact accepted identifier while preserving display-control
resizing. Also replace connect()’s trailing positional options with a named
options object, and update its sole call site to pass enableClipboard,
netbirdClient, onSessionEnd, and enableDisplayControl by name.
- Around line 83-106: The IRON_ERROR_KIND_NAMES array relies on fragile numeric
ordering from IronRDP’s wasm kind() method. Add a focused unit test covering its
expected length and variant order for the pinned ironrdp-web version, so future
enum changes fail visibly; keep the existing mapping and
IRON_ERROR_KIND_MESSAGES behavior unchanged.
🪄 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: 1cb4ff8c-a5bc-4209-a083-a21965545769
📒 Files selected for processing (6)
.github/workflows/build_and_push.ymlsrc/app/(remote-access)/peer/rdp/page.tsxsrc/modules/remote-access/rdp/RDPButton.tsxsrc/modules/remote-access/rdp/RDPCredentialsModal.tsxsrc/modules/remote-access/rdp/ironrdp-wasm-bridge.tssrc/modules/remote-access/rdp/useRemoteDesktop.ts
|
Note: this needs the ironrdp release v0.0.2 which is not yet there |
…tbirdio#717 并汉化 - merge netbirdio/main: Agent Network clusters/RDP/features 更新 - 新增 14 个翻译 key (配置页/Agent Network 通知/RDP 弹窗) - agent-network/configuration page 全面汉化 - RDPCredentialsModal 全面汉化 - NetBirdCloudProvider toast 汉化
…tbirdio#717 并汉化 - merge netbirdio/main: Agent Network clusters/RDP/features 更新 - 新增 14 个翻译 key (配置页/Agent Network 通知/RDP 弹窗) - agent-network/configuration page 全面汉化 - RDPCredentialsModal 全面汉化 - NetBirdCloudProvider toast 汉化
…tbirdio#717 并汉化 - merge netbirdio/main: Agent Network clusters/RDP/features 更新 - 新增 14 个翻译 key (配置页/Agent Network 通知/RDP 弹窗) - agent-network/configuration page 全面汉化 - RDPCredentialsModal 全面汉化 - NetBirdCloudProvider toast 汉化
…tbirdio#717 并汉化 - merge netbirdio/main: Agent Network clusters/RDP/features 更新 - 新增 14 个翻译 key (配置页/Agent Network 通知/RDP 弹窗) - agent-network/configuration page 全面汉化 - RDPCredentialsModal 全面汉化 - NetBirdCloudProvider toast 汉化
…tbirdio#717 并汉化 - merge netbirdio/main: Agent Network clusters/RDP/features 更新 - 新增 14 个翻译 key (配置页/Agent Network 通知/RDP 弹窗) - agent-network/configuration page 全面汉化 - RDPCredentialsModal 全面汉化 - NetBirdCloudProvider toast 汉化
…tbirdio#717 并汉化 - merge netbirdio/main: Agent Network clusters/RDP/features 更新 - 新增 14 个翻译 key (配置页/Agent Network 通知/RDP 弹窗) - agent-network/configuration page 全面汉化 - RDPCredentialsModal 全面汉化 - NetBirdCloudProvider toast 汉化
2 participants
Issue ticket number and link
Fixes #616
Shows the RDP button for Linux and FreeBSD peers (previously Windows-only) and hardens the browser RDP client's error handling and resize behavior.
Documentation
Select exactly one:
The docs already state RDP has no OS restriction; this aligns the UI with that.
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
E2E tests
Optional: override the image tags used by the Playwright e2e workflow.
Defaults to
mainwhen omitted.management-cloud-tag: main
reverse-proxy-tag: main
Summary by CodeRabbit
New Features
Bug Fixes