fix(gui): stop offering in-app updates where no artifact ships - #838
fix(gui): stop offering in-app updates where no artifact ships#8384ni1ak wants to merge 3 commits into
Conversation
Greptile SummaryThis PR prevents unsupported platforms from offering in-app update operations while preserving the existing macOS and Windows behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported Linux consent bypass is now gated both where the prompt opens and where consent triggers the check.
|
| Filename | Overview |
|---|---|
| crates/openlogi-desktop/src/platform/updater.rs | Defines the shared platform capability and consistently gates startup checks and automatic installation. |
| crates/openlogi-desktop/src/windows/update_consent.rs | Completes the prior consent-path fix by preventing accepted consent from invoking the updater on unsupported platforms. |
| crates/openlogi-desktop/src/windows/settings/updates.rs | Replaces unusable updater controls with a package-managed update explanation when in-app artifacts do not ship. |
| crates/openlogi-desktop/src/app/menu.rs | Makes manual update actions platform-aware while continuing to open the Updates page. |
| crates/openlogi-desktop/src/runtime.rs | Suppresses the first-run update-consent prompt on platforms without in-app update artifacts. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Open update entry point] --> B{IN_APP_UPDATES?}
B -->|Yes| C[Use in-app updater]
B -->|No| D[Open package-managed Updates page]
C --> E[Check, download, and install]
D --> F[Show installed version and releases link]
Reviews (5): Last reviewed commit: "fix(gui): stop describing how OpenLogi w..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
AprilNEA
left a comment
AprilNEA
left a comment
There was a problem hiding this comment.
The core Linux updater gating is sound and covers the startup, automatic, manual, Settings, and consent paths. The new Linux-facing UI still makes contradictory and incorrect claims about how the current installation is updated; please address the inline comments before merging. Also change the closing reference from Fixes #769 to Fixes #605, because #769 is already closed as a duplicate and the open tracking issue would otherwise remain open. Per the project UI review requirements, please add a Linux runtime check and screenshot of the resulting Updates page.
| .child(concat!("OpenLogi ", env!("CARGO_PKG_VERSION"))), | ||
| ) | ||
| .child(div().text_caption().text_color(pal.text_muted).child(tr!( | ||
| "Installed from a distribution package — updates come from your package manager." |
There was a problem hiding this comment.
[P2] Do not infer the installation method from target_os. This branch runs for every Linux build, including the source-build path documented in docs/INSTALL-linux.md and reported in #605, yet it tells all of them they were installed from a distribution package. The official .deb/.rpm instructions also download artifacts manually and configure no package repository, so a package manager will not discover future releases on its own. Please use installation-neutral copy such as ‘In-app updates are unavailable on Linux; update using the method you installed OpenLogi with,’ or detect the actual install source.
| .icon(IconName::ArrowDown) | ||
| .resettable(false) | ||
| .group(hero) | ||
| .group(SettingGroup::new().item(SettingItem::render(move |_, _, _| update_source(pal)))) |
There was a problem hiding this comment.
[P2] Avoid reusing the updater-enabled source footer here. On this new Linux page, update_source still labels GitHub Releases as the update source and says OpenLogi connects when automatic checks are enabled or Check for Updates is clicked, even though this PR removes those controls and suppresses every check. Render a source/footer variant for this page so it presents one consistent update path.
| // packages only), running the check just resolves to "no release asset | ||
| // matched the current platform". Open the Updates page anyway — it says | ||
| // where updates come from on this install. | ||
| if crate::platform::updater::IN_APP_UPDATES |
There was a problem hiding this comment.
[P2] This deliberately skips the check on Linux, but the invoking menu item remains ‘Check for Updates…’ and the deeplink/command contract still says it runs a check. A Linux user selecting it gets navigation only. Please rename or hide this action on unsupported platforms, or give it platform-neutral ‘Updates…’ semantics.
| /// resolves to "no release asset matched the current platform", which reads as | ||
| /// a failure rather than as the design it is, so Linux skips the check and the | ||
| /// Updates page says where updates actually come from. | ||
| pub const IN_APP_UPDATES: bool = !cfg!(target_os = "linux"); |
There was a problem hiding this comment.
[P3] Make this capability fail closed. The static manifest only ships in-app artifacts for macOS and Windows, but !cfg!(target_os = "linux") enables every other current or future target and recreates the same NoMatchingAsset failure there. Prefer a positive macOS/Windows predicate.
666a08c to
4f7e9f4
Compare
davidbudnick
added
type: bug
A Linux release publishes distro packages only — `.deb`, `.rpm`, `.pkg.tar.zst` — and `xtask release latest-json` deliberately classifies none of them into the update manifest, because those installs update through the package manager. The GUI asked anyway, so every Linux user who opened Settings → Updates and clicked Check got a red "Update failed: no release asset matched the current platform (linux/x86_64)". Gate the updater on a single `IN_APP_UPDATES` const: the launch check, the auto-install observer, and the menu action skip the check where a release carries no in-place-updatable artifact, and the Updates page shows the running version plus where updates come from instead of a check button and two switches that cannot do anything. macOS and Windows are unchanged. The new string is added to every shipped catalog at the same position. Fixes AprilNEA#769
The consent window asks whether to turn the update check on, and its "Enable" handler ran one check immediately without consulting `IN_APP_UPDATES` — so a first-run Linux user still reached the asset lookup and its "no release asset matched the current platform" failure, one dialog before the Updates page that now avoids exactly that. Skip the window entirely where a release ships no in-place-updatable artifact: offering to enable a check whose every outcome is that failure is the same defect one layer up. The handler keeps its own gate as well, since it is reachable from the window whenever the window is. Found by the Greptile review on this PR.
Four review findings on this PR: The capability was `!cfg!(target_os = "linux")`, which enables every target that is not Linux — including any added later, which would inherit the same "no release asset matched the current platform" failure without anyone choosing it. `xtask release latest-json` classifies exactly two artifacts, so name those: `cfg!(any(target_os = "macos", target_os = "windows"))`. The page told every Linux build it came from a distribution package. It cannot know that: source builds follow `docs/INSTALL-linux.md`, and the official `.deb`/`.rpm` route is a manual download that configures no repository, so no package manager will find the next release for those either. Say only what is true — in-app updates are unavailable here, update the way you installed it. It also reused `update_source`, which calls GitHub Releases the *update source* and explains when OpenLogi connects for automatic checks or a manual Check for Updates — controls this page removes. It gets its own footer: the link is where to read the changelog, and the one outbound request named is the one opening that page makes. Finally the menu item still read "Check for Updates…" while only navigating. It reads "Updates…" where no check can run. Found in review by @AprilNEA.
09e77f2 to
104f790
Compare
3 participants
Summary
Settings → Updates offered a check that can only fail on Linux.
Split out of #802 at your request.
Fixes #769
Changes
A Linux release publishes distro packages only —
.deb,.rpm,.pkg.tar.zst— andxtask release latest-jsondeliberately classifies none ofthem into the update manifest, because those installs update through the package
manager. The GUI asked anyway, so opening Settings → Updates and clicking Check
produced a red "Update failed: no release asset matched the current platform
(linux/x86_64)".
IN_APP_UPDATESconst inplatform::updaterstates whether this build hasan in-place-updatable artifact at all.
check_for_updatesskipthe check where it does not. The menu item stays and still opens the page,
which now explains where updates come from.
check button and two switches that cannot do anything.
The one new string is added to every shipped catalog at the same position.
Testing
Linux, x86_64, Rust 1.98.0:
All green.
Not run on this host:
tests (macos),cargo-deny, macOS clippy (no macOSSDK here). The macOS and Windows paths are unchanged by construction —
IN_APP_UPDATESistruethere and every branch behaves as before — but thatis an argument, not a test run.
Not runtime-tested. To verify: open Settings → Updates on a Linux install
and confirm it states the package manager rather than offering a check; on macOS
confirm the check still runs and reports normally.
Note on ordering: this adds a string to every catalog that exists today. If
the Turkish locale PR lands first, this branch needs a rebase so
tr.ymlgetsthe same key before the parity test passes — happy to rebase whenever it suits
you, and the same the other way round.