Skip to content

Adopt migrated v2 conversations in the legacy mirror instead of refusing them - #224

Open
MaxGhenis wants to merge 1 commit into
claude/interesting-payne-d53972from
claude/mirror-adopt-migrated-threads
Open

MaxGhenis wants to merge 1 commit into
claude/interesting-payne-d53972from
claude/mirror-adopt-migrated-threads

Conversation

@MaxGhenis

Copy link
Copy Markdown
Owner

Stacked on #217 (base is its branch; I'll retarget to main once it merges). This is the follow-up #217 filed under "Not changed".

What was wrong

After #217, the legacy→v2 mirror refuses any thread whose natural key (account_id, remote_conversation_id) already belongs to a v2 row under another id. Every pre-migration thread on a store built by openmessage migrate is such a thread, since the migration keys conversations by v2keys.DeriveID. So a legacy-primary daemon with OPENMESSAGES_V2_SEND=1 on a migrated store (for example after rolling back from v2-primary) could only send to threads created after the migration. Sends, reply sends and mark-read mirroring to migrated threads got a 409.

The three questions

1. What do the adapters consume? RemoteConversationID, never the v2 conversation_id. The dispatcher loads the conversation and passes bridge.ConversationRef{RemoteID: conversation.RemoteConversationID} for text, media, reactions and read receipts (internal/messaging/dispatch.go:206, :377, :530, :680). Replies carry the quoted message's RemoteMessageID (internal/messaging/service.go:176). The Google, WhatsApp and Signal adapters read only req.Conversation.RemoteID and req.ReplyTo.RemoteID. The mirror's doc comment ("byte-for-byte equal to the legacy ID consumed by the live adapters") was therefore wrong about who consumes it.

What does treat the v2 conversation_id as a legacy id is the legacy visibility projector, which runs only on legacy-primary daemons (cmd/v2stack.go:377). legacyProjection wrote each confirmed send into the legacy store with ConversationID: row.ConversationID, and PublishMessages(row.ConversationID) announced it under that id. With a hash id, a send would land in a legacy thread no one lists, and the legacy UI (primary in this mode) would never show it. The legacy upsert also sets conversation_id = excluded.conversation_id on a message-id conflict (internal/db/messages.go:59), so an echo the legacy store already had would be moved out of its thread. Adoption is only safe with the projector fixed.

2. What would adopting break: reply targets. I compared each platform's remote message id across the mirror (replyRemoteID), the migration (deriveRemoteMessageID: source_id, else the legacy id with its platform prefix stripped) and the v2 decoders:

Platform Legacy message_id / source_id (writers) Mirror (before) Migration v2 decoder Duplicate?
Google Google's id (internal/client/events.go:160) / empty (no live writer sets it) message_id message_id GetMessageID() (googledecoder.go:387) No
WhatsApp whatsapp:<id> / <id> source_id source_id envelope id No
Signal, sent signal:<ts> / <ts> (every Signal writer, including the Signal Desktop importer, stores message_id = "signal:" + source_id) signal:<ts> <ts> <ts> (signaldecoder.go:386) Yes

v2 dedupes messages by (account, conversation, remote_message_id), keeping the first message id. The mirror's legacy-reply: row under signal:<ts> would therefore sit beside the migrated or ingested <ts> row as a second copy of the quoted message. That copy would show in reads if the same store became primary again by re-setting OPENMESSAGES_V2_PRIMARY, which resolveV2RuntimeMode allows. A re-cutover through openmessage migrate builds a fresh store and carries only pending outbox intents, so it would not inherit the copy. The same duplicate already happens today in post-cutover threads when v2 ingest runs beside the legacy-primary daemon.

The mirror kept the full id only because the Signal transport (signalQuoteArgs) resolves quotes with legacy GetMessageByID. A probe confirmed it: "signal:1700000000002" resolves, while "1700000000002" fails with signal reply target not found. The same applies on v2-primary: SubmitTextV2 forwards the v2 message's bare remote id, so every v2-native Signal quote-reply failed at dispatch.

3. Adoption is safe with those two fixes, so this PR implements it.

Changes

Where Change
v2wire.mirrorConversation Looks up the natural key first. If a row under another id owns it, returns that row unchanged (adopt). Otherwise UpsertOwnedConversation as before. If v2 ingest takes the key between the lookup and the upsert, the guarded upsert writes nothing and the mirror adopts the ingested row. The lookup comes first so that adopting takes no SQLite write lock.
v2wire.Projector Writes each confirmed send into the legacy thread named by the v2 conversation's remote_conversation_id and publishes that id. For conversations the mirror created, that equals the old value byte for byte.
v2wire.MirrorReplyTarget Uses the remote id the migration and the decoders use. Signal now uses the bare timestamp, and a copy the mirror stored earlier under signal:<ts> is reused. Google refuses (ErrReplyTargetUnavailable, naming the id) when v2 holds the message under a source_id other than its message id: the migration keys by source_id, but the transport quotes the message id. No live Google writer sets source_id; the R5 fixture does.
signallive.signalQuoteArgs A bare id that is not itself a legacy message id is retried as "signal:" + id, and only a Signal row is accepted. Every id that resolved before resolves identically (differential property test).
web/apiv1.go The 409 mapping's comment now says when it can still fire (a concurrent delete of the row the mirror was adopting).
docs/agent-runbook.md The cutover bullet now describes adoption, the projector and reply-target behavior, and the one refused case.

Invariants (tested)

  • One row per thread: each thread resolves to the single v2 row that holds its natural key, and that row's remote_conversation_id is the legacy id. The mirror keys a row by the legacy id only for a thread v2 has no row for.
  • No rewrites: rows the mirror did not create (conversations, accounts, devices) stay byte-identical.
  • No duplicate messages: a reply target resolves to the same v2 message on every call, and v2 never holds two copies of one legacy message.
  • Cursors: they land on the account's local installation device and the resolved row. Cursors only move forward (read_cursors.go:81), so an older mark-read leaves the migrated cursor alone.
  • Projector: it never writes a legacy message under a v2 hash id.
  • Signal quote fallback: it only adds resolutions. For an id that newly resolves, it quotes exactly the message whose legacy id is "signal:" + id.

Tests:

  • TestMirrorOnMigratedStoreProperties: 30 random seeds × up to 10 calls on a store built by migration.Transform, with post-cutover threads randomly pre-keyed by "v2 ingest", with and without the target message.
  • TestSignalQuoteArgsFallbackOnlyAddsResolutions: differential property against exact lookup, 60 random stores.
  • TestMirrorReplyTargetOnMigratedStoreReusesMigratedMessages: differential check of the mirror's remote ids against what the real migration wrote.
  • TestR5RollbackLegacyPrimarySendsIntoMigratedThreads: end to end in cmd. It runs the real openmessage migrate, a legacy-primary v2 stack with scripted adapters, and replies on Google, WhatsApp and Signal. For each it checks that the transport saw the legacy thread and the migrated quoted id, that the v2 thread gained only the sent message, that the projector wrote the send into the legacy thread with the right ReplyToID, and that nothing landed under the hash. It also covers the refused Google case and mark-read.
  • Also: adoption without rewrites, the ingest race (through the test seam beforeOwnedConversationUpsert), the projector on an adopted thread with and without an ingested echo, reuse of an earlier full-id Signal copy, and HTTP /api/mark-read on an adopted thread.

Mutation check (each mutant applied alone, then reverted):

Mutant Result
Remove the conflict-path adopt killed by the race test
Signal back to the full id killed by the differential and property tests
Projector writes under row.ConversationID killed
Projector publishes row.ConversationID killed
Signal fallback removed killed
Fallback ignores the platform killed
Adopted row rewritten killed
Unquotable-source_id check removed killed
Earlier-id reuse removed killed
Signal source_id guard removed killed
Lookup-first adopt removed survives: equivalent, because the guarded upsert writes nothing and the conflict path adopts

Verification

  • GOWORK=off go vet is clean for v2wire, signallive, web and cmd.
  • go test -count=1 passes for v2wire, signallive, bridgeadapters/..., web, tools, messaging, cutover and migration, and for cmd -run 'TestR5RollbackLegacyPrimarySendsIntoMigratedThreads|TestR5BackendIntegrationMigratedData'.
  • I did not run a full local ./... sweep, because the machine was short on disk. CI runs the full suite and the race job.

Residuals and behavior changes to know

  • Rollback replay. A legacy-primary daemon's projector replays the last 24h of confirmed outbox rows when it starts (projectorReplayWindow). After a rollback with V2_SEND=1, sends confirmed under v2-primary are now written into their legacy threads. Before, they were written under the hash id, which moved any WhatsApp or Signal echo the legacy store already had out of its thread. For Google, the projected row is keyed by the TmpID, so if the legacy store already received the permanent echo, a leftover "sending" row can appear. That is the TmpID race the projector already documents (projector.go, "does not persist a correlation from a permanent message ID back to TmpID").
  • Read cursors. A mirrored mark-read writes last_read_message_id = NULL (as it always has) when it advances a migrated cursor that had a message id. Nothing reads v2 cursors for unread state yet.
  • Live install (v2-primary). The mirror does not run there. The only change that reaches it is the Signal quote fallback: replies to Signal messages the legacy store holds now quote instead of failing. Replies to messages the legacy store lacks (for example ones sent through the v2 outbox on v2-primary, which the projector does not write back) still fail; that is filed as a follow-up.

🤖 Generated with Claude Code

…ing them

A legacy-primary daemon with OPENMESSAGES_V2_SEND=1 on a store built by
`openmessage migrate` (for example after rolling back from v2-primary)
could send only to threads created after the migration: the mirror refused
every thread the migration keyed by v2keys.DeriveID hash with
ErrConversationIdentityConflict (409).

The mirror now adopts the v2 row that holds the thread's natural key
(account_id, remote_conversation_id = legacy id) and returns its id without
writing it. It keys a row by the legacy id only for a thread v2 has never
seen, and adopts a row v2 ingest creates between its lookup and its upsert.

Two consumers needed fixing so adoption is safe:

- The legacy visibility projector wrote each confirmed send into the legacy
  store under the v2 conversation_id. The legacy upsert sets conversation_id
  on a message-id conflict, so a hash id would also move an already-ingested
  echo out of its thread. It now writes into the thread named by the v2
  conversation's remote_conversation_id and publishes that id.
- Signal reply targets. The mirror stored them under the full legacy id
  ("signal:<ts>"), while the migration and the v2 decoder store the bare
  timestamp, so a reply in a migrated thread would have added a second copy
  of the quoted message. The mirror now uses the bare id (and reuses copies
  it stored under the full id before). signallive signalQuoteArgs resolves a
  bare id by restoring the "signal:" prefix when it is not itself a legacy
  message id; every id that resolved before resolves identically. This also
  lets v2-native Signal replies (SubmitTextV2), which carry bare ids, quote
  messages the legacy store holds; before, every one failed with "signal
  reply target not found".

A Google reply target the migration keyed by a source_id other than its
message id (no live Google writer sets source_id) is refused with
ErrReplyTargetUnavailable rather than duplicated, since the transport quotes
the message id.

Tests: migrated-shape suites built with the real migration.Transform
(adoption without rewrites, sends and replies to migrated threads, a
differential check of reply remote ids against the migration, the projector
on an adopted thread with and without an ingested echo, the ingest race via
a test seam), a property test over random mirror call sequences on a
migrated store with ingest-keyed threads, a differential property test of
the Signal quote fallback against exact lookup, an HTTP mark-read test, and
an end-to-end rollback test in cmd that runs the real `openmessage migrate`,
a legacy-primary v2 stack and scripted adapters.

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