diff --git a/.github/scripts/check_sibling_pins.sh b/.github/scripts/check_sibling_pins.sh new file mode 100755 index 0000000..3a61b35 --- /dev/null +++ b/.github/scripts/check_sibling_pins.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash +# Shared by test.yml (test job) and build.yml (preflight job) — a release +# tagged straight off `main` never runs test.yml at all, so this guard has +# to live in both places to actually protect a release. +# +# This pin-drift class has bitten the project repeatedly: +# - v43 rode a floating `main` ref and silently picked up a breaking change +# - 2026-07-20: the pinned SHA didn't actually contain the analytics fix a +# changelog entry cited — the bug it "fixed" stayed live for 3 releases +# - 2026-07-26: a local `flutter pub get` run WITH pubspec_overrides.yaml +# present rewrote the tracked pubspec.lock to local path sources, +# silently dropping the git provenance — happened twice in one session +# - 2026-07-30: edge shipped a full day pinned one merge behind BOTH +# protocol and analytics main — including a protocol fix for a decoder +# bug that could eat up to 4092 bytes of good historical data — because +# nothing ever re-checked the pin against upstream after it was set +# +# Two independent checks, run per sibling package: +# 1. pubspec.lock actually resolves what pubspec.yaml pins (not a local +# path, not some other ref). FATAL — catches the pub-get-with-overrides +# class above; a release built from this state doesn't contain what its +# own pin claims. +# 2. the pinned ref is not stale against the sibling's current main. +# NON-FATAL (a warning annotation, not a build failure) — a deliberate +# short lag while a fix bakes is a legitimate call for a maintainer to +# make, but per 2026-07-30 it must be a call someone actually makes, +# not a silent default nobody notices. Skipped (not failed) if the +# sibling repo can't be reached — a network hiccup here must not block +# an otherwise-good build. +set -euo pipefail +fail=0 + +# pubspec_overrides.yaml is gitignored and local-dev only. If one ever arrives +# in the tree, `flutter pub get` picks it up automatically and CI silently +# stops testing the pinned SHAs. Checked before `pub get` runs, so this +# inspects the committed file, not whatever pub resolution produced from it. +if [ -e pubspec_overrides.yaml ]; then + echo "::error::pubspec_overrides.yaml is present in CI — dependency" + echo "::error::resolution would use local paths instead of the pins." + fail=1 +fi + +for pkg in openstrap_protocol openstrap_analytics; do + case "$pkg" in + openstrap_protocol) repo="protocol" ;; + openstrap_analytics) repo="analytics" ;; + esac + + block=$(awk -v p=" $pkg:" '$0==p{f=1;next} /^ [a-z_]+:/{f=0} f' pubspec.lock) + + if printf '%s' "$block" | grep -qE '^\s+source: path'; then + echo "::error::pubspec.lock resolves $pkg from a local path." + echo "::error::Re-run: mv pubspec_overrides.yaml /tmp/ && flutter pub get && mv /tmp/pubspec_overrides.yaml ." + fail=1 + continue + fi + + locked=$(printf '%s' "$block" | grep -E '^\s+resolved-ref:' | head -1 | awk '{print $2}' | tr -d '"') + pinned=$(awk -v p=" $pkg:" '$0==p{f=1;next} /^ [a-z_]+:/{f=0} f' pubspec.yaml \ + | grep -E '^\s+ref:' | head -1 | awk '{print $2}' | tr -d '"') + + if [ -z "$locked" ] || [ -z "$pinned" ]; then + echo "::error::could not read a commit pin for $pkg (lock='$locked' pubspec='$pinned')." + fail=1 + continue + fi + if [ "$locked" != "$pinned" ]; then + echo "::error::$pkg pin drift — pubspec.yaml says $pinned, pubspec.lock resolved $locked." + echo "::error::A release citing a sibling change would not actually contain it." + fail=1 + continue + fi + echo "$pkg pinned at $pinned, lock agrees." + + # Bounded: git ls-remote has no built-in timeout, and this whole probe is + # supposed to degrade to a warning on any network trouble — an unbounded + # hang on a stalled connection would instead wedge the job (test job or, + # worse, release preflight) until CI's own multi-hour job timeout, which is + # a much bigger failure than the staleness check it's guarding against. + if upstream="$(timeout --kill-after=5s 30s git ls-remote "https://github.com/OpenStrap/$repo.git" refs/heads/main 2>/dev/null | awk '{print $1}')" \ + && [ -n "$upstream" ]; then + if [ "$upstream" != "$pinned" ]; then + echo "::warning::$pkg is pinned at $pinned but $repo's main has moved to $upstream." + echo "::warning::Deliberate short lag is fine — just make sure it's a conscious call, not a forgotten one. (This exact gap shipped edge a full day behind protocol's CRC8-length-check fix on 2026-07-30.)" + fi + else + echo "::warning::could not reach $repo's main to check pin staleness — not failing the build for it." + fi +done + +[ "$fail" = 0 ] diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 032395e..7b2e289 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -21,6 +21,15 @@ jobs: fetch-depth: 0 # all tags, so the versionCode guard can read prior releases' pubspec persist-credentials: false # nothing here writes to the repo + # Shared with test.yml — but a tag push (this workflow's only trigger) + # never runs test.yml, so without this the pin-drift guard protected + # nothing about an actual release. Bit the project on 2026-07-30: edge + # shipped a full day pinned one merge behind protocol/analytics main, + # including a decoder fix for a bug that could eat up to 4092 bytes of + # good historical data, and no release-path check ever saw it. + - name: Guard — the lock pins sibling commits, and the pins aren't stale + run: bash .github/scripts/check_sibling_pins.sh + # Fail the release BEFORE building if the Android versionCode is missing or # would go backwards. Flutter derives versionCode from the `+BUILD` suffix # of pubspec's `version:` and silently falls back to 1 when it's absent, so diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c9a5f37..eafcfa0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -53,47 +53,11 @@ jobs: # resolved-ref, so the lock stops recording which sibling commit was # actually tested. It is easy to do by accident — any local `flutter # build` or `flutter test` triggers it — and it has reached main before. - # This runs BEFORE `pub get` so it inspects the committed file. - - name: Guard — the lock pins sibling commits, not local paths - run: | - set -euo pipefail - fail=0 - - # pubspec_overrides.yaml is gitignored and local-dev only. If one ever - # arrives in the tree, `flutter pub get` below picks it up - # automatically and CI silently stops testing the pinned SHAs. - if [ -e pubspec_overrides.yaml ]; then - echo "::error::pubspec_overrides.yaml is present in CI — dependency" - echo "::error::resolution would use local paths instead of the pins." - fail=1 - fi - - for pkg in openstrap_protocol openstrap_analytics; do - block=$(awk -v p=" $pkg:" '$0==p{f=1;next} /^ [a-z_]+:/{f=0} f' pubspec.lock) - - if printf '%s' "$block" | grep -qE '^\s+source: path'; then - echo "::error::pubspec.lock resolves $pkg from a local path." - echo "::error::Re-run: mv pubspec_overrides.yaml /tmp/ && flutter pub get && mv /tmp/pubspec_overrides.yaml ." - fail=1 - continue - fi - - locked=$(printf '%s' "$block" | grep -E '^\s+resolved-ref:' | head -1 | awk '{print $2}' | tr -d '"') - pinned=$(awk -v p=" $pkg:" '$0==p{f=1;next} /^ [a-z_]+:/{f=0} f' pubspec.yaml \ - | grep -E '^\s+ref:' | head -1 | awk '{print $2}' | tr -d '"') - - if [ -z "$locked" ] || [ -z "$pinned" ]; then - echo "::error::could not read a commit pin for $pkg (lock='$locked' pubspec='$pinned')." - fail=1 - elif [ "$locked" != "$pinned" ]; then - echo "::error::$pkg pin drift — pubspec.yaml says $pinned, pubspec.lock resolved $locked." - echo "::error::A release citing a sibling change would not actually contain it." - fail=1 - else - echo "$pkg pinned at $pinned, lock agrees." - fi - done - [ "$fail" = 0 ] + # This runs BEFORE `pub get` so it inspects the committed file. Shared + # with build.yml's `preflight` — a tag push never runs this job, so the + # guard has to live there too to actually protect a release. + - name: Guard — the lock pins sibling commits, and the pins aren't stale + run: bash .github/scripts/check_sibling_pins.sh # Sibling packages resolve from their pinned commit SHAs in pubspec.yaml. # There is no pubspec_overrides.yaml here — that file is gitignored and diff --git a/lib/ble/ble_engine.dart b/lib/ble/ble_engine.dart index 9697054..dbf6dfd 100644 --- a/lib/ble/ble_engine.dart +++ b/lib/ble/ble_engine.dart @@ -709,6 +709,16 @@ class BleEngine { double _wallSecs() => DateTime.now().millisecondsSinceEpoch / 1000.0; + // A monotonic clock for measuring ELAPSED durations — never for wall-clock + // comparisons. DateTime.now() (see _wallSecs above) can jump backward + // mid-measurement (DST fall-back, a manual or NTP time correction), which + // would silently disable a duration-based cap built on it (the + // auto-continue run ceiling in particular). Started once and never reset; + // callers diff two readings of it, same shape as _wallSecs so they compose + // with existing double-seconds call sites like AutoContinueRun's. + final Stopwatch _monotonic = Stopwatch()..start(); + double _monotonicSecs() => _monotonic.elapsedMicroseconds / 1e6; + // Wall-clock of the last BLE notification received on ANY characteristic. iOS // can resume the app with the peripheral still flagged "connected" while its // GATT notifications silently died during suspension — the UI reads connected @@ -2559,11 +2569,13 @@ class BleEngine { rowsPersistedThisSession: d.recordsThisOffload, lastTrimAdvanced: d.lastTrimAdvanced, consecutiveUnproductiveCount: _autoContinue.unproductiveStreak, - elapsedSeconds: _autoContinue.elapsed(_wallSecs()), + // Monotonic, not wall-clock: this ceiling must not un-arm itself if the + // phone's clock steps backward mid-chain. + elapsedSeconds: _autoContinue.elapsed(_monotonicSecs()), ); d.resetOffloadCounters(); if (cont) { - _autoContinue.continued(productive: productive, now: _wallSecs()); + _autoContinue.continued(productive: productive, now: _monotonicSecs()); _log('[SYNC] auto-continue — more backlog remains ' '(unproductive streak ${_autoContinue.unproductiveStreak}).'); await _triggerBackfill(BackfillTrigger.autoContinue); diff --git a/lib/data/db.dart b/lib/data/db.dart index bf19a82..33d11f7 100644 --- a/lib/data/db.dart +++ b/lib/data/db.dart @@ -4543,10 +4543,20 @@ class LocalDb { /// both are derived from `decoded_*` and rewritten whenever a day is /// re-derived. /// - /// [keepVersions] generations are retained, newest first. Keeping more than - /// one matters: a user on a GitHub release can roll back to the previous - /// build, and pruning down to only the current version would leave that build - /// with nothing to read. + /// [keepVersions] generations are retained per day_id, newest first. + /// Keeping more than one matters: a user on a GitHub release can roll back + /// to the previous build, and pruning down to only the current version + /// would leave that build with nothing to read for a day it never + /// re-derives (raw retention is 3 days; a day older than that only gets a + /// fresh-version row if something forces a re-derive). + /// + /// Scoped PER day_id, not table-wide. A table-wide "keep the 2 highest + /// versions present ANYWHERE" cutoff deletes a day's only cached + /// generation the moment any two OTHER days reach newer versions — not + /// when this day does — because a day whose raw substrate has already + /// aged out never re-enters the derive pipeline to write a newer row of + /// its own. That silently orphaned still-needed rows for days that can + /// never be re-derived. static Future pruneSupersededIntermediates({int keepVersions = 2}) async { if (keepVersions < 1) return 0; final db = await instance; @@ -4555,18 +4565,33 @@ class LocalDb { 'sleep_session_candidates', 'wake_day_features', ]) { - final versions = (await db.rawQuery( - 'SELECT DISTINCT algo_version FROM $table ORDER BY algo_version DESC', - )) - .map((r) => r['algo_version'] as int) - .toList(); - if (versions.length <= keepVersions) continue; - final cutoff = versions[keepVersions - 1]; - deleted += await db.delete( - table, - where: 'algo_version < ?', - whereArgs: [cutoff], + final rows = await db.rawQuery( + 'SELECT DISTINCT day_id, algo_version FROM $table', ); + final versionsByDay = >{}; + for (final r in rows) { + final day = r['day_id'] as String; + final v = r['algo_version'] as int; + (versionsByDay[day] ??= []).add(v); + } + // One transaction per table instead of one round-trip per day_id — + // right after a kAlgoVersion bump forces a bulk re-derive (or a user + // runs "Re-analyze data"), many days can cross the keepVersions + // threshold in the same pass, and un-batched deletes on iOS run under + // the same CPU-watchdog constraint the rest of derivation is careful + // about. + await db.transaction((txn) async { + for (final entry in versionsByDay.entries) { + final versions = entry.value..sort((a, b) => b.compareTo(a)); + if (versions.length <= keepVersions) continue; + final cutoff = versions[keepVersions - 1]; + deleted += await txn.delete( + table, + where: 'day_id = ? AND algo_version < ?', + whereArgs: [entry.key, cutoff], + ); + } + }); } return deleted; } diff --git a/pubspec.lock b/pubspec.lock index 49fef28..8d09bdf 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -964,8 +964,8 @@ packages: dependency: "direct main" description: path: "." - ref: f5ccae61cbc6a8083425ae0121b665deadcb421d - resolved-ref: f5ccae61cbc6a8083425ae0121b665deadcb421d + ref: "5d421918b5111f0158ccdfd9fa7f99eda1fa87b4" + resolved-ref: "5d421918b5111f0158ccdfd9fa7f99eda1fa87b4" url: "https://github.com/OpenStrap/analytics.git" source: git version: "1.0.0" @@ -973,8 +973,8 @@ packages: dependency: "direct main" description: path: "." - ref: a98cd7061346681db17a81844c300c46da118c02 - resolved-ref: a98cd7061346681db17a81844c300c46da118c02 + ref: "5bb8606964f2d94d9a73dc42c57416cb74282d52" + resolved-ref: "5bb8606964f2d94d9a73dc42c57416cb74282d52" url: "https://github.com/OpenStrap/protocol.git" source: git version: "1.0.0" diff --git a/pubspec.yaml b/pubspec.yaml index e2a6391..aba1ca9 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -35,24 +35,27 @@ dependencies: openstrap_protocol: git: url: https://github.com/OpenStrap/protocol.git - # Tip of protocol main — OpenStrap/protocol#19 merged: bounded R-R counts, - # NaN accel rejection, v7/v9/v12/v18 routed through parseR24 so a - # historical record keeps its OWN timestamp instead of collapsing to - # capture time, alarm form dispatch, bounded battery. - # Verified present: `git show :lib/src/live.dart | grep kKnownRecordVersions`. - ref: a98cd7061346681db17a81844c300c46da118c02 + # Tip of protocol main — OpenStrap/protocol#20 merged: the reassembler now + # checks the length-field crc8 before trusting it (a corrupted length byte + # used to consume up to 4092 bytes of good stream before this), and + # hexToBytes rejects odd-length hex instead of silently flooring it. This + # PR sat unpinned for a full day after merging — see fix/issues #22 for + # why (edge's own release pipeline shipped the pre-fix decoder). + # NOTE: this ref predates protocol fix/issues#21 (realtimeRr RR-bound, + # historical-family activity/steps_inc null-vs-0) — repin again once #21 + # merges to pick those up too. + # Verified present: `git show :lib/src/framing.dart | grep 'crc8(Uint8List'`. + ref: 5bb8606964f2d94d9a73dc42c57416cb74282d52 openstrap_analytics: git: url: https://github.com/OpenStrap/analytics.git - # Tip of analytics main — OpenStrap/analytics#30 merged: the - # abstain-over-fabricate sweep, plus the step/activity rebuild on a - # calibration-invariant feature (dailyStepEstimate's new signature, - # personalDynFloorFromDailySummaries, dailyDynSummary — all called by - # lib/compute/). + # Tip of analytics main — OpenStrap/analytics#31 merged: cardioStager's + # per-epoch robustZ scale is now computed once per night instead of + # re-sorted every epoch (reviewed for DST/incremental-drain staleness — + # none found; it's a pure perf hoist). # Verified present at THIS sha, not assumed from the PR being merged: - # steps.dart carries the new API, rr_correction.dart has the signed-dRR - # `seg.add(x[k])`, advanced_stager.dart has maxAccelCarryForwardSec. - ref: f5ccae61cbc6a8083425ae0121b665deadcb421d + # cardio_stager.dart builds a RobustScale once and robustZ delegates to it. + ref: 5d421918b5111f0158ccdfd9fa7f99eda1fa87b4 # BLE — flutter_blue_plus is the maintained cross-platform GATT client. flutter_blue_plus: ^1.36.8 diff --git a/test/db_storage_hygiene_test.dart b/test/db_storage_hygiene_test.dart index bde7a5c..199767f 100644 --- a/test/db_storage_hygiene_test.dart +++ b/test/db_storage_hygiene_test.dart @@ -86,4 +86,47 @@ void main() { test('pruning is a no-op when there is nothing superseded', () async { expect(await LocalDb.pruneSupersededIntermediates(), 0); }); + + test( + 'a day stuck on an old version (raw aged out, never re-derived) is not ' + 'orphaned just because OTHER days reached newer versions', () async { + final db = await LocalDb.instance; + // '2026-06-01' only ever got derived once, at v48 — its raw substrate is + // long gone (raw retention is 3 days) so it can never write a v49/v50 + // row of its own. Two unrelated recent days churn through v49 then v50. + await db.insert('sleep_session_candidates', { + 'day_id': '2026-06-01', + 'algo_version': 48, + 'payload_json': '{"stale":true}', + 'computed_at': 0, + }); + for (final v in const [49, 50]) { + for (final day in const ['2026-07-28', '2026-07-29']) { + await db.insert('sleep_session_candidates', { + 'day_id': day, + 'algo_version': v, + 'payload_json': '{}', + 'computed_at': 0, + }); + } + } + + await LocalDb.pruneSupersededIntermediates(); + + final stale = await LocalDb.sleepSessionCandidate('2026-06-01', 48); + expect(stale, isNotNull, + reason: + 'a table-wide "keep the 2 highest versions present anywhere" ' + 'cutoff would delete this the moment two OTHER days reach v49/50 ' + '— it must be scoped per day_id instead'); + expect(stale!['payload_json'], '{"stale":true}'); + + // The recent days still get their own per-day retention (49/50 kept, + // nothing older present to prune here). + final recent = (await db.rawQuery( + "SELECT DISTINCT algo_version FROM sleep_session_candidates " + "WHERE day_id = '2026-07-28' ORDER BY algo_version", + )).map((r) => r['algo_version'] as int).toList(); + expect(recent, [49, 50]); + }); }