Krille/refactor-archive-room-handling#2217
Open
krille-chan wants to merge 2 commits into
Open
Conversation
krille-chan
force-pushed
the
krille/refactor-archive-room-handling
branch
from
January 2, 2026 14:32
7320e86 to
44c54ee
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2217 +/- ##
==========================================
- Coverage 59.61% 59.55% -0.07%
==========================================
Files 161 161
Lines 20308 20288 -20
==========================================
- Hits 12107 12082 -25
- Misses 8201 8206 +5
Continue to review full report in Codecov by Harness.
|
krille-chan
force-pushed
the
krille/refactor-archive-room-handling
branch
from
May 15, 2026 06:52
44c54ee to
129b959
Compare
krille-chan
force-pushed
the
krille/refactor-archive-room-handling
branch
2 times, most recently
from
June 19, 2026 13:24
44634d1 to
84eccf8
Compare
krille-chan
marked this pull request as ready for review
June 19, 2026 13:24
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
krille-chan
force-pushed
the
krille/refactor-archive-room-handling
branch
3 times, most recently
from
June 26, 2026 08:45
4f2d4cf to
8b3c7bf
Compare
krille-chan
force-pushed
the
krille/refactor-archive-room-handling
branch
from
July 6, 2026 06:33
8b3c7bf to
8bb6523
Compare
PR SummaryMedium Risk Overview
When
Reviewed by Cursor Bugbot for commit 590dd1b. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Leave removes room with includeLeave
- Left rooms are now retained in client.rooms with leave membership and prev_batch when includeLeave is enabled.
- ✅ Fixed: Archived getTimeline skips database
- getTimeline now reads persisted events from the database for archived rooms as well as active rooms.
- ✅ Fixed: Leave update skips DB membership
- storeRoomUpdate now updates existing room rows for leave updates, including membership, prev_batch, and lastEvent.
- ✅ Fixed: Re-invite skips membership update
- Existing rooms now update their in-memory and persisted membership when an invite update arrives.
Or push these changes by commenting:
@cursor push 20b0a05559
Preview (20b0a05559)
diff --git a/lib/src/client.dart b/lib/src/client.dart
--- a/lib/src/client.dart
+++ b/lib/src/client.dart
@@ -2983,12 +2983,26 @@
}
// If the membership is "leave" then remove the item and stop here
else if (membership == Membership.leave) {
- if (syncFilter.room?.includeLeave == true && !found) {
- rooms.add(room);
+ final leftUpdate = chatUpdate as LeftRoomUpdate;
+ if (syncFilter.room?.includeLeave == true) {
+ final membershipChanged = found && room.membership != membership;
+ room.membership = membership;
+ room.prev_batch = leftUpdate.timeline?.prevBatch ?? room.prev_batch;
+ if (!found) rooms.add(room);
+ if (membershipChanged) {
+ // ignore: deprecated_member_use_from_same_package
+ room.onUpdate.add(room.id);
+ }
} else if (found) {
rooms.removeAt(roomIndex);
}
}
+ // Update membership for existing invites.
+ else if (found && chatUpdate is InvitedRoomUpdate) {
+ rooms[roomIndex].membership = membership;
+ // ignore: deprecated_member_use_from_same_package
+ rooms[roomIndex].onUpdate.add(rooms[roomIndex].id);
+ }
// Update notification, highlight count and/or additional information
else if (found &&
chatUpdate is JoinedRoomUpdate &&
diff --git a/lib/src/database/matrix_sdk_database.dart b/lib/src/database/matrix_sdk_database.dart
--- a/lib/src/database/matrix_sdk_database.dart
+++ b/lib/src/database/matrix_sdk_database.dart
@@ -1305,8 +1305,10 @@
lastEvent: lastEvent,
).toJson(),
);
- } else if (roomUpdate is JoinedRoomUpdate) {
+ } else {
final currentRoom = Room.fromJson(copyMap(currentRawRoom), client);
+ final joinedUpdate = roomUpdate is JoinedRoomUpdate ? roomUpdate : null;
+ final leftUpdate = roomUpdate is LeftRoomUpdate ? roomUpdate : null;
await _roomsBox.put(
roomId,
Room(
@@ -1314,17 +1316,22 @@
id: roomId,
membership: membership,
highlightCount:
- roomUpdate.unreadNotifications?.highlightCount ??
+ joinedUpdate?.unreadNotifications?.highlightCount ??
currentRoom.highlightCount,
notificationCount:
- roomUpdate.unreadNotifications?.notificationCount ??
+ joinedUpdate?.unreadNotifications?.notificationCount ??
currentRoom.notificationCount,
- prev_batch: roomUpdate.timeline?.prevBatch ?? currentRoom.prev_batch,
- summary: RoomSummary.fromJson(
- currentRoom.summary.toJson()
- ..addAll(roomUpdate.summary?.toJson() ?? {}),
- ),
- lastEvent: lastEvent,
+ prev_batch:
+ joinedUpdate?.timeline?.prevBatch ??
+ leftUpdate?.timeline?.prevBatch ??
+ currentRoom.prev_batch,
+ summary: joinedUpdate == null
+ ? currentRoom.summary
+ : RoomSummary.fromJson(
+ currentRoom.summary.toJson()
+ ..addAll(joinedUpdate.summary?.toJson() ?? {}),
+ ),
+ lastEvent: lastEvent ?? currentRoom.lastEvent,
).toJson(),
);
}
diff --git a/lib/src/room.dart b/lib/src/room.dart
--- a/lib/src/room.dart
+++ b/lib/src/room.dart
@@ -1701,11 +1701,9 @@
var events = <Event>[];
- if (!isArchived) {
- await client.database.transaction(() async {
- events = await client.database.getEventList(this, limit: limit);
- });
- }
+ await client.database.transaction(() async {
+ events = await client.database.getEventList(this, limit: limit);
+ });
var chunk = TimelineChunk(events: events);
// Load the timeline arround eventContextId if set
diff --git a/test/fake_client.dart b/test/fake_client.dart
--- a/test/fake_client.dart
+++ b/test/fake_client.dart
@@ -20,6 +20,7 @@
Future<Client> getClient({
Duration sendTimelineEventTimeout = const Duration(minutes: 1),
String? databasePath,
+ Filter? syncFilter,
}) async {
try {
vodInit ??= vod.init(
@@ -37,6 +38,7 @@
database: await getDatabase(databasePath: databasePath),
onSoftLogout: (client) => client.refreshAccessToken(),
sendTimelineEventTimeout: sendTimelineEventTimeout,
+ syncFilter: syncFilter,
);
FakeMatrixApi.client = client;
await client.checkHomeserver(
diff --git a/test/room_archived_test.dart b/test/room_archived_test.dart
--- a/test/room_archived_test.dart
+++ b/test/room_archived_test.dart
@@ -73,6 +73,122 @@
expect(eventsFromStore.isEmpty, true);
});
+ test('includeLeave keeps left room in rooms and database', () async {
+ await client.dispose().onError((e, s) {});
+ client = await getClient(
+ syncFilter: Filter(
+ room: RoomFilter(
+ state: StateFilter(lazyLoadMembers: true),
+ includeLeave: true,
+ ),
+ ),
+ );
+ client.rooms.clear();
+ await client.database.clearCache();
+
+ const roomId = '!includeLeaveRoom:example.com';
+ await client.handleSync(
+ SyncUpdate(
+ nextBatch: 't_join',
+ rooms: RoomsUpdate(
+ join: {
+ roomId: JoinedRoomUpdate(
+ timeline: TimelineUpdate(
+ prevBatch: 'join_batch',
+ events: [
+ MatrixEvent(
+ type: EventTypes.Message,
+ senderId: '@alice:example.com',
+ eventId: '\$include-leave-join',
+ originServerTs: DateTime.fromMillisecondsSinceEpoch(1),
+ content: {'msgtype': 'm.text', 'body': 'joined'},
+ ),
+ ],
+ ),
+ ),
+ },
+ ),
+ ),
+ );
+
+ await client.handleSync(
+ SyncUpdate(
+ nextBatch: 't_leave',
+ rooms: RoomsUpdate(
+ leave: {
+ roomId: LeftRoomUpdate(
+ timeline: TimelineUpdate(
+ prevBatch: 'leave_batch',
+ events: [
+ MatrixEvent(
+ type: EventTypes.Message,
+ senderId: '@alice:example.com',
+ eventId: '\$include-leave-left',
+ originServerTs: DateTime.fromMillisecondsSinceEpoch(2),
+ content: {'msgtype': 'm.text', 'body': 'left'},
+ ),
+ ],
+ ),
+ ),
+ },
+ ),
+ ),
+ );
+
+ final room = client.getRoomById(roomId);
+ expect(room, isNotNull);
+ expect(room!.membership, Membership.leave);
+ expect(room.prev_batch, 'leave_batch');
+
+ final storedRoom = await client.database.getSingleRoom(client, roomId);
+ expect(storedRoom?.membership, Membership.leave);
+ expect(storedRoom?.prev_batch, 'leave_batch');
+ expect(storedRoom?.lastEvent?.eventId, '\$include-leave-left');
+
+ final timeline = await room.getTimeline();
+ expect(
+ timeline.events.map((event) => event.eventId),
+ contains('\$include-leave-left'),
+ );
+ });
+
+ test('reinvite updates kept left room membership', () async {
+ await client.dispose().onError((e, s) {});
+ client = await getClient(
+ syncFilter: Filter(
+ room: RoomFilter(
+ state: StateFilter(lazyLoadMembers: true),
+ includeLeave: true,
+ ),
+ ),
+ );
+ client.rooms.clear();
+ await client.database.clearCache();
+
+ const roomId = '!reinviteLeftRoom:example.com';
+ await client.handleSync(
+ SyncUpdate(
+ nextBatch: 't_join',
+ rooms: RoomsUpdate(join: {roomId: JoinedRoomUpdate()}),
+ ),
+ );
+ await client.handleSync(
+ SyncUpdate(
+ nextBatch: 't_leave',
+ rooms: RoomsUpdate(leave: {roomId: LeftRoomUpdate()}),
+ ),
+ );
+
+ await client.handleSync(
+ SyncUpdate(
+ nextBatch: 't_invite',
+ rooms: RoomsUpdate(invite: {roomId: InvitedRoomUpdate()}),
+ ),
+ );
+
+ expect(client.getRoomById(roomId)?.membership, Membership.invite);
+ });
+
test('discard room from archives when membership change', () async {
await client.loadArchiveWithTimeline();
await client.handleSync(You can send follow-ups to the cloud agent here.
krille-chan
force-pushed
the
krille/refactor-archive-room-handling
branch
from
July 13, 2026 08:50
8bb6523 to
590dd1b
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Archive timeline state ignored
- Archived room timeline state events are now applied to the room without storing them before last-event calculation.
- ✅ Fixed: Forget leaves room in list
- A successfully forgotten room is now removed from the in-memory client room list.
- ✅ Fixed: History mutates rooms list
- History backfill sync handling now skips room-list membership updates for non-joined rooms.
Or push these changes by commenting:
@cursor push 9299f8f186
Preview (9299f8f186)
diff --git a/lib/src/client.dart b/lib/src/client.dart
--- a/lib/src/client.dart
+++ b/lib/src/client.dart
@@ -1223,6 +1223,17 @@
<String, BasicEvent>{},
);
entry.value.state?.forEach(room.setState);
+ final timelineStateEvents = entry.value.timeline?.events
+ ?.where((event) => event.stateKey != null)
+ .toList();
+ if (timelineStateEvents != null && timelineStateEvents.isNotEmpty) {
+ await _handleRoomEvents(
+ room,
+ timelineStateEvents,
+ EventUpdateType.timeline,
+ store: false,
+ );
+ }
final timeline = Timeline(
room: room,
chunk: TimelineChunk(
@@ -2480,15 +2491,27 @@
}
/// Use this method only for testing utilities!
- Future<void> handleSync(SyncUpdate sync, {Direction? direction}) async {
+ Future<void> handleSync(
+ SyncUpdate sync, {
+ Direction? direction,
+ bool updateRoomList = true,
+ }) async {
// ensure we don't upload keys because someone forgot to set a key count
sync.deviceOneTimeKeysCount ??= {
'signed_curve25519': encryption?.olmManager.maxNumberOfOneTimeKeys ?? 100,
};
- await _handleSync(sync, direction: direction);
+ await _handleSync(
+ sync,
+ direction: direction,
+ updateRoomList: updateRoomList,
+ );
}
- Future<void> _handleSync(SyncUpdate sync, {Direction? direction}) async {
+ Future<void> _handleSync(
+ SyncUpdate sync, {
+ Direction? direction,
+ bool updateRoomList = true,
+ }) async {
final syncToDevice = sync.toDevice;
if (syncToDevice != null) {
await _handleToDeviceEvents(syncToDevice);
@@ -2497,7 +2520,11 @@
if (sync.rooms != null) {
final join = sync.rooms?.join;
if (join != null) {
- await _handleRooms(join, direction: direction);
+ await _handleRooms(
+ join,
+ direction: direction,
+ updateRoomList: updateRoomList,
+ );
}
// We need to handle leave before invite. If you decline an invite and
// then get another invite to the same room, Synapse will include the
@@ -2505,11 +2532,19 @@
// will only be included in leave.
final leave = sync.rooms?.leave;
if (leave != null) {
- await _handleRooms(leave, direction: direction);
+ await _handleRooms(
+ leave,
+ direction: direction,
+ updateRoomList: updateRoomList,
+ );
}
final invite = sync.rooms?.invite;
if (invite != null) {
- await _handleRooms(invite, direction: direction);
+ await _handleRooms(
+ invite,
+ direction: direction,
+ updateRoomList: updateRoomList,
+ );
}
}
for (final newPresence in sync.presence ?? <Presence>[]) {
@@ -2645,6 +2680,7 @@
Future<void> _handleRooms(
Map<String, SyncRoomUpdate> rooms, {
Direction? direction,
+ bool updateRoomList = true,
}) async {
var handledRooms = 0;
for (final entry in rooms.entries) {
@@ -2657,7 +2693,11 @@
final id = entry.key;
final syncRoomUpdate = entry.value;
- final room = await _updateRoomsByRoomUpdate(id, syncRoomUpdate);
+ final room = await _updateRoomsByRoomUpdate(
+ id,
+ syncRoomUpdate,
+ updateRoomList: updateRoomList,
+ );
// Is the timeline limited? Then all previous messages should be
// removed from the database!
@@ -2960,8 +3000,9 @@
Future<Room> _updateRoomsByRoomUpdate(
String roomId,
- SyncRoomUpdate chatUpdate,
- ) async {
+ SyncRoomUpdate chatUpdate, {
+ bool updateRoomList = true,
+ }) async {
// Update the chat list item.
// Search the room in the rooms
final roomIndex = rooms.indexWhere((r) => r.id == roomId);
@@ -2989,13 +3030,13 @@
: Room(id: roomId, membership: membership, client: this));
// Does the chat already exist in the list rooms?
- if (!found && membership != Membership.leave) {
+ if (updateRoomList && !found && membership != Membership.leave) {
final position = membership == Membership.invite ? 0 : rooms.length;
// Add the new chat to the list
rooms.insert(position, room);
}
// If the membership is "leave" then remove the item and stop here
- else if (membership == Membership.leave) {
+ else if (updateRoomList && membership == Membership.leave) {
if (syncFilter.room?.includeLeave == true && !found) {
rooms.add(room);
} else if (found) {
diff --git a/lib/src/room.dart b/lib/src/room.dart
--- a/lib/src/room.dart
+++ b/lib/src/room.dart
@@ -1424,6 +1424,7 @@
Future<void> forget() async {
await client.database.forgetRoom(id);
await client.forgetRoom(id);
+ client.rooms.removeWhere((room) => room.id == id);
return;
}
@@ -1548,6 +1549,7 @@
),
),
direction: direction,
+ updateRoomList: membership == Membership.join,
);
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 590dd1b. Configure here.
| (event) => roomPreviewLastEvents.contains(event.type), | ||
| ); | ||
|
|
||
| archivedRooms.add(ArchivedRoom(room: room, timeline: timeline)); |
There was a problem hiding this comment.
Archive timeline state ignored
Medium Severity
Issue: loadArchiveWithTimeline applies only entry.value.state via setState and builds the timeline separately. Unlike the removed _storeArchivedRoom, it no longer runs timeline events through _handleRoomEvents, so state changes present only in the timeline are not applied to the room.
Fix: Also apply timeline state events to the room (e.g. reuse _handleRoomEvents with store: false) before computing lastEvent.
Reviewed by Cursor Bugbot for commit 590dd1b. Configure here.
| if (syncFilter.room?.includeLeave == true && !found) { | ||
| rooms.add(room); | ||
| } else if (found) { | ||
| rooms.removeAt(roomIndex); |
There was a problem hiding this comment.
History mutates rooms list
Medium Severity
Issue: Room.requestHistory for a leave room synthesizes a LeftRoomUpdate through handleSync. With includeLeave, that update adds an ephemeral loadArchiveWithTimeline room into client.rooms when missing, or removes an existing leave entry when present.
Fix: Do not treat history backfill LeftRoomUpdates like sync membership changes when deciding to add/remove leave rooms under includeLeave.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 590dd1b. Configure here.
| Future<void> forget() async { | ||
| await client.database.forgetRoom(id); | ||
| await client.forgetRoom(id); | ||
| // Update archived rooms, otherwise an archived room may still be in the |
There was a problem hiding this comment.
Forget leaves room in list
Medium Severity
Issue: forget() clears the DB and calls the forget API, but no longer updates any client-side room list. With includeLeave, left rooms live in client.rooms, so a forgotten room can remain visible until restart.
Fix: After a successful forget, remove the room from client.rooms when present.
Reviewed by Cursor Bugbot for commit 590dd1b. Configure here.
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.
closes #2031
Test out with:
If you don't change the syncFilter, the behavior should be as before, just that you now have to cache the archive by yourself when requesting it.