fix(proto): let server-side paths use recoverability hint on network change (#579)

## Description

Currently `handle_network_change` only checks `is_path_recoverable()` on
the client side. The server side always assumed paths are recoverable,
based on the assumption that servers have stable addresses.

This assumption breaks when the server's uplink changes (e.g., mobile
device acting as server replugs to a different network). The dead direct
path would linger until the 3*PTO idle timeout instead of being
immediately abandoned.

Now both sides use the hint when provided. When no hint is given,
clients default to non-recoverable (existing behavior) and servers
default to recoverable (existing behavior).

## Breaking Changes

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

---------

Co-authored-by: Floris Bruynooghe <flub@n0.computer>

noq-proto/src/connection/mod.rs

@@ -5614,15 +5614,11 @@ impl Connection {
             // multipath, even in a single-path scenario, we attempt to migrate the path to a new
             // PathId.
             let attempt_to_recover = if is_multipath_negotiated {
-                if is_client {
-                    hint.map(|h| h.is_path_recoverable(*path_id, network_path))
-                        .unwrap_or(false)
-                } else {
-                    // Servers should have stable addresses so this scenario is generally discouraged.
-                    // There is no way to prevent this, so the best hope is to attempt to recover the
-                    // path
-                    true
-                }
+                // Use the hint to determine if the path can recover. When no hint is
+                // provided, clients default to non-recoverable (abandon and re-open)
+                // while servers default to recoverable (attempt in-place recovery).
+                hint.map(|h| h.is_path_recoverable(*path_id, network_path))
+                    .unwrap_or(!is_client)
             } else {
                 // In the non multipath case, we try to recover the single active path
                 true

noq-proto/src/tests/multipath.rs

@@ -967,6 +967,134 @@ fn network_change_selective_hint() -> TestResult {
     Ok(())
 }
 
+/// Server-side network change with two paths and a selective hint.
+///
+/// The non-recoverable path is abandoned, leaving only the recoverable one.
+#[test]
+fn network_change_server_two_paths_selective_hint() -> TestResult {
+    let _guard = subscribe();
+    let mut pair = multipath_pair();
+
+    // Open a second path from the client side.
+    let server_addr = pair.addrs_to_server();
+    let second_path = pair.open_path(Client, server_addr, PathStatus::Available)?;
+    pair.drive();
+
+    assert_matches!(
+        pair.poll(Client),
+        Some(Event::Path(PathEvent::Opened { id })) if id == second_path
+    );
+    assert_matches!(
+        pair.poll(Server),
+        Some(Event::Path(PathEvent::Opened { id })) if id == second_path
+    );
+
+    // Hint: The provided PathId is recoverable, others are not.
+    #[derive(Debug)]
+    struct SelectiveHint(PathId);
+    impl NetworkChangeHint for SelectiveHint {
+        fn is_path_recoverable(&self, path_id: PathId, _network_path: FourTuple) -> bool {
+            path_id == self.0
+        }
+    }
+
+    pair.handle_network_change(Server, Some(&SelectiveHint(second_path)));
+
+    pair.drive();
+
+    // The non-recoverable path is abandoned on the server. No replacement opens because
+    // servers cannot call open_path.
+    assert_matches!(
+        pair.poll(Server),
+        Some(Event::Path(PathEvent::Abandoned {
+            id,
+            reason: PathAbandonReason::UnusableAfterNetworkChange,
+        })) if id == PathId::ZERO
+    );
+    assert_matches!(
+        pair.poll(Server),
+        Some(Event::Path(PathEvent::Discarded { id, .. })) if id == PathId::ZERO
+    );
+    assert_matches!(pair.poll(Server), None);
+
+    // The client sees PathId::ZERO abandoned by the remote, then discards it.
+    assert_matches!(
+        pair.poll(Client),
+        Some(Event::Path(PathEvent::Abandoned {
+            id: PathId::ZERO,
+            reason: PathAbandonReason::RemoteAbandoned { .. },
+        }))
+    );
+    assert_matches!(
+        pair.poll(Client),
+        Some(Event::Path(PathEvent::Discarded {
+            id: PathId::ZERO,
+            ..
+        }))
+    );
+    assert_matches!(pair.poll(Client), None);
+
+    Ok(())
+}
+
+/// Server-side network change with a single path and a non-recoverable hint.
+///
+/// The path cannot be closed because it is the last one.
+#[test]
+fn network_change_server_single_path_non_recoverable_falls_back() -> TestResult {
+    let _guard = subscribe();
+    let mut pair = multipath_pair();
+
+    // Hint that says all paths are non-recoverable
+    #[derive(Debug)]
+    struct NonRecoverableHint;
+    impl NetworkChangeHint for NonRecoverableHint {
+        fn is_path_recoverable(&self, _path_id: PathId, _network_path: FourTuple) -> bool {
+            false
+        }
+    }
+
+    pair.handle_network_change(Server, Some(&NonRecoverableHint));
+    pair.drive();
+
+    // The path should NOT be abandoned. The last open path cannot be closed.
+    assert_matches!(pair.poll(Server), None);
+    assert_matches!(pair.poll(Client), None);
+
+    Ok(())
+}
+
+/// Server-side network change with no hint defaults to recoverable. Both paths stay open.
+#[test]
+fn network_change_server_no_hint_recovers() -> TestResult {
+    let _guard = subscribe();
+    let mut pair = multipath_pair();
+
+    // Open a second path from the client side.
+    let server_addr = pair.addrs_to_server();
+    let second_path = pair.open_path(Client, server_addr, PathStatus::Available)?;
+    pair.drive();
+
+    assert_matches!(
+        pair.poll(Client),
+        Some(Event::Path(PathEvent::Opened { id })) if id == second_path
+    );
+    assert_matches!(
+        pair.poll(Server),
+        Some(Event::Path(PathEvent::Opened { id })) if id == second_path
+    );
+
+    pair.handle_network_change(Server, None);
+    pair.drive();
+
+    // No path events: the server defaults to recoverable when no hint is provided.
+    // Neither path should be abandoned.
+    assert_matches!(pair.poll(Server), None);
+    assert_matches!(pair.poll(Client), None);
+
+    Ok(())
+}
+
 /// Checks that the deadline given before a path fails to be considered open start only when the
 /// first packet is sent.
 ///