PoC #130 & #208: uinput virtual devices + WebRTC streaming#329
Open
Muneerali199 wants to merge 5 commits intoAOSSIE-Org:mainfrom
Open
PoC #130 & #208: uinput virtual devices + WebRTC streaming#329Muneerali199 wants to merge 5 commits intoAOSSIE-Org:mainfrom
Muneerali199 wants to merge 5 commits intoAOSSIE-Org:mainfrom
Conversation
…quired (Issue AOSSIE-Org#97) - useClipboardSync.ts: React hook that sends clipboard-push / clipboard-pull messages over WebSocket; falls back to execCommand('insertText') when navigator.clipboard is unavailable (plain HTTP / TanStack router limitation) - clipboard-server.ts: server handler that reads/writes host clipboard via clipboardy (cross-platform) and emits Ctrl+C for the copy path - Addresses the core constraint in issue AOSSIE-Org#97: no HTTPS dependency
…WebRTC streaming PoC AOSSIE-Org#130: Replace NutJS with uinput virtual input devices - Added src/server/uinput.ts with Linux uinput device driver - Supports mouse movement, clicks, scroll, and keyboard input - Falls back to ydotool → NutJS on failure - Fixes Wayland cursor desync issue PoC AOSSIE-Org#208: WebRTC for screen streaming without FPS cap - Added useWebRTCCaptureProvider.ts (provider side) - Added useWebRTCStream.ts (consumer side) - Updated websocket.ts for WebRTC signaling relay - Hardware-accelerated encoding, targets 60fps Both PoCs required for GSoC 2026 Rein proposal
Contributor
WalkthroughAdds bidirectional clipboard sync, WebRTC-based screen capture/streaming (provider + consumer hooks), server-side clipboard relay, and a Linux uinput virtual-input backend with InputHandler reworked to prefer uinput with fallbacks. Changes
Sequence DiagramssequenceDiagram
participant Client
participant WS as WebSocket
participant Host as RemoteHost
Client->>WS: {type:"clipboard-push", text}
WS->>Host: handleClipboardMessage -> write host clipboard (clipboardy)
Host->>Host: Simulate modifier+C keypress/release
Client->>WS: {type:"clipboard-pull"}
WS->>Host: read host clipboard (clipboardy)
Host->>WS: {type:"clipboard-text", text}
WS->>Client: Receives clipboard-text
Client->>Client: navigator.clipboard.writeText or insertTextFallback
Client->>Client: onPasteReceived callback
sequenceDiagram
participant ProviderClient as Provider
participant WS as WebSocket
participant ConsumerClient as Consumer
Provider->>Provider: getDisplayMedia() -> MediaStream
Provider->>Provider: create RTCPeerConnection, add tracks
Provider->>WS: {type:"webrtc-offer", sdp}
Provider->>WS: {type:"webrtc-ice", candidate} (as discovered)
WS->>Consumer: relay offer
Consumer->>Consumer: setRemoteDescription, createAnswer
Consumer->>WS: {type:"webrtc-answer", sdp}
WS->>Provider: relay answer
Consumer->>WS: {type:"webrtc-ice", candidate}
WS->>Provider: relay ICE candidates
Provider->>Consumer: PeerConnection established, media flows
Consumer->>Consumer: ontrack -> draw frames to canvas
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Updated DesktopCaptureProvider to try WebRTC first, fall back to capture provider - Updated ScreenMirror to use WebRTC stream when connected - Both PoCs now work end-to-end with the existing UI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/server/InputHandler.ts (2)
114-117:⚠️ Potential issue | 🟡 MinorCopy-paste error in log message.
The error message says "pending move event" but this code handles scroll events.
Proposed fix
this.handleMessage(pending).catch((err) => { - console.error("Error processing pending move event:", err) + console.error("Error processing pending scroll event:", err) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/InputHandler.ts` around lines 114 - 117, The console error message incorrectly mentions a "pending move event" while this block handles scroll events; update the log in the catch of the this.handleMessage(pending) call (near pendingScroll and handleMessage) to refer to "pending scroll event" or include pendingScroll context so the error message accurately reflects the scroll event being processed.
156-188: 🧹 Nitpick | 🔵 TrivialDuplicated button mapping logic in click handler.
The button-to-enum mapping is duplicated in both the uinput fallback path (lines 163-168) and the catch block (lines 178-183). Consider extracting to a helper function.
Proposed refactor
+ private mapButtonToNut(button: "left" | "right" | "middle"): Button { + return button === "left" + ? Button.LEFT + : button === "right" + ? Button.RIGHT + : Button.MIDDLE + } + // In the click handler: case "click": { const VALID_BUTTONS = ["left", "right", "middle"] if (msg.button && VALID_BUTTONS.includes(msg.button)) { try { const uinputSuccess = await pressButtonUinput(msg.button, msg.press) if (!uinputSuccess) { - const btn = - msg.button === "left" - ? Button.LEFT - : msg.button === "right" - ? Button.RIGHT - : Button.MIDDLE + const btn = this.mapButtonToNut(msg.button) if (msg.press) { await mouse.pressButton(btn) } else { await mouse.releaseButton(btn) } } } catch (err) { console.error("Click event failed:", err) - const btn = - msg.button === "left" - ? Button.LEFT - : msg.button === "right" - ? Button.RIGHT - : Button.MIDDLE + const btn = this.mapButtonToNut(msg.button) await mouse.releaseButton(btn).catch(() => {}) } } break }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/InputHandler.ts` around lines 156 - 188, The click handler duplicates button-to-enum mapping; extract the mapping logic into a small helper (e.g., a function mapButton(msgButton: string): Button) and use it in both the uinput fallback path and the catch block instead of repeating the ternary chain. Update the case "click" code to call mapButton(msg.button) before calling pressButtonUinput, mouse.pressButton, and mouse.releaseButton, and handle unknown/invalid button values by guarding early (since VALID_BUTTONS is already checked).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useClipboardSync.ts`:
- Around line 1-19: The file defining the useClipboardSync hook is missing the
Next.js client-side directive; add the "use client" directive as the very first
line of the file so the hook (useClipboardSync and its use of
navigator.clipboard, document.activeElement, window.getSelection, etc.) runs on
the client; ensure the directive precedes all imports and comments so React's
client-side behavior is enabled for the hook.
- Around line 50-52: The non-null assertion on msg.text causes a Biome lint
failure; capture the validated string in a local variable before the async
callback and use that variable instead. Inside the block where you already
checked msg.text is a string, add const text = msg.text and replace
navigator.clipboard.writeText(msg.text).catch(() =>
insertTextFallback(msg.text!)) with navigator.clipboard.writeText(text).catch(()
=> insertTextFallback(text)), referencing msg.text,
navigator.clipboard.writeText, and insertTextFallback to locate and update the
code.
In `@src/hooks/useWebRTCCaptureProvider.ts`:
- Around line 5-8: The file declares an unused interface named RTCMessage;
either remove the RTCMessage declaration to eliminate dead code, or apply it to
the message handling code (e.g., annotate incoming/outgoing message variables
and the message event handler in useWebRTCCaptureProvider to use RTCMessage for
type-safe handling of "offer" | "answer" | "ice-candidate" payloads). Update or
remove the interface so there are no unused types reported.
- Around line 119-138: The current handleMessage in useEffect incorrectly
returns early if peerConnectionRef.current?.remoteDescription is set, which
blocks processing of subsequent webrtc-ice messages; update handleMessage (used
in useWebRTCCaptureProvider) so the early return only applies when handling a
"webrtc-answer" (i.e., if data.type === "webrtc-answer" and remoteDescription
exists, skip applying the answer), and always allow the "webrtc-ice" branch to
run so new RTCIceCandidate messages are added via
peerConnectionRef.current?.addIceCandidate; keep the JSON.parse and try/catch
but relocate the remoteDescription guard into the webrtc-answer branch.
- Around line 39-111: The startSharing flow (startSharing) never registers this
client as a provider before sending the webrtc-offer, so the server won't set
isProvider and answers/ICE will be misrouted; before creating/sending the offer
(and after confirming wsRef.current?.readyState === WebSocket.OPEN) send a
"start-provider" message via wsRef.current.send (matching the server's provider
registration contract) so the server marks this client as a provider; ensure
this send happens before the webrtc-offer send (and keep stopSharing behavior
that sends "stop-webrtc-provider" intact) so subsequent webrtc-answer and ICE
messages are routed back correctly.
In `@src/hooks/useWebRTCStream.ts`:
- Around line 47-71: processFrame currently allocates a new HTMLCanvasElement
and 2D context every frame (inefficient); instead create and reuse a single
canvas (prefer OffscreenCanvas when available) and its context outside/in the
hook scope (e.g., processingCanvas or processingCanvasRef) and only update its
width/height when video.videoWidth/video.videoHeight change, then in
processFrame call ctx.drawImage(video,0,0) and
createImageBitmap(processingCanvas) as before; ensure you stop recreating ctx
and canvas each frame and still close previous frameRef bitmap and manage
rAFRef/remoteStreamRef as existing.
- Around line 129-142: The effect should not depend on the mutable ref property
wsRef.current?.readyState; remove that expression from the dependency array and
instead either (A) attach 'open' and 'close' event listeners to wsRef.current
inside the effect and call startStream/stopStream from those listeners (keep
startStream, stopStream, status in deps), or (B) lift the socket ready state
into React state (e.g. wsOpen) updated by socket events and include that state
variable in the dependency array; update useEffect to rely on status and the
stable indicator (event-driven listeners or wsOpen) rather than
wsRef.current?.readyState. Ensure you reference wsRef, useEffect, startStream,
stopStream, and status when making the change.
In `@src/server/clipboard-server.ts`:
- Around line 69-86: The handler for "clipboard-push" in
src/server/clipboard-server.ts writes incoming text with writeHostClipboard and
then emits Ctrl+C (via keyboard.pressKey(modifier, Key.C) and subsequent
releaseKey calls), which overwrites the clipboard; remove the entire Ctrl+C
emission block (the try/finally and Promise.allSettled releasing Key.C and
modifier) so only writeHostClipboard(text) is performed and the function still
returns true, leaving any modifier/Key imports unused or removing them if no
longer referenced.
In `@src/server/uinput.ts`:
- Line 231: setupKeyboardEvents uses undefined constants KEY_KPMULT and
KEY_KPDIV causing runtime errors; fix by adding proper definitions or importing
them where constants are declared (e.g., from the same keyboard/keycode source
used for other KEY_* constants) and reference those symbols in
setupKeyboardEvents. Locate setupKeyboardEvents in this file, then either (a)
add const KEY_KPMULT = <correct keycode> and const KEY_KPDIV = <correct keycode>
alongside the other KEY_* definitions, or (b) import KEY_KPMULT and KEY_KPDIV
from the module supplying other key constants so the names resolve at runtime.
Ensure the names match usages in setupKeyboardEvents exactly.
- Around line 137-140: The ui_user_dev function is incomplete (it creates
nameBytes but never uses it) and is not referenced anywhere; remove the entire
ui_user_dev function declaration to eliminate dead code, or if you intended to
build a uinput user device, implement it instead by allocating the correct
buffer, writing the four 32-bit fields and copying nameBytes into the final 80
bytes (ensure you use Buffer.writeUInt32LE/BE as appropriate and Buffer.copy for
nameBytes) and then export or call the function where needed; prefer removing
the unused ui_user_dev helper if there is no planned use.
- Around line 1-6: Remove the unused promisified helper: delete the promisify
import and the execFileAsync constant (references: execFileAsync, promisify) OR
if you intended to run child processes with promises, replace existing execFile
callback usage to use execFileAsync (reference: execFile) and update callers to
await execFileAsync; in short either remove the unused execFileAsync/promisify
definitions or convert existing execFile usages to await execFileAsync so the
symbol is used.
- Around line 576-598: typeText is sending raw ASCII codes (char.charCodeAt) to
sendEvent instead of Linux key scan codes; update typeText to map each character
via the existing KEY_MAP (or a similar mapping) to get the proper scan code
before calling sendEvent(EV_KEY, ...), skip characters not present in KEY_MAP,
and keep the current press/sendSync/delay/release/sendSync flow (functions to
look at: typeText, initUinputDevice, sendEvent, sendSync, and constant KEY_MAP)
so keys like 'A' use KEY_MAP['A'] (KEY_A) rather than ASCII 65.
- Around line 415-420: Replace the incorrect async/callback-style call to
fsSync.open with the synchronous API: call fsSync.openSync(UINPUT_PATH,
fsSync.constants.O_WRONLY) and assign its return value directly to deviceFd (or
a temp fd variable) instead of accessing .fd on an async result; update the
block around UINPUT_PATH and deviceFd to use the sync fd and keep the existing
undefined check/early return if needed. Ensure you reference and replace the
usage of fsSync.open and fsSync.O_WRONLY in the code that opens the uinput
device.
- Around line 37-46: The key constant values in src/server/uinput.ts (e.g.,
KEY_DELETE, KEY_INSERT, KEY_HOME, KEY_END, KEY_PAGEUP, KEY_PAGEDOWN, KEY_UP,
KEY_DOWN, KEY_LEFT, KEY_RIGHT, KEY_B, KEY_M, KEY_COMMA, KEY_DOT) collide and are
incorrect; replace each constant with the proper Linux input-event codes from
linux/input-event-codes.h as specified in the review (update KEY_INSERT to 0x6e,
KEY_END to 0x6b, KEY_DELETE to 0x6f, KEY_HOME to 0x66, KEY_PAGEUP to 0x68,
KEY_PAGEDOWN to 0x69, KEY_UP to 0x67, KEY_DOWN to 0x6c, KEY_LEFT to 0x69,
KEY_RIGHT to 0x6a, KEY_B to 0x30, KEY_M to 0x32, KEY_COMMA to 0x33, leaving
KEY_DOT as 0x34) so the constants in the file no longer conflict and match the
kernel codes used by functions that emit events.
In `@src/server/websocket.ts`:
- Around line 282-301: The variable targetRole in the webrtc-ice handling block
is computed but never used; either remove the declaration (the logic already
derives isFromProvider via (ws as ExtWebSocket).isProvider and iterates
wss.clients) or use targetRole in a debug log to clarify routing
decisions—update the code inside the if (msg.type === "webrtc-ice") block near
the (ws as ExtWebSocket).isProvider computation and wss.clients loop to
eliminate the unused variable (or add a processLogger/console.debug message
referencing targetRole).
- Around line 246-262: The webrtc-offer handler broadcasts offers but never
marks the offering socket as a provider, so subsequent routing for webrtc-answer
and ICE candidates fails; update the webrtc-offer branch (the msg.type ===
"webrtc-offer" handler) to set (sender) (the incoming socket) as a provider by
assigning its ExtWebSocket.isProvider = true before broadcasting, ensuring the
same flag used by the webrtc-answer and ICE candidate logic is set; also ensure
the ExtWebSocket type includes isProvider so the assignment is valid.
---
Outside diff comments:
In `@src/server/InputHandler.ts`:
- Around line 114-117: The console error message incorrectly mentions a "pending
move event" while this block handles scroll events; update the log in the catch
of the this.handleMessage(pending) call (near pendingScroll and handleMessage)
to refer to "pending scroll event" or include pendingScroll context so the error
message accurately reflects the scroll event being processed.
- Around line 156-188: The click handler duplicates button-to-enum mapping;
extract the mapping logic into a small helper (e.g., a function
mapButton(msgButton: string): Button) and use it in both the uinput fallback
path and the catch block instead of repeating the ternary chain. Update the case
"click" code to call mapButton(msg.button) before calling pressButtonUinput,
mouse.pressButton, and mouse.releaseButton, and handle unknown/invalid button
values by guarding early (since VALID_BUTTONS is already checked).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96f80179-4df1-4e30-b21e-19fa352b5225
📒 Files selected for processing (7)
src/hooks/useClipboardSync.tssrc/hooks/useWebRTCCaptureProvider.tssrc/hooks/useWebRTCStream.tssrc/server/InputHandler.tssrc/server/clipboard-server.tssrc/server/uinput.tssrc/server/websocket.ts
Comment on lines
+1
to
+19
| /** | ||
| * useClipboardSync — PoC hook for bidirectional clipboard sync (Issue #97) | ||
| * | ||
| * Problem: navigator.clipboard.writeText() requires HTTPS, which TanStack | ||
| * Router doesn't support yet (https://github.com/TanStack/router/issues/4287). | ||
| * | ||
| * Solution: Use the WebSocket channel to relay clipboard text between the | ||
| * phone client and the desktop server without requiring HTTPS on the client. | ||
| * | ||
| * Flow: | ||
| * COPY → client reads its own clipboard → sends {type:"clipboard-push", text} | ||
| * → server calls keyboard.pressKey(Ctrl+C) then writes text to host clipboard | ||
| * PASTE → client sends {type:"clipboard-pull"} | ||
| * → server reads host clipboard → responds {type:"clipboard-text", text} | ||
| * → client writes text into active input via document.execCommand('insertText') | ||
| * (works over plain HTTP, no HTTPS required) | ||
| */ | ||
|
|
||
| import { useCallback, useEffect, useRef } from "react" |
Contributor
There was a problem hiding this comment.
Missing "use client" directive.
This is a client-side React hook that uses browser APIs (navigator.clipboard, document.activeElement, window.getSelection). It requires the "use client" directive at the top of the file.
Proposed fix
+"use client"
+
/**
* useClipboardSync — PoC hook for bidirectional clipboard sync (Issue `#97`)As per coding guidelines: **/*.{ts,tsx,js,jsx}: NextJS: Ensure that "use client" is being used.
📝 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
| /** | |
| * useClipboardSync — PoC hook for bidirectional clipboard sync (Issue #97) | |
| * | |
| * Problem: navigator.clipboard.writeText() requires HTTPS, which TanStack | |
| * Router doesn't support yet (https://github.com/TanStack/router/issues/4287). | |
| * | |
| * Solution: Use the WebSocket channel to relay clipboard text between the | |
| * phone client and the desktop server without requiring HTTPS on the client. | |
| * | |
| * Flow: | |
| * COPY → client reads its own clipboard → sends {type:"clipboard-push", text} | |
| * → server calls keyboard.pressKey(Ctrl+C) then writes text to host clipboard | |
| * PASTE → client sends {type:"clipboard-pull"} | |
| * → server reads host clipboard → responds {type:"clipboard-text", text} | |
| * → client writes text into active input via document.execCommand('insertText') | |
| * (works over plain HTTP, no HTTPS required) | |
| */ | |
| import { useCallback, useEffect, useRef } from "react" | |
| "use client" | |
| /** | |
| * useClipboardSync — PoC hook for bidirectional clipboard sync (Issue `#97`) | |
| * | |
| * Problem: navigator.clipboard.writeText() requires HTTPS, which TanStack | |
| * Router doesn't support yet (https://github.com/TanStack/router/issues/4287). | |
| * | |
| * Solution: Use the WebSocket channel to relay clipboard text between the | |
| * phone client and the desktop server without requiring HTTPS on the client. | |
| * | |
| * Flow: | |
| * COPY → client reads its own clipboard → sends {type:"clipboard-push", text} | |
| * → server calls keyboard.pressKey(Ctrl+C) then writes text to host clipboard | |
| * PASTE → client sends {type:"clipboard-pull"} | |
| * → server reads host clipboard → responds {type:"clipboard-text", text} | |
| * → client writes text into active input via document.execCommand('insertText') | |
| * (works over plain HTTP, no HTTPS required) | |
| */ | |
| import { useCallback, useEffect, useRef } from "react" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useClipboardSync.ts` around lines 1 - 19, The file defining the
useClipboardSync hook is missing the Next.js client-side directive; add the "use
client" directive as the very first line of the file so the hook
(useClipboardSync and its use of navigator.clipboard, document.activeElement,
window.getSelection, etc.) runs on the client; ensure the directive precedes all
imports and comments so React's client-side behavior is enabled for the hook.
Comment on lines
+50
to
+52
| navigator.clipboard.writeText(msg.text).catch(() => { | ||
| insertTextFallback(msg.text!) | ||
| }) |
Contributor
There was a problem hiding this comment.
Fix non-null assertion to resolve pipeline failure.
The pipeline reports a Biome lint error for the non-null assertion on msg.text!. Since the condition on line 47 already validates msg.text is a string, you can capture it in a variable before the async callback.
Proposed fix
if (msg.type === "clipboard-text" && typeof msg.text === "string") {
+ const text = msg.text
// Try modern clipboard API first (only works over HTTPS / localhost)
if (navigator.clipboard && window.isSecureContext) {
- navigator.clipboard.writeText(msg.text).catch(() => {
- insertTextFallback(msg.text!)
+ navigator.clipboard.writeText(text).catch(() => {
+ insertTextFallback(text)
})
} else {
// Fallback: insert into focused element via execCommand (HTTP-safe)
- insertTextFallback(msg.text)
+ insertTextFallback(text)
}
- onPasteRef.current?.(msg.text)
+ onPasteRef.current?.(text)
}🧰 Tools
🪛 GitHub Actions: CI
[error] 51-51: Biome lint/style error (lint/style/noNonNullAssertion): Forbidden non-null assertion. Update the code to avoid using the non-null assertion operator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useClipboardSync.ts` around lines 50 - 52, The non-null assertion
on msg.text causes a Biome lint failure; capture the validated string in a local
variable before the async callback and use that variable instead. Inside the
block where you already checked msg.text is a string, add const text = msg.text
and replace navigator.clipboard.writeText(msg.text).catch(() =>
insertTextFallback(msg.text!)) with navigator.clipboard.writeText(text).catch(()
=> insertTextFallback(text)), referencing msg.text,
navigator.clipboard.writeText, and insertTextFallback to locate and update the
code.
Comment on lines
+5
to
+8
| interface RTCMessage { | ||
| type: "offer" | "answer" | "ice-candidate" | ||
| payload: RTCSessionDescriptionInit | RTCIceCandidateInit | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused RTCMessage interface.
The RTCMessage interface is defined but never used in this file. Consider removing it or using it for type-safe message handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useWebRTCCaptureProvider.ts` around lines 5 - 8, The file declares
an unused interface named RTCMessage; either remove the RTCMessage declaration
to eliminate dead code, or apply it to the message handling code (e.g., annotate
incoming/outgoing message variables and the message event handler in
useWebRTCCaptureProvider to use RTCMessage for type-safe handling of "offer" |
"answer" | "ice-candidate" payloads). Update or remove the interface so there
are no unused types reported.
Comment on lines
+39
to
+111
| const startSharing = useCallback(async () => { | ||
| try { | ||
| const stream = await navigator.mediaDevices.getDisplayMedia({ | ||
| video: { | ||
| displaySurface: "monitor", | ||
| width: { ideal: 1920 }, | ||
| height: { ideal: 1080 }, | ||
| frameRate: { ideal: 60 }, | ||
| }, | ||
| audio: false, | ||
| }) | ||
|
|
||
| localStreamRef.current = stream | ||
|
|
||
| const config: RTCConfiguration = { | ||
| iceServers: [], | ||
| } | ||
|
|
||
| const pc = new RTCPeerConnection(config) | ||
| peerConnectionRef.current = pc | ||
|
|
||
| for (const track of stream.getVideoTracks()) { | ||
| pc.addTrack(track, stream) | ||
| } | ||
|
|
||
| pc.onicecandidate = (event) => { | ||
| if (event.candidate && wsRef.current?.readyState === WebSocket.OPEN) { | ||
| wsRef.current.send( | ||
| JSON.stringify({ | ||
| type: "webrtc-ice", | ||
| payload: event.candidate.toJSON(), | ||
| }), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| pc.onconnectionstatechange = () => { | ||
| const state = pc.connectionState | ||
| console.log("[WebRTC] Connection state:", state) | ||
|
|
||
| if (state === "connected") { | ||
| onConnectedChange?.(true) | ||
| } else if ( | ||
| state === "disconnected" || | ||
| state === "failed" || | ||
| state === "closed" | ||
| ) { | ||
| onConnectedChange?.(false) | ||
| } | ||
| } | ||
|
|
||
| const offer = await pc.createOffer() | ||
| await pc.setLocalDescription(offer) | ||
|
|
||
| if (wsRef.current && wsRef.current.readyState === WebSocket.OPEN) { | ||
| wsRef.current.send( | ||
| JSON.stringify({ | ||
| type: "webrtc-offer", | ||
| payload: offer, | ||
| }), | ||
| ) | ||
| } | ||
|
|
||
| stream.getVideoTracks()[0].onended = () => { | ||
| stopSharing() | ||
| } | ||
|
|
||
| setIsSharing(true) | ||
| } catch (err) { | ||
| console.error("Failed to start WebRTC screen capture:", err) | ||
| setIsSharing(false) | ||
| } | ||
| }, [wsRef, stopSharing, onConnectedChange]) |
Contributor
There was a problem hiding this comment.
Missing provider registration before sending WebRTC offer.
The hook sends webrtc-offer (line 94-99) and stop-webrtc-provider (line 32), but never sends start-provider to register as a provider on the server. This means the server's isProvider flag is never set, causing:
webrtc-answermessages won't be routed back to this client- ICE candidates from consumers will be misrouted
Proposed fix: Send start-provider before the offer
const offer = await pc.createOffer()
await pc.setLocalDescription(offer)
if (wsRef.current && wsRef.current.readyState === WebSocket.OPEN) {
+ wsRef.current.send(JSON.stringify({ type: "start-provider" }))
wsRef.current.send(
JSON.stringify({
type: "webrtc-offer",
payload: offer,
}),
)
}📝 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
| const startSharing = useCallback(async () => { | |
| try { | |
| const stream = await navigator.mediaDevices.getDisplayMedia({ | |
| video: { | |
| displaySurface: "monitor", | |
| width: { ideal: 1920 }, | |
| height: { ideal: 1080 }, | |
| frameRate: { ideal: 60 }, | |
| }, | |
| audio: false, | |
| }) | |
| localStreamRef.current = stream | |
| const config: RTCConfiguration = { | |
| iceServers: [], | |
| } | |
| const pc = new RTCPeerConnection(config) | |
| peerConnectionRef.current = pc | |
| for (const track of stream.getVideoTracks()) { | |
| pc.addTrack(track, stream) | |
| } | |
| pc.onicecandidate = (event) => { | |
| if (event.candidate && wsRef.current?.readyState === WebSocket.OPEN) { | |
| wsRef.current.send( | |
| JSON.stringify({ | |
| type: "webrtc-ice", | |
| payload: event.candidate.toJSON(), | |
| }), | |
| ) | |
| } | |
| } | |
| pc.onconnectionstatechange = () => { | |
| const state = pc.connectionState | |
| console.log("[WebRTC] Connection state:", state) | |
| if (state === "connected") { | |
| onConnectedChange?.(true) | |
| } else if ( | |
| state === "disconnected" || | |
| state === "failed" || | |
| state === "closed" | |
| ) { | |
| onConnectedChange?.(false) | |
| } | |
| } | |
| const offer = await pc.createOffer() | |
| await pc.setLocalDescription(offer) | |
| if (wsRef.current && wsRef.current.readyState === WebSocket.OPEN) { | |
| wsRef.current.send( | |
| JSON.stringify({ | |
| type: "webrtc-offer", | |
| payload: offer, | |
| }), | |
| ) | |
| } | |
| stream.getVideoTracks()[0].onended = () => { | |
| stopSharing() | |
| } | |
| setIsSharing(true) | |
| } catch (err) { | |
| console.error("Failed to start WebRTC screen capture:", err) | |
| setIsSharing(false) | |
| } | |
| }, [wsRef, stopSharing, onConnectedChange]) | |
| const startSharing = useCallback(async () => { | |
| try { | |
| const stream = await navigator.mediaDevices.getDisplayMedia({ | |
| video: { | |
| displaySurface: "monitor", | |
| width: { ideal: 1920 }, | |
| height: { ideal: 1080 }, | |
| frameRate: { ideal: 60 }, | |
| }, | |
| audio: false, | |
| }) | |
| localStreamRef.current = stream | |
| const config: RTCConfiguration = { | |
| iceServers: [], | |
| } | |
| const pc = new RTCPeerConnection(config) | |
| peerConnectionRef.current = pc | |
| for (const track of stream.getVideoTracks()) { | |
| pc.addTrack(track, stream) | |
| } | |
| pc.onicecandidate = (event) => { | |
| if (event.candidate && wsRef.current?.readyState === WebSocket.OPEN) { | |
| wsRef.current.send( | |
| JSON.stringify({ | |
| type: "webrtc-ice", | |
| payload: event.candidate.toJSON(), | |
| }), | |
| ) | |
| } | |
| } | |
| pc.onconnectionstatechange = () => { | |
| const state = pc.connectionState | |
| console.log("[WebRTC] Connection state:", state) | |
| if (state === "connected") { | |
| onConnectedChange?.(true) | |
| } else if ( | |
| state === "disconnected" || | |
| state === "failed" || | |
| state === "closed" | |
| ) { | |
| onConnectedChange?.(false) | |
| } | |
| } | |
| const offer = await pc.createOffer() | |
| await pc.setLocalDescription(offer) | |
| if (wsRef.current && wsRef.current.readyState === WebSocket.OPEN) { | |
| wsRef.current.send(JSON.stringify({ type: "start-provider" })) | |
| wsRef.current.send( | |
| JSON.stringify({ | |
| type: "webrtc-offer", | |
| payload: offer, | |
| }), | |
| ) | |
| } | |
| stream.getVideoTracks()[0].onended = () => { | |
| stopSharing() | |
| } | |
| setIsSharing(true) | |
| } catch (err) { | |
| console.error("Failed to start WebRTC screen capture:", err) | |
| setIsSharing(false) | |
| } | |
| }, [wsRef, stopSharing, onConnectedChange]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useWebRTCCaptureProvider.ts` around lines 39 - 111, The
startSharing flow (startSharing) never registers this client as a provider
before sending the webrtc-offer, so the server won't set isProvider and
answers/ICE will be misrouted; before creating/sending the offer (and after
confirming wsRef.current?.readyState === WebSocket.OPEN) send a "start-provider"
message via wsRef.current.send (matching the server's provider registration
contract) so the server marks this client as a provider; ensure this send
happens before the webrtc-offer send (and keep stopSharing behavior that sends
"stop-webrtc-provider" intact) so subsequent webrtc-answer and ICE messages are
routed back correctly.
Comment on lines
+119
to
+138
| useEffect(() => { | ||
| if (!wsRef.current) return | ||
|
|
||
| const handleMessage = (event: MessageEvent) => { | ||
| if (peerConnectionRef.current?.remoteDescription) return | ||
|
|
||
| try { | ||
| const data = JSON.parse(event.data) | ||
|
|
||
| if (data.type === "webrtc-answer") { | ||
| const answer = new RTCSessionDescription(data.payload) | ||
| peerConnectionRef.current?.setRemoteDescription(answer) | ||
| } else if (data.type === "webrtc-ice") { | ||
| const candidate = new RTCIceCandidate(data.payload) | ||
| peerConnectionRef.current?.addIceCandidate(candidate) | ||
| } | ||
| } catch (err) { | ||
| console.error("Error handling WebRTC message:", err) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Critical: ICE candidates blocked after remote description is set.
Line 123 returns early if remoteDescription is already set, which blocks processing of all subsequent messages, including ICE candidates. In WebRTC trickle ICE, candidates often arrive after the answer. This will prevent peer connection establishment.
Proposed fix: Move the early return to only apply to webrtc-answer
useEffect(() => {
if (!wsRef.current) return
const handleMessage = (event: MessageEvent) => {
- if (peerConnectionRef.current?.remoteDescription) return
-
try {
const data = JSON.parse(event.data)
if (data.type === "webrtc-answer") {
+ if (peerConnectionRef.current?.remoteDescription) return
const answer = new RTCSessionDescription(data.payload)
peerConnectionRef.current?.setRemoteDescription(answer)
} else if (data.type === "webrtc-ice") {
const candidate = new RTCIceCandidate(data.payload)
peerConnectionRef.current?.addIceCandidate(candidate)
}
} catch (err) {
console.error("Error handling WebRTC message:", err)
}
}📝 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
| useEffect(() => { | |
| if (!wsRef.current) return | |
| const handleMessage = (event: MessageEvent) => { | |
| if (peerConnectionRef.current?.remoteDescription) return | |
| try { | |
| const data = JSON.parse(event.data) | |
| if (data.type === "webrtc-answer") { | |
| const answer = new RTCSessionDescription(data.payload) | |
| peerConnectionRef.current?.setRemoteDescription(answer) | |
| } else if (data.type === "webrtc-ice") { | |
| const candidate = new RTCIceCandidate(data.payload) | |
| peerConnectionRef.current?.addIceCandidate(candidate) | |
| } | |
| } catch (err) { | |
| console.error("Error handling WebRTC message:", err) | |
| } | |
| } | |
| useEffect(() => { | |
| if (!wsRef.current) return | |
| const handleMessage = (event: MessageEvent) => { | |
| try { | |
| const data = JSON.parse(event.data) | |
| if (data.type === "webrtc-answer") { | |
| if (peerConnectionRef.current?.remoteDescription) return | |
| const answer = new RTCSessionDescription(data.payload) | |
| peerConnectionRef.current?.setRemoteDescription(answer) | |
| } else if (data.type === "webrtc-ice") { | |
| const candidate = new RTCIceCandidate(data.payload) | |
| peerConnectionRef.current?.addIceCandidate(candidate) | |
| } | |
| } catch (err) { | |
| console.error("Error handling WebRTC message:", err) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useWebRTCCaptureProvider.ts` around lines 119 - 138, The current
handleMessage in useEffect incorrectly returns early if
peerConnectionRef.current?.remoteDescription is set, which blocks processing of
subsequent webrtc-ice messages; update handleMessage (used in
useWebRTCCaptureProvider) so the early return only applies when handling a
"webrtc-answer" (i.e., if data.type === "webrtc-answer" and remoteDescription
exists, skip applying the answer), and always allow the "webrtc-ice" branch to
run so new RTCIceCandidate messages are added via
peerConnectionRef.current?.addIceCandidate; keep the JSON.parse and try/catch
but relocate the remoteDescription guard into the webrtc-answer branch.
| KEY_DOT, | ||
| KEY_SLASH, | ||
| KEY_RIGHTSHIFT, | ||
| KEY_KPMULT, |
Contributor
There was a problem hiding this comment.
Undefined constants KEY_KPMULT and KEY_KPDIV used in setupKeyboardEvents.
Lines 231 and 264 reference KEY_KPMULT and KEY_KPDIV which are never defined in this file. This will cause a runtime error when setupKeyboardEvents is called.
Proposed fix: Add missing constant definitions
const KEY_KPDOT = 0x53
const KEY_KPENTER = 0x5c
+const KEY_KPMULT = 0x37 // Keypad *
+const KEY_KPDIV = 0x62 // Keypad /Also applies to: 264-266
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/uinput.ts` at line 231, setupKeyboardEvents uses undefined
constants KEY_KPMULT and KEY_KPDIV causing runtime errors; fix by adding proper
definitions or importing them where constants are declared (e.g., from the same
keyboard/keycode source used for other KEY_* constants) and reference those
symbols in setupKeyboardEvents. Locate setupKeyboardEvents in this file, then
either (a) add const KEY_KPMULT = <correct keycode> and const KEY_KPDIV =
<correct keycode> alongside the other KEY_* definitions, or (b) import
KEY_KPMULT and KEY_KPDIV from the module supplying other key constants so the
names resolve at runtime. Ensure the names match usages in setupKeyboardEvents
exactly.
Comment on lines
+415
to
+420
| try { | ||
| const fd = fsSync.open(UINPUT_PATH, fsSync.O_WRONLY).fd | ||
| if (fd === undefined) { | ||
| return false | ||
| } | ||
| deviceFd = fd |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Node.js fs.openSync API returns number directly
node -e "const fs = require('fs'); console.log('openSync returns:', typeof fs.openSync('/dev/null', 'r'))"Repository: AOSSIE-Org/Rein
Length of output: 83
🏁 Script executed:
cat -n src/server/uinput.ts | sed -n '410,425p'Repository: AOSSIE-Org/Rein
Length of output: 485
🏁 Script executed:
# Check Node.js fs API for open() vs openSync() and constants
node -e "const fs = require('fs'); console.log('fs.open type:', typeof fs.open); console.log('fs.openSync type:', typeof fs.openSync); console.log('fs.constants.O_WRONLY:', fs.constants.O_WRONLY); console.log('fs.O_WRONLY exists:', typeof fs.O_WRONLY);"Repository: AOSSIE-Org/Rein
Length of output: 163
🏁 Script executed:
head -50 src/server/uinput.ts | cat -nRepository: AOSSIE-Org/Rein
Length of output: 1390
🏁 Script executed:
rg "fsSync" src/server/uinput.ts | head -20Repository: AOSSIE-Org/Rein
Length of output: 434
Fix incorrect async open() call—use openSync() instead.
Line 412 calls fsSync.open() (async callback-based method) and tries to access the result's .fd property synchronously, which will be undefined. Additionally, fsSync.O_WRONLY doesn't exist—use fsSync.constants.O_WRONLY.
Proposed fix
try {
- const fd = fsSync.open(UINPUT_PATH, fsSync.O_WRONLY).fd
- if (fd === undefined) {
- return false
- }
- deviceFd = fd
+ deviceFd = fsSync.openSync(UINPUT_PATH, fsSync.constants.O_WRONLY | fsSync.constants.O_NONBLOCK)📝 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
| try { | |
| const fd = fsSync.open(UINPUT_PATH, fsSync.O_WRONLY).fd | |
| if (fd === undefined) { | |
| return false | |
| } | |
| deviceFd = fd | |
| try { | |
| deviceFd = fsSync.openSync(UINPUT_PATH, fsSync.constants.O_WRONLY | fsSync.constants.O_NONBLOCK) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/uinput.ts` around lines 415 - 420, Replace the incorrect
async/callback-style call to fsSync.open with the synchronous API: call
fsSync.openSync(UINPUT_PATH, fsSync.constants.O_WRONLY) and assign its return
value directly to deviceFd (or a temp fd variable) instead of accessing .fd on
an async result; update the block around UINPUT_PATH and deviceFd to use the
sync fd and keep the existing undefined check/early return if needed. Ensure you
reference and replace the usage of fsSync.open and fsSync.O_WRONLY in the code
that opens the uinput device.
Comment on lines
+576
to
+598
| export async function typeText(text: string): Promise<boolean> { | ||
| if (!(await initUinputDevice())) { | ||
| return false | ||
| } | ||
|
|
||
| try { | ||
| for (const char of text) { | ||
| const code = char.charCodeAt(0) | ||
|
|
||
| if (code >= 32 && code <= 126) { | ||
| sendEvent(EV_KEY, code, 1) | ||
| sendSync() | ||
| await new Promise((r) => setTimeout(r, 5)) | ||
| sendEvent(EV_KEY, code, 0) | ||
| sendSync() | ||
| await new Promise((r) => setTimeout(r, 5)) | ||
| } | ||
| } | ||
| return true | ||
| } catch (err) { | ||
| console.error("[uinput] Error in typeText:", err) | ||
| return false | ||
| } |
Contributor
There was a problem hiding this comment.
typeText uses ASCII codes instead of Linux scan codes.
The function sends char.charCodeAt(0) directly as key codes (lines 586, 589), but Linux uinput expects scan codes, not ASCII codes. For example, 'A' (ASCII 65) would send key code 65, but the correct scan code for 'A' is KEY_A (0x1e = 30).
The function should map characters through KEY_MAP instead.
Proposed fix
export async function typeText(text: string): Promise<boolean> {
if (!(await initUinputDevice())) {
return false
}
try {
for (const char of text) {
- const code = char.charCodeAt(0)
-
- if (code >= 32 && code <= 126) {
- sendEvent(EV_KEY, code, 1)
+ const lowerChar = char.toLowerCase()
+ const keyCode = KEY_MAP[lowerChar]
+
+ if (keyCode !== undefined) {
+ const needsShift = char !== lowerChar && /[A-Z]/.test(char)
+ if (needsShift) {
+ sendEvent(EV_KEY, KEY_LEFTSHIFT, 1)
+ sendSync()
+ }
+ sendEvent(EV_KEY, keyCode, 1)
sendSync()
await new Promise((r) => setTimeout(r, 5))
- sendEvent(EV_KEY, code, 0)
+ sendEvent(EV_KEY, keyCode, 0)
sendSync()
+ if (needsShift) {
+ sendEvent(EV_KEY, KEY_LEFTSHIFT, 0)
+ sendSync()
+ }
await new Promise((r) => setTimeout(r, 5))
}
}
return true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/uinput.ts` around lines 576 - 598, typeText is sending raw ASCII
codes (char.charCodeAt) to sendEvent instead of Linux key scan codes; update
typeText to map each character via the existing KEY_MAP (or a similar mapping)
to get the proper scan code before calling sendEvent(EV_KEY, ...), skip
characters not present in KEY_MAP, and keep the current
press/sendSync/delay/release/sendSync flow (functions to look at: typeText,
initUinputDevice, sendEvent, sendSync, and constant KEY_MAP) so keys like 'A'
use KEY_MAP['A'] (KEY_A) rather than ASCII 65.
Comment on lines
+246
to
+262
| if (msg.type === "webrtc-offer") { | ||
| logger.info("Received WebRTC offer, relaying to consumer") | ||
| for (const client of wss.clients) { | ||
| if ( | ||
| (client as ExtWebSocket).isWebRTCConsumer && | ||
| client.readyState === WebSocket.OPEN | ||
| ) { | ||
| client.send( | ||
| JSON.stringify({ | ||
| type: "webrtc-offer", | ||
| payload: msg.payload, | ||
| }), | ||
| ) | ||
| } | ||
| } | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
WebRTC provider is never registered before sending offer.
The webrtc-offer handler broadcasts to consumers, but the provider's isProvider flag is never set. Looking at the context from src/hooks/useWebRTCCaptureProvider.ts, the hook sends webrtc-offer directly without first sending start-provider (line 228-232 in this file).
This causes:
webrtc-answerwon't route back to the provider (line 264-280 checksisProvider)- ICE candidates from the consumer will be misrouted (line 282-301 uses
isProviderto determine direction)
The provider hook should send start-provider before the offer, or the webrtc-offer handler should implicitly set isProvider = true on the sender.
Proposed fix: Set isProvider when receiving webrtc-offer
if (msg.type === "webrtc-offer") {
+ ;(ws as ExtWebSocket).isProvider = true
logger.info("Received WebRTC offer, relaying to consumer")
for (const client of wss.clients) {📝 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
| if (msg.type === "webrtc-offer") { | |
| logger.info("Received WebRTC offer, relaying to consumer") | |
| for (const client of wss.clients) { | |
| if ( | |
| (client as ExtWebSocket).isWebRTCConsumer && | |
| client.readyState === WebSocket.OPEN | |
| ) { | |
| client.send( | |
| JSON.stringify({ | |
| type: "webrtc-offer", | |
| payload: msg.payload, | |
| }), | |
| ) | |
| } | |
| } | |
| return | |
| } | |
| if (msg.type === "webrtc-offer") { | |
| (ws as ExtWebSocket).isProvider = true | |
| logger.info("Received WebRTC offer, relaying to consumer") | |
| for (const client of wss.clients) { | |
| if ( | |
| (client as ExtWebSocket).isWebRTCConsumer && | |
| client.readyState === WebSocket.OPEN | |
| ) { | |
| client.send( | |
| JSON.stringify({ | |
| type: "webrtc-offer", | |
| payload: msg.payload, | |
| }), | |
| ) | |
| } | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/websocket.ts` around lines 246 - 262, The webrtc-offer handler
broadcasts offers but never marks the offering socket as a provider, so
subsequent routing for webrtc-answer and ICE candidates fails; update the
webrtc-offer branch (the msg.type === "webrtc-offer" handler) to set (sender)
(the incoming socket) as a provider by assigning its ExtWebSocket.isProvider =
true before broadcasting, ensuring the same flag used by the webrtc-answer and
ICE candidate logic is set; also ensure the ExtWebSocket type includes
isProvider so the assignment is valid.
Comment on lines
+282
to
+301
| if (msg.type === "webrtc-ice") { | ||
| const isFromProvider = (ws as ExtWebSocket).isProvider | ||
| const targetRole = isFromProvider ? "consumer" : "provider" | ||
|
|
||
| for (const client of wss.clients) { | ||
| const isTarget = isFromProvider | ||
| ? (client as ExtWebSocket).isWebRTCConsumer | ||
| : (client as ExtWebSocket).isProvider | ||
|
|
||
| if (isTarget && client.readyState === WebSocket.OPEN) { | ||
| client.send( | ||
| JSON.stringify({ | ||
| type: "webrtc-ice", | ||
| payload: msg.payload, | ||
| }), | ||
| ) | ||
| } | ||
| } | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused targetRole variable.
The targetRole variable is computed but never used. Consider removing it or using it in a log statement for debugging purposes.
Proposed fix
if (msg.type === "webrtc-ice") {
const isFromProvider = (ws as ExtWebSocket).isProvider
- const targetRole = isFromProvider ? "consumer" : "provider"
for (const client of wss.clients) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/websocket.ts` around lines 282 - 301, The variable targetRole in
the webrtc-ice handling block is computed but never used; either remove the
declaration (the logic already derives isFromProvider via (ws as
ExtWebSocket).isProvider and iterates wss.clients) or use targetRole in a debug
log to clarify routing decisions—update the code inside the if (msg.type ===
"webrtc-ice") block near the (ws as ExtWebSocket).isProvider computation and
wss.clients loop to eliminate the unused variable (or add a
processLogger/console.debug message referencing targetRole).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Trackpad/ScreenMirror.tsx`:
- Around line 28-33: The current code unconditionally calls useWebRTCStream and
useMirrorStream causing concurrent canvas writes and uses status to select
frames; change it so useMirrorStream is not invoked while WebRTC is active (or
pass a disable flag into useMirrorStream) and compute hasFrame as a availability
OR (hasWebRTCFrame || hasLegacyFrame) rather than using status; ensure
useWebRTCStream exposes a success/active signal you can check before skipping
the legacy hook, and keep wsRef, canvasRef, and status passed unchanged to the
chosen hook (reference useWebRTCStream, useMirrorStream, hasWebRTCFrame,
hasLegacyFrame, and hasFrame).
In `@src/routes/__root.tsx`:
- Around line 64-67: The current fallback race occurs because startWebRTC()
resolves when setIsSharing(true) is called, before the actual RTCPeerConnection
reaches "connected"; update the logic so the fallback triggers on real
connection failure: modify startWebRTC (in useWebRTCCaptureProvider) to either
(a) return a Promise that only resolves when onconnectionstatechange reports
connectionState === "connected" and rejects on "failed"/"disconnected"/"closed",
or (b) expose an onConnectedChange callback / isWebRTCSharing flag that
__root.tsx can observe and use with a timeout to call startCaptureProvider()
when connection doesn't become "connected" within N ms; implement one of these
approaches and update the call site where startWebRTC() is invoked in __root.tsx
to rely on the new semantics (i.e., only treat success when connection is truly
established or trigger fallback via the supplied onConnectedChange/timeout).
- Around line 50-52: The destructured variable isWebRTCSharing from the
useWebRTCCaptureProvider(wsRef) call is never used; remove it from the
destructuring to avoid dead code (change const { startSharing: startWebRTC,
isSharing: isWebRTCSharing } = useWebRTCCaptureProvider(wsRef) to only extract
startSharing as startWebRTC), or if WebRTC sharing state is needed later, use
the isWebRTCSharing value where required (e.g., in conditional logic to decide
fallback behavior) and keep the destructuring as-is; reference
useWebRTCCaptureProvider, startWebRTC, and isWebRTCSharing when making the
change.
In `@src/server/uinput.ts`:
- Around line 57-62: The modifier key constants in src/server/uinput.ts are
incorrect: update KEY_RIGHTCTRL, KEY_RIGHTALT, KEY_LEFTMETA, and KEY_RIGHTMETA
to the proper Linux scan codes; specifically set KEY_RIGHTCTRL to 0x61 (97),
KEY_RIGHTALT to 0x64 (100), KEY_LEFTMETA to 0x7d (125), and KEY_RIGHTMETA to
0x7e (126) so the right-side modifiers are distinct from the left ones (modify
the constant declarations for KEY_RIGHTCTRL, KEY_RIGHTALT, KEY_LEFTMETA, and
KEY_RIGHTMETA).
- Around line 138-149: The createUinputDevice function builds an incorrect
uinput_user_dev layout; reorder and expand the buffer to match the kernel
struct: allocate a buffer large enough for name[80] + input_id (bustype, vendor,
product, version as u16) + ff_effects_max (int) + the abs arrays (approx 1116
bytes total), write the device name into offset 0 (80 bytes), then write
bustype, vendorId, productId, version at the input_id offsets using
writeUInt16LE, then write ff_effects_max, and finally zero the abs arrays;
update uses of config (name, vendorId, productId, version, bustype,
ffEffectsMax) and write to fd with fsSync.writeSync(fd, buffer) after
constructing the correctly laid out buffer so uinput_user_dev matches kernel
expectations.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0842b985-d334-4042-9eb8-1c9eaffe795a
📒 Files selected for processing (4)
src/components/Trackpad/ScreenMirror.tsxsrc/hooks/useClipboardSync.tssrc/routes/__root.tsxsrc/server/uinput.ts
Comment on lines
+28
to
+33
| // Try WebRTC first (PoC #208), fall back to old method | ||
| const { hasFrame: hasWebRTCFrame } = useWebRTCStream(wsRef, canvasRef, status) | ||
| const { hasFrame: hasLegacyFrame } = useMirrorStream(wsRef, canvasRef, status) | ||
|
|
||
| // Prefer WebRTC if connected | ||
| const hasFrame = status === "connected" ? hasWebRTCFrame : hasLegacyFrame |
Contributor
There was a problem hiding this comment.
Concurrent canvas writes and flawed fallback logic.
Two issues with this implementation:
-
Race condition on canvas: Both hooks are invoked unconditionally. Per
useMirrorStream.ts:66-85, the legacy hook continues drawing to the canvas whenstatus === "connected". When WebRTC is also active, both streams write to the same canvas concurrently, causing visual tearing or flickering. -
Fallback doesn't actually fall back: The ternary
status === "connected" ? hasWebRTCFrame : hasLegacyFramechecks connection status, not frame availability. If WebRTC fails to establish (provider rejected, no answer, etc.),hasWebRTCFramestaysfalse,hasLegacyFrameis ignored, and the UI shows "Waiting for screen..." indefinitely despite the legacy stream potentially working.
The fallback should check frame availability, not connection status:
🔧 Suggested approach
- // Prefer WebRTC if connected
- const hasFrame = status === "connected" ? hasWebRTCFrame : hasLegacyFrame
+ // Prefer WebRTC frames, fall back to legacy
+ const hasFrame = hasWebRTCFrame || hasLegacyFrameHowever, this still leaves the concurrent canvas write problem. A cleaner solution would be to:
- Pass a flag to
useMirrorStreamto disable it when WebRTC is active, OR - Have
useWebRTCStreamsignal success souseMirrorStreamcan be conditionally skipped entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Trackpad/ScreenMirror.tsx` around lines 28 - 33, The current
code unconditionally calls useWebRTCStream and useMirrorStream causing
concurrent canvas writes and uses status to select frames; change it so
useMirrorStream is not invoked while WebRTC is active (or pass a disable flag
into useMirrorStream) and compute hasFrame as a availability OR (hasWebRTCFrame
|| hasLegacyFrame) rather than using status; ensure useWebRTCStream exposes a
success/active signal you can check before skipping the legacy hook, and keep
wsRef, canvasRef, and status passed unchanged to the chosen hook (reference
useWebRTCStream, useMirrorStream, hasWebRTCFrame, hasLegacyFrame, and hasFrame).
Comment on lines
+50
to
+52
| const { startSharing: startCaptureProvider } = useCaptureProvider(wsRef) | ||
| const { startSharing: startWebRTC, isSharing: isWebRTCSharing } = | ||
| useWebRTCCaptureProvider(wsRef) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused variable: isWebRTCSharing is destructured but never referenced.
If this was intentionally left for future use, consider omitting it from the destructuring to avoid dead code. If it's needed for detecting WebRTC state (e.g., for the fallback fix), it should be used.
♻️ Proposed fix
- const { startSharing: startWebRTC, isSharing: isWebRTCSharing } =
- useWebRTCCaptureProvider(wsRef)
+ const { startSharing: startWebRTC } = useWebRTCCaptureProvider(wsRef)📝 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
| const { startSharing: startCaptureProvider } = useCaptureProvider(wsRef) | |
| const { startSharing: startWebRTC, isSharing: isWebRTCSharing } = | |
| useWebRTCCaptureProvider(wsRef) | |
| const { startSharing: startCaptureProvider } = useCaptureProvider(wsRef) | |
| const { startSharing: startWebRTC } = useWebRTCCaptureProvider(wsRef) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/__root.tsx` around lines 50 - 52, The destructured variable
isWebRTCSharing from the useWebRTCCaptureProvider(wsRef) call is never used;
remove it from the destructuring to avoid dead code (change const {
startSharing: startWebRTC, isSharing: isWebRTCSharing } =
useWebRTCCaptureProvider(wsRef) to only extract startSharing as startWebRTC), or
if WebRTC sharing state is needed later, use the isWebRTCSharing value where
required (e.g., in conditional logic to decide fallback behavior) and keep the
destructuring as-is; reference useWebRTCCaptureProvider, startWebRTC, and
isWebRTCSharing when making the change.
Comment on lines
+64
to
+67
| // Try WebRTC first (PoC #208), fall back to capture provider | ||
| startWebRTC().catch(() => { | ||
| startCaptureProvider() | ||
| }) |
Contributor
There was a problem hiding this comment.
Fallback logic has a race condition — WebRTC failures won't trigger the fallback.
Per the context in useWebRTCCaptureProvider.ts:39-111, startWebRTC() resolves as soon as setIsSharing(true) is called (after the offer is sent), before the WebRTC connection is actually established. The actual connection state is only tracked later via onconnectionstatechange. If the connection fails (no answer, ICE failure, etc.), the promise has already resolved and .catch() never fires.
Consider one of:
- Have
startWebRTC()return a Promise that resolves only afterconnectionState === "connected". - Use the
onConnectedChangecallback to detect failure and trigger fallback. - Monitor
isWebRTCSharingcombined with a timeout to detect stalled connections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/__root.tsx` around lines 64 - 67, The current fallback race occurs
because startWebRTC() resolves when setIsSharing(true) is called, before the
actual RTCPeerConnection reaches "connected"; update the logic so the fallback
triggers on real connection failure: modify startWebRTC (in
useWebRTCCaptureProvider) to either (a) return a Promise that only resolves when
onconnectionstatechange reports connectionState === "connected" and rejects on
"failed"/"disconnected"/"closed", or (b) expose an onConnectedChange callback /
isWebRTCSharing flag that __root.tsx can observe and use with a timeout to call
startCaptureProvider() when connection doesn't become "connected" within N ms;
implement one of these approaches and update the call site where startWebRTC()
is invoked in __root.tsx to rely on the new semantics (i.e., only treat success
when connection is truly established or trigger fallback via the supplied
onConnectedChange/timeout).
Comment on lines
+57
to
+62
| const KEY_LEFTCTRL = 0x1d | ||
| const KEY_RIGHTCTRL = 0x1d | ||
| const KEY_LEFTALT = 0x38 | ||
| const KEY_RIGHTALT = 0x38 | ||
| const KEY_LEFTMETA = 0x5b | ||
| const KEY_RIGHTMETA = 0x5c |
Contributor
There was a problem hiding this comment.
Modifier key codes are incorrect — right modifiers will behave as left modifiers.
KEY_RIGHTCTRL and KEY_RIGHTALT are assigned the same scan codes as their left counterparts, and KEY_LEFTMETA/KEY_RIGHTMETA use wrong values:
| Constant | Current | Correct (Linux) |
|---|---|---|
KEY_RIGHTCTRL |
0x1d | 0x61 (97) |
KEY_RIGHTALT |
0x38 | 0x64 (100) |
KEY_LEFTMETA |
0x5b | 0x7d (125) |
KEY_RIGHTMETA |
0x5c | 0x7e (126) |
Proposed fix
const KEY_LEFTCTRL = 0x1d
-const KEY_RIGHTCTRL = 0x1d
+const KEY_RIGHTCTRL = 0x61
const KEY_LEFTALT = 0x38
-const KEY_RIGHTALT = 0x38
-const KEY_LEFTMETA = 0x5b
-const KEY_RIGHTMETA = 0x5c
+const KEY_RIGHTALT = 0x64
+const KEY_LEFTMETA = 0x7d
+const KEY_RIGHTMETA = 0x7eLinux input-event-codes.h KEY_RIGHTCTRL KEY_RIGHTALT KEY_LEFTMETA values
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/uinput.ts` around lines 57 - 62, The modifier key constants in
src/server/uinput.ts are incorrect: update KEY_RIGHTCTRL, KEY_RIGHTALT,
KEY_LEFTMETA, and KEY_RIGHTMETA to the proper Linux scan codes; specifically set
KEY_RIGHTCTRL to 0x61 (97), KEY_RIGHTALT to 0x64 (100), KEY_LEFTMETA to 0x7d
(125), and KEY_RIGHTMETA to 0x7e (126) so the right-side modifiers are distinct
from the left ones (modify the constant declarations for KEY_RIGHTCTRL,
KEY_RIGHTALT, KEY_LEFTMETA, and KEY_RIGHTMETA).
Comment on lines
+138
to
+149
| const createUinputDevice = async ( | ||
| fd: number, | ||
| config: VirtualInputConfig, | ||
| ): Promise<void> => { | ||
| const buffer = Buffer.alloc(4 + 4 + 4 + 4 + 80) | ||
| buffer.writeUInt16LE(config.vendorId ?? 0x1234, 0) | ||
| buffer.writeUInt16LE(config.productId ?? 0x5678, 2) | ||
| buffer.writeUInt16LE(config.version ?? 1, 4) | ||
| const nameBuffer = Buffer.from(`${config.name.slice(0, 79)}\0`) | ||
| nameBuffer.copy(buffer, 8) | ||
| fsSync.writeSync(fd, buffer) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify uinput_user_dev struct layout from Linux headers
curl -sL "https://raw.githubusercontent.com/torvalds/linux/master/include/uapi/linux/uinput.h" | grep -A 20 "struct uinput_user_dev"Repository: AOSSIE-Org/Rein
Length of output: 2707
🏁 Script executed:
#!/bin/bash
# Get UINPUT_MAX_NAME_SIZE and full struct input_id definition
curl -sL "https://raw.githubusercontent.com/torvalds/linux/master/include/uapi/linux/uinput.h" | grep -E "(UINPUT_MAX_NAME_SIZE|#define.*UINPUT)"
echo "---"
curl -sL "https://raw.githubusercontent.com/torvalds/linux/master/include/uapi/linux/input.h" | grep -A 10 "struct input_id"Repository: AOSSIE-Org/Rein
Length of output: 2536
uinput_user_dev struct layout is incorrect — device registration will fail.
The Linux uinput_user_dev struct expects: name[80] first, then struct input_id (bustype, vendor, product, version — 2 bytes each), then ff_effects_max, then abs arrays. The current code writes vendor/product at offset 0, name at offset 8, and omits bustype entirely — this inverted layout will prevent device registration.
Additionally, the buffer is allocated as only 96 bytes, which is insufficient for the full struct including the absolute axis arrays (minimum ~1116 bytes).
Proposed fix
const createUinputDevice = async (
fd: number,
config: VirtualInputConfig,
): Promise<void> => {
- const buffer = Buffer.alloc(4 + 4 + 4 + 4 + 80)
- buffer.writeUInt16LE(config.vendorId ?? 0x1234, 0)
- buffer.writeUInt16LE(config.productId ?? 0x5678, 2)
- buffer.writeUInt16LE(config.version ?? 1, 4)
- const nameBuffer = Buffer.from(`${config.name.slice(0, 79)}\0`)
- nameBuffer.copy(buffer, 8)
+ // uinput_user_dev: name[80], id{bustype,vendor,product,version}, ff_effects_max, abs arrays
+ const UINPUT_USER_DEV_SIZE = 80 + 8 + 4 + (64 * 4 * 4) // name + id + ff + abs arrays
+ const buffer = Buffer.alloc(UINPUT_USER_DEV_SIZE)
+ const nameBuffer = Buffer.from(`${config.name.slice(0, 79)}\0`)
+ nameBuffer.copy(buffer, 0) // name at offset 0
+ buffer.writeUInt16LE(0x03, 80) // BUS_VIRTUAL = 0x06 or BUS_USB = 0x03
+ buffer.writeUInt16LE(config.vendorId ?? 0x1234, 82)
+ buffer.writeUInt16LE(config.productId ?? 0x5678, 84)
+ buffer.writeUInt16LE(config.version ?? 1, 86)
fsSync.writeSync(fd, buffer)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/uinput.ts` around lines 138 - 149, The createUinputDevice function
builds an incorrect uinput_user_dev layout; reorder and expand the buffer to
match the kernel struct: allocate a buffer large enough for name[80] + input_id
(bustype, vendor, product, version as u16) + ff_effects_max (int) + the abs
arrays (approx 1116 bytes total), write the device name into offset 0 (80
bytes), then write bustype, vendorId, productId, version at the input_id offsets
using writeUInt16LE, then write ff_effects_max, and finally zero the abs arrays;
update uses of config (name, vendorId, productId, version, bustype,
ffEffectsMax) and write to fd with fsSync.writeSync(fd, buffer) after
constructing the correctly laid out buffer so uinput_user_dev matches kernel
expectations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
PoC #130 - uinput Virtual Input Devices
PoC #208 - WebRTC Screen Streaming
Testing Notes
Both PoCs required for GSoC 2026 Rein proposal per issue requirements.
Summary by CodeRabbit