diff --git a/desktop/packaging/install.sh b/desktop/packaging/install.sh index be5247e..a6e06b3 100755 --- a/desktop/packaging/install.sh +++ b/desktop/packaging/install.sh @@ -128,6 +128,25 @@ say "Installing ${version:-unknown} from the $channel channel" tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT INT TERM +# Tell the app which channel it was installed from. The installer is the only thing +# that knows, and without this the app kept its own `stable` default and a dev install +# checked the stable feed — which advertises an OLDER version — reporting "up to date" +# forever (issue 2183). +# +# A plain file rather than a write into the app's SQLite store: shell has no business +# knowing that schema, and a file it can't misread is the narrowest possible contract. +# The app reads it at startup (src-tauri/src/update.rs, INSTALL_MARKER) and only acts +# when the value CHANGED, so switching channel in the app isn't undone on next launch. +# +# The directory is Tauri's app-data dir for identifier com.fabledsword.thoughtsync; +# both sides hardcode it, so a change to the identifier has to change both. +record_channel() { + marker_dir="${XDG_DATA_HOME:-$HOME/.local/share}/com.fabledsword.thoughtsync" + # Best-effort: a failure here costs the channel setting, not the install, and a + # native install run as root would only be writing into root's home anyway. + mkdir -p "$marker_dir" 2>/dev/null && printf '%s\n' "$channel" > "$marker_dir/install-channel" 2>/dev/null || true +} + # Both native paths install system-wide, so they need root. Resolved once here # rather than duplicated per branch; the AppImage path below never calls this. need_root() { @@ -163,6 +182,7 @@ if have pacman && [ -n "$pkg_url" ]; then curl -fSL -o "$pkg_file" "$pkg_url" need_root $sudo pacman -U --noconfirm "$pkg_file" + record_channel say "Done. Launch ThoughtSync from your application menu, or run thoughtsync." native_update_note exit 0 @@ -178,6 +198,7 @@ if have dpkg && have apt-get && [ -n "$deb_url" ]; then # unconfigured, so `apt-get -f install` is what actually completes that path. $sudo apt-get install -y "$tmp/thoughtsync.deb" || { $sudo dpkg -i "$tmp/thoughtsync.deb" || true; $sudo apt-get -f install -y; } + record_channel say "Done. Launch ThoughtSync from your application menu." native_update_note exit 0 @@ -233,12 +254,13 @@ have update-desktop-database && update-desktop-database "$apps_menu" >/dev/null mkdir -p "$HOME/.local/bin" ln -sf "$dest" "$HOME/.local/bin/thoughtsync" +record_channel + say "Installed to $dest" printf ' Launch it from your application menu, or run \033[1mthoughtsync\033[0m' printf ' (if ~/.local/bin is on your PATH).\n' -# The AppImage CAN update itself, but the app's own channel is a separate, -# stable-by-default setting — installing from dev here does not move it. +# This is the one path where the app can update itself, so say what it will follow. if [ "$channel" = "dev" ]; then - printf ' You installed from the \033[1mdev\033[0m channel — set the app to match in\n' - printf ' Sync → App updates → Development, or in-app updates will follow stable.\n' + printf ' In-app updates will follow the \033[1mdev\033[0m channel.' + printf ' Change it in Sync → App updates.\n' fi diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 02de8c5..1ac2b29 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -72,6 +72,9 @@ pub fn run() { log::info!("opening local store: {}", db_path.display()); let db = local::open(&db_path)?; log::info!("local store ready — {}", local::summary(&db)); + // Before anything can ask what channel we're on: the installer left a note + // in this directory saying which one the user picked (issue 2183). + update::adopt_installer_channel(&db, &dir); sweep_local_trash(&db); app.manage(db); // Attachment bytes live beside the database, filed by content hash, so a diff --git a/desktop/src-tauri/src/update.rs b/desktop/src-tauri/src/update.rs index ecfc7e0..1537863 100644 --- a/desktop/src-tauri/src/update.rs +++ b/desktop/src-tauri/src/update.rs @@ -15,6 +15,8 @@ //! half exists only as a CI secret, and is generated by the operator — a release //! signing key that has passed through anyone else's hands is not a signing key. +use std::path::Path; + use serde::{Deserialize, Serialize}; use tauri::State; use tauri_plugin_updater::UpdaterExt; @@ -26,6 +28,16 @@ const FEED_BASE: &str = "https://git.fabledsword.com/bvandeusen/thoughtsync/rele const CHANNEL_PREF: &str = "update_channel"; +/// The channel the app follows, as recorded by whatever installed it. Written into +/// the app-data directory by `desktop/packaging/install.sh`; both sides must agree on +/// this name. +const INSTALL_MARKER: &str = "install-channel"; + +/// The marker value we last acted on. Deliberately separate from `CHANNEL_PREF`: it +/// records what the INSTALLER said, so the two can be compared and a change told apart +/// from a repeat. See `adopt_installer_channel`. +const CHANNEL_SEED_PREF: &str = "update_channel_seed"; + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum Channel { @@ -50,6 +62,22 @@ impl Channel { } } + /// Strict parse, for the installer's marker file only. + /// + /// Unrecognized reads as "no opinion" here rather than as `stable` (which is what + /// `parse` does), because the two are asked different questions. `parse` reads a + /// pref that is definitely *this app's* setting, so it has to answer with a + /// channel. The marker is a file written by something else, and a truncated or + /// hand-edited one must not silently move someone OFF the channel they chose. + /// Ignoring it is the only reading that can't do harm in either direction. + fn from_marker(raw: &str) -> Option { + match raw.trim() { + "stable" => Some(Channel::Stable), + "dev" => Some(Channel::Dev), + _ => None, + } + } + fn feed_url(self) -> String { format!("{FEED_BASE}/{}/latest.json", self.as_str()) } @@ -89,6 +117,62 @@ fn self_update_blocker() -> Option { None } +/// Adopt the channel the installer recorded, when it differs from the one we last +/// adopted. Called once at startup, before the frontend can ask what the channel is. +/// +/// The installer knows which channel the user asked for; the app is what acts on it. +/// Until this existed, `install.sh --channel dev` left the pref at its `stable` +/// default, so a dev build checked the stable feed — which advertises an OLDER +/// version — and reported "up to date" forever (issue 2183). Two places held the same +/// decision and only one of them was set. +/// +/// Comparing against the last-adopted value, rather than only seeding when the pref is +/// unset, is what makes both directions work: choosing a channel in the app sticks +/// across launches (the marker hasn't changed since we adopted it), while re-running +/// the installer on a DIFFERENT channel is honoured (it has). Seeding-when-unset would +/// have fixed only the very first install and left the second one silently wrong. +/// +/// Every failure is logged and stepped over. No marker at all is the ordinary state of +/// a build installed some other way — from source, from a downloaded AppImage, or by a +/// Windows installer that has no channel concept — and none of that should keep the +/// app from opening. +pub fn adopt_installer_channel(db: &Db, data_dir: &Path) { + let path = data_dir.join(INSTALL_MARKER); + let Ok(raw) = std::fs::read_to_string(&path) else { + return; + }; + let Some(channel) = Channel::from_marker(&raw) else { + log::warn!( + "ignoring an unreadable install channel marker at {}", + path.display() + ); + return; + }; + match adopt(db, channel) { + Ok(true) => log::info!( + "following the {} update channel, as recorded by the installer", + channel.as_str() + ), + Ok(false) => {} + Err(e) => log::warn!("could not apply the installer's update channel: {e}"), + } +} + +/// Apply `channel` unless we already applied this same marker value. Returns whether +/// anything changed. +fn adopt(db: &Db, channel: Channel) -> Result { + let conn = db.0.lock().map_err(|e| e.to_string())?; + let adopted = store::pref(&conn, CHANNEL_SEED_PREF).map_err(|e| e.to_string())?; + // Already acted on this marker — whatever the pref says now is the user's own + // choice, and re-applying would quietly undo it. + if adopted.as_deref() == Some(channel.as_str()) { + return Ok(false); + } + store::set_pref(&conn, CHANNEL_PREF, channel.as_str()).map_err(|e| e.to_string())?; + store::set_pref(&conn, CHANNEL_SEED_PREF, channel.as_str()).map_err(|e| e.to_string())?; + Ok(true) +} + fn read_channel(db: &State<'_, Db>) -> Result { let conn = db.0.lock().map_err(|e| e.to_string())?; let raw = store::pref(&conn, CHANNEL_PREF).map_err(|e| e.to_string())?; @@ -224,6 +308,100 @@ fn describe_check_error(detail: &str) -> String { #[cfg(test)] mod tests { use super::*; + use crate::local::schema; + use rusqlite::Connection; + use std::sync::Mutex; + + fn db() -> Db { + let conn = Connection::open_in_memory().expect("in-memory db"); + schema::migrate(&conn).expect("migrate"); + Db(Mutex::new(conn)) + } + + /// The channel `update_check` would actually use. + fn effective(db: &Db) -> Channel { + let conn = db.0.lock().expect("lock"); + store::pref(&conn, CHANNEL_PREF) + .expect("read pref") + .as_deref() + .map(Channel::parse) + .unwrap_or(Channel::Stable) + } + + fn set_channel(db: &Db, channel: Channel) { + let conn = db.0.lock().expect("lock"); + store::set_pref(&conn, CHANNEL_PREF, channel.as_str()).expect("set pref"); + } + + /// A scratch directory holding a marker file, named uniquely so tests can run in + /// parallel. Dropped by the OS' temp cleanup; nothing here is worth a new dependency. + fn data_dir_with_marker(contents: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("ts-marker-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&dir).expect("scratch dir"); + std::fs::write(dir.join(INSTALL_MARKER), contents).expect("write marker"); + dir + } + + #[test] + fn a_dev_install_follows_the_dev_channel() { + // The bug this whole mechanism exists for (2183): the installer knew, the app + // didn't, and a dev install checked the stable feed forever. + let db = db(); + assert_eq!( + effective(&db), + Channel::Stable, + "default before any install" + ); + adopt_installer_channel(&db, &data_dir_with_marker("dev\n")); + assert_eq!(effective(&db), Channel::Dev); + } + + #[test] + fn choosing_a_channel_in_the_app_survives_the_next_launch() { + // Adoption runs on EVERY startup, so the marker must not keep re-asserting + // itself over a choice the user made afterwards. + let db = db(); + let dir = data_dir_with_marker("dev"); + adopt_installer_channel(&db, &dir); + set_channel(&db, Channel::Stable); + adopt_installer_channel(&db, &dir); + assert_eq!(effective(&db), Channel::Stable); + } + + #[test] + fn reinstalling_on_a_different_channel_moves_the_app() { + // The half a seed-only-when-unset fix would have missed: install stable, then + // install dev. The pref is already set, but the user just asked for dev. + let db = db(); + adopt_installer_channel(&db, &data_dir_with_marker("stable")); + assert_eq!(effective(&db), Channel::Stable); + adopt_installer_channel(&db, &data_dir_with_marker("dev")); + assert_eq!(effective(&db), Channel::Dev); + } + + #[test] + fn a_damaged_marker_changes_nothing() { + // Ignored, NOT read as stable — a corrupt file must not move someone off the + // channel they're on. + let db = db(); + set_channel(&db, Channel::Dev); + adopt_installer_channel(&db, &data_dir_with_marker("de")); + assert_eq!(effective(&db), Channel::Dev); + assert_eq!(Channel::from_marker("nightly"), None); + assert_eq!(Channel::from_marker(""), None); + assert_eq!(Channel::from_marker(" dev\n"), Some(Channel::Dev)); + } + + #[test] + fn no_marker_at_all_is_not_an_error() { + // Built from source, or installed by anything that isn't install.sh. + let db = db(); + set_channel(&db, Channel::Dev); + let empty = std::env::temp_dir().join(format!("ts-nomarker-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&empty).expect("scratch dir"); + adopt_installer_channel(&db, &empty); + assert_eq!(effective(&db), Channel::Dev); + } #[test] fn each_channel_has_its_own_fixed_feed() {