Skip to content

Conversation

@cyole
Copy link
Owner

@cyole cyole commented Dec 10, 2025

  • 为每个客户端生成唯一 ID
  • ClipboardMessage 添加 client_id 字段
  • 客户端接收消息时检查并跳过自己发送的消息
  • 统一客户端剪贴板管理,使用单一 ClipboardMonitor 实例
  • 优化网络传输,减少不必要的处理

这解决了设备复制内容后收到自己消息的问题

Description

Linked Issues

Additional context


Note

Add client IDs to clipboard messages, skip self-originated updates, and consolidate client clipboard monitoring/receiving.

  • Protocol & Data Model
    • Add client_id: Option<String> to ClipboardMessage (with serde default).
    • Switch server/client channels to carry ClipboardMessage instead of ClipboardContent.
  • Server (src/main.rs, src/modules/sync.rs)
    • In relay-only mode, forward full ClipboardMessage to preserve client_id.
    • When broadcasting server-local clipboard changes, set client_id: None.
  • Client
    • Generate unique client_id and pass to SyncClient.
    • Include client_id in outgoing messages; skip processing messages where message.client_id matches own ID.
    • Unify clipboard handling into a single task using tokio::select! (poll local changes and handle server messages), removing the separate receive task.
  • Networking
    • Update bidirectional connect/send/receive to read/write ClipboardMessage end-to-end.

Written by Cursor Bugbot for commit 24dbe58. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced per-client identification system enabling better multi-client clipboard synchronization
    • Each client now generates a unique identifier at startup for improved traceability
    • Clients automatically filter out clipboard updates they themselves initiated, preventing unnecessary re-processing
  • Improvements

    • Enhanced observability with client identification for improved debugging and message tracking

✏️ Tip: You can customize this high-level summary in your review settings.

- 为每个客户端生成唯一 ID
- ClipboardMessage 添加 client_id 字段
- 客户端接收消息时检查并跳过自己发送的消息
- 统一客户端剪贴板管理,使用单一 ClipboardMonitor 实例
- 优化网络传输,减少不必要的处理

这解决了设备复制内容后收到自己消息的问题
@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

Walkthrough

This pull request introduces per-client identification in the clipboard synchronization protocol. A unique client_id is generated at startup and embedded in ClipboardMessage structures. The server and clients now distinguish messages by originator, and clients filter out self-originated messages to prevent feedback loops.

Changes

Cohort / File(s) Summary
Client initialization and ID generation
src/main.rs
Client generates unique client_id (OS + timestamp micros), prints it for tracing, and passes it to SyncClient constructor
SyncClient implementation
src/modules/sync.rs
SyncClient now stores client_id field, updated constructor signature to accept client_id parameter, and includes client_id in outbound ClipboardMessage
Server and message structure updates
src/modules/sync.rs
ClipboardMessage struct extended with optional client_id field; SyncServer and handle_client updated to work with ClipboardMessage instead of ClipboardContent; broadcast channels now transmit full ClipboardMessage with client_id embedded
Client-server message flow
src/main.rs, src/modules/sync.rs
Message relay preserves client_id; outbound updates include client_id; inbound messages are filtered to ignore self-originated messages; server-local changes marked with client_id: None

Sequence Diagram(s)

sequenceDiagram
    participant Client1
    participant Server
    participant Client2
    
    Note over Client1: Startup<br/>Generate unique client_id
    Client1->>Server: Connect with client_id
    
    rect rgb(200, 220, 255)
    Note over Client1, Server: Scenario A: Client1 sends clipboard update
    Client1->>Server: ClipboardMessage {<br/>content, timestamp,<br/>client_id: Some("client_1")<br/>}
    Server->>Server: Broadcast to all clients
    Server->>Client1: ClipboardMessage {<br/>client_id: Some("client_1")<br/>}
    Note over Client1: Filter: Ignore<br/>(self-originated)
    Server->>Client2: ClipboardMessage {<br/>client_id: Some("client_1")<br/>}
    Note over Client2: Accept & apply<br/>(from other client)
    end
    
    rect rgb(220, 255, 220)
    Note over Server: Scenario B: Server-local clipboard change
    Server->>Server: Local clipboard update
    Server->>Client1: ClipboardMessage {<br/>client_id: None<br/>}
    Note over Client1: Accept<br/>(server-originated)
    Server->>Client2: ClipboardMessage {<br/>client_id: None<br/>}
    Note over Client2: Accept<br/>(server-originated)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • SyncClient constructor signature change — affects API contract; verify all call sites pass correct client_id
  • ClipboardMessage struct extension — check serde(default) behavior and backward compatibility with existing message formats
  • Server-side type refactoring — broadcast channel type changes from ClipboardContent to ClipboardMessage; ensure all send/receive paths are updated consistently
  • Client-side filtering logic — review self-message detection logic to confirm it correctly identifies and filters messages originating from the current client
  • Message routing in relay mode — verify that client_id is preserved through the entire relay path and not accidentally stripped or overwritten

Poem

🐰 A rabbit hops through wires bright,
Each client marked with ID light,
No feedback loops, just smooth relay,
The clipboard syncs without delay!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding client ID filtering to prevent receiving self-sent messages, which aligns with the main objectives and changeset modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/client-id-filtering

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cyole cyole merged commit 28cf2d8 into main Dec 10, 2025
3 of 4 checks passed
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/main.rs (2)

172-180: Client ID generation may not be unique across fast restarts or clock adjustments.

Using as_micros() from SystemTime::now() could produce collisions if:

  1. Two clients start within the same microsecond
  2. System clock is adjusted backward
  3. Client restarts very quickly

Consider using a UUID or random component for stronger uniqueness guarantees.

-    let client_id = format!(
-        "{}-{}",
-        std::env::consts::OS,
-        std::time::SystemTime::now()
-            .duration_since(std::time::UNIX_EPOCH)
-            .unwrap()
-            .as_micros()
-    );
+    let client_id = format!(
+        "{}-{}-{}",
+        std::env::consts::OS,
+        std::process::id(),
+        std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap()
+            .as_micros()
+    );

Alternatively, consider adding the uuid crate for guaranteed uniqueness.


222-237: Clipboard monitoring uses polling instead of callback pattern.

The monitoring spawns a separate task that sends a unit signal every 500ms, which then triggers get_clipboard_content(). This is less efficient than the callback-based monitor() used on the server side (line 104). Consider using the same pattern for consistency.

However, the current approach does provide unified control in the select! loop, which is necessary for coordinating local changes with server updates.

src/modules/sync.rs (1)

222-225: unwrap() on duration_since could panic if system time is before UNIX epoch.

While extremely unlikely on modern systems, consider using unwrap_or(Duration::ZERO) or similar defensive handling.

                         timestamp: std::time::SystemTime::now()
                             .duration_since(std::time::UNIX_EPOCH)
-                            .unwrap()
+                            .unwrap_or_default()
                             .as_secs(),
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f35b0d and 24dbe58.

📒 Files selected for processing (2)
  • src/main.rs (7 hunks)
  • src/modules/sync.rs (8 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/main.rs (2)
src/modules/sync.rs (2)
  • new (84-94)
  • new (178-180)
src/modules/clipboard.rs (3)
  • new (25-58)
  • get_clipboard_content (86-141)
  • set_clipboard_content (222-269)
src/modules/sync.rs (1)
src/main.rs (2)
  • broadcast (62-62)
  • broadcast (184-184)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Cursor Bugbot
🔇 Additional comments (13)
src/main.rs (6)

75-88: Relay mode correctly preserves client_id for filtering.

The relay mode now broadcasts the full ClipboardMessage (including client_id), enabling downstream clients to filter out their own messages. This aligns well with the PR objective.


113-119: Server-originated messages correctly marked with client_id: None.

This ensures server clipboard changes are delivered to all clients without being filtered.


264-268: Self-message filtering logic is correct.

The comparison message.client_id.as_ref() == Some(&client_id_for_clipboard) properly handles the Option<String> and correctly skips messages originating from this client.


292-293: Monitor handle abort on loop exit is good cleanup.

Properly aborting the spawned polling task prevents resource leaks when the main loop exits.


139-157: Server receive path correctly updates and broadcasts clipboard content.

The server properly accesses message.content when setting the clipboard. Client-sent content updates the server clipboard through set_clipboard_content(), which then triggers the clipboard monitor to broadcast the change to other clients. This design correctly establishes the server as the clipboard authority and ensures consistency across all connected clients.


284-287: No race condition exists in this code.

Both get_clipboard_content() and set_clipboard_content() execute within the same async task in a tokio::select! block (lines 240-300). The select! macro ensures that only one branch executes at a time, providing mutual exclusion on the shared clipboard reference. When set_clipboard_content() is called from the server message branch, it completes fully—including the last_hash update on line 267 of clipboard.rs—before the next 500ms polling cycle can call get_clipboard_content(). The last_hash will always be synchronized with the clipboard state before any subsequent poll reads it.

src/modules/sync.rs (7)

20-26: ClipboardMessage structure with #[serde(default)] for backward compatibility.

Using #[serde(default)] on client_id allows deserialization of messages from older clients that don't include this field—good for rolling upgrades.


77-81: Type change from ClipboardContent to ClipboardMessage is correct.

This change propagates the full message (including client_id) through the server's internal channel, enabling proper filtering downstream.


124-129: Server forwards full ClipboardMessage preserving client_id.

The server now sends the complete message to its internal channel, which is essential for the relay mode to preserve origin information.


143-159: Server broadcasts to all connected clients including the sender.

The broadcast task sends messages to all clients without filtering. This means the originating client will receive its own message back. The filtering correctly happens client-side (in main.rs lines 266-268). This is a valid design choice—server stays simple, clients filter.


177-180: SyncClient constructor correctly stores client_id.

The client_id is stored and later used when constructing outbound messages.


182-186: Type mismatch between tx and rx parameters.

tx sends ClipboardMessage but rx receives ClipboardContent. This is intentional—the client receives raw content from clipboard monitoring and wraps it in a ClipboardMessage with its client_id before sending. The naming could be clearer, but the logic is sound.


218-227: Outbound messages correctly include client_id.

The message construction properly attaches the client's ID, enabling other clients to filter appropriately.

@cyole cyole deleted the feat/client-id-filtering branch December 10, 2025 09:02
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

}
if let Err(e) = clipboard.set_clipboard_content(&content) {
eprintln!("Failed to set clipboard: {}", e);
else => break,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Unreachable else branch due to kept channel sender

The else => break branch in the tokio::select! loop is unreachable. The original local_tx sender created at line 222 remains alive in the same async block as the loop, so local_rx.recv() will never return None. Since the else branch requires ALL futures' patterns to not match simultaneously, and local_rx will always either return Some(()) (from the spawned monitor) or block (never None), this branch can never execute. The clone at line 227 should be removed and the original local_tx should be moved directly into the spawned task instead.

Additional Locations (1)

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants