Conversation
Reviewer's Guide为与剪贴板相关的负载和局域网对等消息添加 Unix 时间戳，在剪贴板 setter 中引入去重与新鲜度检查，并通过在发送失败时重新绑定 UDP 套接字并添加适当日志，使局域网 beacon 广播器更加健壮。 局域网 beacon 广播器 UDP 发送失败与重绑定的时序图sequenceDiagram
participant LanBeaconBroadcaster
participant UdpSocket
participant EventChannel
LanBeaconBroadcaster->>UdpSocket: bind(INADDR_ANY, port=0)
UdpSocket-->>LanBeaconBroadcaster: Ok(socket)
LanBeaconBroadcaster->>UdpSocket: set_broadcast(true)
loop periodic_beacon
LanBeaconBroadcaster->>UdpSocket: send_to(beacon_payload, broadcast_addr)
alt send_ok
UdpSocket-->>LanBeaconBroadcaster: Ok(bytes_sent)
else send_error
UdpSocket-->>LanBeaconBroadcaster: Err(e)
LanBeaconBroadcaster->>EventChannel: RuntimeLogEvent(Level_Warn, LAN beacon send failed (attempting rebind))
LanBeaconBroadcaster->>UdpSocket: bind(INADDR_ANY, port=0)
alt rebind_ok
UdpSocket-->>LanBeaconBroadcaster: Ok(new_socket)
LanBeaconBroadcaster->>UdpSocket: new_socket.set_broadcast(true)
alt set_broadcast_ok
UdpSocket-->>LanBeaconBroadcaster: Ok()
LanBeaconBroadcaster->>LanBeaconBroadcaster: socket = new_socket
LanBeaconBroadcaster->>EventChannel: RuntimeLogEvent(Level_Info, LAN beacon rebind successful)
else set_broadcast_error
UdpSocket-->>LanBeaconBroadcaster: Err(err)
LanBeaconBroadcaster->>EventChannel: RuntimeLogEvent(Level_Error, LAN beacon rebind set_broadcast failed)
end
else rebind_error
UdpSocket-->>LanBeaconBroadcaster: Err(err)
LanBeaconBroadcaster->>EventChannel: RuntimeLogEvent(Level_Error, LAN beacon rebind failed)
end
end
end
更新后的剪贴板消息类型类图classDiagram
class ClipboardUpdatePayload {
+String content_type
+String data
+String sender_device_id
+u64 timestamp
}
class ClipboardBroadcastPayload {
+String content_type
+String data
+u64 timestamp
}
class ClipboardBroadcast {
+String source_device_id
+ClipboardBroadcastPayload payload
}
class PeerMessage {
}
class PeerMessageClipboardVariant {
+String content_type
+String data
+u64 timestamp
}
ClipboardBroadcast o-- ClipboardBroadcastPayload
PeerMessage <|-- PeerMessageClipboardVariant
文件级变更
相对于关联 Issue 的评估
可能关联的 Issue
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideAdds Unix timestamps to clipboard-related payloads and LAN peer messages, introduces de-duplication and freshness checks in the clipboard setter, and makes the LAN beacon broadcaster more robust by rebinding the UDP socket on send failures with appropriate logging. Sequence diagram for LAN beacon broadcaster UDP send failure and rebindsequenceDiagram
participant LanBeaconBroadcaster
participant UdpSocket
participant EventChannel
LanBeaconBroadcaster->>UdpSocket: bind(INADDR_ANY, port=0)
UdpSocket-->>LanBeaconBroadcaster: Ok(socket)
LanBeaconBroadcaster->>UdpSocket: set_broadcast(true)
loop periodic_beacon
LanBeaconBroadcaster->>UdpSocket: send_to(beacon_payload, broadcast_addr)
alt send_ok
UdpSocket-->>LanBeaconBroadcaster: Ok(bytes_sent)
else send_error
UdpSocket-->>LanBeaconBroadcaster: Err(e)
LanBeaconBroadcaster->>EventChannel: RuntimeLogEvent(Level_Warn, LAN beacon send failed (attempting rebind))
LanBeaconBroadcaster->>UdpSocket: bind(INADDR_ANY, port=0)
alt rebind_ok
UdpSocket-->>LanBeaconBroadcaster: Ok(new_socket)
LanBeaconBroadcaster->>UdpSocket: new_socket.set_broadcast(true)
alt set_broadcast_ok
UdpSocket-->>LanBeaconBroadcaster: Ok()
LanBeaconBroadcaster->>LanBeaconBroadcaster: socket = new_socket
LanBeaconBroadcaster->>EventChannel: RuntimeLogEvent(Level_Info, LAN beacon rebind successful)
else set_broadcast_error
UdpSocket-->>LanBeaconBroadcaster: Err(err)
LanBeaconBroadcaster->>EventChannel: RuntimeLogEvent(Level_Error, LAN beacon rebind set_broadcast failed)
end
else rebind_error
UdpSocket-->>LanBeaconBroadcaster: Err(err)
LanBeaconBroadcaster->>EventChannel: RuntimeLogEvent(Level_Error, LAN beacon rebind failed)
end
end
end
Updated class diagram for clipboard message typesclassDiagram
class ClipboardUpdatePayload {
+String content_type
+String data
+String sender_device_id
+u64 timestamp
}
class ClipboardBroadcastPayload {
+String content_type
+String data
+u64 timestamp
}
class ClipboardBroadcast {
+String source_device_id
+ClipboardBroadcastPayload payload
}
class PeerMessage {
}
class PeerMessageClipboardVariant {
+String content_type
+String data
+u64 timestamp
}
ClipboardBroadcast o-- ClipboardBroadcastPayload
PeerMessage <|-- PeerMessageClipboardVariant
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些高层次的反馈：
- 使用
SystemTime::now().duration_since(UNIX_EPOCH).unwrap()时，如果系统时钟被设置在 Unix 纪元之前，就可能发生 panic；建议显式处理Err分支并回退到一个安全的默认值，而不是直接 unwrap。 - 在 LAN beacon 重新绑定的路径中，
UdpSocket::bind或set_broadcast(true)失败会被静默忽略；建议记录这些错误日志，这样在生产环境中就能观察到 beacon 重新绑定相关的问题。
给 AI Agent 的提示
请根据本次代码审查中的评论进行修改：
## 总体评论
- 使用 `SystemTime::now().duration_since(UNIX_EPOCH).unwrap()` 时，如果系统时钟被设置在 Unix 纪元之前，就可能发生 panic；建议显式处理 `Err` 分支并回退到一个安全的默认值，而不是直接 unwrap。
- 在 LAN beacon 重新绑定的路径中，`UdpSocket::bind` 或 `set_broadcast(true)` 失败会被静默忽略；建议记录这些错误日志，这样在生产环境中就能观察到 beacon 重新绑定相关的问题。
## 逐条评论
### 评论 1
<location path="src-tauri/src/runtime/clipboard.rs" line_range="202-205" />
<code_context>
_ = cancel.cancelled() => break,
maybe_payload = rx.recv() => {
if let Some(payload) = maybe_payload {
+ if payload.timestamp < last_timestamp {
+ continue;
+ }
+ last_timestamp = payload.timestamp;
match payload.content_type.as_str() {
CONTENT_TYPE_TEXT => {
</code_context>
<issue_to_address>
**issue (bug_risk):** 依赖基于墙上时间（wall-clock）的时间戳来做排序，在系统时钟被回拨时可能会丢弃有效的更新。
`SystemTime::now()` 并不保证单调递增，可能会往回跳或向前跳（例如 NTP 同步、手动修改时间），因此这种比较有可能错误地跳过更新的剪贴板内容。对于本地排序，更推荐使用单调递增的序列（例如一个计数器，或者用 `Instant` 映射到一个只增不减的值）。如果这个时间戳是用于跨设备排序，还需要在设计上考虑不同机器之间的时钟偏差。
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据你的反馈改进后续的审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Using
SystemTime::now().duration_since(UNIX_EPOCH).unwrap()can panic if the system clock is set before the Unix epoch; consider handling theErrcase explicitly and falling back to a safe default instead of unwrapping. - In the LAN beacon rebind path, failures of
UdpSocket::bindorset_broadcast(true)are silently ignored; consider logging these errors so issues with beacon rebinding are observable in production.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `SystemTime::now().duration_since(UNIX_EPOCH).unwrap()` can panic if the system clock is set before the Unix epoch; consider handling the `Err` case explicitly and falling back to a safe default instead of unwrapping.
- In the LAN beacon rebind path, failures of `UdpSocket::bind` or `set_broadcast(true)` are silently ignored; consider logging these errors so issues with beacon rebinding are observable in production.
## Individual Comments
### Comment 1
<location path="src-tauri/src/runtime/clipboard.rs" line_range="202-205" />
<code_context>
_ = cancel.cancelled() => break,
maybe_payload = rx.recv() => {
if let Some(payload) = maybe_payload {
+ if payload.timestamp < last_timestamp {
+ continue;
+ }
+ last_timestamp = payload.timestamp;
match payload.content_type.as_str() {
CONTENT_TYPE_TEXT => {
</code_context>
<issue_to_address>
**issue (bug_risk):** Relying on wall-clock timestamps for ordering can drop valid updates when the system clock moves backwards.
`SystemTime::now()` is not guaranteed to be monotonic and can move backward or jump forward (e.g., NTP, manual clock changes), so this comparison can incorrectly skip newer clipboard updates. For local ordering, prefer a monotonic sequence (e.g., a counter or `Instant` mapped to an increasing value). If the timestamp is intended for cross-device ordering, you’ll also need to account for clock skew between machines in the design.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR adds a timestamp field to clipboard-related payloads/messages, uses it to ignore stale inbound clipboard updates, and improves LAN discovery robustness by rebinding the UDP broadcast socket after send failures.
Changes:
- Added
timestampto clipboard update/broadcast payloads and LANPeerMessage::Clipboard. - Introduced stale-update filtering in the clipboard setter using
last_timestamp. - On LAN beacon send failure, attempt to rebind a new UDP socket with broadcast enabled.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src-tauri/src/runtime/messages.rs |
Adds timestamp fields to clipboard payload structs used for JSON messaging. |
src-tauri/src/runtime/lan/protocol.rs |
Extends LAN peer protocol clipboard message to carry a timestamp. |
src-tauri/src/runtime/lan/peer.rs |
Propagates timestamp through LAN peer send/receive clipboard relays. |
src-tauri/src/runtime/lan/discovery.rs |
Rebinds UDP socket on beacon send failures to recover from network changes. |
src-tauri/src/runtime/clipboard.rs |
Generates timestamps for outbound updates and drops inbound updates older than the last seen timestamp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+90
to
+93
| timestamp: std::time::SystemTime::now() | ||
| .duration_since(std::time::UNIX_EPOCH) | ||
| .unwrap() | ||
| .as_secs(), |
Comment on lines
+196
to
+205
| let mut last_timestamp = 0; | ||
| loop { | ||
| tokio::select! { | ||
| _ = cancel.cancelled() => break, | ||
| maybe_payload = rx.recv() => { | ||
| if let Some(payload) = maybe_payload { | ||
| if payload.timestamp < last_timestamp { | ||
| continue; | ||
| } | ||
| last_timestamp = payload.timestamp; |
Comment on lines
137
to
151
| if let Err(e) = socket.send_to(&packet, broadcast_addr).await { | ||
| let _ = events | ||
| .send(RuntimeEvent::Log(RuntimeLogEvent::new( | ||
| Level::Warn, | ||
| format!("LAN beacon send failed: {}", e), | ||
| format!("LAN beacon send failed (attempting rebind): {}", e), | ||
| ))) | ||
| .await; | ||
|
|
||
| // Attempt to rebind on network error | ||
| if let Ok(new_socket) = UdpSocket::bind(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, 0)).await { | ||
| if new_socket.set_broadcast(true).is_ok() { | ||
| socket = new_socket; | ||
| } | ||
| } | ||
| } |
Comment on lines
38
to
43
| pub struct ClipboardUpdatePayload { | ||
| pub content_type: String, | ||
| pub data: String, | ||
| pub sender_device_id: String, | ||
| pub timestamp: u64, | ||
| } |
| pub struct ClipboardBroadcastPayload { | ||
| pub content_type: String, | ||
| pub data: String, | ||
| pub timestamp: u64, |
| @@ -100,6 +100,7 @@ pub enum PeerMessage { | |||
| Clipboard { | |||
| content_type: String, | |||
| data: String, | |||
Owner
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
- 描述中提到要忽略早于最后一次已应用时间戳的剪贴板更新，但
start_clipboard_setter目前只按内容（last_text/last_image_data）去重，从不比较时间戳；建议分别为每种内容类型记录最后一次应用的时间戳，并跳过时间戳小于或等于该值的更新，而不是（或不仅仅是）基于内容做去重。 - 时间戳生成逻辑（
SystemTime::now().duration_since(UNIX_EPOCH)...as_secs()）在多个地方被重复使用；建议抽取成一个小的辅助函数，以保持行为一致并减少重复代码。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- The description mentions ignoring incoming clipboard updates older than the last applied timestamp, but `start_clipboard_setter` only deduplicates by content (`last_text` / `last_image_data`) and never compares timestamps; consider tracking the last applied timestamp per content type and skipping updates with lower or equal timestamps instead of (or in addition to) content-based deduplication.
- The timestamp generation logic (`SystemTime::now().duration_since(UNIX_EPOCH)...as_secs()`) is duplicated in multiple places; consider extracting this into a small helper function to keep the behavior consistent and reduce repetition.
## Individual Comments
### Comment 1
<location path="src-tauri/src/runtime/clipboard.rs" line_range="196-200" />
<code_context>
events: mpsc::Sender<RuntimeEvent>,
cancel: CancellationToken,
) {
+ let mut last_text = String::new();
+ let mut last_image_data = String::new();
loop {
</code_context>
<issue_to_address>
**issue (bug_risk):** The `last_text`/`last_image_data` dedup logic will skip applying empty-string payloads forever.
Since `last_text` and `last_image_data` start as `String::new()`, the first empty-string payload is treated as a duplicate and skipped, and all later empty-string updates are also ignored. If an empty clipboard value should be propagated, initialize these as `Option<String>` (or similar) and treat `None` as “no previous value,” or track first use with a separate flag so an initial empty value isn’t deduped.
</issue_to_address>帮我变得更有用！请在每条评论上点选 👍 或 👎，我会根据你的反馈改进后续的代码评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The description mentions ignoring incoming clipboard updates older than the last applied timestamp, but
start_clipboard_setteronly deduplicates by content (last_text/last_image_data) and never compares timestamps; consider tracking the last applied timestamp per content type and skipping updates with lower or equal timestamps instead of (or in addition to) content-based deduplication. - The timestamp generation logic (
SystemTime::now().duration_since(UNIX_EPOCH)...as_secs()) is duplicated in multiple places; consider extracting this into a small helper function to keep the behavior consistent and reduce repetition.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The description mentions ignoring incoming clipboard updates older than the last applied timestamp, but `start_clipboard_setter` only deduplicates by content (`last_text` / `last_image_data`) and never compares timestamps; consider tracking the last applied timestamp per content type and skipping updates with lower or equal timestamps instead of (or in addition to) content-based deduplication.
- The timestamp generation logic (`SystemTime::now().duration_since(UNIX_EPOCH)...as_secs()`) is duplicated in multiple places; consider extracting this into a small helper function to keep the behavior consistent and reduce repetition.
## Individual Comments
### Comment 1
<location path="src-tauri/src/runtime/clipboard.rs" line_range="196-200" />
<code_context>
events: mpsc::Sender<RuntimeEvent>,
cancel: CancellationToken,
) {
+ let mut last_text = String::new();
+ let mut last_image_data = String::new();
loop {
</code_context>
<issue_to_address>
**issue (bug_risk):** The `last_text`/`last_image_data` dedup logic will skip applying empty-string payloads forever.
Since `last_text` and `last_image_data` start as `String::new()`, the first empty-string payload is treated as a duplicate and skipped, and all later empty-string updates are also ignored. If an empty clipboard value should be propagated, initialize these as `Option<String>` (or similar) and treat `None` as “no previous value,” or track first use with a separate flag so an initial empty value isn’t deduped.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些总体反馈：
start_clipboard_setter中的时间戳检查（payload.timestamp <= last_*_ts）配合以秒为粒度的current_timestamp()，会导致在同一秒内发生的两次不同剪贴板更新共享同一个时间戳，即使数据不同，后一次更新也会被丢弃；建议使用更高精度的时间戳，或者调整条件，仅在时间戳和数据都相同时才跳过。start_clipboard_setter中新的去重状态（last_text、last_image_data以及时间戳）在所有发送方之间是全局共享的；如果期望不同设备能够互相覆盖对方的剪贴板内容，你可能需要按发送方分别追踪“最新性”，或者重新考虑使用单一全局的last_*，以避免产生出乎意料的行为。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The `start_clipboard_setter` timestamp check (`payload.timestamp <= last_*_ts`) with second-level `current_timestamp()` means two different clipboard updates within the same second will share the same timestamp and the latter will be dropped even if the data is different; consider using a higher-resolution timestamp or adjusting the condition to only skip when both timestamp and data match.
- The new deduplication state in `start_clipboard_setter` (`last_text`, `last_image_data`, and timestamps) is maintained globally across all senders; if different devices are expected to be able to overwrite each other’s clipboard contents, you may want to track recency per sender or reconsider using a single global `last_*` to avoid surprising behavior.
## Individual Comments
### Comment 1
<location path="src-tauri/src/runtime/clipboard.rs" line_range="201-204" />
<code_context>
if let Some(payload) = maybe_payload {
match payload.content_type.as_str() {
CONTENT_TYPE_TEXT => {
+ if payload.timestamp <= last_text_ts || Some(&payload.data) == last_text.as_ref() {
+ continue;
+ }
+ last_text_ts = payload.timestamp;
+ last_text = Some(payload.data.clone());
if let Err(err) = set_text(&payload.data, disable_flag.clone()).await {
</code_context>
<issue_to_address>
**issue:** Relying on remote wall-clock timestamps can drop valid clipboard updates when clocks are skewed or out of order.
Using `payload.timestamp <= last_text_ts` as a hard gate assumes loosely synchronized, strictly increasing wall clocks across devices. With clock skew, a newer clipboard value from a lagging device can have a smaller timestamp and be dropped even when `payload.data` has changed.
If the intent is mainly to break feedback loops, consider instead:
- Comparing only data (and possibly sender id), or
- Using a per-device monotonic sequence, or
- Treating the timestamp as a hint (e.g., require both `timestamp < last_text_ts` and equal content before skipping).
In its current form, legitimate cross-device updates can be lost when clocks differ.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/runtime/clipboard.rs" line_range="310-314" />
<code_context>
}
}
+fn current_timestamp() -> u64 {
+ std::time::SystemTime::now()
+ .duration_since(std::time::UNIX_EPOCH)
+ .unwrap_or_default()
+ .as_secs()
+}
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Second-level timestamp granularity might be too coarse for disambiguating rapid clipboard changes.
`current_timestamp()` truncates to whole seconds via `as_secs()`. Multiple clipboard updates within the same second will get identical timestamps, so with the `<= last_*_ts` checks, later updates can be incorrectly treated as stale and skipped. Consider using millisecond precision (e.g. `as_millis()`) or a `(secs, nanos)` pair to avoid these collisions.
Suggested implementation:
```rust
fn current_timestamp() -> u64 {
// Milliseconds since UNIX_EPOCH; higher precision to disambiguate rapid clipboard changes
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap_or_default()
.as_millis() as u64
}
```
To fully adopt millisecond precision, you should:
1. Ensure all timestamp variables that store the result of `current_timestamp()` (e.g. `last_text_ts`, `last_image_ts`, payload `timestamp` fields) are consistently treated as “milliseconds since UNIX_EPOCH`.
2. Verify that any comparisons like `if payload.timestamp <= last_*_ts` still work as intended (they should, but comments/docs may still mention "seconds" and should be updated).
3. If timestamps are serialized (e.g. sent over IPC/JSON), confirm the consumer side expects milliseconds, and update any corresponding types/docs there as well.
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据这些反馈改进后续的代码审查。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The
start_clipboard_settertimestamp check (payload.timestamp <= last_*_ts) with second-levelcurrent_timestamp()means two different clipboard updates within the same second will share the same timestamp and the latter will be dropped even if the data is different; consider using a higher-resolution timestamp or adjusting the condition to only skip when both timestamp and data match. - The new deduplication state in
start_clipboard_setter(last_text,last_image_data, and timestamps) is maintained globally across all senders; if different devices are expected to be able to overwrite each other’s clipboard contents, you may want to track recency per sender or reconsider using a single globallast_*to avoid surprising behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `start_clipboard_setter` timestamp check (`payload.timestamp <= last_*_ts`) with second-level `current_timestamp()` means two different clipboard updates within the same second will share the same timestamp and the latter will be dropped even if the data is different; consider using a higher-resolution timestamp or adjusting the condition to only skip when both timestamp and data match.
- The new deduplication state in `start_clipboard_setter` (`last_text`, `last_image_data`, and timestamps) is maintained globally across all senders; if different devices are expected to be able to overwrite each other’s clipboard contents, you may want to track recency per sender or reconsider using a single global `last_*` to avoid surprising behavior.
## Individual Comments
### Comment 1
<location path="src-tauri/src/runtime/clipboard.rs" line_range="201-204" />
<code_context>
if let Some(payload) = maybe_payload {
match payload.content_type.as_str() {
CONTENT_TYPE_TEXT => {
+ if payload.timestamp <= last_text_ts || Some(&payload.data) == last_text.as_ref() {
+ continue;
+ }
+ last_text_ts = payload.timestamp;
+ last_text = Some(payload.data.clone());
if let Err(err) = set_text(&payload.data, disable_flag.clone()).await {
</code_context>
<issue_to_address>
**issue:** Relying on remote wall-clock timestamps can drop valid clipboard updates when clocks are skewed or out of order.
Using `payload.timestamp <= last_text_ts` as a hard gate assumes loosely synchronized, strictly increasing wall clocks across devices. With clock skew, a newer clipboard value from a lagging device can have a smaller timestamp and be dropped even when `payload.data` has changed.
If the intent is mainly to break feedback loops, consider instead:
- Comparing only data (and possibly sender id), or
- Using a per-device monotonic sequence, or
- Treating the timestamp as a hint (e.g., require both `timestamp < last_text_ts` and equal content before skipping).
In its current form, legitimate cross-device updates can be lost when clocks differ.
</issue_to_address>
### Comment 2
<location path="src-tauri/src/runtime/clipboard.rs" line_range="310-314" />
<code_context>
}
}
+fn current_timestamp() -> u64 {
+ std::time::SystemTime::now()
+ .duration_since(std::time::UNIX_EPOCH)
+ .unwrap_or_default()
+ .as_secs()
+}
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Second-level timestamp granularity might be too coarse for disambiguating rapid clipboard changes.
`current_timestamp()` truncates to whole seconds via `as_secs()`. Multiple clipboard updates within the same second will get identical timestamps, so with the `<= last_*_ts` checks, later updates can be incorrectly treated as stale and skipped. Consider using millisecond precision (e.g. `as_millis()`) or a `(secs, nanos)` pair to avoid these collisions.
Suggested implementation:
```rust
fn current_timestamp() -> u64 {
// Milliseconds since UNIX_EPOCH; higher precision to disambiguate rapid clipboard changes
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap_or_default()
.as_millis() as u64
}
```
To fully adopt millisecond precision, you should:
1. Ensure all timestamp variables that store the result of `current_timestamp()` (e.g. `last_text_ts`, `last_image_ts`, payload `timestamp` fields) are consistently treated as “milliseconds since UNIX_EPOCH`.
2. Verify that any comparisons like `if payload.timestamp <= last_*_ts` still work as intended (they should, but comments/docs may still mention "seconds" and should be updated).
3. If timestamps are serialized (e.g. sent over IPC/JSON), confirm the consumer side expects milliseconds, and update any corresponding types/docs there as well.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我给出了一些总体反馈：
- 描述中提到要忽略早于上次已应用时间戳的剪贴板更新，但
start_clipboard_setter目前只按重复的data来过滤；建议按内容类型记录last_timestamp，并跳过所有timestamp <= last_timestamp的负载，这样才能完整实现预期行为。 current_timestamp使用SystemTime::now()并将as_millis()转为u64；如果你需要在多设备之间依赖时间戳的顺序，建议记录或规范化时间源（例如，明确使用自纪元起的毫秒值，并处理潜在的时钟偏移或回绕问题），以避免出现微妙的排序问题。
提供给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The description mentions ignoring clipboard updates older than the last applied timestamp, but `start_clipboard_setter` currently only filters by duplicate `data`; consider tracking `last_timestamp` per content type and skipping payloads with `timestamp <= last_timestamp` to fully implement the intended behavior.
- `current_timestamp` uses `SystemTime::now()` and casts `as_millis()` to `u64`; if you rely on timestamp ordering across devices, consider documenting or normalizing the time source (e.g., clearly using milliseconds-since-epoch and handling potential clock skew or wraparound) to avoid subtle ordering issues.帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've left some high level feedback:
- The description mentions ignoring clipboard updates older than the last applied timestamp, but
start_clipboard_settercurrently only filters by duplicatedata; consider trackinglast_timestampper content type and skipping payloads withtimestamp <= last_timestampto fully implement the intended behavior. current_timestampusesSystemTime::now()and castsas_millis()tou64; if you rely on timestamp ordering across devices, consider documenting or normalizing the time source (e.g., clearly using milliseconds-since-epoch and handling potential clock skew or wraparound) to avoid subtle ordering issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The description mentions ignoring clipboard updates older than the last applied timestamp, but `start_clipboard_setter` currently only filters by duplicate `data`; consider tracking `last_timestamp` per content type and skipping payloads with `timestamp <= last_timestamp` to fully implement the intended behavior.
- `current_timestamp` uses `SystemTime::now()` and casts `as_millis()` to `u64`; if you rely on timestamp ordering across devices, consider documenting or normalizing the time source (e.g., clearly using milliseconds-since-epoch and handling potential clock skew or wraparound) to avoid subtle ordering issues.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
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.
Add timestamps to clipboard updates
Include Unix timestamp in clipboard payloads and peer messages.
Make clipboard setter ignore incoming updates older than the last seen
timestamp to prevent applying stale clipboard contents.
When LAN beacon send fails, attempt to rebind a new UDP socket and set
broadcast before retrying.
close #13
Summary by Sourcery
添加剪贴板时间戳，以防止应用过期的剪贴板内容，并在 UDP 发送失败时提升局域网广播信标的鲁棒性。
New Features:
Bug Fixes:
Original summary in English
Summary by Sourcery
Add clipboard timestamps to prevent applying stale clipboard contents and improve LAN beacon robustness when UDP sends fail.
New Features:
Bug Fixes:
Summary by Sourcery
为剪贴板添加时间戳用于新鲜度追踪，并改进局域网发现广播和剪贴板应用逻辑的健壮性。
New Features:
Bug Fixes:
Original summary in English
Summary by Sourcery
Add clipboard timestamps for freshness tracking and improve robustness of LAN discovery beacons and clipboard application logic.
New Features:
Bug Fixes:
新功能：
错误修复：
Original summary in English
Summary by Sourcery
为剪贴板添加时间戳用于新鲜度追踪，并改进局域网发现广播和剪贴板应用逻辑的健壮性。
New Features:
Bug Fixes:
Original summary in English
Summary by Sourcery
Add clipboard timestamps for freshness tracking and improve robustness of LAN discovery beacons and clipboard application logic.
New Features:
Bug Fixes:
新功能：
错误修复：
Original summary in English
Summary by Sourcery
为剪贴板添加时间戳用于新鲜度追踪，并改进局域网发现广播和剪贴板应用逻辑的健壮性。
New Features:
Bug Fixes:
Original summary in English
Summary by Sourcery
Add clipboard timestamps for freshness tracking and improve robustness of LAN discovery beacons and clipboard application logic.
New Features:
Bug Fixes:
新功能：
错误修复：
Original summary in English
Summary by Sourcery
为剪贴板添加时间戳用于新鲜度追踪，并改进局域网发现广播和剪贴板应用逻辑的健壮性。
New Features:
Bug Fixes:
Original summary in English
Summary by Sourcery
Add clipboard timestamps for freshness tracking and improve robustness of LAN discovery beacons and clipboard application logic.
New Features:
Bug Fixes: