macltok

macltok review, 2026-10-25

Read-only review of everything that landed on 2026-10-25. In macltok that is event context pagination, the encrypted mentions inbox, bounded history repair, segmented history and the pack fixes, as published on vapour 8fd8dcf6. In the sdk fork it is the sliding sync commit fence, reseeding, room-owned history views, replacement snapshots, segmented history and cinny reaction shortcodes, up to 9b2c17fb1.

Nothing was built or run. Every finding was read against the code, and the ones marked verified were reread by hand after the reviewer reported them. Ids carry a status that starts as OPEN.

S. sdk fork correctness

S1. A limited sync can lose its gap. verified, OPEN

handle_sync takes older_boundary from the in-memory linked chunk only (event_cache/caches/room/state.rs:949). After auto-shrink, or a restart with lazy loading, a room whose last chunk is the trailing gap from a recent load holds no events in memory. A limited sync for that room then records an ordinary gap and no HistoryGap. On reopen the retained events and the new ones render as continuous, which is the lost interval malice reported.

Fix direction. Take the boundary from the store’s newest event when memory has none.

S2. An empty fill page panics when the next chunk is a gap. verified, OPEN

apply_page always passes a gap to push_backwards_pagination_events (room/history.rs:549). With no events, replace_gap_at removes the gap and returns the next chunk’s first position. If that chunk is itself a gap, insert_gap_at returns ChunkIsAGap and the .expect at event_linked_chunk.rs:403 fires. On native the room’s history coordinator dies and every later history call for the room returns ClientDropped. On wasm it likely takes the worker with it.

S3. A limited sync during a recent fetch leaves two adjacent gaps. OPEN

The recent fetch captures its anchor before the network request (room/history.rs:221). A limited sync that lands meanwhile installs its own gap, and the fetch then inserts its events and gap after it. The stale sync gap shows as a gap row over continuous history, and filling it walks into S2.

S4. Every room open persists a gap chunk. OPEN

The recent load persists a gap whenever an anchor exists, Joined included (room/history.rs:284-319). macltok runs it on every open, so joined gaps pile up in storage, in history_gaps() and in every gap publication. The trailing gap left behind is also what produces the empty in-memory chunk S1 needs. The RecentHistoryOutcome.gap doc says the field is absent when nothing needs joining, and it is not.

S5. history_candidate decrypts the whole room twice per operation. OPEN

It walks every stored chunk under the state write lock, which also blocks the room’s sync (room/state.rs:805-825). A recent load and each fill page do it twice, and the commit then holds the whole history in memory until shrink. The insertion loop at history.rs:301-312 is also O(n·m).

S6. sqlite event cache reads moved onto the write connection. OPEN

72db3bb7d moved load_all_chunks, load_all_chunks_metadata, load_last_chunk, load_previous_chunk, filter_duplicated_events and find_event_relations into with_immediate_transaction so that register_linked_chunk_owner can do a one-time INSERT OR IGNORE. Reads now serialise behind every write. Plausibly part of the slow sqlite login.

S7. Pinned and event-focused views die after a clear. OPEN

clear_loaded_views drops the pinned cache and drains event-focused caches, while threads are cleared and kept (room/state.rs:866-877). An open pinned timeline gets a Clear and then a closed stream. An event-focused timeline gets a Clear, then MissingCache on paginate. An interrupted event-focused pagination reports hit_end_of_timeline.

S8. A room Update::Clear on error paths wipes view chunks under live caches. plausible, OPEN

Room Clear now deletes thread and pinned chunks and standalone events on all three backends. The pagination and shrink error paths send it without calling clear_loaded_views, so loaded thread and pinned caches keep writing against chunk ids that no longer exist.

S9. Reset on the live timeline drops receipts and identities. OPEN

The Reset arm of the timeline state transaction clears read_receipts and fully_read_event and never repopulates them (timeline/controller/state_transaction.rs:216-231). It also discards recycled_timeline_ids, so every surviving row gets a new id.

S10. A commit hook that never resolves stalls sync for good. OPEN

The hook runs inside handle_response holding the position lock (sliding_sync/mod.rs:338). Nothing can cancel it, and every later sync_once and expire_session waits on that lock. macltok’s hook always resolves today, so this is a hazard in the api, not a live bug.

S11. Smaller sdk findings. OPEN

• visited_cursors grows by one cursor per page, forever, and is re-encrypted on every gap rewrite.
• RoomEventCache::clear and reseed .expect their join handles, so a task panic or runtime shutdown panics the caller.
• sqlite writes plain gaps as a bare string and history gaps as an object behind an untagged reader. Always writing the object would leave one reader shim.
• A downgrade fails to read history gaps and clears the room. Fine pre-1.0, noted.
• IndexedDB room Clear decrypts every chunk in the database to find one room’s.
• The cinny "shortcode" field is a bare literal in three places, and reaction_shortcode’s doc says it only reads MSC4027.
• The pinned Lagged path subscribes to the new cache and keeps reading the old receiver. Upstream bug, now hit after every clear.

M. macltok correctness

M1. The mentions inbox rewrites itself on every sync response. OPEN

batch writes a new checkpoint for every response, typing and receipts included, and save serialises, encrypts and durably commits the whole inbox inside the sync hook (mentions/service.rs). With the 64 MiB default budget that is up to 64 MiB written per typing notification before pos may advance, at roughly four times that in peak memory.

Related costs on the same path.

• Every background completion also does a full save. Permanently undecryptable candidates are never dropped and retry forever with backoff capped at 256s, and any decryption report resets every candidate’s retry.
• filter_ignored reads the ignore list from the state store once per entry per batch.
• Budget eviction drops one window and re-encrypts the whole inbox before checking again.
• Candidate eviction removes the smallest key, which means lexicographic room id, not the oldest.

M2. A mentions failure stops sync. OPEN

A failed save fails the hook and the hook stops room list sync. An IndexedDB quota error means no messages at all. A corrupt mentions store fails Account::build, and the only way out is StoreConfig::delete, which also drops the crypto store.

M3. A crash between hook ack and pos write raises a false global gap. verified, OPEN

The hook records next_pos P2 durably, then the process dies before the sdk writes pos. On restart the sdk replays with previous_pos P1, the checkpoint says P2, and batch treats it as a discontinuity, marking every observed room unresolved (mentions/service.rs:404). Keeping the previous checkpoint and accepting a replay from it would fix it.

M4. Event context lost what the timeline gave it. OPEN

ywsvysot moved EventContext from an event-focused sdk timeline to raw /messages through normalise_observed. That fixed real bugs in the focused path, but items now have no display names or avatars, edits arrive as separate messages, reactions and redactions arrive as Other, replies stay ReplyDetails::Pending forever and UTDs never retry.

Also in context.

• Truncating a page to limit while returning the server’s end token skips whatever was cut, and limit: 0 is accepted for pages.
• Fetch awaits before and after one after the other.
• Context media go into the shared 2048-entry registry and are never released, so heavy context paging can evict media for rows still on screen.

M5. Cancel is reported as failure. OPEN

A user Cancel and the cancel that runs on room close both publish RecentHistoryState::Failed("…interrupted") and a report with a failure and no stop (engine/repair.rs:284, :347). HistoryRepairStop::Cancelled exists and is never sent.

M6. Network failures report as store errors. OPEN

cache_error maps every EventCacheError to ErrorKind::Store (engine/repair.rs:444), PaginationError included. The Failed => Network arm cannot be reached because the sdk returns the error after committing Failed.

M7. Every history completion sends a full reset. OPEN

publish_history flushes and then always resyncs (engine/timeline.rs:134-145), so every completion, failures and close included, sends a TimelineReset, and every room open produces two. The segmented history doc promises incremental insertions. tests/engine.rs ignores resets during recent loading, which hides it.

M8. Smaller macltok findings. OPEN

• The mirror never removes recent_history or repairs entries when a room closes, and RecentHistoryState::Idle is never emitted.
• initial_history_limit no longer applies to room timelines, and a failed recent load leaves a room with no initial history at all.
• An Updated result for an already joined gap still counts a page.
• The Joined gap arm in timeline normalisation cannot be reached, because the sdk drops joined gaps.
• A finished recent load publishes both RecentHistoryChanged(Loaded) and a HistoryRepairReport for the same edge.
• evict_one sets an entry’s revision to the current one, not the next one, so revision-keyed consumers miss the change.
• Mentions in a room left within the same response are dropped by the joined check.

D. dead api and design

D1. Most of the history repair api does nothing. OPEN

Start.budget is validated and ignored. The policy is echoed and stored in the mentions inbox and never read. Continue always errors. StartRecent duplicates the open-time load. GetPolicy and SetPolicy in the actor are unreachable because the engine handles them first. Never emitted are HistoryRepairStrategy::Overlap, HistoryRepairStop::Cancelled and NoAnchor, HistoryRepairPhase::FetchingRecent, coverage_duration_ms and threshold_evaluated. Pre-1.0, so delete rather than keep them for serde.

D2. HistoryGapLifecycle::Fetching is never emitted. OPEN

A consumer has to match a Bridging report’s gap_id to know a gap is filling, then infer the end from a later report. Either emit it on the gap row while a fill runs or delete it.

D3. Mentions shapes. OPEN

• MentionGap.reason is a string.
• MentionCoverage.incomplete_history is always true.
• Every newly seen room adds a gap and the list caps at 128, so truncated is true after the first sync on any account with more than 128 rooms.
• The store is a generic multi-record store with HMAC’d keys that only ever holds "inbox".
• MentionsCommand is consumer-built and missing #[non_exhaustive], and so are HistoryRepairCommand, HistoryRepairPolicy and HistoryRepairBudget.

D4. Pack shims. OPEN

clone_pack_image_with_usage, clone_sticker_with_usage and clone_pack_with_usage sit beside wrappers under the old names. known_usage drops usage values it does not know, so clone stops being faithful for msc2545 extensions, and it duplicates the default-usage rule in normalise::image_pack::usage. Some(vec![]) silently means inherit.

D5. reseed has no caller. OPEN

RoomEventCache::reseed, its state method, reseed.md and its tests are about 500 lines of fork surface. macltok uses fill_history_gap_once and the segmented history doc says it does not reseed. The generation fencing stays, clear and commit_history_candidate need it.

D6. Event context shape. OPEN

EventContext.event is optional but always present on success. EventContextPage.end_reached repeats next_token.is_none(). The transport takes u32 and clamps to 50 while the engine rejects anything over 50.

H. housekeeping

• H1. sdk 9b2c17fb1 is only on the fork’s segmented-history branch. The macltok branch is at 9b9b16982. Fast-forward it before tidying branches or cargo cannot fetch the pin. OPEN
• H2. lyvnurlv d1313f47 is an empty change merged into vapour, and the segmented history doc cites it. At xnzknouu the pins are split three ways. OPEN
• H3. repair.rs landed without rustfmt. OPEN
• H4. docs/segmented-history.md says the lost interval drops are an earlier path, and both prev_batch drops are still at state.rs:943. It says concurrent history operations return RoomBusy, and only a fill does. Much of it is handoff status that will rot. OPEN
• H5. docs/mentions-discovery.md says replies need a notifying highlight while the code accepts direct alone, and says context capture runs after the commit while the watch publication runs before the ack. OPEN
• H6. American spellings in sqlite.rs and the mentions doc. OPEN

Checked and sound

• Gap metadata and the mentions inbox are encrypted at rest on sqlite and IndexedDB. The inbox head compare-and-swap holds.
• The sliding sync pos advances only after the hook acks, on both stores.
• Coordinator cancellation, shared recent fetches and atomic event, gap and cursor commits.
• The old timeline watcher stops before the reset goes out, and the mirror ack precedes the command reply.
• Both merges keep both sides. All nine sdk pins and both lockfiles agree on 9b2c17fb1.
• The spare sdk worktrees hold nothing that is not already committed.