Fix Copy/Paste Functionality and Resolve Connection Loss on App Switch#92
Fix Copy/Paste Functionality and Resolve Connection Loss on App Switch#92b-u-g-g wants to merge 5 commits intoAOSSIE-Org:mainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds functional Copy and Paste controls to the trackpad UI, wires them through the trackpad route to the remote connection, enhances the WebSocket hook with live socket tracking and visibility-aware reconnection, extends server input message types for copy/paste, and refines WebSocket server logging and error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as ControlBar (UI)
participant Route as trackpad.tsx
participant Hook as useRemoteConnection
participant WS as WebSocket Client
participant Server as InputHandler
User->>UI: Click "Copy"
UI->>Route: onCopy()
Route->>Route: handleCopy() logs
Route->>Hook: send({ type: "copy" })
Hook->>WS: ws.send(message)
WS->>Server: receive copy message
Server->>Server: perform platform-aware copy (Cmd/Ctrl+C)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
src/components/Trackpad/ControlBar.tsx (1)
9-9: 🛠️ Refactor suggestion | 🟠 Major
onLeftClickis still declared and destructured but no longer used in the JSX.The L-Click button was removed, but
onLeftClickremains in the interface (line 9) and destructuring (line 22). This is dead code within the component. Either remove it from the props interface and destructuring, or re-add the L-Click button if it was removed unintentionally.♻️ Proposed fix
interface ControlBarProps { scrollMode: boolean; modifier: ModifierState; buffer: string; onToggleScroll: () => void; - onLeftClick: () => void; onRightClick: () => void; onKeyboardToggle: () => void; onModifierToggle: () => void; onPaste: () => void; onCopy: () => void; } export const ControlBar: React.FC<ControlBarProps> = ({ scrollMode, modifier, buffer, onToggleScroll, - onLeftClick, onRightClick, onKeyboardToggle, onModifierToggle, onPaste, onCopy, }) => {Note: The caller in
trackpad.tsx(line 187) still passesonLeftClick={() => handleClick('left')}— that prop should be removed there as well.Also applies to: 22-22
src/routes/trackpad.tsx (1)
187-187: 🛠️ Refactor suggestion | 🟠 MajorRemove the
onLeftClickprop — it's no longer used byControlBar.The L-Click button was removed from ControlBar's render, so this prop is dead. Clean it up here as part of the same change.
♻️ Proposed fix
<ControlBar scrollMode={scrollMode} modifier={modifier} buffer={buffer.join(" + ")} onToggleScroll={() => setScrollMode(!scrollMode)} - onLeftClick={() => handleClick('left')} onRightClick={() => handleClick('right')} onKeyboardToggle={focusInput} onModifierToggle={handleModifierState} onPaste={handlePaste} onCopy={handleCopy} />
🤖 Fix all issues with AI agents
In `@src/hooks/useRemoteConnection.ts`:
- Around line 63-69: The send and sendCombo callbacks capture a stale React
state variable ws and can miss a freshly reconnected socket; update both
callbacks to reference the live socket reference (currentWs or a useRef like
currentWsRef) instead of the ws state, remove ws from their dependency arrays
(or depend on the ref), and check currentWs.readyState === WebSocket.OPEN before
sending so they always use the most recent connection established by the
reconnect logic (leave setWs as-is for state updates).
- Around line 42-51: The visibility handler handleVisibilityChange currently
calls connect() whenever currentWs.readyState !== WebSocket.OPEN, which can race
with the onclose reconnect timer and create duplicate sockets or abandon a
CONNECTING socket; change the condition to only reconnect when currentWs is
explicitly WebSocket.CLOSED (i.e., if (!currentWs || currentWs.readyState ===
WebSocket.CLOSED) { clearTimeout(reconnectTimer); connect(); }) so it won't
interfere with CONNECTING or CLOSING states—use the existing reconnectTimer
clear to still prevent duplicate onclose timers and reference currentWs,
reconnectTimer, connect, and handleVisibilityChange when applying the change.
In `@src/server/InputHandler.ts`:
- Around line 138-164: In the 'paste' and 'copy' cases inside InputHandler, the
modifier key (computed via Key.LeftControl or Key.LeftCmd) is pressed before
pressing V/C but not guaranteed to be released if keyboard.pressKey(Key.V) or
pressKey(Key.C) throws; update both cases to wrap the press/release sequence in
a try/finally so that after await keyboard.pressKey(modifier) you always call
keyboard.releaseKey(modifier) in the finally block (also ensure you still
press/release the letter key inside the try), mirroring the pattern used in the
zoom handler to prevent leaving the modifier stuck.
In `@src/server/websocket.ts`:
- Around line 67-69: The catch block in the message processing try/catch is
silently swallowing all errors; modify the catch after the receive/parse/handle
flow (where inputHandler.handleMessage is invoked) to distinguish JSON parse
errors from unexpected runtime errors: if the error is a SyntaxError from
JSON.parse, keep silent or log at trace, but for other errors (e.g., from
inputHandler.handleMessage or nut-js) log them at debug/error level including
the error object and context (the raw message or client id) so issues are
observable during debugging.
🧹 Nitpick comments (4)
src/components/Trackpad/ControlBar.tsx (2)
62-97: Addtype="button"to all<button>elements.As flagged by static analysis, buttons without an explicit
typedefault to"submit", which can cause unintended form submissions. Addtype="button"to each button.♻️ Example fix (apply to all 6 buttons)
<button + type="button" className={`btn btn-sm ${scrollMode ? "btn-primary" : "btn-outline"}`} onPointerDown={(e) => handleInteraction(e, onToggleScroll)} >
59-99: Unnecessary fragment wrapper.The component returns a single
<div>child. The<>...</>fragment on lines 60/99 serves no purpose and can be removed.src/server/websocket.ts (1)
76-78: Silentonerrorhandler.Same concern — completely silencing WebSocket errors makes production debugging difficult. A one-line log would be beneficial without being noisy.
🔧 Proposed fix
- ws.onerror = () => { - // Silent + ws.onerror = (err) => { + console.error('[WS] Socket error:', err.message); };src/server/InputHandler.ts (1)
140-141: Misleading variable name and duplicated logic.
isWinistruefor both Windows and Linux, making the name misleading. Also, the paste and copy blocks are nearly identical — consider extracting a helper.♻️ Extract a helper method
+ private async simulateShortcut(key: Key) { + const useCtrl = process.platform === 'win32' || process.platform === 'linux'; + const modifier = useCtrl ? Key.LeftControl : Key.LeftCmd; + + await keyboard.pressKey(modifier); + try { + await keyboard.pressKey(key); + await keyboard.releaseKey(key); + } finally { + await keyboard.releaseKey(modifier); + } + }Then the cases become:
case 'paste': - try { ... } catch { ... } + await this.simulateShortcut(Key.V); break; case 'copy': - try { ... } catch { ... } + await this.simulateShortcut(Key.C); break;Also applies to: 154-155
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/InputHandler.ts (1)
65-82:⚠️ Potential issue | 🟡 MinorZoom handler uses
Key.LeftControlunconditionally — won't zoom on macOS.The
pasteandcopyhandlers correctly detect the platform and useKey.LeftCmdon macOS, but thezoomhandler on line 76 always usesKey.LeftControl. On macOS, Ctrl+Scroll doesn't trigger system zoom in most apps — it's typically Cmd-based or handled differently. If zoom is intended to work cross-platform, this should mirror the platform detection logic from the copy/paste handlers.Proposed fix
+ const isWin = process.platform === 'win32' || process.platform === 'linux'; + const zoomModifier = isWin ? Key.LeftControl : Key.LeftCmd; - await keyboard.pressKey(Key.LeftControl); + await keyboard.pressKey(zoomModifier); try { await mouse.scrollDown(amount); } finally { - await keyboard.releaseKey(Key.LeftControl); + await keyboard.releaseKey(zoomModifier); }
🤖 Fix all issues with AI agents
In `@src/hooks/useRemoteConnection.ts`:
- Around line 57-61: The onclose handler in useRemoteConnection schedules a
reconnect (reconnectTimer = setTimeout(connect, 1000)) even during unmount,
causing state updates after unmount; add a disposed flag (e.g., const
disposedRef = useRef(false)) and set disposedRef.current = true in the cleanup
before calling currentWsRef.current?.close(), then update the onclose handler
(in useRemoteConnection) to guard all post-close actions (setStatus, setWs, and
scheduling reconnectTimer) with if (!disposedRef.current) { ... } so no
reconnect or state changes occur after unmount; keep clearing any existing
reconnectTimer as part of cleanup.
In `@src/server/InputHandler.ts`:
- Around line 26-32: The code in InputHandler.ts is using mouse.getPosition()
and constructing a new Point to perform relative moves, which bypasses the
library's helpers; replace the block inside the dx/dy handling so it no longer
calls mouse.getPosition() or constructs new Point, and instead call the nut-js
relative movement helpers via mouse.move(right(msg.dx)) and
mouse.move(down(msg.dy)) (use right/left/up/down as appropriate when dx/dy can
be negative) so the InputHandler mouse.move usage conforms to the library API
and remove the unnecessary position query.
🧹 Nitpick comments (2)
src/server/InputHandler.ts (1)
138-170: Reduce duplication betweenpasteandcopyhandlers.The two cases are identical except for the letter key (
Key.VvsKey.C). Consider extracting a small helper to keep things DRY and easier to maintain.♻️ Example helper extraction
+ private async performShortcut(letterKey: Key) { + const isWin = process.platform === 'win32' || process.platform === 'linux'; + const modifier = isWin ? Key.LeftControl : Key.LeftCmd; + + await keyboard.pressKey(modifier); + try { + await keyboard.pressKey(letterKey); + await keyboard.releaseKey(letterKey); + } finally { + await keyboard.releaseKey(modifier); + } + }Then the cases become:
case 'paste': - try { - const isWin = process.platform === 'win32' || process.platform === 'linux'; - const modifier = isWin ? Key.LeftControl : Key.LeftCmd; - - await keyboard.pressKey(modifier); - try { - await keyboard.pressKey(Key.V); - await keyboard.releaseKey(Key.V); - } finally { - await keyboard.releaseKey(modifier); - } - } catch (error) { - // Silent - } + try { await this.performShortcut(Key.V); } catch { /* silent */ } break; case 'copy': - try { - const isWin = process.platform === 'win32' || process.platform === 'linux'; - const modifier = isWin ? Key.LeftControl : Key.LeftCmd; - - await keyboard.pressKey(modifier); - try { - await keyboard.pressKey(Key.C); - await keyboard.releaseKey(Key.C); - } finally { - await keyboard.releaseKey(modifier); - } - } catch (error) { - // Silent - } + try { await this.performShortcut(Key.C); } catch { /* silent */ } break;src/hooks/useRemoteConnection.ts (1)
84-84: Consider removing the unusedwsreturn value to avoid API confusion.The returned
wsstate can lag behindcurrentWsRef.currentafter reconnection sincesetWs()is asynchronous. While the hook correctly providessend()andsendCombo()functions that safely use the ref, exporting thewssocket directly creates confusion and risk for future consumers. The only current consumer (trackpad.tsx) explicitly does not destructurews, indicating it's unused—remove it from the return value to clarify that external code should always use the providedsend()andsendCombo()methods instead.
| if (msg.dx !== undefined && msg.dy !== undefined) { | ||
| // ========== OPTIMIZED: Use relative move ========== | ||
| const currentPos = await mouse.getPosition(); | ||
|
|
||
| await mouse.setPosition(new Point( | ||
| currentPos.x + msg.dx, | ||
| currentPos.y + msg.dy | ||
| )); | ||
| await mouse.move([ | ||
| new Point(currentPos.x + msg.dx, currentPos.y + msg.dy) | ||
| ]); | ||
| // ================================================== |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the InputHandler.ts file to see the context
cat -n src/server/InputHandler.ts | head -50Repository: AOSSIE-Org/Rein
Length of output: 2177
🏁 Script executed:
# Check all uses of mouse.move in the project
rg -n "mouse\.move" --type=ts -C3Repository: AOSSIE-Org/Rein
Length of output: 1095
🏁 Script executed:
# Look for the nut-tree-fork/nut-js library type definitions or documentation
fd -e "d.ts" -e "package.json" | grep -i nut | head -20Repository: AOSSIE-Org/Rein
Length of output: 41
🏁 Script executed:
# Check if there's a node_modules directory with nut-tree-fork types
find . -type d -name "@nut-tree*" 2>/dev/null | head -5Repository: AOSSIE-Org/Rein
Length of output: 41
🏁 Script executed:
# Check package.json for library version and imports
cat package.json | grep -A5 -B5 "nut-tree"Repository: AOSSIE-Org/Rein
Length of output: 431
🏁 Script executed:
# Search for where right, down, left, up are imported/used
rg -n "import.*\b(right|down|left|up)\b" --type=tsRepository: AOSSIE-Org/Rein
Length of output: 151
🏁 Script executed:
# Look for any documentation or comments about mouse.move API
rg -n "mouse\.move" -B2 -A2 --type=tsRepository: AOSSIE-Org/Rein
Length of output: 832
🌐 Web query:
@nut-tree-fork/nut-js mouse.move API documentation relative movement
💡 Result:
mouse.move(...) in nut.js takes a path (Point[] / Promise<Point[]>) and moves the cursor along it; for relative movement, you typically pass one of the built-in relative movement helpers that generate a path from the current cursor position: left(x), right(x), up(x), down(x). Example: await mouse.move(left(10)). [1]
import { mouse, left, right, up, down } from "@nut-tree-fork/nut-js";
await mouse.move(left(100));
await mouse.move(up(50));
await mouse.move(right(100));
await mouse.move(down(50));This “relative movement” API is documented as the intended way to move relative to the current position (vs. straightTo(point) for absolute targeting). [1][2]
🏁 Script executed:
# Check if there are more complex movement chains in the codebase
rg -n "\.move\(" --type=ts -B1 -A1Repository: AOSSIE-Org/Rein
Length of output: 603
🏁 Script executed:
# Look for any chaining patterns with right, down, left, up
rg -n "(right|down|left|up)\(" --type=ts -C2Repository: AOSSIE-Org/Rein
Length of output: 396
Use the library's relative movement helpers instead of manually calculating absolute positions.
The @nut-tree-fork/nut-js library provides right(), down(), left(), and up() helpers specifically for relative movement. The current code manually calculates an absolute position and passes a single-element Point array, which bypasses the intended API.
Replace:
const currentPos = await mouse.getPosition();
await mouse.move([
new Point(currentPos.x + msg.dx, currentPos.y + msg.dy)
]);With:
await mouse.move(right(msg.dx));
await mouse.move(down(msg.dy));This follows the library's documented pattern (see test-input.ts for reference) and eliminates unnecessary position queries.
🤖 Prompt for AI Agents
In `@src/server/InputHandler.ts` around lines 26 - 32, The code in InputHandler.ts
is using mouse.getPosition() and constructing a new Point to perform relative
moves, which bypasses the library's helpers; replace the block inside the dx/dy
handling so it no longer calls mouse.getPosition() or constructs new Point, and
instead call the nut-js relative movement helpers via mouse.move(right(msg.dx))
and mouse.move(down(msg.dy)) (use right/left/up/down as appropriate when dx/dy
can be negative) so the InputHandler mouse.move usage conforms to the library
API and remove the unnecessary position query.
1 participant
Summary
This PR addresses two critical stability and functionality issues: implementing functional Copy and Paste buttons via backend keyboard simulation and resolving WebSocket connection drops when switching between apps on mobile devices.
Problem Statement
1. Copy/Paste Buttons Non-Functional
2. Connection State Lost on App Switch
Changes Made
1. Implemented Copy/Paste Button Handlers
app/server/InputHandler.tsKey.LeftControlfor Windows/Linux orKey.LeftCmdfor macOS).pressKeyandreleaseKeyevents to execute system-level shortcuts reliably.2. Fixed Connection State Management
Heartbeat System (
app/server/websocket.ts): * Implemented a server-side check that expects a ping every 10 seconds, terminating the connection only after 35 seconds of silence to prevent premature drops.Auto-Reconnection Logic (
app/hooks/useRemoteConnection.ts): * Added event listeners forvisibilitychange,focus, andpageshowto detect when a user returns to the foreground.The app now checks connection status immediately upon return and forces a reconnect if the socket is closed.
Wake Lock API (
app/hooks/useRemoteConnection.ts): * Integrated the Wake Lock API to request that the mobile screen remains active, helping to keep the WebSocket alive while the app is in use.Testing Done
Summary by CodeRabbit
New Features
Improvements
UI Updates
Bug Fixes