fix(acp): state the retry-hint carry policy in the network-error arm

A 429 with a `retry in {N}s` hint followed by a network error kept the
hint for the next sleep, but only because the sole assignment to
`retry_hint_secs` sat in the Ok-retriable arm. The behaviour was correct
and entirely incidental: moving that line, or adding an arm, would have
silently changed a policy nobody had written down.

Make the match an expression whose value is the hint governing the next
sleep, so every outcome names its own policy. The network-error arm now
yields `retry_hint_secs` explicitly, with a comment giving the reason
(a network error says nothing about the quota window) and the cost
asymmetry that picks the direction: over-sleeping wastes time we were
told to wait anyway, while under-sleeping earns a fresh denial that
still costs a counter increment, because the limiter's INCR runs on
refused checks too.

The comment also fences the one nuance a future reader would trip on:
the kept hint is exactly right only for the sleep immediately after the
429, and subtracting elapsed time to "fix" the over-sleep would
reintroduce under-sleep via clock arithmetic — the expensive direction.

Spelling note: the literal `retry_hint_secs = retry_hint_secs` in that
arm is a deny-by-default clippy error (clippy::self_assignment), so the
policy is expressed by making every arm yield its hint instead.

Covered by a behavioral test, because the arm is otherwise invisible:
it holds the value it already had, so yielding `None` instead is a
silent revert that all four pure `rest_retry_delay` tests stay green
under, as does the existing 429->200 wiring test (which never produces
a network error). The new test serves one 429 with a 51s hint and then
drops the listener, so the ladder sleeps hint/hint/hint (~153s) when
the hint is carried and hint/1s/2s (~54s) when it is dropped, and
asserts into the gap. Mutating the arm to `None` fails it at 53.8s;
across the whole 688-test lib run that mutant is caught by exactly one
test, so the coverage claim is measured rather than assumed.

Verified at rustc 1.95.0: buzz-acp 688 + 9 integration, buzz-cli 322,
0 failed; cargo fmt --all --check and clippy -p buzz-acp -p buzz-cli
--all-targets -D warnings both clean.

Co-authored-by: Sami <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@buzz.block.builderlab.xyz>
Signed-off-by: Sami <f4a42a97e594b77bdbd8ee35191c8b28a94a4cb871d96f32921558275421fb68@buzz.block.builderlab.xyz>

crates/buzz-acp/src/relay.rs

@@ -396,14 +396,17 @@ impl RestClient {
                 tokio::time::sleep(jittered).await;
             }
 
-            match build_request().await {
+            // Every arm yields the hint that governs the NEXT sleep, so each
+            // outcome states its own policy instead of inheriting one from
+            // wherever an assignment happens to sit.
+            retry_hint_secs = match build_request().await {
                 Ok(resp) if resp.status().is_success() => return Ok(resp),
                 Ok(resp) if is_retriable_status(resp.status()) => {
                     let status = resp.status();
                     // Read the body on a 429 for the relay's `retry in {N}s`
                     // hint. Only the sleep length depends on it, so a body we
                     // cannot read simply leaves the ladder rung in charge.
-                    retry_hint_secs = if status == reqwest::StatusCode::TOO_MANY_REQUESTS {
+                    let hint = if status == reqwest::StatusCode::TOO_MANY_REQUESTS {
                         resp.text()
                             .await
                             .ok()
@@ -412,7 +415,7 @@ impl RestClient {
                     } else {
                         None
                     };
-                    if let Some(secs) = retry_hint_secs {
+                    if let Some(secs) = hint {
                         tracing::warn!("{method} {path} rate-limited, relay asked for {secs}s");
                     } else {
                         tracing::warn!("{method} {path} returned retriable HTTP {status}");
@@ -420,6 +423,7 @@ impl RestClient {
                     last_err = Some(RelayError::Http(format!(
                         "{method} {path} returned HTTP {status}"
                     )));
+                    hint
                 }
                 Ok(resp) => {
                     return Err(RelayError::Http(format!(
@@ -431,9 +435,27 @@ impl RestClient {
                 Err(e) if e.is_timeout() || e.is_connect() => {
                     tracing::warn!("{method} {path} network error: {e}");
                     last_err = Some(RelayError::Http(e.to_string()));
+                    // Deliberately keep the previous 429's hint. A network
+                    // error says nothing about the quota window, so dropping
+                    // the hint here would send the next attempt back to a
+                    // sub-2s rung and inside a window the relay already
+                    // closed. The cost is one-sided: over-sleeping wastes
+                    // time we were told to wait anyway, while under-sleeping
+                    // earns a fresh denial that still costs a counter
+                    // increment, because the limiter's `INCR` runs on refused
+                    // checks too.
+                    //
+                    // This is only exactly right for the sleep immediately
+                    // after the 429. If the network error itself burned most
+                    // of the hinted window we over-sleep by up to a second
+                    // window — accepted, same cheap direction. Do not
+                    // "optimize" that by subtracting elapsed time: the
+                    // subtraction reintroduces under-sleep whenever the clock
+                    // arithmetic is off, which is the expensive direction.
+                    retry_hint_secs
                 }
                 Err(e) => return Err(RelayError::Http(e.to_string())),
-            }
+            };
         }
 
         Err(last_err
@@ -6143,6 +6165,81 @@ mod tests {
         server.abort();
     }
 
+    /// A network error after a 429 keeps the relay's hint for the next sleep.
+    ///
+    /// This pins the policy in the `is_timeout() || is_connect()` arm, which
+    /// is otherwise invisible: the arm assigns the hint it already held, so
+    /// deleting the assignment (yielding `None` instead) is a silent revert
+    /// that every pure `rest_retry_delay` test stays green under. The test
+    /// above cannot see it either — it never produces a network error.
+    ///
+    /// Shape: the first attempt is refused with a 51s hint, then the listener
+    /// is dropped so every later attempt is refused at connect. Sleeps are
+    /// therefore hint, hint, hint (~153s) when the hint is carried, and hint,
+    /// 1s rung, 2s rung (~54s) when it is dropped. The assertion sits in the
+    /// gap, so it discriminates on the sleep the ladder actually took rather
+    /// than on any single call's arguments.
+    ///
+    /// A network error says nothing about the quota window, so retrying on a
+    /// sub-2s rung would land inside a window the relay already closed and
+    /// earn a denial that still costs a counter increment.
+    #[tokio::test(start_paused = true)]
+    async fn rest_retry_keeps_the_hint_across_a_network_error() {
+        use tokio::io::{AsyncReadExt, AsyncWriteExt};
+
+        let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
+            .await
+            .expect("bind test HTTP server");
+        let base_url = format!("http://{}", listener.local_addr().unwrap());
+        let server = tokio::spawn(async move {
+            // Serve exactly one 429 carrying the hint, then drop the listener
+            // so every subsequent attempt fails at connect.
+            if let Ok((mut socket, _)) = listener.accept().await {
+                let body = r#"{"error":"rate-limited: quota exceeded (api); retry in 51s"}"#;
+                let mut buf = vec![0u8; 8192];
+                let _ = socket.read(&mut buf).await;
+                let response = format!(
+                    "HTTP/1.1 429 Too Many Requests\r\nContent-Type: application/json\r\n\
+                     Content-Length: {}\r\nConnection: close\r\n\r\n{body}",
+                    body.len()
+                );
+                let _ = socket.write_all(response.as_bytes()).await;
+                let _ = socket.flush().await;
+            }
+            drop(listener);
+        });
+
+        let client = RestClient {
+            http: reqwest::Client::new(),
+            base_url,
+            keys: Keys::generate(),
+            auth_tag_json: None,
+        };
+
+        let start = tokio::time::Instant::now();
+        let resp = client
+            .submit_event(&make_test_event(&Keys::generate(), 1_000))
+            .await;
+        let elapsed = start.elapsed();
+
+        assert!(
+            resp.is_err(),
+            "every attempt after the 429 is refused at connect, so the call fails: {resp:?}"
+        );
+        assert!(
+            elapsed >= Duration::from_secs(150),
+            "the ladder slept {elapsed:?} across three retries after a 51s hint. \
+             Around 54s means the hint was dropped by the network-error arm and \
+             the last two sleeps fell back to the 1s and 2s rungs, waking inside \
+             a window the relay had already closed"
+        );
+        assert!(
+            elapsed < Duration::from_secs(200),
+            "slept {elapsed:?}: three hint-length sleeps, not a compounding one"
+        );
+        server.abort();
+    }
+
     /// Corroboration: the armed gate honours the computed delay.
     ///
     /// Integration-level, so it can only sample the entropy the call site