From 01003b3448b8ae4174a27f4127c9b50ebdb9f371 Mon Sep 17 00:00:00 2001
From: EdisonJwa <me@edison.network>
Date: Sat, 16 May 2026 10:10:03 +0800
Subject: [PATCH] refactor(protocol): clarify pending_moves expiry sweep

The original mem::replace + retain pattern worked but was opaque.
Switch to a two-pass approach: collect expired MessageHandles into
a small Vec, then remove + resolve. Behaviour-preserving; the
clippy-style readability win is worth the tiny extra allocation
(typical case: 0 or 1 expired entries per loop iteration).
---
 crates/chanora_protocol/src/adapter.rs | 30 +++++++++++++-------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/crates/chanora_protocol/src/adapter.rs b/crates/chanora_protocol/src/adapter.rs
index 6bedd34..e8319f6 100644
--- a/crates/chanora_protocol/src/adapter.rs
+++ b/crates/chanora_protocol/src/adapter.rs
@@ -500,24 +500,24 @@ async fn connection_task(
 
         // 2b. Sweep stale pending_moves whose deadline has passed.
         // The server should always reply within ~1 s; 3 s is a
-        // generous ceiling. Expired entries get an Ok() so the
-        // caller's snapshot-confirmation polling still has a chance
-        // to detect success (fall back to the previous optimistic
-        // behaviour rather than blocking the user with a fake
-        // ServerRejected).
+        // generous ceiling. Expired entries fall back to Ok() so
+        // the caller's snapshot-confirmation polling still has a
+        // chance to detect success — better than a fake
+        // ServerRejected for legacy servers that never reply to
+        // move requests.
         if !pending_moves.is_empty() {
             let now = std::time::Instant::now();
-            pending_moves.retain(|_, (reply, deadline)| {
-                if now >= *deadline {
-                    // Cannot move `reply` out of `&mut` cleanly here
-                    // without an intermediate take(); use a sentinel
-                    // sender so retain's signature works.
-                    let _ = std::mem::replace(reply, oneshot::channel().0).send(Ok(()));
-                    false
-                } else {
-                    true
+            let expired: Vec<MessageHandle> = pending_moves
+                .iter()
+                .filter_map(|(handle, (_, deadline))| {
+                    if now >= *deadline { Some(*handle) } else { None }
+                })
+                .collect();
+            for handle in expired {
+                if let Some((reply, _)) = pending_moves.remove(&handle) {
+                    let _ = reply.send(Ok(()));
                 }
-            });
+            }
         }
 
         // 3. Service at most one control request (non-blocking).