From 46255f1ef400450da7dcdb7541772d41cbdcc6fb Mon Sep 17 00:00:00 2001 From: diffwireauto <330573775+diffwireauto@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:18:35 -0500 Subject: [PATCH] fix(#1063): keep per-slot message state from crossing channel identities Persisted channel history has been PSK-keyed since #194; the remnants that could still cross-wire on a slot reassignment were in-memory and legacy. Epic B of #1061, stacked on #1062: - _completeChannelSync's identity diff now drops the slot's in-memory conversation and reloads it from the store, which reads the NEW identity's own history through the (now fail-closed) resolver. - setChannel's reassignment clear purges only the LEGACY slot-index keys (new clearLegacySlotHistory), so the outgoing occupant's PSK-keyed history survives and comes back if the channel is re-added. True deletes (deleteChannel) keep the full both-key clear. - A WRITE that falls back to the slot-index key now logs an error with the orphaning consequence spelled out; post-#1062 live ingress cannot reach it. - The #193-era comment claiming history is slot-keyed is corrected and the setChannel/resolver interplay documented. Tests: 2 connector tests (in-memory drop on identity change, kept on same identity) + 5 store tests (legacy purge, PSK history survival, no adoption onto the new occupant, true delete). Issue: #1063 Co-Authored-By: Claude Fable 5 --- lib/connector/meshcore_connector.dart | 25 +++- lib/storage/channel_message_store.dart | 79 ++++++++--- test/connector/channel_sync_truth_test.dart | 61 +++++++++ .../channel_message_store_identity_test.dart | 128 ++++++++++++++++++ 4 files changed, 267 insertions(+), 26 deletions(-) create mode 100644 test/storage/channel_message_store_identity_test.dart diff --git a/lib/connector/meshcore_connector.dart b/lib/connector/meshcore_connector.dart index 0341a480..7f554189 100644 --- a/lib/connector/meshcore_connector.dart +++ b/lib/connector/meshcore_connector.dart @@ -6414,6 +6414,13 @@ class MeshCoreConnector extends ChangeNotifier { '"${prior.name}" -> "${fresh.name}"', tag: 'ChannelSync', ); + // #1063: the in-memory conversation is the one per-slot message store + // left (persisted history is PSK-keyed, #194). Drop the old occupant's + // messages so they cannot render under the new channel, then reload + // this slot from the store; the resolver now sees the fresh list, so + // the load reads the NEW identity's own history. + _channelMessages.remove(fresh.index); + unawaited(_loadChannelMessages(fresh.index)); } } @@ -6574,19 +6581,23 @@ class MeshCoreConnector extends ChangeNotifier { Future setChannel(int index, String name, Uint8List psk) async { if (!isConnected) return; - // Channel history is stored keyed by the reusable slot [index], not by the - // channel's PSK. If this slot is being (re)assigned to a different channel - // than the one whose history is stored here, clear the stale history so a - // previous occupant's messages (e.g. #echo) don't surface under the newly - // added channel. Preserve only when the slot already holds the SAME channel - // (same PSK), e.g. a rename. (#193) + // Persisted channel history is keyed by the channel's PSK identity (#194), + // so reassigning this slot to a different channel cannot surface the old + // occupant's messages THERE. Two slot-keyed remnants still can, and both + // are handled when the slot changes occupant (different PSK): + // - legacy slot-index keys (pre-#194) would be adopted onto the new + // occupant by the store's one-time migration; purge them (#193/#1063). + // - the in-memory conversation map is slot-keyed; drop this slot's entry. + // The old occupant's PSK-keyed history is deliberately left intact: if + // that channel is ever re-added (any slot), its history comes back + // (#1063). A rename (same PSK) keeps everything. final existing = _findChannelByIndex(index); final sameChannel = existing != null && listEquals(existing.psk, psk); await sendFrame(buildSetChannelFrame(index, name, psk)); if (!sameChannel) { - await _channelMessageStore.clearChannelMessages(index); + await _channelMessageStore.clearLegacySlotHistory(index); _channelMessages.remove(index); } diff --git a/lib/storage/channel_message_store.dart b/lib/storage/channel_message_store.dart index 9c5bd445..adc5b5be 100644 --- a/lib/storage/channel_message_store.dart +++ b/lib/storage/channel_message_store.dart @@ -126,23 +126,45 @@ class ChannelMessageStore { /// Active storage key: PSK identity when known, else the slot index. /// - /// The index fallback is legitimate before a channel list has loaded, but it - /// reads a DIFFERENT key: if history was already migrated to the PSK key, the - /// caller gets an empty list that is indistinguishable from data loss. - /// SAFELANE 6 - this must never be silent. Falling back where a resolver - /// exists means the channel list was not ready, which is the #333 race. - String _storageKey(int channelIndex) { + /// The index fallback is legitimate on a READ before a channel list has + /// loaded, but it reads a DIFFERENT key: if history was already migrated to + /// the PSK key, the caller gets an empty list that is indistinguishable from + /// data loss. SAFELANE 6 - this must never be silent. Falling back where a + /// resolver exists means the channel list was not ready, which is the #333 + /// race. + /// + /// A WRITE must never land on the index fallback: the one-time adoption in + /// [loadChannelMessages] only runs while the PSK key is EMPTY, so a message + /// written to the index key of a channel that already has PSK-keyed history + /// is orphaned - stored, never read again. Since #1062 the connector files + /// live traffic only after the channel list is confirmed (ingress barrier + + /// fail-closed slot lookup), so a write-side fallback is a defect; it is + /// logged as an error and [forWrite] marks the call sites that save, remove + /// or clear. (#1063) + String _storageKey(int channelIndex, {bool forWrite = false}) { final pskHex = channelPskResolver?.call(channelIndex); if (pskHex != null && pskHex.isNotEmpty) { return '$keyFor$_pskMarker$pskHex'; } if (channelPskResolver != null) { - appLogger.warn( - 'Channel $channelIndex has no PSK yet; falling back to the slot-index ' - 'key. Any history already migrated to the PSK key will read as EMPTY. ' - 'This means the channel list was not loaded first (#333).', - tag: 'Storage', - ); + if (forWrite) { + appLogger.error( + 'WRITE for channel $channelIndex fell back to the slot-index key: ' + 'the slot did not resolve to a PSK. If this channel already has ' + 'PSK-keyed history, this write is orphaned (adoption only runs on ' + 'an empty PSK key). Post-#1062 ingress cannot reach this; a user ' + 'action on an unconfirmed slot can. (#1063)', + tag: 'Storage', + ); + } else { + appLogger.warn( + 'Channel $channelIndex has no PSK yet; falling back to the ' + 'slot-index key. Any history already migrated to the PSK key will ' + 'read as EMPTY. This means the channel list was not loaded first ' + '(#333).', + tag: 'Storage', + ); + } } return _indexKey(channelIndex); } @@ -165,7 +187,7 @@ class ChannelMessageStore { // the in-memory copy win so edits/reactions/status updates are captured. // Deletion has its own path (removeChannelMessage) so this never // resurrects a message the user deleted. - final key = _storageKey(channelIndex); + final key = _storageKey(channelIndex, forWrite: true); final blobs = BlobStore.instance; // Rows (#920): one upsert per message in the list, which costs the same @@ -230,7 +252,7 @@ class ChannelMessageStore { ChannelMessage message, ) async { if (publicKeyHex.isEmpty) return; - final key = _storageKey(channelIndex); + final key = _storageKey(channelIndex, forWrite: true); final blobs = BlobStore.instance; if (await blobs.ensureMessageRowsMigrated(key, channelHistoryToRows)) { await blobs.deleteMessageRow(key, _mergeKey(message)); @@ -351,19 +373,38 @@ class ChannelMessageStore { return out; } - /// Clear messages for a specific channel slot. Removes BOTH the PSK-identity - /// key and the raw slot-index key: deleteChannel needs the channel's own - /// history gone, and setChannel's reuse-clear (#193) needs any stale - /// slot-index history gone so it can't be migrated onto the new occupant. + /// Clear a channel's history outright: BOTH the PSK-identity key and the + /// raw slot-index key. This is the true-delete path (deleteChannel, the + /// user's explicit clear) - the channel's own history must be gone. + /// Slot reassignment uses [clearLegacySlotHistory] instead, which keeps the + /// outgoing occupant's PSK-keyed history. (#1063) Future clearChannelMessages(int channelIndex) async { final blobs = BlobStore.instance; - for (final key in {_storageKey(channelIndex), _indexKey(channelIndex)}) { + for (final key in { + _storageKey(channelIndex, forWrite: true), + _indexKey(channelIndex), + }) { await blobs.deleteEverywhere(key); // Rows as well, or a clear would leave the history readable (#920). await blobs.deleteMessageRows(key); } } + /// Purges only the LEGACY slot-keyed history for [channelIndex]: the + /// device-scoped index key and the pre-device-scoping unscoped key. Used + /// when a slot is reassigned to a different channel (#193): without this, + /// [loadChannelMessages]' one-time adoption would migrate the previous + /// occupant's pre-#194 history onto the new occupant. The previous + /// occupant's PSK-keyed history is NOT touched, so re-adding that channel + /// later restores its messages. (#1063) + Future clearLegacySlotHistory(int channelIndex) async { + final blobs = BlobStore.instance; + for (final key in {_indexKey(channelIndex), '$_keyPrefix$channelIndex'}) { + await blobs.deleteEverywhere(key); + await blobs.deleteMessageRows(key); + } + } + /// Clear all channel messages Future clearAllChannelMessages() async { final blobs = BlobStore.instance; diff --git a/test/connector/channel_sync_truth_test.dart b/test/connector/channel_sync_truth_test.dart index 83371cee..ae37b3b8 100644 --- a/test/connector/channel_sync_truth_test.dart +++ b/test/connector/channel_sync_truth_test.dart @@ -13,6 +13,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:meshcore_open/connector/meshcore_connector.dart'; import 'package:meshcore_open/connector/meshcore_protocol.dart'; import 'package:meshcore_open/models/channel.dart'; +import 'package:meshcore_open/models/channel_message.dart'; import 'package:meshcore_open/storage/prefs_manager.dart'; import 'package:shared_preferences/shared_preferences.dart'; @@ -174,6 +175,66 @@ void main() { }, ); + test( + 'in-memory messages do not follow a slot across an identity change (#1063)', + () async { + final c = _connector(); + c.seedCachedChannelsForTest([_channel(0, 'OldChannel', 70)]); + c.channelMessagesForTest[0] = [ + ChannelMessage( + senderName: 'x', + text: 'old occupant message', + timestamp: DateTime.fromMillisecondsSinceEpoch(1000), + isOutgoing: false, + channelIndex: 0, + messageId: 'old1', + ), + ]; + + await c.getChannels(maxChannels: 1); + c.handleFrameForTest( + _channelInfoFrame(index: 0, name: 'NewChannel', pskSeed: 10), + ); + // Settle the sync completion and the unawaited history reload. + await Future.delayed(Duration.zero); + await Future.delayed(Duration.zero); + + expect( + c.channelMessagesForTest[0] ?? const [], + isEmpty, + reason: + "the old occupant's conversation must not render under the new " + 'channel; the slot reloads from the NEW identity\'s (empty) store', + ); + }, + ); + + test( + 'in-memory messages stay when the slot keeps the same channel', + () async { + final c = _connector(); + c.seedCachedChannelsForTest([_channel(0, 'Keep', 10)]); + c.channelMessagesForTest[0] = [ + ChannelMessage( + senderName: 'x', + text: 'kept', + timestamp: DateTime.fromMillisecondsSinceEpoch(1000), + isOutgoing: false, + channelIndex: 0, + messageId: 'k1', + ), + ]; + + await c.getChannels(maxChannels: 1); + c.handleFrameForTest( + _channelInfoFrame(index: 0, name: 'Keep', pskSeed: 10), + ); + await Future.delayed(Duration.zero); + + expect(c.channelMessagesForTest[0]!.single.text, 'kept'); + }, + ); + test( 'a sync with unconfirmed slots does not overwrite the offline cache', () async { diff --git a/test/storage/channel_message_store_identity_test.dart b/test/storage/channel_message_store_identity_test.dart new file mode 100644 index 00000000..b2f8e0be --- /dev/null +++ b/test/storage/channel_message_store_identity_test.dart @@ -0,0 +1,128 @@ +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/models/channel_message.dart'; +import 'package:meshcore_open/storage/channel_message_store.dart'; +import 'package:meshcore_open/storage/drift/blob_store.dart'; +import 'package:meshcore_open/storage/drift/offband_database.dart'; +import 'package:meshcore_open/storage/prefs_manager.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +/// #1063 (Epic B of #1061): slot reassignment must not move history between +/// channel identities. Persisted history is PSK-keyed (#194); the remnants a +/// reassignment has to deal with are the LEGACY slot-index keys, and only +/// those - the outgoing occupant's PSK-keyed history survives so re-adding +/// that channel brings its messages back. +void main() { + late OffbandDatabase db; + late BlobStore blobs; + + const pskA = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; + const pskB = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; + + setUp(() async { + SharedPreferences.setMockInitialValues({}); + PrefsManager.reset(); + await PrefsManager.initialize(); + db = OffbandDatabase(NativeDatabase.memory()); + blobs = BlobStore(db); + BlobStore.overrideForTest(blobs); + }); + + tearDown(() async { + await db.close(); + BlobStore.clearTestOverride(); + }); + + ChannelMessage chan(int i, {String? text}) => ChannelMessage( + senderName: 's', + text: text ?? 'm$i', + timestamp: DateTime.fromMillisecondsSinceEpoch(1000 + i), + isOutgoing: false, + channelIndex: 0, + messageId: 'id$i', + ); + + ChannelMessageStore store({String? psk}) => ChannelMessageStore() + ..setPublicKeyHex = 'd' * 20 + ..channelPskResolver = (_) => psk; + + test('clearLegacySlotHistory purges the legacy slot keys', () async { + // Pre-#194 remnants: device-scoped index key and the unscoped one. + await blobs.write( + 'channel_messages_dddddddddd0', + ChannelMessageStore.encodeHistoryForTest([chan(1)]), + ); + await blobs.write( + 'channel_messages_0', + ChannelMessageStore.encodeHistoryForTest([chan(2)]), + ); + + await store(psk: pskA).clearLegacySlotHistory(0); + + expect( + await blobs.readWithPrefsFallback('channel_messages_dddddddddd0'), + isNull, + ); + expect(await blobs.readWithPrefsFallback('channel_messages_0'), isNull); + }); + + test('clearLegacySlotHistory keeps the PSK-keyed history', () async { + final s = store(psk: pskA); + await s.saveChannelMessages(0, [chan(1), chan(2)]); + + await s.clearLegacySlotHistory(0); + + final loaded = await s.loadChannelMessages(0); + expect(loaded.map((m) => m.messageId), ['id1', 'id2']); + }); + + test('after a reassignment clear, the new occupant does not adopt the old ' + 'occupant\'s legacy slot history', () async { + // Old occupant A left pre-#194 history under the slot-index key. + await blobs.write( + 'channel_messages_dddddddddd0', + ChannelMessageStore.encodeHistoryForTest([chan(1, text: 'old-A')]), + ); + + // Slot 0 is reassigned to channel B: the connector purges the legacy + // slot keys (setChannel, #1063). + await store(psk: pskB).clearLegacySlotHistory(0); + + // Channel B's first load must not inherit A's messages via the one-time + // slot-index adoption. + expect(await store(psk: pskB).loadChannelMessages(0), isEmpty); + }); + + test('the outgoing occupant\'s history survives a reassignment and comes ' + 'back when the channel is re-added', () async { + // Channel A lives in slot 0 with PSK-keyed history. + final asA = store(psk: pskA); + await asA.saveChannelMessages(0, [chan(1, text: 'kept')]); + + // Slot 0 is reassigned to channel B. + await store(psk: pskB).clearLegacySlotHistory(0); + await store(psk: pskB).saveChannelMessages(0, [chan(9, text: 'b-msg')]); + + // Channel A is re-added (any slot; identity is the PSK, not the slot). + final loaded = await store(psk: pskA).loadChannelMessages(5); + expect(loaded.single.text, 'kept'); + }); + + test('clearChannelMessages (true delete) removes the PSK-keyed history ' + 'and the legacy index key', () async { + final s = store(psk: pskA); + await s.saveChannelMessages(0, [chan(1)]); + await blobs.write( + 'channel_messages_dddddddddd0', + ChannelMessageStore.encodeHistoryForTest([chan(2)]), + ); + + await s.clearChannelMessages(0); + + expect(await s.loadChannelMessages(0), isEmpty); + expect( + await blobs.readWithPrefsFallback('channel_messages_dddddddddd0'), + isNull, + ); + }); +}