Implement cross-platform native input injection backends#358
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 54 minutes and 1 second. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
WalkthroughThis PR replaces NutJS/ydotool with Koffi-backed platform injectors (Windows/Linux/macOS), adds shared input contracts and utilities, refactors InputHandler to dispatch per-connection to platform injectors with sanitization/throttling, and rewires client settings/gesture flows to send raw deltas and config updates. ChangesCross-platform Virtual Input Injection and Client-Server Config Pipeline
Sequence DiagramsequenceDiagram
participant Client
participant WebSocket
participant InputHandler
participant Injector
participant OSLevel
Client->>WebSocket: "start-provider" + { config, screenWidth, screenHeight }
WebSocket->>InputHandler: updateConfig(config)
InputHandler->>Injector: updateConfig(config)
Client->>WebSocket: move {dx, dy}
WebSocket->>InputHandler: handleMessage(move)
InputHandler->>Injector: injectMouseMove(dx, dy)
Injector->>OSLevel: native mouse event
Client->>WebSocket: "update-settings" {sensitivity, invertScroll}
WebSocket->>InputHandler: handleMessage(update-settings)
InputHandler->>Injector: updateConfig({sensitivity, invertScroll})
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 30
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/server/websocket.ts (2)
342-353:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAllow
touchthrough the websocket whitelist.
InputHandlerhandles"touch", but the websocket gate rejects it here, so the native touch injector path never runs.Minimal fix
const VALID_INPUT_TYPES = [ "move", "click", "scroll", "key", "text", "zoom", "combo", + "touch", "update-settings", "copy", "paste", ]🤖 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/websocket.ts` around lines 342 - 353, The websocket input whitelist constant VALID_INPUT_TYPES in websocket.ts is missing "touch", which prevents the native touch handling flow (InputHandler's "touch") from ever running; add "touch" to the VALID_INPUT_TYPES array so the websocket gate allows touch events through (update the array literal where VALID_INPUT_TYPES is declared to include the string "touch").
254-259:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist and hot-apply the new scroll settings in
update-config.
SERVER_CONFIG_KEYSstill excludessensitivityandinvertScroll, so this endpoint cannot round-trip the fields you just added tosrc/server-config.json. Even after persisting them, onlyinputThrottleMsis pushed into the live handler.Suggested wiring
const SERVER_CONFIG_KEYS = [ "host", "frontendPort", "address", "inputThrottleMs", + "sensitivity", + "invertScroll", ] ... + } else if (key === "sensitivity") { + const sensitivity = Number(msg.config[key]) + if (!Number.isFinite(sensitivity) || sensitivity <= 0) { + ws.send( + JSON.stringify({ + type: "config-updated", + success: false, + error: "Invalid sensitivity", + }), + ) + return + } + filtered[key] = sensitivity + } else if (key === "invertScroll") { + if (typeof msg.config[key] !== "boolean") { + ws.send( + JSON.stringify({ + type: "config-updated", + success: false, + error: "Invalid invertScroll flag", + }), + ) + return + } + filtered[key] = msg.config[key] } ... if (typeof filtered.inputThrottleMs === "number") { inputHandler.setThrottleMs(filtered.inputThrottleMs) } + if ("sensitivity" in filtered || "invertScroll" in filtered) { + inputHandler.updateConfig({ + sensitivity: filtered.sensitivity as number | undefined, + invertScroll: filtered.invertScroll as boolean | undefined, + }) + }Also applies to: 323-325
🤖 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/websocket.ts` around lines 254 - 259, SERVER_CONFIG_KEYS currently omits the new scroll settings so update-config cannot persist or hot-apply them; add "sensitivity" and "invertScroll" to the SERVER_CONFIG_KEYS array (and the matching array at the other location around the second occurrence) and ensure the update-config handler that currently pushes inputThrottleMs into the live handler also pushes the new keys into the live handler state (use the same code path that updates inputThrottleMs to set sensitivity and invertScroll so they are persisted and hot-applied).src/routes/settings.tsx (1)
35-60:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the rendered settings in state and debounce with a shared timer.
This ref-based rewrite breaks the UI: the sensitivity badge stays on the initial value, and the invert helper text below will not track checkbox changes. The timeout also is not actually debounced because the cleanup returned from
setConfigis ignored by the event handlers, so every slider tick still queues another websocket send.♻️ Suggested fix
- const sensitivity = useRef(initialSensitivity) - const invertScroll = useRef(initialInvert) + const [sensitivity, setSensitivity] = useState(initialSensitivity) + const [invertScroll, setInvertScroll] = useState(initialInvert) + const configUpdateTimer = useRef<number | null>(null) - const setConfig = (sensitivity_val: number, invertedScroll_val: boolean) => { - sensitivity.current = sensitivity_val - invertScroll.current = invertedScroll_val - localStorage.setItem("rein_sensitivity", String(sensitivity_val)) - localStorage.setItem("rein_invert", JSON.stringify(invertedScroll_val)) - const timer = setTimeout(() => { - sendConfigUpdate(sensitivity.current, invertScroll.current) - }, 300) - return () => clearTimeout(timer) - } + const setConfig = (nextSensitivity: number, nextInvertScroll: boolean) => { + setSensitivity(nextSensitivity) + setInvertScroll(nextInvertScroll) + localStorage.setItem("rein_sensitivity", String(nextSensitivity)) + localStorage.setItem("rein_invert", JSON.stringify(nextInvertScroll)) + if (configUpdateTimer.current !== null) { + clearTimeout(configUpdateTimer.current) + } + configUpdateTimer.current = window.setTimeout(() => { + sendConfigUpdate(nextSensitivity, nextInvertScroll) + }, 300) + }- {sensitivity.current.toFixed(1)}x + {sensitivity.toFixed(1)}x ... - defaultValue={sensitivity.current} + value={sensitivity} ... - invertScroll.current, + invertScroll, ... - defaultChecked={invertScroll.current} + checked={invertScroll} ... - setConfig(sensitivity.current, e.target.checked) + setConfig(sensitivity, e.target.checked)Also applies to: 185-200, 222-225
🤖 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 35 - 60, The UI uses refs for sensitivity and invertScroll which prevents re-renders and the returned cleanup from setConfig is ignored, so change to keep rendered settings in state (e.g., useState for sensitivity and invert), still persist to localStorage inside the setter, and debounce websocket sends with a single shared timer stored in a useRef (e.g., configSendTimerRef). Update the setConfig function (and any handlers calling it) to 1) call the state setters, 2) clear any existing timeout via configSendTimerRef.current, 3) create a new timeout that calls sendConfigUpdate with the latest state values, and 4) not rely on returning a cleanup function (since handlers don’t use it); continue to update sensitivity.current/invertScroll.current only if you need ref access elsewhere.src/server/drivers/windows/index.ts (1)
1-169:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFormat the file to resolve CI check failure.
The GitHub check "validate" reports that file content differs from formatting output. Please run your formatter (Biome) to fix this.
#!/bin/bash # Run Biome formatter npx biome check src/server/drivers/windows/index.ts --write🤖 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/drivers/windows/index.ts` around lines 1 - 169, The file fails formatting CI; run the project's Biome formatter to rewrite this file so it matches style (e.g., run npx biome check src/server/drivers/windows/index.ts --write). Ensure you save the formatted changes for the WindowsInputInjector class and its methods (e.g., injectMouseMove, injectMouseButton, injectMouseWheel, injectKey, injectCombo, injectText, injectTouch, destroy) so the formatting-only diff fixes the "validate" check.Source: Linters/SAST tools
src/server/drivers/linux/touch.ts (1)
1-179: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAddress the formatting issue flagged by static analysis.
The validator reports that this file's content differs from the expected formatting output. Please run the project's formatter to ensure consistency.
🤖 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/drivers/linux/touch.ts` around lines 1 - 179, The file fails the project's formatter; run the project's formatting tool (e.g., npm/yarn format or deno fmt as appropriate) and commit the formatted result so the content matches the expected output. Locate the LinuxTouch class (constructor, injectTouch, releaseAll, liftContact, selectSlot, emitToolButtons, sync) and reformat the file; ensure whitespace, indentation, trailing newlines, and import ordering match the project's formatter rules before pushing the change.Source: Linters/SAST tools
src/server/drivers/keyMap.ts (1)
1-353: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAddress the formatting issue flagged by static analysis.
The validator reports that this file's content differs from the expected formatting output. Please run the project's formatter to ensure consistency.
🤖 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/drivers/keyMap.ts` around lines 1 - 353, The file fails the project's formatting check; run the project's formatter (e.g., prettier/eslint --fix or the repo's designated format script) on the file containing VK_MAP, LINUX_KEY_MAP, MAC_KEY_MAP, and SHIFTED_CHARS to normalize spacing, trailing commas, and quote usage; then stage the updated file and re-run the project's lint/format verification (or commit the formatted output) so CI passes.Source: Linters/SAST tools
🤖 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`:
- Line 26: Update the koffi dependency in package.json from "^2.16.2" to the
current major "^3.0.2" (edit the "koffi" entry), then run npm install to update
package-lock.json and finally run npm audit (or npm audit --package-lock-only)
to ensure the lockfile reflects the new version and to re-check for
vulnerabilities; commit the updated package.json and package-lock.json.
In `@src/server/drivers/linux/index.ts`:
- Around line 173-174: screenWidth and screenHeight are hardcoded to
DEFAULT_SCREEN_WIDTH/HEIGHT and never updated; change the driver class
(constructor) to accept optional screenWidth and screenHeight parameters (or
query actual resolution on init) and use those to set this.screenWidth and
this.screenHeight (falling back to DEFAULT_* if absent), and update any code
that relies on screenWidth/screenHeight (e.g., touch coordinate scaling in
methods handling absolute touch events) so coordinates are computed against the
actual dimensions; ensure the constructor signature and the properties
(screenWidth, screenHeight) are the only symbols changed so callers can pass
real display sizes or the class can query them during initialization.
- Around line 68-70: The module-level platform check prevents safe
conditional/dynamic imports; remove the top-level if (process.platform !==
"linux") throw ... and instead perform the platform guard inside the exported
LinuxInputInjector class (e.g., in its constructor) and/or inside any exported
functions/methods (e.g., methods that call native APIs) so the module can be
imported on non-Linux hosts without throwing; keep the same error
message/behavior by throwing an Error("LinuxInputInjector can only be used on
Linux") from the constructor or from methods when called on non-Linux platforms.
In `@src/server/drivers/linux/keyboard.ts`:
- Around line 73-80: The code currently falls back to 0 for the shift key (using
LINUX_KEY_MAP.shift ?? 0) which will emit KEY_RESERVED; instead ensure the shift
key is present and fail loudly: update the logic around sendKeyEvent (the block
that calls sendKeyEvent(LINUX_KEY_MAP.shift ?? 0, KEY_PRESS) and
sendKeyEvent(LINUX_KEY_MAP.shift ?? 0, KEY_RELEASE)) to check that
LINUX_KEY_MAP.shift is defined and throw or return an error if missing (or
assert during initialization), then use the defined value directly when calling
sendKeyEvent with KEY_PRESS and KEY_RELEASE; do not use a 0 fallback. Ensure any
error mentions LINUX_KEY_MAP.shift so callers can remediate.
In `@src/server/drivers/linux/structs.ts`:
- Around line 64-70: The openUinput function currently calls _open(path,
O_WRONLY | O_NONBLOCK) and returns the raw numeric result, which can be -1 on
failure; update openUinput to validate the returned file descriptor from _open:
if the result is a negative value, retrieve the errno (or use libc's errno
representation) and throw a clear Error (e.g., "failed to open uinput:
<errno/description>") instead of returning -1, otherwise return the positive fd;
keep ensureLibc() and the _open/O_WRONLY|O_NONBLOCK call but add the
negative-check and error construction so callers never receive an unvalidated
negative descriptor.
- Around line 95-122: The ioctl* helpers (ioctlInt, ioctlStruct, ioctlNull)
currently return the raw libc _ioctl value (0 on success, -1 on error) without
documenting that callers must check for negative results; update each function
to either (a) add JSDoc above ioctlInt, ioctlStruct, and ioctlNull that clearly
states the return semantics (returns libc's raw int, 0 on success, -1 on error,
and errno is set on error) or (b) make the helpers throw on error by checking
the returned value from _ioctl (use the existing _ioctl symbol) and, when result
< 0, construct and throw an Error that includes context (function name, request,
and the errno value/message if available) so callers get exceptions instead of
raw negative numbers; use ensureLibc, _ioctl, and the exact function names to
locate where to add the JSDoc or the post-call error check/throw.
In `@src/server/drivers/mac/index.ts`:
- Around line 83-92: The injectMouseMove method only switches to drag mode when
"left" is held; update it to check this.buttonsHeld for "right" and "middle" as
well and set eventType to the corresponding drag constants (e.g.,
kCGEventRightMouseDragged, kCGEventOtherMouseDragged) and choose the matching
mouse button constant (e.g., kCGMouseButtonRight, kCGMouseButtonCenter) instead
of always using kCGMouseButtonLeft; adjust the postMouseEvent call to use the
computed eventType and button constant so right- and middle-button drags are
emitted correctly.
- Around line 36-55: BUTTON_MAP currently maps the drag event for both right and
middle buttons to kCGEventLeftMouseDragged; update the right.drag to use
kCGEventRightMouseDragged and middle.drag to use kCGEventOtherMouseDragged, and
ensure those constants (kCGEventRightMouseDragged, kCGEventOtherMouseDragged)
are imported at the top of the file so BUTTON_MAP's entries for right and middle
reference the correct platform drag event constants.
In `@src/server/drivers/mac/keyboard.ts`:
- Around line 55-58: The code is using the fallback "?? 0" for modifier keycodes
which maps to kVK_ANSI_A and injects an unintended 'A'; in both
MacKeyboard.injectText and MacTouch.emitPinchZoom replace the
"MAC_KEY_MAP.<modifier> ?? 0" pattern with an explicit undefined check: lookup
the modifier via MAC_KEY_MAP (e.g., MAC_KEY_MAP.shift or MAC_KEY_MAP.control),
and if it's undefined either log a warning and skip sending the modifier via
postKeyEvent or throw a clear error, ensuring postKeyEvent is only called with
valid keycodes; update both injectText and emitPinchZoom to validate modifier
existence rather than using numeric fallback.
- Around line 46-64: The code in injectText uses MAC_KEY_MAP.shift ?? 0 which
can wrongly synthesize the 'A' key when shift is undefined; update injectText to
only call postKeyEvent for the shift key when MAC_KEY_MAP.shift is explicitly
defined (e.g., check typeof MAC_KEY_MAP.shift !== 'undefined' or use an
if/guard) before posting the shift down and up around the character events,
leaving the existing postKeyEvent(code, true/false) logic unchanged; reference
injectText, MAC_KEY_MAP.shift, and postKeyEvent to locate and fix the logic.
In `@src/server/drivers/mac/structs.ts`:
- Around line 66-107: remove the redundant optional chaining after
ensureFunctions() in postMouseEvent, postKeyEvent, and postScrollEvent: replace
uses like _CGEventCreateMouseEvent?.(...), _CGEventPost?.(...),
_CFRelease?.(...), _CGEventCreateKeyboardEvent?.(...), and
_CGEventCreateScrollWheelEvent?.(...) with non-null assertions (e.g.
!_CGEventCreateMouseEvent(...)) or direct calls so the function pointers are
treated as initialized; keep the existing null checks for ref and run the
project formatter to address the noted formatting warning.
In `@src/server/drivers/mac/touch.ts`:
- Around line 171-179: The emitPinchZoom function uses MAC_KEY_MAP.control ?? 0
which can substitute keycode 0 (a valid key) when control is missing; change
emitPinchZoom to detect missing MAC_KEY_MAP.control explicitly and bail (or
log/warn) instead of defaulting to 0: look up MAC_KEY_MAP.control first, if it
is undefined or null return early (or emit a warning), otherwise call
postKeyEvent(ctrlCode, true), postScrollEvent(...), postKeyEvent(ctrlCode,
false) using that valid ctrlCode; ensure you reference MAC_KEY_MAP.control,
emitPinchZoom, postKeyEvent, postScrollEvent and preserve PINCH_SCROLL_SCALE
behavior.
- Around line 128-169: Replace the hardcoded 1-pixel threshold in
handleTwoFingerMove with a named constant or configurable parameter so pinch vs
pan sensitivity is adjustable: introduce a PINCH_PAN_THRESHOLD_PX (or an options
field on the TouchDriver/constructor) and use Math.abs(delta) >
PINCH_PAN_THRESHOLD_PX instead of 1; ensure the default value preserves current
behavior (1px) but allow callers to override via a config field, and reference
this new value when computing isPinch so emitPinchZoom and postScrollEvent
behavior follows the configurable threshold.
- Around line 64-87: processDowns uses the current first contact coords to post
a LeftMouseUp when a second finger arrives, but that may not match the original
mouse-down location; modify processDowns so the original down position is
preserved and used for the mouse-up. Concretely: when handling the first finger
(active === 1), record its initial coordinates (e.g., attach startX/startY to
the contact stored in this.activeContacts or set a small field like
this.mouseDownPosition); then when handling the second finger arrival (active
=== 2) retrieve that stored startX/startY (or this.mouseDownPosition) instead of
using [...this.activeContacts.values()][0].x/y to call
postMouseEvent(kCGEventLeftMouseUp,...). Ensure the symbols referenced are
processDowns, this.activeContacts, postMouseEvent, and buildPinchState.
In `@src/server/drivers/README.md`:
- Line 54: Add a single trailing newline at the end of the README.md file so the
final line "* Clipboard shortcuts (copy/paste)" is followed by one newline
character; update the file (src/server/drivers/README.md) to ensure it ends with
exactly one newline for Markdown consistency.
In `@src/server/drivers/types.ts`:
- Around line 14-35: The InputMessage interface currently uses a single type
discriminator with all payload fields optional, which prevents safe narrowing;
refactor by replacing InputMessage with a discriminated union of specific
message types (e.g., MoveMessage, PasteMessage, CopyMessage, ClickMessage,
ScrollMessage, KeyMessage, TextMessage, ZoomMessage, ComboMessage,
TouchMessage), each having a fixed literal type property and only the fields
relevant to that message (e.g., MoveMessage has dx and dy required; ClickMessage
has button and press; KeyMessage has key or keys; TouchMessage has contacts),
then export InputMessage as the union of those concrete interfaces and remove
the unrelated optional fields so TypeScript can narrow on the type discriminator
and enforce exhaustive checks in switch statements.
In `@src/server/drivers/utils.ts`:
- Around line 4-20: The applyMotion function uses unexplained magic numbers
(exponent 1.2, multiplier 0.8, and threshold mag < 1) making tuning hard; update
applyMotion to document these values by extracting them into clearly named
constants or config fields (e.g., ACC_EXPONENT, ACC_MULTIPLIER, ACC_THRESHOLD)
and add a brief comment above the computation explaining the formula intent (how
exponent and multiplier shape acceleration vs magnitude) and why the threshold
is used; also consider making them configurable via InputConfig if runtime
tuning is desirable and reference applyMotion, sdx/sdy, mag, acc and ratio when
adding the comments/fields.
In `@src/server/drivers/windows/index.ts`:
- Line 164: The inline comment "//Cleanup" is missing a space after the slashes;
update the comment to "// Cleanup" to match project style. Locate the occurrence
of the comment token "//Cleanup" in src/server/drivers/windows/index.ts (around
the cleanup section) and add a single space after the slashes so it reads "//
Cleanup". Ensure there are no other occurrences of "//Cleanup" in that file and
adjust them similarly for consistency.
- Around line 103-144: The injectMouseWheel function currently applies
this.config.invertScroll only to vertical (dy) but not horizontal (dx); decide
and implement one of two fixes: either apply the same inversion to dx by
computing const horizAmount = this.config.invertScroll ? dx : -dx and using
horizAmount for mouseData in the INPUT_MOUSE with MOUSEEVENTF_HWHEEL (update the
dx branch in injectMouseWheel), or explicitly document and enforce that
invertScroll is vertical-only (add a clarifying comment near injectMouseWheel
and consider adding a new config flag like invertHorizontalScroll if horizontal
inversion is desired later); reference injectMouseWheel,
this.config.invertScroll, and the dx branch/MOUSEEVENTF_HWHEEL to locate
changes.
In `@src/server/drivers/windows/keyboard.ts`:
- Around line 91-128: The injectText method currently sends input for every
character with no length guard; add a maximum allowed length (e.g.,
MAX_INJECT_TEXT_LENGTH) and validate the input at the start of injectText (or
truncate to the max and log a warning) to prevent DoS from extremely long
strings; reference the injectText function and the sendInput call so the check
happens before the for-loop iterating characters and ensure you use a clear log
message when rejecting/truncating input.
- Around line 14-44: The injectKey method currently falls back to injectText for
any single-character key not found in VK_MAP, which can send unintended Unicode
or control chars; update injectKey to validate the character is a
printable/printable-ASCII (e.g., using a helper like isPrintableChar or a regex
checking codepoint ranges and excluding control chars) before calling
injectText, and for non-printable characters either log a warning or handle via
an explicit mapping or ignore; reference injectKey, injectText and VK_MAP when
making the change so you only alter the single-character fallback behavior and
add the small printable-character validator helper.
In `@src/server/drivers/windows/structs.ts`:
- Around line 106-116: The SendInput wrapper forwards parameters to the native
_SendInput without validation; add defensive checks in SendInput: verify count
is a positive integer (>0), size is a positive integer and matches the expected
INPUT struct size constant, and that events is non-null/defined and of an
acceptable type (e.g., Buffer/ArrayBuffer/TypedArray or a native pointer) before
calling _SendInput; throw clear TypeError/RangeError if checks fail, and keep
the existing ensureLib() and _SendInput error handling intact.
- Around line 132-142: Add parameter validation at the top of InjectPointerInput
(before ensureLib() or immediately after) to mirror SendInput: verify that
device and pointerInfo are neither null nor undefined, and that count is a
finite non-negative integer (and ideally >0 if SendInput enforces that). If any
check fails, throw a TypeError with a clear message indicating which argument is
invalid (e.g., "Invalid count for InjectPointerInput" or "pointerInfo is
required"). Keep the existing check for _InjectPointerInput and call it only
after validations pass.
In `@src/server/drivers/windows/touch.ts`:
- Around line 57-68: In getOrAllocPointerId, avoid returning 0 when
freePointerIds is exhausted because 0 is a valid pointer ID and causes
collisions; instead either throw a descriptive error (e.g., "Max contacts
exceeded") or return undefined as a sentinel and update callers to skip/ignore
contacts with undefined allocations; adjust logic around contactIdMap and
freePointerIds (and any caller that expects a number) to handle the new failure
path so injected frames never reuse a valid ID.
- Around line 40-55: In initialize(), after obtaining handle from
CreateSyntheticPointerDevice (used in the PT_TOUCHPAD branch), wrap the call to
koffi.address(handle) in a try-catch and validate its return before assigning to
this.hDevice; if koffi.address throws or returns an unexpected value, log an
error via console.error and do not set this.hDevice (and keep the existing
"Failed to create touch device" path or augment it with the caught error),
ensuring CreateSyntheticPointerDevice, koffi.address, and this.hDevice are
referenced so the fix is easy to locate.
- Around line 175-190: The destroy() method currently clears freePointerIds to
an empty array which loses the original pool; update destroy() in the Touch
injector so it re-initializes freePointerIds back to the full set
(0..MAX_CONTACTS-1) instead of an empty array (or explicitly document that
instances are unusable after destroy); locate the destroy() method and reset
freePointerIds using the same initialization logic used in the constructor
(referencing MAX_CONTACTS), while still clearing contactIdMap, nulling hDevice
and primarySourceId and sending the release events via injectTouch.
- Around line 79-80: Define a concrete type for each frame entry instead of
using Record<string, unknown>: create a FrameEntry (or TouchFrame) type with a
numeric type field and a typed touchInfo object (e.g., { x: number; y: number;
id?: number; pressure?: number } matching how touchInfo is used) and replace the
current frame declaration (const frame: Array<{ type: number; touchInfo:
Record<string, unknown> }> = []) with Array<FrameEntry>. Update any places that
push into or read from frame (including the sort comparator that compares
touchInfo.x / touchInfo.y) to use the new typed fields so you can remove type
assertions and get proper type checking.
In `@src/server/InputHandler.ts`:
- Around line 96-110: The platform-specific injector loading in InputHandler
(the plat branch that constructs LinuxInputInjector and MacInputInjector)
currently uses process.cwd() anchored .ts paths which breaks under ESM/noEmit;
change this to use runtime-resolvable dynamic imports that target the built JS
module relative to the current module (not process.cwd()) — e.g., replace the
require(path.join(process.cwd(), ".../index.ts")) usage with an await
import(...) that resolves via import.meta.url or __dirname-equivalent so you
import the compiled driver JS, then instantiate LinuxInputInjector /
MacInputInjector from that module and assign to this.injector; keep the
platform-switch logic in InputHandler and preserve the constructor call new
LinuxInputInjector(this.config) / new MacInputInjector(this.config).
In `@src/server/websocket.ts`:
- Around line 123-127: The InputHandler is being recreated for every WebSocket
connection due to a local always-null check and also never fully torn down,
causing native injector initialization and leaked device handles/timers; change
the lifecycle so a shared/single InputHandler instance is created only for
native backends (not for mirror-only clients) and not per-connection, ensure the
native Linux injector constructor does not call initialize() implicitly (move
initialization into an explicit initialize() method on the injector or
InputHandler), and update the connection close logic (the close() path) to call
destroy() on the native injector/InputHandler to release device handles/timers;
locate uses of InputHandler, the native Linux injector constructor,
initialize(), close(), and destroy() in the websocket handling code and apply
these changes.
- Around line 43-47: The computed default values from serverConfig (e.g.,
_inputThrottleMs, sensitivity, invertScroll) are being discarded; update new
InputHandler instantiation(s) (the InputHandler constructor calls) to pass these
parsed defaults instead of relying on the hard-coded 8ms and other internal
defaults. Locate where InputHandler is constructed (instances created around the
top-level connection handling and the later block referenced by lines 123-127)
and add parameters or an options object containing _inputThrottleMs,
sensitivity, and invertScroll so each new connection initializes with the
server-config defaults.
---
Outside diff comments:
In `@src/routes/settings.tsx`:
- Around line 35-60: The UI uses refs for sensitivity and invertScroll which
prevents re-renders and the returned cleanup from setConfig is ignored, so
change to keep rendered settings in state (e.g., useState for sensitivity and
invert), still persist to localStorage inside the setter, and debounce websocket
sends with a single shared timer stored in a useRef (e.g., configSendTimerRef).
Update the setConfig function (and any handlers calling it) to 1) call the state
setters, 2) clear any existing timeout via configSendTimerRef.current, 3) create
a new timeout that calls sendConfigUpdate with the latest state values, and 4)
not rely on returning a cleanup function (since handlers don’t use it); continue
to update sensitivity.current/invertScroll.current only if you need ref access
elsewhere.
In `@src/server/drivers/keyMap.ts`:
- Around line 1-353: The file fails the project's formatting check; run the
project's formatter (e.g., prettier/eslint --fix or the repo's designated format
script) on the file containing VK_MAP, LINUX_KEY_MAP, MAC_KEY_MAP, and
SHIFTED_CHARS to normalize spacing, trailing commas, and quote usage; then stage
the updated file and re-run the project's lint/format verification (or commit
the formatted output) so CI passes.
In `@src/server/drivers/linux/touch.ts`:
- Around line 1-179: The file fails the project's formatter; run the project's
formatting tool (e.g., npm/yarn format or deno fmt as appropriate) and commit
the formatted result so the content matches the expected output. Locate the
LinuxTouch class (constructor, injectTouch, releaseAll, liftContact, selectSlot,
emitToolButtons, sync) and reformat the file; ensure whitespace, indentation,
trailing newlines, and import ordering match the project's formatter rules
before pushing the change.
In `@src/server/drivers/windows/index.ts`:
- Around line 1-169: The file fails formatting CI; run the project's Biome
formatter to rewrite this file so it matches style (e.g., run npx biome check
src/server/drivers/windows/index.ts --write). Ensure you save the formatted
changes for the WindowsInputInjector class and its methods (e.g.,
injectMouseMove, injectMouseButton, injectMouseWheel, injectKey, injectCombo,
injectText, injectTouch, destroy) so the formatting-only diff fixes the
"validate" check.
In `@src/server/websocket.ts`:
- Around line 342-353: The websocket input whitelist constant VALID_INPUT_TYPES
in websocket.ts is missing "touch", which prevents the native touch handling
flow (InputHandler's "touch") from ever running; add "touch" to the
VALID_INPUT_TYPES array so the websocket gate allows touch events through
(update the array literal where VALID_INPUT_TYPES is declared to include the
string "touch").
- Around line 254-259: SERVER_CONFIG_KEYS currently omits the new scroll
settings so update-config cannot persist or hot-apply them; add "sensitivity"
and "invertScroll" to the SERVER_CONFIG_KEYS array (and the matching array at
the other location around the second occurrence) and ensure the update-config
handler that currently pushes inputThrottleMs into the live handler also pushes
the new keys into the live handler state (use the same code path that updates
inputThrottleMs to set sensitivity and invertScroll so they are persisted and
hot-applied).
🪄 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: 401231d1-9356-4049-a746-741003f1f637
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
package.jsonsrc/components/Trackpad/ScreenMirror.tsxsrc/hooks/useCaptureProvider.tssrc/hooks/useRemoteConnection.tssrc/hooks/useTrackpadGesture.tssrc/routes/settings.tsxsrc/routes/trackpad.tsxsrc/server-config.jsonsrc/server/InputHandler.tssrc/server/KeyMap.tssrc/server/drivers/README.mdsrc/server/drivers/constants.tssrc/server/drivers/keyMap.tssrc/server/drivers/linux/constants.tssrc/server/drivers/linux/index.tssrc/server/drivers/linux/keyboard.tssrc/server/drivers/linux/structs.tssrc/server/drivers/linux/touch.tssrc/server/drivers/mac/constants.tssrc/server/drivers/mac/index.tssrc/server/drivers/mac/keyboard.tssrc/server/drivers/mac/structs.tssrc/server/drivers/mac/touch.tssrc/server/drivers/types.tssrc/server/drivers/utils.tssrc/server/drivers/windows/constants.tssrc/server/drivers/windows/index.tssrc/server/drivers/windows/keyboard.tssrc/server/drivers/windows/structs.tssrc/server/drivers/windows/touch.tssrc/server/websocket.tssrc/server/ydotool.tssrc/utils/math.ts
💤 Files with no reviewable changes (3)
- src/server/ydotool.ts
- src/server/KeyMap.ts
- src/utils/math.ts
| "@tanstack/react-router-ssr-query": "^1.166.12", | ||
| "@tanstack/react-start": "^1.167.61", | ||
| "@tanstack/router-plugin": "^1.167.32", | ||
| "koffi": "^2.16.2", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest koffi version on npm
npm view koffi version
# Check for security advisories
npm audit --package-lock-only 2>/dev/null || echo "Run 'npm audit' after npm install to check for vulnerabilities"
# Check GitHub advisories for koffi
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "koffi") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'Repository: AOSSIE-Org/Rein
Length of output: 2722
Update koffi to the latest major and re-check audits.
package.jsonpinskoffias^2.16.2, butnpm view koffi versionshows the latest published version is3.0.2(the current range won’t includev3).- GitHub Advisory search returned no known
koffivulnerabilities, andnpm audit --package-lock-onlydidn’t report issues forkoffi. - Run
npm installand thennpm auditso the audit reflects the updated dependency in the lockfile.
🤖 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 `@package.json` at line 26, Update the koffi dependency in package.json from
"^2.16.2" to the current major "^3.0.2" (edit the "koffi" entry), then run npm
install to update package-lock.json and finally run npm audit (or npm audit
--package-lock-only) to ensure the lockfile reflects the new version and to
re-check for vulnerabilities; commit the updated package.json and
package-lock.json.
Comment on lines
+79
to
+80
| const frame: Array<{ type: number; touchInfo: Record<string, unknown> }> = | ||
| [] |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Improve type safety for frame array.
The frame array uses a generic type with Record<string, unknown> for touchInfo. This bypasses type checking and makes the sort comparator on lines 148-154 require verbose type assertions.
♻️ Define a proper frame entry type
+interface PointerTouchInfo {
+ type: number
+ touchInfo: {
+ pointerInfo: {
+ pointerId: number
+ frameId: number
+ pointerType: number
+ pointerFlags: number
+ // ... other fields
+ }
+ // ... other touch fields
+ }
+}
+
injectTouch(contacts: TouchContact[]): void {
if (!this.hDevice || contacts.length === 0) return
// ...
- const frame: Array<{ type: number; touchInfo: Record<string, unknown> }> = []
+ const frame: PointerTouchInfo[] = []This enables type-safe access and cleaner sorting.
🤖 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/drivers/windows/touch.ts` around lines 79 - 80, Define a concrete
type for each frame entry instead of using Record<string, unknown>: create a
FrameEntry (or TouchFrame) type with a numeric type field and a typed touchInfo
object (e.g., { x: number; y: number; id?: number; pressure?: number } matching
how touchInfo is used) and replace the current frame declaration (const frame:
Array<{ type: number; touchInfo: Record<string, unknown> }> = []) with
Array<FrameEntry>. Update any places that push into or read from frame
(including the sort comparator that compares touchInfo.x / touchInfo.y) to use
the new typed fields so you can remove type assertions and get proper type
checking.
Comment on lines
+43
to
47
| const _inputThrottleMs = | ||
| typeof serverConfig.inputThrottleMs === "number" && | ||
| serverConfig.inputThrottleMs > 0 | ||
| ? serverConfig.inputThrottleMs | ||
| : 8 |
Contributor
There was a problem hiding this comment.
Pass the parsed server defaults into each InputHandler.
_inputThrottleMs is computed here but discarded, so every new connection still boots with the constructor’s hard-coded 8ms throttle. The new sensitivity / invertScroll defaults in src/server-config.json are also ignored unless a later provider message overwrites them.
Suggested wiring
- const _inputThrottleMs =
+ const inputThrottleMs =
typeof serverConfig.inputThrottleMs === "number" &&
serverConfig.inputThrottleMs > 0
? serverConfig.inputThrottleMs
: 8
+ const inputConfig = {
+ sensitivity:
+ typeof serverConfig.sensitivity === "number" ? serverConfig.sensitivity : 1,
+ invertScroll: serverConfig.invertScroll === true,
+ }
...
- inputHandler = new InputHandler()
+ inputHandler = new InputHandler(inputConfig, inputThrottleMs)Also applies to: 123-127
🤖 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/websocket.ts` around lines 43 - 47, The computed default values
from serverConfig (e.g., _inputThrottleMs, sensitivity, invertScroll) are being
discarded; update new InputHandler instantiation(s) (the InputHandler
constructor calls) to pass these parsed defaults instead of relying on the
hard-coded 8ms and other internal defaults. Locate where InputHandler is
constructed (instances created around the top-level connection handling and the
later block referenced by lines 123-127) and add parameters or an options object
containing _inputThrottleMs, sensitivity, and invertScroll so each new
connection initializes with the server-config defaults.
Comment on lines
+123
to
+127
| let inputHandler: InputHandler | null = null | ||
| if (!inputHandler) { | ||
| inputHandler = new InputHandler() | ||
| console.log(`[WebSocket] InputHandler initialized`) | ||
| } |
Contributor
There was a problem hiding this comment.
Fix the InputHandler lifecycle for native backends.
This allocates a native injector for every websocket connection, even mirror-only clients. The provided Linux injector constructor immediately calls initialize(), and close never calls destroy(), so device handles/timers can leak across disconnects.
Also applies to: 369-372
🤖 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/websocket.ts` around lines 123 - 127, The InputHandler is being
recreated for every WebSocket connection due to a local always-null check and
also never fully torn down, causing native injector initialization and leaked
device handles/timers; change the lifecycle so a shared/single InputHandler
instance is created only for native backends (not for mirror-only clients) and
not per-connection, ensure the native Linux injector constructor does not call
initialize() implicitly (move initialization into an explicit initialize()
method on the injector or InputHandler), and update the connection close logic
(the close() path) to call destroy() on the native injector/InputHandler to
release device handles/timers; locate uses of InputHandler, the native Linux
injector constructor, initialize(), close(), and destroy() in the websocket
handling code and apply these changes.
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 (6)
src/server/drivers/mac/keyboard.ts (1)
24-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAbort the combo if any key is unresolved.
Right now an unknown combo element is only logged and the remaining keys are still injected. That can synthesize a different shortcut than the client requested, which is especially risky for system-level combos. Treat an unresolved key as a hard stop for the whole combo instead of sending a partial chord.
🤖 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/drivers/mac/keyboard.ts` around lines 24 - 35, The injectCombo method currently logs unknown keys but continues sending the rest of the chord; change it to abort the entire combo when any key is unresolved: inside injectCombo, while iterating over keys from MAC_KEY_MAP (reference: injectCombo and MAC_KEY_MAP), if a lookup returns undefined, call console.warn with the same message and immediately return (do not push any codes or send any events), so no partial chord is synthesized; otherwise continue building codes and proceed as before only when all keys resolved.src/server/drivers/mac/touch.ts (2)
137-166:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftTwo-finger gesture updates are dropped when only the second contact moves.
handleTwoFingerMove()only processes events forthis.pinch.contactIds[0], so any frame where the other finger is the only one that moved is ignored. The pan branch also says it averages both fingers but only uses one finger's delta. This needs pair-level state or move-batch coalescing so either contact can drive scroll/pinch updates correctly.🤖 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/drivers/mac/touch.ts` around lines 137 - 166, handleTwoFingerMove currently early-returns unless the mover is this.pinch.contactIds[0], dropping updates when only the second finger moves; change it so either contact can drive the logic by always retrieving both current contacts from this.activeContacts using this.pinch.contactIds and their previous positions (from the existing prev lookup or a prev-positions map), then compute newSpread and centroid delta from those pairs; use those pair-level values to decide isPinch/isPan, call emitPinchZoom(newSpread - this.pinch.lastSpread) and update this.pinch.lastSpread, or compute averaged dx/dy across both fingers and call postScrollEvent for pan—ensure you reference handleTwoFingerMove, this.pinch.contactIds, this.pinch.lastSpread, emitPinchZoom, activeContacts, and postScrollEvent when making the change.
64-86:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't retain extra contacts after saying they're ignored.
processDowns()inserts every touch intoactiveContactsbefore theactive > 2branch. After a third finger lands, the state stays at 3+ contacts, so the single-finger and two-finger paths stop running until enough fingers lift. If this backend only supports two contacts, reject or drop extras before storing them.🤖 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/drivers/mac/touch.ts` around lines 64 - 86, processDowns currently inserts all incoming TouchContact entries into activeContacts which lets active stay >2 and freezes single/two-finger behavior; change processDowns to reject or drop extras so activeContacts never holds more than two contacts: before adding, filter downs (or after insertion remove) to ensure only the first two unique contact IDs are stored in this.activeContacts, update logic that calls postMouseEvent and this.pinch (in processDowns, using postMouseEvent, buildPinchState and this.activeContacts) to operate only on those retained contacts, and log/ignore any additional touches without persisting them.src/server/drivers/keyMap.ts (1)
257-266:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the missing macOS mapping for
"0".
MAC_KEY_MAPdefines digits1-9but omits0. That means combos such as["cmd", "0"]resolve to only the modifier path on macOS, so common shortcuts like reset-zoom never fire correctly.🔧 Proposed fix
+ "0": 0x1d, "1": 0x12, "2": 0x13, "3": 0x14,🤖 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/drivers/keyMap.ts` around lines 257 - 266, MAC_KEY_MAP is missing the "0" entry which breaks combos like ["cmd","0"]; update the MAC_KEY_MAP object in src/server/drivers/keyMap.ts by adding the "0" key with the appropriate macOS keycode (e.g. add "0": 0x1d) so numeric shortcuts (like cmd+0) resolve correctly alongside the existing "1"-"9" mappings.src/server/InputHandler.ts (1)
180-192:⚠️ Potential issue | 🟠 Major | ⚡ Quick winZoom implementation won't work—Control is released before scroll.
injectKey("control")performs a full press+release cycle, so the Control key is released beforeinjectMouseWheel()executes. For Ctrl+scroll zoom to work, Control must be held during the scroll. You need to use the injector's combo or separate press/release calls.🐛 Proposed fix using combo with wheel injection
The
PlatformInjectorinterface doesn't expose separate key press/release, but you could:
- Add
injectKeyDown/injectKeyUpmethods to the interface, or- Use a different approach for zoom (e.g., a dedicated
injectZoommethod)A workaround using existing APIs (if platform injectors support holding modifiers via combo timing):
case "zoom": { if (!Number.isFinite(msg.delta) || msg.delta === 0) break const MAX_ZOOM_STEP = 5 const delta = msg.delta ?? 0 const scaled = Math.sign(delta) * Math.min(Math.abs(delta) * 0.5, MAX_ZOOM_STEP) const amount = Math.round(-scaled) if (amount !== 0) { - this.injector.injectKey("control") - this.injector.injectMouseWheel(0, amount) - this.injector.injectKey("control") + // Inject Ctrl+wheel as a combo-style sequence + // This requires platform injectors to support held modifiers + this.injector.injectCombo(["control"]) // Press control + // Note: This still won't work - need dedicated zoom/modifier support } break }Consider adding a dedicated
injectZoom(delta: number)method to thePlatformInjectorinterface that each platform implements with proper modifier handling.🤖 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/InputHandler.ts` around lines 180 - 192, The zoom handler in InputHandler.ts uses injector.injectKey("control") which does a press+release so Control is not held during injector.injectMouseWheel; update the implementation so Control is held while scrolling—either extend PlatformInjector with dedicated key press/release methods (e.g., injectKeyDown/injectKeyUp) and change the "zoom" case to call injectKeyDown("control"), injectMouseWheel(0, amount), injectKeyUp("control"), or add a new injector.injectZoom(delta:number) to PlatformInjector and implement it per platform, then call injector.injectZoom(amount) from the "zoom" case; adjust all platform implementations accordingly.src/server/drivers/linux/index.ts (1)
74-87: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueDead code after
openUinputnow throws on failure.Since
openUinput()instructs.tsnow throws anErrorwhen_open()returns a negative fd (lines 70-72), the checkif (fd < 0)at line 76 will never be true—the function throws before returning. The error logging at lines 77-84 is unreachable.Consider wrapping the
openUinput()call in a try/catch if you want to log context before propagating, or remove the dead branch.♻️ Proposed refactor
open(): boolean { - const fd = openUinput(UINPUT_PATH) - if (fd < 0) { - console.error(`[${this.name}] Failed to open ${UINPUT_PATH} (fd=${fd})`) - console.error( - `[${this.name}] Ensure /dev/uinput exists and the process has write permission.`, - ) - console.error( - `[${this.name}] Run: sudo chmod 0660 /dev/uinput or add udev rule.`, - ) + try { + this.fd = openUinput(UINPUT_PATH) + return true + } catch (err) { + console.error(`[${this.name}] Failed to open ${UINPUT_PATH}:`, err) + console.error( + `[${this.name}] Ensure /dev/uinput exists and the process has write permission.`, + ) + console.error( + `[${this.name}] Run: sudo chmod 0660 /dev/uinput or add udev rule.`, + ) return false } - this.fd = fd - return true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/drivers/linux/index.ts` around lines 74 - 87, The open() method currently calls openUinput(UINPUT_PATH) but openUinput now throws on failure, so the fd<0 branch is dead; wrap the openUinput call in a try/catch inside open() (method name: open, symbol: openUinput, constant: UINPUT_PATH) and in the catch log the same contextual messages using this.name plus the caught error (include error.message or the error object) before returning false (or rethrow if you prefer to propagate); on success set this.fd = fd and return true as before.
🤖 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/hooks/useCaptureProvider.ts`:
- Around line 115-123: The JSON payload sent for "start-provider" uses
settings.width/settings.height which are number | undefined; update the code
that builds the config (the object passed to JSON.stringify in
useCaptureProvider.ts) to coerce those values to definite numbers—e.g., use
fallbacks when reading settings.width and settings.height so screenWidth and
screenHeight always satisfy InputConfig (number); keep getConfig() spread as-is
and only replace screenWidth: settings.width with screenWidth: settings.width ??
<defaultNumber> and similarly for screenHeight (choose a sensible default like 0
or a documented constant).
- Around line 105-106: stream.getVideoTracks()[0] may be undefined causing
track.getSettings() to throw; in the startSharing flow inside useCaptureProvider
(where stream is used) add a null check for the track after const track =
stream.getVideoTracks()[0], guard access to track.getSettings() (e.g.,
return/cleanup or fall back to safe defaults if track is missing), ensure you
stop/cleanup the stream and update any state/handlers the same way as the error
path to avoid leaking resources or leaving UI in an inconsistent state.
In `@src/server/drivers/linux/keyboard.ts`:
- Around line 73-91: The code validates LINUX_KEY_MAP.shift into a local
shiftCode but then uses LINUX_KEY_MAP.shift ?? 0 when sending shift
press/release; update the sendKeyEvent calls to use the validated shiftCode
variable (e.g., replace LINUX_KEY_MAP.shift ?? 0 with shiftCode) and remove the
redundant second undefined check in the release block (since the first check
already continues if undefined) inside the method handling key presses in
src/server/drivers/linux/keyboard.ts so the validated value is consistently
used.
In `@src/server/types.ts`:
- Around line 31-54: Replace the duplicated inline touch payload with the
existing TouchContact type to avoid drift: update InputMessage.contacts to be
contacts?: TouchContact[] (instead of the inline object array) and change
PlatformInjector.injectTouch signature to injectTouch(contacts: TouchContact[]):
void (removing the ad-hoc NonNullable<InputMessage["contacts"]> usage); ensure
you import or reference TouchContact where needed so both the message shape and
PlatformInjector use the single canonical TouchContact definition.
---
Outside diff comments:
In `@src/server/drivers/keyMap.ts`:
- Around line 257-266: MAC_KEY_MAP is missing the "0" entry which breaks combos
like ["cmd","0"]; update the MAC_KEY_MAP object in src/server/drivers/keyMap.ts
by adding the "0" key with the appropriate macOS keycode (e.g. add "0": 0x1d) so
numeric shortcuts (like cmd+0) resolve correctly alongside the existing "1"-"9"
mappings.
In `@src/server/drivers/linux/index.ts`:
- Around line 74-87: The open() method currently calls openUinput(UINPUT_PATH)
but openUinput now throws on failure, so the fd<0 branch is dead; wrap the
openUinput call in a try/catch inside open() (method name: open, symbol:
openUinput, constant: UINPUT_PATH) and in the catch log the same contextual
messages using this.name plus the caught error (include error.message or the
error object) before returning false (or rethrow if you prefer to propagate); on
success set this.fd = fd and return true as before.
In `@src/server/drivers/mac/keyboard.ts`:
- Around line 24-35: The injectCombo method currently logs unknown keys but
continues sending the rest of the chord; change it to abort the entire combo
when any key is unresolved: inside injectCombo, while iterating over keys from
MAC_KEY_MAP (reference: injectCombo and MAC_KEY_MAP), if a lookup returns
undefined, call console.warn with the same message and immediately return (do
not push any codes or send any events), so no partial chord is synthesized;
otherwise continue building codes and proceed as before only when all keys
resolved.
In `@src/server/drivers/mac/touch.ts`:
- Around line 137-166: handleTwoFingerMove currently early-returns unless the
mover is this.pinch.contactIds[0], dropping updates when only the second finger
moves; change it so either contact can drive the logic by always retrieving both
current contacts from this.activeContacts using this.pinch.contactIds and their
previous positions (from the existing prev lookup or a prev-positions map), then
compute newSpread and centroid delta from those pairs; use those pair-level
values to decide isPinch/isPan, call emitPinchZoom(newSpread -
this.pinch.lastSpread) and update this.pinch.lastSpread, or compute averaged
dx/dy across both fingers and call postScrollEvent for pan—ensure you reference
handleTwoFingerMove, this.pinch.contactIds, this.pinch.lastSpread,
emitPinchZoom, activeContacts, and postScrollEvent when making the change.
- Around line 64-86: processDowns currently inserts all incoming TouchContact
entries into activeContacts which lets active stay >2 and freezes
single/two-finger behavior; change processDowns to reject or drop extras so
activeContacts never holds more than two contacts: before adding, filter downs
(or after insertion remove) to ensure only the first two unique contact IDs are
stored in this.activeContacts, update logic that calls postMouseEvent and
this.pinch (in processDowns, using postMouseEvent, buildPinchState and
this.activeContacts) to operate only on those retained contacts, and log/ignore
any additional touches without persisting them.
In `@src/server/InputHandler.ts`:
- Around line 180-192: The zoom handler in InputHandler.ts uses
injector.injectKey("control") which does a press+release so Control is not held
during injector.injectMouseWheel; update the implementation so Control is held
while scrolling—either extend PlatformInjector with dedicated key press/release
methods (e.g., injectKeyDown/injectKeyUp) and change the "zoom" case to call
injectKeyDown("control"), injectMouseWheel(0, amount), injectKeyUp("control"),
or add a new injector.injectZoom(delta:number) to PlatformInjector and implement
it per platform, then call injector.injectZoom(amount) from the "zoom" case;
adjust all platform implementations accordingly.
🪄 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: 5b5aa903-ff19-4fd2-9883-4fcf0aeaef66
📒 Files selected for processing (20)
src/hooks/useCaptureProvider.tssrc/server/InputHandler.tssrc/server/constants.tssrc/server/drivers/README.mdsrc/server/drivers/keyMap.tssrc/server/drivers/linux/index.tssrc/server/drivers/linux/keyboard.tssrc/server/drivers/linux/structs.tssrc/server/drivers/linux/touch.tssrc/server/drivers/mac/index.tssrc/server/drivers/mac/keyboard.tssrc/server/drivers/mac/structs.tssrc/server/drivers/mac/touch.tssrc/server/drivers/utils.tssrc/server/drivers/windows/index.tssrc/server/drivers/windows/keyboard.tssrc/server/drivers/windows/structs.tssrc/server/drivers/windows/touch.tssrc/server/types.tssrc/server/websocket.ts
💤 Files with no reviewable changes (1)
- src/server/drivers/mac/structs.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/useCaptureProvider.ts`:
- Around line 125-126: The fallback to hard-coded 1920x1080 for
screenWidth/screenHeight in useCaptureProvider should be replaced with runtime
display/video dimensions: when settings.width or settings.height are undefined,
use the actual runtime dimensions (e.g., video element's videoWidth/videoHeight
or window.innerWidth/window.innerHeight or display media dimensions captured at
start) instead of 1920/1080; update the assignment where screenWidth:
settings.width ?? 1920 and screenHeight: settings.height ?? 1080 to use the
computed runtimeDisplayWidth/runtimeDisplayHeight (or
videoRef.current?.videoWidth/videoHeight) so pointer/touch mapping remains
accurate across devices.
🪄 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: 7cf91ea6-3498-4a8d-b034-0885a82458d5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonsrc/hooks/useCaptureProvider.tssrc/server/drivers/linux/keyboard.tssrc/server/drivers/linux/structs.tssrc/server/types.ts
31 tasks
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 #130
Description
Implemented a cross-platform native input injection backend supporting Windows, Linux, and macOS.
Highlights
Added platform-specific input injectors for:
Added native keyboard injection support.
Added native mouse movement, button, and scroll injection.
Added multi-touch support across supported platforms.
Added touch gesture handling including drag, scrolling, and pinch-to-zoom.
Unified all platforms behind a common input interface.
Added runtime configuration support for sensitivity, acceleration, and inverted scrolling.
Added input validation, sanitization, and event throttling in the input handler.
Improved maintainability through platform-specific abstractions and shared utilities.
Screenshots/Recordings:
N/A (backend/input system changes)
Functional Verification
Screen Mirror
Authentication
Basic Gestures
One-finger tap: Verified as Left Click.
Two-finger tap: Verified as Right Click.
Click and drag: Verified selection behavior.
Pinch to zoom: Verified zoom functionality (if applicable).
Modes & Settings
Cursor mode: Cursor moves smoothly and accurately.
Scroll mode: Page scrolls as expected.
Sensitivity: Verified changes in cursor speed/sensitivity settings.
Copy and Paste: Verified both Copy and Paste functionality.
Invert Scrolling: Verified scroll direction toggles correctly.
Advanced Input
Key combinations: Verified "hold" behavior for modifiers (e.g., Ctrl+C) and held keys are shown in buffer.
Keyboard input: Verified Space, Backspace, and Enter keys work correctly.
Glide typing: Verified path drawing and text output.
Voice input: Verified speech-to-text functionality for full sentences.
Backspace doesn't send the previous input.
Any other gesture or input behavior introduced:
Additional Notes:
This PR primarily refactors and extends the native input injection layer. Testing should be performed on all supported operating systems (Windows, Linux, and macOS) to verify platform-specific behavior and gesture handling.
Summary by CodeRabbit
New Features
Improvements
Documentation