feat(#89): verify broker enable/disable actually applied before confirming

A firmware build can ACK a broker enable/disable as success without applying it
(HV4; meshcore-firmware#179). The quick toggle and the editor save previously
trusted the ACK and showed a success snackbar, so an ack-but-no-op looked like
it worked.

Both paths now SET, wait a short settle, re-read the slot, and report what the
device ACTUALLY did:
- applied   -> confirm
- notApplied -> warn "possible firmware issue"; the editor stays and re-seeds to
  the device's true state instead of popping a false success
- unverified -> re-read failed (e.g. an enable that rebooted the device)

Every device is treated identically: the UI reflects the re-read, never assuming
a change took that the device didn't confirm.

- BrokerConfig.classifyApply pure classifier + BrokerApplyOutcome (unit-tested)
- ObserverConfigService.applySettleDelay
- wired into mqtt_brokers_screen quick toggle + broker_editor_screen save
- widget test for the ACK-but-not-applied path

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

lib/models/observer_config.dart

@@ -100,6 +100,23 @@ class SecretSet extends SecretField {
   final String value;
 }
 
+/// Outcome of verifying that a broker enable/disable actually took effect, by
+/// re-reading the slot after the SET (#89). A firmware build can ACK a toggle as
+/// success without applying it (meshcore-firmware#179), so the UI never trusts
+/// the ACK alone — it re-reads and reports what the device actually says.
+enum BrokerApplyOutcome {
+  /// Re-read confirms the device is in the intended enabled state.
+  applied,
+
+  /// The device ACKed success but the re-read shows the unchanged state — it did
+  /// not apply the toggle. Surfaced as a warning, never a false success.
+  notApplied,
+
+  /// The re-read itself failed (timeout/disconnect — e.g. an enable that rebooted
+  /// the device), so the result can't be confirmed either way.
+  unverified,
+}
+
 /// One MQTT broker slot (0-9), as read back from the `OCFG_BROKERS` dump.
 /// Non-secret fields only — `password` is reported as presence ([passwordSet]),
 /// `jwt_token` is never a config key.
@@ -155,6 +172,20 @@ class BrokerConfig {
     return null;
   }
 
+  /// Classify whether a toggle/save took, comparing the [intendedEnabled] state
+  /// to a re-read [actualEnabled] (null = the re-read failed). Pure and
+  /// device-agnostic: it reports only what the device reported back, never
+  /// assuming a change took that the device didn't confirm (#89).
+  static BrokerApplyOutcome classifyApply({
+    required bool intendedEnabled,
+    required bool? actualEnabled,
+  }) {
+    if (actualEnabled == null) return BrokerApplyOutcome.unverified;
+    return actualEnabled == intendedEnabled
+        ? BrokerApplyOutcome.applied
+        : BrokerApplyOutcome.notApplied;
+  }
+
   /// Build from one slot's decoded `key=value` lines (the OCFG_BROKER_KV bodies).
   /// Unknown keys are ignored; missing keys keep the defaults.
   factory BrokerConfig.fromWireFields(int slot, Map<String, String> kv) {

lib/screens/settings/broker_editor_screen.dart

@@ -140,6 +140,7 @@ class _BrokerEditorScreenState extends State<BrokerEditorScreen> {
         return;
       }
     }
+    final intended = _enabled;
     final svc = context.read<ObserverConfigService>();
     final messenger = ScaffoldMessenger.of(context);
     final navigator = Navigator.of(context);
@@ -148,26 +149,63 @@ class _BrokerEditorScreenState extends State<BrokerEditorScreen> {
     final result = await svc.saveBroker(
       _baseline.slot,
       fields: _changedFields(),
-      enable: _enabled,
+      enable: intended,
       wasLive: _baseline.enabled,
     );
     if (!mounted) return;
+    if (!result.ok) {
       setState(() => _busy = false);
-    if (result.ok) {
+      messenger.showSnackBar(
+        SnackBar(
+          content: Text(
+            'Save failed at "${result.failedField ?? 'a field'}" — the slot '
+            'was left disabled. Re-read and retry.',
+          ),
+          backgroundColor: errorColor,
+        ),
+      );
+      return;
+    }
+    // The writes were ACKed — but a firmware build can ACK without applying the
+    // enabled state (meshcore-firmware#179). Settle, re-read, and only claim
+    // success if the device actually matches what we asked for.
+    await Future.delayed(ObserverConfigService.applySettleDelay);
+    if (!mounted) return;
+    final fresh = await svc.getBroker(_baseline.slot);
+    if (!mounted) return;
+    setState(() => _busy = false);
+    switch (BrokerConfig.classifyApply(
+      intendedEnabled: intended,
+      actualEnabled: fresh?.enabled,
+    )) {
+      case BrokerApplyOutcome.applied:
         messenger.showSnackBar(
           SnackBar(content: Text('Broker ${_baseline.slot} saved')),
         );
         navigator.pop(true);
-    } else {
+      case BrokerApplyOutcome.notApplied:
+        // Stay on the editor, re-seed to the device's true state, and warn — the
+        // fields were written but the device didn't honor the enable/disable.
+        if (fresh != null) _seedFrom(fresh);
         messenger.showSnackBar(
           SnackBar(
             content: Text(
-            'Save failed at "${result.failedField ?? 'a field'}" — the slot '
+              'Saved, but the device did not ${intended ? 'enable' : 'disable'} '
-            'was left disabled. Re-read and retry.',
+              'broker ${_baseline.slot} — possible firmware issue',
             ),
             backgroundColor: errorColor,
           ),
         );
+      case BrokerApplyOutcome.unverified:
+        messenger.showSnackBar(
+          SnackBar(
+            content: Text(
+              'Broker ${_baseline.slot} saved — could not confirm enabled '
+              'state (device may be rebooting)',
+            ),
+          ),
+        );
+        navigator.pop(true);
     }
   }
 

lib/screens/settings/mqtt_brokers_screen.dart

@@ -74,25 +74,70 @@ class _MqttBrokersScreenState extends State<MqttBrokersScreen> {
       }
     }
     final svc = context.read<ObserverConfigService>();
+    setState(() => _busy = true);
     final ok = await svc.setBrokerField(b.slot, 'enabled', enable ? '1' : '0');
     if (!mounted) return;
-    if (ok) {
+    if (!ok) {
+      setState(() => _busy = false);
+      messenger.showSnackBar(
+        SnackBar(
+          content: Text(
+            svc.lastError ??
+                'Failed to ${enable ? 'enable' : 'disable'} broker ${b.slot}',
+          ),
+          backgroundColor: errorColor,
+        ),
+      );
+      return;
+    }
+    // The SET was ACKed — but a firmware build can ACK without applying it
+    // (meshcore-firmware#179). Settle, re-read the slot, and report what the
+    // device ACTUALLY did rather than trusting the ACK.
+    await Future.delayed(ObserverConfigService.applySettleDelay);
+    if (!mounted) return;
+    final fresh = await svc.getBroker(b.slot);
+    if (!mounted) return;
+    setState(() {
+      _busy = false;
+      if (fresh != null) {
+        _brokers = [
+          for (final x in _brokers)
+            if (x.slot == b.slot) fresh else x,
+        ];
+      }
+    });
+    switch (BrokerConfig.classifyApply(
+      intendedEnabled: enable,
+      actualEnabled: fresh?.enabled,
+    )) {
+      case BrokerApplyOutcome.applied:
         messenger.showSnackBar(
           SnackBar(
-          content: Text('Broker ${b.slot} ${enable ? 'enabled' : 'disabled'}'),
+            content: Text(
+              'Broker ${b.slot} ${enable ? 'enabled' : 'disabled'}',
+            ),
           ),
         );
-      await _reload();
+      case BrokerApplyOutcome.notApplied:
-    } else {
         messenger.showSnackBar(
           SnackBar(
             content: Text(
-            svc.lastError ??
+              'Device reported success but broker ${b.slot} is still '
-                'Failed to ${enable ? 'enable' : 'disable'} broker ${b.slot}',
+              '${fresh!.enabled ? 'enabled' : 'disabled'} — possible firmware '
+              'issue',
             ),
             backgroundColor: errorColor,
           ),
         );
+      case BrokerApplyOutcome.unverified:
+        messenger.showSnackBar(
+          SnackBar(
+            content: Text(
+              'Broker ${b.slot} ${enable ? 'enable' : 'disable'} sent — could '
+              'not confirm (device may be rebooting). Tap Refresh to re-check.',
+            ),
+          ),
+        );
     }
   }
 

lib/services/observer_config_service.dart

@@ -24,6 +24,10 @@ class ObserverConfigService extends ChangeNotifier {
   /// an unsupported node or a dropped frame.
   Duration timeout = const Duration(seconds: 6);
 
+  /// After a broker enable/disable SET, wait this long before the verify re-read
+  /// so a device that applies the change asynchronously has settled (#89).
+  static const Duration applySettleDelay = Duration(milliseconds: 600);
+
   ObserverConfig? _config;
   ObserverConfig? get config => _config;
 

test/models/broker_apply_outcome_test.dart

@@ -0,0 +1,61 @@
+// Honest-apply classifier (#89). A firmware build can ACK a broker
+// enable/disable as success but not apply it (HV4 desync,
+// meshcore-firmware#179). After a toggle/save the UI re-reads the slot and
+// compares to what it asked for; this classifier turns (intended, re-read) into
+// the outcome the UI reports. It treats every device identically — it only ever
+// reflects the re-read, never assuming the change took.
+
+import 'package:flutter_test/flutter_test.dart';
+import 'package:meshcore_open/models/observer_config.dart';
+
+void main() {
+  group('BrokerConfig.classifyApply', () {
+    test('enable the device confirms -> applied', () {
+      expect(
+        BrokerConfig.classifyApply(intendedEnabled: true, actualEnabled: true),
+        BrokerApplyOutcome.applied,
+      );
+    });
+
+    test('disable the device confirms -> applied', () {
+      expect(
+        BrokerConfig.classifyApply(
+          intendedEnabled: false,
+          actualEnabled: false,
+        ),
+        BrokerApplyOutcome.applied,
+      );
+    });
+
+    test(
+      'disable ACKed but re-read still enabled -> notApplied (the HV4 bug)',
+      () {
+        expect(
+          BrokerConfig.classifyApply(
+            intendedEnabled: false,
+            actualEnabled: true,
+          ),
+          BrokerApplyOutcome.notApplied,
+        );
+      },
+    );
+
+    test('enable ACKed but re-read still disabled -> notApplied', () {
+      expect(
+        BrokerConfig.classifyApply(intendedEnabled: true, actualEnabled: false),
+        BrokerApplyOutcome.notApplied,
+      );
+    });
+
+    test('re-read failed (null) -> unverified, whatever the intent', () {
+      expect(
+        BrokerConfig.classifyApply(intendedEnabled: true, actualEnabled: null),
+        BrokerApplyOutcome.unverified,
+      );
+      expect(
+        BrokerConfig.classifyApply(intendedEnabled: false, actualEnabled: null),
+        BrokerApplyOutcome.unverified,
+      );
+    });
+  });
+}

test/screens/mqtt_brokers_screen_test.dart

@@ -1,6 +1,7 @@
 // Widget tests for the MQTT brokers screen (#80): a tile per broker, the + FAB
 // opens the editor on the next empty slot, and long-press exposes the
-// Enable/Disable/Edit/Clear actions (Clear behind a confirm).
+// Enable/Disable/Edit/Clear actions (Clear behind a confirm). A toggle/save now
+// verifies the device actually applied the change before confirming (#89).
 
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -21,6 +22,11 @@ class _FakeSvc extends ObserverConfigService {
   bool toggleOk = true;
   String? errorText;
 
+  /// When false, the device ACKs an enabled-SET but the verify re-read reports
+  /// the slot UNCHANGED — models the HV4 ack-but-doesn't-apply bug (#89).
+  bool deviceApplies = true;
+  final Map<int, bool> _enabledOverride = {};
+
   @override
   String? get lastError => errorText;
   @override
@@ -30,9 +36,21 @@ class _FakeSvc extends ObserverConfigService {
   @override
   Future<bool> setBrokerField(int slot, String field, String value) async {
     setCalls.add('$slot.$field=$value');
+    if (toggleOk && deviceApplies && field == 'enabled') {
+      _enabledOverride[slot] = value == '1';
+    }
     return toggleOk;
   }
 
+  @override
+  Future<BrokerConfig?> getBroker(int slot) async {
+    final base = _brokers.firstWhere(
+      (b) => b.slot == slot,
+      orElse: () => BrokerConfig(slot: slot),
+    );
+    return base.copyWith(enabled: _enabledOverride[slot] ?? base.enabled);
+  }
+
   @override
   Future<bool> clearBroker(int slot) async {
     clearCalls.add(slot);
@@ -102,7 +120,9 @@ void main() {
     expect(fake.clearCalls, contains(2));
   });
 
-  testWidgets('a successful quick Enable confirms', (tester) async {
+  testWidgets('a successful quick Enable verifies then confirms', (
+    tester,
+  ) async {
     final fake = _FakeSvc(const [BrokerConfig(slot: 2, url: 'a', port: 1883)]);
     await _pump(tester, fake);
 
@@ -110,9 +130,31 @@ void main() {
     await tester.pumpAndSettle();
     await tester.tap(find.text('Enable'));
     await tester.pumpAndSettle();
+    // Advance past the settle delay so the verify re-read runs.
+    await tester.pump(const Duration(milliseconds: 700));
+    await tester.pumpAndSettle();
+
+    expect(fake.setCalls, contains('2.enabled=1'));
+    expect(find.textContaining('Broker 2 enabled'), findsOneWidget);
+    await tester.pump(const Duration(seconds: 5));
+  });
+
+  testWidgets('an ACK-but-not-applied toggle warns of a firmware issue', (
+    tester,
+  ) async {
+    final fake = _FakeSvc(const [BrokerConfig(slot: 2, url: 'a', port: 1883)])
+      ..deviceApplies = false; // ACKs success but re-read stays disabled
+    await _pump(tester, fake);
+
+    await tester.longPress(find.text('[2] a'));
+    await tester.pumpAndSettle();
+    await tester.tap(find.text('Enable'));
+    await tester.pumpAndSettle();
+    await tester.pump(const Duration(milliseconds: 700));
+    await tester.pumpAndSettle();
 
     expect(fake.setCalls, contains('2.enabled=1'));
-    expect(find.textContaining('enabled'), findsOneWidget);
+    expect(find.textContaining('possible firmware issue'), findsOneWidget);
     await tester.pump(const Duration(seconds: 5));
   });