refactor(channels/webhook): address review feedback on retry logic

Post-review cleanup on dd93422:
- Drop dead `last_err` state and replace unreachable tail `bail!` with
  `unreachable!` — the loop always exits via return/bail on the final
  attempt, so the trailing error was never reached.
- Expand comment above the 429/503 Retry-After branch to explain why 429
  appears twice: once here (server-supplied delay) and once in the
  fallback exponential-backoff branch when no Retry-After header was sent.
- Note in `compute_backoff` doc that the `retry_max_delay_ms` cap is
  approximate — ±25% jitter is applied after capping.
- Add `send_honors_retry_after_on_503` wiremock test mirroring the 429
  case, closing a gap where only 429+Retry-After was covered.
- Add schema tests for the 3 new `WebhookConfig` retry fields:
  `webhook_config_retry_fields_default_to_none` and
  `webhook_config_retry_fields_roundtrip` (JSON + TOML).

Author: mn13

crates/zeroclaw-channels/src/webhook.rs

@@ -83,7 +83,8 @@ impl WebhookChannel {
     }
 
     /// Compute the backoff delay for a given attempt, bounded by `retry_max_delay_ms`
-    /// and with ±25% jitter applied.
+    /// and with ±25% jitter applied. The cap is approximate: jitter is applied after
+    /// capping, so the returned delay may exceed `retry_max_delay_ms` by up to 25%.
     fn compute_backoff(&self, attempt: u32) -> Duration {
         let multiplier = 1_u64.checked_shl(attempt).unwrap_or(u64::MAX);
         let base = self.retry_base_delay_ms.saturating_mul(multiplier);
@@ -182,7 +183,6 @@ impl Channel for WebhookChannel {
             },
         };
 
-        let mut last_err: Option<String> = None;
         let total_attempts = self.max_retries.saturating_add(1);
 
         for attempt in 0..total_attempts {
@@ -207,7 +207,6 @@ impl Channel for WebhookChannel {
                     tokio::time::sleep(capped).await;
                 }
                 AttemptOutcome::Retry(err_msg) => {
-                    last_err = Some(err_msg.clone());
                     if attempt + 1 >= total_attempts {
                         bail!(
                             "Webhook send failed after {total_attempts} attempt(s); last error: {err_msg}"
@@ -226,11 +225,7 @@ impl Channel for WebhookChannel {
             }
         }
 
-        // Unreachable in practice: loop always returns or bails on the last attempt.
-        bail!(
-            "Webhook send failed after {total_attempts} attempt(s){}",
-            last_err.map(|e| format!(": {e}")).unwrap_or_default()
-        )
+        unreachable!("send loop exits via return or bail on the final attempt")
     }
 
     async fn listen(&self, tx: tokio::sync::mpsc::Sender<ChannelMessage>) -> Result<()> {
@@ -402,7 +397,9 @@ impl WebhookChannel {
             .await
             .unwrap_or_else(|e| format!("<failed to read response: {e}>"));
 
-        // 429 and 503 may include Retry-After; honor it if present.
+        // 429 and 503 may include Retry-After; honor it if present. 429 appears here
+        // *and* in the branch below: here we take the server-supplied delay, below we
+        // fall back to exponential backoff when no Retry-After header was sent.
         if (code == 429 || code == 503)
             && let Some(ms) = retry_after
         {
@@ -787,6 +784,49 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn send_honors_retry_after_on_503() {
+        use std::time::Instant;
+        use wiremock::matchers::{method, path};
+        use wiremock::{Mock, MockServer, ResponseTemplate};
+
+        let mock = MockServer::start().await;
+        Mock::given(method("POST"))
+            .and(path("/cb"))
+            .respond_with(ResponseTemplate::new(503).insert_header("Retry-After", "1"))
+            .up_to_n_times(1)
+            .expect(1)
+            .mount(&mock)
+            .await;
+        Mock::given(method("POST"))
+            .and(path("/cb"))
+            .respond_with(ResponseTemplate::new(200))
+            .expect(1)
+            .mount(&mock)
+            .await;
+
+        let ch = WebhookChannel::new(
+            8080,
+            None,
+            Some(format!("{}/cb", mock.uri())),
+            None,
+            None,
+            None,
+            Some(2),
+            Some(10),
+            Some(2_000),
+        );
+
+        let start = Instant::now();
+        ch.send(&test_message()).await.unwrap();
+        let elapsed = start.elapsed();
+        assert!(
+            elapsed >= Duration::from_millis(900),
+            "expected to wait ~1s for Retry-After on 503, elapsed = {:?}",
+            elapsed
+        );
+    }
+
     #[tokio::test]
     async fn send_max_retries_zero_disables_retry() {
         use wiremock::matchers::{method, path};

crates/zeroclaw-config/src/schema.rs

@@ -13037,6 +13037,43 @@ bot_token = "xoxb-tok"
         assert_eq!(parsed.port, 8080);
     }
 
+    #[test]
+    async fn webhook_config_retry_fields_default_to_none() {
+        let json = r#"{"port":8080}"#;
+        let parsed: WebhookConfig = serde_json::from_str(json).unwrap();
+        assert!(parsed.max_retries.is_none());
+        assert!(parsed.retry_base_delay_ms.is_none());
+        assert!(parsed.retry_max_delay_ms.is_none());
+    }
+
+    #[test]
+    async fn webhook_config_retry_fields_roundtrip() {
+        let wc = WebhookConfig {
+            enabled: true,
+            port: 8080,
+            listen_path: None,
+            send_url: Some("https://example.com/cb".into()),
+            send_method: None,
+            auth_header: None,
+            secret: None,
+            max_retries: Some(5),
+            retry_base_delay_ms: Some(250),
+            retry_max_delay_ms: Some(10_000),
+        };
+
+        let json = serde_json::to_string(&wc).unwrap();
+        let parsed: WebhookConfig = serde_json::from_str(&json).unwrap();
+        assert_eq!(parsed.max_retries, Some(5));
+        assert_eq!(parsed.retry_base_delay_ms, Some(250));
+        assert_eq!(parsed.retry_max_delay_ms, Some(10_000));
+
+        let toml_str = toml::to_string(&wc).unwrap();
+        let parsed: WebhookConfig = toml::from_str(&toml_str).unwrap();
+        assert_eq!(parsed.max_retries, Some(5));
+        assert_eq!(parsed.retry_base_delay_ms, Some(250));
+        assert_eq!(parsed.retry_max_delay_ms, Some(10_000));
+    }
+
     // ── WhatsApp config ──────────────────────────────────────
 
     #[test]