logs.gokuls.in

Review follow-ups on #442. The mailbox park itself was right; what was wrong is

how much of a burst it reached, and a handful of places where the reasoning was

either overstated or not written down.

The React Native bridges only carried stored for the first frame of a burst.

The relay answers every SendMessage separately, but handleRecipientUnreachable

reported only what drainRecipient returned — and the first verdict to arrive for

a recipient drains that recipient's whole in-flight queue. So with frames A, B and

C in flight to a sleeping peer, DeliveryError { A, stored } resolved all three:

A took relay_stored, B and C took recipient_unreachable and a probe. The

verdicts naming B and C then found nothing to drain and reported nothing at all,

so each of them paid a probe rung — one relay write and one

MessageUndeliverable — before the relay said stored again and the frame

finally parked probe-free. Nothing was stranded, but on mobile the feature only

really took effect for one frame per burst, which is the traffic it exists to

remove.

The Python client never had the gap: it appends the echoed id when the drain

missed it. Both bridges now do the same, guarded on the id not already being in

the drain so a frame still in flight is reported once rather than twice. It is

safe for an id that is not ours, because the core ignores an id with no outbox

entry, and safe for one that is already parked, because park_unreachable_dm

clears the retry slot before it decides whether to re-arm it — which is what

actually retires the probe on the second verdict.

Tests for the path the fix stands on

• test_relay_stored_verdict_drops_an_existing_probe (core) — a relay_stored

verdict for a frame that is *already* being probed retires that probe. This is

the ordering the bridge fix depends on and nothing pinned it; without it the

bridges could report the right token and change nothing.

• test_a_later_verdict_still_reports_its_own_frame_as_held (Python) — the burst

sequence end to end: the first verdict resolves both frames, the second still

reports its own frame as held. This documents the contract the bridges were

violating, on the client that already satisfied it.

• react_native_relay_parks_a_stored_delivery_error (uniffi source guard) — now

also pins the append and its not-already-drained guard, per platform. The guard

as written asserted token *selection* only, so it would not have caught this.

• test_stored_without_pushed_parks_nothing (Python) — `MessageSent

{ pushed: false, stored: true }` is an accepted delivery to a live session, so

there is no park to make and no probe to drop. Every client already gates the

park on pushed; nothing said so.

Two things the rationale claimed, or left out

park_unreachable_dm's comment and the changelog both said recovery was "the

reachability edge behind it for a copy that is evicted or expires." The edge is

not conditional on eviction: it fires on any reconnect, and

flush_outbox_for_peer_via selects on "in the outbox, not awaiting an ACK", so a

returning peer re-drives a held frame alongside the relay's own drain exactly as

it would an unheld one. What the flag actually saves is the 15s→600s ladder

*while the peer is away*. Both now say that, and both now name the consequence:

past the mailbox's own retention the guarantee is that edge and the outbox

lifetime, which is the same trade edge_driven_unreachable_dm opts into for

every parked frame, taken unconditionally here because the mailbox is already

committed to the redelivery.

The comment also now records why stored is deliberately *not* persisted on the

outbox entry, unlike relay_pushed. A restart re-drives the outbox and the relay

answers stored again for a frame it still holds, so the park is re-derived;

relay_pushed has to survive because it suppresses an app-facing event no later

answer restates. Persisting this one would also be wrong on its own terms — it

records what the mailbox held at park time, and the mailbox can evict before the

next launch, so a restored flag would suppress the probe on the strength of a

copy that may be gone.

The gateway contract documented neither flag

docs/spec/gateway-contract.md is the shared spec, and its Submit/Verdict shapes

never mentioned pushed (which predates #442) or stored. Both are now in the

shapes and in prose: optional, defaulting to false when absent, with a missing

key read as false and never as unknown. It also states the rule the bridge bug

came out of — both flags are statements about the one frame the verdict names,

never about the recipient — and the obligation that follows for a gateway holding

several frames: set stored on each frame's own verdict.

Deliberately not in this PR

• A stored field on MessageUndeliverable, so an app could tell "held, will be

delivered" from "lost, being probed". The reason string must stay normalized at

the boundary, so this needs a real field, which touches the telemetry wire

crate, the scrubber, the React Native event types and UPGRADING.md. Its own

change.

• A Welcome the mailbox holds still runs its own probe ladder. The argument for

dropping it is identical to the DM case, but the Welcome lifecycle is a

separate state machine and #442 deliberately left it alone.

• Extracting the id/token selection into a testable policy object per platform,

the way PresenceWatchPolicy and GatewayVerdictTracker are. It would let the

source guard shrink to nothing. Not worth refactoring shipping bridge code on

two platforms in the same change as the fix.

Verification

cargo fmt --all -- --check clean. cargo test -p offline-protocol --lib and

cargo test -p offline-protocol-uniffi --lib pass, including the three new and

extended tests. The full Python suite passes locally (269 tests) including the two

new ones. The Kotlin expression added to the Android bridge was compiled and

exercised standalone over all four of its branches, since InternetManager itself

has no local unit-test seam; CI's Android and iOS jobs cover the real builds.