fix(import): the later backup decides a message's mark and text, unsent part included (#1741, #1804)

[mbeisser1]

Bug Description

Expected: Between two copies of one message from one source, the copy from the later backup decides the message's mark (Deleted in the source app or Unsent, mark or no mark) and its text, parts, earlier versions and unsent marks, across imports and within one import, in any file order. A file without a backup date keeps the rules for files without one: a mark adds and is never cleared, and the copy whose newest earlier version is newer gives the text.

Actual (before #1924): A mark was only ever added, never cleared, and within one import the first file staged decided the mark, because staging_messages is unique on (account_id, source, guid) and the insert is ON CONFLICT DO NOTHING (#1741). An append took a later edit only when its newest earlier version was newer, so a later backup whose only change was an unsent part, or one edit after an unsend, kept the older text (#1804).

Root Cause

The conversation file did not say when its backup was made, so the import could not tell which copy was newer and used rules of thumb: marks add, and version times compare.

Fix Description

#1924 (commit 7a8435e, PR #1966) already built both decisions. It added export.backup_taken_at_unix_ms to the conversation file, staging_messages.backup_taken_at and messages.backup_taken_at, and the one rule later_backup_sql / later_backup in crates/server/server/src/db/staging.rs. promote_deletion_marks lets the later backup set or clear the mark across imports; write_edit_map takes the later backup's text and versions whatever their times say, and keeps later_edit_sql only as the fallback for undated (or equally dated) copies; add_staged_copy in imports_api/staging.rs handles the second copy that ON CONFLICT DO NOTHING skips, so the second file of one import gives its mark and text. docs/architecture/contacts-identities-and-messages.md, the common-message page and the user import page say the rule and that a file without a date keeps marks additive. Its tests in imports_api/tests/backup_dates.rs cover the #1741 criteria in every order (together, together reversed, apart, apart reversed), the swapped dates, the undated fallback, and the #1804 text case.

What was left, and what this PR adds:

• A test of the An append keeps the older text when a later backup only unsent a part #1804 scenario as the issue gives it. The The conversation file says when the backup was made, so an append knows which backup is newer #1924 test ran that scenario without B's unsent part (B carried no mark). The new test, the_later_backup_decides_an_unsent_part_whatever_the_version_times_say, gives B the Unsent mark its unsent part brings and checks all four orders, both orders of a single two-file import included: B later gives B's text, part 0 version and Unsent mark, and search finds neither copy's text; dates swapped gives A's text and part 1 versions with no mark, and search finds A's text; no dates gives A's text by the version times, as fix(server): an append from a later backup takes a stored message's later edit #1801 left them, with B's mark added.
• Two CHANGELOG entries under 0.11.0 Fixes, Importing, one per issue. The The conversation file says when the backup was made, so an append knows which backup is newer #1924 Features entry already describes the behaviour; these say what was wrong before.

No code change was needed.

How to Reproduce (Before Fix)

1. Check out the commit before 7a8435e (or disable the date comparison in later_backup_sql and later_backup).
2. Run cargo test -p message-crate-server --lib backup_dates::the_later_backup_decides_an_unsent_part.
3. It fails on the first order: the message holds A's text and versions and the earlier backup's date.

How to Verify (After Fix)

1. cargo test -p message-crate-server --lib backup_dates (10 tests pass).
2. With the date comparison disabled (AND 0 in later_backup_sql, && false in both later_backup arms), the new test fails with together: left: Held { text: "zqharbor zqyonder", … } right: Held { text: "zqbeacon", … }. Checked locally, then reverted.
3. ./scripts/check-pr.sh and cargo test -p message-crate-server pass.

Impact Assessment

• Severity: Medium
• Users affected: Subset: people who import more than one backup of the same phone.
• Duration: From the import of marks (Deleted in the source app and Unsent: record the mark, filled by Apple Messages #1643, Deleted in the source app and Unsent: record the mark, filled by Apple Messages #1740) and later edits (fix(server): an append from a later backup takes a stored message's later edit #1801) until The conversation file says when the backup was made, so an append knows which backup is newer #1924.

Regression Risk

None in behaviour: this PR adds one test and two CHANGELOG entries.

Checklist

• Root cause identified and documented above
• Fix addresses the root cause (not just symptoms)
• Added test to prevent regression
• Existing tests pass locally
• Tested the specific reproduction steps

Related Issues

Closes #1741
Closes #1804

Built on #1924 (PR #1966).

🤖 Generated with Claude Code

[mbeisser1]

Review of 42be2b2 (the PR head, up to date with main). Standards 6, Spec 3, Correctness 2. Three findings with no line in the diff follow in a top-level comment. Each is answered in its thread.

[mbeisser1]

Findings of the review of 42be2b2 with no line in the diff. Each is answered by a later marked comment quoting it.

1. Spec · judgement (crates/server/server/src/imports_api/tests/backup_dates.rs, the_later_backup_decides_the_text_whatever_the_version_times_say, outside the diff). The new test now does everything this one does, plus the mark, yet this one's doc comment still calls itself "The scenario of An append keeps the older text when a later backup only unsent a part #1804". Fix: delete it, or retitle it as the case without a mark.

2. Spec · judgement (crates/server/server/src/db/staging.rs, the doc comment on later_edit_sql, on main already). It still says "The one rule for which of two copies of a message is the later backup", which contradicts later_backup_sql. Fix: say it is the fallback for undated copies, here or in a follow-up issue.

3. Correctness · bug (crates/server/server/src/db/staging.rs, add_staged_copy / take_later_staged_copy / add_staged_copy_mark, on main already). When one import holds a dated file and an undated one, the stored backup date depends on which file comes first, and a later import then gives different results. Import 1 holds A (dated EARLIER, no mark) and B (no date, Unsent). Order A,B: the staged row keeps EARLIER and adds B's mark. Order B,A: the staged row keeps NULL, because later_backup returns Undecided and neither take_later_staged_copy nor add_staged_copy_mark sets backup_taken_at. Import 2 is C (dated LATER, no mark): after A,B the mark is cleared; after B,A it stays unsent, because promote_deletion_marks falls back to marks-add. The module comment and add_staged_copy's doc say one import stores the same in either file order (One import of two backups holding a new message stores both rows' earlier versions #1806, One import of two backups drops the attachments and reactions of the copy staged second #1837); that holds for the mark and text but not for the date that decides later imports. Fix: in the Undecided case have the staged row keep a date when either copy has one, with a test; or decide on purpose that a mixed import stores no date and test that.

[mbeisser1]

Re-review of the fix commits (Standards 3, Correctness 1), posted after the push at d92d4b2. Each is answered in its thread.

All four findings are about commit 6c96678, which 6562383 reverts, so their lines are no longer in the diff and each is listed and answered here.

1. Standards · rule (docs/architecture/, the backup date rule). The architecture doc's "backup_taken_at keeps the date of the backup that decided the stored copy" no longer matched 6c96678: a mixed import stored the dated copy's date beside the undated copy's mark.
   Answered by the revert 6562383: the attempted fix is withdrawn and the order-dependence is Deferred to #1989.

2. Standards · judgement (crates/server/server/src/imports_api/staging.rs). 6c96678 pushed a doc comment line to about 115 characters.
   Answered by the revert 6562383: the attempted fix is withdrawn and the order-dependence is Deferred to #1989.

3. Standards · judgement (crates/server/server/src/imports_api/tests/backup_dates.rs). The test 6c96678 added copied the order names instead of using every_order, and left out the two "apart" orders.
   Answered by the revert 6562383: the attempted fix is withdrawn and the order-dependence is Deferred to #1989.

4. Correctness · bug (crates/server/server/src/db/staging.rs, with 6c96678). With a stored message from a LATER backup, one import of an undated Unsent copy plus an EARLIER dated copy got the date EARLIER, and the Unsent mark was dropped at promotion, where two separate imports of the same files keep the message Unsent.
   Answered by the revert 6562383: the attempted fix is withdrawn and the order-dependence is Deferred to #1989.

[mbeisser1]

Answers to the findings in #1983 (comment).

1. Spec · judgement … The new test now does everything this one does, plus the mark, yet this one's doc comment still calls itself "The scenario of An append keeps the older text when a later backup only unsent a part #1804".

Fixed in 28350fd: the two tests are now one. the_later_backup_decides_the_text_whatever_the_version_times_say gives backup B the Unsent mark and is the only test of the #1804 scenario.

2. Spec · judgement … later_edit_sql … still says "The one rule for which of two copies of a message is the later backup", which contradicts later_backup_sql.

Fixed in 75218e2: the doc comment on later_edit_sql now says it is the fallback for copies that are undated or have equal dates, and later_backup_sql is the rule.

3. Correctness · bug … When one import holds a dated file and an undated one, the stored backup date depends on which file comes first, and a later import then gives different results.

Deferred: #1989. A fix was tried (6c96678) and reverted in 6562383: giving the staged row the dated copy's date lets that date decide for the undated copy's mark and text at promotion, which drops a mark separate imports keep. The issue records both scenarios.

[mbeisser1]

Review summary for 42be2b2, pushed as d92d4b2 (the PR with main merged in twice, conflict-free).

• Standards: 6 findings (two duplicate CHANGELOG bullets and two sentences about files the reader cannot make; a helper's place, copied order names, a near-copy test, a doc comment). 6 Fixed.
• Spec (issues A message's deletion mark is never cleared by a later import, and the first file staged decides it #1741 and An append keeps the older text when a later backup only unsent a part #1804): 3 findings (the old test's doc comment, the later_edit_sql doc on main, a search index never checked). 3 Fixed.
• Correctness: 2 findings (the search assertions passing without the fix; a mixed dated/undated import storing a different backup date by file order). 1 Fixed, 1 Deferred to A mixed import of a dated and an undated file stores a different backup date by file order #1989 after a fix was tried (6c96678) and reverted (6562383) because it dropped a mark separate imports keep.
• Re-review of the fixes: 4 findings, all on the reverted commit, answered by the revert.

Every acceptance box of #1741 and #1804 was verified against the tests from #1966 plus the one merged here. No commit was needed for CI and no user thread is open. CI run 37727199790 on d92d4b2: success.