From 2634e6a24ea49231a41ee79dae8e84ba41676a53 Mon Sep 17 00:00:00 2001 From: MotherSphere Date: Mon, 20 Jul 2026 18:03:11 +0200 Subject: [PATCH] fix: state-integrity pass on the update loop (detail by name, honest checks, badge lifecycle) Five confirmed audit findings, all in the category 'the UI lies about state or loses it': - The open detail page was tracked by raw index into the catalog vector, which every refresh replaces AND reorders (GitHub sorts by last push): a refresh landing while a detail page was open silently swapped it to a different app's page. The selection is now keyed by repo name and resolved through App::active_repo(); a repo that vanished falls back to the grid instead of showing someone else's detail. - Update badges were never cleared: after a successful update the card kept showing 'Update vX -> vX' until the next global check, and an uninstalled app kept its badge. Both paths now clear the entry. - The launcher update check swallowed every failure into 'up to date': offline, rate-limited, or a bad tag all pretended the check ran. check_launcher_update now returns Result; failures surface (as an error toast for a manual click, in the status line for the automatic check) and never claim currency. 'Up to date' only toasts on a manual check - the automatic boot check stays quiet - and the boot no longer runs the check twice (the direct dispatch duplicated the CheckUpdates-chained one). - auto_check_updates now defaults ON: a store that never checks leaves its update badges permanently invisible. The check is batched and makes zero API calls for apps pinned to a fixed tag. - The scan cache was written on every scan but never read: with the startup scan disabled, every boot showed '0 apps'. Boot now restores the cached scan when the scan is skipped. --- src/github.rs | 28 ++++++++--------- src/main.rs | 41 ++++++++++++++++++++----- src/message.rs | 14 +++++++-- src/persistence.rs | 10 +++++++ src/state.rs | 17 ++++++++--- src/ui/app_grid.rs | 34 +++++++++++---------- src/ui/settings.rs | 2 +- src/update.rs | 75 ++++++++++++++++++++++++++++++++-------------- 8 files changed, 154 insertions(+), 67 deletions(-) diff --git a/src/github.rs b/src/github.rs index 44506d6..aaf8283 100644 --- a/src/github.rs +++ b/src/github.rs @@ -27,9 +27,9 @@ const MAX_CONCURRENT_REPO_FETCHES: usize = 8; pub use crate::download::{apply_launcher_update, download_launcher_asset, download_release_asset}; pub use crate::persistence::{ colony_apps_dir, colony_data_dir, installed_app_path, load_favorites, - load_installed_version, load_preferences, load_repos_cache, read_repo_doc, save_favorites, - save_installed_asset, save_installed_version, save_preferences, save_repos_cache, save_scan_cache, - CachedApp, UserPreferences, + load_installed_version, load_preferences, load_repos_cache, load_scan_cache, read_repo_doc, + save_favorites, save_installed_asset, save_installed_version, save_preferences, + save_repos_cache, save_scan_cache, CachedApp, UserPreferences, }; use crate::persistence::save_repo_doc; use crate::persistence::save_repo_icon; @@ -832,21 +832,21 @@ pub fn launcher_asset_name() -> String { /// Check if a newer version of the Colony launcher itself is available. /// Returns Some((latest_tag, asset_filename)) if an update exists, None otherwise. +/// `Ok(None)` means the check RAN and Colony is current; failures propagate so +/// the UI never reports "up to date" when the check could not run at all +/// (offline, rate limited, or an unparseable release tag). pub async fn check_launcher_update( client: &reqwest::Client, -) -> Option<(String, String)> { - let latest_tag = fetch_latest_release_tag_for(client, LAUNCHER_OWNER, LAUNCHER_REPO) - .await - .ok()?; +) -> Result> { + let latest_tag = + fetch_latest_release_tag_for(client, LAUNCHER_OWNER, LAUNCHER_REPO).await?; - let current = parse_version_tag(APP_VERSION)?; - let latest = parse_version_tag(&latest_tag)?; + let current = parse_version_tag(APP_VERSION) + .ok_or_else(|| anyhow::anyhow!("unparseable app version '{APP_VERSION}'"))?; + let latest = parse_version_tag(&latest_tag) + .ok_or_else(|| anyhow::anyhow!("unrecognized release tag '{latest_tag}'"))?; - if latest > current { - Some((latest_tag, launcher_asset_name())) - } else { - None - } + Ok((latest > current).then(|| (latest_tag, launcher_asset_name()))) } // --- Offline cache --- diff --git a/src/main.rs b/src/main.rs index b393d37..e24176c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -69,11 +69,33 @@ impl App { // the boot path (dispatched as a Rescan task below) so the window // appears immediately instead of after a recursive directory walk. let should_scan = prefs.scan_on_startup.unwrap_or(true); - let applications: Vec = Vec::new(); + let applications: Vec = if should_scan { + Vec::new() + } else { + // The startup scan is disabled: restore the last scan from cache + // (which was written on every scan but never read back) instead of + // greeting the user with a permanently empty local-apps grid. + github::load_scan_cache() + .unwrap_or_default() + .into_iter() + .map(|c| scan::Application { + name: c.name, + exec: c.exec, + icon: c.icon, + category: scan::AppCategory::from_name(&c.category), + origin: match c.origin.as_str() { + "Windows" => scan::AppOrigin::Windows, + "Colony" => scan::AppOrigin::Colony, + "Linux" => scan::AppOrigin::Linux, + _ => scan::AppOrigin::External, + }, + }) + .collect() + }; let status_message = if should_scan { i18n::t("scanning") } else { - i18n::t_fmt("apps_found", &[("count", "0")]) + i18n::t_fmt("apps_found", &[("count", &applications.len().to_string())]) }; let sections = sections::load_sections(); @@ -168,7 +190,10 @@ impl App { .language .clone() .unwrap_or_else(|| i18n::current_lang().to_string()), - auto_check_updates: prefs.auto_check_updates.unwrap_or(false), + // ON by default: a store that never checks for updates leaves its + // badges permanently invisible. The check is cheap (batched; zero + // API calls for apps pinned to a fixed tag). + auto_check_updates: prefs.auto_check_updates.unwrap_or(true), // Appearance extras font_size: prefs.font_size.clone().unwrap_or_else(|| "default".into()), animations: prefs.animations.unwrap_or(true), @@ -218,11 +243,11 @@ impl App { set_active_accent(accent_key_to_color(&app.selected_accent)); } - let launcher_check_task = if app.auto_check_updates { - Task::done(Message::CheckLauncherUpdate) - } else { - Task::none() - }; + // No direct launcher-update check here: with auto-check on, the boot + // catalog fetch chains GitHubReposFetched -> CheckUpdates -> + // CheckLauncherUpdate already - a second dispatch meant every boot ran + // the check twice. + let launcher_check_task = Task::none(); let tutorial_task = if app.show_first_launch { ui::fetch_bounds_task() diff --git a/src/message.rs b/src/message.rs index 487a206..c7bff7f 100644 --- a/src/message.rs +++ b/src/message.rs @@ -16,7 +16,9 @@ pub enum Message { Rescan, RescanCompleted(Result, String>), LaunchApp(String), - ColonyRepoSelected(usize), + /// Open a repo's detail page, keyed by repo NAME (indexes go stale when a + /// catalog refresh reorders the vector). + ColonyRepoSelected(String), ColonyRepoBack, ClearStatus, FontLoaded(Result<(), font::Error>), @@ -78,8 +80,14 @@ pub enum Message { DetailTabSelected(DetailTab), OpenUrl(String), // Launcher self-update - CheckLauncherUpdate, - LauncherUpdateChecked(Option<(String, String)>), // Option<(tag, asset_filename)> + /// `manual` is true when the user clicked "Check for updates": only a + /// manual check gets an "up to date" toast (an automatic one would toast + /// on every boot) and network failures surface as errors instead of the + /// check lying that Colony is current. + CheckLauncherUpdate { manual: bool }, + /// (manual, result of the check). Ok(None) = genuinely up to date; + /// Err = the check could not run (network/rate limit/bad tag). + LauncherUpdateChecked(bool, Result, String>), DownloadLauncherUpdate, LauncherDownloadProgress(f32), LauncherDownloadCompleted(Result), diff --git a/src/persistence.rs b/src/persistence.rs index 52774c7..3e8179f 100644 --- a/src/persistence.rs +++ b/src/persistence.rs @@ -243,6 +243,16 @@ pub struct CachedApp { pub origin: String, } +/// Load the cached application scan (`None` if absent or unreadable). Read at +/// boot when the startup scan is disabled, so the local-apps grid restores the +/// last known state instead of showing "0 apps" at every launch. +pub fn load_scan_cache() -> Option> { + let path = scan_cache_path().ok()?; + let json = std::fs::read_to_string(path).ok()?; + let cached: CachedScanResult = serde_json::from_str(&json).ok()?; + Some(cached.apps) +} + /// Save scanned applications to cache. pub fn save_scan_cache(apps: &[CachedApp]) -> Result<()> { let timestamp = std::time::SystemTime::now() diff --git a/src/state.rs b/src/state.rs index 286464e..8f632a6 100644 --- a/src/state.rs +++ b/src/state.rs @@ -140,7 +140,10 @@ pub struct App { pub sections: Vec
, pub selected_section: usize, pub status_message: String, - pub active_colony_repo: Option, + /// The repo whose detail page is open, tracked by NAME: the catalog vector + /// is replaced and reordered by every refresh (GitHub sorts by last push), + /// so an index here would silently swap the open page to a different app. + pub active_colony_repo: Option, pub font: Font, // GitHub / OAuth pub github_state: GitHubState, @@ -236,6 +239,13 @@ impl App { &self.colony_repo_list } + /// Resolve the repo whose detail page is open, surviving catalog refreshes + /// (`None` if the repo disappeared from the catalog). + pub fn active_repo(&self) -> Option<&ColonyRepo> { + let name = self.active_colony_repo.as_deref()?; + self.colony_repo_list.iter().find(|r| r.name == name) + } + /// Filter local applications by the currently selected section. pub fn filtered_applications(&self) -> Vec<&Application> { let query = self.search_query.to_lowercase(); @@ -262,15 +272,14 @@ impl App { } /// Filter Colony repos by the currently selected section's category. - pub fn filtered_colony_repos(&self) -> Vec<(usize, &ColonyRepo)> { + pub fn filtered_colony_repos(&self) -> Vec<&ColonyRepo> { let query = self.search_query.to_lowercase(); let selected_section = self.sections.get(self.selected_section); let is_favorites = selected_section.map(|s| s.is_favorites).unwrap_or(false); self.colony_repos() .iter() - .enumerate() - .filter(|(_, repo)| { + .filter(|repo| { if is_favorites { if !self.is_favorite(&repo.name) { return false; diff --git a/src/ui/app_grid.rs b/src/ui/app_grid.rs index d0c61cf..8dc2631 100644 --- a/src/ui/app_grid.rs +++ b/src/ui/app_grid.rs @@ -273,12 +273,11 @@ impl App { } pub(crate) fn view_content(&self) -> Element<'_, Message> { - // Show Colony repo detail view if one is selected - if let Some(index) = self.active_colony_repo { - let repos = self.colony_repos(); - if let Some(repo) = repos.get(index) { - return self.view_colony_detail(repo); - } + // Show Colony repo detail view if one is selected (resolved by name, + // so a catalog refresh landing mid-view cannot swap the page to a + // different app; a repo that vanished falls back to the grid). + if let Some(repo) = self.active_repo() { + return self.view_colony_detail(repo); } let search_input = text_input(&crate::i18n::t("search_placeholder"), &self.search_query) @@ -399,8 +398,8 @@ impl App { iced::widget::responsive(move |size| { let cols = App::grid_columns(size.width); let mut all_cards: Vec> = Vec::new(); - for (index, repo) in &colony_repos { - all_cards.push(self.view_colony_card(*index, repo)); + for repo in &colony_repos { + all_cards.push(self.view_colony_card(repo)); } for app in &filtered { all_cards.push(self.view_app_card(app)); @@ -410,7 +409,7 @@ impl App { .into() } - pub(crate) fn view_colony_grid<'a>(&'a self, repos: &[(usize, &'a ColonyRepo)]) -> Element<'a, Message> { + pub(crate) fn view_colony_grid<'a>(&'a self, repos: &[&'a ColonyRepo]) -> Element<'a, Message> { if repos.is_empty() { return container( text(crate::i18n::t("no_apps_found")) @@ -424,19 +423,19 @@ impl App { .into(); } - let repos: Vec<(usize, &'a ColonyRepo)> = repos.to_vec(); + let repos: Vec<&'a ColonyRepo> = repos.to_vec(); iced::widget::responsive(move |size| { let cols = App::grid_columns(size.width); let mut cards: Vec> = Vec::new(); - for (index, repo) in &repos { - cards.push(self.view_colony_card(*index, repo)); + for repo in &repos { + cards.push(self.view_colony_card(repo)); } scrollable(build_card_grid(cards, cols)).height(Fill).into() }) .into() } - pub(crate) fn view_colony_card<'a>(&'a self, index: usize, repo: &'a ColonyRepo) -> Element<'a, Message> { + pub(crate) fn view_colony_card<'a>(&'a self, repo: &'a ColonyRepo) -> Element<'a, Message> { let category = AppCategory::from_name(&repo.manifest.category); let tint = theme::app_tint(&repo.name); let tile = self.icon_tile(&repo.name, tint, category.glyph()); @@ -483,8 +482,13 @@ impl App { .align_y(iced::Alignment::Center) .width(Fill); - let selected = self.active_colony_repo == Some(index); - self.cell_shell(content.into(), Message::ColonyRepoSelected(index), selected, tint) + let selected = self.active_colony_repo.as_deref() == Some(repo.name.as_str()); + self.cell_shell( + content.into(), + Message::ColonyRepoSelected(repo.name.clone()), + selected, + tint, + ) } pub(crate) fn view_app_card(&self, app: &Application) -> Element<'_, Message> { diff --git a/src/ui/settings.rs b/src/ui/settings.rs index cc990fb..38dab69 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -654,7 +654,7 @@ impl App { if is_checking { btn.into() } else { - btn.on_press(Message::CheckLauncherUpdate).into() + btn.on_press(Message::CheckLauncherUpdate { manual: true }).into() } }; diff --git a/src/update.rs b/src/update.rs index cb06f86..1479aa9 100644 --- a/src/update.rs +++ b/src/update.rs @@ -64,10 +64,7 @@ impl App { /// Rebuild `detail_blocks` for the currently-viewed (repo, tab) if that /// key differs from the last parse. Cheap no-op when the cache is valid. pub fn refresh_detail_markdown(&mut self) { - let Some(repo) = self - .active_colony_repo - .and_then(|idx| self.colony_repos().get(idx).cloned()) - else { + let Some(repo) = self.active_repo().cloned() else { self.detail_blocks.clear(); self.detail_md_source = None; self.detail_is_placeholder = false; @@ -259,8 +256,8 @@ impl App { } } } - Message::ColonyRepoSelected(index) => { - self.active_colony_repo = Some(index); + Message::ColonyRepoSelected(name) => { + self.active_colony_repo = Some(name); self.refresh_detail_markdown(); Task::none() } @@ -532,6 +529,10 @@ impl App { if let Err(e) = github::save_installed_version(&repo_name, &tag) { tracing::warn!("Failed to save version info: {e}"); } + // The just-installed version IS the one the badge was + // advertising: clear it, or the card keeps showing + // "Update vX -> vX" until the next global check. + self.available_updates.remove(&repo_name); let display_name = path .file_name() .map(|n| n.to_string_lossy().to_string()) @@ -601,6 +602,8 @@ impl App { } Message::UninstallColonyApp(repo_name) => { self.confirm_uninstall = None; + // An uninstalled app has no meaningful "update available". + self.available_updates.remove(&repo_name); match github::colony_apps_dir() { Ok(apps_dir) => { let app_dir = apps_dir.join(&repo_name); @@ -788,9 +791,12 @@ impl App { && !self.show_first_launch && self.active_colony_repo.is_none() => { - let first = self.filtered_colony_repos().first().map(|(idx, _)| *idx); - if let Some(idx) = first { - self.active_colony_repo = Some(idx); + let first = self + .filtered_colony_repos() + .first() + .map(|r| r.name.clone()); + if let Some(name) = first { + self.active_colony_repo = Some(name); // Refresh the (repo, tab) markdown cache — the // detail view reads only cached blocks/placeholder // now, so opening a repo must recompute them. @@ -833,7 +839,7 @@ impl App { "apps_found", &[("count", &self.applications.len().to_string())], ); - return Task::done(Message::CheckLauncherUpdate); + return Task::done(Message::CheckLauncherUpdate { manual: false }); } let token = if let GitHubState::Connected { session, .. } = &self.github_state { @@ -893,7 +899,7 @@ impl App { self.push_notification(msg, NotificationLevel::Info) }; // Also check for launcher self-update - Task::batch([notif_task, Task::done(Message::CheckLauncherUpdate)]) + Task::batch([notif_task, Task::done(Message::CheckLauncherUpdate { manual: false })]) } Message::ToggleFavorite(name) => { if let Some(pos) = self.favorites.iter().position(|f| f == &name) { @@ -1050,7 +1056,7 @@ impl App { Task::none() } // --- Launcher self-update --- - Message::CheckLauncherUpdate => { + Message::CheckLauncherUpdate { manual } => { if self.is_checking_launcher_update { return Task::none(); } @@ -1064,27 +1070,52 @@ impl App { Task::perform( async move { - let client = github::build_update_client(token.as_deref()).ok()?; - github::check_launcher_update(&client).await + let client = + github::build_update_client(token.as_deref()).map_err(|e| e.to_string())?; + github::check_launcher_update(&client) + .await + .map_err(|e| e.to_string()) }, - Message::LauncherUpdateChecked, + move |result| Message::LauncherUpdateChecked(manual, result), ) } - Message::LauncherUpdateChecked(result) => { + Message::LauncherUpdateChecked(manual, result) => { self.is_checking_launcher_update = false; match result { - Some((ref tag, _)) => { + Ok(Some((tag, asset))) => { let tag_display = tag.clone(); - self.launcher_update_available = result; + self.launcher_update_available = Some((tag, asset)); self.push_notification( i18n::t_fmt("launcher_update_available", &[("version", &tag_display)]), NotificationLevel::Info, ) } - // Give explicit feedback that the check ran and Colony is - // current, instead of silently doing nothing. - None => self - .push_notification(i18n::t("launcher_up_to_date"), NotificationLevel::Info), + Ok(None) => { + self.launcher_update_available = None; + self.status_message = i18n::t("launcher_up_to_date"); + if manual { + // Explicit feedback for an explicit click; the + // automatic boot check stays quiet when current. + self.push_notification( + i18n::t("launcher_up_to_date"), + NotificationLevel::Info, + ) + } else { + Task::none() + } + } + Err(e) => { + // The check DID NOT run: never claim "up to date". + self.status_message = i18n::t_fmt("github_api_error", &[("error", &e)]); + if manual { + self.push_notification( + i18n::t_fmt("github_api_error", &[("error", &e)]), + NotificationLevel::Error, + ) + } else { + Task::none() + } + } } } Message::DownloadLauncherUpdate => {