From 945883bb9ddfdbcf593d3d7283a14e6ba0ddf216 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 27 Aug 2026 10:24:24 -0700 Subject: [PATCH] Actually offer a preview the release it precedes, and fix two more gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An Opus review of the previous commit found its headline claim didn't hold: a preview and the release it precedes compute to the identical numeric version by construction, but check_for_updates compared with a strict `>` against the bare CARGO_PKG_VERSION (never the suffixed display string), so `(0,4,13) > (0,4,13)` is false and the release was never offered. Plain semver ordering doesn't make a `-preview.` suffix sort below the same numeric release on its own here, since the comparison never sees the suffix at all. pick_update now takes is_preview_build, derived from whether TRIPLE_C_BUILD_SUFFIX was baked in, and relaxes that one comparison to `>=` — so "a release exists at my own number" reads as an update. A production build still requires strictly newer. Also: ported build-app.yml's `git tag --points-at HEAD` guard into the preview version computation. Without it, workflow_dispatch (which this workflow allows on main, not just PR builds) run on a commit a release was already cut from would compute one past that release — reintroducing "preview outranks production" through the manual-dispatch door. And corrected two comments that claimed the prerelease filter was currently a no-op: backfill-releases.yml mirrors every Gitea release to GitHub unfiltered, prerelease flag included, so it's real defence-in-depth against a dispatched backfill leaking a preview release, not a no-op. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ --- .gitea/workflows/build-app-preview.yml | 45 ++++++++--- app/src-tauri/src/commands/update_commands.rs | 77 ++++++++++++++++--- app/src-tauri/src/models/update_info.rs | 16 +++- 3 files changed, 113 insertions(+), 25 deletions(-) diff --git a/.gitea/workflows/build-app-preview.yml b/.gitea/workflows/build-app-preview.yml index 70ffc2c..ac8b5f3 100644 --- a/.gitea/workflows/build-app-preview.yml +++ b/.gitea/workflows/build-app-preview.yml @@ -43,13 +43,18 @@ name: Build App (Preview) # prunes previous previews itself, keeping the newest few. Bundles are ~130 MB a # release; the point of a preview is the build you are testing now. # -# Nothing here reaches GitHub: `build-app.yml` is the only workflow that -# mirrors a release there, and it does so inline, gated on `prerelease: false` -# — a preview release is never a candidate. (The previous mechanism here, -# `sync-release.yml`, was `workflow_dispatch`-only and read +# A preview release is not meant to reach GitHub. `build-app.yml`'s inline +# mirror never sees one (it only runs for its own `push`-triggered release), +# but `backfill-releases.yml` pulls every Gitea release unfiltered and would +# faithfully forward a preview's `prerelease: true` if it were ever dispatched +# while one existed — so `GitHubRelease::prerelease` in `update_commands.rs` +# is real defence, not a no-op, even though the `preview-` tag shape +# (never valid semver) already blocks it independently. (The previous +# mechanism here, `sync-release.yml`, was `workflow_dispatch`-only and read # `gitea.event.release.*` fields that are only ever populated by a `release` # trigger, so it could never have actually run; deleted rather than fixed, -# since build-app.yml's inline mirror already does its job. See triple-c#32.) +# since build-app.yml's inline mirror already does what it was meant to do +# for real releases. See triple-c#32.) env: GITEA_URL: ${{ gitea.server_url }} @@ -115,15 +120,37 @@ jobs: # Reading the same `v${MAJOR_MINOR}.*` tags (including the `-mac` # / `-win` suffixed ones a partially-published release can leave # behind) means a preview built right before a release computes the - # exact number that release is about to take — so semver already - # orders `0.4.12-preview. < 0.4.12`, and a preview user is - # offered the real release the moment it ships. See triple-c#32. + # exact number that release is about to take — e.g. `0.4.13` for + # both. That makes the two numerically *equal*, not "preview less + # than release" — plain semver ordering does not make a + # `-preview.` suffix sort lower on its own here, because + # `check_for_updates` compares against the bare, stripped + # `CARGO_PKG_VERSION`, never the suffixed display string. What + # closes the loop is `update_commands.rs`'s `is_preview_build` + # check, which relaxes that one comparison to `>=` specifically so + # "a release exists at my own number" reads as an update. See + # triple-c#32. HIGHEST=$(git tag -l "v${MAJOR_MINOR}.*" \ | grep -E "^v${MAJOR_MINOR}\.[0-9]+(-mac|-win)?$" \ | sed -E "s/^v${MAJOR_MINOR}\.([0-9]+).*/\1/" \ | sort -n | tail -1 || true) - if [ -n "$HIGHEST" ]; then + # Mirrors build-app.yml's own `EXISTING` guard: this workflow is + # also `workflow_dispatch`-able on `main`, not just PR-triggered, so + # HEAD can be a commit a release was already cut from. Without this, + # dispatching a preview there would compute `HIGHEST + 1` — one past + # that release — and produce exactly the "preview outranks + # production" failure triple-c#32 was filed over, just reintroduced + # through the manual-dispatch door instead of the automatic one. + EXISTING=$(git tag --points-at HEAD \ + | grep -E "^v${MAJOR_MINOR}\.[0-9]+$" \ + | sed -E "s/^v${MAJOR_MINOR}\.([0-9]+)$/\1/" \ + | sort -n | tail -1 || true) + + if [ -n "$EXISTING" ]; then + echo "HEAD is already tagged v${MAJOR_MINOR}.${EXISTING} — matching it" + PATCH="${EXISTING}" + elif [ -n "$HIGHEST" ]; then echo "Highest patch already used on this line: ${HIGHEST}" PATCH=$((HIGHEST + 1)) else diff --git a/app/src-tauri/src/commands/update_commands.rs b/app/src-tauri/src/commands/update_commands.rs index 8ead9ce..3f99e64 100644 --- a/app/src-tauri/src/commands/update_commands.rs +++ b/app/src-tauri/src/commands/update_commands.rs @@ -69,7 +69,19 @@ pub async fn check_for_updates() -> Result, String> { &[".AppImage", ".deb", ".rpm"] }; - match pick_update(&releases, current_semver, platform_extensions) { + // `current_version` above is always the bare, stripped `CARGO_PKG_VERSION` + // — the preview workflow patches `Cargo.toml` with that before compiling, + // never the `-preview.`-suffixed one `get_app_version()` reports — + // so a preview build and the release it precedes compile to the identical + // numeric tuple by construction (see `build-app-preview.yml`'s "highest + // tag used, +1" computation). A strict `>` therefore never fires for the + // one release a preview most needs to be offered. `is_preview_build` + // relaxes that one comparison to `>=` so "there is a real release at my + // own number" reads as an update, without touching the production case + // — see `pick_update`. + let is_preview_build = option_env!("TRIPLE_C_BUILD_SUFFIX").is_some_and(|s| !s.is_empty()); + + match pick_update(&releases, current_semver, platform_extensions, is_preview_build) { Some(release) => { // Only include assets matching the current platform let assets = release @@ -108,15 +120,22 @@ pub async fn check_for_updates() -> Result, String> { /// /// Three filters, all of which must pass: not a prerelease (see the long /// comment on `GitHubRelease::prerelease`), at least one asset for this -/// platform, and a tag that parses as semver *and* is newer than what is -/// running. A tag that does not parse — a `-preview.` suffix, most -/// realistically — is skipped rather than erroring, the same as it always -/// has been; nothing here changes what an update tag is expected to look -/// like, only what channel it is allowed to come from. +/// platform, and a tag that parses as semver *and* beats what is running. A +/// tag that does not parse — a `-preview.` suffix, most realistically — +/// is skipped rather than erroring, the same as it always has been; nothing +/// here changes what an update tag is expected to look like, only what +/// channel it is allowed to come from. +/// +/// `is_preview_build` relaxes "beats" from `>` to `>=`. A preview build's +/// `current_semver` is the bare number it was compiled with, which is by +/// construction identical to the release it precedes — see the comment at +/// `check_for_updates`'s call site — so a strict `>` would never fire for +/// exactly the release a preview install most needs to be told about. fn pick_update<'a>( releases: &'a [GitHubRelease], current_semver: (u32, u32, u32), platform_extensions: &[&str], + is_preview_build: bool, ) -> Option<&'a GitHubRelease> { releases .iter() @@ -127,7 +146,13 @@ fn pick_update<'a>( .any(|a| platform_extensions.iter().any(|ext| a.name.ends_with(ext))) }) .filter_map(|r| parse_semver_from_tag(&r.tag_name).map(|ver| (r, ver))) - .filter(|(_, ver)| *ver > current_semver) + .filter(|(_, ver)| { + if is_preview_build { + *ver >= current_semver + } else { + *ver > current_semver + } + }) .max_by_key(|(_, ver)| *ver) .map(|(r, _)| r) } @@ -205,19 +230,19 @@ mod tests { #[test] fn a_prerelease_is_never_offered_even_if_its_tag_would_otherwise_win() { let releases = vec![release("v9.9.9", true, &["app-9.9.9.AppImage"])]; - assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).is_none()); + assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).is_none()); } #[test] fn a_release_with_no_asset_for_this_platform_is_skipped() { let releases = vec![release("v0.4.12", false, &["app-0.4.12.msi"])]; - assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).is_none()); + assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).is_none()); } #[test] fn a_release_that_is_not_newer_is_not_offered() { let releases = vec![release("v0.4.10", false, &["app.AppImage"])]; - assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).is_none()); + assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).is_none()); } #[test] @@ -228,7 +253,7 @@ mod tests { release("preview-a1b2c3d", false, &["app.AppImage"]), release("v0.4.12", false, &["app.AppImage"]), ]; - let best = pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).unwrap(); + let best = pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).unwrap(); assert_eq!(best.tag_name, "v0.4.12"); } @@ -239,9 +264,37 @@ mod tests { release("v0.4.13", false, &["app.AppImage"]), release("v0.4.12", false, &["app.AppImage"]), ]; - let best = pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).unwrap(); + let best = pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).unwrap(); assert_eq!(best.tag_name, "v0.4.13"); } + + // ── is_preview_build (>= instead of >) ───────────────────────────────── + + /// The exact scenario triple-c#32 was filed to fix: a preview compiled as + /// `0.4.12-preview.` (bare `CARGO_PKG_VERSION` "0.4.12") must be + /// offered the `v0.4.12` release that follows it, even though the two + /// compute to the identical numeric tuple. + #[test] + fn a_preview_build_is_offered_the_release_it_precedes() { + let releases = vec![release("v0.4.12", false, &["app.AppImage"])]; + assert!(pick_update(&releases, (0, 4, 12), LINUX_EXTENSIONS, false).is_none()); + let best = pick_update(&releases, (0, 4, 12), LINUX_EXTENSIONS, true).unwrap(); + assert_eq!(best.tag_name, "v0.4.12"); + } + + #[test] + fn a_preview_build_is_not_offered_an_older_release() { + let releases = vec![release("v0.4.11", false, &["app.AppImage"])]; + assert!(pick_update(&releases, (0, 4, 12), LINUX_EXTENSIONS, true).is_none()); + } + + #[test] + fn a_production_build_still_requires_strictly_newer() { + // A production build must never treat "equal" as an update — that + // would perpetually re-offer the version already running. + let releases = vec![release("v0.4.12", false, &["app.AppImage"])]; + assert!(pick_update(&releases, (0, 4, 12), LINUX_EXTENSIONS, false).is_none()); + } } /// Check whether a newer container image is available in the registry. diff --git a/app/src-tauri/src/models/update_info.rs b/app/src-tauri/src/models/update_info.rs index 0bcc0f9..478bf3b 100644 --- a/app/src-tauri/src/models/update_info.rs +++ b/app/src-tauri/src/models/update_info.rs @@ -29,10 +29,18 @@ pub struct GitHubRelease { /// Whether GitHub itself has this release marked as a prerelease. /// `#[serde(default)]` rather than required: every response GitHub sends /// carries this, but nothing here should refuse to parse the rest of a - /// release over one missing field. No production release is ever - /// mirrored with this `true` today — see `check_for_updates`, which - /// filters on it explicitly rather than relying on that being an - /// accident of what happens to get mirrored. + /// release over one missing field. Defaults to `false` (offered) rather + /// than `true` (excluded) — a missing field only happens if GitHub's API + /// shape changes, and "API changed, therefore updates silently stop + /// working forever" is the worse failure of the two. + /// + /// `build-app.yml`'s own mirror never publishes a prerelease, but + /// `.gitea/workflows/backfill-releases.yml` forwards every Gitea release + /// unfiltered, `prerelease` included — so if it were ever dispatched + /// while a preview release existed, this field is what stops + /// `check_for_updates` from offering it (the `preview-` tag shape + /// already fails semver parsing independently, but this is real + /// defence-in-depth, not a no-op). #[serde(default)] pub prerelease: bool, }