Skip to content

Leave unaddressable roster entries off live Google snapshots instead of quarantining them - #211

Draft
MaxGhenis wants to merge 2 commits into
mainfrom
fix/live-roster-skip-unkeyable
Draft

MaxGhenis wants to merge 2 commits into
mainfrom
fix/live-roster-skip-unkeyable

Conversation

@MaxGhenis

Copy link
Copy Markdown
Owner

Why

Finding F1 of the third review of #200. On the live path, refreshConversation keyed every roster entry of a Google conversation snapshot. One entry with no number (libgm delivers these) or a number v2keys rejects made IdentityKey fail, and the whole frame was quarantined. The thread then missed that update: its kind, title and roster.

The review's worst case is a group that v2 first sees through an incoming message:

The live inbox has 4 such frames; details under "Live inbox evidence" below.

What

  • refreshConversation drops roster entries without a usable address (addressable: a non-empty value that v2keys.IdentityKey accepts) before anything reads the roster. The roster it writes and Route Google frames by participant identity across device id-space resets #176's rebinding (googleConversationEventTarget) therefore judge the same entries. resolveEventPeers uses the same check.

  • A snapshot that left entries off removes only members it can rule out. A stored member the snapshot doesn't name stays when a left-off entry could be that member:

    • If any left-off entry isn't flagged as the account, it could be anyone (Dedupe duplicated conversation participants instead of quarantining the snapshot #160 saw the account's own number listed unflagged), so every unnamed member stays.
    • If every left-off entry is flagged as the account (the only shape seen live), only the account's own rows stay. The snapshot is complete for peers, so a departed peer is still removed.
    • A complete snapshot replaces the roster as before.

    Without this rule, a 1:1 snapshot whose peer has no number would wipe the thread's peer. That would also disable Route Google frames by participant identity across device id-space resets #176's sender check for the thread.

  • One Info log per such snapshot: Left conversation snapshot participants without a usable address off the roster (unaddressable, unaddressable_self).

  • Runbook:

    • The quarantine bullet now diagnoses the recurring alerts.
    • A new section explains the rule and why re-applying quarantined frames is unsafe (below).
    • It gives the online-backup command for copying the live store.

Invariants

These are executed by TestLiveRosterPropertyUnaddressableEntriesChangeNothingElse (testing/quick, 120 cases). Each case compares two stores given the same setup: one gets the snapshot, the other the same snapshot without its unaddressable entries.

The generated rosters mix several kinds of entry:

  • addressable ones: some given only as a formatted number, some renamed, some listed twice, and the account's own number listed unflagged;
  • entries with no number, a bare +, or a blank number, flagged as the account or not.

They are applied to a wire v2 didn't know, or knew from an incoming message, an outgoing message, a 1:1 or a group, with or without other threads to rebind to.

  • P1: nothing is quarantined.
  • P2: the binding, every conversation row, identity and message, and remote_rebinds are identical to the clean snapshot's.
  • P3: the target thread's roster is the clean snapshot's roster plus exactly the stored members the rule above keeps. Every other thread's roster is unchanged.
  • P4: a complete snapshot's roster is exactly its addressable entries.
  • P5: a history frame fetched afterwards with that snapshot imports its message, in both stores.

A coverage check fails the test if any of 6 case classes goes unsampled. The classes include: self-only drop removes an unnamed peer, peer drop keeps one, rebound, and direct corrected to group.

Tests

  • TestLiveGroupSnapshotWithANumberlessMemberCorrectsAMessageMintedThread, the review's scenario through the real decoder:
    • Karl's message mints lr-g as direct.
    • A fetched frame is skipped (history_skipped=2).
    • The live group snapshot, which includes a member with no number, corrects the thread in place: kind group, title, Karl and Shoshana, remote_rebinds=0, quarantined=0.
    • The early frame fetched again and a new fetched frame both import.
  • TestLiveSnapshotListingSelfWithoutANumberAppliesLikeTheCleanOne: the observed shape (self listed 5 times, 4 without a number). Every table matches the clean snapshot's.
  • TestLiveSnapshotWithUnaddressableEntriesKeepsTheMembersTheyMayBe, four subtests:
    • a 1:1 thread keeps its peer;
    • a group keeps an unnamed member until a complete snapshot removes it;
    • left-off self entries say nothing about peers;
    • the account's row stays when its only entry has no number.
  • TestLiveRebindJudgesTheAddressableRoster: a stale binding plus a + entry and an entry with no number moves exactly as the clean snapshot does. It is a differential against a twin store, so it holds under Keep same-roster Google threads distinct; re-key only threads whose id is dead #201's rebinding rules too.

Mutants (each through go test -overlay; generator in the evidence folder):

  • Killed: the old code (fails all 5 tests) and 13 mutants:

    • the filter is not applied;
    • an empty-only check;
    • members never kept;
    • members kept on complete snapshots;
    • the stored row wins over the snapshot's;
    • rebinding sees the unfiltered roster;
    • self-flagged entries kept;
    • kept rows marked inactive;
    • a peer drop that skips self rows;
    • three variants of the self/peer split;
    • self counted by name.

    The property test alone kills each mutant of the keep rule.

  • Equivalent: rebinding given the unfiltered event while resolveEventPeers checks addressable (it reads the roster only through that function).

  • Not killed: resolveEventPeers reverted to the empty-only check. The live path filters first, so it is unreachable there. On the history path it only decides whether an unbound snapshot with a + entry mints or is skipped, and that is unreachable once History stale-binding check: ignore unkeyable roster entries, and pin its controls #207's filter lands.

go test ./internal/ingest/ and go vet pass.

Live inbox evidence

These figures come from an online backup of the live v2/store.sqlite3 (9,155 Google frames, 2026-07-19 to 10-09).

  • 4 of 3,810 conversation frames hit this. All 4 are 1:1 threads whose snapshot lists the account itself 3–5 times (IsMe), all but one copy without a number: wires 1334 (08-08), 2923 (09-11), 2411 (09-21) and 2981 (10-09 05:50 EDT). No group was affected, and no + entry was seen.
  • Replay into an empty store:
    • main quarantines 66 frames: 59 updated_at_ms >= created_at_ms CHECK failures (Keep same-roster Google threads distinct; re-key only threads whose id is dead #201 changes this), 4 identity value is empty, and 3 message_id UNIQUE failures. With this branch it quarantines 62.
    • Every other counter is equal, and the final state (2,925 rows of threads, rosters, message counts and identities) is identical.
    • The live daemon's quarantined: 1 on 10/9 is consistent with the 2981 frame, the only replayed quarantine after 10/2.

Frames already quarantined: no replay

No command re-applies a processed frame. The only mechanism is to clear processed_at_ms with the daemon down; Worker.Run then drains every unprocessed row. I tried it on a copy of the live store with this branch's worker:

  • 2411, 2923 and 2981 change only updated_at_ms. Later clean snapshots had already given those threads the same state.
  • The 1334 frame dates from the old phone's ID space. Route Google frames by participant identity across device id-space resets #176 judges it against today's bindings and moves the short code's current thread (wire 1333, 26 messages) to wire 1334 (remote_rebinds=1). An outgoing message on 1333 then started a new thread.

So nothing needs repairing on this install, and a blind re-apply can split a live thread. The runbook says to wait for the thread's next live snapshot instead, then run a window backfill. It also lists the precautions if a re-apply is ever needed:

  • dry-run on a copy first;
  • clear only the newest snapshot per wire, and only frames received since the last re-pair;
  • remote_rebinds must stay 0.

This matters for the planned re-drive of processed-but-unapplied frames (#196 follow-up).

Known limits

  • A snapshot with no addressable peer trusts the stored binding. If a snapshot's only non-self entries lack a number, googleConversationEventTarget keeps the stored binding (no peers to judge it by), and the snapshot's title and kind are written to that thread. main already does the same for a snapshot that lists only the account. Before, such a snapshot was quarantined. After a phone swap that left the wire stale-bound, the title would land on the old thread until that thread's own snapshot arrives. This has never been seen live.
  • Kept members stay until a complete snapshot. A member who left a group stays listed until a snapshot that addresses every entry, or whose left-off entries are all the account's, removes them.
  • Senders and reactors are unchanged. An incoming message whose sender is a bare +, or a reaction from one, still quarantines its frame; a sender with no number already becomes a null sender. That is History stale-binding check: ignore unkeyable roster entries, and pin its controls #207's review finding 4, and it is filed as a separate task.

Coordination

Evidence, harnesses and the store snapshot: ~/reviews/openmessage-live-roster-2026-10-09/.

🤖 Generated with Claude Code

MaxGhenis and others added 2 commits October 9, 2026 08:13
…of quarantining them

A live conversation snapshot with a roster entry that has no number, or one
v2keys can't key, failed IdentityKey in refreshConversation and quarantined
the whole frame, so the thread missed that update (kind, title, roster). A
group v2 first saw through an incoming message stayed a direct thread with
the sender as its peer, and catch-up history skipped every fetched message of
it as contradicting that thread (third review of #200, finding F1).

refreshConversation now drops such entries before anything reads the roster,
so the roster write and #176's rebinding (googleConversationEventTarget,
through resolveEventPeers) judge the same entries. A snapshot that left
entries off keeps each stored member it doesn't name that a left-off entry
may be: any member when one of them isn't flagged as the account, only the
account's own rows when all are. A complete snapshot still replaces the
roster.

Live inbox (9,155 Google frames, 2026-07-19 to 10-09): 4 snapshots hit this,
all 1:1 threads listing the account itself several times without a number.
Replayed into an empty store, quarantines drop from 66 to 62 and the final
state is identical. The runbook now diagnoses the recurring quarantine alerts
and explains why re-applying quarantined frames is unsafe.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant