Don't delete conversations that arrive while a list sync is in flight - #6749
Draft
AndyScherzinger wants to merge 1 commit into
Draft
AndyScherzinger wants to merge 1 commit into
AndyScherzinger wants to merge 1 commit into
Conversation
A conversation list sync reads the locally known conversations after the server has answered, and treats every one the response does not mention as a conversation the user left. A conversation that reached the database while the request was in flight - a single-room fetch, a room joined on another device, one the user just created - was never in that response and could not have been, so it was deleted, taking its cached messages and chat blocks with it through the foreign key cascade. Take the known conversation ids before the request goes out and reconcile removals only against those. Anything that appeared afterwards is left alone until the next full sync, which is the first one whose response can speak about it at all. Reading the conversations themselves earlier would not do: that same read feeds the pending local state guard, which needs the freshest state to protect a favourite or read marker that is still on its way to the server. Only the removal reconcile wants the older snapshot, so only it gets one, as an id query that does not load the rows. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Contributor
📱 QA build
The QA build installs alongside a released Nextcloud app, so you can keep Downloading the file requires a GitHub account, so open this link on the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A conversation list sync reads the locally known conversations after the server has answered, and treats every one the response does not mention as a conversation the user left. A conversation that reached the database while the request was in flight — a single-room fetch, a room joined on another device, one the user just created — was never in that response and could not have been, so it was deleted, taking its cached messages and chat blocks with it through the foreign key cascade.
What it does
Takes the set of known conversation ids before the request goes out, and reconciles removals only against those. A conversation that appeared afterwards is left alone until the next full sync, which is the first one whose response could actually speak about it.
Why not simply read the conversations earlier
The same read feeds
preservePendingLocalState, which needs the freshest local state to protect an in-flight favourite or read marker. Moving it earlier would trade this bug for that one. Only the removal reconcile wants the older snapshot, so only it gets one — as an id-only query, so the extra read does not load every conversation row.Note for reviewers
detektis red on this branch. It is red onmastertoo, at the same count: 111 weighted issues against amaxIssuesof 110, measured onmasterwith this branch stashed. This change adds none.🚧 TODO
🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)