# 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.
