fix: deliver store-and-forward mail on a peer's second and later reconnects

[Chessing234]

Store-and-forward exists so mail for an offline peer is handed over when it comes back. On current main that happens once per peer per app session, and never again.

The peer latch is set but never released

fun sendCachedMessages(peerID: String) {
    if (cachedMessagesSentToPeer.contains(peerID)) {
        return // Already sent cached messages to this peer
    }
    cachedMessagesSentToPeer.add(peerID)

Nothing removes a peer from cachedMessagesSentToPeer. Not cacheMessage, not a disconnect — the only writes are that add, and two clear()s. So:

1. Peer connects. It is handed whatever is held for it and goes into the latch.
2. Peer leaves. Messages for it are cached — which is the entire point of the feature.
3. Peer returns. sendCachedMessages returns at the first line. Nothing is sent.
4. The mail sits in messageCache until MESSAGE_CACHE_TIMEOUT_MS (12h) drops it.

The latch is a reasonable idea stated wrongly: it means "this peer has been handed everything we hold", and caching new mail for that peer is exactly the event that stops being true. Released there. A reconnect with nothing new still sends nothing, which is what the latch is for and is pinned by a test.

The periodic cleanup bought its bound by forgetting

if (deliveredMessages.size > 1000) { deliveredMessages.clear() }
if (cachedMessagesSentToPeer.size > 200) { cachedMessagesSentToPeer.clear() }

Both sets are correctness state, not caches. deliveredMessages is what stops an already-delivered message being queued a second time; the peer latch is what stops a batch going out twice in a session. Emptying them wholesale discards that and does it repeatedly, since the next entry immediately starts regrowing toward the same threshold.

Both are now LinkedHashSet and trimmed oldest-first to their cap. Same memory bound, without the amnesia. (It is worth noting the wipe was also what accidentally papered over the bug above — every few cleanup cycles a peer would become eligible again and its backlog would finally go out. That is not a mechanism anyone would design.)

Verification

Local run of the CI job (testDebugUnitTest lintDebug, JDK 21). Counts from --rerun-tasks on both sides so they are full-suite runs rather than incremental leftovers:

• 608 → 612 tests, 0 failures, lint clean. Per-class diff shows one class changed — the new StoreForwardManagerTest at 4 — and nothing else moved.
• The reconnect test fails on unmodified main: peer served once, mail cached while away, nothing delivered on return. I ran that one against main separately, since the trim tests reference constants this PR introduces and so cannot compile there.
• The other three are the properties the fix must not break: a reconnect with no new mail still sends nothing, the trim leaves the cap populated instead of emptying it, and a recently delivered message is not the one forgotten.

Scope

Deliberately not touched: MESSAGE_CACHE_TIMEOUT_MS, the cache size caps, and the favourite/regular split. Whether 12h is the right retention is a separate question from whether delivery works at all.

CI has not run. Fork PRs here sit at action_required until a maintainer approves the workflow, so the local run is the evidence, not a green check.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eab61056dd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Run this Android-dependent test under Robolectric

This new plain JUnit class instantiates StoreForwardManager and calls methods that execute android.util.Log.*. The app unit-test config does not enable mocked Android return defaults, so :app:testDebugUnitTest runs this on the local JVM with the Android stub methods and fails with the usual “Method d in android.util.Log not mocked” before the assertions; existing mesh tests that touch Android APIs use @RunWith(RobolectricTestRunner::class)/@Config. Since CI runs testDebugUnitTest, please add the Robolectric runner or remove the Android Log dependency from this test.

AGENTS.md reference: AGENTS.md:L79-L79

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Encode test recipients like production packets

The helper builds recipientID from the ASCII characters of PEER, but the existing mesh send paths put the peer id on the packet as the 8-byte binary value from hexStringToByteArray(peerID). Because the new reconnect tests use a different recipient encoding than real packets, they can pass while releaseAlreadySentLatch and the later cache filter still fail to match the hex peerID used by sendCachedMessages; build these test packets with the same binary peer-id encoding and normalize inside the manager accordingly.

Useful? React with 👍 / 👎.