the issue audit: everything still true, fixed

.github/workflows/pr-agent.yml

@@ -13,9 +13,19 @@ jobs:
       issues: write
       pull-requests: write
       contents: write
+    # Job-level so the step `if` below can see it: the `secrets` context is not
+    # available in an `if` expression, but `env` is.
+    env:
+      PR_AGENT_API_KEY: ${{ secrets.PR_AGENT_API_KEY }}
     steps:
       - name: PR Agent action step
-        uses: the-pr-agent/pr-agent@main
+        # A PR from a fork gets no secrets, so this ran with an empty key,
+        # reviewed nothing, and still went green - a check that says "reviewed"
+        # when it did not is worse than no check. Skip instead.
+        if: env.PR_AGENT_API_KEY != ''
+        # Pinned, not @main: this action runs with `contents: write` and a token
+        # on every PR, and a floating ref means whatever landed upstream today.
+        uses: the-pr-agent/pr-agent@f6af7d77554ff8d26adffded077e6461329e92fa # v0.42.0
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           # Credentials only. The model chain lives in .pr_agent.toml so it is

.pr_agent.toml

@@ -21,9 +21,13 @@ fallback_models = [
     "openai/gpt-oss-120b-medium",
 ]
 # Required: an `openai/`-prefixed name is not in PR-Agent's MAX_TOKENS map, and
-# get_max_tokens (algo/utils.py:1008) raises rather than defaulting. Effective
-# input is still min(this, max_model_tokens=32000).
+# get_max_tokens (algo/utils.py:1008) raises rather than defaulting.
 custom_model_max_tokens = 200000
+# The effective input is min(custom_model_max_tokens, max_model_tokens), and
+# max_model_tokens defaults to 32000 - so the 200k above bought nothing and a
+# large diff was silently clipped to a third of the review it looked like it
+# got. Raise the ceiling to match.
+max_model_tokens = 200000
 
 # Inject AGENTS.md as repository context into /review, /improve, /describe, /ask.
 # NOTE: read from the DEFAULT BRANCH by default, so AGENTS.md only takes effect

PRIVACY.md

@@ -64,9 +64,10 @@ because a sideloaded app has no store to tell you a security fix exists.
 
 **Barcode lookup for food logging**
 The food log can read a barcode with the camera and fill in the nutrition
-figures for you. Doing that means asking a database, so it is **off until you
-turn it on**, and the App asks you before the first lookup ever happens — not
-after.
+figures for you. Doing that means asking a database, so it is **on by
+default**: what leaves is a number the manufacturer printed on the packet, and
+nothing about you goes with it. Turn it off and the scan stops asking anybody
+anything.
 
 - **What is sent is the barcode.** It goes to openfoodfacts.org, the free and
   open food database. Nothing about you, your meals, your health or your device
@@ -157,7 +158,7 @@ moment you tap it:
   further upload immediately, and the App tells you when the last one was.
 - **Check for updates** — turning it off stops the App making any network
   request of its own accord.
-- **Look barcodes up online** — off by default; turning it off stops any
+- **Look barcodes up online** — on by default; turning it off stops any
   further lookup immediately, and the food log keeps working by hand.
 
 You can also disable AI Coach or Health app integration at any time in Settings

README.md

@@ -79,7 +79,8 @@ drawer-bracelet problem can use it, or go dig through the code themselves.
 
 ## Checklist
 
-- **WHOOP 4.0 only.** Haven't touched a WHOOP 5, don't know if it even shares a protocol.
+- **WHOOP 4.0 is the one that's properly tested.** WHOOP 5 and MG work too, but they're
+  experimental — see the note further down.
 - Not affiliated with WHOOP, doesn't talk to their servers.
 - Not a clone of their algorithms — different math, published methods, cited in the
   analytics repo. Don't expect identical numbers to what their app shows.
@@ -143,9 +144,10 @@ shortcuts, a smart alarm that buzzes the band.
   against a lab, don't treat any of it as a diagnosis.
 - Not on the App Store or Play Store yet. iOS is a public TestFlight beta, which is a
   normal install but still a beta; Android is an APK straight off Releases.
-- WHOOP 5.0 / MG support is in progress and **experimental** — the band is detected and
-  spoken to, but it hasn't been validated against real 5.0 hardware. WHOOP 4.0 is the
-  only one that's actually tested.
+- WHOOP 5.0 / MG support is **experimental**. Both pair, sync and decode, and the work is
+  checked against real records off real bands — but 4.0 is the one I wear every day, so
+  it's the one that gets found out when it breaks. Expect rough edges on 5 and MG, and
+  open an issue when you hit one.
 
 ## Run it
 

docs/privacy.html

@@ -68,9 +68,10 @@ <h2>Anonymous diagnostics</h2>
 
     <h2>Barcode lookup for food logging</h2>
     <p>The food log can read a barcode with the camera and fill in the nutrition
-      figures for you. Doing that means asking a database, so it is <strong>off
-      until you turn it on</strong>, and the App asks you before the first
-      lookup ever happens &mdash; not after.</p>
+      figures for you. Doing that means asking a database, so it is <strong>on
+      by default</strong>: what leaves is a number the manufacturer printed on
+      the packet, and nothing about you goes with it. Turn it off and the scan
+      stops asking anybody anything.</p>
     <ul>
       <li><strong>What is sent is the barcode.</strong> It goes to
         openfoodfacts.org, the free and open food database. Nothing about you,
@@ -164,8 +165,8 @@ <h2>Your controls</h2>
       integration at any time in Settings if you'd previously turned them on. If
       you explicitly installed a GitHub release and enabled health data
       contribution, you can disable that feature at any time from the app's
-      settings. Barcode lookup for the food log is off by default and can be
-      turned off again at any time in Settings &rsaquo; Privacy &rsaquo;
+      settings. Barcode lookup for the food log is on by default and can be
+      turned off at any time in Settings &rsaquo; Privacy &rsaquo;
       &ldquo;Look barcodes up online&rdquo;.</p>
 
     <p>Uninstalling the App deletes all of your locally stored data immediately.

ios/Podfile.lock

@@ -208,6 +208,9 @@ PODS:
     - Flutter
   - home_widget (0.0.1):
     - Flutter
+  - mobile_scanner (7.0.0):
+    - Flutter
+    - FlutterMacOS
   - nanopb (3.30910.0):
     - nanopb/decode (= 3.30910.0)
     - nanopb/encode (= 3.30910.0)
@@ -232,9 +235,6 @@ PODS:
   - SwiftyGif (5.4.5)
   - url_launcher_ios (0.0.1):
     - Flutter
-  - video_player_avfoundation (0.0.1):
-    - Flutter
-    - FlutterMacOS
   - workmanager_apple (0.0.1):
     - Flutter
 
@@ -255,12 +255,12 @@ DEPENDENCIES:
   - geolocator_apple (from `.symlinks/plugins/geolocator_apple/darwin`)
   - health (from `.symlinks/plugins/health/ios`)
   - home_widget (from `.symlinks/plugins/home_widget/ios`)
+  - mobile_scanner (from `.symlinks/plugins/mobile_scanner/darwin`)
   - package_info_plus (from `.symlinks/plugins/package_info_plus/ios`)
   - share_plus (from `.symlinks/plugins/share_plus/ios`)
   - shared_preferences_foundation (from `.symlinks/plugins/shared_preferences_foundation/darwin`)
   - sqflite_darwin (from `.symlinks/plugins/sqflite_darwin/darwin`)
   - url_launcher_ios (from `.symlinks/plugins/url_launcher_ios/ios`)
-  - video_player_avfoundation (from `.symlinks/plugins/video_player_avfoundation/darwin`)
   - workmanager_apple (from `.symlinks/plugins/workmanager_apple/ios`)
 
 SPEC REPOS:
@@ -323,6 +323,8 @@ EXTERNAL SOURCES:
     :path: ".symlinks/plugins/health/ios"
   home_widget:
     :path: ".symlinks/plugins/home_widget/ios"
+  mobile_scanner:
+    :path: ".symlinks/plugins/mobile_scanner/darwin"
   package_info_plus:
     :path: ".symlinks/plugins/package_info_plus/ios"
   share_plus:
@@ -333,8 +335,6 @@ EXTERNAL SOURCES:
     :path: ".symlinks/plugins/sqflite_darwin/darwin"
   url_launcher_ios:
     :path: ".symlinks/plugins/url_launcher_ios/ios"
-  video_player_avfoundation:
-    :path: ".symlinks/plugins/video_player_avfoundation/darwin"
   workmanager_apple:
     :path: ".symlinks/plugins/workmanager_apple/ios"
 
@@ -374,6 +374,7 @@ SPEC CHECKSUMS:
   GoogleUtilities: 766ace00c6b10d8148408f329d10c4f051931850
   health: a4ddeac72091000e94776864d0028f6be31ec7a5
   home_widget: f169fc41fd807b4d46ab6615dc44d62adbf9f64f
+  mobile_scanner: 9157936403f5a0644ca3779a38ff8404c5434a93
   nanopb: fad817b59e0457d11a5dfbde799381cd727c1275
   package_info_plus: af8e2ca6888548050f16fa2f1938db7b5a5df499
   PromisesObjC: 752c3227f599e3467650e47ea36f433eeb10c273
@@ -384,7 +385,6 @@ SPEC CHECKSUMS:
   sqflite_darwin: 20b2a3a3b70e43edae938624ce550a3cbf66a3d0
   SwiftyGif: 706c60cf65fa2bc5ee0313beece843c8eb8194d4
   url_launcher_ios: 7a95fa5b60cc718a708b8f2966718e93db0cef1b
-  video_player_avfoundation: 3453f792138786248960ca029747fcd9f318ef52
   workmanager_apple: 904529ae31e97fc5be632cf628507652294a0778
 
 PODFILE CHECKSUM: b50997058227f33b81189532a9f3fc5007ec070b

lib/app.dart

@@ -27,6 +27,7 @@ import 'ui2/screens/what_changed.dart';
 import 'ui2/screens/health_screen.dart';
 import 'ui2/screens/home_screen.dart';
 import 'ui2/screens/journal_compose.dart';
+import 'ui2/screens/log_workout.dart';
 import 'ui2/screens/nutrition_screen.dart';
 import 'ui2/screens/wellness_screen.dart';
 import 'ui2/screens/workout_screen.dart';
@@ -337,13 +338,14 @@ ShellDomain domainForRoute(String route) => switch (route) {
 /// The focused screen a deep link pushes on top of its domain, when one
 /// exists. Null means the domain itself is the destination.
 ///
-/// One route still resolves to null and should not: `/workouts/suggestion`
-/// ("Tap to log it" has nothing to tap through to — nothing reads
-/// `workout_suggestions`). It is recorded in the sweep; the fix is to stop
-/// making the promise, not to route it somewhere plausible.
+/// `/workouts/suggestion` used to be in that list, and it was the one route
+/// where the fallback was a broken promise: "Tap to log it" landed on the
+/// plain Workouts tab, because the screen that could log it was deleted with
+/// `lib/ui/workouts/` and nothing read `workout_suggestions`. There is a
+/// destination again, and confirming on it writes a real session.
 ///
-/// `/ai/*` used to be in that list. It now lands on the briefing itself, which
-/// also carries the exact snapshot that was sent to produce it.
+/// `/ai/*` used to be in that list too. It now lands on the briefing itself,
+/// which also carries the exact snapshot that was sent to produce it.
 Widget? screenForRoute(String route) => switch (route) {
       kRouteAiMorning =>
         const AiBriefingScreen(period: BriefingPeriod.morning),
@@ -357,6 +359,9 @@ Widget? screenForRoute(String route) => switch (route) {
       // which is how the tile that everybody actually used stayed add-only for
       // so long — the thing that could clear a value was behind a notification.
       kRouteWater => const NutritionScreen(),
+      // The detected bout, with the three answers to it: log it, adjust the
+      // times first, or say it never happened.
+      kRouteWorkoutSuggestion => const WorkoutSuggestionScreen(),
       // Battery, band and sources all live behind this one.
       kRouteProfile => const ProfileHome(),
       // The weekly recap used to land on the Health tab and push nothing,

lib/coach/coach_actions.dart

@@ -30,6 +30,7 @@ import '../data/journal_fields.dart';
 import '../data/local_repository.dart';
 import '../data/med_store.dart';
 import '../data/nutrition_store.dart';
+import '../health/health_export.dart';
 
 /// Raised when the model's arguments cannot be honoured. The message goes back
 /// into the transcript so the model can correct itself rather than retrying the
@@ -264,6 +265,11 @@ class CoachActions {
         endTs: startTs + mins * 60,
         type: type,
       );
+      // Every other write path exports; without this a workout logged through
+      // the coach reached the health store only if the next day-result pass
+      // happened to sweep it up (#130). The seam checks `healthSyncEnabled`
+      // itself, so this is a no-op with the switch off, and it never throws.
+      await HealthExporter.exportWorkoutId(r['workout_id'] as String?);
       return jsonEncode({'saved': true, 'date': d, 'type': type, ...r});
     } catch (e) {
       // The repo rejects overlaps, futures and absurd durations. Hand the

lib/coach/coach_config.dart

@@ -72,6 +72,32 @@ class CoachConfig extends ChangeNotifier {
   /// late `_key = null` would wipe the key they just saved out of the session.
   int _generation = 0;
 
+  /// ONE keychain MUTATION at a time.
+  ///
+  /// [load] does not only read: it writes the value it just read back, to
+  /// upgrade an item stored before this class asked for `first_unlock`. That
+  /// write is awaited, but `load` itself is not — the startup call is
+  /// fire-and-forget — so nothing stopped it overlapping the user's Save. Two
+  /// ways that ends badly: the upgrade lands last and puts the OLD key back
+  /// over the one they just pasted, or, on iOS, a write races a delete inside
+  /// the plugin and comes out as `PlatformException(-25299)`
+  /// (errSecDuplicateItem). [_generation] already orders the in-memory half of
+  /// that race; it cannot order two calls that are both inside the plugin.
+  ///
+  /// WRITES ONLY, deliberately. The read is left outside, because a keystore
+  /// read can hang outright (the documented Samsung Knox case this file's
+  /// `load` is already shaped around) and a lock that a hung read holds would
+  /// block Save forever — trading a rare clobber for a wedged settings screen.
+  Future<void> _keychainLock = Future.value();
+
+  Future<void> _serialized(Future<void> Function() op) {
+    final done = _keychainLock.then((_) => op());
+    // A failed operation must not wedge the queue — the next caller runs either
+    // way, and the error still reaches whoever awaited `done`.
+    _keychainLock = done.catchError((_) {});
+    return done;
+  }
+
   String get baseUrl => _baseUrl;
   String get model => _model;
   String? get apiKey => _key;
@@ -150,13 +176,19 @@ class CoachConfig extends ChangeNotifier {
         // Keystore (the documented Samsung Knox hang) on the startup path for
         // no reason.
         if (marker != true) {
-          await _secure.write(
-            key: _kKey,
-            value: read,
-            iOptions: _apple,
-            mOptions: _macos,
-          );
-          await prefs.setBool(_kKeyPresent, true);
+          await _serialized(() async {
+            // Re-checked INSIDE the lock, not just before the read. A save can
+            // land while this upgrade is queued behind it, and writing `read`
+            // then would put the superseded key back.
+            if (generation != _generation) return;
+            await _secure.write(
+              key: _kKey,
+              value: read,
+              iOptions: _apple,
+              mOptions: _macos,
+            );
+            await prefs.setBool(_kKeyPresent, true);
+          });
         }
       } else if (trusted) {
         // Foreground, so the keychain is readable and an empty answer is the
@@ -225,24 +257,26 @@ class CoachConfig extends ChangeNotifier {
       // other order leaves memory holding a key that was never persisted (lost
       // at the next launch, with no marker to even flag it as missing), or
       // hiding one that is still stored.
-      if (k.isEmpty) {
-        await _secure.delete(key: _kKey, iOptions: _apple, mOptions: _macos);
-        // The marker follows the keychain, and its own failure is not worth
-        // failing the save: a stale `true` costs a retry, never a lost key.
-        try {
-          await prefs.setBool(_kKeyPresent, false);
-        } catch (_) {/* re-established by the next load */}
-      } else {
-        await _secure.write(
-          key: _kKey,
-          value: k,
-          iOptions: _apple,
-          mOptions: _macos,
-        );
-        try {
-          await prefs.setBool(_kKeyPresent, true);
-        } catch (_) {/* re-established by the next load */}
-      }
+      await _serialized(() async {
+        if (k.isEmpty) {
+          await _secure.delete(key: _kKey, iOptions: _apple, mOptions: _macos);
+          // The marker follows the keychain, and its own failure is not worth
+          // failing the save: a stale `true` costs a retry, never a lost key.
+          try {
+            await prefs.setBool(_kKeyPresent, false);
+          } catch (_) {/* re-established by the next load */}
+        } else {
+          await _secure.write(
+            key: _kKey,
+            value: k,
+            iOptions: _apple,
+            mOptions: _macos,
+          );
+          try {
+            await prefs.setBool(_kKeyPresent, true);
+          } catch (_) {/* re-established by the next load */}
+        }
+      });
       _key = k.isEmpty ? null : k;
       _keyUnreadable = false;
       _keyUndetermined = false;