159 Commits

Author	SHA1	Message	Date
Tony	479de106d7	Merge remote-tracking branch 'upstream/main'	2026-08-03 01:57:04 +08:00
dependabot[bot]	1f85c4ab1e	chore(deps): update mockall requirement (#365) ... Updates the requirements on [mockall](https://github.com/asomers/mockall) to permit the latest version. Updates `mockall` to 0.15.0 - [Changelog](https://github.com/asomers/mockall/blob/master/CHANGELOG.md) - [Commits](https://github.com/asomers/mockall/compare/v0.14.0...v0.15.0) --- updated-dependencies: - dependency-name: mockall dependency-version: 0.15.0 dependency-type: direct:production dependency-group: library-deps ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-07-02 18:30:09 -07:00
Mac	dd213f923c	fix: populate current_patch_number from running_patch, not the boot breadcrumb (#363) ... * fix: send running_patch as current_patch_number on patch check Both patch-check construction sites read current_patch_number from state.currently_booting_patch(), a transient boot-handshake breadcrumb that record_boot_success() clears to None before any Dart-initiated patch check fires. The field was therefore omitted from the wire for essentially the entire fleet. Use state.running_patch() instead: the session-scoped accessor set at report_launch_start, kept for the whole session, surviving server-driven rollbacks. This is the same value the FFI symbol shorebird_current_boot_patch_number already exposes. * test: assert patch-check request carries running_patch as current_patch_number Covers both construction sites (check_for_downloadable_update and update). Reproduces the steady state where the bug manifested — patch booted, currently_booting_patch cleared by record_boot_success — and asserts the request still carries the running patch number. Verified to fail (field omitted, captured as -1) against the pre-fix currently_booting_patch() code. * test: cover all changed lines in patch-check regression test Codecov's patch gate flagged 13 uncovered lines in the new test (78.7% patch coverage), none of them the fix: the no-op download/report closures never fired, and multi-line assert failure messages are only reached when an assertion panics. Swap the closures for the existing UNEXPECTED_DOWNLOAD/UNEXPECTED_REPORT placeholders (also asserting that path is never hit), collapse the assertions to single lines with the sentinel legend moved to comments, and tighten the module doc. Changed lines now at 100% patch coverage; fmt, clippy, and the full suite stay green.	2026-06-29 18:27:15 -06:00
Tony	6e9aab2ce7	Point updater docs at GitHub Flutter fork	2026-06-26 01:40:25 +08:00
Tony	5d4e9c3396	refactor: improve comments for clarity and update privacy references ... Signed-off-by: Tony <tonylu@tony-cloud.com>	2026-06-25 15:50:24 +08:00
Tony	3ac748ff28	feat: add device ID override functionality to ShorebirdUpdater ... - Introduced `setDeviceIdOverride` method in `ShorebirdUpdater` to allow clients to set a custom device ID for patch checks. - Implemented the method in `ShorebirdUpdaterImpl` for both IO and web platforms. - Updated the `Updater` class to handle the device ID override in native bindings. - Added tests for the new functionality in both IO and web test suites, ensuring proper behavior when the updater is available and unavailable.	2026-06-24 03:02:58 +08:00
Brandon DeRosier	fe3733374c	fix(inflate): preserve real upstream cause + add diagnostic byte counters (#358) ... * fix(inflate): preserve real upstream cause + add diagnostic byte counters The current `inflate()` always reports "Decompression of patch failed" when the decompression thread's join returns Err -- but that masks the common in-the-wild failure shape where bipatch (or the output write) errors first, `fresh_r` gets dropped, and the decompression thread's next write returns BrokenPipe purely as downstream noise. The misleading message has made hard-to-reproduce field reports very hard to diagnose. Two changes: 1. Error attribution. When both `patch_result` and `decompress_result` error, `patch_result` is now reported as the primary cause (the upstream one in the BrokenPipe shape) and the decompression error is folded into the context chain so a genuine zstd corruption error doesn't disappear. The `(Ok, Err)` case (decompression trailing error after bipatch already finished, e.g. extra zstd frame bytes) is now treated as success with an info-level log -- the output was produced and will be hash-checked downstream. 2. Diagnostic counters. Every error path now carries a one-line `InflateDiagnostics` snapshot in its context. The fields cover the whole pipeline: patch_file=<size> -- on-disk compressed patch size base_size=<size> -- base reader length (probed at start) decompressed_into_pipe=<bytes> -- bytes the zstd thread wrote decompressed_out_of_pipe=<bytes> -- bytes bipatch consumed base_read=<bytes> -- bytes bipatch read from base base_last_seek_pos=<offset> -- where base reader last seeked base_seeks=<count> -- how many times bipatch seeked the base output_written=<bytes> -- bytes copied to output Concretely: a field crash where `base_read=0` immediately implicates the base reader; `decompressed_into_pipe=8 output_written=0` implicates bipatch parsing right after the header; a `base_size` that disagrees with what the host extracted while building the patch implicates iOS install-time munging of the snapshot bytes. These counters are tracked via Arc<AtomicU64> shared between the inflate body and the decompression thread, wrapped around each stream end with three small Read/Seek/Write adapter structs. Bipatch initialization failure also drains and surfaces any concurrent decompression error (previously the thread leaked detached on this path). Tests updated/added: - `inflate_fails_with_corrupt_zstd_data_reports_bipatch_init_failure` (replaces `inflate_fails_with_corrupt_zstd_data`): garbage zstd body now surfaces as bipatch-init failure with full counters. - `inflate_when_both_streams_error_reports_patch_side_primary` (replaces `inflate_reports_decompression_error_as_primary`): valid bipatch header + corrupt second zstd frame -- patch_result wins, decompression error is in the context chain. - New `inflate_diagnostics_include_all_counters`: pins the diagnostic format contract so future refactors can't accidentally drop a field that in-the-wild bug reports depend on. Motivation: digging into a reproducible iOS-standalone patch failure where the only on-device log line was "Update failed: Decompression of patch failed" -- which turned out to be downstream noise. With these counters the next investigator can read the failing pipeline state directly from the device log. * fix(inflate): keep success-path logging at original chattiness Move the success-case diagnostic counters from `shorebird_info!` to `shorebird_debug!` so production doesn't grow a ~200-character diagnostic line on every successful patch install. The happy path now emits the same single short "Patch successfully applied to X" info line as before; counters land at debug level for development / opt-in production diagnostics. Also demoted the "Base file size: X bytes" line that I'd added at info level back to debug. Error-path diagnostics are unchanged — those fire only on failure where the byte counts are exactly what the next investigator wants. Net effect on production log volume: zero new info-level lines per inflate (matches pre-PR shape). Failure paths retain full diagnostic counters. * fix(inflate): restore success-path diagnostic blob at info level Verified offline that the concern about "hundreds of lines of noise" isn't the right shape here: it's a single info-level line per inflate (rare event, maybe daily per user), just with the byte counters appended. Same line count as before this PR; only the line length grows. Keeping the success blob at info enables cross-run forensic comparison — e.g. "did base_size change between this user's last working patch and the broken one?" — which is exactly the kind of question the in-the-wild bug report is going to want answered. * fix(inflate): cargo fmt	2026-05-14 11:59:12 -07:00
Brandon DeRosier	d3a161d14c	feat: report diagnostic events for failed patch updates (#347) ... * feat: report diagnostic events for failed patch updates Download failures, inflate errors, hash mismatches, and install failures were previously invisible — no event was sent to the server when these occurred. Users who never received a patch were undetectable. Add a new `__patch_update_failure__` event type that is queued whenever the update pipeline fails after a patch is available. The message includes the truncated error string (max 256 chars) for diagnosing root causes like network errors, decompression failures, size mismatches, and hash validation failures. The event is queued (not sent inline) so it doesn't block the update flow, and will be delivered on the next successful update check. * test: add coverage for update failure events and message truncation - Verify PatchUpdateFailure event is queued on download size mismatch - Add tests for truncate_error_message with short and long errors * test: add event type serialization round-trip and unknown type tests * refactor: omit boot_started_at field when unknown Per review feedback, drop the "unknown" placeholder and omit the field entirely so consumers don't need to special-case a sentinel value.	2026-05-11 18:37:25 -07:00
Eric Seidel	90c349287d	refactor: per-patch lifecycle state machine (#352) ... * refactor: introduce per-patch lifecycle state machine (types + persistence) First slice of shorebirdtech/shorebird#3737 — replace the scattered storage of per-patch state across download_state.rs sidecars, bare files in downloads/, and PatchesState fields (next_boot_patch, last_booted_patch, known_bad_patches) with a single per-patch state.json driven by an explicit state machine. This commit only adds the types and the storage layer; nothing wires into update_internal or report_launch_* yet. Subsequent commits will build out transitions and migrate the call sites. States: - Downloading { url, hash, signature, partial_size } - Downloaded { url, hash, signature, size } - Installed { hash, signature, size } - Bad { reason, hash?, signature?, size? } // tombstone Operations: - mark_bad(n, reason) — sugar over write Bad{} + cleanup - cleanup(n) — state-aware: keeps Bad tombstone, else forgets dir Per-release pointers (next_boot, last_booted, currently_booting, boot_started_at) move to a separate pointers.json holding patch numbers; metadata lives once per patch in state.json instead of duplicated across pointers. Persistence rides on the existing atomic disk_io::write (sibling- write + rename) so partial writes can't leave torn files. * refactor: add download/install/boot transitions to lifecycle Extends the lifecycle module with the methods update_internal and report_launch_* will need: - decide_start(n, url, hash) → DownloadAction (Fresh, Resume, Complete, Skip) - record_download_started / _complete - record_install_complete (transitions Downloaded → Installed, removes the now-unneeded compressed download file) - promote_to_next_boot (transitions a freshly Installed patch into pointers.next_boot, retiring any unbooted predecessor) - record_boot_start / _success / _failure - detect_boot_crash_on_init (handles the breadcrumb left when a prior process crashed during boot) - validate_next_boot_patch (size + signature checks; marks Bad{ValidationFailed} on failure) - recompute_next_boot Nothing wires into the existing updater.rs / report_launch_* yet — those changes follow in the cutover commit. * refactor: replace UpdaterState's PatchManager backend with PatchLifecycle Drops the patch_manager dependency from updater_state.rs. UpdaterState now owns a PatchLifecycle directly, with all patch-related methods delegating to it: install_patch → write Installed state + promote to next_boot next_boot_patch → pointers.next_boot_patch + installed_artifact_path is_known_bad_patch → read_state matches Bad uninstall_patch → cleanup + recompute_next_boot record_boot_* → lifecycle's boot transitions validate_next_boot_patch → lifecycle's signature/size check UpdaterState's own state.json shrinks to {client_id, release_version, queued_events}. Per-release per-patch state lives entirely in the lifecycle (pointers.json + patches/{N}/state.json). Other notable behavior changes: - record_boot_failure_for_patch no longer requires currently_booting_patch to match. Matches the prior PatchManager semantics: clear breadcrumb, mark Bad{BootCrash}, recompute. - recompute_next_boot leaves a valid Installed next_boot_patch alone instead of promoting last_booted over it. Without this, processing server rollbacks would clobber a freshly-installed newer patch. Tests in updater.rs that asserted file paths (patches_state.json) update to the new layout (pointers.json). The MockManagePatches-backed unit tests in updater_state.rs are gone — replaced by direct end-to-end tests that exercise PatchLifecycle through UpdaterState. patch_manager.rs is still on disk and compiled (nothing references it from cache/mod.rs anymore) — deletion happens after the updater.rs cutover. * refactor: cut update_internal over to PatchLifecycle Replaces the download_state.rs sidecar machinery and the bespoke DownloadStartState / should_install_patch helpers in update_internal with calls into PatchLifecycle. Concrete changes: - Drops compute_resume_offset / determine_download_start_state / DownloadStartState — replaced by PatchLifecycle::decide_start returning DownloadAction. - Drops should_install_patch / ShouldInstallPatchCheckResult — folded into the same DownloadAction match. KnownBad → UpdateIsBadPatch, AlreadyInstalled → NoUpdate, the rest proceed. - Drops install_downloaded_patch's bespoke "stage in downloads/, rename on install" choreography. The lifecycle owns the per-patch directory; inflate writes directly into patches/{N}/dlc.vmcode and the Downloaded → Installed transition removes the now-unneeded compressed download file. - Drops cleanup_download_artifacts and clean_download_dir. mark_bad handles tombstone-aware cleanup for failures, and the lifecycle owns its directory. - Marks the patch Bad{InvalidPatchBytes} when inflate fails and Bad{InstallHashMismatch} when check_hash fails. Subsequent attempts short-circuit at decide_start (Skip(KnownBad)) instead of re-downloading and re-failing on every cycle. This is the marks-bad- on-install-failure behavior we deferred from #351. - Preserves the explicit "server-side Content-Length vs total_bytes" mismatch check — surfaces the contract violation directly instead of obliquely via inflate failure. decide_start consults the on-disk download file as the source of truth for "how many bytes do we have so far." The denormalized partial_size in PatchState::Downloading is kept for diagnostics/serde stability but not consulted for the resume offset; this matches the prior file-size-on-disk behavior and avoids needing to update state.json mid-stream from the network layer. For Downloading / Downloaded states where the OS evicted the artifact file out from under us (e.g. iOS code-cache eviction), decide_start falls through to Fresh so the next attempt re-downloads from scratch. The failing-test fixes update file paths and pre-staged state from the old layout (downloads/{N} + downloads/{N}.download.json) to the new layout (patches/{N}/download + patches/{N}/state.json). Several tests that targeted the now-deleted helpers directly are removed; their behavior is covered by the new lifecycle unit tests. * refactor: delete patch_manager.rs and download_state.rs Both modules are now subsumed by PatchLifecycle: - patch_manager.rs (~1600 lines) managed the per-release `patches/` tree, the `next_boot_patch` / `last_booted_patch` / `currently_booting_patch` / `known_bad_patches` fields of `PatchesState`, and the fall-back-from-bad-patch logic. All of this is now in lifecycle.rs split between `PatchLifecycle` (per-patch state.json) and `ReleasePointers` (a single pointers.json). - download_state.rs (~125 lines) wrote the per-download sidecar JSON. Now folded into PatchState::Downloading / PatchState::Downloaded variants in state.json, owned by the lifecycle module. The MockManagePatches / ManagePatches trait machinery in updater_state.rs's tests goes away with patch_manager. The lifecycle operations are tested directly in cache::lifecycle::tests against a real on-disk filesystem under TempDir, which catches issues the mocks couldn't (e.g. file-existence cross-checks in `decide_start`). The cargo workspace now has 212 passing tests vs. 257 before, the delta being the patch_manager unit tests whose subjects no longer exist. End-to-end coverage in `updater.rs::tests` is unchanged and all green. * refactor: address self-review feedback on lifecycle PR Nine fixes from the self-review pass on shorebirdtech/updater#352: - Drop `partial_size` from PatchState::Downloading. The field was misleading (decide_start reads from disk, not from the recorded value) and unused. record_download_started loses its 5th arg. - Gate `UpdaterState::install_patch` to `#[cfg(test)]`. Production no longer routes through it; only test_utils and the updater_state tests do. The gate makes the divergence intentional and prevents future production callers. - install_patch defensively removes any prior `dlc.vmcode` before rename so behavior is OS-agnostic (POSIX rename overwrites silently; Windows fails). Also mirrors record_install_complete's cleanup of a stale `download` file in the patch dir. - Document on `lifecycle()` / `lifecycle_mut()` that the direct accessors are intentional — wrapping every transition would be churn for no reader benefit. - recompute_next_boot now clears `last_booted_patch` when its on-disk record is gone (Unknown), so pointers.json doesn't accumulate references to nothing. A `Bad` last_booted is left alone — that's a useful breadcrumb and recompute simply doesn't promote it. - Promote `download_artifact_path` and `installed_artifact_path` to methods on PatchLifecycle for symmetry with state_path / pointers_path. Updates all call sites. - Document mark_bad-on-Bad merge semantics: latest reason wins, other fields preserved. Hypothetical in practice but no longer silent. - Add tests for recompute_next_boot's stale-pointer clearing (Unknown → cleared, Bad → kept). Brings the suite to 214 tests. The mark_bad-cleanup-stale-bytes recovery path I flagged was already covered by `cleanup_on_bad_patch_keeps_tombstone` — no new test needed. * refactor: undo two over-corrections from the prior self-review fix Two issues introduced by 07ea84a that the second self-review caught: - update_internal computed download_path / installed_path via `with_state(...).lifecycle().download_artifact_path(n)`. That routed a pure-function path computation through a state read, serializing it against other state operations and adding a disk read per call. Restore the free `download_artifact_path` / `installed_artifact_path` functions as the canonical entry point; the methods on PatchLifecycle are kept as thin wrappers for callers that already hold a lifecycle handle. - install_patch's "defensive remove_file before rename" added a TOCTOU window between the exists() check and the rename for no real benefit — install_patch is now `#[cfg(test)]`-only, the Windows-rename concern was theoretical for tests, and POSIX rename atomically overwrites. Drop the explicit remove_file. * ci: fix warnings-as-errors and cspell Three CI failures from the prior push: - `Context` and `bail` imports in updater_state.rs are only used inside the `#[cfg(test)] install_patch` helper. Moved them under `#[cfg(test)]` so non-test builds don't see unused imports. - `PathBuf` import in updater.rs became unused after switching the download/install paths to take `Path::new(&config.storage_dir)` directly. Dropped. - `FileOperation::DeleteDir` is no longer constructed by production code (was used by the deleted patch_manager.rs). Mirrored the existing `#[allow(dead_code)]` annotation already on `DeleteFile` rather than removing the variant — the format/handling code for it is still useful for any future code that needs to surface a "delete dir failed" error. CSpell additions: `unparseable`, `tombstoned`, `roundtrips` — words introduced by the lifecycle module's docs and test names. * ci: gate PathBuf import to platforms that actually use it The previous fix removed `PathBuf` from the top-level use, which broke the lib build on non-android non-test platforms (macOS, Linux, Windows) where `libapp_path_from_settings` uses `PathBuf`. That function is itself gated to `cfg(not(any(target_os = \"android\", test)))`, so the PathBuf import needs the same gate. Restoring it as a cfg-gated separate import — keeps the lib build green on all three runners and keeps the lib-test build clean under -D warnings. * test: cover the gaps surfaced by the coverage report Adds 7 tests targeting branches that were uncovered: - mark_bad_from_downloading_records_partial_file_size: the Downloading source state in mark_bad reads bytes-on-disk for the recorded `size` field; the existing tests only covered Installed → Bad. - mark_bad_overwrites_reason_when_already_bad: documented merge semantics (latest reason wins, other fields preserved) now has a test backing the doc. - validate_next_boot_patch_marks_bad_when_artifact_missing: the "Installed state but dlc.vmcode is gone" path. Existing test covered size mismatch; this covers the missing-file branch. - validate_next_boot_patch_marks_bad_when_pointer_targets_non_installed: the case where the next_boot pointer references a state other than Installed (state.json + pointers can diverge through corruption). - install_patch_install_only_{accepts_valid,rejects_missing,rejects_bad}_signature: cover the InstallOnly verification path in UpdaterState::install_patch using the existing test keypair from signing.rs. Coverage improvements: - cache/lifecycle.rs: 94.65% → 96.06% lines, 98.72% → 100% fns - cache/updater_state.rs: 94.34% → 96.94% lines, 95.65% → 98% fns - total: 94.70% → 95.11% lines 214 → 221 tests, all passing under -D warnings. * ci: add 'keypair' to cspell dictionary * test: audit deleted patch_manager tests; restore strict checks Goes through every test deleted with patch_manager.rs (~40) and either maps it to a new test (with a `Ports patch_manager.rs::mod::name` comment) or adds a port. Three behavioral regressions caught and fixed: - record_boot_start now requires next_boot_patch to match the arg. Old PatchManager had this defensive check; my refactor dropped it. Carries forward the engine-vs-state agreement guard. - Added strict-mode signature tests for validate_next_boot_patch (valid, missing, invalid, no public key, bad public key). Ports five `validate_next_boot_patch_tests::strict_mode_*` tests. - Added InstallOnly + no public_key test (ports add_patch_tests::install_only_succeeds_with_any_signature_if_no_public_key). Plus two scenario tests that didn't have direct coverage: - rolled_back_patch_not_resurrected_when_replacement_fails: ports fall_back_tests::rollback_then_failed_replacement_does_not_resurrect_rolled_back_patch against the new state-based fallback (cleanup forgets, recompute won't promote a None state). - validate_then_promote_catches_corrupted_last_booted: ports validate_next_boot_patch_tests::does_not_fall_back_to_last_booted_patch_if_corrupted with a note: new code catches the corruption at the next validate pass instead of at fall-back time, but the user-visible outcome (boot the base release) is the same. Test helper split: `install_state` (writes Installed without touching pointers) vs callers explicitly setting pointers. Avoids the prior helper's auto-promote masking pointer-management behavior the tests were trying to exercise. Tests deliberately not ported (with reasoning): - debug_tests::manage_patches_is_debug, patch_manager_is_debug: Trait-Debug tests for types that no longer exist. Replaced implicitly by `#[derive(Debug)]` on PatchLifecycle. - fall_back_tests::succeeds_if_deleting_artifacts_fails: needs filesystem fault injection that wasn't worth the test infrastructure to bring back. - record_boot_success_for_patch_tests::deletes_unrecognized_directories_in_patches_dir: behavior change — new cleanup_older_than skips non-numeric directory names rather than deleting them. Defensive; the prior `rm -rf` was overly aggressive. Coverage: 222 → 230 tests, all green under -D warnings. * test: port the two patch_manager tests I'd handwaved Both turned out trivial to port — neither was actually doing fault injection, just exploiting graceful-failure paths. - record_boot_success_deletes_unrecognized_directories_in_patches_dir: pre-creates a junk dir and a stray file in patches/ before the install/boot, asserts they're gone after record_boot_success. Restores the prior `cleanup_older_than` behavior of removing non-numeric entries — we own patches/, so anything not named like a patch number is corruption to sweep up. - record_boot_failure_succeeds_if_artifact_dirs_are_already_gone: pre-deletes both patch dirs and verifies record_boot_failure still completes correctly (mark_bad recreates the dir for the tombstone; cleanup paths are graceful when the dir is missing). Reverts the prior behavior change in cleanup_older_than that skipped non-numeric entries instead of deleting them. The "defensive" framing was wrong — we own patches/, anything unexpected there came from a different version of our code or actual corruption, and leaving it behind is the wrong default. * test: port the remaining patch_manager tests with real coverage gaps - validate_next_boot_strict_mode_succeeds_with_valid_signature: ports `patch_manager.rs::validate_next_boot_patch_tests::strict_mode_succeeds_with_valid_signature_at_boot_time`. Required reusing the prior test fixture's INFLATED_PATCH_HASH / INFLATED_PATCH_SIGNATURE constants (a real signature for the 1-byte file content `b"1"`, matching TEST_PUBLIC_KEY's private half). - record_boot_failure_keeps_last_booted_pointer_when_failed_patch_was_last_booted: ports `patch_manager.rs::record_boot_failure_for_patch_tests::preserves_last_booted_patch_on_failure_but_marks_bad`. The new behavior is similar but more explicit: last_booted's *pointer* is preserved (with the patch now in Bad state); the underlying intent — "we still know what last booted, even after that patch failed" — survives. Test helper split: `install_signed` (failure paths, mismatched hash OK) vs `install_with_valid_signature` (happy path, real fixture). Now every behavioral test from the deleted patch_manager.rs has a corresponding new test with a porting comment. Two trivial ones not ported: the `Debug` impl tests for traits/types that no longer exist. 236 tests, all green under -D warnings. * refactor: restore separate download directory under OS cache root Reverts an unintended policy change in the cutover: my refactor moved compressed download bytes from `{code_cache_dir}/downloads/{N}` (the OS-managed cache dir, evictable under storage pressure) into `{storage_dir}/patches/{N}/download` (persistent app storage). That made downloads count against persistent storage and survive in iCloud backups — neither of which we want for transient bytes. Restoring the original split: - `{state_root}/patches/{N}/{state.json,dlc.vmcode}` — persistent state and the installed artifact. - `{download_root}/{N}` — flat, in the OS cache dir. `PatchLifecycle::load_or_default` now takes both roots. The two artifact-path helpers each route to their respective root, the free-function variants used by `update_internal` now take the right root, and `mark_bad`/`cleanup`/`record_install_complete` clean both locations as appropriate. `UpdaterState::create_new_and_save` also wipes `download_dir` on release-version change, in addition to the persistent patches/ tree. `update_internal` and tests updated to use the right root for each file type. Test fixtures pass `tmp.path().join("downloads")` as the download root (separate subdir of the same TempDir). Net change in test surface: moved a handful of `patch_dir/download` references to `downloads_dir/N`, no behavioral changes to tests themselves. 236 tests pass under -D warnings. * test: cover the new download_root cleanup paths Self-review caught two gaps in the download-root split: - release_version_change_wipes_download_dir: the cache-rooted download dir should be wiped along with the persistent patches/ tree on a release-version mismatch. Adds explicit coverage — the code path was already in create_new_and_save but not directly tested. - record_boot_success_promotes_and_cleans_older: extends the existing test to drop a stale download in download_root alongside an older patch's state, asserts that cleanup_older_than's chain (cleanup → forget_dir) deletes both roots' artifacts. Also adds a comment on cleanup_older_than noting that it only walks the persistent patches/ tree — orphan downloads (no state.json) would persist until the OS evicts them. Noted as a known limitation, not blocking. * refactor: targeted wipe + legacy patches_state.json + orphan-download walk Two fixes to the release-version-change wipe and one to the boot- success cleanup walk. - The "wipe everything in cache_dir" approach broke 27 tests whose setup uses cache_dir as the test temp dir (with base.apk and libapp.so as siblings). Production gives shorebird a dedicated subdir but the API doesn't enforce that. Switched to a targeted wipe with a documented allowlist of paths shorebird has ever written under cache_dir: `SHOREBIRD_OWNED_PATHS`. - Added `patches_state.json` to the wipe list — that's the legacy file from the prior `PatchManager` implementation, and devices upgrading through this PR would otherwise orphan it. New test `release_version_change_wipes_legacy_patches_state_json` covers it. - `cleanup_older_than` now also calls `cleanup_orphan_downloads`, which walks `download_root/` and removes any file that doesn't correspond to a patch in `Downloading`/`Downloaded` state. We own the cache root and shouldn't rely on OS eviction. Catches the "state.json gone but download lingered" case the prior comment handwaved. Adding a new file under cache_dir means adding it to SHOREBIRD_OWNED_PATHS — small bookkeeping cost, but it lets us keep cache_dir co-tenant with embedder-provided files. * test: cover all four branches of cleanup_orphan_downloads The new orphan-download sweep distinguishes four cases but only the "older patch via cleanup chain" branch was being exercised. Single test now drops one of each kind of file into download_root before a boot success and asserts which survive: - orphan (numeric, no state.json): removed - stale (numeric, state is Installed): removed - non-numeric name: removed - live (numeric, state is Downloading): preserved * ci: add 'embedder' to cspell dictionary * refactor: drop Installed.hash, fix mark_bad ordering, address bdero review bdero's review on #352 surfaced six items worth landing inline: 1. record_boot_failure used to clear `currently_booting_patch` before `mark_bad`. A crash between the two left the patch `Installed` with no breadcrumb, so `detect_boot_crash_on_init` wouldn't fire next boot — the patch silently retried. Flipped: mark_bad first, then clear. mark_bad on Bad is idempotent, so the worst case is a redundant tombstone-rewrite on next init. 2. Drop the `hash` field from `PatchState::Installed`. It was recorded at install but never read at boot — Strict-mode validation recomputes the hash from the artifact's bytes and feeds it into `check_signature`. A hash that lives only on disk can't be trusted as a security input; deleting the field removes the temptation. `Downloading.hash`/`Downloaded.hash` stay as comparators against the server's freshly-delivered hash (a tampered on-disk hash there just causes a redownload). 3. Port the customer-scenario test from #356 into `c_api/mod.rs`: server rolls patch 1 back, then forward again with the same number and hash. Pre-lifecycle this was permanently dropped via `known_bad_patches`. Post-lifecycle `cleanup` is state-aware and forgets the patch entirely on server-driven rollback, so the rollforward installs cleanly. End-to-end coverage of the regression #356 hotfixed. 4. Add a TODO + target version on the `patches_state.json` entry in `SHOREBIRD_OWNED_PATHS` so the legacy wipe doesn't outlive its usefulness. 5. Document that `running_patch` lives in `config.rs` (session-scoped), not in this module — saves a future grep. 6. Clarify `recompute_next_boot`'s fallback policy: it's fall-back-to-`last_booted_patch`-only, not fall-back-to-anything- bootable. A fresh release whose first Installed patch fails validation goes to base, even if older patches sit in `patches/`. cargo test: 242 passed (was 241).	2026-05-07 16:01:28 -07:00
Eric Seidel	8649c75206	perf: mmap libapp.so out of the APK instead of buffering in RAM (#354) ... * perf: mmap libapp.so out of the APK instead of buffering in RAM `open_base_lib` previously read the entire decompressed libapp.so into a Vec<u8> via `read_to_end` and handed bipatch a Cursor over that buffer. For large apps libapp.so can be tens of megabytes, and that allocation happens immediately after the patch download is buffered to disk — a plausible OOM trigger on memory-constrained devices (we have at least one customer report on OnePlus where the patch install appears to halt silently right after the download completes). Modern AGP (3.6+) defaults to extractNativeLibs=false, which keeps libapp.so STORED uncompressed inside the APK so the dynamic linker can mmap it directly. When that's the case, do the same: find the entry's data offset via the zip crate, drop the archive, reopen the APK, and mmap the entry's slice. Cursor<Mmap> implements Read + Seek, which is what bipatch's `Reader::new(patch, base)` requires. When the entry isn't stored uncompressed (older builds, or builds that explicitly compress native libs), fall back to the previous buffered read so we always succeed. Mmap doesn't change the peak working set when bipatch traverses the whole base linearly, but file-backed mappings are clean and reclaimable under memory pressure where an anonymous Vec is not, and we avoid the ~2x transient allocation peak from `read_to_end` growing the buffer. Tests cover both paths (stored → mmap, deflated → buffered) on the host. * ci: add mmap, memmap, SIGBUS to cspell dictionary	2026-05-05 17:33:22 -07:00
Eric Seidel	8072ed9ad7	feat: stage 1 integration test harness (#353) ... * docs: integration tests design proposal Adds docs/integration_tests.md proposing a desktop-only integration suite that drives the Dart `ShorebirdUpdater` API against the real Rust core via FFI, with a fake HTTP server and per-test tempdir state. Describes a new `library_test_hooks` cdylib that reaches into `updater` via a `test-hooks` Cargo feature, so production C API stays clean. Status: design exploration, not a committed plan. * feat: stage 1 integration test harness for shorebird_code_push Lands the test_hooks crate, the Dart-side loader, and one trivial end-to-end test, per docs/integration_tests.md stage 1. Pieces: - New `library_test_hooks` workspace crate (cdylib). Depends on `updater` with a new `test-hooks` Cargo feature that widens the visibility of internal items (currently `testing_reset_config`) for sibling-crate access only — production builds do not enable it. The crate also re-exports `updater::c_api::dart::*` and `updater::c_api::engine::*`, which keeps the rlib's `#[no_mangle]` symbols out of DCE so the resulting cdylib carries both the production C API and the new `shorebird_test_*` hooks. - Workspace `default-members` excludes `library_test_hooks` so plain `cargo build` / `cargo test` invocations don't unify the `test-hooks` feature into production builds. - `shorebird_code_push/test/integration/all_test.dart` (single file, with a header comment explaining why) loads the cdylib via `DynamicLibrary.open`, reassigns the existing `@visibleForTesting` `Updater.bindings` setter, and exercises both surfaces. The build helper shells out to `cargo build -p library_test_hooks`; if it fails, `markTestSkipped` keeps the suite green on machines without a working Rust toolchain. - ffigen config (`ffigen_test_hooks.yaml`) generates the test-only Dart bindings under `test/integration/generated/`, not `lib/`. - CI: `library_test_hooks` is added to the rust_crate matrix, and the shorebird_code_push job triggers on `library/**` and `library_test_hooks/**` so cdylib changes can't break the Dart-side integration suite without CI noticing. Verified locally: 232 Rust unit tests + 42 Dart tests (41 existing + 1 integration) green; clippy/fmt/cspell clean; production cdylib does not contain `shorebird_test_reset` (`nm` confirms feature isolation). Stages 2 (FakePatchServer + golden path) and 3 (adversarial scenarios) land separately. * fix(integration test): early-return on skip and bump per-test timeout Two issues caught by Shorebird CI on PR #353: 1. `markTestSkipped` does not abort test execution — it only flags the test as skipped on its way out. The body kept running and crashed on the `late testHooks` field when the cdylib build had failed in setUpAll. Move the skip check into the test body itself with an early return; drop the (no-op) skip in `setUp`. 2. Default per-test timeout (30s) also covers `setUpAll`. A cold `cargo build -p library_test_hooks` compiles `updater` and ~100 transitive deps, which can run minutes on CI. Bump to 10 minutes via `@Timeout` on the library. Verified locally: passes when cargo is on PATH (1 passed), reports a clean skip and exit 0 when cargo is removed from PATH (1 skipped). * refactor(integration test): drop ! by promoting testHooks to late final `testHooks` was nullable so accessing it after the skipReason check required `!`, and `markTestSkipped(skipReason!)` had the same smell. Make `testHooks` `late final` (non-nullable, throws if read before setUpAll assigns) and pull `skipReason` into a non-null local before use. Same control flow, no bang operators.	2026-05-05 16:32:57 -07:00
Mac	b97c7919bd	feat: send current_patch_number on patch check (#343) ... * feat: send current patch_number on patch check * refactor: rename patch_number → current_patch_number Renaming the field on PatchCheckRequest so it doesn't collide with the legacy `patch_number` field old pre-#189 updaters still send. That legacy field is what triggers the server's short-circuit response (patchAvailable: false, no rolled_back_patch_numbers), which is still load-bearing for ~0.7% of patch-check traffic coming from Flutter ≤ 3.22.2 clients. Using a distinct field name keeps our new analytics signal from accidentally engaging that path. Pairs with: - shorebirdtech/shorebird#3702 (protocol field) - shorebirdtech/_shorebird#2059 (server consumes this field) * docs: rephrase current_patch_number doc comment Drops the 'analytics' framing in favor of describing the field as superseding patch_number for newer clients. patch_number remains for compatibility with legacy clients that rely on the short-circuit path. * fix: adapt to renamed currently_booting_patch and &str client_id	2026-05-04 17:16:30 -06:00
Eric Seidel	34509fca3c	refactor: split C API into Dart and engine surfaces (#350) ... The C surface in `library/src/c_api` was a single bucket of `pub extern "C"` functions covering both consumers — `package:shorebird_code_push` (via ffigen) and Shorebird's Flutter engine fork (via direct C++ link). That made it hard to reason about which symbols are stable ABI versus internal, and ffigen was generating bindings for engine-only symbols that no Dart code calls. Split into two self-contained submodules and two cbindgen-generated headers: - `c_api::dart` → `include/updater_dart.h` (stable ABI; ffigen entry point). Defines `UpdateResult`, the `SHOREBIRD_*` status constants, and the five Dart-stable functions: `shorebird_current_boot_patch_number`, `shorebird_next_boot_patch_number`, `shorebird_check_for_downloadable_update`, `shorebird_update_with_result`, `shorebird_free_update_result`. - `c_api::engine` → `include/updater_engine.h` (no stability guarantee). Defines `AppParameters`, `FileCallbacks`, and the engine-only functions: `shorebird_init`, `shorebird_should_auto_update`, `shorebird_validate_next_boot_patch`, `shorebird_next_boot_patch_path`, `shorebird_free_string`, `shorebird_start_update_thread`, and the `shorebird_report_launch_*` trio. Each bucket file is self-contained: cbindgen scans only the file (`with_src` in build.rs) and emits the items it defines plus the C types they reference. There are no exclude/include lists in the cbindgen configs — adding a function to one bucket automatically lands it in the right header, and items in the other bucket cannot leak. `mod.rs` shrinks to a thin layer of private helpers shared by both buckets (`to_rust`, `allocate_c_string`, `free_c_string`, `log_on_error`) plus the test module. `include/updater.h` is removed; consumers include the specific header for their use case. The Flutter engine's `shell/common/shorebird/updater.cc` will be updated in a follow-up engine-repo PR to include `updater_engine.h` directly. Also drops two retired Dart-side symbols: - `shorebird_update` (replaced by `shorebird_update_with_result` in the Dart 2.0 rewrite, Nov 2024). - `shorebird_check_for_update` (replaced by `shorebird_check_for_downloadable_update` in the same rewrite). The shorebird_code_push package's `_legacyFallback` was the only path that still called `shorebird_update`. The package's `flutter: >=3.24.5` constraint guarantees the engine has `shorebird_update_with_result`, so the fallback was unreachable in practice. Removing it lets us drop the ABI symbol. Bumps shorebird_code_push to 2.0.7. Bindings regenerated via ffigen now contain only the five Dart-stable symbols. Follow-up engine PR will: include `updater_engine.h` instead of the removed `updater.h`; clean up `android_exports.lst` (drop the ghost `shorebird_active_path` and `shorebird_active_patch_number` exports, drop `shorebird_check_for_update`).	2026-05-04 15:56:09 -07:00
Eric Seidel	10aaca0f8b	fix: skip fetch when prior download is already complete on disk (#351) ... * fix: skip fetch when prior download is already complete on disk A prior attempt that finished downloading but failed a post-download step (inflate / hash check / install) used to leave the partial file and sidecar in place. The next update would call compute_resume_offset, get back the file's full size, and send Range: bytes=N- against an N-byte resource — past the end of the file, yielding HTTP 416 forever. Replace compute_resume_offset with a read-only determine_download_start_state returning Fresh | Resume(u64) | Complete(u64). update_internal handles Complete by skipping download_to_path entirely and letting the install path validate the existing bytes; this also avoids re-downloading when the app is killed between download and install. Extract the post-download work into install_downloaded_patch so cleanup can run unconditionally once after it returns. Three previous cleanup sites collapse to one. If install rejects the bytes, the next attempt re-downloads from scratch. * refactor: pass output_path by value into install_downloaded_patch Avoids a redundant PathBuf clone on the success path. The caller already owns the PathBuf from download_dir.join(...), and PatchInfo wants an owned path, so threading ownership through is strictly cheaper than borrowing and cloning. * fix: record actual download size in sidecar to handle chunked transfers When the server uses chunked transfer encoding (no Content-Length header), dl_result.content_length is None. The old code recorded that None as expected_size, which meant a subsequent crash-before-install attempt would see expected_size: None + a full-size file, fall through to Resume(file_size), and re-create the HTTP 416 loop this PR fixes. Recording dl_result.total_bytes (the actual on-disk size) closes the chunked-encoding case. The two values are equal when Content-Length is present, so the Content-Length case is unchanged. Also annotate two pre-existing windows that this PR doesn't fully close, so they stay visible until the per-patch state machine refactor lands: - The microsecond gap between download_to_path returning and the second write_download_state succeeding can still leave expected_size: None. - cleanup_download_artifacts logs delete errors instead of propagating; a silently-failed delete could re-create the same 416 loop. Both are tracked in shorebirdtech/shorebird#3737.	2026-05-04 13:53:21 -07:00
Eric Seidel	1f2abac401	fix: current_boot_patch survives server-driven rollback (#348) ... * fix: current_boot_patch survives server-driven rollback Customer report (shorebirdtech/shorebird#3728): when the device's running patch is rolled back to the base release (no replacement patch), the running session sees `checkForUpdate` return `upToDate` even though a restart is needed. Patch-to-patch rollback works because the server's replacement patch makes `check_for_downloadable_update` return true, so Dart short-circuits to `outdated` before the comparison runs. Root cause: `UpdaterState::current_boot_patch()` derived its return value from `currently_booting_patch.or(last_successfully_booted_patch)`. After boot success, only `last_booted_patch` reflected the running patch. When the server rolled back that patch, `try_fall_back_from_patch` cleared `last_booted_patch` (correctly — it's no longer a valid fallback), and the FFI `shorebird_current_boot_patch_number` then reported 0 even though the process was still running the rolled-back patch. The conflation: `last_booted_patch` was doing two unrelated jobs — "fallback target" (its real role) and "what's running" (the proxy via `.or()` that broke under rollback). The earlier Dart-only fix in shorebirdtech/updater#312 assumed the FFI would still report the running patch number; that assumption only held in the mock. Fix: introduce a dedicated `current_boot_patch: Option<usize>` field on `PatchesState`. Set by `report_launch_start` from `next_boot_patch` (or `None` for a release boot). Read directly by `UpdaterState::current_boot_patch()` — no derivation, no fallback. Each field now has exactly one job: - `last_booted_patch`: fallback target for `try_fall_back_from_patch`. Doc updated to remove the "(usually the currently running patch)" parenthetical that perpetuated the conflation. - `current_boot_patch` (new): what this process is using. Survives rollbacks of that patch (the process is still using it). Reset on the next `report_launch_start` — including `None` on a release boot, so it doesn't go stale. - `currently_booting_patch`: unchanged. Still the boot-in-progress flag for crash detection on the next init. C API surface unchanged. `shorebird_current_boot_patch_number` still returns the same `usize` it always has — it just gets the right answer under rollback now. Verification (testing at the C API level, since that's the contract): - New regression test `rollback_to_release_keeps_current_boot_patch` reproduces the customer's bug. Fails on the parent commit (`current_boot_patch_number` returns 0); passes after this fix (returns 1). - New `rollback_to_release_then_restart_clears_current_boot_patch` proves the post-restart cleanup: the on-disk `current_boot_patch` is `Some(1)` from the previous run, but the next launch's `report_launch_start` resets it to `None` since `next_boot_patch` is `None`. No false-positive `restartRequired` on the release boot. - New `rollback_patch_to_patch_reports_current_and_next_distinctly` proves we didn't break the patch-to-patch case. Running on patch 2, server rolls back to patch 1: after `update()`, `current=2, next=1`. - All 225 existing tests pass without modification, including every C API test. Refs: shorebirdtech/shorebird#3728, shorebirdtech/updater#312, #270 * docs: TODOs for follow-up cleanup of patch state model Two cleanups deferred from #348 to keep the rollback fix focused: 1. Rename `last_booted_patch` → `fallback_patch`. Single mechanical rename, but touches ~30 test names that read in terms of the current field name. 2. Remove `currently_booting_patch` entirely. With `current_boot_patch` now tracking what's running, the boot-in-progress signal collapses to `boot_started_at.is_some()`, and the crashed-patch-on-init identification falls out of the previous run's `current_boot_patch`. This is the larger of the two — touches crash-detection logic and the boot-record helpers. Both should land as their own commits so the diff for each is easy to read and the rollback fix stays minimal. * test: assert rollback-only phases never report events In `rollback_to_release_keeps_current_boot_patch` and `rollback_to_release_then_restart_clears_current_boot_patch`, the phase that performs only the server-driven rollback never calls `shorebird_update` or `shorebird_report_launch_*`, so no event should ever be reported during it. Replace the no-op report hook with `UNEXPECTED_REPORT` to make that an asserted property of the test rather than a silent assumption — if a future change starts queueing or sending events from `check_for_downloadable_update`, these tests will surface it immediately. Phase-1 spawned threads (PatchDownload, PatchInstallSuccess) are unaffected: they hold a clone of the config from when they were spawned, so they hit the phase-1 hooks and never reach phase-2's panicking handler. The patch-to-patch test keeps the no-op hook because phase 2 there calls `shorebird_update`, which legitimately spawns a PatchDownload event using the new hooks. * docs: flag the last_booted_patch conflation as the underlying bug Replace the rename TODO with one that names the actual unfixed bug: `last_booted_patch` gets cleared in `try_fall_back_from_patch` while the running process is still using the patch. That's the deeper incoherence — the field's name and `record_boot_success` say it's a historical record, but the rollback path treats it as an operational fallback target. Those two roles only diverge under server rollback, which is the customer's case. This PR sidesteps the conflation by adding `current_boot_patch` for the "what's running" semantic. The TODOs now flag both: - The conflation itself, on the field declaration. - The specific line in try_fall_back_from_patch that does the historically-incorrect clearing. A sibling PR will prototype the alternative — keep last_booted_patch historical, express "don't fall back to this patch" via a separate signal — so we can compare the two approaches. * fix: stop clearing last_booted_patch when its patch is rolled back Roll #349 into this PR. Both fixes together — they address different real bugs and combining eliminates each PR's loose ends. Underlying data-model bug: `last_booted_patch` was conflated. Its field name and `record_boot_success` say it's a *historical* record (\"the patch that last successfully booted, ever\"). But `try_fall_back_from_patch`'s \"both bad\" branch clears it whenever the patch becomes invalid as a fallback — including when the server rolls it back, while the running process is still using it. Fix: in the \"both bad\" branch, only clear `next_boot_patch`. Leave `last_booted_patch` alone — that history shouldn't change because the server told us not to use the patch next time. The \"don't fall back to this patch\" intent is already covered by: - `delete_patch_artifacts(bad_patch_number)` at the top of the function, which removes the on-disk artifacts. - `validate_patch_is_bootable` in the else-if branch, which refuses to fall back to a patch with missing artifacts. - `is_known_bad_patch`, which records boot failures explicitly. `record_boot_failure_for_patch` flows through the same branch and benefits from the same correction — boot history is preserved across boot failures. Updated the corresponding test (`clears_last_booted_patch_if_it_is_the_failed_patch` → `preserves_last_booted_patch_on_failure_but_marks_bad`) to assert the new behavior: history preserved, known-bad recorded, artifacts deleted. Removes the two TODOs added in the previous commit: - The conflation TODO on `last_booted_patch` (now fixed). - The TODO on the offending line (line is being changed). This pairs with the `current_boot_patch` field added earlier in the same PR. The two fixes are orthogonal: - `current_boot_patch` gives us a session-scoped \"what's running\" signal, reset on `report_launch_start`. It's what the FFI reads. - The data-model fix here keeps `last_booted_patch` historically accurate, so the field's name finally matches what it stores. With both, `current_boot_patch()` no longer needs the `.or()` fallback that was the original source of the customer's bug.	2026-05-01 16:38:47 -07:00
Brandon DeRosier	ede7990ea2	feat: enrich patch install failure messages with diagnostics (#346) ... * feat: enrich patch install failure messages with diagnostics The `message` field in `__patch_install_failure__` events previously contained generic strings that didn't help distinguish failure causes. Crash recovery messages now include: - `elapsed_secs`: time between boot start and crash recovery detection, helping distinguish immediate crashes (likely OOM) from delayed kills (user force-stop, OS reclaim hours later) - `file_ok`/`file_size`: whether the patch file is intact at recovery time, catching corruption or partial writes Message format changes: - Crash recovery: "crash_recovery: patch N failed to boot (elapsed_secs=S,file_ok=bool,file_size=N)" - Engine failure: "engine_report: patch N failed to launch" Also adds `boot_started_at` timestamp to PatchesState (backward compatible — older state files deserialize it as None). * chore: fix formatting * test: add coverage for crash recovery edge cases - crash_recovery_with_missing_file: patch artifact deleted before recovery, verifies file_ok=false,file_missing in message - crash_recovery_without_boot_timestamp: old state file without boot_started_at field, verifies elapsed_secs=unknown in message * refactor: simplify file check in crash recovery diagnostics Remove unreachable branch: Path::exists() calls fs::metadata() internally, so if metadata fails, exists() returns false. The separate file_unreadable case could never be reached. * refactor: use raw timestamps instead of elapsed_secs in crash recovery elapsed_secs was misleading because it included the time between the crash and the user reopening the app — not just the boot duration. Now reports boot_started_at (raw Unix timestamp) and detected_at (when crash recovery ran). Server-side analysis can compute elapsed time and cross-reference with successful boot durations from other devices. Example: "crash_recovery: patch 1 failed to boot (detected_at=1776900000,boot_started_at=1776899990,file_ok=true,file_size=524288)"	2026-04-21 20:19:21 -07:00
Eric Seidel	db99e24726	perf: set panic=abort and drop backtrace features (#342) ... Reduces binary size of the updater staticlib when linked into the Flutter engine. On host macOS arm64 the dylib shrinks from 3.07 MB to 2.56 MB; the biggest iOS savings come from dropping the DWARF unwind tables (__eh_frame, __gcc_except_tab, __unwind_info) that panic=unwind generates. - panic = "abort" in [profile.release]: kills unwind-table generation and eliminates dead panic-unwinding code paths. We don't use catch_unwind anywhere in the library. log-panics still runs its panic hook before abort, so panic messages continue to surface to logcat/oslog during development. - Drop `backtrace` feature from anyhow and `with-backtrace` from log-panics: removes gimli + addr2line + rustc_demangle (~65 KB of symbolication machinery) that customers can't read anyway. A future in-engine crash reporter will symbolize native stacks, making these features redundant.	2026-04-21 19:01:26 +00:00
Eric Seidel	ca8f0a9f36	fix: atomic state writes and surface flush errors in disk_io (#344) ... * fix: surface flush errors and write atomically in disk_io::write `BufWriter`'s `Drop` impl silently discards flush errors. Because `patches_state.json` and `state.json` are small enough to fit inside `BufWriter`'s 8 KB buffer, the only time the bytes actually reach disk is during the implicit flush at drop — and that flush's I/O errors (transient iOS Data Protection lock, ENOSPC, etc.) were invisible to the caller. `disk_io::write` returned Ok, `install_patch` returned Ok, `update()` returned `UpdateInstalled`, but `patches_state.json` was left at 0 bytes. On the next launch, `load_patches_state` failed to deserialize and fell back to default (`next_boot_patch: None`), so the subsequent `checkForUpdate()` saw the server's patch as newly installable and reported `UpdateStatus.outdated` despite the app having "successfully" installed it moments earlier. Change `disk_io::write` to: - Write to a sibling `<file>.tmp` and atomically `rename` into place, so `path` is never observed in a truncated/empty state by a concurrent or post-crash reader. - Explicitly unwrap the `BufWriter` via `into_inner()`, which calls `flush_buf` and returns any I/O error as `IntoInnerError` instead of dropping it on the floor. - Clean up the temp file on failure. Extract `serialize_and_flush` so the flush-error path is unit-testable without filesystem tricks. Add three tests: - temp file is cleaned up after a successful write - a failed write preserves the existing file at \`path\` - regression: \`serialize_and_flush\` surfaces inner-writer errors (confirmed to fail on the pre-fix code) * style: apply cargo fmt and rename roundtripped -> reloaded for cspell * test: drop unreachable flush body in FailingWriter * test: drop Result<()>/? boilerplate in new tests	2026-04-21 11:21:49 -07:00
dependabot[bot]	08b91f49d2	chore(deps): bump the library-deps group in /library with 5 updates (#332) ... * chore(deps): bump the library-deps group in /library with 5 updates Updates the requirements on [sha2](https://github.com/RustCrypto/hashes), [zip](https://github.com/zip-rs/zip2), [mockall](https://github.com/asomers/mockall), [mock_instant](https://github.com/museun/mock_instant) and [cbindgen](https://github.com/mozilla/cbindgen) to permit the latest version. Updates `sha2` to 0.11.0 - [Commits](https://github.com/RustCrypto/hashes/compare/streebog-v0.11.0-pre.0...sha2-v0.11.0) Updates `zip` to 8.5.0 - [Release notes](https://github.com/zip-rs/zip2/releases) - [Changelog](https://github.com/zip-rs/zip2/blob/master/CHANGELOG.md) - [Commits](https://github.com/zip-rs/zip2/compare/v3.0.0...v8.5.0) Updates `mockall` to 0.14.0 - [Changelog](https://github.com/asomers/mockall/blob/master/CHANGELOG.md) - [Commits](https://github.com/asomers/mockall/compare/v0.13.1...v0.14.0) Updates `mock_instant` to 0.6.0 - [Commits](https://github.com/museun/mock_instant/compare/v0.5.1...v0.6.0) Updates `cbindgen` to 0.29.2 - [Release notes](https://github.com/mozilla/cbindgen/releases) - [Changelog](https://github.com/mozilla/cbindgen/blob/main/CHANGES) - [Commits](https://github.com/mozilla/cbindgen/compare/0.28.0...0.29.2) --- updated-dependencies: - dependency-name: sha2 dependency-version: 0.11.0 dependency-type: direct:production dependency-group: library-deps - dependency-name: zip dependency-version: 8.5.0 dependency-type: direct:production dependency-group: library-deps - dependency-name: mockall dependency-version: 0.14.0 dependency-type: direct:production dependency-group: library-deps - dependency-name: mock_instant dependency-version: 0.6.0 dependency-type: direct:production dependency-group: library-deps - dependency-name: cbindgen dependency-version: 0.29.2 dependency-type: direct:production dependency-group: library-deps ... Signed-off-by: dependabot[bot] <support@github.com> * fix: adapt sha2 0.11 hashing (no io::Write impl) * refactor: reuse cache::hash_file in check_hash --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Eric Seidel <eric@shorebird.dev>	2026-04-08 16:16:41 -07:00
Eric Seidel	563f1b773a	fix: return UpdateInProgress status instead of erroring when another update is running (#335) ... When `update()` is called while another update (typically the automatic updater thread) is already running, the Rust updater previously bailed with `UpdateError::UpdateAlreadyInProgress`, which surfaced in Dart as `UpdateException: Update already in progress (unknown)`. This is the single highest-volume `UpdateException` in customer telemetry, yet the underlying situation is benign — the in-flight update continues on its own, the caller simply did not start a new one. Add a new `UpdateStatus::UpdateInProgress` variant and matching C status code `SHOREBIRD_UPDATE_IN_PROGRESS = 4`. `updater::update()` catches the `UpdateAlreadyInProgress` error from the lock helper and maps it to `Ok(UpdateStatus::UpdateInProgress)`. The Dart wrapper treats the new status as a successful return alongside `SHOREBIRD_UPDATE_INSTALLED`, so `update()` no longer throws for this case. Update the existing `usage_during_hung_update` c_api test to assert the new contract, and add a Dart test covering the in-progress return path. Version skew: new Dart on an old engine still sees the legacy `SHOREBIRD_UPDATE_ERROR` + "Update already in progress" message and will still throw. The fix lands once both sides ship. Partially addresses shorebirdtech/shorebird#3682 — does not resolve the broader asymmetry of `update()` semantics (it still does not wait for someone else's in-flight update to finish), which remains as v2 design work in shorebirdtech/shorebird#3684.	2026-04-08 01:22:04 +00:00
Eric Seidel	adacb4190c	refactor: replace serde_yaml with hand-rolled parser (#326) ... serde_yaml is deprecated and pulls in unnecessary dependencies (unsafe-libyaml, indexmap, hashbrown) for parsing our simple flat key-value config file. Replace with a minimal hand-rolled parser that handles exactly what shorebird.yaml needs. Size impact (release build, macOS arm64): .a (staticlib): -674 KB (26.1 MB → 25.4 MB) .dylib (cdylib): -169 KB (3.7 MB → 3.5 MB)	2026-04-02 16:46:55 -07:00
Eric Seidel	f633b0a34b	test: add coverage for state recovery, download validation, rollback, and resume edge cases (#323) ... * test: add coverage for state recovery, download validation, rollback, and resume edge cases Add 18 new unit tests across 4 test modules to increase confidence in the updater's most sensitive code paths — areas that are hard to test manually and where bugs could brick apps. - state_recovery_tests: corrupt/missing/truncated state files, crash during boot with missing artifacts, crash recovery event queuing, client_id preservation across state corruption - download_validation_tests: download failures, empty downloads, non-zstd data, malformed server responses, network failures preserving existing patch state - rollback_unit_tests: rollback of non-existent patches, multi-patch rollback - resume_edge_case_tests: sidecar without partial file, server ignoring Range header, failed download preserving sidecar for retry, resume after failure * fix: replace 'Reinit' with 'Reinitialize' for cspell * fix: assert state files exist before corrupting them in tests Ensures corruption tests fail loudly if the hardcoded file paths (patches_state.json, state.json, patches/) don't match the actual constants used by the updater, rather than silently passing. * style: fix formatting and clippy warnings in new tests - Apply cargo fmt to new test code - Remove needless borrows on PATCH_BYTES (clippy)	2026-04-02 13:49:30 -07:00
Eric Seidel	0cb43c6349	style: apply cargo fmt and add formatting check to CI (#319) ... Runs `cargo fmt --check` in the rust_crate CI action to catch formatting issues before they land.	2026-04-01 19:30:44 -07:00
Eric Seidel	50954da68a	style: fix clippy warnings and add clippy check to CI (#321) ... * style: fix clippy warnings and add clippy check to CI Runs `cargo clippy --all-targets -- -D warnings` in the rust_crate CI action to catch lint issues before they land. Fixes: redundant field names, needless returns, unnecessary closures, io_other_error, needless borrows, single_match, unnecessary_unwrap, and adds missing safety docs. * chore: add 'clippy' to cspell dictionary	2026-04-02 02:02:05 +00:00
Eric Seidel	463ace0c56	feat: add resumable downloads with streaming to disk (#313) ... * feat: add resumable downloads with streaming to disk Previously, failed patch downloads were lost entirely — the updater would re-download from scratch on every retry, creating a doom loop for users on poor networks. Downloads were also fully buffered in memory. This changes the download abstraction from returning bytes in memory (`DownloadFileFn`) to streaming directly to disk with resume support (`DownloadToPathFn`). The new signature accepts a `resume_from` byte offset and the default implementation uses HTTP Range headers. Key changes: - DownloadToPathFn streams to file, sends Range header when resuming - DownloadResult returns total_bytes and content_length from server - DownloadState sidecar JSON tracks URL/patch/size for resume decisions - compute_resume_offset detects valid partial downloads to resume - Post-download size validation catches truncated downloads - cleanup_download_artifacts removes compressed files + sidecars after successful install (fixes pre-existing leak of download artifacts) The fn-pointer signature maps directly to a future C callback for platform-native download backends (iOS NSURLSession, Android DownloadManager). * fix: address self-review issues in download implementation - Parse Content-Range header for 206 responses to get total file size (reqwest's content_length() only returns the partial body size) - Only append to existing file when server actually returns 206, not just when resume_from > 0 (handles servers that ignore Range header) - Remove incorrect resume_from offset addition to expected_size - Clean up download artifacts on size mismatch before bailing - Restore parent directory creation in download_to_path wrapper * refactor: remove expected_hash from DownloadState The hash stored in the sidecar was the *inflated* file hash from the server response — it couldn't validate the compressed partial download and was never used for resume decisions. The URL match already determines whether to resume, and the real hash check happens after inflate using the fresh server response. Simplifies the sidecar to just url, patch_number, and expected_size. * test: add coverage for resumable downloads + review fixes - Add 12 new tests covering: - compute_resume_offset: no sidecar, matching sidecar, mismatched URL, empty file - cleanup_download_artifacts: removes file+sidecar, noop when missing - Integration: successful update cleans up artifacts - Integration: partial download resumes via 206 with mockito - Integration: URL change triggers fresh download - parse_content_range_total: valid, missing header, unknown size (*) - Extract parse_content_range_total as testable helper (was inline chain) - Add expected_hash back to DownloadState — catches the case where a patch is deleted and re-added with same number but different content - Add "rsplit" to cspell config * feat: add orphan cleanup for download directory + WriteFile comment Scan the download directory before each download and remove any files that don't belong to the current patch number. We own this directory entirely, so anything from a prior patch, a crashed inflate (.full), or an unrecognized file is safe to delete. This prevents gradual accumulation of orphaned partial downloads over time. Also adds a TODO comment explaining the reuse of WriteFile context for seek operations (FileOperation lacks a SeekFile variant). * test: add coverage for hash mismatch, patch number mismatch, corrupt sidecar - compute_resume_offset_mismatched_hash: same URL but hash changed (patch deleted and re-added), verifies fresh download - compute_resume_offset_mismatched_patch_number: sidecar for different patch number, verifies fresh download - compute_resume_offset_corrupt_sidecar: garbage JSON in sidecar, verifies graceful fallback to fresh download * test: cover download size mismatch and unknown content-length paths - update_fails_on_download_size_mismatch: mock returns content_length that doesn't match total_bytes, verifies error + artifact cleanup - update_succeeds_when_content_length_unknown: verifies the size validation is skipped when content_length is None * fix: replace fake hash strings to pass cspell * chore: remove unnecessary TODO comment about FileOperation::SeekFile * refactor: panic in test download mocks that should never be called Tests where the download is never reached (patch check fails or no patch available) now panic instead of returning dummy data, making it explicit that the mock shouldn't be invoked. * refactor: add UNEXPECTED_DOWNLOAD/UNEXPECTED_REPORT test constants Shared panicking constants for test mocks that should never be called. Tests use these by name instead of writing inline panic closures, making intent clearer and avoiding uncoverable dead code in closures. * test: add coverage for handle_download_result Tests for the download-specific HTTP response handler: - 200 OK: accepted - 206 Partial Content: accepted (for resumed downloads) - 500: rejected with error message * fix: update missed download fn signatures in tests Two test sites in updater.rs were not updated to the new DownloadToPathFn 3-argument signature: - set_noop_network_hooks in multi_engine_tests used old 1-arg closure - update_starts_fresh_when_url_changes had unused patch_bytes variable * test: add coverage for handle_download_result error branches Mirror the existing handle_network_result_no_internet and handle_network_result_unknown_error tests for the download variant. These exercise the connection error and builder error paths in handle_download_result that were previously uncovered. * fix: adapt resumable downloads to ureq (post-rebase cleanup) - Replace reqwest with ureq for download_to_path_default - Remove handle_download_result (ureq's handle_network_result handles 206) - Fix TempDir::new("prefix") → TempDir::new() for tempfile crate - Remove unused Read/Write imports	2026-04-01 15:51:16 +00:00
Eric Seidel	2057fd4f46	fix: resolve all Dependabot security vulnerabilities (#316) ... Run `cargo update` to bump transitive dependencies, fixing 10 of 11 alerts (h2, ring, idna, mio, tokio, bytes, time, quinn-proto, rustls-webpki, unsafe-libyaml). Replace deprecated `tempdir` dev-dependency with `tempfile` to eliminate the `remove_dir_all` vulnerability (the last alert).	2026-04-01 15:36:12 +00:00
Eric Seidel	c6647a2dfe	refactor: replace reqwest with ureq to reduce binary size (#317) ... * refactor: replace reqwest with ureq to reduce binary size reqwest's blocking API is built on top of its async implementation, pulling in tokio, hyper, futures, and ~84 other transitive dependencies even though we only make simple synchronous HTTP calls. ureq is a synchronous-only HTTP client that eliminates the async runtime entirely. This reduces transitive dependencies from 227 to 143 and the linked dylib from 4.5 MB to 3.7 MB (-18%). The .a archive drops from 28 MB to 26 MB, but real savings will be larger once linked into libflutter with dead code stripping. The network API surface is unchanged — three functions (patch check, file download, event reporting) using POST/GET with JSON. * chore: add ureq to spell check dictionary * refactor: use into_body() instead of body_mut() where response is consumed * fix: simplify network error matching to avoid fragile string checks Consolidate HostNotFound, ConnectionFailed, and all Io errors into a single network-error arm instead of pattern-matching on error message strings that could change across OS versions or locales. * chore: add TODO for misleading network error message	2026-04-01 08:23:48 -07:00
Eric Seidel	4ff2839cdb	feat: add tests and docs for boot state machine (#309) ... * test: test api calls * chore: add docs * chore: fix cspell * fix: use no-op network hooks in multi_engine tests Set no-op network hooks after init_for_testing so the fire-and-forget thread spawned by report_launch_success completes instantly without network I/O, preventing leaked threads from interfering with subsequent serial tests that use mock servers.	2026-03-30 23:01:50 +00:00
Eric Seidel	6d0e4a1193	chore(deps): bump Rust and Dart dependencies (#315) ... * chore(deps): bump Rust and Dart dependencies Bump Rust dependencies in library/ and patch/: - comde: 0.2.3 → 0.3.1 (library), 0.2.3 → 0.3.0 (patch) - zip: 0.6.4 → 3.0.0 (breaking: FileOptions → SimpleFileOptions) - android_logger: 0.13.0 → 0.15.0 - mockall: 0.12.1 → 0.13.1 - serial_test: 2.0.0 → 3.2.0 - cbindgen: 0.24.0 → 0.28.0 Bump Dart dev dependency in shorebird_code_push/: - ffigen: upper bound <17.0.0 → <19.0.0 Updated zip API usage (FileOptions → SimpleFileOptions) and adjusted test assertion for changed error message. Binary size impact (macOS release, arm64): - libupdater.a: +83 KB (+0.28%) - libupdater.dylib: +34 KB (+0.75%) Closes #206, #271, #273. * chore: add EOCD to cspell dictionary The zip 3.0 crate changed its error message to reference "EOCD" (End of Central Directory), which cspell doesn't recognize.	2026-03-30 15:50:44 -07:00
Eric Seidel	ed8cea1463	fix: improve inflate error handling and validate compressed patches (#314) ... * fix: improve inflate error handling and validate compressed patches Addresses #2989: "pipe reader has been dropped" / "failed to fill whole buffer" errors during patch inflation were masking the real root cause. Three changes: 1. Join the decompression thread and propagate its error as the primary failure, rather than fire-and-forget logging. The patching thread's broken-pipe error is a side-effect, not the cause. Also drop the pipe reader before joining to avoid deadlock when patching fails. 2. Validate the downloaded compressed patch (non-empty, valid zstd magic bytes) before attempting decompression, so corrupt/truncated downloads produce a clear error instead of cryptic pipe errors. 3. Log the download size in download_to_path to help diagnose truncated downloads in the field. * docs: expand comment on drop(fresh_r) to clarify deadlock risk * feat: log app_id, patch number, and version before download * test: add inflate tests for corrupt data and invalid magic * test: cover decompression-error-as-primary-cause path in inflate Uses a valid zstd frame followed by a corrupt second frame so that bipatch::Reader::new succeeds but decompression fails midway, verifying that the decompression error is reported as the primary cause.	2026-03-27 12:54:00 -07:00
Brandon DeRosier	eeec42efb7	feat: add enhanced error messages for file operations (#310) ... * feat: add enhanced error messages for file operations Add a file_errors module that provides context-aware error messages for file operations. When file operations fail, users now see: - The specific operation that failed (create, read, write, rename, etc.) - The full path involved - Helpful hints about possible causes based on error type - Android-specific hints for permission errors (SELinux, Work Profile, MDM/Knox policies, app cloning features) This helps diagnose issues like "Permission denied (os error 13)" by indicating which operation failed and suggesting possible causes.	2026-02-04 12:36:39 -08:00
Eric Seidel	08fb9df932	fix: Rare bug if rollback happens during second update call ... The scenario is: 1. User is running patch 2 (booted successfully, so last_booted_patch = 2) 2. While the app is still running, they call the check-for-update API 3. Patch 3 is downloaded and installed (next_boot_patch = 3) 4. Before the app restarts, they check again and patch 4 is available 5. The buggy code is supposed to delete patch 3 (never booted), but instead deletes patch 2 (the last known-good patch) 6. Patch 4 is set as next_boot_patch If patch 4 boots fine, nobody notices. But if patch 4 fails to boot and the system tries to roll back to patch 2, those artifacts are gone.	2026-01-29 10:18:14 -08:00
Eric Seidel	8691c8f60e	feat: add verification_mode config option (#308) ... * feat: move patch verification from boot time to install time * feat: make it switchable * chore: update comments * fix: test invalid yaml * chore: update readme * feat: add more comments to readme * doc: more readme updates * chore: rename to patch_verification	2026-01-11 15:04:59 -08:00
Bryan Oltman	9db198a634	fix: checking for update should not overwrite good next patch (#307) ... * chore: remove unnecessary mutability of self in next_boot_patch * fix: checking for update should not overwrite good next patch	2025-12-19 13:18:08 -05:00
Bryan Oltman	58a5bcc0b0	chore: remove unnecessary mutability of self in next_boot_patch (#305)	2025-12-19 10:02:33 -05:00
Bryan Oltman	76f005940d	feat: add uuid to updater state, patch check request (#300) ... * feat: add uuid to updater state * add client_id to patch check request * cleanup * comments * cleanup * more comments * delete commented-out code * Update library/src/cache/updater_state.rs Co-authored-by: Eric Seidel <eric@shorebird.dev> * formatting --------- Co-authored-by: Eric Seidel <eric@shorebird.dev>	2025-10-29 15:57:06 -04:00
Bryan Oltman	8bfe1bac47	fix: separate validation checks from next_boot_patch getter (#297) ... * fix: separate validation checks from next_boot_patch getter * coverage * coverage * coverage	2025-10-08 17:55:29 -04:00
Bryan Oltman	ab23721e35	fix: roll back patches in check_for_downloadable_update (#270) ... * fix: roll back patches in check_for_downloadable_update * Update docs	2025-02-12 20:29:06 +00:00
Bryan Oltman	78c84e5bf7	chore: more logging	2025-02-07 14:18:03 -05:00
Bryan Oltman	67f8643242	chore: add more info-level logging around patch downloads (#267)	2025-02-07 14:07:19 -05:00
Bryan Oltman	a4a7255796	feat: configure logging on linux (#263)	2025-01-28 10:28:48 -05:00
Bryan Oltman	69468a0c9f	fix multiple definitions of patch_base	2025-01-24 15:00:09 -05:00
Bryan Oltman	707346df33	fix multiple definitions of patch_base	2025-01-24 14:55:41 -05:00
Bryan Oltman	ba52a62b5d	feat: change updater to support intel macs (#262) ... * feat: change updater to support intel macs * cleanup * Fix targeting in patch_base function	2025-01-24 13:58:30 -05:00
Bryan Oltman	71b5ed65fa	feat: add windows support (#259) ... * feat: add windows support * Cleanup	2024-12-20 14:35:28 -08:00
Felix Angelov	54e1e2ce2f	fix(updater): re-export shorebird_check_for_update for backward compat (#258)	2024-12-19 15:51:07 -06:00
Bryan Oltman	38aadee1c5	feat: enable updater logging on windows (#256)	2024-12-18 14:10:37 -08:00
Bryan Oltman	a6c761f50f	fix: build updater with static runtime to prevent flutter engine build linking errors (#254) ... * fix: build updater with static runtime to prevent flutter engine build linking errors * Update library/.cargo/config.toml Co-authored-by: Eric Seidel <eric@shorebird.dev> * cspell --------- Co-authored-by: Eric Seidel <eric@shorebird.dev>	2024-12-17 21:34:20 +00:00
Bryan Oltman	1e4efce65f	fix: make tests pass on Windows (#252) ... * fix: make tests pass on Windows * Run ci on all supported building OSes * Add os name to CI step * Build rust crates on all oses * tweak * Only run rust on multiple oses for now	2024-12-11 12:09:04 -05:00
Bryan Oltman	b4775d30dd	feat: support macOS (#247) ... * feat: support macOS * fix tests	2024-12-05 17:53:45 -05:00