From 2b1ad3477436e10919e42a0e443a4b34024a4948 Mon Sep 17 00:00:00 2001 From: fonkamloic Date: Wed, 26 Aug 2026 01:10:45 -0400 Subject: [PATCH 1/7] Write status before the single-flight early return in checkAndInstall checkAndInstall's contract is that every false return leaves its reason in CodePush.status. The single-flight early return (a second check while one is already running) was the one false return that wrote nothing, so a losing caller surfacing status after false rendered the in-flight check's foreign progress state as its own result. It now writes 'A check is already running' before returning. The write is transient by design: the in-flight check keeps overwriting status as it progresses, and ValueNotifier notifications are synchronous, so the 'Patch active' edge is always delivered before any overwrite. Adds a regression test in the single-flight group. Fixes #32 --- lib/src/code_push.dart | 13 ++++++++++++- test/hardening_test.dart | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/src/code_push.dart b/lib/src/code_push.dart index 91db55b..1dbff31 100644 --- a/lib/src/code_push.dart +++ b/lib/src/code_push.dart @@ -456,7 +456,18 @@ abstract final class CodePush { // is deliberately global (not per appId/channel): apps use a single // config, and a losing caller's `false` means "another check is // already running", not "no update". - if (_checkInFlight) return false; + // + // Write [status] before this early return too — checkAndInstall's + // contract is that every `false` return leaves its reason in [status], + // and without this write a losing caller would surface the OTHER + // check's in-flight state as if it were its own result. The in-flight + // check keeps overwriting [status] as it progresses, so this write is + // transient and cannot mask a 'Patch active' edge (notifications are + // synchronous, so that edge has already been delivered). + if (_checkInFlight) { + status.value = 'A check is already running'; + return false; + } _checkInFlight = true; try { print('[CP] checkAndInstall start'); diff --git a/test/hardening_test.dart b/test/hardening_test.dart index 20e2b68..c56c98a 100644 --- a/test/hardening_test.dart +++ b/test/hardening_test.dart @@ -169,6 +169,27 @@ void main() { expect(updateChecks, 1, reason: 'losers return before the network'); expect(downloads, 1); }); + + test('a losing overlapped check writes its own reason to status', + () async { + // Contract: checkAndInstall leaves its reason in CodePush.status + // before EVERY false return — including the single-flight early + // return, which previously left the other check's in-flight state + // in place (so a caller surfacing status after `false` rendered a + // foreign operation's progress as its own result). + offeredHash = patchHash; + offerDelay = const Duration(milliseconds: 300); + serve(); + + final winner = check(); + // Let the winner latch the guard and reach the (stalled) server. + await Future.delayed(const Duration(milliseconds: 50)); + + expect(await check(), isFalse); + expect(CodePush.status.value, 'A check is already running'); + + await winner; + }); }); group('download size cap', () { From 5e515284d610a71bb727fcb8c56c99f718997eb9 Mon Sep 17 00:00:00 2001 From: fonkamloic Date: Wed, 26 Aug 2026 01:10:45 -0400 Subject: [PATCH 2/7] Example: guard setState after every await with a mounted check The overlay re-keys the app subtree when a patch activates, which disposes the demo State mid-await; a setState landing after that throws in debug builds. Every setState that follows an await in _loadStatus, _manualCheck (including the async onUpdateReady callback and the catch branch), and _rollback (both catch branches) now bails out first when the State is no longer mounted. Pre-await synchronous setState calls need no guard and are unchanged; the early returns skip only UI updates on a disposed State, no side effects are lost. Fixes #33 --- example/lib/main.dart | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/example/lib/main.dart b/example/lib/main.dart index d291a6a..48dd09d 100644 --- a/example/lib/main.dart +++ b/example/lib/main.dart @@ -58,6 +58,11 @@ class _CodePushDemoState extends State { Future _loadStatus() async { final patched = await CodePush.isPatched; final patch = await CodePush.currentPatch; + // Guard every setState that follows an await: the overlay re-keys the + // app subtree when a patch activates (see the build-method note below), + // which disposes this State mid-await — a late setState would then + // throw in debug builds. + if (!mounted) return; setState(() { _isPatched = patched; _currentPatch = patch; @@ -72,9 +77,13 @@ class _CodePushDemoState extends State { appId: 'your-app-id', releaseVersion: '1.0.0+1', onUpdateReady: () { + // This callback fires long after the await below resumes, so it + // needs its own mounted guard. + if (!mounted) return; setState(() => _status = 'Patch installed! Restart to apply.'); }, ); + if (!mounted) return; if (!installed) { // `false` is not just "no update" — checkAndInstall also returns false // for "a check is already running", a download failure, a hash @@ -86,6 +95,7 @@ class _CodePushDemoState extends State { } await _loadStatus(); } on CodePushException catch (e) { + if (!mounted) return; setState(() => _status = 'Error: ${e.message}'); } } @@ -94,15 +104,18 @@ class _CodePushDemoState extends State { setState(() => _status = 'Rolling back...'); try { await CodePush.rollback(); + if (!mounted) return; setState(() => _status = 'Rolled back. Restart to revert.'); await _loadStatus(); } on CodePushException catch (e) { + if (!mounted) return; setState(() => _status = 'Rollback failed: ${e.message}'); } catch (e) { // The iOS Dart-side rollback deletes the resident patch file, which can // throw a FileSystemException (not a CodePushException) if it is already // gone or unreadable — catch it so the button never leaves an exception // unhandled. + if (!mounted) return; setState(() => _status = 'Rollback failed: $e'); } } From 9f546e220ca95d27cb3d31ed5b74ebaf5fe940dd Mon Sep 17 00:00:00 2001 From: fonkamloic Date: Thu, 27 Aug 2026 22:38:26 -0400 Subject: [PATCH 3/7] Write terminal statuses after best-effort awaits; deterministic guard test Review round 1: the four sites where a check writes a terminal error status and then awaits a best-effort call (telemetry POST / rollback) before returning false now write the status after the await, so a concurrent check's 'A check is already running' write during that window cannot replace the actionable message. Audited all other status writes: no other site has the write-await-return-false shape. Also: the single-flight regression test no longer depends on wall-clock timing (the guard and the loser's status write both land before the first await, so both calls run in one synchronous turn); the example's onUpdateReady comment now states the real reason for the mounted guard (the callback is synchronous, but the overlay can re-key the subtree mid-check); CHANGELOG's Unreleased section now notes the behavior change instead of claiming doc-only changes. --- CHANGELOG.md | 4 +++- example/lib/main.dart | 6 ++++-- lib/src/code_push.dart | 29 ++++++++++++++++++++++------- test/hardening_test.dart | 11 +++++++---- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14bbb23..2faed70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ ## Unreleased -- Documentation fixes only; no API or behavior changes. +- A concurrent `checkAndInstall` call no longer overwrites the active check's + status message; the overlapping call reports its own reason in + `CodePush.status`. - `CodePushOverlay.bannerBuilder`: return a widget to show no banner (`const SizedBox.shrink()`) — it never accepted `null`. Keep side effects out of the builder; wire the handed `onRestart`/`onDismiss` to your UI instead. diff --git a/example/lib/main.dart b/example/lib/main.dart index 48dd09d..31c02cd 100644 --- a/example/lib/main.dart +++ b/example/lib/main.dart @@ -77,8 +77,10 @@ class _CodePushDemoState extends State { appId: 'your-app-id', releaseVersion: '1.0.0+1', onUpdateReady: () { - // This callback fires long after the await below resumes, so it - // needs its own mounted guard. + // checkAndInstall invokes this synchronously, before the await + // below resumes — but the overlay can re-key this subtree while + // the check is in flight, so the callback can land on a + // disposed State. Hence the mounted guard. if (!mounted) return; setState(() => _status = 'Patch installed! Restart to apply.'); }, diff --git a/lib/src/code_push.dart b/lib/src/code_push.dart index 1dbff31..d052dc6 100644 --- a/lib/src/code_push.dart +++ b/lib/src/code_push.dart @@ -618,7 +618,6 @@ abstract final class CodePush { ); if (actualEngineFingerprint == null) { - status.value = 'Incompatible baseline: engine has no code push support'; await _reportIncompatibleBaseline( serverUrl: serverUrl, appId: appId, @@ -628,6 +627,12 @@ abstract final class CodePush { expectedFingerprint: expectedEngineFingerprint, actualFingerprint: null, ); + // Terminal status is written AFTER the awaited report: a + // concurrent check() during that POST stamps 'A check is + // already running', and this actionable message must be what + // stands when we return. It's an error state, not progress, so + // writing it last loses nothing. + status.value = 'Incompatible baseline: engine has no code push support'; return false; } @@ -638,8 +643,6 @@ abstract final class CodePush { if (expectedEngineFingerprint != null && actualEngineFingerprint != 'unknown' && expectedEngineFingerprint != actualEngineFingerprint) { - status.value = 'Incompatible baseline: engine ABI mismatch ' - '($actualEngineFingerprint vs $expectedEngineFingerprint)'; await _reportIncompatibleBaseline( serverUrl: serverUrl, appId: appId, @@ -648,6 +651,11 @@ abstract final class CodePush { expectedFingerprint: expectedEngineFingerprint, actualFingerprint: actualEngineFingerprint, ); + // After the await, so a concurrent check()'s guard write can't + // overwrite this terminal status (see the null-fingerprint + // branch above). + status.value = 'Incompatible baseline: engine ABI mismatch ' + '($actualEngineFingerprint vs $expectedEngineFingerprint)'; return false; } @@ -1917,15 +1925,19 @@ abstract final class CodePush { 'bytes=${container.length}', ); } - status.value = 'Patch format is unexpected — rolling back. ' - 'Upgrade flutter_compile to the latest version and ' - 'rebuild the patch.'; await _iosImmediateRollback( serverUrl: serverUrl, appId: appId, patchId: patchId, errorMessage: 'Patch format mismatch — rejected before load', ); + // Written after the awaited rollback (file deletes + telemetry + // POST): a concurrent check() during that window stamps 'A + // check is already running', and this upgrade hint must be the + // status that stands when we return. + status.value = 'Patch format is unexpected — rolling back. ' + 'Upgrade flutter_compile to the latest version and ' + 'rebuild the patch.'; return false; } @@ -2004,7 +2016,6 @@ abstract final class CodePush { ); } print('[CP] MODULE LOAD THREW ($origin) — $e'); - status.value = 'Module error: $e — rolling back patch'; // Real load failure. Delete immediately instead of waiting for // the three-strike auto-rollback. await _iosImmediateRollback( @@ -2013,6 +2024,10 @@ abstract final class CodePush { patchId: patchId, errorMessage: 'Patch load threw $e — deleted immediately', ); + // Terminal status goes after the awaited rollback so a + // concurrent check()'s 'A check is already running' write during + // that window can't be what callers observe on return. + status.value = 'Module error: $e — rolling back patch'; return false; } } diff --git a/test/hardening_test.dart b/test/hardening_test.dart index c56c98a..fc81b46 100644 --- a/test/hardening_test.dart +++ b/test/hardening_test.dart @@ -181,12 +181,15 @@ void main() { offerDelay = const Duration(milliseconds: 300); serve(); + // Both calls in the same synchronous turn: the winner latches the + // guard before its first await, and the loser's status write also + // lands before ITS first await — so no wall-clock sequencing is + // needed (a delay could even over-sleep past the offer window and + // turn the "loser" into a real second check). final winner = check(); - // Let the winner latch the guard and reach the (stalled) server. - await Future.delayed(const Duration(milliseconds: 50)); - - expect(await check(), isFalse); + final loser = check(); expect(CodePush.status.value, 'A check is already running'); + expect(await loser, isFalse); await winner; }); From 1007381f50b2332c211d246783740be4ab266749 Mon Sep 17 00:00:00 2001 From: fonkamloic Date: Thu, 27 Aug 2026 22:43:50 -0400 Subject: [PATCH 4/7] Correct CHANGELOG claim and status-ordering comments to match real behavior Review round 2: - CHANGELOG: the entry claimed a concurrent checkAndInstall no longer overwrites the active check's status message, but the loser's 'A check is already running' stamp does transiently replace the winner's in-progress status until its next write. Reworded to state what is actually guaranteed: the overlapping call reports its own reason instead of surfacing the other check's state. - The two status-reorder comments on the payload-load error paths claimed the terminal message 'must be what stands when we return'. That guarantee does not hold there: the install path returns the load future without awaiting it (releasing the single-flight guard before the rollback completes) and the cold-boot/reload callers never take the guard, so a concurrent full check can still overwrite the message later. The reorder stays (it is still the last write this method makes); the comments now describe it as best-effort ordering, not a guarantee. --- CHANGELOG.md | 6 +++--- lib/src/code_push.dart | 16 ++++++++++------ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2faed70..7f2ad5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,8 @@ ## Unreleased -- A concurrent `checkAndInstall` call no longer overwrites the active check's - status message; the overlapping call reports its own reason in - `CodePush.status`. +- Overlapping `checkAndInstall` calls now report their own reason + ("A check is already running") in `CodePush.status` instead of surfacing the + other check's state. - `CodePushOverlay.bannerBuilder`: return a widget to show no banner (`const SizedBox.shrink()`) — it never accepted `null`. Keep side effects out of the builder; wire the handed `onRestart`/`onDismiss` to your UI instead. diff --git a/lib/src/code_push.dart b/lib/src/code_push.dart index d052dc6..2f49bbe 100644 --- a/lib/src/code_push.dart +++ b/lib/src/code_push.dart @@ -1932,9 +1932,12 @@ abstract final class CodePush { errorMessage: 'Patch format mismatch — rejected before load', ); // Written after the awaited rollback (file deletes + telemetry - // POST): a concurrent check() during that window stamps 'A - // check is already running', and this upgrade hint must be the - // status that stands when we return. + // POST) so this upgrade hint is the last write this method + // makes. The single-flight guard is not held here (the install + // path returns this future without awaiting it, and cold-boot / + // reload callers never take the guard), so a concurrent full + // check can still overwrite it later — best-effort ordering, + // not a guarantee. status.value = 'Patch format is unexpected — rolling back. ' 'Upgrade flutter_compile to the latest version and ' 'rebuild the patch.'; @@ -2024,9 +2027,10 @@ abstract final class CodePush { patchId: patchId, errorMessage: 'Patch load threw $e — deleted immediately', ); - // Terminal status goes after the awaited rollback so a - // concurrent check()'s 'A check is already running' write during - // that window can't be what callers observe on return. + // Terminal status goes after the awaited rollback so it is the + // last write this method makes. The single-flight guard is not + // held on this path, so a concurrent full check can still + // overwrite it later — best-effort ordering, not a guarantee. status.value = 'Module error: $e — rolling back patch'; return false; } From 040614b92be8c820707248b3c7f7357c216f7d01 Mon Sep 17 00:00:00 2001 From: fonkamloic Date: Thu, 27 Aug 2026 23:06:36 -0400 Subject: [PATCH 5/7] Kill switch: write the terminal status after the revert await in the catch path too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The success path already rewrote status after the awaited revert, but the catch path — the majority-fleet path, since unpatched devices have nothing to roll back — kept only the pre-await write, which a concurrent check's single-flight status could overwrite during the revert's async gap. Move the write into both post-await branches and pin the ordering with a deterministic concurrent-check test. --- lib/src/code_push.dart | 8 ++++++-- test/ota_controls_test.dart | 29 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/lib/src/code_push.dart b/lib/src/code_push.dart index 2f49bbe..a27646c 100644 --- a/lib/src/code_push.dart +++ b/lib/src/code_push.dart @@ -518,7 +518,6 @@ abstract final class CodePush { // any installed patch, so the fleet returns to the store baseline // within one check interval of the switch being flipped. if (data['ota_disabled'] == true) { - status.value = 'OTA disabled by server'; try { // Unconditional: the rollback knows where each platform keeps // the patch (engine on Android/desktop, file removal on iOS @@ -531,7 +530,12 @@ abstract final class CodePush { status.value = 'OTA disabled by server — patch removed (restart to apply)'; } catch (_) { - // Best-effort: a clean device has nothing to revert. + // Best-effort: a clean device has nothing to revert. The + // status is written AFTER the awaited rollback attempt (in + // both branches): a concurrent check() during that await + // stamps 'A check is already running', and the kill-switch + // signal must be what stands when we return. + status.value = 'OTA disabled by server'; } return false; } diff --git a/test/ota_controls_test.dart b/test/ota_controls_test.dart index 50bff4f..ea3ff22 100644 --- a/test/ota_controls_test.dart +++ b/test/ota_controls_test.dart @@ -78,6 +78,35 @@ void main() { expect(CodePush.status.value, isNot(contains('patch removed'))); }); + test( + 'clean device: kill-switch status survives a concurrent check ' + 'during the revert attempt', () async { + serveOtaDisabled(); + Future? loser; + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(engineChannel, (MethodCall call) async { + if (call.method == 'CodePush.rollback') { + // The winner is parked on the revert await with the + // single-flight guard held — exactly the window where a + // concurrent check stamps its loser status. The kill-switch + // branch must rewrite AFTER this await so the fleet-wide + // signal is what stands when the winner returns. + loser ??= check(); + return false; // Unpatched device: the revert attempt no-ops. + } + return null; + }); + + final installed = await check(); + + expect(installed, isFalse); + expect(loser, isNotNull); + expect(await loser, isFalse); + expect(CodePush.status.value, contains('OTA disabled')); + expect(CodePush.status.value, isNot(contains('patch removed'))); + expect(CodePush.status.value, isNot(contains('already running'))); + }); + test('patched device: reverts to the store baseline', () async { serveOtaDisabled(); final engineCalls = []; From c8ed9b2872608d8f4534942b699de2dad8451bae Mon Sep 17 00:00:00 2001 From: fonkamloic Date: Thu, 27 Aug 2026 23:23:07 -0400 Subject: [PATCH 6/7] Round 4 Lows: reordered-write regression test, loser-stamp guard, status contract docs - New hardening test pins the reordered-write class at its cheapest site: a concurrent check() fired while the winner is parked on the awaited incompatible-baseline telemetry POST cannot overwrite the terminal 'Incompatible baseline: engine ABI mismatch' status (verified red on the pre-fix ordering). - The kill-switch concurrency test now pins that the loser actually stamped 'A check is already running' inside the revert window. The status is captured in the mock handler and asserted in the test body: an expect() thrown inside the handler is swallowed by the kill-switch branch's own catch around the rollback await (verified empirically), so the inline form would itself be vacuous. - Docs now state the enforced contract: checkAndInstall's dartdoc says every false return leaves its reason as that check's final status write and a losing concurrent call stamps its own status without disturbing the active check's terminal message; README lists 'A check is already running' among the status values; both the status dartdoc and README reconcile the transition-not-level wording with the ordering guarantee (which transition a check ends on, not a level). --- README.md | 10 ++++++++- lib/src/code_push.dart | 13 +++++++++++ test/hardening_test.dart | 44 +++++++++++++++++++++++++++++++++++++ test/ota_controls_test.dart | 8 +++++++ 4 files changed, 74 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 0debc28..ae37659 100644 --- a/README.md +++ b/README.md @@ -245,7 +245,15 @@ CodePush.status.addListener(() { ``` Values include: `init`, `Checking server...`, `Downloading patch...`, -`Patch active`, `No update (204)`, `Restart to apply`, etc. +`Patch active`, `No update (204)`, `Restart to apply`, +`A check is already running`, etc. + +One ordering guarantee holds across those transitions: when `checkAndInstall` +returns `false`, the reason is that check's **final** `status` write, so +reading the value immediately after the `await` reliably explains the result. +A concurrent (losing) call stamps `A check is already running` without +disturbing the active check's terminal message. The guarantee is about which +transition a check ends on — the value still does not stay put afterwards. Don't use this as an app-facing "a patch loaded" signal. `CodePushOverlay` latches the `Patch active` edge internally, but on that transition it re-keys diff --git a/lib/src/code_push.dart b/lib/src/code_push.dart index a27646c..03d3217 100644 --- a/lib/src/code_push.dart +++ b/lib/src/code_push.dart @@ -163,6 +163,12 @@ abstract final class CodePush { /// `'Patch already installed'` at the next check, and again on every resume), /// so it is a fleeting TRANSITION, not a level you can poll. /// + /// One ordering guarantee holds across those transitions: when + /// [checkAndInstall] returns `false`, its reason was that check's final + /// write, so reading the value right after the `await` is reliable — the + /// guarantee is about WHICH transition a check ends on, not about the + /// value staying put afterwards. + /// /// Do NOT use it as an app-facing "a patch loaded" signal. [CodePushOverlay] /// latches the `'Patch active'` edge internally, but on that same transition /// it re-keys its child subtree — disposing any latch a widget under it holds @@ -442,6 +448,13 @@ abstract final class CodePush { /// Checks the server for updates, downloads and installs if available. /// /// Returns `true` if a patch was installed (restart needed). + /// + /// Status contract: every `false` return leaves its reason as the FINAL + /// [status] write of that check, so reading [status] immediately after the + /// `await` reliably explains the result (a later check will overwrite it). + /// A concurrent call loses the single-flight guard: it stamps + /// `'A check is already running'` and returns `false` without disturbing + /// the active check's terminal message. static Future checkAndInstall({ required String serverUrl, required String appId, diff --git a/test/hardening_test.dart b/test/hardening_test.dart index fc81b46..b699e0f 100644 --- a/test/hardening_test.dart +++ b/test/hardening_test.dart @@ -23,6 +23,8 @@ void main() { Duration offerDelay = Duration.zero; String? offeredHash; String? offeredUrlOverride; + String? offeredEngineFingerprint; + Future Function()? onTelemetry; void serve() { server.listen((HttpRequest req) async { @@ -36,9 +38,14 @@ void main() { 'patch_available': true, 'patch_id': 'p1', if (offeredHash != null) 'patch_hash': offeredHash, + if (offeredEngineFingerprint != null) + 'engine_fingerprint': offeredEngineFingerprint, 'patch_url': offeredUrlOverride ?? 'http://127.0.0.1:${server.port}/patch', })); + } else if (req.uri.path.endsWith('/telemetry/client-error')) { + if (onTelemetry != null) await onTelemetry!(); + req.response.statusCode = HttpStatus.ok; } else { downloads++; req.response.statusCode = HttpStatus.ok; @@ -55,6 +62,8 @@ void main() { offerDelay = Duration.zero; offeredHash = null; offeredUrlOverride = null; + offeredEngineFingerprint = null; + onTelemetry = null; CodePush.debugResetBaselineHashCache(); TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(engineChannel, (MethodCall call) async { @@ -193,6 +202,41 @@ void main() { await winner; }); + + test( + 'incompatible-baseline terminal status survives a concurrent check ' + 'during the telemetry POST', () async { + // Regression guard for the reordered terminal writes: the engine + // ABI-mismatch branch reports telemetry (awaited) BEFORE writing + // its terminal status. A concurrent check() arriving inside that + // await stamps 'A check is already running'; the terminal write + // must land AFTER the await so the actionable message is what + // stands when the winner returns. Moving that write back up next + // to its branch condition turns this test red. + offeredHash = patchHash; + offeredEngineFingerprint = 'flutter-9.9.9'; // mocked engine: 'abi' + Future? loser; + String? statusDuringReport; + onTelemetry = () async { + // The winner is parked on the awaited telemetry POST with the + // single-flight guard held — exactly the reordered-write window. + loser ??= check(); + statusDuringReport = CodePush.status.value; + }; + serve(); + + expect(await check(), isFalse); + expect(loser, isNotNull, reason: 'the telemetry hook must have fired'); + expect(await loser, isFalse); + expect(statusDuringReport, 'A check is already running', + reason: 'the loser must have stamped its status inside the ' + 'window, or this test is not exercising the race'); + expect( + CodePush.status.value, + startsWith('Incompatible baseline: engine ABI mismatch'), + ); + expect(downloads, 0, reason: 'mismatch is refused before download'); + }); }); group('download size cap', () { diff --git a/test/ota_controls_test.dart b/test/ota_controls_test.dart index ea3ff22..0c94f3e 100644 --- a/test/ota_controls_test.dart +++ b/test/ota_controls_test.dart @@ -83,6 +83,7 @@ void main() { 'during the revert attempt', () async { serveOtaDisabled(); Future? loser; + String? statusDuringRevert; TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger .setMockMethodCallHandler(engineChannel, (MethodCall call) async { if (call.method == 'CodePush.rollback') { @@ -92,6 +93,10 @@ void main() { // branch must rewrite AFTER this await so the fleet-wide // signal is what stands when the winner returns. loser ??= check(); + // Captured (not expect()ed) here: a TestFailure thrown inside + // this handler is swallowed by the kill-switch catch around + // the rollback await; the body assertion below fails loudly. + statusDuringRevert = CodePush.status.value; return false; // Unpatched device: the revert attempt no-ops. } return null; @@ -101,6 +106,9 @@ void main() { expect(installed, isFalse); expect(loser, isNotNull); + expect(statusDuringRevert, contains('already running'), + reason: 'the loser must actually have stamped its status inside ' + 'the revert window — otherwise this test is vacuous'); expect(await loser, isFalse); expect(CodePush.status.value, contains('OTA disabled')); expect(CodePush.status.value, isNot(contains('patch removed'))); From 6c8f800de5ab99be1559178c72214349536051c8 Mon Sep 17 00:00:00 2001 From: fonkamloic Date: Thu, 27 Aug 2026 23:33:34 -0400 Subject: [PATCH 7/7] Docs: qualify the status contract for the iOS post-download load path The round-5 review is right that the new dartdoc and README stated the final-write guarantee unconditionally while the code's own comments (and the guard-release at the un-awaited install return) document the iOS post-download load path as best-effort ordering. Add the qualifying clause to both doc blocks so the public docs agree with the code. --- README.md | 3 +++ lib/src/code_push.dart | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ae37659..406707b 100644 --- a/README.md +++ b/README.md @@ -254,6 +254,9 @@ reading the value immediately after the `await` reliably explains the result. A concurrent (losing) call stamps `A check is already running` without disturbing the active check's terminal message. The guarantee is about which transition a check ends on — the value still does not stay put afterwards. +The one exception is the iOS post-download load path, where the guard is +released before the load completes: its writes are best-effort ordering, +and a concurrent full check may overwrite them. Don't use this as an app-facing "a patch loaded" signal. `CodePushOverlay` latches the `Patch active` edge internally, but on that transition it re-keys diff --git a/lib/src/code_push.dart b/lib/src/code_push.dart index 03d3217..9d879ce 100644 --- a/lib/src/code_push.dart +++ b/lib/src/code_push.dart @@ -454,7 +454,10 @@ abstract final class CodePush { /// `await` reliably explains the result (a later check will overwrite it). /// A concurrent call loses the single-flight guard: it stamps /// `'A check is already running'` and returns `false` without disturbing - /// the active check's terminal message. + /// the active check's terminal message — except on the iOS post-download + /// load path, where the guard is released before the load completes, so + /// its writes are best-effort ordering rather than a guarantee (a + /// concurrent full check may overwrite them). static Future checkAndInstall({ required String serverUrl, required String appId,