gate the workout health write on a delete that actually cleared

lib/health/health_export.dart

@@ -93,6 +93,39 @@ List<HealthDataType> healthDeleteTypes({required bool isApplePlatform}) {
       : types.where((type) => type != HealthDataType.HEART_RATE).toList();
 }
 
+/// Does a `delete()` answer mean the window is now clear of OUR samples — i.e.
+/// is it safe to write the replacement?
+///
+/// The two stores answer the EMPTY range differently, so the raw bool cannot be
+/// read the same way on both, and reading it wrong breaks the delete-then-write
+/// idempotency in opposite directions:
+///
+///   * Health Connect (`HealthPlugin.deleteData`) calls `deleteRecords` over a
+///     time range and reports success unless it THREW. A zero-match range is a
+///     normal success. So `false` there is always a genuine failure — a denied
+///     permission, an unmapped type, a store error — and the samples we meant
+///     to replace may well still be sitting there.
+///
+///   * HealthKit (`SwiftHealthPlugin.delete`) queries our own samples
+///     (`HKSource.default()`) and hands whatever came back to
+///     `HKHealthStore.delete`, which Apple documents as: "Deleting an empty
+///     array fails with an `errorInvalidArgument` error." So on Apple `false`
+///     is the NORMAL answer for a window we have never written — every FIRST
+///     export of every day and every workout sees it — and it says nothing
+///     about whether a real sample survived.
+///
+/// Hence: gate the write on Android, never on Apple. Trusting Apple's `false`
+/// is not optimism, it is the only reading the platform supports; and the
+/// HealthKit failures that CAN leave one of our samples behind (share
+/// permission never requested, or denied) suppress the following write for the
+/// same reason, so they cannot produce the duplicate this gate exists to stop.
+///
+/// Parameterised on [ios] rather than reading `Platform` for the same reason
+/// [healthActivityForType] is — so a host-VM test can exercise both branches.
+@visibleForTesting
+bool healthDeleteClearedRange({required bool deleted, required bool ios}) =>
+    deleted || ios;
+
 /// Cursor for the one-shot Apple Health sleep rewrite. Bump when the writer
 /// changes enough that nights already sitting in HealthKit should be replaced
 /// (plugin Core/in-bed misses, leftover 11pm fragments). Does not bump
@@ -355,6 +388,32 @@ class HealthExporter {
     }
   }
 
+  /// Delete OUR [type] samples in [start,end) and report whether the window is
+  /// now safe to write into — see [healthDeleteClearedRange] for why the two
+  /// stores' `false` cannot be read the same way.
+  ///
+  /// A THROW is a genuine failure on both stores (and on Apple the only signal
+  /// there is), so it is the one case that reports "not cleared" everywhere.
+  Future<bool> _deleteOwnSamples(
+    HealthDataType type,
+    DateTime start,
+    DateTime end,
+  ) async {
+    try {
+      return healthDeleteClearedRange(
+        deleted: await _health.delete(
+          type: type,
+          startTime: start,
+          endTime: end,
+        ),
+        ios: isApple,
+      );
+    } catch (e) {
+      debugPrint('[health] delete ${type.name}: $e');
+      return false;
+    }
+  }
+
   // We do NOT gate on a write-permission check: HealthKit hides write-auth by
   // design, and Health Connect's hasPermissions(WRITE) frequently returns
   // null/false even after the user grants everything — which would leave the UI
@@ -844,21 +903,20 @@ class HealthExporter {
     // Idempotency: remove OUR previously-written samples for this day (HealthKit /
     // Health Connect only let an app delete its own data), then re-write fresh.
     // Sleep is not in this list — native replace already deleted it.
+    //
+    // A type whose delete did NOT clear the window must not be re-written on
+    // top of the survivor — that is how a retry turns one stale sample into
+    // two, then three. Only WORKOUT is tracked, because a duplicated workout
+    // is the one that shows up as a second entry in the user's activity list
+    // (and is the type `exportWorkout` re-writes out-of-band too); the scalar
+    // types below still write unconditionally, so an uncleared RHR/HRV window
+    // can still double until the next successful delete replaces both.
+    var workoutCleared = true;
     for (final t in _rewriteTypes) {
-      try {
-        final deleted = await _health.delete(
-          type: t,
-          startTime: dayStart,
-          endTime: dayEnd,
-        );
-        if (!deleted) {
-          debugPrint('[health] delete ${t.name} returned false');
-          success = false;
-        }
-      } catch (e) {
-        debugPrint('[health] delete ${t.name}: $e');
-        success = false;
-      }
+      if (await _deleteOwnSamples(t, dayStart, dayEnd)) continue;
+      debugPrint('[health] delete ${t.name} did not clear the day');
+      success = false;
+      if (t == HealthDataType.WORKOUT) workoutCleared = false;
     }
 
     final scalars = (b['scalars'] as Map?)?.cast<String, dynamic>() ?? const {};
@@ -1099,7 +1157,12 @@ class HealthExporter {
         debugPrint('[health] query workouts: $e');
         success = false;
       }
-      if (rows != null) {
+      // `workoutCleared` is false only when the day's WORKOUT delete genuinely
+      // failed (see the rewrite loop above), in which case every row here would
+      // land beside a survivor. `success` is already false, so the day retries
+      // and re-attempts the delete first; skipping only this block keeps that
+      // failure from touching the day's unrelated exports.
+      if (rows != null && workoutCleared) {
         for (final r in rows) {
           if (await _writeOneWorkout(r) == false) {
             debugPrint('[health] write workout returned false');
@@ -1161,6 +1224,11 @@ class HealthExporter {
   /// [_exportDay], which owns the whole-day delete). Best-effort — never
   /// throws; no-op if the health store isn't configured/available/permitted
   /// (mirrors [_exportDay]'s silent-no-op-on-missing-permission contract).
+  ///
+  /// The idempotency is only as good as the delete, so the write is GATED on
+  /// it: a delete that did not clear the window returns false without writing,
+  /// because writing beside a survivor is what turns a retry into a duplicate.
+  /// See [healthDeleteClearedRange] for what "did not clear" means per store.
   Future<bool> exportWorkout(Map<String, Object?> session) async {
     if ((session['status']?.toString() ?? '') == 'live') return false;
     final st = (session['start_ts'] as num?)?.toInt();
@@ -1171,20 +1239,16 @@ class HealthExporter {
       if (await _androidUnavailable() != null) return false;
       final start = DateTime.fromMillisecondsSinceEpoch(st * 1000);
       final end = DateTime.fromMillisecondsSinceEpoch(en * 1000);
-      var success = true;
-      try {
-        final deleted = await _health.delete(
-          type: HealthDataType.WORKOUT,
-          startTime: start,
-          endTime: end,
-        );
-        if (!deleted) success = false;
-      } catch (e) {
-        debugPrint('[health] delete workout @$st: $e');
-        success = false;
+      if (!await _deleteOwnSamples(HealthDataType.WORKOUT, start, end)) {
+        // The window still holds a copy we could not remove, so writing now
+        // would leave TWO of this workout in the store — and returning false
+        // hands the caller a retry, which would write a third. Bail instead:
+        // the same false still asks for a retry, but the retry re-attempts the
+        // delete FIRST and only writes once it actually clears.
+        debugPrint('[health] delete workout @$st did not clear the window');
+        return false;
       }
-      final wrote = (await _writeOneWorkout(session)) ?? false;
-      return success && wrote;
+      return (await _writeOneWorkout(session)) ?? false;
     } catch (e) {
       debugPrint('[health] exportWorkout: $e');
       return false;

test/health_workout_export_delete_gate_test.dart

@@ -0,0 +1,129 @@
+import 'package:flutter/services.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:openstrap_edge/health/health_export.dart';
+
+/// A stand-in for the platform health store, driven over the `health` plugin's
+/// own method channel — the only seam `HealthExporter` reaches the store
+/// through. Records every call so a test can assert what the exporter did
+/// AFTER a delete came back false, which is the whole point: the bug was that
+/// it wrote anyway.
+class _FakeHealthStore {
+  _FakeHealthStore({required this.deleteResult, this.deleteThrows = false});
+
+  /// What `delete` answers. On Health Connect a `false` here means the delete
+  /// genuinely failed and whatever we wrote before is STILL in the store.
+  final bool deleteResult;
+  final bool deleteThrows;
+
+  final calls = <String>[];
+
+  static const _channel = MethodChannel('flutter_health');
+
+  void install() {
+    TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
+        .setMockMethodCallHandler(_channel, (call) async {
+          calls.add(call.method);
+          switch (call.method) {
+            case 'delete':
+              if (deleteThrows) {
+                throw PlatformException(code: 'delete-failed');
+              }
+              return deleteResult;
+            case 'writeWorkoutData':
+              return true;
+            default:
+              return null;
+          }
+        });
+  }
+
+  void remove() {
+    TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
+        .setMockMethodCallHandler(_channel, null);
+  }
+}
+
+Map<String, Object?> _session() {
+  final start = DateTime(2026, 8, 26, 18, 30);
+  final end = DateTime(2026, 8, 26, 19, 15);
+  return {
+    'status': 'done',
+    'type': 'run',
+    'start_ts': start.millisecondsSinceEpoch ~/ 1000,
+    'end_ts': end.millisecondsSinceEpoch ~/ 1000,
+    'calories': 412,
+  };
+}
+
+void main() {
+  TestWidgetsFlutterBinding.ensureInitialized();
+
+  group('healthDeleteClearedRange', () {
+    // The fact the whole gate hinges on, pinned so a future reader does not
+    // "simplify" it back to a bare `deleted`. Verified against the plugin
+    // sources in health 12.2.1 and Apple's own docs:
+    //
+    //   * HealthPlugin.kt `deleteData` -> deleteRecords over a time range,
+    //     `result.success(true)` unless it threw. Zero matches is a success.
+    //   * SwiftHealthPlugin.swift `delete` -> HKSampleQuery scoped to
+    //     HKSource.default(), then HKHealthStore.delete(samples) with whatever
+    //     came back — including an EMPTY array, which Apple documents as
+    //     "Deleting an empty array fails with an errorInvalidArgument error".
+    //
+    // So on Apple a false delete is what every first export sees, and gating
+    // the write on it would mean no workout ever reaches HealthKit at all.
+    test('Health Connect false is a genuine failure', () {
+      expect(healthDeleteClearedRange(deleted: false, ios: false), isFalse);
+      expect(healthDeleteClearedRange(deleted: true, ios: false), isTrue);
+    });
+
+    test('HealthKit false is the documented empty-range answer', () {
+      expect(healthDeleteClearedRange(deleted: false, ios: true), isTrue);
+      expect(healthDeleteClearedRange(deleted: true, ios: true), isTrue);
+    });
+  });
+
+  group('exportWorkout delete-then-write', () {
+    // The host VM is neither iOS nor macOS, so `HealthExporter.isApple` is
+    // false and these exercise the Health-Connect reading of `delete`.
+    late _FakeHealthStore store;
+
+    tearDown(() => store.remove());
+
+    test('a failed delete does not write a duplicate on top of it', () async {
+      store = _FakeHealthStore(deleteResult: false)..install();
+
+      final ok = await HealthExporter().exportWorkout(_session());
+
+      expect(ok, isFalse, reason: 'the caller must retry this workout');
+      expect(store.calls, contains('delete'));
+      expect(
+        store.calls,
+        isNot(contains('writeWorkoutData')),
+        reason:
+            'the previously exported copy survived the delete, so writing '
+            'would leave two of this workout in the store — and the false '
+            'return drives a retry that would write a third',
+      );
+    });
+
+    test('a thrown delete does not write either', () async {
+      store = _FakeHealthStore(deleteResult: true, deleteThrows: true)
+        ..install();
+
+      final ok = await HealthExporter().exportWorkout(_session());
+
+      expect(ok, isFalse);
+      expect(store.calls, isNot(contains('writeWorkoutData')));
+    });
+
+    test('a successful delete still writes', () async {
+      store = _FakeHealthStore(deleteResult: true)..install();
+
+      final ok = await HealthExporter().exportWorkout(_session());
+
+      expect(ok, isTrue);
+      expect(store.calls, containsAllInOrder(['delete', 'writeWorkoutData']));
+    });
+  });
+}