From ff8e5a14abe445a61dc200fd30a70870c75d183c Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Wed, 20 May 2026 00:34:04 +0300 Subject: [PATCH] fix: stop auto-opening release notes on startup --- src/app/input/mod.rs | 1 - src/app/input/modal.rs | 45 ++++++++++++++++ src/app/input/navigate.rs | 3 -- src/app/input/terminal.rs | 1 - src/app/mod.rs | 105 ++++++++++++++++++++++++++++---------- src/app/runtime.rs | 2 - src/main.rs | 3 -- src/release_notes.rs | 24 ++------- src/server/headless.rs | 4 +- 9 files changed, 130 insertions(+), 58 deletions(-) diff --git a/src/app/input/mod.rs b/src/app/input/mod.rs index ae8fb12b..ad68f9da 100644 --- a/src/app/input/mod.rs +++ b/src/app/input/mod.rs @@ -295,7 +295,6 @@ fn app_for_mouse_test() -> App { &crate::config::Config::default(), true, None, - None, api_rx, crate::api::EventHub::default(), ); diff --git a/src/app/input/modal.rs b/src/app/input/modal.rs index 954f97de..ff5e8582 100644 --- a/src/app/input/modal.rs +++ b/src/app/input/modal.rs @@ -627,6 +627,23 @@ mod tests { use super::super::{capture_snapshot, state_with_workspaces}; use super::*; + fn config_env_lock() -> &'static std::sync::Mutex<()> { + static LOCK: std::sync::OnceLock> = std::sync::OnceLock::new(); + LOCK.get_or_init(|| std::sync::Mutex::new(())) + } + + fn temp_config_path(name: &str) -> std::path::PathBuf { + let unique = format!( + "herdr-modal-{name}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + std::env::temp_dir().join(unique).join("config.toml") + } + #[test] fn custom_resize_key_exits_resize_mode() { let mut state = state_with_workspaces(&["test"]); @@ -642,6 +659,34 @@ mod tests { assert_eq!(state.mode, Mode::Terminal); } + #[test] + fn global_menu_whats_new_opens_saved_release_notes() { + let _guard = config_env_lock().lock().unwrap(); + let path = temp_config_path("whats-new-saved-release-notes"); + std::env::set_var(crate::config::CONFIG_PATH_ENV_VAR, &path); + crate::release_notes::save_pending(env!("CARGO_PKG_VERSION"), "### Changed\n- Menu") + .unwrap(); + + let mut state = state_with_workspaces(&["test"]); + state.latest_release_notes_available = true; + + assert!(global_menu_actions(&state).contains(&GlobalMenuAction::WhatsNew)); + + apply_global_menu_action(&mut state, GlobalMenuAction::WhatsNew); + + assert_eq!(state.mode, Mode::ReleaseNotes); + assert_eq!( + state + .release_notes + .as_ref() + .map(|notes| notes.body.as_str()), + Some("### Changed\n- Menu") + ); + + std::env::remove_var(crate::config::CONFIG_PATH_ENV_VAR); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + #[test] fn rename_modal_keyboard_and_mouse_share_actions() { let mut state = state_with_workspaces(&["test"]); diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs index e2e0993e..34d0a342 100644 --- a/src/app/input/navigate.rs +++ b/src/app/input/navigate.rs @@ -1035,7 +1035,6 @@ mod tests { &Config::default(), true, None, - None, api_rx, crate::api::EventHub::default(), ); @@ -1084,7 +1083,6 @@ mod tests { &Config::default(), true, None, - None, api_rx, crate::api::EventHub::default(), ); @@ -1153,7 +1151,6 @@ mod tests { &Config::default(), true, None, - None, api_rx, crate::api::EventHub::default(), ); diff --git a/src/app/input/terminal.rs b/src/app/input/terminal.rs index b5ceda12..12fb67b2 100644 --- a/src/app/input/terminal.rs +++ b/src/app/input/terminal.rs @@ -461,7 +461,6 @@ mod tests { &Config::default(), true, None, - None, api_rx, crate::api::EventHub::default(), ); diff --git a/src/app/mod.rs b/src/app/mod.rs index 39cf0c13..3e8a8b83 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -198,7 +198,6 @@ impl App { config: &Config, no_session: bool, config_diagnostic: Option, - startup_release_notes: Option, api_rx: tokio::sync::mpsc::UnboundedReceiver, event_hub: crate::api::EventHub, ) -> Self { @@ -321,8 +320,6 @@ impl App { let mode = if config.should_show_onboarding() { state::Mode::Onboarding - } else if startup_release_notes.is_some() { - state::Mode::ReleaseNotes } else if startup_product_announcement.is_some() { state::Mode::ProductAnnouncement } else if active.is_some() { @@ -353,12 +350,7 @@ impl App { request_complete_onboarding: false, name_input: String::new(), name_input_replace_on_type: false, - release_notes: startup_release_notes.map(|notes| state::ReleaseNotesState { - version: notes.version, - body: notes.body, - scroll: 0, - preview: notes.preview, - }), + release_notes: None, product_announcement: startup_product_announcement.map(|announcement| { state::ProductAnnouncementState { version: announcement.version, @@ -1131,7 +1123,6 @@ mod tests { &Config::default(), true, None, - None, api_rx, crate::api::EventHub::default(), ) @@ -1154,6 +1145,14 @@ mod tests { std::env::temp_dir().join(unique).join("config.toml") } + fn restore_xdg_state_home(original: Option) { + if let Some(value) = original { + std::env::set_var("XDG_STATE_HOME", value); + } else { + std::env::remove_var("XDG_STATE_HOME"); + } + } + #[test] fn git_refresh_deadline_is_suppressed_while_in_flight() { let mut app = test_app(); @@ -1204,14 +1203,7 @@ mod tests { config.ui.agent_panel_scope = crate::config::AgentPanelScopeConfig::Current; let (_api_tx, api_rx) = tokio::sync::mpsc::unbounded_channel(); - let app = App::new( - &config, - true, - None, - None, - api_rx, - crate::api::EventHub::default(), - ); + let app = App::new(&config, true, None, api_rx, crate::api::EventHub::default()); assert_eq!( app.state.agent_panel_scope, @@ -1254,6 +1246,74 @@ mod tests { let _ = std::fs::remove_dir_all(path.parent().unwrap()); } + #[test] + fn startup_keeps_pending_release_notes_available_without_auto_opening() { + let _guard = config_env_lock().lock().unwrap(); + let path = temp_config_path("startup-pending-release-notes-no-auto-open"); + std::env::set_var(crate::config::CONFIG_PATH_ENV_VAR, &path); + + crate::release_notes::save_pending(env!("CARGO_PKG_VERSION"), "### Changed\n- One") + .unwrap(); + let config = Config { + onboarding: Some(false), + ..Default::default() + }; + let (_api_tx, api_rx) = tokio::sync::mpsc::unbounded_channel(); + + let app = App::new(&config, true, None, api_rx, crate::api::EventHub::default()); + + assert_eq!(app.state.mode, Mode::Navigate); + assert!(app.state.release_notes.is_none()); + assert!(app.state.latest_release_notes_available); + + std::env::remove_var(crate::config::CONFIG_PATH_ENV_VAR); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + + #[test] + fn startup_still_auto_opens_unseen_product_announcement() { + let _guard = config_env_lock().lock().unwrap(); + let path = temp_config_path("startup-product-announcement-auto-open"); + let state_home = path.parent().unwrap().join("state"); + let original_xdg_state_home = std::env::var_os("XDG_STATE_HOME"); + std::env::set_var(crate::config::CONFIG_PATH_ENV_VAR, &path); + std::env::set_var("XDG_STATE_HOME", &state_home); + + crate::release_notes::save_pending(env!("CARGO_PKG_VERSION"), "### Changed\n- One") + .unwrap(); + crate::product_announcements::save_manifest_announcement( + env!("CARGO_PKG_VERSION"), + Some(&crate::product_announcements::ManifestAnnouncement { + id: "startup-announcement".into(), + title: Some("Startup announcement".into()), + body: "### Announcement\n- One".into(), + }), + ) + .unwrap(); + + let config = Config { + onboarding: Some(false), + ..Default::default() + }; + let (_api_tx, api_rx) = tokio::sync::mpsc::unbounded_channel(); + + let app = App::new(&config, true, None, api_rx, crate::api::EventHub::default()); + + assert_eq!(app.state.mode, Mode::ProductAnnouncement); + assert_eq!( + app.state + .product_announcement + .as_ref() + .map(|announcement| announcement.id.as_str()), + Some("startup-announcement") + ); + assert!(app.state.release_notes.is_none()); + + std::env::remove_var(crate::config::CONFIG_PATH_ENV_VAR); + restore_xdg_state_home(original_xdg_state_home); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + #[test] fn reload_config_updates_live_state() { let _guard = config_env_lock().lock().unwrap(); @@ -1382,14 +1442,7 @@ mod tests { config.ui.sidebar_max_width = 30; let (_api_tx, api_rx) = tokio::sync::mpsc::unbounded_channel(); - let app = App::new( - &config, - true, - None, - None, - api_rx, - crate::api::EventHub::default(), - ); + let app = App::new(&config, true, None, api_rx, crate::api::EventHub::default()); assert_eq!( app.state.sidebar_min_width, 18, diff --git a/src/app/runtime.rs b/src/app/runtime.rs index 739d3548..5671bf38 100644 --- a/src/app/runtime.rs +++ b/src/app/runtime.rs @@ -419,7 +419,6 @@ mod tests { &crate::config::Config::default(), true, None, - None, tokio::sync::mpsc::unbounded_channel().1, crate::api::EventHub::default(), ); @@ -528,7 +527,6 @@ mod tests { &crate::config::Config::default(), true, None, - None, tokio::sync::mpsc::unbounded_channel().1, crate::api::EventHub::default(), ); diff --git a/src/main.rs b/src/main.rs index 1946531a..c8811328 100644 --- a/src/main.rs +++ b/src/main.rs @@ -517,13 +517,10 @@ fn main() -> io::Result<()> { std::io::stdout().flush()?; } - let startup_release_notes = crate::release_notes::load_pending_for_current_version(); - let mut app = app::App::new( config, true, // no_session — monolithic mode never saves/restores sessions config_diagnostic, - startup_release_notes, api_rx, event_hub, ); diff --git a/src/release_notes.rs b/src/release_notes.rs index dd09aaf8..aeaef0b8 100644 --- a/src/release_notes.rs +++ b/src/release_notes.rs @@ -70,22 +70,10 @@ fn load_stored_from_path(path: &Path) -> Option { serde_json::from_str(&content).ok() } -pub fn load_pending_for_current_version() -> Option { - load_pending_from_path(&pending_path(), env!("CARGO_PKG_VERSION")) -} - pub fn load_latest() -> Option { load_latest_from_path(&pending_path(), env!("CARGO_PKG_VERSION")) } -fn load_pending_from_path(path: &Path, current_version: &str) -> Option { - let stored = load_stored_from_path(path)?; - if stored.version != current_version || !stored.show_on_startup { - return None; - } - release_notes_from_stored(stored, current_version) -} - fn load_latest_from_path(path: &Path, current_version: &str) -> Option { let stored = load_stored_from_path(path)?; release_notes_from_stored(stored, current_version) @@ -240,7 +228,7 @@ mod tests { } #[test] - fn marking_current_version_seen_preserves_latest_notes_without_startup_modal() { + fn marking_current_version_seen_preserves_latest_notes() { let path = std::env::temp_dir().join(format!( "herdr-release-notes-{}-{}.json", std::process::id(), @@ -249,12 +237,10 @@ mod tests { let _ = clear_pending_at(&path); save_pending_to_path(&path, "0.3.1", "### Changed\n- One").unwrap(); - let startup = load_pending_from_path(&path, "0.3.1").expect("startup notes"); - assert!(!startup.preview); - mark_current_version_seen_at(&path, "0.3.1").unwrap(); - assert!(load_pending_from_path(&path, "0.3.1").is_none()); + let stored = load_stored_from_path(&path).expect("stored notes"); + assert!(!stored.show_on_startup); let latest = load_latest_from_path(&path, "0.3.1").expect("latest notes"); assert_eq!(latest.version, "0.3.1"); assert!(!latest.preview); @@ -263,7 +249,7 @@ mod tests { } #[test] - fn legacy_notes_without_show_on_startup_default_to_unseen() { + fn legacy_notes_without_show_on_startup_remain_available_as_latest() { let path = std::env::temp_dir().join(format!( "herdr-release-notes-{}-{}.json", std::process::id(), @@ -276,7 +262,7 @@ mod tests { ) .unwrap(); - assert!(load_pending_from_path(&path, "0.3.1").is_some()); + assert!(load_latest_from_path(&path, "0.3.1").is_some()); clear_pending_at(&path).unwrap(); } diff --git a/src/server/headless.rs b/src/server/headless.rs index 58a2b178..56164cfa 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -2188,7 +2188,6 @@ pub fn run_server() -> io::Result<()> { &loaded_config.config, no_session, config::config_diagnostic_summary(&loaded_config.diagnostics), - crate::release_notes::load_pending_for_current_version(), api_rx, event_hub, ); @@ -2242,8 +2241,7 @@ mod tests { fn test_headless_server() -> HeadlessServer { let config = crate::config::Config::default(); let (_api_tx, api_rx) = tokio::sync::mpsc::unbounded_channel(); - let mut app = - crate::app::App::new(&config, true, None, None, api_rx, api::EventHub::default()); + let mut app = crate::app::App::new(&config, true, None, api_rx, api::EventHub::default()); app.state.local_sound_playback = false; app.local_terminal_notifications = false;