feat: implement GStreamer-based screen sharing with HTTP and WebRTC signaling#361
Merged
Conversation
Contributor
WalkthroughThe PR replaces WebSocket-based mirroring and input transport with HTTP signaling, WebRTC peer connections, and RTCDataChannels. It also adds server-side session, host, and GStreamer orchestration, rewires the trackpad/settings routes, updates Linux input setup docs, and adjusts runtime dependencies. ChangesWebRTC signaling and transport overhaul
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TrackpadPage
participant useWebRtcStream
participant apiHandlers
participant HostRunner
participant GstManager
TrackpadPage->>useWebRtcStream: useWebRtcStream({ token })
useWebRtcStream->>apiHandlers: POST /api/session
apiHandlers->>HostRunner: ensureHostRunnerActive()
apiHandlers->>HostRunner: handleIncomingClientOffer(sessionId, sdp)
HostRunner->>GstManager: start(token, whipPort)
useWebRtcStream->>apiHandlers: GET /api/webrtc/events?sessionId=...
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✨ 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 |
![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: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/routes/settings.tsx (2)
52-78: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winMake the config debounce actually cancel prior requests.
setConfigreturns a cleanup function, butonChangecallers ignore it, so every slider/toggle change schedules its own POST.🤖 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/routes/settings.tsx` around lines 52 - 78, The debounce in setConfig is ineffective because its cleanup return value is never used, so each sensitivity/invertScroll change still schedules a separate POST. Update the Settings flow so the prior timeout is actually cleared on subsequent calls, either by managing the timer outside setConfig or by storing it in a ref and canceling it before scheduling a new request. Keep the change localized around setConfig and the onChange handlers that invoke it.
93-98: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not overwrite the runtime frontend port on mount.
Line 18 sets
frontendPortfromwindow.location.port, but this later effect immediately resets it toserverConfig.frontendPort, breaking the runtime-port behavior.Proposed fix
useEffect(() => { const defaultIp = typeof window !== "undefined" ? window.location.hostname : "localhost" setIp(defaultIp) - setFrontendPort(String(serverConfig.frontendPort)) }, [])🤖 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/routes/settings.tsx` around lines 93 - 98, The settings page mount effect is overwriting the runtime frontend port that was already initialized from window.location.port. Update the useEffect in settings.tsx so it only sets the default IP and does not call setFrontendPort(serverConfig.frontendPort) on mount; keep the existing runtime-port value intact and only adjust the frontendPort state when the user explicitly changes it or when it is truly unset.src/routes/trackpad.tsx (1)
1-11: 🎯 Functional Correctness | 🟠 MajorAdd "use client" directive to
src/routes/trackpad.tsx.This file uses React hooks (
useState,useEffect,useRef) and accesseswindow. In a React Server Components environment (such as Next.js), it must be marked as a client component.Place
"use client"before the first import:Diff
+"use client" + import { BufferBar } from "`@/components/Trackpad/Buffer`" import type { ModifierState } from "`@/types`"🤖 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/routes/trackpad.tsx` around lines 1 - 11, The trackpad route component uses React hooks and browser-only APIs, so it needs to be explicitly treated as a client component. Add the "use client" directive at the very top of src/routes/trackpad.tsx before any imports so the Trackpad route and its hooks like useState, useEffect, and useRef can run in a client-side React environment.Source: Path instructions
🤖 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 `@package.json`:
- Around line 25-26: The SSR build is trying to bundle Node-only dependencies,
so update vite.config.ts to explicitly externalize dbus-next and eventsource
under the ssr.external setting. Locate the existing defineConfig export and add
those package names to ssr.external so Vite leaves them unbundled during
server-side build.
In `@README.md`:
- Around line 37-42: The README setup instructions currently add the user to the
broad input group for /dev/uinput access, which should be narrowed. Update the
udev rule and install steps to use a dedicated rein or uinput group instead of
input, and make sure the guidance in the README refers to the setup snippets
around the uinput rule and usermod command so they can be changed consistently.
In `@src/components/Trackpad/ScreenMirror.tsx`:
- Around line 38-47: The mirrored video element in ScreenMirror is currently
unlabeled, so screen readers cannot identify the remote desktop surface. Update
the video element in ScreenMirror to include an accessible name, such as an
appropriate aria-label or equivalent label reference, while keeping the existing
playback and visibility behavior unchanged.
- Around line 28-33: The ScreenMirror effect currently only assigns a
MediaStream when videoStream is present and never clears
videoElementRef.current.srcObject, so stale tracks can remain attached after the
stream changes or the component unmounts. Update the useEffect in ScreenMirror
to explicitly set srcObject to null whenever videoStream is absent or changes,
and add cleanup in the effect return so the video element is detached on
unmount; keep the play() call only for the active stream.
In `@src/contexts/ConnectionProvider.tsx`:
- Around line 53-62: The send path in ConnectionProvider’s message పంపing logic
is silently dropping input when both DataChannels are unavailable. Update the
send flow to use the required fallback path instead of doing nothing: in the
send handling around targetDc/fallbackDc selection, detect when neither channel
is open and route the message through the WebSocket/signaling fallback or
surface a failure, rather than discarding it. Keep the change localized to the
send logic in ConnectionProvider so clicks/keys are not lost during negotiation
or transient channel failures.
- Around line 49-51: The gesture routing in ConnectionProvider’s input dispatch
treats only move/scroll/touch as unordered, so useTrackpadGesture’s zoom events
still go through the ordered data channel. Update the isUnordered check in the
ConnectionProvider logic to include the zoom type alongside move, scroll, and
touch so pinch/zoom gestures are sent via unorderedDcRef.current rather than
orderedDcRef.current.
In `@src/hooks/useRemoteConnection.ts`:
- Around line 3-8: Restore the missing client-component marker in
useRemoteConnection so it can safely use useConnection and React Context in
Next.js App Router. Add the "use client" directive at the top of
src/hooks/useRemoteConnection.ts, keeping the hook’s existing useConnection and
sendCombo logic unchanged so it remains usable from client routes like trackpad.
In `@src/hooks/useWebRtcStream.ts`:
- Around line 123-127: The SSE subscription in useWebRtcStream should be
established before calling sendInputOffer(), since fast server responses can
emit input-answer before the EventSource and its listeners are ready. Move the
EventSource creation and listener registration ahead of the sendInputOffer()
call, keeping the existing sseUrl and SSE handling in useWebRtcStream so the
input peer cannot miss the answer and get stuck negotiating.
- Around line 126-127: The SSE URL construction in useWebRtcStream currently
interpolates sessionId and token directly into the query string, which can break
when values contain reserved characters. Update the URL building logic to use
URLSearchParams in the useWebRtcStream hook so sessionId and token are properly
encoded before creating the EventSource. Keep the existing endpoint path and
only replace the manual string concatenation with encoded query parameter
assembly.
In `@src/routes/trackpad.tsx`:
- Around line 30-34: The token persistence in the trackpad flow should not use
web storage; remove the localStorage write in the useEffect that handles
urlToken and replace it with a safer auth handoff. Update the trackpad component
to rely on a server-issued HttpOnly, Secure, SameSite cookie or a short-lived
pairing/exchange flow, and make sure any code that reads rein_auth_token is
removed or redirected to the new mechanism.
- Around line 92-113: Restore the IME composition guard in handleInput so
onChange events do not process intermediate composition text and then get resent
by onCompositionEnd. Update the input flow in src/routes/trackpad.tsx around
handleInput, onChange, and onCompositionEnd so composed CJK text is only emitted
once after composition finishes, while still handling normal key, backspace, and
enter cases.
In `@src/server/api/apiHandlers.ts`:
- Around line 86-97: The body-size guard in readBody is only rejecting the
promise, so the IncomingMessage continues streaming and raw keeps growing past
the limit. Update readBody to stop processing as soon as the 64 KB threshold is
exceeded by destroying the request or removing the data/end/error listeners, and
make sure the limit violation can only trigger once. Keep the fix localized to
readBody and its req.on("data") handling.
- Around line 685-709: Clear the WHIP polling interval when the client
disconnects so the handler does not write to an aborted response. In the WHIP
handshake flow in apiHandlers.ts, add cleanup tied to the request/response close
events alongside answerCheckInterval, and make sure the interval is cleared
before any later res.writeHead/res.end path in the polling callback. Use the
existing sessionId/session lookup logic in the WHIP handler to locate the right
spot.
- Around line 367-398: The WHIP and GStreamer gateway handlers are exposed
publicly and need loopback-only protection to prevent remote session injection.
Add a requireLocalhost guard at the start of both handleWhipSignalingExchange
and handleGstSignalingGateway so non-local requests are rejected before reading
or parsing the body. Keep the existing session and parsing logic unchanged, and
place the new check alongside the other request validation in those handlers.
In `@src/server/api/InputPeerConnection.ts`:
- Line 1: The issue is not a missing dependency in InputPeerConnection, but that
the native module node-datachannel must be excluded from server bundling. Keep
the import in InputPeerConnection as-is, and update the build configuration in
vite.config.ts (or the Nitro server build config if that is what applies) to
externalize node-datachannel for SSR and rollup so the native addon is never
bundled. Make sure the externalization is applied in the server build path used
by PeerConnection consumers.
In `@src/server/gstreamer/gstManager.ts`:
- Around line 145-164: Redact sensitive GStreamer stderr before any logger
output in gstManager’s stderr data handler. Update the logging path around the
"WARN"/"error"/"ERROR" branches to sanitize the raw logStr so values like
signaller::auth-token=Bearer_${token}, WHIP endpoints, and any session/token
material are masked before calling logger.warn or logger.error. Keep the
preroll-failure handling in the same flow, but ensure any stderr content that is
emitted is passed through a redaction helper first.
- Around line 151-173: The preroll-failure fallback path in GStreamer process
handling is losing process ownership because stop() clears this.process before
triggerTestFallbackPipeline() starts the replacement, and the original close
handler later nulls the active process and may emit exit. Update the GstManager
process lifecycle so the fallback is initiated from the original close path or
guarded by the captured child process plus a pending-fallback flag, and ensure
the close handler only clears/emits for the process instance it owns. Apply the
same ownership guard consistently in the related close/cleanup paths in
GstManager so the fallback pipeline remains active and the session is not
dropped.
- Around line 136-137: `GstManager` needs safer process handling in the main and
fallback `spawn` paths: add `error` event listeners to the `gst-launch-1.0`
process creation sites in `spawn` so unavailable binaries or startup failures
are handled without crashing, redact sensitive values such as
`signaller::auth-token=Bearer_${token}` before any stdout/stderr logging, and
update the `close` handling in the pipeline lifecycle so it only clears
`this.process` if it still refers to the same process instance, preserving a
newly started fallback process.
In `@src/server/gstreamer/utils.ts`:
- Around line 38-64: The waitForPortalResponse helper leaves the dbusConnection
"message" listener and timeout active after Promise.race settles, so stale
handlers can accumulate across portal failures. Update waitForPortalResponse to
track both the message handler and the timeout timer, remove the listener and
clear the timer in every success/failure path, and ensure the Promise.race
cleanup happens whether the portal response arrives first or the timeout wins.
- Around line 13-14: Replace the `any` usage in `GstreamerUtils` with explicit
D-Bus types by typing `dbusConnection` and `dbusModule` from `dbus-next` (for
example, using `SessionBus`/`Message` or a strict interface), and update the
`waitForPortalResponse` message handler signature accordingly. Also fix the
listener leak in `waitForPortalResponse` by ensuring the `"message"` handler is
always detached when the timeout branch wins, ideally by centralizing cleanup in
a `finally` block or in the timeout callback so the listener is removed
regardless of how `Promise.race` settles.
In `@src/server/server.ts`:
- Around line 88-103: Add an error listener to startWhipInternalServer’s HTTP
server and handle listen failures gracefully, since
server.listen(WHIP_INTERNAL_PORT, ...) can throw an uncaught EADDRINUSE when
attachSignalingRoutes triggers the internal WHIP server more than once or
another process is already bound. Update startWhipInternalServer (and its
server.listen call) to guard against double-starts, log the failure through
logger, and avoid crashing the process if the internal endpoint is already
active or unavailable.
---
Outside diff comments:
In `@src/routes/settings.tsx`:
- Around line 52-78: The debounce in setConfig is ineffective because its
cleanup return value is never used, so each sensitivity/invertScroll change
still schedules a separate POST. Update the Settings flow so the prior timeout
is actually cleared on subsequent calls, either by managing the timer outside
setConfig or by storing it in a ref and canceling it before scheduling a new
request. Keep the change localized around setConfig and the onChange handlers
that invoke it.
- Around line 93-98: The settings page mount effect is overwriting the runtime
frontend port that was already initialized from window.location.port. Update the
useEffect in settings.tsx so it only sets the default IP and does not call
setFrontendPort(serverConfig.frontendPort) on mount; keep the existing
runtime-port value intact and only adjust the frontendPort state when the user
explicitly changes it or when it is truly unset.
In `@src/routes/trackpad.tsx`:
- Around line 1-11: The trackpad route component uses React hooks and
browser-only APIs, so it needs to be explicitly treated as a client component.
Add the "use client" directive at the very top of src/routes/trackpad.tsx before
any imports so the Trackpad route and its hooks like useState, useEffect, and
useRef can run in a client-side React environment.
🪄 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: 23e97f8f-bcde-4027-b85d-f77fd35bc176
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
README.mdpackage.jsonsrc/components/Trackpad/ScreenMirror.tsxsrc/contexts/ConnectionProvider.tsxsrc/hooks/useCaptureProvider.tssrc/hooks/useMirrorStream.tssrc/hooks/useRemoteConnection.tssrc/hooks/useWebRtcStream.tssrc/routes/__root.tsxsrc/routes/settings.tsxsrc/routes/trackpad.tsxsrc/server-config.jsonsrc/server/InputHandler.tssrc/server/api/InputPeerConnection.tssrc/server/api/apiHandlers.tssrc/server/api/apiState.tssrc/server/api/getLocalIp.tssrc/server/gstreamer/captureProvider.tssrc/server/gstreamer/gstManager.tssrc/server/gstreamer/hostRunner.tssrc/server/gstreamer/utils.tssrc/server/server.tssrc/server/types.tssrc/server/websocket.tsvite.config.ts
💤 Files with no reviewable changes (5)
- src/hooks/useMirrorStream.ts
- src/hooks/useCaptureProvider.ts
- src/server/websocket.ts
- src/server/types.ts
- src/routes/__root.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/__root.tsx (1)
16-25: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winWrap error/not-found rendering with
ConnectionProvidertoo.
errorComponentrendersRootDocumentdirectly, butRootDocumentrendersNavbar→LatencyBadge→useConnection(). OutsideConnectionProvider, that error UI can throw while handling the original route error.Proposed fix
errorComponent: (props) => { return ( - <RootDocument> - <div>Error: {props.error.message}</div> - </RootDocument> + <ConnectionProvider> + <RootDocument> + <div>Error: {props.error.message}</div> + </RootDocument> + </ConnectionProvider> ) }, - notFoundComponent: () => <div>Not Found</div>, + notFoundComponent: () => ( + <ConnectionProvider> + <RootDocument> + <div>Not Found</div> + </RootDocument> + </ConnectionProvider> + ),Also applies to: 28-33, 82-84
🤖 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/routes/__root.tsx` around lines 16 - 25, The root route’s error and not-found UIs are rendering RootDocument without the same provider tree as the normal AppWithConnection path, so Navbar/LatencyBadge can still call useConnection() and crash. Update createRootRoute’s errorComponent and notFoundComponent to wrap RootDocument in ConnectionProvider (or a shared wrapper used by AppWithConnection) so all root-level render paths have connection context available.
♻️ Duplicate comments (7)
src/server/gstreamer/gstManager.ts (3)
210-224: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd an
errorhandler for the fallback GStreamer process.The fallback
spawncan still emit an unhandled"error"event, which can crash the Node process. Mirror the main spawn error handling and guardthis.processby process identity.🛡️ Proposed fix
const proc = spawn("gst-launch-1.0", pipelineArgs, { env: spawnedEnv }) this.process = proc this.intentionalStop = false + + proc.on("error", (err) => { + logger.error(`GStreamer fallback spawn failed: ${err.message}`) + if (this.process === proc) this.process = null + this.emit("capture-failure", err) + }) proc.stderr?.on("data", (data: Buffer) => {🤖 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/server/gstreamer/gstManager.ts` around lines 210 - 224, The fallback process created in GStreamerManager’s spawn flow only handles stderr and close, so an unhandled spawn error can still crash the app. Add an explicit proc.on("error") handler alongside the existing close logic, matching the main spawn error handling pattern, and make sure both the error and close handlers only clear this.process and emit exit when the proc matches the current process identity.
162-180: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftPreserve process ownership during fallback handoff.
The preroll path calls
stop(), starts fallback, and resetsintentionalStop; when the original process later emits"close", Line 176 can null out the fallback process and Line 179 can emit"exit"for an active fallback session. Capture the child process and only clear/emit for the process instance that closed.🤖 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/server/gstreamer/gstManager.ts` around lines 162 - 180, The preroll fallback handoff in GstManager is losing process ownership when the original child later closes, which can incorrectly clear the active fallback pipeline and emit exit for the wrong session. Update the process lifecycle handling in the gstreamer manager’s on("close") path to capture the specific child process instance and only run cleanup, nulling, and exit emission when the closing process matches the one currently owned by the class. Use the existing GstManager.process, stop(), triggerTestFallbackPipeline(), cleanup(), and intentionalStop flow to ensure fallback handoff does not get overwritten by a stale close event.
214-217: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRedact fallback stderr before logging.
The main stderr path now masks
auth-token, but fallback stderr still logs raw GStreamer output and can exposesignaller::auth-token=Bearer_${token}.🔒 Proposed fix
proc.stderr?.on("data", (data: Buffer) => { + const logStr = data + .toString() + .replace(/auth-token=\S+/g, "auth-token=REDACTED") logger.warn( - `GStreamer fallback [${this.sessionId}]: ${data.toString().trim()}`, + `GStreamer fallback [${this.sessionId}]: ${logStr.trim()}`, ) })As per path instructions, “Security: No exposed API keys or sensitive data.”
🤖 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/server/gstreamer/gstManager.ts` around lines 214 - 217, The fallback stderr logging in gstreamer’s proc.stderr handler is still emitting raw GStreamer output, which can leak sensitive auth-token values. Update the stderr handling in gstManager.ts (the proc.stderr?.on("data") callback inside the GStreamer process management flow) to apply the same redaction used in the main stderr path before calling logger.warn, so any signaller::auth-token or similar secrets are masked consistently.Source: Path instructions
src/server/api/apiHandlers.ts (2)
663-675: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winKeep WHIP exchange loopback-only before reading the offer.
This handler is still registered on the public
/api/webrtc/whiproute and reachesreadBodywithoutrequireLocalhostor auth, so a LAN client can inject offers for a knownsessionId. Add the guard before URL/body processing.🔒 Proposed fix
export async function handleWhipSignalingExchange( req: IncomingMessage, res: ServerResponse, ): Promise<void> { + if (!requireLocalhost(req, res)) return const url = new URL(req.url ?? "", `http://${req.headers.host}`)As per path instructions, “Security: Check for common security vulnerabilities” and “No exposed API keys or sensitive data.”
🤖 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/server/api/apiHandlers.ts` around lines 663 - 675, The WHIP signaling handler is reachable on a public route before any locality check, so add the loopback-only guard at the start of handleWhipSignalingExchange before parsing the URL or calling readBody. Reuse the existing requireLocalhost-style protection used elsewhere in the API handlers and make sure the function returns immediately for non-local requests, preventing LAN clients from submitting offers for a known sessionId.Source: Path instructions
689-712: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClear WHIP polling on response close as well.
Line 712 only observes
req.close, which may already have fired afterreadBody; if the client disconnects while the handler is waiting for an answer, the interval can still write to an abortedres. Register one cleanup function onres.closetoo.🛡️ Proposed fix
let checkCount = 0 + const cleanup = () => clearInterval(answerCheckInterval) const answerCheckInterval = setInterval(() => { const activeSession = sessions.get(sessionId) checkCount++ @@ res.end(JSON.stringify({ error: "WHIP signaling handshake timeout" })) } }, 100) - req.on("close", () => clearInterval(answerCheckInterval)) + req.on("close", cleanup) + res.on("close", cleanup) }🤖 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/server/api/apiHandlers.ts` around lines 689 - 712, The WHIP polling cleanup in apiHandlers.ts only stops the interval on req.close, so the handler can still try to write to an aborted response if the client disconnects while waiting for an answer. Update the WHIP handshake logic around the answerCheckInterval to use a shared cleanup function and register it for both req.close and res.close, ensuring the interval is cleared whenever either side closes. Keep the existing sessionId polling behavior, res.writeHead/res.end handling, and logger.info flow in place.src/server/server.ts (1)
108-150: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake route attachment and WHIP startup idempotent.
attachSignalingRoutescan be called more than once, and each call prepends another request listener and callsstartWhipInternalServer(). The newerrorhandler avoids an uncaughtEADDRINUSE, but duplicate listeners can still execute state-changing/apihandlers multiple times on the same request.🛡️ Proposed fix
+let signalingRoutesAttached = false +let whipInternalServerStarted = false + function startWhipInternalServer(): void { + if (whipInternalServerStarted) return + whipInternalServerStarted = true const server = http.createServer((req, res) => { @@ server.on("error", (err) => { + whipInternalServerStarted = false logger.error(`WHIP internal server failed to start: ${String(err)}`) }) } @@ export function attachSignalingRoutes( server: NonNullable<import("vite").ViteDevServer["httpServer"]>, ): void { + if (signalingRoutesAttached) return + signalingRoutesAttached = true server.prependListener(🤖 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/server/server.ts` around lines 108 - 150, Make `attachSignalingRoutes` idempotent so repeated calls do not add duplicate request listeners or start WHIP more than once. Add a guard around the `server.prependListener("request", ...)` registration and the `startWhipInternalServer()` call so they only run the first time `attachSignalingRoutes` is invoked, preserving the existing route matching and error handling logic without re-attaching handlers.src/server/gstreamer/utils.ts (1)
49-78: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClean up the D-Bus listener and timeout in all paths.
Promise.racestill leaves the"message"handler registered when the timeout wins, and leaves the timer alive when the response wins. Use one promise with centralized cleanup.🛡️ Proposed fix
private async waitForPortalResponse( requestPath: string, ): Promise<Record<string, DBus.Variant>> { - return Promise.race([ - new Promise<Record<string, DBus.Variant>>((resolve, reject) => { - if (!this.dbusConnection) return - const handler = (msg: Message) => { + return new Promise<Record<string, DBus.Variant>>((resolve, reject) => { + if (!this.dbusConnection) { + reject(new Error("D-Bus connection is not initialized")) + return + } + let timeout: ReturnType<typeof setTimeout> + const cleanup = () => { + clearTimeout(timeout) + this.dbusConnection?.removeListener("message", handler) + } + const handler = (msg: Message) => { if (msg.path !== requestPath) return if (msg.interface !== "org.freedesktop.portal.Request") return if (msg.member !== "Response") return - this.dbusConnection?.removeListener("message", handler) + cleanup() const [responseCode, results] = msg.body if (responseCode === 0) resolve(results) @@ - } - this.dbusConnection.on("message", handler) - }), - new Promise<Record<string, DBus.Variant>>((_, reject) => - setTimeout(() => reject(new Error("Portal request timeout")), 30000), - ), - ]) + } + timeout = setTimeout(() => { + cleanup() + reject(new Error("Portal request timeout")) + }, 30000) + this.dbusConnection.on("message", handler) + }) }🤖 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/server/gstreamer/utils.ts` around lines 49 - 78, In waitForPortalResponse, the Promise.race setup leaves the D-Bus "message" listener and the timeout active when either side wins, so refactor this into a single Promise with centralized cleanup. Use the existing waitForPortalResponse method and its handler logic to register the listener and timer, then ensure both are always cleared on success, cancellation, timeout, and any rejection by removing the message listener and clearing the timeout before resolving or rejecting.
🤖 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/components/Trackpad/ScreenMirror.tsx`:
- Line 16: Externalize the new user-facing strings in ScreenMirror so they come
from the i18n resource layer instead of inline literals. Update the
TEXTS.AUTOMATIC constant and the video element’s aria-label usage to pull from
localized resources, and keep the rendering logic in ScreenMirror aligned with
the existing text/resource pattern used elsewhere in the component.
In `@src/hooks/useWebRtcStream.ts`:
- Around line 110-113: The SSE connection setup in useWebRtcStream currently
appends the bearer token to the EventSource URL, which exposes sensitive
credentials in logs. Update the EventSource creation flow to avoid passing token
through URLSearchParams and sseUrl; instead use a cookie-based approach or a
short-lived SSE-specific token obtained through a safer channel. Keep the
existing sessionId handling intact, and adjust any server-side auth path that
reads the SSE request so it no longer depends on a query-string token.
- Around line 215-228: The sendInputOffer flow in useWebRtcStream currently
assumes the /api/webrtc/input-offer request succeeded because fetch only rejects
on network errors. Update sendInputOffer to inspect the fetch response before
proceeding, and throw or handle an error when the status is not ok so failed
auth/session validation is surfaced instead of leaving the input peer hanging.
Keep the fix close to the existing createOffer, setLocalDescription, and fetch
call sequence, and make sure the error path is handled by the existing
catch(console.error) or equivalent.
In `@src/server/drivers/linux/index.ts`:
- Around line 74-89: The Linux driver’s open() method in the driver class still
promises a boolean result, but it now throws on uinput permission failures,
which breaks the existing degraded initialization path. Update open() to
preserve the boolean failure contract by catching the error, logging it, and
returning false, or else change the init flow that calls the setup*Device()
methods to handle the exception consistently; keep the behavior aligned with the
existing startup logic that expects false from device setup failures.
---
Outside diff comments:
In `@src/routes/__root.tsx`:
- Around line 16-25: The root route’s error and not-found UIs are rendering
RootDocument without the same provider tree as the normal AppWithConnection
path, so Navbar/LatencyBadge can still call useConnection() and crash. Update
createRootRoute’s errorComponent and notFoundComponent to wrap RootDocument in
ConnectionProvider (or a shared wrapper used by AppWithConnection) so all
root-level render paths have connection context available.
---
Duplicate comments:
In `@src/server/api/apiHandlers.ts`:
- Around line 663-675: The WHIP signaling handler is reachable on a public route
before any locality check, so add the loopback-only guard at the start of
handleWhipSignalingExchange before parsing the URL or calling readBody. Reuse
the existing requireLocalhost-style protection used elsewhere in the API
handlers and make sure the function returns immediately for non-local requests,
preventing LAN clients from submitting offers for a known sessionId.
- Around line 689-712: The WHIP polling cleanup in apiHandlers.ts only stops the
interval on req.close, so the handler can still try to write to an aborted
response if the client disconnects while waiting for an answer. Update the WHIP
handshake logic around the answerCheckInterval to use a shared cleanup function
and register it for both req.close and res.close, ensuring the interval is
cleared whenever either side closes. Keep the existing sessionId polling
behavior, res.writeHead/res.end handling, and logger.info flow in place.
In `@src/server/gstreamer/gstManager.ts`:
- Around line 210-224: The fallback process created in GStreamerManager’s spawn
flow only handles stderr and close, so an unhandled spawn error can still crash
the app. Add an explicit proc.on("error") handler alongside the existing close
logic, matching the main spawn error handling pattern, and make sure both the
error and close handlers only clear this.process and emit exit when the proc
matches the current process identity.
- Around line 162-180: The preroll fallback handoff in GstManager is losing
process ownership when the original child later closes, which can incorrectly
clear the active fallback pipeline and emit exit for the wrong session. Update
the process lifecycle handling in the gstreamer manager’s on("close") path to
capture the specific child process instance and only run cleanup, nulling, and
exit emission when the closing process matches the one currently owned by the
class. Use the existing GstManager.process, stop(),
triggerTestFallbackPipeline(), cleanup(), and intentionalStop flow to ensure
fallback handoff does not get overwritten by a stale close event.
- Around line 214-217: The fallback stderr logging in gstreamer’s proc.stderr
handler is still emitting raw GStreamer output, which can leak sensitive
auth-token values. Update the stderr handling in gstManager.ts (the
proc.stderr?.on("data") callback inside the GStreamer process management flow)
to apply the same redaction used in the main stderr path before calling
logger.warn, so any signaller::auth-token or similar secrets are masked
consistently.
In `@src/server/gstreamer/utils.ts`:
- Around line 49-78: In waitForPortalResponse, the Promise.race setup leaves the
D-Bus "message" listener and the timeout active when either side wins, so
refactor this into a single Promise with centralized cleanup. Use the existing
waitForPortalResponse method and its handler logic to register the listener and
timer, then ensure both are always cleared on success, cancellation, timeout,
and any rejection by removing the message listener and clearing the timeout
before resolving or rejecting.
In `@src/server/server.ts`:
- Around line 108-150: Make `attachSignalingRoutes` idempotent so repeated calls
do not add duplicate request listeners or start WHIP more than once. Add a guard
around the `server.prependListener("request", ...)` registration and the
`startWhipInternalServer()` call so they only run the first time
`attachSignalingRoutes` is invoked, preserving the existing route matching and
error handling logic without re-attaching handlers.
🪄 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: 1257df48-3a50-4685-94b6-495d68cb47a0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
package.jsonsrc/components/Trackpad/ScreenMirror.tsxsrc/contexts/ConnectionProvider.tsxsrc/hooks/useRemoteConnection.tssrc/hooks/useWebRtcStream.tssrc/routes/__root.tsxsrc/server/api/apiHandlers.tssrc/server/api/apiState.tssrc/server/drivers/linux/index.tssrc/server/gstreamer/captureProvider.tssrc/server/gstreamer/gstManager.tssrc/server/gstreamer/utils.tssrc/server/server.tstest-input.tsvite.config.ts
Comment on lines
+110
to
+113
| const sseParams = new URLSearchParams({ sessionId: activeSessionId }) | ||
| if (token) sseParams.set("token", token) | ||
| const sseUrl = `/api/webrtc/events?${sseParams.toString()}` | ||
| const sse = new EventSource(sseUrl) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Avoid putting bearer tokens in the SSE URL.
EventSource URLs commonly appear in browser/proxy/server logs, so passing token as a query parameter exposes the same credential used in Authorization. Prefer an HttpOnly/SameSite cookie or a short-lived, single-purpose SSE token instead. As per path instructions, “No exposed API keys or sensitive data” and “Sensitive data exposure”.
🤖 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/hooks/useWebRtcStream.ts` around lines 110 - 113, The SSE connection
setup in useWebRtcStream currently appends the bearer token to the EventSource
URL, which exposes sensitive credentials in logs. Update the EventSource
creation flow to avoid passing token through URLSearchParams and sseUrl; instead
use a cookie-based approach or a short-lived SSE-specific token obtained through
a safer channel. Keep the existing sessionId handling intact, and adjust any
server-side auth path that reads the SSE request so it no longer depends on a
query-string token.
Source: Path instructions
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/server.ts (1)
95-98: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid parsing API paths with the untrusted Host header.
new URL(..., \http://${req.headers.host}`)can throw on a malformedHostheader before any error handling runs. Since onlypathname` is needed, use a fixed base and fail closed.🛡️ Proposed fix
- const pathname = new URL( - req.url ?? "", - `http://${req.headers.host ?? "localhost"}`, - ).pathname + let pathname: string + try { + pathname = new URL(req.url ?? "", "http://localhost").pathname + } catch { + return + }🤖 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/server/server.ts` around lines 95 - 98, The pathname parsing in the request handling flow should not depend on req.headers.host, since the untrusted Host header can throw before error handling. Update the URL construction in the server request logic to use a fixed safe base instead of req.headers.host, and keep only the pathname extraction/fail-closed behavior so malformed Host values cannot affect parsing. Locate this in the server request handler around the pathname assignment that uses new URL(...).
🧹 Nitpick comments (1)
src/components/Trackpad/ScreenMirror.tsx (1)
17-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor inconsistency:
TEXTS.AUTOMATICwrapper vs. directt()calls.
getWaitingText/getSubTextcallt()directly for every other status text, but the default branch ofgetSubTextroutes through theTEXTSobject's getter for a single value. Consider removingTEXTSand inliningt("screenMirror", "establishingSecure")for consistency.♻️ Proposed simplification
-const TEXTS = { - get AUTOMATIC() { - return t("screenMirror", "establishingSecure") - }, -} - export const ScreenMirror = ({ @@ switch (status) { case "disconnected": return t("screenMirror", "checkNetwork") case "connected": return t("screenMirror", "settingUpScreen") default: - return TEXTS.AUTOMATIC + return t("screenMirror", "establishingSecure") } }Also applies to: 57-66
🤖 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/components/Trackpad/ScreenMirror.tsx` around lines 17 - 20, The `TEXTS.AUTOMATIC` getter in `ScreenMirror` is inconsistent with the rest of the status copy, since `getWaitingText` and `getSubText` already call `t()` directly. Remove the `TEXTS` wrapper and inline `t("screenMirror", "establishingSecure")` in the default branch of `getSubText`, keeping the translation lookup consistent with the other text branches and simplifying the `ScreenMirror` component.
🤖 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/server/server.ts`:
- Around line 149-157: The route execution in server.ts only catches promise
rejections, so synchronous throws from route.handler can escape before
Promise.resolve(...) attaches .catch(). Update the handler invocation around
reinStorage.run and route.handler to ensure synchronous failures are captured
too, either by wrapping the call in try/catch or by deferring it with
Promise.resolve().then(...). Keep the existing logger.error and json fallback
behavior in the same error path.
- Around line 134-139: The SSE wrapper in server.ts is incorrectly suppressing
server-originated writes when reinStorage is absent, so later broadcasts from
pushEvent() are dropped even though res.write reports success. Update the
res.write override in the server response handling path to preserve an unwrapped
server-owned writer (or otherwise distinguish request-owned writes from
server-owned SSE broadcasts), and have pushEvent() use that preserved writer
instead of the AsyncLocalStorage-gated wrapper. Keep the fix centered around the
existing res.write override and pushEvent() flow so SSE events continue to send
after the original request handler returns.
---
Outside diff comments:
In `@src/server/server.ts`:
- Around line 95-98: The pathname parsing in the request handling flow should
not depend on req.headers.host, since the untrusted Host header can throw before
error handling. Update the URL construction in the server request logic to use a
fixed safe base instead of req.headers.host, and keep only the pathname
extraction/fail-closed behavior so malformed Host values cannot affect parsing.
Locate this in the server request handler around the pathname assignment that
uses new URL(...).
---
Nitpick comments:
In `@src/components/Trackpad/ScreenMirror.tsx`:
- Around line 17-20: The `TEXTS.AUTOMATIC` getter in `ScreenMirror` is
inconsistent with the rest of the status copy, since `getWaitingText` and
`getSubText` already call `t()` directly. Remove the `TEXTS` wrapper and inline
`t("screenMirror", "establishingSecure")` in the default branch of `getSubText`,
keeping the translation lookup consistent with the other text branches and
simplifying the `ScreenMirror` component.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8106b87b-8a0f-44cb-87e5-71370ea40613
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
package.jsonsrc/components/Trackpad/ScreenMirror.tsxsrc/contexts/ConnectionProvider.tsxsrc/hooks/useRemoteConnection.tssrc/hooks/useWebRtcStream.tssrc/routes/__root.tsxsrc/routes/trackpad.tsxsrc/server/api/apiHandlers.tssrc/server/api/apiState.tssrc/server/drivers/linux/index.tssrc/server/gstreamer/captureProvider.tssrc/server/gstreamer/gstManager.tssrc/server/gstreamer/hostRunner.tssrc/server/gstreamer/utils.tssrc/server/server.tssrc/utils/i18n.tstest-input.tsvite.config.ts
✅ Files skipped from review due to trivial changes (1)
- test-input.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- src/server/gstreamer/hostRunner.ts
- src/server/gstreamer/captureProvider.ts
- src/hooks/useRemoteConnection.ts
- src/contexts/ConnectionProvider.tsx
- vite.config.ts
- src/routes/__root.tsx
- src/server/drivers/linux/index.ts
- src/server/gstreamer/utils.ts
- src/hooks/useWebRtcStream.ts
- src/server/gstreamer/gstManager.ts
- src/routes/trackpad.tsx
- src/server/api/apiHandlers.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/server/api/apiHandlers.ts`:
- Around line 713-738: The WHIP polling cleanup in apiHandlers.ts only listens
to req.on("close"), so a timer tick can still try to write to a closed response.
Update the WHIP poller logic around answerCheckInterval in the request handler
to also clear the interval on res.on("close"), and guard the success/timeout
branches so they return without calling res.writeHead or res.end when the
response is already destroyed or ended.
In `@src/server/server.ts`:
- Around line 106-118: The server response wrapper in server.ts is storing an
unused __reinOriginalWrite field on the anyRes object, which is never read
anywhere in the flow. Remove the __reinOriginalWrite property from the
ServerResponse augmentation and the assignment in the response-wrapping logic,
keeping the existing originalWrite/originalEnd handling and the pushEvent
wrapper 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 915cccbb-1fac-4973-8c12-c635850a3c33
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
README.mdbiome.jsonpackage.jsonsrc/hooks/useWebRtcStream.tssrc/server/api/apiHandlers.tssrc/server/api/apiState.tssrc/server/drivers/linux/index.tssrc/server/server.tssrc/utils/i18n.ts
Comment on lines
+713
to
+738
| let checkCount = 0 | ||
| const answerCheckInterval = setInterval(() => { | ||
| reinStorage.run(true, () => { | ||
| const activeSession = sessions.get(sessionId) | ||
| checkCount++ | ||
|
|
||
| if (activeSession?.answer) { | ||
| clearInterval(answerCheckInterval) | ||
| res.writeHead(201, { | ||
| "Content-Type": "application/sdp", | ||
| Location: `/api/webrtc/whip?sessionId=${sessionId}`, | ||
| }) | ||
| res.end(activeSession.answer) | ||
| logger.info(`WHIP handshake complete for session: ${sessionId}`) | ||
| } else if ( | ||
| checkCount >= 50 || | ||
| !activeSession || | ||
| activeSession.state === "closed" | ||
| ) { | ||
| clearInterval(answerCheckInterval) | ||
| res.writeHead(408, { "Content-Type": "application/json" }) | ||
| res.end(JSON.stringify({ error: "WHIP signaling handshake timeout" })) | ||
| } | ||
| }) | ||
| }, 100) | ||
| req.on("close", () => clearInterval(answerCheckInterval)) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify WHIP polling cleanup is tied to ServerResponse lifecycle, not only IncomingMessage.
rg -n -C 6 'answerCheckInterval|req\.on\("close"|res\.on\("close"|res\.destroyed|writableEnded' src/server/api/apiHandlers.ts src/server/server.tsRepository: AOSSIE-Org/Rein
Length of output: 3634
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- apiHandlers.ts around WHIP handler ---'
sed -n '660,750p' src/server/api/apiHandlers.ts
echo
echo '--- server.ts response wrapper ---'
sed -n '110,150p' src/server/server.tsRepository: AOSSIE-Org/Rein
Length of output: 3703
Add response-close cleanup for the WHIP poller. req.on("close") clears the interval, but a timer tick can still race with a closed res; wire res.on("close") too and skip writeHead/end once the response is destroyed or ended.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 725-725: Avoid logging sensitive data
Context: logger.info(WHIP handshake complete for session: ${sessionId})
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🤖 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/server/api/apiHandlers.ts` around lines 713 - 738, The WHIP polling
cleanup in apiHandlers.ts only listens to req.on("close"), so a timer tick can
still try to write to a closed response. Update the WHIP poller logic around
answerCheckInterval in the request handler to also clear the interval on
res.on("close"), and guard the success/timeout branches so they return without
calling res.writeHead or res.end when the response is already destroyed or
ended.
Comment on lines
+106
to
+118
| const anyRes = res as ServerResponse & { | ||
| __handledByRein?: boolean | ||
| __reinOriginalWrite?: typeof originalWrite | ||
| } | ||
| anyRes.__handledByRein = true | ||
|
|
||
| const originalSetHeader = res.setHeader.bind(res) | ||
| const originalWriteHead = res.writeHead.bind(res) | ||
| const originalWrite = res.write.bind(res) | ||
| const originalEnd = res.end.bind(res) | ||
|
|
||
| anyRes.__reinOriginalWrite = originalWrite | ||
|
|
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '__reinOriginalWrite' --type=tsRepository: AOSSIE-Org/Rein
Length of output: 153
🏁 Script executed:
#!/bin/bash
rg -n "__reinOriginalWrite|__handledByRein|reinStorage\.run|pushEvent|setHeader|writeHead|res\.write" srcRepository: AOSSIE-Org/Rein
Length of output: 3787
🏁 Script executed:
sed -n '100,170p' src/server/server.ts && printf '\n---\n' && sed -n '60,90p' src/server/api/apiState.tsRepository: AOSSIE-Org/Rein
Length of output: 2926
Remove the unused __reinOriginalWrite field
__reinOriginalWrite is assigned here but never read; pushEvent already wraps res.write directly. Removing it would simplify the wrapper state.
🤖 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/server/server.ts` around lines 106 - 118, The server response wrapper in
server.ts is storing an unused __reinOriginalWrite field on the anyRes object,
which is never read anywhere in the flow. Remove the __reinOriginalWrite
property from the ServerResponse augmentation and the assignment in the
response-wrapping logic, keeping the existing originalWrite/originalEnd handling
and the pushEvent wrapper unchanged.
28 tasks
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
2 participants
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.
Addressed Issues:
Fixes #351
Description
This PR introduces a major overhaul of Rein's screen mirroring and input architecture by replacing the previous WebSocket-based streaming pipeline with a WebRTC-based implementation.
Highlights
Added cross-platform screen mirroring using GStreamer.
Integrated WHIP (WebRTC-HTTP Ingestion Protocol) based signaling for media streaming.
Implemented a lightweight HTTP signaling layer for WebRTC session management.
Added a dedicated WebRTC DataChannel connection for low-latency input events.
Refactored the capture backend into platform-specific providers:
Added automatic platform-specific GStreamer pipeline generation.
Improved session lifecycle management, authentication, ICE candidate exchange, and connection handling.
Removed the previous WebSocket-based screen mirroring implementation.
Successfully tested screen capture and streaming on Windows, Linux (X11 & Wayland), and macOS.
Screenshots/Recordings:
recording.mp4
Functional Verification
Screen Mirror
Authentication
Basic Gestures
Modes & Settings
Advanced Input
Any other gesture or input behavior introduced:
Additional Notes:
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit
/dev/uinput(with udev guidance) and clarified Wayland capture prerequisites.