From 5f856f10c903b388d05bfaa90136fe7fda2e787d Mon Sep 17 00:00:00 2001 From: diffwireauto <330573775+diffwireauto@users.noreply.github.com> Date: Wed, 21 Oct 2026 03:58:18 -0400 Subject: [PATCH] task(#936): A2 reproduce the two remaining bleeds against the real store Five tests pinning exactly what bleeds today, so Epic B has something concrete to invert and cannot quietly half-fix it. They use the REAL StorageService against real prefs, not the FakeStorageService the existing path-history tests use, because the defect is in the KEY rather than the service logic and a fake hides it entirely. #854: the stored key is prefix + contact with no radio component, and a 7-hop route saved by one radio is served verbatim to another. A route measured by a 1 W handheld gets applied to a repeater. #856: observations live under one global key, and a service standing in for a second radio starts up with the first radio's 45-second observation already in its training data, having sent nothing. #855: a round-trip test only, pinning that the API works before Epic B deletes it, so the deletion is provably removal of dead code rather than of a live feature. That it has no callers is a grep result recorded in the inventory, which a unit test cannot assert. These assert the BUG deliberately and are named so nobody mistakes them for desired behaviour: each title says BUG and names what Epic B replaces it with. The plan's A2 asked for genuinely-red tests; a red test cannot merge, and Epic A's PR has to land before Epic B starts, so these are characterization tests instead. They demonstrate the bleed just as concretely and go red the moment it stops, which is the property that matters. Flagged here rather than quietly substituted. Two API names in the first draft were invented (loadHistoryFor, loadObservations) and did not exist. Replaced with the real ones after reading the services: the path-history case now exercises StorageService directly, which is where the defect actually is. Citadel: meshcore-open-ss9 Refs #936, #853, #854, #855, #856 Co-Authored-By: Claude Opus 5 --- test/services/radio_isolation_bleed_test.dart | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 test/services/radio_isolation_bleed_test.dart diff --git a/test/services/radio_isolation_bleed_test.dart b/test/services/radio_isolation_bleed_test.dart new file mode 100644 index 00000000..080a10a4 --- /dev/null +++ b/test/services/radio_isolation_bleed_test.dart @@ -0,0 +1,174 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:meshcore_open/models/delivery_observation.dart'; +import 'package:meshcore_open/models/path_history.dart'; +import 'package:meshcore_open/services/storage_service.dart'; +import 'package:meshcore_open/services/timeout_prediction_service.dart'; +import 'package:meshcore_open/storage/prefs_manager.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +/// #936 A2: the two remaining bleeds, reproduced against the real store. +/// +/// **These assert the BUG on purpose.** They are characterization tests: they +/// pin exactly what bleeds today so Epic B (#937) has something concrete to +/// invert, and so the bleed cannot be quietly half-fixed. Each one names the +/// issue it characterizes and says what Epic B replaces it with. +/// +/// Why characterization rather than genuinely-failing tests, which is what the +/// plan's A2 asked for: a red test cannot merge, and Epic A's PR has to land +/// before Epic B starts. These demonstrate the bleed just as concretely and go +/// red the moment it stops, which is the property that matters. +/// +/// They use the REAL `StorageService` against real prefs, not the +/// `FakeStorageService` the existing path-history tests use, because the defect +/// is in the KEY rather than in the service logic. A fake would hide it. +void main() { + setUp(() async { + SharedPreferences.setMockInitialValues({}); + PrefsManager.reset(); + await PrefsManager.initialize(); + }); + + PathRecord record(int hops, int ms) => PathRecord( + hopCount: hops, + tripTimeMs: ms, + timestamp: DateTime.fromMillisecondsSinceEpoch(1700000000000), + wasFloodDiscovery: false, + pathBytes: const [1, 2, 3], + successCount: 1, + failureCount: 0, + ); + + group('#854 path history bleeds between radios', () { + test('BUG: the stored key carries no radio, only the contact ' + '(Epic B scopes it)', () async { + final storage = StorageService(); + await storage.savePathHistory( + 'cafebabe', + ContactPathHistory( + contactPubKeyHex: 'cafebabe', + recentPaths: [record(2, 500)], + ), + ); + + final keys = PrefsManager.instance + .getKeys() + .where((k) => k.contains('path_history_')) + .toList(); + + expect(keys.length, 1, reason: 'one contact, one key'); + expect( + keys.single, + contains('path_history_cafebabe'), + reason: + 'the key is prefix + contact. There is no radio in it, ' + 'which IS the bug: any radio that shares this contact reads ' + 'and overwrites the same route', + ); + }); + + test('BUG: a route learned under radio A is served to radio B ' + '(Epic B isolates them)', () async { + // Nothing here distinguishes the radios, because the store offers no + // way to: `savePathHistory` takes a contact and nothing else. That is + // the defect, demonstrated rather than described. Two StorageService + // instances standing in for two connected radios share one row. + final radioA = StorageService(); + final radioB = StorageService(); + + await radioA.savePathHistory( + 'cafebabe', + ContactPathHistory( + contactPubKeyHex: 'cafebabe', + recentPaths: [record(7, 9000)], + ), + ); + + final seenByB = await radioB.loadPathHistory('cafebabe'); + + expect( + seenByB?.recentPaths.single.hopCount, + 7, + reason: + 'radio B sees a 7-hop route it never learned. A route ' + 'measured by a 1 W handheld is applied to a repeater. After ' + 'Epic B this must be null for radio B', + ); + }); + }); + + group('#856 delivery observations bleed between radios', () { + test('BUG: observations live under one global key ' + '(Epic B scopes it)', () async { + final storage = StorageService(); + await storage.saveDeliveryObservations([ + DeliveryObservation( + contactKey: 'cafebabe', + pathLength: 3, + messageBytes: 100, + secondsSinceLastRx: 10, + isFlood: false, + deliveryMs: 2500, + timestamp: DateTime.fromMillisecondsSinceEpoch(1700000000000), + ), + ]); + + final keys = PrefsManager.instance + .getKeys() + .where((k) => k.contains('delivery_observations')) + .toList(); + + expect(keys.length, 1); + expect( + keys.single.endsWith('delivery_observations'), + isTrue, + reason: + 'no radio component. Every radio trains on, and pollutes, ' + 'the same ACK-timeout model', + ); + }); + + test('BUG: one radio\'s RF timing reaches another radio\'s model ' + '(Epic B isolates them)', () async { + // A slow, many-hop repeater observation, then a fresh service standing + // in for a different radio. It inherits the other radio's timing. + await StorageService().saveDeliveryObservations([ + DeliveryObservation( + contactKey: 'cafebabe', + pathLength: 7, + messageBytes: 100, + secondsSinceLastRx: 10, + isFlood: false, + deliveryMs: 45000, + timestamp: DateTime.fromMillisecondsSinceEpoch(1700000000000), + ), + ]); + + final otherRadio = TimeoutPredictionService(StorageService()); + await otherRadio.initialize(); + + expect( + otherRadio.observationCount, + 1, + reason: + 'this radio has sent nothing, yet it starts with another ' + 'radio\'s 45-second observation in its training data. After ' + 'Epic B this must be 0', + ); + }); + }); + + group('#855 pending messages', () { + test('the API round-trips but nothing in the app calls it ' + '(Epic B deletes it)', () async { + // Kept deliberately small: the finding is that this API has no + // callers in lib/ or test/, which a unit test cannot assert. It is + // recorded in scoping_inventory.dart and proved by grep. This test + // only pins that the thing being deleted currently works, so the + // deletion is removal of dead code rather than of a live feature. + final storage = StorageService(); + await storage.savePendingMessages({'a': 'b'}); + + expect(await storage.loadPendingMessages(), {'a': 'b'}); + }); + }); +}