Draft PR for HTTP based copy and paste functionality - #368
Conversation
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughThe trackpad now supports session-aware clipboard copy and paste through browser APIs and new authenticated server endpoints. The server accesses the host clipboard and injects messages into active input connections, while WebRTC remains a fallback. ChangesClipboard integration
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant Trackpad
participant ClipboardAPI
participant InputPeerConnection
participant HostClipboard
Browser->>Trackpad: Copy or paste action
Trackpad->>ClipboardAPI: Read or write client clipboard
Trackpad->>InputPeerConnection: POST clipboard request with sessionId
InputPeerConnection->>HostClipboard: Read or write host clipboard
InputPeerConnection-->>Trackpad: Clipboard response
Trackpad-->>Browser: Update client clipboard or use WebRTC fallback
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/routes/trackpad.tsx`:
- Around line 124-131: Add AbortController-based timeouts to the clipboard
fetches in handleCopy and handlePaste, matching the 2-second timeout convention
used by checkServerActive. Pass each controller’s signal to the corresponding
/api/clipboard/copy or /api/clipboard/paste request and ensure timeout aborts
follow the existing fallback/error handling path.
- Around line 132-138: Define an explicit interface for the clipboard-copy
response shape and apply it when parsing the result of response.json() in the
response handling block. Update the data access around writeClientClipboard so
data is typed with the expected text field, avoiding implicit any while
preserving the existing invalid-data error path.
In `@src/server/api/apiHandlers.ts`:
- Around line 816-818: Update both handlers containing the sessionId
destructuring to guard JSON.parse failures with local error handling. Return a
400 Bad Request for malformed request bodies, while preserving the existing
parsed-body behavior for valid JSON and avoiding propagation to the generic
route-level error handler.
- Around line 828-829: Update the clipboard-copy flow around
inputPc.handleMessage({ type: "copy" }) to catch and handle errors through the
handler’s structured 500 response, matching handleClipboardPaste. Replace the
fixed 100ms delay with a reliable synchronization or polling mechanism that
waits until the host clipboard reflects the injected copy operation before
calling readHostClipboard(), while preserving the existing success and error
response behavior.
🪄 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: ad3e5d52-5706-4239-aedb-28bbe5a005d1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
src/hooks/useWebRtcStream.tssrc/routes/trackpad.tsxsrc/server/api/InputPeerConnection.tssrc/server/api/apiHandlers.tssrc/server/server.ts
| await inputPc.handleMessage({ type: "copy" }) | ||
| await new Promise((resolve) => setTimeout(resolve, 100)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Unguarded copy-trigger + fixed 100ms delay race before reading clipboard.
Two related concerns here:
await inputPc.handleMessage({ type: "copy" })is not wrapped in try/catch. If it throws (e.g. an unsupported-utility path the PR explicitly calls out), the error skips the handler's structured 500 response and falls through to the generic route-level catch inserver.ts, unlikehandleClipboardPaste, which wraps its equivalentinputPc.handleMessage({type:"text", text})call (lines 862-868).- A fixed 100ms sleep is used to "wait" for the host OS to populate the clipboard after the injected copy keystroke. This is inherently racy: on a slower app/host,
readHostClipboard()can return stale (previous) clipboard content silently, with no indication to the caller that the read may be wrong.
🛠️ Proposed fix
- await inputPc.handleMessage({ type: "copy" })
- await new Promise((resolve) => setTimeout(resolve, 100))
- try {
- const text = readHostClipboard()
- json(res, 200, { text })
- } catch (err) {
- logger.error(`Failed to read host clipboard: ${String(err)}`)
- json(res, 500, { error: "Failed to read host clipboard" })
- }
+ try {
+ await inputPc.handleMessage({ type: "copy" })
+ } catch (err) {
+ logger.error(`Failed to trigger host copy: ${String(err)}`)
+ json(res, 500, { error: "Failed to trigger host copy" })
+ return
+ }
+ try {
+ let text = ""
+ for (let i = 0; i < 5; i++) {
+ await new Promise((resolve) => setTimeout(resolve, 50))
+ text = readHostClipboard()
+ if (text) break
+ }
+ json(res, 200, { text })
+ } catch (err) {
+ logger.error(`Failed to read host clipboard: ${String(err)}`)
+ json(res, 500, { error: "Failed to read host clipboard" })
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await inputPc.handleMessage({ type: "copy" }) | |
| await new Promise((resolve) => setTimeout(resolve, 100)) | |
| try { | |
| await inputPc.handleMessage({ type: "copy" }) | |
| } catch (err) { | |
| logger.error(`Failed to trigger host copy: ${String(err)}`) | |
| json(res, 500, { error: "Failed to trigger host copy" }) | |
| return | |
| } | |
| try { | |
| let text = "" | |
| for (let i = 0; i < 5; i++) { | |
| await new Promise((resolve) => setTimeout(resolve, 50)) | |
| text = readHostClipboard() | |
| if (text) break | |
| } | |
| json(res, 200, { text }) | |
| } catch (err) { | |
| logger.error(`Failed to read host clipboard: ${String(err)}`) | |
| json(res, 500, { error: "Failed to read host clipboard" }) | |
| } |
🤖 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 828 - 829, Update the
clipboard-copy flow around inputPc.handleMessage({ type: "copy" }) to catch and
handle errors through the handler’s structured 500 response, matching
handleClipboardPaste. Replace the fixed 100ms delay with a reliable
synchronization or polling mechanism that waits until the host clipboard
reflects the injected copy operation before calling readHostClipboard(), while
preserving the existing success and error response behavior.
1 participant
Summary
This PR implements HTTP-based clipboard synchronization for the trackpad client, enabling clipboard contents to be transferred between the client and the host over HTTP.
Changes
Testing
Manual Testing
Known Issues
The primary functionality is working as expected. The following edge cases are still under investigation:
Notes
This is a draft PR to gather feedback on the implementation and discuss the remaining edge cases before marking it as ready for review.
Summary by CodeRabbit
New Features
Bug Fixes