From 34892d98444f083eda030a33938d90efb36df79d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 17 Aug 2026 22:07:36 +1000 Subject: [PATCH 1/2] feat(lifecycle): keep Staged running when its last window closes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing the window (red button / Cmd+W) terminated the app, taking every running agent session with it — and Cmd+Q was worse: `PredefinedMenuItem::quit` maps to `NSApp terminate:`, which reaches no Tauri hook, so it skipped the action-shutdown handler entirely and left agent CLIs (spawned with their own process group and only `kill_on_drop`) orphaned. New `app_lifecycle` module owns both halves of the fix, adapted to the multi-window model (#928) this lands on top of: - Closing a window with peers still live (visible or hidden) is just a close: sessions belong to the process, so the window is destroyed normally and the existing `Destroyed` hook does the per-window cleanup. Closing the *last* window is where the interception bites: on macOS `CloseRequested` is prevented and the window hidden, so sessions keep streaming; the Dock icon (`RunEvent::Reopen`), `Window ▸ Staged`, or a quit arriving while hidden brings it back — `show_a_window` prefers `main` for its restored geometry but recovers any surviving `win-N` peer. Hiding also drops that window's `tauri-{label}` PR-poll client to its unfocused tier, which a hidden window's missing webview blur would not. Other platforms have no Dock or tray to recover a hidden window, so closing the last window still quits there — now through the confirmation gate. - A custom Quit menu item makes Cmd+Q routable, so `request_quit` can gate it on active sessions and raise a confirmation dialog; confirming cancels each session with `CompletionReason::AppQuit` (the cancel is what runs the ACP child's graceful stop), waits for sessions and actions inside one shared 2s budget, sweeps any still-active rows to cancelled/app_quit, then exits. Queued sessions count as active; running actions are reported but don't gate. Quit and `Window ▸ Staged` route through the shared `dispatch_menu_event` router as focus-independent backend actions — every window being hidden is exactly when they matter. The dialog is addressed to exactly one window (`emit_to` plus a window-scoped frontend listener, the same pattern as menu routing): where the user is, or a window revealed for the purpose. A broadcast would raise one dialog per window, each unaware of the others' answers. The pending-prompt flag remembers its host window and is cleared when that window is destroyed, so the force-on-second- request escape hatch can't fire with no dialog on screen. `RunEvent::Exit` now runs the same idempotent cleanup, which is the only hook on the terminate: path — Dock ▸ Quit and logout stop sessions and actions instead of orphaning them. Ownership is checked against `owner_pid`, so a quit never prompts about or cancels another Staged instance's work. The quit commands are refused in the web-mode dispatch table: a browser client must not be able to terminate the desktop host. The store-incompatibility screens' Close buttons now quit rather than closing a window that would only hide. Phase 4 of the plan (routing Dock ▸ Quit through the prompt via a runtime `applicationShouldTerminate:`) is deliberately left out: the shared cleanup already prevents the process and data damage there, only the prompt is missing. Verified with `just check-all`. Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/app_lifecycle.rs | 646 ++++++++++++++++++ apps/staged/src-tauri/src/lib.rs | 133 ++-- .../staged/src-tauri/src/pr_poll_scheduler.rs | 15 + apps/staged/src-tauri/src/session_commands.rs | 2 +- apps/staged/src-tauri/src/session_runner.rs | 67 ++ apps/staged/src-tauri/src/web_server.rs | 10 + apps/staged/src/App.svelte | 14 +- apps/staged/src/lib/commands.ts | 27 + .../lifecycle/QuitConfirmDialog.svelte | 62 ++ .../features/lifecycle/quitPromptCopy.test.ts | 63 ++ .../lib/features/lifecycle/quitPromptCopy.ts | 60 ++ .../lib/features/projects/ProjectHome.svelte | 5 +- .../src/lib/listeners/quitListener.test.ts | 137 ++++ apps/staged/src/lib/listeners/quitListener.ts | 24 + .../src/lib/stores/quitPrompt.svelte.ts | 61 ++ apps/staged/src/lib/types.ts | 12 + 16 files changed, 1291 insertions(+), 47 deletions(-) create mode 100644 apps/staged/src-tauri/src/app_lifecycle.rs create mode 100644 apps/staged/src/lib/features/lifecycle/QuitConfirmDialog.svelte create mode 100644 apps/staged/src/lib/features/lifecycle/quitPromptCopy.test.ts create mode 100644 apps/staged/src/lib/features/lifecycle/quitPromptCopy.ts create mode 100644 apps/staged/src/lib/listeners/quitListener.test.ts create mode 100644 apps/staged/src/lib/listeners/quitListener.ts create mode 100644 apps/staged/src/lib/stores/quitPrompt.svelte.ts diff --git a/apps/staged/src-tauri/src/app_lifecycle.rs b/apps/staged/src-tauri/src/app_lifecycle.rs new file mode 100644 index 00000000..aae9e791 --- /dev/null +++ b/apps/staged/src-tauri/src/app_lifecycle.rs @@ -0,0 +1,646 @@ +//! Window close, the quit gate, and shutdown cleanup. +//! +//! Staged's work outlives its windows: agent sessions and long-running actions +//! are child processes this process owns. Two rules follow from that, and this +//! module owns both. +//! +//! **Closing a window is not quitting.** With peer windows still open, a close +//! is just a close — the process lives on in the others, so the window is +//! destroyed normally (`window_commands` owns that cleanup). Closing the *last* +//! window is where the rules bite: on macOS `CloseRequested` is prevented and +//! the window hidden, so sessions keep streaming; the Dock icon +//! (`RunEvent::Reopen`) or `Window ▸ Staged` brings it back. Other platforms +//! have no Dock/tray to recover a hidden window, so closing the last window +//! still quits there — but through the same confirmation gate as `Cmd+Q`. +//! +//! **Quitting with sessions running asks first, then stops them cleanly.** +//! [`request_quit`] gates on active sessions and hands the decision to the +//! frontend dialog, addressed to a single live window (revealed first if every +//! window is hidden); [`shutdown_cleanup`] cancels sessions with +//! [`CompletionReason::AppQuit`] and stops actions. That cancel is the only +//! thing that shuts an agent down: ACP children are spawned with +//! `process_group(0)` and `kill_on_drop`, and `process::exit` runs no +//! destructors, so a bare exit leaves the agent CLIs running. +//! +//! Every exit path funnels into [`shutdown_cleanup`], which runs its work at +//! most once — a confirmed quit calls it directly, `RunEvent::ExitRequested` +//! covers programmatic exits, and `RunEvent::Exit` is the only hook on the +//! `NSApp terminate:` path (Dock ▸ Quit, logout), which never emits +//! `ExitRequested`. + +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; + +use serde::Serialize; +use tauri::{AppHandle, Emitter, Manager, WebviewWindow, Window, WindowEvent}; + +use crate::actions; +use crate::session_commands::{self, ActiveSessionInfo}; +use crate::session_runner::SessionRegistry; +use crate::store::{CompletionReason, Session, SessionStatus, Store}; + +/// Event that raises the frontend's quit confirmation dialog. +const QUIT_REQUESTED_EVENT: &str = "app:quit-requested"; + +/// Menu id of the app-menu Quit item. Custom rather than +/// `PredefinedMenuItem::quit` so `Cmd+Q` is routable at all: the predefined item +/// maps to `NSApp terminate:`, which reaches no Tauri hook that can gate it. +pub(crate) const QUIT_MENU_ID: &str = "quit"; + +/// Menu id of `Window ▸ Staged`. The recovery path for `Cmd+Tab`-ing to an app +/// whose windows are all hidden — macOS sends no reopen event for that. +pub(crate) const SHOW_WINDOW_MENU_ID: &str = "show_window"; + +/// Label of the cold-start window (the `tauri.conf.json` entry). Secondary +/// windows are `win-N` peers — see `window_commands` — with nothing privileged +/// about `main` beyond being the one whose geometry is restored, which makes it +/// the nicest default to reveal. +const MAIN_WINDOW_LABEL: &str = "main"; + +/// Total budget for stopping sessions and actions. Sessions and actions are +/// signalled first and waited on against this one deadline, because the +/// `RunEvent::Exit` path runs inside `applicationWillTerminate:`, where the OS +/// gives us limited time before killing the process outright. +const SHUTDOWN_BUDGET: Duration = Duration::from_secs(2); + +/// Grace period before an action's process group is escalated to `SIGKILL`. +const ACTION_FORCE_KILL_AFTER: Duration = Duration::from_secs(1); + +/// Quit bookkeeping, managed as Tauri state. +#[derive(Default)] +pub struct QuitState { + /// Set by the first caller into [`shutdown_cleanup`], so the cleanup runs + /// exactly once however many exit events follow it. + quit_in_progress: AtomicBool, + /// Label of the window showing an unanswered confirmation dialog. A quit + /// request arriving while it is set forces the quit — a wedged webview must + /// never be able to trap the app, so a second `Cmd+Q` always gets out. The + /// label is what lets a destroyed host window clear the flag instead of + /// leaving that force path armed with no dialog on screen. + prompt_host: Mutex>, +} + +impl QuitState { + fn set_prompt_host(&self, label: &str) { + *self.prompt_host.lock().unwrap() = Some(label.to_string()); + } + + /// Clear any pending prompt, returning whether one was pending. + fn take_prompt(&self) -> bool { + self.prompt_host.lock().unwrap().take().is_some() + } + + /// Clear the pending prompt if `label` was hosting it. + fn clear_prompt_if_host(&self, label: &str) { + let mut host = self.prompt_host.lock().unwrap(); + if host.as_deref() == Some(label) { + *host = None; + } + } +} + +/// What a quit would interrupt, as sent to the confirmation dialog. +#[derive(Debug, Clone, Default, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct QuitBlockers { + /// Running and queued sessions owned by this process — the only thing that + /// gates a quit. + pub sessions: Vec, + /// Running actions. Reported so the dialog can say they stop too, but they + /// don't gate the quit on their own: a dev server left running is the normal + /// state of a workspace, and blocking `Cmd+Q` on it would be noise. + pub running_action_count: usize, +} + +/// Whether a quit should stop and ask first. +/// +/// Queued sessions count: they're work the user asked for that a quit silently +/// drops, so they belong in the prompt. +fn should_prompt(blockers: &QuitBlockers) -> bool { + !blockers.sessions.is_empty() +} + +// ============================================================================= +// Window events +// ============================================================================= + +/// `Builder::on_window_event` hook — see the module docs for why closing the +/// last window doesn't end the process. +pub fn on_window_event(window: &Window, event: &WindowEvent) { + match event { + WindowEvent::CloseRequested { api, .. } => on_close_requested(window, api), + // A destroyed window takes its webview — and any dialog in it — with + // it. Left set, the pending flag would turn the next quit request into + // a silent force-quit; cleared, that quit just asks again. + WindowEvent::Destroyed => { + if let Some(quit_state) = window.app_handle().try_state::() { + quit_state.clear_prompt_if_host(window.label()); + } + } + _ => {} + } +} + +fn on_close_requested(window: &Window, api: &tauri::CloseRequestApi) { + let app = window.app_handle(); + + // Mid-shutdown, closes are the exit tearing windows down — stay out of the + // way. + if let Some(quit_state) = app.try_state::() { + if quit_state.quit_in_progress.load(Ordering::SeqCst) { + return; + } + } + + // With peer windows still live (visible or hidden), a close is just a + // close: sessions belong to the process, not this window. The `Destroyed` + // hook in lib.rs does the per-window cleanup. + if app.webview_windows().len() > 1 { + return; + } + + // Last window: the window-state plugin has its own `CloseRequested` handler + // and saves geometry there, so preventing the close still persists the + // window's position and size. + api.prevent_close(); + + #[cfg(target_os = "macos")] + hide_window(window); + + // No Dock or tray icon elsewhere, so a hidden window would be unreachable — + // closing the last window still quits, with the confirmation gate in front + // of it. + #[cfg(not(target_os = "macos"))] + request_quit(app, false); +} + +/// Hide the window and drop its PR-poll client to the unfocused tier. +/// +/// `prPollingService` derives focus from `document.hasFocus()` and the webview's +/// focus events, and hiding the native window does not reliably deliver a blur +/// to the webview — so tell the scheduler directly instead of leaving it polling +/// on behalf of a window nobody can see. +#[cfg(target_os = "macos")] +fn hide_window(window: &Window) { + if let Err(e) = window.hide() { + log::warn!("Failed to hide window on close: {e}"); + return; + } + set_native_focus(window.app_handle(), window.label(), false); +} + +/// Bring a window back on screen: the Dock-icon click, `Window ▸ Staged`, and a +/// quit arriving with no visible window all funnel here. +pub fn show_a_window(app: &AppHandle) { + if reveal_a_window(app).is_none() { + log::warn!("No window left to show"); + } +} + +/// Pick a window and make sure it is on screen and focused, returning it. +/// +/// Prefers where the user already is (focused, then visible — reachable when a +/// quit request arrives from the store-incompatibility screen or the web +/// dispatch refusal path while windows are up), then falls back to unhiding one: +/// `main` for its restored geometry, else any. `None` only if every window has +/// been destroyed, which no close path produces — closing the last window hides +/// it instead. +fn reveal_a_window(app: &AppHandle) -> Option { + let windows = app.webview_windows(); + let window = windows + .values() + .find(|window| window.is_focused().unwrap_or(false)) + .or_else(|| { + windows + .values() + .find(|window| window.is_visible().unwrap_or(false)) + }) + .or_else(|| windows.get(MAIN_WINDOW_LABEL)) + .or_else(|| windows.values().next())?; + + if let Err(e) = window.show() { + log::warn!("Failed to show window: {e}"); + } + if let Err(e) = window.unminimize() { + log::warn!("Failed to unminimize window: {e}"); + } + if let Err(e) = window.set_focus() { + log::warn!("Failed to focus window: {e}"); + } + set_native_focus(app, window.label(), true); + Some(window.clone()) +} + +/// Mirror a native window's visibility onto its PR-poll client's focus hint. +/// Paired with the webview's own focus events, which report the same value once +/// the window is back on screen. +fn set_native_focus(app: &AppHandle, window_label: &str, focused: bool) { + if let Some(scheduler) = app.try_state::>() { + crate::pr_poll_scheduler::set_tauri_client_focus(&scheduler, window_label, focused); + } +} + +// ============================================================================= +// Quit gate +// ============================================================================= + +/// Handle a quit request from the app menu, `Cmd+Q`, or (off macOS) the last +/// window's close. Cheap enough for the main thread: it snapshots blockers and +/// either hands off to a background quit or raises the dialog. +pub fn request_quit(app: &AppHandle, force: bool) { + let quit_state = app.state::(); + + // A quit arriving while the dialog is unanswered (a second `Cmd+Q`) is the + // escape hatch from a webview that never rendered or answered it. + if force || quit_state.take_prompt() { + spawn_quit(app); + return; + } + + let blockers = collect_quit_blockers(app); + if !should_prompt(&blockers) { + spawn_quit(app); + return; + } + + // The dialog goes to exactly one window — where the user is, or a window + // revealed for the purpose if the quit arrived with everything hidden. A + // broadcast would raise one dialog per window, each unaware of the others' + // answers. No window at all means nobody to ask, so the quit proceeds. + let Some(host) = reveal_a_window(app) else { + spawn_quit(app); + return; + }; + quit_state.set_prompt_host(host.label()); + + if let Err(e) = app.emit_to(host.label(), QUIT_REQUESTED_EVENT, &blockers) { + log::warn!("Failed to ask for quit confirmation, quitting anyway: {e}"); + quit_state.take_prompt(); + spawn_quit(app); + } +} + +/// Quit from the UI, through the same gate as `Cmd+Q`. +/// +/// Used by the store-incompatibility screens' "Close" button, which has to end +/// the app: closing the last window only hides it, and those screens have no +/// working database behind them to come back to. +#[tauri::command] +pub fn quit_app(app_handle: AppHandle) { + request_quit(&app_handle, false); +} + +/// Quit confirmed in the dialog: stop sessions and actions, then exit. +/// +/// Deliberately absent from the web-mode `dispatch` table — a browser client +/// must not be able to terminate the desktop host. +#[tauri::command] +pub fn confirm_quit(app_handle: AppHandle) { + app_handle.state::().take_prompt(); + spawn_quit(&app_handle); +} + +/// Quit declined in the dialog: sessions keep running. +#[tauri::command] +pub fn cancel_quit(app_handle: AppHandle) { + app_handle.state::().take_prompt(); +} + +/// Run the quit sequence off the main thread so the bounded waits never freeze +/// the event loop — the dialog stays interactive and can render its +/// "Stopping sessions…" state while agents shut down. +fn spawn_quit(app: &AppHandle) { + let app = app.clone(); + std::thread::spawn(move || { + shutdown_cleanup(&app); + app.exit(0); + }); +} + +/// Snapshot what a quit would interrupt. +fn collect_quit_blockers(app: &AppHandle) -> QuitBlockers { + let sessions = match app_store(app) { + Some(store) => owned_active_sessions(&store) + .iter() + .map(|session| session_commands::project_active_session(&store, session)) + .collect(), + None => Vec::new(), + }; + + let running_action_count = match ( + app.try_state::>(), + app.try_state::>(), + ) { + (Some(executor), Some(registry)) => { + actions::commands::get_all_running_actions_impl(&executor, ®istry) + .map(|running| running.len()) + .unwrap_or(0) + } + _ => 0, + }; + + QuitBlockers { + sessions, + running_action_count, + } +} + +// ============================================================================= +// Shutdown cleanup +// ============================================================================= + +/// Stop everything this process owns. Idempotent — the first caller does the +/// work, later ones return immediately. +pub fn shutdown_cleanup(app: &AppHandle) { + let Some(quit_state) = app.try_state::() else { + return; + }; + if quit_state.quit_in_progress.swap(true, Ordering::SeqCst) { + return; + } + + // Signal both kinds of work before waiting on either, so they shut down in + // parallel inside one shared budget instead of one after the other. + let session_ids = cancel_owned_sessions(app); + let execution_ids = stop_running_actions(app); + + let deadline = Instant::now() + SHUTDOWN_BUDGET; + if !session_ids.is_empty() && !wait_for_sessions(app, &session_ids, deadline) { + log::warn!( + "Timed out waiting for {} session(s) to stop during app shutdown", + session_ids.len() + ); + } + if !execution_ids.is_empty() && !wait_for_actions(app, &execution_ids, deadline) { + log::warn!( + "Timed out waiting for {} action(s) to stop during app shutdown", + execution_ids.len() + ); + } + + // Last, so the rows reflect whatever the session threads managed to write + // for themselves first. + sweep_active_sessions(app); +} + +/// Cancel every session this process is running, recording `AppQuit` as the +/// reason. Returns the ids that were signalled. +fn cancel_owned_sessions(app: &AppHandle) -> Vec { + let Some(registry) = app.try_state::>() else { + return Vec::new(); + }; + + let session_ids = registry.running_session_ids(); + for session_id in &session_ids { + registry.cancel_with_completion_reason(session_id, CompletionReason::AppQuit); + } + session_ids +} + +/// Send every running action's process group a hangup, escalating to `SIGKILL` +/// after a grace period. Returns the execution ids that were signalled. +fn stop_running_actions(app: &AppHandle) -> Vec { + let (Some(executor), Some(registry)) = ( + app.try_state::>(), + app.try_state::>(), + ) else { + return Vec::new(); + }; + + actions::commands::stop_all_actions( + &executor, + ®istry, + actions::StopOptions { + force_kill_after: Some(ACTION_FORCE_KILL_AFTER), + }, + ) +} + +fn wait_for_sessions(app: &AppHandle, session_ids: &[String], deadline: Instant) -> bool { + let Some(registry) = app.try_state::>() else { + return true; + }; + registry.wait_for_sessions(session_ids, remaining_until(deadline)) +} + +fn wait_for_actions(app: &AppHandle, execution_ids: &[String], deadline: Instant) -> bool { + let Some(executor) = app.try_state::>() else { + return true; + }; + executor.wait_for_executions(execution_ids, remaining_until(deadline)) +} + +fn remaining_until(deadline: Instant) -> Duration { + deadline.saturating_duration_since(Instant::now()) +} + +/// Mark whatever is still active in the DB as cancelled by the quit. +/// +/// Covers sessions whose thread didn't finish its own terminal write inside the +/// budget, plus queued sessions that never started. Without this the next launch +/// finds them owned by a dead process and reports them as errored sessions. +fn sweep_active_sessions(app: &AppHandle) { + let Some(store) = app_store(app) else { + return; + }; + + let swept = owned_active_sessions(&store) + .iter() + .filter(|session| { + // Guarded CAS per row: a session thread that wrote its own terminal + // status while we were waiting keeps that status. + store + .transition_from_active( + &session.id, + SessionStatus::Cancelled, + None, + Some(&CompletionReason::AppQuit), + ) + .unwrap_or_else(|e| { + log::warn!("Failed to cancel session {} on quit: {e}", session.id); + false + }) + }) + .count(); + + if swept > 0 { + log::info!("Marked {swept} session(s) cancelled (app_quit) during shutdown"); + } +} + +/// Running and queued sessions **this process owns**. +/// +/// The store is shared with any other Staged instance pointed at the same data +/// dir — that's what `owner_pid` is for — so a quit must neither prompt about +/// nor cancel another instance's work. Queued rows carry no owner yet, so they +/// count as ours: claiming one (`transition_queued_to_running`) stamps a pid +/// atomically, which is what takes another instance's claim out of this set. +fn owned_active_sessions(store: &Store) -> Vec { + let sessions = match store.get_active_sessions() { + Ok(sessions) => sessions, + Err(e) => { + log::warn!("Failed to query active sessions during quit: {e}"); + return Vec::new(); + } + }; + + sessions + .into_iter() + .filter(|session| { + session.status == SessionStatus::Queued || session.owner_pid == Some(std::process::id()) + }) + .collect() +} + +fn app_store(app: &AppHandle) -> Option> { + app.try_state::>>>() + .and_then(|slot| slot.lock().unwrap().clone()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::Path; + + fn active_session(status: &str) -> ActiveSessionInfo { + ActiveSessionInfo { + session_id: "s1".to_string(), + project_id: None, + branch_id: None, + session_type: None, + status: status.to_string(), + } + } + + #[test] + fn running_sessions_prompt() { + let blockers = QuitBlockers { + sessions: vec![active_session("running")], + running_action_count: 0, + }; + assert!(should_prompt(&blockers)); + } + + #[test] + fn queued_sessions_prompt() { + let blockers = QuitBlockers { + sessions: vec![active_session("queued")], + running_action_count: 0, + }; + assert!(should_prompt(&blockers)); + } + + #[test] + fn running_actions_alone_do_not_prompt() { + let blockers = QuitBlockers { + sessions: Vec::new(), + running_action_count: 3, + }; + assert!(!should_prompt(&blockers)); + } + + #[test] + fn nothing_active_does_not_prompt() { + assert!(!should_prompt(&QuitBlockers::default())); + } + + /// The pending flag turns the next quit into a force-quit, so it must not + /// outlive the window whose dialog it stands for — but a *peer* window + /// closing must not answer a dialog it isn't showing. + #[test] + fn prompt_clears_only_when_its_host_window_is_destroyed() { + let state = QuitState::default(); + + state.set_prompt_host("win-2"); + state.clear_prompt_if_host("main"); + assert!(state.take_prompt(), "peer destruction dropped the prompt"); + + state.set_prompt_host("win-2"); + state.clear_prompt_if_host("win-2"); + assert!( + !state.take_prompt(), + "host destruction left the prompt armed" + ); + } + + #[test] + fn owned_active_sessions_skips_other_instances_running_sessions() { + let store = Store::in_memory().unwrap(); + + let ours = Session::new_running("ours", Path::new("/tmp")); + store.create_session(&ours).unwrap(); + let queued = Session::new_queued("queued"); + store.create_session(&queued).unwrap(); + let mut theirs = Session::new_running("theirs", Path::new("/tmp")); + theirs.owner_pid = Some(std::process::id().wrapping_add(1)); + store.create_session(&theirs).unwrap(); + + let owned = owned_active_sessions(&store); + assert_eq!(owned.len(), 2); + assert!(owned.iter().any(|session| session.id == ours.id)); + assert!(owned.iter().any(|session| session.id == queued.id)); + } + + /// The DB sweep is what keeps the next launch from reporting these sessions + /// as errors recovered from a dead process. + #[test] + fn sweep_cancels_running_and_queued_sessions() { + let store = Store::in_memory().unwrap(); + + let running = Session::new_running("running", Path::new("/tmp")); + store.create_session(&running).unwrap(); + let queued = Session::new_queued("queued"); + store.create_session(&queued).unwrap(); + + for session in owned_active_sessions(&store) { + assert!(store + .transition_from_active( + &session.id, + SessionStatus::Cancelled, + None, + Some(&CompletionReason::AppQuit), + ) + .unwrap()); + } + + for id in [&running.id, &queued.id] { + let session = store.get_session(id).unwrap().unwrap(); + assert_eq!(session.status, SessionStatus::Cancelled); + assert_eq!(session.completion_reason, Some(CompletionReason::AppQuit)); + } + } + + #[test] + fn sweep_leaves_terminal_sessions_alone() { + let store = Store::in_memory().unwrap(); + + let completed = Session::new_running("completed", Path::new("/tmp")); + store.create_session(&completed).unwrap(); + store + .update_session_status( + &completed.id, + SessionStatus::Completed, + None, + Some(&CompletionReason::TurnComplete), + ) + .unwrap(); + + assert!(owned_active_sessions(&store).is_empty()); + assert!(!store + .transition_from_active( + &completed.id, + SessionStatus::Cancelled, + None, + Some(&CompletionReason::AppQuit), + ) + .unwrap()); + + let session = store.get_session(&completed.id).unwrap().unwrap(); + assert_eq!(session.status, SessionStatus::Completed); + assert_eq!( + session.completion_reason, + Some(CompletionReason::TurnComplete) + ); + } +} diff --git a/apps/staged/src-tauri/src/lib.rs b/apps/staged/src-tauri/src/lib.rs index 44968f50..f782093a 100644 --- a/apps/staged/src-tauri/src/lib.rs +++ b/apps/staged/src-tauri/src/lib.rs @@ -8,6 +8,7 @@ pub mod acp_tools; pub mod acp_tools_reconciler; pub mod actions; pub mod agent; +pub mod app_lifecycle; pub mod background_sync; pub mod blox; pub mod branches; @@ -48,9 +49,7 @@ pub mod test_utils; use serde::Serialize; use std::path::PathBuf; -use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; -use std::time::Duration; use store::Store; use tauri::{Emitter, Manager}; @@ -67,11 +66,6 @@ struct DbState { needs_reset: Mutex>, } -#[derive(Default)] -struct ShutdownState { - quit_in_progress: AtomicBool, -} - pub(crate) fn preferences_store_path_buf() -> Option { crate::paths::data_dir().map(|d| d.join("preferences.json")) } @@ -258,29 +252,6 @@ pub(crate) fn get_store( .ok_or_else(|| "Database not initialized — please reset from the startup prompt".into()) } -fn stop_actions_for_app_shutdown(app_handle: &tauri::AppHandle) { - let executor = app_handle.state::>(); - let registry = app_handle.state::>(); - let stopped_execution_ids = actions::commands::stop_all_actions( - &executor, - ®istry, - actions::StopOptions { - force_kill_after: Some(Duration::from_secs(1)), - }, - ); - - if stopped_execution_ids.is_empty() { - return; - } - - if !executor.wait_for_executions(&stopped_execution_ids, Duration::from_secs(2)) { - log::warn!( - "Timed out waiting for {} action(s) to stop during app shutdown", - stopped_execution_ids.len() - ); - } -} - fn start_store_services( store: Arc, pr_scheduler: Arc, @@ -1772,6 +1743,10 @@ enum MenuDispatch { EmitToFocused(&'static str), /// Create a window here in the backend, with no project seed. OpenWindowUnseeded, + /// Run the quit gate in the backend (`app_lifecycle::request_quit`). + RequestQuit, + /// Reveal a window in the backend (`app_lifecycle::show_a_window`). + ShowWindow, /// Nothing to do — unknown item, or a window-scoped item with no target. Drop, } @@ -1790,6 +1765,15 @@ enum MenuDispatch { /// can just create it. That also un-strands the other items: the new window is /// focused, so Settings/Find/zoom route normally again. fn dispatch_menu_event(id: &str, has_focused_window: bool) -> MenuDispatch { + // Lifecycle items are app-scoped and handled in the backend, focus or no + // focus — every window being hidden is exactly when `Window ▸ Staged` and a + // gateable `Cmd+Q` matter most. + match id { + app_lifecycle::QUIT_MENU_ID => return MenuDispatch::RequestQuit, + app_lifecycle::SHOW_WINDOW_MENU_ID => return MenuDispatch::ShowWindow, + _ => {} + } + let event_name = match id { "new_window" => "menu:new-window", "settings" => "menu:settings", @@ -1933,6 +1917,26 @@ pub fn run() { true, Some("CmdOrCtrl+0"), )?; + // Custom rather than `PredefinedMenuItem::quit`: that one maps + // straight to `NSApp terminate:`, which reaches no Tauri hook, + // so Cmd+Q could never be gated on running sessions. + let quit_item = MenuItem::with_id( + handle, + app_lifecycle::QUIT_MENU_ID, + "Quit Staged", + true, + Some("CmdOrCtrl+Q"), + )?; + // Recovery path for an app whose windows are all hidden: Cmd+Tab + // sends no reopen event, so without this the app looks dead (the + // same reason Slack exposes `Window ▸ Slack`). + let show_window_item = MenuItem::with_id( + handle, + app_lifecycle::SHOW_WINDOW_MENU_ID, + "Staged", + true, + None::<&str>, + )?; let app_menu = Submenu::with_items( handle, @@ -1952,7 +1956,7 @@ pub fn run() { &PredefinedMenuItem::hide(handle, None)?, &PredefinedMenuItem::hide_others(handle, None)?, &PredefinedMenuItem::separator(handle)?, - &PredefinedMenuItem::quit(handle, Some("Quit Staged"))?, + &quit_item, ], )?; @@ -2010,6 +2014,8 @@ pub fn run() { &PredefinedMenuItem::maximize(handle, None)?, &PredefinedMenuItem::separator(handle)?, &PredefinedMenuItem::close_window(handle, None)?, + &PredefinedMenuItem::separator(handle)?, + &show_window_item, ], )?; @@ -2147,7 +2153,7 @@ pub fn run() { app.manage(window_commands::UpdaterWindowState::default()); app.manage(Arc::new(actions::ActionExecutor::new())); app.manage(Arc::new(actions::ActionRegistry::new())); - app.manage(ShutdownState::default()); + app.manage(app_lifecycle::QuitState::default()); app.manage(DbState { db_path, needs_reset: Mutex::new(reset_info), @@ -2207,10 +2213,16 @@ pub fn run() { log::warn!("Failed to open window from menu: {e}"); } } + MenuDispatch::RequestQuit => app_lifecycle::request_quit(app, false), + MenuDispatch::ShowWindow => app_lifecycle::show_a_window(app), MenuDispatch::Drop => {} } }) .on_window_event(|window, event| { + // Close-to-hide / the quit gate (`CloseRequested`), and dropping a + // pending quit prompt whose host window went away (`Destroyed`). + app_lifecycle::on_window_event(window, event); + if let tauri::WindowEvent::Destroyed = event { // Native windows have no WS heartbeat and their PR-poll client // ids are exempt from TTL eviction, so a closed window must @@ -2246,6 +2258,11 @@ pub fn run() { window_commands::new_window, window_commands::take_window_seed, window_commands::claim_updater_ownership, + // Lifecycle — desktop only; the web-mode `dispatch` table refuses + // these so a browser client can't quit the host. + app_lifecycle::quit_app, + app_lifecycle::confirm_quit, + app_lifecycle::cancel_quit, list_projects, create_project, list_project_repos, @@ -2441,17 +2458,30 @@ pub fn run() { ]) .build(tauri::generate_context!()) .expect("error while building tauri application") - .run(|app_handle, event| { - if let tauri::RunEvent::ExitRequested { api, .. } = event { - let shutdown = app_handle.state::(); - if shutdown.quit_in_progress.swap(true, Ordering::SeqCst) { - return; - } - - api.prevent_exit(); - stop_actions_for_app_shutdown(app_handle); - app_handle.exit(0); + .run(|app_handle, event| match event { + // Now that window close is intercepted, the only producers are our + // own confirmed quit (which has already cleaned up) and the updater's + // relaunch — which ignores `prevent_exit` anyway, so nothing here + // tries to hold the exit back. + tauri::RunEvent::ExitRequested { .. } => { + app_lifecycle::shutdown_cleanup(app_handle); + } + // The only hook on the `NSApp terminate:` path (Dock ▸ Quit, logout), + // which never emits `ExitRequested`. Without it those quits orphan + // the agent and action child processes. + tauri::RunEvent::Exit => { + app_lifecycle::shutdown_cleanup(app_handle); + } + // Dock-icon click or `open -a Staged` on an app whose windows are + // all hidden. + #[cfg(target_os = "macos")] + tauri::RunEvent::Reopen { + has_visible_windows: false, + .. + } => { + app_lifecycle::show_a_window(app_handle); } + _ => {} }); } @@ -2587,12 +2617,29 @@ mod tests { #[test] fn unknown_menu_events_drop_regardless_of_focus() { - for id in ["", "quit", "menu:new-window", "New Window"] { + for id in ["", "menu:new-window", "New Window"] { assert_eq!(dispatch_menu_event(id, true), MenuDispatch::Drop); assert_eq!(dispatch_menu_event(id, false), MenuDispatch::Drop); } } + /// The lifecycle items must route with no window focused: every window + /// being hidden is exactly when `Window ▸ Staged` and a gateable `Cmd+Q` + /// matter most. + #[test] + fn lifecycle_menu_events_route_to_the_backend_regardless_of_focus() { + for focused in [true, false] { + assert_eq!( + dispatch_menu_event(crate::app_lifecycle::QUIT_MENU_ID, focused), + MenuDispatch::RequestQuit + ); + assert_eq!( + dispatch_menu_event(crate::app_lifecycle::SHOW_WINDOW_MENU_ID, focused), + MenuDispatch::ShowWindow + ); + } + } + fn remote_branch( project_id: &str, id: &str, diff --git a/apps/staged/src-tauri/src/pr_poll_scheduler.rs b/apps/staged/src-tauri/src/pr_poll_scheduler.rs index 893e8247..41c295e9 100644 --- a/apps/staged/src-tauri/src/pr_poll_scheduler.rs +++ b/apps/staged/src-tauri/src/pr_poll_scheduler.rs @@ -635,6 +635,21 @@ pub fn set_foreground_project( scheduler.set_foreground(client_id, project_id); } +/// Report a native window's focus from the backend, bypassing the frontend. +/// +/// `app_lifecycle` hides and shows windows itself, and a hidden native window +/// does not reliably deliver a blur to its webview — so without this the +/// scheduler would keep polling on the focused tier for a window nobody can +/// see. The id mirrors the frontend's own `tauri-{label}` scheme, so both sides +/// address the same per-window client. +pub(crate) fn set_tauri_client_focus( + scheduler: &PrPollScheduler, + window_label: &str, + focused: bool, +) { + scheduler.set_focus(format!("{TAURI_CLIENT_PREFIX}{window_label}"), focused); +} + /// Report a client's window focus. With no client focused, periodic polling /// pauses (an explicit `refresh_now` still fetches). #[tauri::command(rename_all = "camelCase")] diff --git a/apps/staged/src-tauri/src/session_commands.rs b/apps/staged/src-tauri/src/session_commands.rs index 4f8fcc67..75810e57 100644 --- a/apps/staged/src-tauri/src/session_commands.rs +++ b/apps/staged/src-tauri/src/session_commands.rs @@ -912,7 +912,7 @@ pub struct ActiveSessionInfo { /// sessions (pr/push) link no artifact, so their branch comes from the /// session row's own `branch_id` and their type falls back to prompt /// inference. -fn project_active_session(store: &Store, session: &store::Session) -> ActiveSessionInfo { +pub(crate) fn project_active_session(store: &Store, session: &store::Session) -> ActiveSessionInfo { let project_note = store .get_project_note_by_session(&session.id) .ok() diff --git a/apps/staged/src-tauri/src/session_runner.rs b/apps/staged/src-tauri/src/session_runner.rs index 1ac8c0e4..0f47160b 100644 --- a/apps/staged/src-tauri/src/session_runner.rs +++ b/apps/staged/src-tauri/src/session_runner.rs @@ -290,6 +290,44 @@ impl SessionRegistry { self.inner.lock().unwrap().running.contains_key(session_id) } + /// Ids of every session this process is currently running. + /// + /// The shutdown path uses this to cancel them all: the registry, not the DB, + /// is what says which running rows belong to *this* process's threads. + pub fn running_session_ids(&self) -> Vec { + self.inner.lock().unwrap().running.keys().cloned().collect() + } + + /// Wait until none of `session_ids` are registered as running, or until + /// `timeout` elapses. Returns `true` if they all deregistered in time. + /// + /// Modelled on `ActionExecutor::wait_for_executions`: session threads + /// deregister themselves as they exit, so polling the registry is how the + /// shutdown path learns a cancelled session's agent is actually gone rather + /// than exiting out from under it. + pub fn wait_for_sessions(&self, session_ids: &[String], timeout: Duration) -> bool { + let deadline = std::time::Instant::now() + timeout; + + loop { + let all_stopped = { + let inner = self.inner.lock().unwrap(); + session_ids + .iter() + .all(|session_id| !inner.running.contains_key(session_id)) + }; + + if all_stopped { + return true; + } + + if std::time::Instant::now() >= deadline { + return false; + } + + std::thread::sleep(Duration::from_millis(25)); + } + } + /// Register a session whose work is driven outside `start_session` (e.g. a /// pikchr diagram child session run by a `generate_pikchr` worker thread), /// so a user cancel reaches the actual work instead of taking @@ -3579,6 +3617,35 @@ mod tests { assert_eq!(failed.completion_reason, Some(CompletionReason::Crashed)); } + #[test] + fn wait_for_sessions_returns_once_every_session_deregisters() { + let registry = Arc::new(SessionRegistry::new()); + registry.register("session-1"); + registry.register("session-2"); + let session_ids = registry.running_session_ids(); + assert_eq!(session_ids.len(), 2); + + let deregistering = Arc::clone(®istry); + std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(50)); + deregistering.deregister("session-1"); + deregistering.deregister("session-2"); + }); + + assert!(registry.wait_for_sessions(&session_ids, Duration::from_secs(2))); + assert!(registry.running_session_ids().is_empty()); + } + + #[test] + fn wait_for_sessions_times_out_while_a_session_is_still_running() { + let registry = SessionRegistry::new(); + registry.register("session-1"); + + assert!(!registry.wait_for_sessions(&["session-1".to_string()], Duration::from_millis(50))); + // Unknown ids count as stopped, so a stale snapshot can't block a quit. + assert!(registry.wait_for_sessions(&["gone".to_string()], Duration::from_millis(50))); + } + #[test] fn running_project_session_cancellation_records_completion_reason_override() { let registry = SessionRegistry::new(); diff --git a/apps/staged/src-tauri/src/web_server.rs b/apps/staged/src-tauri/src/web_server.rs index 2c76aaaf..2c80e18a 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -555,6 +555,16 @@ async fn dispatch(command: &str, args: Value, state: &WebAppState) -> Result { + Err(format!("{command} is not available in web mode")) + } + // ===================================================================== // Projects // ===================================================================== diff --git a/apps/staged/src/App.svelte b/apps/staged/src/App.svelte index 4d680c14..3dd41cbe 100644 --- a/apps/staged/src/App.svelte +++ b/apps/staged/src/App.svelte @@ -19,6 +19,7 @@ import ProjectsList from './lib/features/projects/ProjectsList.svelte'; import ProjectsSidebar from './lib/features/projects/ProjectsSidebar.svelte'; import ProjectDeleteDialog from './lib/features/projects/ProjectDeleteDialog.svelte'; + import QuitConfirmDialog from './lib/features/lifecycle/QuitConfirmDialog.svelte'; import ReposListView from './lib/features/projects/ReposListView.svelte'; import SessionLauncher from './lib/features/sessions/SessionLauncher.svelte'; import SettingsPage from './lib/features/settings/SettingsPage.svelte'; @@ -62,6 +63,7 @@ import { listenForPageLifecycle } from './lib/listeners/pageLifecycleListener'; import { listenForAcpToolsReconciled } from './lib/listeners/acpToolsListener'; import { listenForMenuEvents } from './lib/listeners/menuListener'; + import { listenForQuitRequests } from './lib/listeners/quitListener'; import { darkMode } from './lib/stores/isDark.svelte'; import * as prPollingService from './lib/services/prPollingService'; import type { StoreIncompatibility } from './lib/types'; @@ -77,6 +79,7 @@ let unlistenAcpToolsReconciled: UnlistenFn | undefined; let unlistenStoreReset: UnlistenFn | undefined; let unlistenUpdaterOwnerAvailable: UnlistenFn | undefined; + let unlistenQuitRequests: UnlistenFn | undefined; let unregisterShortcuts: (() => void) | null = null; let stopUpdaterLoop: (() => void) | null = null; let updaterStartPending = false; @@ -356,6 +359,9 @@ // Refresh provider discovery (and any loaded doctor report) once the // backend finishes installing/upgrading the managed ACP bridges. unlistenAcpToolsReconciled = listenForAcpToolsReconciled(); + // Raise the quit confirmation when the backend gates a quit on running + // sessions (Tauri only — see quitListener.ts). + unlistenQuitRequests = listenForQuitRequests(); // Keep the shared project-list cache fresh for the app's lifetime — the // store dedupes, so starting before any view consumes it is safe. projectsDataStore.startListeners(); @@ -568,6 +574,7 @@ unlistenAcpToolsReconciled?.(); unlistenStoreReset?.(); unlistenUpdaterOwnerAvailable?.(); + unlistenQuitRequests?.(); projectsDataStore.stopListeners(); projectRunActionsStore.stopListening(); stopUpdaterLoop?.(); @@ -586,8 +593,10 @@ } } + // Quits rather than closing the window: closing the last window only hides + // it, and there is no usable app behind this screen to come back to. function handleClose() { - getWindowSync().close(); + void commands.quitApp().catch((e) => console.error('Failed to quit:', e)); } @@ -697,6 +706,9 @@ point (sidebar, landing grid, ProjectHome top bar/shortcut). --> + + + {/if} diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index 2dedb910..89a8fcee 100644 --- a/apps/staged/src/lib/commands.ts +++ b/apps/staged/src/lib/commands.ts @@ -68,6 +68,33 @@ export function confirmResetStore(): Promise { return invokeCommand('confirm_reset_store'); } +// ============================================================================= +// App lifecycle +// ============================================================================= + +/** + * Quit Staged, through the same gate as `Cmd+Q` — it raises the confirmation + * dialog when sessions are still running. Closing the window only hides it, so + * UI that means "end the app" (the store-incompatibility screens) needs this. + */ +export function quitApp(): Promise { + return invokeCommand('quit_app'); +} + +/** + * Confirm the quit raised by `app:quit-requested`: the backend stops the active + * sessions and running actions, then exits. Desktop only — the command is not in + * the web-mode dispatch table, so a browser client cannot quit the host. + */ +export function confirmQuit(): Promise { + return invokeCommand('confirm_quit'); +} + +/** Decline the quit; sessions keep running. */ +export function cancelQuit(): Promise { + return invokeCommand('cancel_quit'); +} + // ============================================================================= // Projects // ============================================================================= diff --git a/apps/staged/src/lib/features/lifecycle/QuitConfirmDialog.svelte b/apps/staged/src/lib/features/lifecycle/QuitConfirmDialog.svelte new file mode 100644 index 00000000..db45d8fd --- /dev/null +++ b/apps/staged/src/lib/features/lifecycle/QuitConfirmDialog.svelte @@ -0,0 +1,62 @@ + + + + !v && quitPrompt.cancel()}> + + + Quit Staged? + {description} + + + Cancel + quitPrompt.confirm()} + > + {quitPrompt.stopping ? 'Stopping sessions…' : 'Quit & Stop Sessions'} + + + + diff --git a/apps/staged/src/lib/features/lifecycle/quitPromptCopy.test.ts b/apps/staged/src/lib/features/lifecycle/quitPromptCopy.test.ts new file mode 100644 index 00000000..0f1310ce --- /dev/null +++ b/apps/staged/src/lib/features/lifecycle/quitPromptCopy.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from 'vitest'; +import type { ActiveSessionInfo } from '../../types'; +import { quitPromptDescription, quitSessionLabel } from './quitPromptCopy'; + +function session(overrides: Partial = {}): ActiveSessionInfo { + return { + sessionId: 's1', + projectId: 'p1', + branchId: 'b1', + sessionType: 'review', + status: 'running', + ...overrides, + }; +} + +describe('quitSessionLabel', () => { + it('names the session type and where it runs', () => { + expect(quitSessionLabel(session(), 'fix-login')).toBe('review on fix-login'); + }); + + it('marks queued sessions', () => { + expect(quitSessionLabel(session({ status: 'queued' }), 'fix-login')).toBe( + 'review on fix-login (queued)' + ); + }); + + it('falls back to "session" for an unknown or missing type', () => { + expect(quitSessionLabel(session({ sessionType: null }), 'fix-login')).toBe( + 'session on fix-login' + ); + expect(quitSessionLabel(session({ sessionType: 'mystery' }), 'fix-login')).toBe( + 'session on fix-login' + ); + }); + + it('drops the location when there is none to show', () => { + expect(quitSessionLabel(session({ sessionType: 'note' }), null)).toBe('note'); + }); +}); + +describe('quitPromptDescription', () => { + it('reads singular for one session', () => { + expect(quitPromptDescription(['commit on fix-login'], 0)).toBe( + '1 session is still running. Quitting will stop it. commit on fix-login.' + ); + }); + + it('lists every session for a plural count', () => { + expect(quitPromptDescription(['commit on fix-login', 'note on docs'], 0)).toBe( + '2 sessions are still running. Quitting will stop them. commit on fix-login, note on docs.' + ); + }); + + it('mentions running actions only when there are some', () => { + expect(quitPromptDescription(['commit on fix-login'], 1)).toContain( + '1 running action will also stop.' + ); + expect(quitPromptDescription(['commit on fix-login'], 3)).toContain( + '3 running actions will also stop.' + ); + expect(quitPromptDescription(['commit on fix-login'], 0)).not.toContain('action'); + }); +}); diff --git a/apps/staged/src/lib/features/lifecycle/quitPromptCopy.ts b/apps/staged/src/lib/features/lifecycle/quitPromptCopy.ts new file mode 100644 index 00000000..508f3dbc --- /dev/null +++ b/apps/staged/src/lib/features/lifecycle/quitPromptCopy.ts @@ -0,0 +1,60 @@ +/** + * Copy for the quit confirmation dialog. + * + * Kept out of the component so the wording is unit-testable: the dialog resolves + * each session's branch/project names from the stores and passes labels in. + */ + +import type { ActiveSessionInfo } from '../../types'; + +/** How each session type reads in the dialog body. */ +const SESSION_TYPE_LABELS: Record = { + note: 'note', + commit: 'commit', + review: 'review', + pr: 'PR', + push: 'push', + pull: 'pull', +}; + +/** + * Label for one session that a quit would stop, e.g. `review on fix-login` or + * `commit on fix-login (queued)`. + * + * `where` is the branch name when the session belongs to one, otherwise the + * project name — project-level sessions (notes on a project) have no branch. + */ +export function quitSessionLabel(session: ActiveSessionInfo, where: string | null): string { + const kind = session.sessionType ? SESSION_TYPE_LABELS[session.sessionType] : null; + const base = where ? `${kind ?? 'session'} on ${where}` : (kind ?? 'session'); + return session.status === 'queued' ? `${base} (queued)` : base; +} + +/** + * Dialog body: how much stops, what it is, and whether actions go with it. + * + * Actions never gate the quit (see `should_prompt` in `app_lifecycle.rs`), so + * they are mentioned only as a consequence of one. + */ +export function quitPromptDescription(sessionLabels: string[], runningActionCount: number): string { + const count = sessionLabels.length; + const sentences = [ + count === 1 + ? '1 session is still running. Quitting will stop it.' + : `${count} sessions are still running. Quitting will stop them.`, + ]; + + if (sessionLabels.length > 0) { + sentences.push(`${sessionLabels.join(', ')}.`); + } + + if (runningActionCount > 0) { + sentences.push( + runningActionCount === 1 + ? '1 running action will also stop.' + : `${runningActionCount} running actions will also stop.` + ); + } + + return sentences.join(' '); +} diff --git a/apps/staged/src/lib/features/projects/ProjectHome.svelte b/apps/staged/src/lib/features/projects/ProjectHome.svelte index c6beb8ab..26fda21e 100644 --- a/apps/staged/src/lib/features/projects/ProjectHome.svelte +++ b/apps/staged/src/lib/features/projects/ProjectHome.svelte @@ -12,7 +12,6 @@ import Pause from '@lucide/svelte/icons/pause'; import Plus from '@lucide/svelte/icons/plus'; import Trash2 from '@lucide/svelte/icons/trash-2'; - import { getWindowSync } from '../../transport'; import type { Project, ProjectRepo, @@ -208,8 +207,10 @@ } } + // Quits rather than closing the window: closing the last window only hides + // it, and there is no usable app behind this screen to come back to. function handleClose() { - getWindowSync().close(); + void commands.quitApp().catch((e) => console.error('Failed to quit:', e)); } function scheduleDeferredTask(callback: () => void, timeout = 1500): () => void { diff --git a/apps/staged/src/lib/listeners/quitListener.test.ts b/apps/staged/src/lib/listeners/quitListener.test.ts new file mode 100644 index 00000000..70c885fb --- /dev/null +++ b/apps/staged/src/lib/listeners/quitListener.test.ts @@ -0,0 +1,137 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { ActiveSessionInfo, QuitRequestedPayload } from '../types'; + +const confirmQuit = vi.fn<() => Promise>(); +const cancelQuit = vi.fn<() => Promise>(); +const unlisten = vi.fn(); +// Window-scoped on purpose: the backend addresses the event to one window with +// `emit_to`, and an any-target listener would raise the dialog in all of them. +const listenToWindowEvent = vi.fn(); + +let handlers: Array<(payload: QuitRequestedPayload) => void>; + +/** + * Load the listener and store fresh, with transport in the requested mode. The + * store is a singleton, so each test needs its own module registry. + */ +async function load({ isTauri = true } = {}) { + vi.resetModules(); + vi.doMock('../transport', () => ({ isTauri, listenToWindowEvent })); + vi.doMock('../api/commands', () => ({ confirmQuit, cancelQuit })); + + const { listenForQuitRequests } = await import('./quitListener'); + const { quitPrompt } = await import('../stores/quitPrompt.svelte'); + return { listenForQuitRequests, quitPrompt }; +} + +function session(overrides: Partial = {}): ActiveSessionInfo { + return { + sessionId: 's1', + projectId: 'p1', + branchId: 'b1', + sessionType: 'commit', + status: 'running', + ...overrides, + }; +} + +describe('quitListener', () => { + beforeEach(() => { + // The store's runes compile away in the app build; under vitest they stay + // plain global calls, so stub $state as identity (projectsData.test.ts + // precedent). + vi.stubGlobal('$state', (initial: unknown) => initial); + handlers = []; + confirmQuit.mockReset().mockResolvedValue(undefined); + cancelQuit.mockReset().mockResolvedValue(undefined); + unlisten.mockReset(); + listenToWindowEvent.mockReset().mockImplementation((_event, handler) => { + handlers.push(handler as (payload: QuitRequestedPayload) => void); + return unlisten; + }); + }); + + afterEach(() => { + vi.doUnmock('../transport'); + vi.doUnmock('../api/commands'); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('opens the prompt with the payload from app:quit-requested', async () => { + const { listenForQuitRequests, quitPrompt } = await load(); + listenForQuitRequests(); + + expect(listenToWindowEvent).toHaveBeenCalledWith('app:quit-requested', expect.any(Function)); + expect(quitPrompt.open).toBe(false); + + handlers[0]({ sessions: [session()], runningActionCount: 2 }); + + expect(quitPrompt.open).toBe(true); + expect(quitPrompt.payload?.sessions).toHaveLength(1); + expect(quitPrompt.payload?.runningActionCount).toBe(2); + expect(quitPrompt.stopping).toBe(false); + }); + + it('registers no listener in web mode', async () => { + const { listenForQuitRequests } = await load({ isTauri: false }); + + // Callable no-op, so App.svelte's teardown needs no extra guard. + listenForQuitRequests()(); + + expect(listenToWindowEvent).not.toHaveBeenCalled(); + }); + + it('confirming invokes confirm_quit and leaves the dialog stopping', async () => { + const { listenForQuitRequests, quitPrompt } = await load(); + listenForQuitRequests(); + handlers[0]({ sessions: [session()], runningActionCount: 0 }); + + await quitPrompt.confirm(); + + expect(confirmQuit).toHaveBeenCalledTimes(1); + // The backend exits the process; until it does, the dialog reports progress + // instead of pretending the app is still usable. + expect(quitPrompt.open).toBe(true); + expect(quitPrompt.stopping).toBe(true); + + await quitPrompt.confirm(); + expect(confirmQuit).toHaveBeenCalledTimes(1); + }); + + it('closes the dialog when confirm_quit fails', async () => { + const { listenForQuitRequests, quitPrompt } = await load(); + listenForQuitRequests(); + handlers[0]({ sessions: [session()], runningActionCount: 0 }); + confirmQuit.mockRejectedValueOnce(new Error('nope')); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + await quitPrompt.confirm(); + + expect(quitPrompt.open).toBe(false); + expect(quitPrompt.stopping).toBe(false); + }); + + it('cancelling invokes cancel_quit and closes the dialog', async () => { + const { listenForQuitRequests, quitPrompt } = await load(); + listenForQuitRequests(); + handlers[0]({ sessions: [session()], runningActionCount: 0 }); + + quitPrompt.cancel(); + + expect(cancelQuit).toHaveBeenCalledTimes(1); + expect(quitPrompt.open).toBe(false); + }); + + it('ignores a cancel once the quit is under way', async () => { + const { listenForQuitRequests, quitPrompt } = await load(); + listenForQuitRequests(); + handlers[0]({ sessions: [session()], runningActionCount: 0 }); + + await quitPrompt.confirm(); + quitPrompt.cancel(); + + expect(cancelQuit).not.toHaveBeenCalled(); + expect(quitPrompt.open).toBe(true); + }); +}); diff --git a/apps/staged/src/lib/listeners/quitListener.ts b/apps/staged/src/lib/listeners/quitListener.ts new file mode 100644 index 00000000..f88e6ac9 --- /dev/null +++ b/apps/staged/src/lib/listeners/quitListener.ts @@ -0,0 +1,24 @@ +/** + * Listener for the backend's `app:quit-requested` event. + * + * `Cmd+Q` / the app-menu Quit item reach `app_lifecycle::request_quit`, which + * emits this event instead of exiting when sessions are still active. The + * backend addresses it to exactly one window with `emit_to`, so this must be a + * *window-scoped* listener — the any-target `listenToEvent` also matches emits + * addressed to other windows, and every window would raise its own dialog. + * Wired at App level so it works on any route, and Tauri-only: quitting is a + * desktop-host action, and the `confirm_quit` command a browser client would + * need is deliberately absent from the web-mode dispatch table. + */ + +import { isTauri, listenToWindowEvent, type UnlistenFn } from '../transport'; +import { quitPrompt } from '../stores/quitPrompt.svelte'; +import type { QuitRequestedPayload } from '../types'; + +export function listenForQuitRequests(): UnlistenFn { + if (!isTauri) return () => {}; + + return listenToWindowEvent('app:quit-requested', (payload) => { + quitPrompt.requested(payload); + }); +} diff --git a/apps/staged/src/lib/stores/quitPrompt.svelte.ts b/apps/staged/src/lib/stores/quitPrompt.svelte.ts new file mode 100644 index 00000000..0ee0dbec --- /dev/null +++ b/apps/staged/src/lib/stores/quitPrompt.svelte.ts @@ -0,0 +1,61 @@ +/** + * State behind the quit confirmation dialog. + * + * The backend raises `app:quit-requested` when the user quits with sessions + * still active (see `app_lifecycle.rs`); quitListener.ts feeds that payload in + * here and QuitConfirmDialog renders it. Answering is a round trip back to the + * backend: confirming hands off to the shutdown sequence, which stops the + * sessions and then exits the process — so the dialog stays up, in its + * `stopping` state, until the app goes away underneath it. + */ + +import * as commands from '../api/commands'; +import type { QuitRequestedPayload } from '../types'; + +class QuitPromptStore { + private _payload = $state(null); + /** The quit was confirmed and the backend is stopping sessions. */ + private _stopping = $state(false); + + get payload(): QuitRequestedPayload | null { + return this._payload; + } + + get open(): boolean { + return this._payload !== null; + } + + get stopping(): boolean { + return this._stopping; + } + + /** A quit is waiting on the user's answer. */ + requested(payload: QuitRequestedPayload): void { + this._payload = payload; + this._stopping = false; + } + + /** Quit and stop the listed sessions. */ + async confirm(): Promise { + if (this._stopping) return; + this._stopping = true; + try { + await commands.confirmQuit(); + } catch (e) { + // The quit never started, so drop the dialog rather than leaving it stuck + // on "Stopping sessions…" for an app that isn't going anywhere. + console.error('Failed to confirm quit:', e); + this._payload = null; + this._stopping = false; + } + } + + /** Keep running. Also the Esc / click-outside path. */ + cancel(): void { + if (this._stopping) return; + this._payload = null; + void commands.cancelQuit().catch((e) => console.error('Failed to cancel quit:', e)); + } +} + +export const quitPrompt = new QuitPromptStore(); diff --git a/apps/staged/src/lib/types.ts b/apps/staged/src/lib/types.ts index 2d023080..f6379193 100644 --- a/apps/staged/src/lib/types.ts +++ b/apps/staged/src/lib/types.ts @@ -576,6 +576,18 @@ export interface ActiveSessionInfo { status: SessionStatus; } +/** + * Payload of the `app:quit-requested` event: what a quit would interrupt. + * + * Emitted by `app_lifecycle::request_quit` when the user quits with sessions + * still active. Sessions are what gate the quit; running actions are reported + * so the dialog can say they stop too. + */ +export interface QuitRequestedPayload { + sessions: ActiveSessionInfo[]; + runningActionCount: number; +} + /** * Payload emitted by the `pr-created` domain event when a completed PR * session produced a pull request. The backend has already persisted the PR From 941614d3881899b0b4993b6e9b8a8d3d4eac7624 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 1 Sep 2026 16:11:21 +1000 Subject: [PATCH 2/2] refactor(lifecycle): ask before quitting with an unparented native alert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The quit confirmation lived in the webview: `request_quit` emitted `app:quit-requested` to one chosen window and a Svelte `AlertDialog` rendered it. That forced a window into the quit path, and the case it hurt is the central scenario of this branch — the whole point of closing-window-is-not- quitting is that sessions keep streaming with everything hidden, so "all windows hidden, sessions running, user hits Cmd+Q" isn't a corner to tolerate, it's the primary path the confirmation exists to serve. Reaching it materialised a full application window (restored geometry, hydrating project tree) to host a two-button question, and `cancel_quit` did nothing but clear a flag — so Cmd+Q then Cancel left the user with a visible window they had to close a second time, having asked for neither. Parenting a native alert would not have fixed that: `.parent()` is exactly what makes `tauri-plugin-dialog` render an `NSAlert` as a window-modal *sheet*, so the reveal would have stayed. Unparented is a different widget, not a different modality of the same one — rfd 0.16 routes a parentless dialog to `CFUserNotificationDisplayAlert`, displayed by the system rather than by AppKit. That's what buys window-independence, so the reveal drops out of the quit path entirely: quitting from a hidden state stays hidden, cancelling returns the app to exactly the state the user left it in, and the branch where no window could be revealed and the app quit *without asking* disappears rather than being preserved. Structurally this resolves the review finding about a pending prompt outliving its host window by removing the concept of a host window. `QuitState.prompt_host` collapses to `prompt_pending: AtomicBool`; `clear_prompt_if_host`, the `Destroyed` arm of `on_window_event`, and `reveal_a_window` (now inlined into its one caller, `show_a_window`) all go away. The frontend half goes with them: `QuitConfirmDialog`, the `quitPrompt` store, `quitListener`, `quitPromptCopy`, the `QuitRequestedPayload` type, and the `confirm_quit` / `cancel_quit` commands. `quit_app` stays — the store-incompatibility screens still need it — and stays refused in the web-mode dispatch table. The prompt copy ports to Rust, where `get_branch` / `get_project` resolve the names the Svelte dialog used to read from its stores; the review's suggestion to fold the session list into the preceding sentence with a colon is taken while the wording moves. `OkCancelCustom`'s ok slot is the default (Return) button, so it holds "Keep Running" and the *cancel* slot holds "Quit & Stop Sessions" — a stray Return must not be what kills running agents. Accepted costs, all inherent to the widget: the alert carries generic system chrome rather than Staged's icon; the "Stopping sessions…" progress state is gone, since a native alert dismisses on click while `shutdown_cleanup` runs out its 2s budget; and it is not modal to the app, so work can start behind it. The last resolves correctly — `shutdown_cleanup` re-queries active sessions instead of trusting the prompt's snapshot — and it keeps the force-quit escape hatch dispatchable, which a second Cmd+Q needs. Because that dismissal leaves nothing on screen, `request_quit` now returns early when a shutdown is already under way instead of raising a second alert about sessions the first one is stopping. Verified with `just check-all`. Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/app_lifecycle.rs | 477 +++++++++++++----- apps/staged/src-tauri/src/lib.rs | 7 +- apps/staged/src-tauri/src/web_server.rs | 7 +- apps/staged/src/App.svelte | 10 - apps/staged/src/lib/commands.ts | 24 +- .../lifecycle/QuitConfirmDialog.svelte | 62 --- .../features/lifecycle/quitPromptCopy.test.ts | 63 --- .../lib/features/lifecycle/quitPromptCopy.ts | 60 --- .../src/lib/listeners/quitListener.test.ts | 137 ----- apps/staged/src/lib/listeners/quitListener.ts | 24 - .../src/lib/stores/quitPrompt.svelte.ts | 61 --- apps/staged/src/lib/types.ts | 12 - 12 files changed, 350 insertions(+), 594 deletions(-) delete mode 100644 apps/staged/src/lib/features/lifecycle/QuitConfirmDialog.svelte delete mode 100644 apps/staged/src/lib/features/lifecycle/quitPromptCopy.test.ts delete mode 100644 apps/staged/src/lib/features/lifecycle/quitPromptCopy.ts delete mode 100644 apps/staged/src/lib/listeners/quitListener.test.ts delete mode 100644 apps/staged/src/lib/listeners/quitListener.ts delete mode 100644 apps/staged/src/lib/stores/quitPrompt.svelte.ts diff --git a/apps/staged/src-tauri/src/app_lifecycle.rs b/apps/staged/src-tauri/src/app_lifecycle.rs index aae9e791..43f715e6 100644 --- a/apps/staged/src-tauri/src/app_lifecycle.rs +++ b/apps/staged/src-tauri/src/app_lifecycle.rs @@ -14,14 +14,25 @@ //! still quits there — but through the same confirmation gate as `Cmd+Q`. //! //! **Quitting with sessions running asks first, then stops them cleanly.** -//! [`request_quit`] gates on active sessions and hands the decision to the -//! frontend dialog, addressed to a single live window (revealed first if every -//! window is hidden); [`shutdown_cleanup`] cancels sessions with -//! [`CompletionReason::AppQuit`] and stops actions. That cancel is the only -//! thing that shuts an agent down: ACP children are spawned with -//! `process_group(0)` and `kill_on_drop`, and `process::exit` runs no +//! [`request_quit`] gates on active sessions and asks; [`shutdown_cleanup`] +//! cancels sessions with [`CompletionReason::AppQuit`] and stops actions. That +//! cancel is the only thing that shuts an agent down: ACP children are spawned +//! with `process_group(0)` and `kill_on_drop`, and `process::exit` runs no //! destructors, so a bare exit leaves the agent CLIs running. //! +//! The question is asked by a native alert with **no parent window**, not by a +//! dialog in a webview. Quitting is scoped to the application, and the case the +//! confirmation exists for is precisely the one where every window is hidden: +//! parenting the alert (which `tauri-plugin-dialog` renders as a window-modal +//! sheet) would drag a full window back on screen — restored geometry, +//! hydrating project tree and all — to host a two-button question, and +//! cancelling would leave it there. Unparented, rfd reaches for +//! `CFUserNotificationDisplayAlert` on macOS instead of `NSAlert`: system +//! chrome rather than the app's, and not modal to the app, in exchange for +//! needing no window at all. So quitting from a hidden state stays hidden, +//! cancelling returns the app to exactly the state the user left it in, and +//! there is no longer any state where a quit can't ask. +//! //! Every exit path funnels into [`shutdown_cleanup`], which runs its work at //! most once — a confirmed quit calls it directly, `RunEvent::ExitRequested` //! covers programmatic exits, and `RunEvent::Exit` is the only hook on the @@ -32,16 +43,28 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; -use serde::Serialize; -use tauri::{AppHandle, Emitter, Manager, WebviewWindow, Window, WindowEvent}; +use tauri::{AppHandle, Manager, Window, WindowEvent}; +use tauri_plugin_dialog::{ + DialogExt, MessageDialogButtons, MessageDialogKind, MessageDialogResult, +}; use crate::actions; use crate::session_commands::{self, ActiveSessionInfo}; use crate::session_runner::SessionRegistry; use crate::store::{CompletionReason, Session, SessionStatus, Store}; -/// Event that raises the frontend's quit confirmation dialog. -const QUIT_REQUESTED_EVENT: &str = "app:quit-requested"; +/// Title of the quit confirmation alert. +const QUIT_PROMPT_TITLE: &str = "Quit Staged?"; + +/// Alert button that goes through with the quit. +/// +/// It sits in `OkCancelCustom`'s *cancel* slot, and [`KEEP_RUNNING_BUTTON`] in +/// the ok slot, because the ok slot is the default (`Return`) button — a stray +/// Return must not be what kills a room full of running agents. +const QUIT_BUTTON: &str = "Quit & Stop Sessions"; + +/// Alert button that dismisses the prompt and leaves the sessions alone. +const KEEP_RUNNING_BUTTON: &str = "Keep Running"; /// Menu id of the app-menu Quit item. Custom rather than /// `PredefinedMenuItem::quit` so `Cmd+Q` is routable at all: the predefined item @@ -73,44 +96,33 @@ pub struct QuitState { /// Set by the first caller into [`shutdown_cleanup`], so the cleanup runs /// exactly once however many exit events follow it. quit_in_progress: AtomicBool, - /// Label of the window showing an unanswered confirmation dialog. A quit - /// request arriving while it is set forces the quit — a wedged webview must - /// never be able to trap the app, so a second `Cmd+Q` always gets out. The - /// label is what lets a destroyed host window clear the flag instead of - /// leaving that force path armed with no dialog on screen. - prompt_host: Mutex>, + /// Set while a confirmation alert is unanswered. A quit request arriving + /// while it is set forces the quit, so an alert that never appeared or never + /// came back can't trap the app: a second `Cmd+Q` always gets out. + prompt_pending: AtomicBool, } impl QuitState { - fn set_prompt_host(&self, label: &str) { - *self.prompt_host.lock().unwrap() = Some(label.to_string()); + fn set_prompt_pending(&self) { + self.prompt_pending.store(true, Ordering::SeqCst); } /// Clear any pending prompt, returning whether one was pending. fn take_prompt(&self) -> bool { - self.prompt_host.lock().unwrap().take().is_some() - } - - /// Clear the pending prompt if `label` was hosting it. - fn clear_prompt_if_host(&self, label: &str) { - let mut host = self.prompt_host.lock().unwrap(); - if host.as_deref() == Some(label) { - *host = None; - } + self.prompt_pending.swap(false, Ordering::SeqCst) } } -/// What a quit would interrupt, as sent to the confirmation dialog. -#[derive(Debug, Clone, Default, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct QuitBlockers { - /// Running and queued sessions owned by this process — the only thing that - /// gates a quit. - pub sessions: Vec, - /// Running actions. Reported so the dialog can say they stop too, but they +/// What a quit would interrupt, as the alert describes it. +#[derive(Debug, Default)] +struct QuitBlockers { + /// One label per running or queued session owned by this process, e.g. + /// `review on fix-login` — the only thing that gates a quit. + session_labels: Vec, + /// Running actions. Reported so the alert can say they stop too, but they /// don't gate the quit on their own: a dev server left running is the normal /// state of a workspace, and blocking `Cmd+Q` on it would be noise. - pub running_action_count: usize, + running_action_count: usize, } /// Whether a quit should stop and ask first. @@ -118,7 +130,7 @@ pub struct QuitBlockers { /// Queued sessions count: they're work the user asked for that a quit silently /// drops, so they belong in the prompt. fn should_prompt(blockers: &QuitBlockers) -> bool { - !blockers.sessions.is_empty() + !blockers.session_labels.is_empty() } // ============================================================================= @@ -128,17 +140,8 @@ fn should_prompt(blockers: &QuitBlockers) -> bool { /// `Builder::on_window_event` hook — see the module docs for why closing the /// last window doesn't end the process. pub fn on_window_event(window: &Window, event: &WindowEvent) { - match event { - WindowEvent::CloseRequested { api, .. } => on_close_requested(window, api), - // A destroyed window takes its webview — and any dialog in it — with - // it. Left set, the pending flag would turn the next quit request into - // a silent force-quit; cleared, that quit just asks again. - WindowEvent::Destroyed => { - if let Some(quit_state) = window.app_handle().try_state::() { - quit_state.clear_prompt_if_host(window.label()); - } - } - _ => {} + if let WindowEvent::CloseRequested { api, .. } = event { + on_close_requested(window, api); } } @@ -190,25 +193,16 @@ fn hide_window(window: &Window) { set_native_focus(window.app_handle(), window.label(), false); } -/// Bring a window back on screen: the Dock-icon click, `Window ▸ Staged`, and a -/// quit arriving with no visible window all funnel here. -pub fn show_a_window(app: &AppHandle) { - if reveal_a_window(app).is_none() { - log::warn!("No window left to show"); - } -} - -/// Pick a window and make sure it is on screen and focused, returning it. +/// Bring a window back on screen: the Dock-icon click and `Window ▸ Staged` +/// both funnel here. Deliberately *not* on the quit path — see the module docs. /// -/// Prefers where the user already is (focused, then visible — reachable when a -/// quit request arrives from the store-incompatibility screen or the web -/// dispatch refusal path while windows are up), then falls back to unhiding one: -/// `main` for its restored geometry, else any. `None` only if every window has -/// been destroyed, which no close path produces — closing the last window hides -/// it instead. -fn reveal_a_window(app: &AppHandle) -> Option { +/// Prefers where the user already is (focused, then visible), then falls back to +/// unhiding one: `main` for its restored geometry, else any surviving `win-N` +/// peer. Finds nothing only if every window has been destroyed, which no close +/// path produces — closing the last window hides it instead. +pub fn show_a_window(app: &AppHandle) { let windows = app.webview_windows(); - let window = windows + let Some(window) = windows .values() .find(|window| window.is_focused().unwrap_or(false)) .or_else(|| { @@ -217,7 +211,11 @@ fn reveal_a_window(app: &AppHandle) -> Option { .find(|window| window.is_visible().unwrap_or(false)) }) .or_else(|| windows.get(MAIN_WINDOW_LABEL)) - .or_else(|| windows.values().next())?; + .or_else(|| windows.values().next()) + else { + log::warn!("No window left to show"); + return; + }; if let Err(e) = window.show() { log::warn!("Failed to show window: {e}"); @@ -229,7 +227,6 @@ fn reveal_a_window(app: &AppHandle) -> Option { log::warn!("Failed to focus window: {e}"); } set_native_focus(app, window.label(), true); - Some(window.clone()) } /// Mirror a native window's visibility onto its PR-poll client's focus hint. @@ -247,38 +244,71 @@ fn set_native_focus(app: &AppHandle, window_label: &str, focused: bool) { /// Handle a quit request from the app menu, `Cmd+Q`, or (off macOS) the last /// window's close. Cheap enough for the main thread: it snapshots blockers and -/// either hands off to a background quit or raises the dialog. +/// either hands off to a background quit or raises the alert. pub fn request_quit(app: &AppHandle, force: bool) { let quit_state = app.state::(); - // A quit arriving while the dialog is unanswered (a second `Cmd+Q`) is the - // escape hatch from a webview that never rendered or answered it. + // A quit arriving while the alert is unanswered (a second `Cmd+Q`) is the + // escape hatch from a prompt that never appeared or never came back. The + // system alert isn't app-modal, so that second `Cmd+Q` is still dispatchable + // with the alert on screen. if force || quit_state.take_prompt() { spawn_quit(app); return; } + // Already shutting down, and the alert dismissed on click while cleanup runs + // out its budget — so there is nothing on screen saying so, and a `Cmd+Q` + // here means "I already answered", not "ask me again". + if quit_state.quit_in_progress.load(Ordering::SeqCst) { + return; + } + let blockers = collect_quit_blockers(app); if !should_prompt(&blockers) { spawn_quit(app); return; } - // The dialog goes to exactly one window — where the user is, or a window - // revealed for the purpose if the quit arrived with everything hidden. A - // broadcast would raise one dialog per window, each unaware of the others' - // answers. No window at all means nobody to ask, so the quit proceeds. - let Some(host) = reveal_a_window(app) else { - spawn_quit(app); - return; - }; - quit_state.set_prompt_host(host.label()); + quit_state.set_prompt_pending(); + ask_before_quitting(app, &blockers); +} - if let Err(e) = app.emit_to(host.label(), QUIT_REQUESTED_EVENT, &blockers) { - log::warn!("Failed to ask for quit confirmation, quitting anyway: {e}"); - quit_state.take_prompt(); - spawn_quit(app); - } +/// Raise the confirmation alert and act on the answer. +/// +/// No `.parent()`, which is what keeps this window-independent — see the module +/// docs. `tauri-plugin-dialog` hops to the main thread to start the alert and +/// then runs it on its own thread, so this returns immediately and the event +/// loop keeps turning underneath it. +fn ask_before_quitting(app: &AppHandle, blockers: &QuitBlockers) { + let app = app.clone(); + app.dialog() + .message(quit_prompt_message(blockers)) + .title(QUIT_PROMPT_TITLE) + .kind(MessageDialogKind::Warning) + .buttons(MessageDialogButtons::OkCancelCustom( + KEEP_RUNNING_BUTTON.to_string(), + QUIT_BUTTON.to_string(), + )) + .show_with_result(move |result| { + app.state::().take_prompt(); + // Anything that isn't the quit button — "Keep Running", or the + // system dismissing the alert itself — leaves the sessions alone. + // Nothing to undo on that path: no window was revealed to host the + // question, so the app is already in the state the user left it in. + if quit_confirmed(&result) { + // The snapshot the message was built from may be stale by now: + // the alert is not modal to the app, so a session could have + // started or finished behind it. `shutdown_cleanup` re-queries, + // so it stops what is actually running. + spawn_quit(&app); + } + }); +} + +/// Whether the alert was answered with [`QUIT_BUTTON`]. +fn quit_confirmed(result: &MessageDialogResult) -> bool { + matches!(result, MessageDialogResult::Custom(label) if label == QUIT_BUTTON) } /// Quit from the UI, through the same gate as `Cmd+Q`. @@ -286,30 +316,17 @@ pub fn request_quit(app: &AppHandle, force: bool) { /// Used by the store-incompatibility screens' "Close" button, which has to end /// the app: closing the last window only hides it, and those screens have no /// working database behind them to come back to. -#[tauri::command] -pub fn quit_app(app_handle: AppHandle) { - request_quit(&app_handle, false); -} - -/// Quit confirmed in the dialog: stop sessions and actions, then exit. /// /// Deliberately absent from the web-mode `dispatch` table — a browser client /// must not be able to terminate the desktop host. #[tauri::command] -pub fn confirm_quit(app_handle: AppHandle) { - app_handle.state::().take_prompt(); - spawn_quit(&app_handle); -} - -/// Quit declined in the dialog: sessions keep running. -#[tauri::command] -pub fn cancel_quit(app_handle: AppHandle) { - app_handle.state::().take_prompt(); +pub fn quit_app(app_handle: AppHandle) { + request_quit(&app_handle, false); } /// Run the quit sequence off the main thread so the bounded waits never freeze -/// the event loop — the dialog stays interactive and can render its -/// "Stopping sessions…" state while agents shut down. +/// the event loop — windows keep repainting while agents shut down, and the +/// close events the exit generates are still delivered. fn spawn_quit(app: &AppHandle) { let app = app.clone(); std::thread::spawn(move || { @@ -320,10 +337,14 @@ fn spawn_quit(app: &AppHandle) { /// Snapshot what a quit would interrupt. fn collect_quit_blockers(app: &AppHandle) -> QuitBlockers { - let sessions = match app_store(app) { + let session_labels = match app_store(app) { Some(store) => owned_active_sessions(&store) .iter() - .map(|session| session_commands::project_active_session(&store, session)) + .map(|session| { + let session = session_commands::project_active_session(&store, session); + let location = session_location(&store, &session); + quit_session_label(&session, location.as_deref()) + }) .collect(), None => Vec::new(), }; @@ -341,11 +362,94 @@ fn collect_quit_blockers(app: &AppHandle) -> QuitBlockers { }; QuitBlockers { - sessions, + session_labels, running_action_count, } } +// ============================================================================= +// Prompt copy +// ============================================================================= + +/// How each session type reads in the alert. +fn session_type_label(session_type: &str) -> Option<&'static str> { + match session_type { + "note" => Some("note"), + "commit" => Some("commit"), + "review" => Some("review"), + "pr" => Some("PR"), + "push" => Some("push"), + "pull" => Some("pull"), + _ => None, + } +} + +/// Where a session is running, as the user knows it: its branch name, or its +/// project name for project-level sessions (a note on a project has no branch). +fn session_location(store: &Store, session: &ActiveSessionInfo) -> Option { + let branch_name = session + .branch_id + .as_deref() + .and_then(|id| store.get_branch(id).ok().flatten()) + .map(|branch| branch.branch_name); + + branch_name.or_else(|| { + session + .project_id + .as_deref() + .and_then(|id| store.get_project(id).ok().flatten()) + .map(|project| project.name) + }) +} + +/// Label for one session a quit would stop, e.g. `review on fix-login` or +/// `commit on fix-login (queued)`. +/// +/// Both halves can be missing — an unrecognised session type, or a row whose +/// branch and project have already been deleted — so each falls back rather than +/// dropping the session from the list. +fn quit_session_label(session: &ActiveSessionInfo, location: Option<&str>) -> String { + let kind = session + .session_type + .as_deref() + .and_then(session_type_label) + .unwrap_or("session"); + let base = match location { + Some(location) => format!("{kind} on {location}"), + None => kind.to_string(), + }; + + if session.status == SessionStatus::Queued.as_str() { + format!("{base} (queued)") + } else { + base + } +} + +/// Alert body: how much stops, what it is, and whether actions go with it. +/// +/// Actions never gate the quit (see [`should_prompt`]), so they are mentioned +/// only as a consequence of one. +fn quit_prompt_message(blockers: &QuitBlockers) -> String { + let labels = blockers.session_labels.join(", "); + let mut message = if blockers.session_labels.len() == 1 { + format!("1 session is still running: {labels}. Quitting will stop it.") + } else { + format!( + "{} sessions are still running: {labels}. Quitting will stop them.", + blockers.session_labels.len() + ) + }; + + match blockers.running_action_count { + 0 => {} + 1 => message.push_str(" 1 running action will also stop."), + count => message.push_str(&format!(" {count} running actions will also stop.")), + } + + message +} + // ============================================================================= // Shutdown cleanup // ============================================================================= @@ -503,41 +607,31 @@ mod tests { use super::*; use std::path::Path; - fn active_session(status: &str) -> ActiveSessionInfo { + fn active_session(session_type: Option<&str>, status: SessionStatus) -> ActiveSessionInfo { ActiveSessionInfo { session_id: "s1".to_string(), - project_id: None, - branch_id: None, - session_type: None, - status: status.to_string(), + project_id: Some("p1".to_string()), + branch_id: Some("b1".to_string()), + session_type: session_type.map(str::to_string), + status: status.as_str().to_string(), } } - #[test] - fn running_sessions_prompt() { - let blockers = QuitBlockers { - sessions: vec![active_session("running")], - running_action_count: 0, - }; - assert!(should_prompt(&blockers)); + fn blockers(session_labels: &[&str], running_action_count: usize) -> QuitBlockers { + QuitBlockers { + session_labels: session_labels.iter().map(|s| s.to_string()).collect(), + running_action_count, + } } #[test] - fn queued_sessions_prompt() { - let blockers = QuitBlockers { - sessions: vec![active_session("queued")], - running_action_count: 0, - }; - assert!(should_prompt(&blockers)); + fn active_sessions_prompt() { + assert!(should_prompt(&blockers(&["review on fix-login"], 0))); } #[test] fn running_actions_alone_do_not_prompt() { - let blockers = QuitBlockers { - sessions: Vec::new(), - running_action_count: 3, - }; - assert!(!should_prompt(&blockers)); + assert!(!should_prompt(&blockers(&[], 3))); } #[test] @@ -545,23 +639,128 @@ mod tests { assert!(!should_prompt(&QuitBlockers::default())); } - /// The pending flag turns the next quit into a force-quit, so it must not - /// outlive the window whose dialog it stands for — but a *peer* window - /// closing must not answer a dialog it isn't showing. + /// The pending flag turns the next quit into a force-quit, so answering the + /// alert has to disarm it — otherwise the next `Cmd+Q` quits without asking. #[test] - fn prompt_clears_only_when_its_host_window_is_destroyed() { + fn answering_the_prompt_disarms_the_force_path() { let state = QuitState::default(); - state.set_prompt_host("win-2"); - state.clear_prompt_if_host("main"); - assert!(state.take_prompt(), "peer destruction dropped the prompt"); + assert!(!state.take_prompt(), "nothing pending, nothing to force"); + + state.set_prompt_pending(); + assert!(state.take_prompt(), "pending prompt did not arm the force"); + assert!(!state.take_prompt(), "prompt stayed armed after answering"); + } + + /// The ok slot is the default (`Return`) button, so it holds "Keep Running" + /// and the cancel slot holds the destructive answer. + #[test] + fn only_the_quit_button_confirms() { + assert!(quit_confirmed(&MessageDialogResult::Custom( + QUIT_BUTTON.to_string() + ))); + assert!(!quit_confirmed(&MessageDialogResult::Custom( + KEEP_RUNNING_BUTTON.to_string() + ))); + // What a system-dismissed alert reports. + assert!(!quit_confirmed(&MessageDialogResult::Cancel)); + assert!(!quit_confirmed(&MessageDialogResult::Ok)); + } + + #[test] + fn session_label_names_the_type_and_where_it_runs() { + assert_eq!( + quit_session_label( + &active_session(Some("review"), SessionStatus::Running), + Some("fix-login") + ), + "review on fix-login" + ); + } + + #[test] + fn session_label_marks_queued_sessions() { + assert_eq!( + quit_session_label( + &active_session(Some("review"), SessionStatus::Queued), + Some("fix-login") + ), + "review on fix-login (queued)" + ); + } + + #[test] + fn session_label_falls_back_for_unknown_type_or_missing_location() { + assert_eq!( + quit_session_label(&active_session(None, SessionStatus::Running), Some("docs")), + "session on docs" + ); + assert_eq!( + quit_session_label( + &active_session(Some("mystery"), SessionStatus::Running), + Some("docs") + ), + "session on docs" + ); + assert_eq!( + quit_session_label(&active_session(Some("note"), SessionStatus::Running), None), + "note" + ); + } + + /// A branch session reads as its branch; a project-level session (a note on + /// a project) has no branch, so it reads as its project. + #[test] + fn session_location_prefers_the_branch_then_the_project() { + let store = Store::in_memory().unwrap(); + let mut project = crate::store::Project::new("owner/repo"); + project.name = "Widgets".to_string(); + store.create_project(&project).unwrap(); + let branch = crate::store::Branch::new(&project.id, "fix-login", "main"); + store.create_branch(&branch).unwrap(); + + let mut session = active_session(Some("note"), SessionStatus::Running); + session.project_id = Some(project.id.clone()); + session.branch_id = Some(branch.id.clone()); + assert_eq!( + session_location(&store, &session).as_deref(), + Some("fix-login") + ); - state.set_prompt_host("win-2"); - state.clear_prompt_if_host("win-2"); - assert!( - !state.take_prompt(), - "host destruction left the prompt armed" + session.branch_id = None; + assert_eq!( + session_location(&store, &session).as_deref(), + Some("Widgets") ); + + session.project_id = None; + assert_eq!(session_location(&store, &session), None); + } + + #[test] + fn prompt_message_reads_singular_for_one_session() { + assert_eq!( + quit_prompt_message(&blockers(&["commit on fix-login"], 0)), + "1 session is still running: commit on fix-login. Quitting will stop it." + ); + } + + #[test] + fn prompt_message_lists_every_session_for_a_plural_count() { + assert_eq!( + quit_prompt_message(&blockers(&["commit on fix-login", "note on docs"], 0)), + "2 sessions are still running: commit on fix-login, note on docs. \ + Quitting will stop them." + ); + } + + #[test] + fn prompt_message_mentions_actions_only_when_there_are_some() { + assert!(quit_prompt_message(&blockers(&["commit on fix-login"], 1)) + .ends_with(" 1 running action will also stop.")); + assert!(quit_prompt_message(&blockers(&["commit on fix-login"], 3)) + .ends_with(" 3 running actions will also stop.")); + assert!(!quit_prompt_message(&blockers(&["commit on fix-login"], 0)).contains("action")); } #[test] diff --git a/apps/staged/src-tauri/src/lib.rs b/apps/staged/src-tauri/src/lib.rs index f782093a..4cd2994c 100644 --- a/apps/staged/src-tauri/src/lib.rs +++ b/apps/staged/src-tauri/src/lib.rs @@ -2219,8 +2219,7 @@ pub fn run() { } }) .on_window_event(|window, event| { - // Close-to-hide / the quit gate (`CloseRequested`), and dropping a - // pending quit prompt whose host window went away (`Destroyed`). + // Close-to-hide / the quit gate (`CloseRequested`). app_lifecycle::on_window_event(window, event); if let tauri::WindowEvent::Destroyed = event { @@ -2259,10 +2258,8 @@ pub fn run() { window_commands::take_window_seed, window_commands::claim_updater_ownership, // Lifecycle — desktop only; the web-mode `dispatch` table refuses - // these so a browser client can't quit the host. + // this so a browser client can't quit the host. app_lifecycle::quit_app, - app_lifecycle::confirm_quit, - app_lifecycle::cancel_quit, list_projects, create_project, list_project_repos, diff --git a/apps/staged/src-tauri/src/web_server.rs b/apps/staged/src-tauri/src/web_server.rs index 2c80e18a..11c4ea03 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -560,10 +560,9 @@ async fn dispatch(command: &str, args: Value, state: &WebAppState) -> Result { - Err(format!("{command} is not available in web mode")) - } + // confirmation this gates on is a native alert on the host anyway, with + // no browser client to answer it. + "quit_app" => Err(format!("{command} is not available in web mode")), // ===================================================================== // Projects diff --git a/apps/staged/src/App.svelte b/apps/staged/src/App.svelte index 3dd41cbe..0aa5144a 100644 --- a/apps/staged/src/App.svelte +++ b/apps/staged/src/App.svelte @@ -19,7 +19,6 @@ import ProjectsList from './lib/features/projects/ProjectsList.svelte'; import ProjectsSidebar from './lib/features/projects/ProjectsSidebar.svelte'; import ProjectDeleteDialog from './lib/features/projects/ProjectDeleteDialog.svelte'; - import QuitConfirmDialog from './lib/features/lifecycle/QuitConfirmDialog.svelte'; import ReposListView from './lib/features/projects/ReposListView.svelte'; import SessionLauncher from './lib/features/sessions/SessionLauncher.svelte'; import SettingsPage from './lib/features/settings/SettingsPage.svelte'; @@ -63,7 +62,6 @@ import { listenForPageLifecycle } from './lib/listeners/pageLifecycleListener'; import { listenForAcpToolsReconciled } from './lib/listeners/acpToolsListener'; import { listenForMenuEvents } from './lib/listeners/menuListener'; - import { listenForQuitRequests } from './lib/listeners/quitListener'; import { darkMode } from './lib/stores/isDark.svelte'; import * as prPollingService from './lib/services/prPollingService'; import type { StoreIncompatibility } from './lib/types'; @@ -79,7 +77,6 @@ let unlistenAcpToolsReconciled: UnlistenFn | undefined; let unlistenStoreReset: UnlistenFn | undefined; let unlistenUpdaterOwnerAvailable: UnlistenFn | undefined; - let unlistenQuitRequests: UnlistenFn | undefined; let unregisterShortcuts: (() => void) | null = null; let stopUpdaterLoop: (() => void) | null = null; let updaterStartPending = false; @@ -359,9 +356,6 @@ // Refresh provider discovery (and any loaded doctor report) once the // backend finishes installing/upgrading the managed ACP bridges. unlistenAcpToolsReconciled = listenForAcpToolsReconciled(); - // Raise the quit confirmation when the backend gates a quit on running - // sessions (Tauri only — see quitListener.ts). - unlistenQuitRequests = listenForQuitRequests(); // Keep the shared project-list cache fresh for the app's lifetime — the // store dedupes, so starting before any view consumes it is safe. projectsDataStore.startListeners(); @@ -574,7 +568,6 @@ unlistenAcpToolsReconciled?.(); unlistenStoreReset?.(); unlistenUpdaterOwnerAvailable?.(); - unlistenQuitRequests?.(); projectsDataStore.stopListeners(); projectRunActionsStore.stopListening(); stopUpdaterLoop?.(); @@ -706,9 +699,6 @@ point (sidebar, landing grid, ProjectHome top bar/shortcut). --> - - - {/if} diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index 89a8fcee..dd4a0e67 100644 --- a/apps/staged/src/lib/commands.ts +++ b/apps/staged/src/lib/commands.ts @@ -73,28 +73,18 @@ export function confirmResetStore(): Promise { // ============================================================================= /** - * Quit Staged, through the same gate as `Cmd+Q` — it raises the confirmation - * dialog when sessions are still running. Closing the window only hides it, so - * UI that means "end the app" (the store-incompatibility screens) needs this. + * Quit Staged, through the same gate as `Cmd+Q` — the backend raises a native + * confirmation alert when sessions are still running, and owns the answer. + * Closing the window only hides it, so UI that means "end the app" (the + * store-incompatibility screens) needs this. + * + * Desktop only: the command is not in the web-mode dispatch table, so a browser + * client cannot quit the host. */ export function quitApp(): Promise { return invokeCommand('quit_app'); } -/** - * Confirm the quit raised by `app:quit-requested`: the backend stops the active - * sessions and running actions, then exits. Desktop only — the command is not in - * the web-mode dispatch table, so a browser client cannot quit the host. - */ -export function confirmQuit(): Promise { - return invokeCommand('confirm_quit'); -} - -/** Decline the quit; sessions keep running. */ -export function cancelQuit(): Promise { - return invokeCommand('cancel_quit'); -} - // ============================================================================= // Projects // ============================================================================= diff --git a/apps/staged/src/lib/features/lifecycle/QuitConfirmDialog.svelte b/apps/staged/src/lib/features/lifecycle/QuitConfirmDialog.svelte deleted file mode 100644 index db45d8fd..00000000 --- a/apps/staged/src/lib/features/lifecycle/QuitConfirmDialog.svelte +++ /dev/null @@ -1,62 +0,0 @@ - - - - !v && quitPrompt.cancel()}> - - - Quit Staged? - {description} - - - Cancel - quitPrompt.confirm()} - > - {quitPrompt.stopping ? 'Stopping sessions…' : 'Quit & Stop Sessions'} - - - - diff --git a/apps/staged/src/lib/features/lifecycle/quitPromptCopy.test.ts b/apps/staged/src/lib/features/lifecycle/quitPromptCopy.test.ts deleted file mode 100644 index 0f1310ce..00000000 --- a/apps/staged/src/lib/features/lifecycle/quitPromptCopy.test.ts +++ /dev/null @@ -1,63 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import type { ActiveSessionInfo } from '../../types'; -import { quitPromptDescription, quitSessionLabel } from './quitPromptCopy'; - -function session(overrides: Partial = {}): ActiveSessionInfo { - return { - sessionId: 's1', - projectId: 'p1', - branchId: 'b1', - sessionType: 'review', - status: 'running', - ...overrides, - }; -} - -describe('quitSessionLabel', () => { - it('names the session type and where it runs', () => { - expect(quitSessionLabel(session(), 'fix-login')).toBe('review on fix-login'); - }); - - it('marks queued sessions', () => { - expect(quitSessionLabel(session({ status: 'queued' }), 'fix-login')).toBe( - 'review on fix-login (queued)' - ); - }); - - it('falls back to "session" for an unknown or missing type', () => { - expect(quitSessionLabel(session({ sessionType: null }), 'fix-login')).toBe( - 'session on fix-login' - ); - expect(quitSessionLabel(session({ sessionType: 'mystery' }), 'fix-login')).toBe( - 'session on fix-login' - ); - }); - - it('drops the location when there is none to show', () => { - expect(quitSessionLabel(session({ sessionType: 'note' }), null)).toBe('note'); - }); -}); - -describe('quitPromptDescription', () => { - it('reads singular for one session', () => { - expect(quitPromptDescription(['commit on fix-login'], 0)).toBe( - '1 session is still running. Quitting will stop it. commit on fix-login.' - ); - }); - - it('lists every session for a plural count', () => { - expect(quitPromptDescription(['commit on fix-login', 'note on docs'], 0)).toBe( - '2 sessions are still running. Quitting will stop them. commit on fix-login, note on docs.' - ); - }); - - it('mentions running actions only when there are some', () => { - expect(quitPromptDescription(['commit on fix-login'], 1)).toContain( - '1 running action will also stop.' - ); - expect(quitPromptDescription(['commit on fix-login'], 3)).toContain( - '3 running actions will also stop.' - ); - expect(quitPromptDescription(['commit on fix-login'], 0)).not.toContain('action'); - }); -}); diff --git a/apps/staged/src/lib/features/lifecycle/quitPromptCopy.ts b/apps/staged/src/lib/features/lifecycle/quitPromptCopy.ts deleted file mode 100644 index 508f3dbc..00000000 --- a/apps/staged/src/lib/features/lifecycle/quitPromptCopy.ts +++ /dev/null @@ -1,60 +0,0 @@ -/** - * Copy for the quit confirmation dialog. - * - * Kept out of the component so the wording is unit-testable: the dialog resolves - * each session's branch/project names from the stores and passes labels in. - */ - -import type { ActiveSessionInfo } from '../../types'; - -/** How each session type reads in the dialog body. */ -const SESSION_TYPE_LABELS: Record = { - note: 'note', - commit: 'commit', - review: 'review', - pr: 'PR', - push: 'push', - pull: 'pull', -}; - -/** - * Label for one session that a quit would stop, e.g. `review on fix-login` or - * `commit on fix-login (queued)`. - * - * `where` is the branch name when the session belongs to one, otherwise the - * project name — project-level sessions (notes on a project) have no branch. - */ -export function quitSessionLabel(session: ActiveSessionInfo, where: string | null): string { - const kind = session.sessionType ? SESSION_TYPE_LABELS[session.sessionType] : null; - const base = where ? `${kind ?? 'session'} on ${where}` : (kind ?? 'session'); - return session.status === 'queued' ? `${base} (queued)` : base; -} - -/** - * Dialog body: how much stops, what it is, and whether actions go with it. - * - * Actions never gate the quit (see `should_prompt` in `app_lifecycle.rs`), so - * they are mentioned only as a consequence of one. - */ -export function quitPromptDescription(sessionLabels: string[], runningActionCount: number): string { - const count = sessionLabels.length; - const sentences = [ - count === 1 - ? '1 session is still running. Quitting will stop it.' - : `${count} sessions are still running. Quitting will stop them.`, - ]; - - if (sessionLabels.length > 0) { - sentences.push(`${sessionLabels.join(', ')}.`); - } - - if (runningActionCount > 0) { - sentences.push( - runningActionCount === 1 - ? '1 running action will also stop.' - : `${runningActionCount} running actions will also stop.` - ); - } - - return sentences.join(' '); -} diff --git a/apps/staged/src/lib/listeners/quitListener.test.ts b/apps/staged/src/lib/listeners/quitListener.test.ts deleted file mode 100644 index 70c885fb..00000000 --- a/apps/staged/src/lib/listeners/quitListener.test.ts +++ /dev/null @@ -1,137 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { ActiveSessionInfo, QuitRequestedPayload } from '../types'; - -const confirmQuit = vi.fn<() => Promise>(); -const cancelQuit = vi.fn<() => Promise>(); -const unlisten = vi.fn(); -// Window-scoped on purpose: the backend addresses the event to one window with -// `emit_to`, and an any-target listener would raise the dialog in all of them. -const listenToWindowEvent = vi.fn(); - -let handlers: Array<(payload: QuitRequestedPayload) => void>; - -/** - * Load the listener and store fresh, with transport in the requested mode. The - * store is a singleton, so each test needs its own module registry. - */ -async function load({ isTauri = true } = {}) { - vi.resetModules(); - vi.doMock('../transport', () => ({ isTauri, listenToWindowEvent })); - vi.doMock('../api/commands', () => ({ confirmQuit, cancelQuit })); - - const { listenForQuitRequests } = await import('./quitListener'); - const { quitPrompt } = await import('../stores/quitPrompt.svelte'); - return { listenForQuitRequests, quitPrompt }; -} - -function session(overrides: Partial = {}): ActiveSessionInfo { - return { - sessionId: 's1', - projectId: 'p1', - branchId: 'b1', - sessionType: 'commit', - status: 'running', - ...overrides, - }; -} - -describe('quitListener', () => { - beforeEach(() => { - // The store's runes compile away in the app build; under vitest they stay - // plain global calls, so stub $state as identity (projectsData.test.ts - // precedent). - vi.stubGlobal('$state', (initial: unknown) => initial); - handlers = []; - confirmQuit.mockReset().mockResolvedValue(undefined); - cancelQuit.mockReset().mockResolvedValue(undefined); - unlisten.mockReset(); - listenToWindowEvent.mockReset().mockImplementation((_event, handler) => { - handlers.push(handler as (payload: QuitRequestedPayload) => void); - return unlisten; - }); - }); - - afterEach(() => { - vi.doUnmock('../transport'); - vi.doUnmock('../api/commands'); - vi.unstubAllGlobals(); - vi.restoreAllMocks(); - }); - - it('opens the prompt with the payload from app:quit-requested', async () => { - const { listenForQuitRequests, quitPrompt } = await load(); - listenForQuitRequests(); - - expect(listenToWindowEvent).toHaveBeenCalledWith('app:quit-requested', expect.any(Function)); - expect(quitPrompt.open).toBe(false); - - handlers[0]({ sessions: [session()], runningActionCount: 2 }); - - expect(quitPrompt.open).toBe(true); - expect(quitPrompt.payload?.sessions).toHaveLength(1); - expect(quitPrompt.payload?.runningActionCount).toBe(2); - expect(quitPrompt.stopping).toBe(false); - }); - - it('registers no listener in web mode', async () => { - const { listenForQuitRequests } = await load({ isTauri: false }); - - // Callable no-op, so App.svelte's teardown needs no extra guard. - listenForQuitRequests()(); - - expect(listenToWindowEvent).not.toHaveBeenCalled(); - }); - - it('confirming invokes confirm_quit and leaves the dialog stopping', async () => { - const { listenForQuitRequests, quitPrompt } = await load(); - listenForQuitRequests(); - handlers[0]({ sessions: [session()], runningActionCount: 0 }); - - await quitPrompt.confirm(); - - expect(confirmQuit).toHaveBeenCalledTimes(1); - // The backend exits the process; until it does, the dialog reports progress - // instead of pretending the app is still usable. - expect(quitPrompt.open).toBe(true); - expect(quitPrompt.stopping).toBe(true); - - await quitPrompt.confirm(); - expect(confirmQuit).toHaveBeenCalledTimes(1); - }); - - it('closes the dialog when confirm_quit fails', async () => { - const { listenForQuitRequests, quitPrompt } = await load(); - listenForQuitRequests(); - handlers[0]({ sessions: [session()], runningActionCount: 0 }); - confirmQuit.mockRejectedValueOnce(new Error('nope')); - vi.spyOn(console, 'error').mockImplementation(() => {}); - - await quitPrompt.confirm(); - - expect(quitPrompt.open).toBe(false); - expect(quitPrompt.stopping).toBe(false); - }); - - it('cancelling invokes cancel_quit and closes the dialog', async () => { - const { listenForQuitRequests, quitPrompt } = await load(); - listenForQuitRequests(); - handlers[0]({ sessions: [session()], runningActionCount: 0 }); - - quitPrompt.cancel(); - - expect(cancelQuit).toHaveBeenCalledTimes(1); - expect(quitPrompt.open).toBe(false); - }); - - it('ignores a cancel once the quit is under way', async () => { - const { listenForQuitRequests, quitPrompt } = await load(); - listenForQuitRequests(); - handlers[0]({ sessions: [session()], runningActionCount: 0 }); - - await quitPrompt.confirm(); - quitPrompt.cancel(); - - expect(cancelQuit).not.toHaveBeenCalled(); - expect(quitPrompt.open).toBe(true); - }); -}); diff --git a/apps/staged/src/lib/listeners/quitListener.ts b/apps/staged/src/lib/listeners/quitListener.ts deleted file mode 100644 index f88e6ac9..00000000 --- a/apps/staged/src/lib/listeners/quitListener.ts +++ /dev/null @@ -1,24 +0,0 @@ -/** - * Listener for the backend's `app:quit-requested` event. - * - * `Cmd+Q` / the app-menu Quit item reach `app_lifecycle::request_quit`, which - * emits this event instead of exiting when sessions are still active. The - * backend addresses it to exactly one window with `emit_to`, so this must be a - * *window-scoped* listener — the any-target `listenToEvent` also matches emits - * addressed to other windows, and every window would raise its own dialog. - * Wired at App level so it works on any route, and Tauri-only: quitting is a - * desktop-host action, and the `confirm_quit` command a browser client would - * need is deliberately absent from the web-mode dispatch table. - */ - -import { isTauri, listenToWindowEvent, type UnlistenFn } from '../transport'; -import { quitPrompt } from '../stores/quitPrompt.svelte'; -import type { QuitRequestedPayload } from '../types'; - -export function listenForQuitRequests(): UnlistenFn { - if (!isTauri) return () => {}; - - return listenToWindowEvent('app:quit-requested', (payload) => { - quitPrompt.requested(payload); - }); -} diff --git a/apps/staged/src/lib/stores/quitPrompt.svelte.ts b/apps/staged/src/lib/stores/quitPrompt.svelte.ts deleted file mode 100644 index 0ee0dbec..00000000 --- a/apps/staged/src/lib/stores/quitPrompt.svelte.ts +++ /dev/null @@ -1,61 +0,0 @@ -/** - * State behind the quit confirmation dialog. - * - * The backend raises `app:quit-requested` when the user quits with sessions - * still active (see `app_lifecycle.rs`); quitListener.ts feeds that payload in - * here and QuitConfirmDialog renders it. Answering is a round trip back to the - * backend: confirming hands off to the shutdown sequence, which stops the - * sessions and then exits the process — so the dialog stays up, in its - * `stopping` state, until the app goes away underneath it. - */ - -import * as commands from '../api/commands'; -import type { QuitRequestedPayload } from '../types'; - -class QuitPromptStore { - private _payload = $state(null); - /** The quit was confirmed and the backend is stopping sessions. */ - private _stopping = $state(false); - - get payload(): QuitRequestedPayload | null { - return this._payload; - } - - get open(): boolean { - return this._payload !== null; - } - - get stopping(): boolean { - return this._stopping; - } - - /** A quit is waiting on the user's answer. */ - requested(payload: QuitRequestedPayload): void { - this._payload = payload; - this._stopping = false; - } - - /** Quit and stop the listed sessions. */ - async confirm(): Promise { - if (this._stopping) return; - this._stopping = true; - try { - await commands.confirmQuit(); - } catch (e) { - // The quit never started, so drop the dialog rather than leaving it stuck - // on "Stopping sessions…" for an app that isn't going anywhere. - console.error('Failed to confirm quit:', e); - this._payload = null; - this._stopping = false; - } - } - - /** Keep running. Also the Esc / click-outside path. */ - cancel(): void { - if (this._stopping) return; - this._payload = null; - void commands.cancelQuit().catch((e) => console.error('Failed to cancel quit:', e)); - } -} - -export const quitPrompt = new QuitPromptStore(); diff --git a/apps/staged/src/lib/types.ts b/apps/staged/src/lib/types.ts index f6379193..2d023080 100644 --- a/apps/staged/src/lib/types.ts +++ b/apps/staged/src/lib/types.ts @@ -576,18 +576,6 @@ export interface ActiveSessionInfo { status: SessionStatus; } -/** - * Payload of the `app:quit-requested` event: what a quit would interrupt. - * - * Emitted by `app_lifecycle::request_quit` when the user quits with sessions - * still active. Sessions are what gate the quit; running actions are reported - * so the dialog can say they stop too. - */ -export interface QuitRequestedPayload { - sessions: ActiveSessionInfo[]; - runningActionCount: number; -} - /** * Payload emitted by the `pr-created` domain event when a completed PR * session produced a pull request. The backend has already persisted the PR