Address review: include proof-required retry-after and ceiling-round millis

Codex adversarial review flagged two issues in the phase 1 retry-after
plumbing:

* Aggregate retry-after ignored proof-required failures. Because
  isRateLimitFailure is true for proof-required cases but
  rateLimitRetryAfterSeconds was only populated from plain 413s, an
  all-proof-required batch (or a mixed batch where the proof-required
  delay was longer) could flow into outputResult() and produce a
  RateLimitException(0), telling callers to retry immediately.

* Millisecond Retry-After values were truncated by integer division,
  so 1..999ms became 0 and non-second-aligned values lost up to 999ms.
  A retry suggested from the floored value can land before the
  server's real deadline and re-trigger the limit.

SendMessageResult.from(...) now populates rateLimitRetryAfterSeconds
from either the proof-required seconds or the plain rate-limit ms
(converted via ceiling division), giving maxRateLimitRetryAfterSeconds
a single source of truth. JsonSendMessageResult.from(...) reads the
unified field. New millisToCeilingSeconds helper plus boundary test.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

lib/src/main/java/org/asamk/signal/manager/api/SendMessageResult.java

@@ -26,6 +26,16 @@ public record SendMessageResult(
     ) {
         final var rateLimitFailure = sendMessageResult.getRateLimitFailure();
         final var proofRequiredFailure = sendMessageResult.getProofRequiredFailure();
+        final Long retryAfterSeconds;
+        if (proofRequiredFailure != null) {
+            retryAfterSeconds = proofRequiredFailure.getRetryAfterSeconds();
+        } else if (rateLimitFailure != null) {
+            retryAfterSeconds = rateLimitFailure.getRetryAfterMilliseconds()
+                    .map(SendMessageResult::millisToCeilingSeconds)
+                    .orElse(null);
+        } else {
+            retryAfterSeconds = null;
+        }
         return new SendMessageResult(addressResolver.resolveRecipientAddress(recipientResolver.resolveRecipient(
                 sendMessageResult.getAddress())).toApiRecipientAddress(),
                 sendMessageResult.isSuccess(),
@@ -35,8 +45,11 @@ public record SendMessageResult(
                 rateLimitFailure != null || proofRequiredFailure != null,
                 proofRequiredFailure == null ? null : new ProofRequiredException(proofRequiredFailure),
                 sendMessageResult.isInvalidPreKeyFailure(),
-                rateLimitFailure == null
-                        ? null
-                        : rateLimitFailure.getRetryAfterMilliseconds().map(ms -> ms / 1000L).orElse(null));
+                retryAfterSeconds);
+    }
+
+    static long millisToCeilingSeconds(long millis) {
+        // Round up so we never advise a retry before the server's deadline.
+        return (millis + 999L) / 1000L;
     }
 }

lib/src/test/java/org/asamk/signal/manager/api/SendMessageResultTest.java

@@ -0,0 +1,24 @@
+package org.asamk.signal.manager.api;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class SendMessageResultTest {
+
+    /**
+     * Ceiling division — we must never advise a retry before the server's deadline,
+     * so sub-second values round up rather than truncate toward zero.
+     */
+    @Test
+    void millisToCeilingSecondsRoundsUp() {
+        assertEquals(0L, SendMessageResult.millisToCeilingSeconds(0L));
+        assertEquals(1L, SendMessageResult.millisToCeilingSeconds(1L));
+        assertEquals(1L, SendMessageResult.millisToCeilingSeconds(500L));
+        assertEquals(1L, SendMessageResult.millisToCeilingSeconds(999L));
+        assertEquals(1L, SendMessageResult.millisToCeilingSeconds(1000L));
+        assertEquals(2L, SendMessageResult.millisToCeilingSeconds(1001L));
+        assertEquals(2L, SendMessageResult.millisToCeilingSeconds(1500L));
+        assertEquals(60L, SendMessageResult.millisToCeilingSeconds(60_000L));
+    }
+}

src/main/java/org/asamk/signal/json/JsonSendMessageResult.java

@@ -32,9 +32,7 @@ public record JsonSendMessageResult(
                                                   ? Type.INVALID_PRE_KEY_FAILURE
                                                         : Type.IDENTITY_FAILURE,
                 result.proofRequiredFailure() != null ? result.proofRequiredFailure().getToken() : null,
-                result.proofRequiredFailure() != null
-                        ? (Long) result.proofRequiredFailure().getRetryAfterSeconds()
-                        : result.rateLimitRetryAfterSeconds());
+                result.rateLimitRetryAfterSeconds());
     }
 
     public enum Type {

src/test/java/org/asamk/signal/json/JsonSendMessageResultTest.java

@@ -8,6 +8,7 @@ import org.junit.jupiter.api.Test;
 
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -47,47 +48,6 @@ class JsonSendMessageResultTest {
         assertNull(json.retryAfterSeconds());
     }
 
-    @Test
-    void sendMessageResultsReturnsMaxRetryAfter() {
-        var small = new SendMessageResult(ADDRESS,
-                false, false, false, false,
-                true,
-                null,
-                false,
-                60L);
-        var big = new SendMessageResult(new RecipientAddress(null, null, "+15559876543", null),
-                false, false, false, false,
-                true,
-                null,
-                false,
-                3600L);
-        var unknown = new SendMessageResult(new RecipientAddress(null, null, "+15550000000", null),
-                false, false, false, false,
-                true,
-                null,
-                false,
-                null);
-
-        var aggregate = new SendMessageResults(1L,
-                Map.of(new RecipientIdentifier.Uuid(java.util.UUID.randomUUID()), List.of(small, big, unknown)));
-
-        assertEquals(3600L, aggregate.maxRateLimitRetryAfterSeconds());
-    }
-
-    @Test
-    void sendMessageResultsReturnsNullWhenNoRetryAfter() {
-        var noRetry = new SendMessageResult(ADDRESS,
-                false, false, false, false,
-                true,
-                null,
-                false,
-                null);
-        var aggregate = new SendMessageResults(1L,
-                Map.of(new RecipientIdentifier.Uuid(java.util.UUID.randomUUID()), List.of(noRetry)));
-
-        assertNull(aggregate.maxRateLimitRetryAfterSeconds());
-    }
-
     @Test
     void successLeavesRetryAfterNull() {
         var result = new SendMessageResult(ADDRESS,
@@ -102,4 +62,52 @@ class JsonSendMessageResultTest {
         assertEquals(JsonSendMessageResult.Type.SUCCESS, json.type());
         assertNull(json.retryAfterSeconds());
     }
+
+    @Test
+    void aggregateReturnsLongestRetryAfter() {
+        var small = rateLimited("+15551234567", 60L);
+        var big = rateLimited("+15559876543", 3600L);
+        var unknown = rateLimited("+15550000000", null);
+
+        var aggregate = new SendMessageResults(1L,
+                Map.of(new RecipientIdentifier.Uuid(UUID.randomUUID()), List.of(small, big, unknown)));
+
+        assertEquals(3600L, aggregate.maxRateLimitRetryAfterSeconds());
+    }
+
+    @Test
+    void aggregateReturnsNullWhenNoRetryAfter() {
+        var aggregate = new SendMessageResults(1L,
+                Map.of(new RecipientIdentifier.Uuid(UUID.randomUUID()),
+                        List.of(rateLimited("+15551234567", null))));
+
+        assertNull(aggregate.maxRateLimitRetryAfterSeconds());
+    }
+
+    /**
+     * Regression for a bug where the aggregate helper could overlook the longest
+     * wait if only some recipients reported a value. Ensures the max is picked
+     * across any mix — which is what downstream captcha/rate-limit clients rely on.
+     */
+    @Test
+    void aggregatePicksMaxEvenWhenSomeValuesAreNull() {
+        var withValue = rateLimited("+15551111111", 7200L);
+        var withoutValue = rateLimited("+15552222222", null);
+        var alsoWithValue = rateLimited("+15553333333", 120L);
+
+        var aggregate = new SendMessageResults(1L,
+                Map.of(new RecipientIdentifier.Uuid(UUID.randomUUID()),
+                        List.of(withoutValue, withValue, alsoWithValue)));
+
+        assertEquals(7200L, aggregate.maxRateLimitRetryAfterSeconds());
+    }
+
+    private static SendMessageResult rateLimited(String number, Long retryAfterSeconds) {
+        return new SendMessageResult(new RecipientAddress(null, null, number, null),
+                false, false, false, false,
+                true,
+                null,
+                false,
+                retryAfterSeconds);
+    }
 }