logs.gokuls.in

What

One flaky Android test, on main since #465: PeerStreamSocketsTest > a burst to a reading peer is not dropped. It failed on #466's first CI run (a PR that touches no Android code), passed on re-run, and passed on main's own run and both #465 runs.

Why it flakes

The test sends eight 1 MiB frames and asserts all eight are QUEUED. It built each frame between sends:

val results = (0 until 8).map { a.send("peer-b", ByteArray(1_048_576) { it.toByte() }) }

enqueue refuses a frame only when the queue is over its 4 MiB budget (from the fifth send) and the writer has been blocked on one 64 KiB piece longer than writeStallMs, which the test's fast limits set to 100 ms. The peer does not read until the sends are done, so the writer fills both loopback buffers and blocks during the first frame. From then on, anything slow on the test thread between two sends counts toward the stall threshold. Four arrays of a million elements built by a cold JIT on a shared runner is enough.

Reproduced locally on the CI harness by adding a 150 ms pause between the fourth and fifth send, which yields the CI failure verbatim:

expected:<[QUEUED x8]> but was:<[QUEUED, QUEUED, QUEUED, QUEUED, QUEUE_FULL, QUEUE_FULL, QUEUE_FULL, QUEUE_FULL]>

The fix

Build the eight frames first, then send them back to back, which is what the test's own comment ("eight ceiling frames in one tick") describes. Eight enqueues take microseconds, so the writer cannot pass a 100 ms stall threshold between them. The comment now says why.

No production code changes. The bound's behaviour under a real stall is covered by the neighbouring test and is unchanged.

Verification

Full Android suite on the CI harness from this branch: 552 tests, 0 failures. The pause-between-sends variant fails on main's test and cannot be applied to this one, since there is no longer anything between sends.

What

A conformance suite for MlsStorage, the trait every binding implements for key material, at offline_protocol_mls::storage_conformance::run and re-exported as offline_protocol::mls_storage_conformance beside the protocol-state suite. Same report shape, same rule: green is the definition of supported.

Nothing has ever held an MlsStorage to the contract the engine relies on. The protocol-state suite exercises the in-memory store only through a test adapter, and the trait's two defaulted methods, its colon-bearing session ids and its concurrency clause were never probed at all.

The checks

Seventeen, each with a negative control in the suite's own tests (one injected defect per check, Faulty in storage_conformance.rs), so a check that stops catching its defect fails CI, and so does a check added without a control. Forcing each check to pass in turn fails the tests for all seventeen.

Key-type isolation runs first, with point writes and deletes of ids nothing else uses, and a failure ends the run before any listed delete. Without that, a backend that merges categories lists the identity under the probe type and the opening cleanup deletes it.

Check	Defect it exists for
store_then_load, binary_values_round_trip, empty_value_is_not_missing	a string-typed backend mangling ciphertext, an empty value reading as absent
store_overwrites	a backend that keeps the first write serves last epoch's keys forever
absent_key_loads_as_none	KeyNotFound where the manager expects Ok(None) before creating a session
delete_removes_and_is_idempotent	a second delete after a crash turning a clean recovery into a failure
key_types_are_separate_namespaces, list_keys_is_accurate_and_scoped, unused_key_type_lists_empty	categories bleeding, a group omitted from the restart listing
key_ids_are_byte_exact	a case-folding or Unicode-normalising backend keeping two peer-chosen group ids in one record
key_ids_as_the_sdk_writes_them	a type:id composition split back on the first colon, which loses every session:<a>:<b> id
long_key_ids_round_trip	a key-length limit below GroupId::MAX_LEN (4096), which group state is keyed by verbatim
large_records_round_trip	a value limit below a large group's ratchet tree (512 KiB probe, see below)
exists_agrees_with_load	an exists override that lies, so the manager mints a second identity
clear_type_is_scoped_and_idempotent	a clear_type that clears everything, taking the identity with the key packages
concurrent_writes_land_whole	an unlocked copy handing a reader half of one epoch and half of the next
listed_records_can_all_be_deleted	the logout contract

The suite writes only under mls_storage_conformance_probe* key types and deletes everything it wrote; tests pin that a record under identity survives a run, including against a backend that merges key types. clear_type is the one check that cannot be made safe before it runs, so the docs say to probe a new backend on a scratch instance until green. Green is not a persistence test: every check runs on one open instance.

The large-record size

Measured, not guessed: a throwaway test grew a group on a counting store. The OpenMLS tree record is the largest thing an MlsStorage sees, at about 1.1 KiB per member:

Members	tree record
65	74 KiB
129	141 KiB
256 (default cap)	270 KiB

The probe is 512 KiB. docs/mls-integration.md said the interface carries "key material a few hundred bytes at a time", which is what a Keychain- or preferences-backed provider would have sized itself for; corrected.

Shape

• The report types (ConformanceReport, ConformanceFailure) move to the MLS crate and the engine re-exports them, so the two suites share one JSON shape and the FFI's existing run_storage_conformance is unchanged. pass, fail and check become public so the engine's suite can keep building its report.
• The concurrency check uses scoped threads (eight writers, sixteen overwrites each, 4 KiB values of one repeated byte per writer), so a torn read is any value that is not exactly one writer's; nothing is timed. The tearing negative control is deterministic by construction (the second distinct writer's next load must see the mix), and the negative-control test was repeated 20 times locally without a flake.

Not in this PR

• FFI exposure. The bindings' MlsStorageProvider implementations are not held to the suite yet; C11 and the MLS guide say so. A run_mls_storage_conformance(MlsStorageProvider) mirror of the state one is a UDL change with a three-binding regeneration, so it is its own PR.
• The file-backed Rust stores (PR7 of the same program), which this suite exists to gate.

Verification

cargo fmt --all -- --check, cargo clippy --workspace -- -D warnings, cargo test --workspace --lib (1864 in the engine, 133 in mls, all green), RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps. The measurement test was run and removed; it is not in the diff.