diff --git a/src-tauri/src/app/mac_update.rs b/src-tauri/src/app/mac_update.rs index ae19d77..d1e6a1b 100644 --- a/src-tauri/src/app/mac_update.rs +++ b/src-tauri/src/app/mac_update.rs @@ -12,7 +12,7 @@ //! already on the latest build (the user's case during development). use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use serde::Serialize; @@ -577,7 +577,7 @@ pub fn stage_macos_update_with_network( // A standalone stage can also be cancelled (its download sets the abort // latch). Own a guard so a cancelled stage can't leave the latch set and // make the NEXT perform/install abort itself at its first checkpoint. - let _abort_guard = AbortGuard; + let _abort_guard = AbortGuard::new(); let installed = detect_managed_installed(); let (_, xml) = fetch_appcast_for_arch(arch_of(&installed), network)?; let appcast = parse_appcast(&xml).map_err(|e| AppError::Engine(e.to_string()))?; @@ -838,7 +838,7 @@ pub fn perform_macos_update_with_network_and_phase( }; // Reset the latch when THIS op ends (not at entry) so a cancel racing the // op's startup isn't wiped. See AbortGuard. - let _abort_guard = AbortGuard; + let _abort_guard = AbortGuard::new(); set_phase(OperationPhase::Preparing); // A vanished install is itself a stale snapshot: the user confirmed an // update against a Codex that is no longer there (deleted / moved between @@ -999,10 +999,9 @@ pub fn perform_macos_update_with_network_and_phase( )); } - // Point of no return. Honor a cancel one last time BEFORE we touch the - // user's running Codex — this also closes the gap after the preparing- - // phase checkpoint where a fully-cached artifact skips the download loop - // (so its cancel flag never arms) yet reconstruct/gate still ran. + // Honor a cancel before touching the user's running Codex. A second, + // phase-linearized checkpoint immediately before the swap closes the + // later quit/cancel window. check_update_abort()?; // Re-verify the TARGET right before the destructive tail: the early @@ -1038,6 +1037,15 @@ pub fn perform_macos_update_with_network_and_phase( quit_codex_gracefully(&install_path)?; log::info!("macOS perform step=swap"); set_phase(OperationPhase::Committing); + // The phase transition and the app quit preparation share the operation + // mutex. If quit won first its latch is now visible; if commit won first, + // quit was blocked. No destructive rename occurs between those outcomes. + if let Err(err) = check_update_abort() { + if was_running { + let _ = relaunch(&install_path); + } + return Err(err); + } let had_previous = install_path.exists(); let mut tx = ActiveInstallTx::begin( InstallTxKind::MacosSwap, @@ -1331,10 +1339,18 @@ pub fn install_macos(progress: &dyn Fn(DownloadProgress)) -> Result Result { + install_macos_with_network_and_phase(progress, network, None) +} + +pub fn install_macos_with_network_and_phase( + progress: &dyn Fn(DownloadProgress), + network: &NetworkConfig, + phase_hook: Option<&PhaseHook<'_>>, ) -> Result { log::info!("macOS install start"); // Reset on op end, not entry — race-free cancel (see AbortGuard). - let _abort_guard = AbortGuard; + let _abort_guard = AbortGuard::new(); if detect_installed().is_some() { return Err(AppError::Engine( "已检测到 Codex,请使用更新而非安装".to_string(), @@ -1369,6 +1385,7 @@ pub fn install_macos_with_network( &staging, progress, network, + phase_hook, ); match install_result { Ok(status) => { @@ -1396,6 +1413,7 @@ fn install_macos_in_staging( staging: &StagingDir, progress: &dyn Fn(DownloadProgress), network: &NetworkConfig, + phase_hook: Option<&PhaseHook<'_>>, ) -> Result { let work = staging.path(); let out_app = staging.join("Codex.app"); @@ -1403,6 +1421,9 @@ fn install_macos_in_staging( // Full package only — no basis bundle to delta against. reconstruct_full(appcast, work, &out_app, progress, network)?; + if let Some(hook) = phase_hook { + hook(OperationPhase::Verifying); + } gate_reconstructed(&out_app) .map_err(|e| AppError::Engine(format!("codesign 闸失败（拒绝安装）: {e}")))?; @@ -1412,11 +1433,19 @@ fn install_macos_in_staging( install_dir.display() ))); } - // Point of no return. Last cancel check before writing into the install - // location — covers a cached-artifact path that skipped the download loop. + // Publish the point-of-no-return while the same operation token is still + // held, then perform one final cancel checkpoint. A cancel that won the + // operation mutex first leaves its latch for this checkpoint; a later cancel + // sees Committing and is rejected instead of racing the rename. + if let Some(hook) = phase_hook { + hook(OperationPhase::Committing); + } check_update_abort()?; std::fs::rename(&out_app, install_path) .map_err(|e| AppError::Engine(format!("写入 {} 失败: {e}", install_dir.display())))?; + if let Some(hook) = phase_hook { + hook(OperationPhase::Finishing); + } let mut outcome = OperationOutcome::full_success("present", Some("managed")); outcome.cleanup = StepOutcome::not_applicable(); @@ -1487,7 +1516,7 @@ pub fn launch_codex() -> Result<(), AppError> { /// a cancel pressed during "正在准备" be honored at the next checkpoint. static UPDATE_ABORT: AtomicBool = AtomicBool::new(false); -fn clear_update_abort() { +pub(crate) fn clear_update_abort() { UPDATE_ABORT.store(false, Ordering::SeqCst); } @@ -1497,13 +1526,27 @@ fn clear_update_abort() { /// showing its cancel button and this operation reaching its first checkpoint is /// NOT wiped by an entry-clear, so the checkpoint still observes it; the latch is /// reset only once this operation is done, leaving the next one clean. The cancel -/// command does not hold the op lock, so this startup window is real — hence the -/// guard instead of an entry reset. +/// command now binds the latch to the owning operation under the op lock, but the +/// worker may still be between entry and its first checkpoint — hence the guard +/// instead of an entry reset. struct AbortGuard; +static UPDATE_ABORT_GUARD_DEPTH: AtomicUsize = AtomicUsize::new(0); + +impl AbortGuard { + fn new() -> Self { + UPDATE_ABORT_GUARD_DEPTH.fetch_add(1, Ordering::SeqCst); + Self + } +} + impl Drop for AbortGuard { fn drop(&mut self) { - clear_update_abort(); + let previous = UPDATE_ABORT_GUARD_DEPTH.fetch_sub(1, Ordering::SeqCst); + debug_assert!(previous > 0, "AbortGuard depth underflow"); + if previous == 1 { + clear_update_abort(); + } } } @@ -1851,20 +1894,29 @@ mod tests { #[test] fn abort_guard_preserves_a_startup_race_cancel_and_resets_on_drop() { + let _serial = crate::app::oplock::CANCEL_LATCH_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); // The race a reviewer flagged: a cancel can land BEFORE perform reaches - // its first checkpoint — the cancel command holds no op lock, and the UI - // shows the cancel button the moment it enters the progress state, before - // the backend call returns. Clearing the latch at op ENTRY would wipe that + // its first checkpoint even though the command now validates the owning + // token under the op lock. Clearing the latch at op ENTRY would wipe that // cancel; AbortGuard clears on DROP instead, so a pending cancel survives // the guard's creation and is still observed, while the next op starts - // clean. (Only this test touches UPDATE_ABORT, so it can't race others.) + // clean. OperationManager cleanup shares the same test-only serial lock. UPDATE_ABORT.store(true, Ordering::SeqCst); // a cancel that beat the op { - let _guard = AbortGuard; + let outer = AbortGuard::new(); + let inner = AbortGuard::new(); assert!( check_update_abort().is_err(), "guard creation must not wipe a pending cancel" ); + drop(inner); + assert!( + check_update_abort().is_err(), + "a nested guard must not clear the owning operation's cancel" + ); + drop(outer); } assert!( check_update_abort().is_ok(), diff --git a/src-tauri/src/app/oplock.rs b/src-tauri/src/app/oplock.rs index 5dde09c..971e735 100644 --- a/src-tauri/src/app/oplock.rs +++ b/src-tauri/src/app/oplock.rs @@ -171,6 +171,11 @@ impl Drop for OperationGuard { .is_some_and(|active| active.token == self.token.0) { let _ = OperationManager::unlock_lock_file(&mut inner); + // Linearize latch cleanup with removal of the owning lease. A cancel + // command takes the same mutex for both snapshots, so it either sees + // this owner and is followed by this cleanup, or sees no owner and + // immediately rolls its speculative latch back. + clear_cancel_latches(); inner.active.take(); log::info!( "released operation lock kind={} token_prefix={}", @@ -235,6 +240,7 @@ impl OperationManager { return Ok(()); } Self::unlock_lock_file(&mut inner)?; + clear_cancel_latches(); log::info!( "ended operation lock kind={} token_prefix={}", active_kind.as_str(), @@ -250,6 +256,24 @@ impl OperationManager { } pub fn validate(&self, token: &OperationToken) -> Result<(), OperationError> { + self.validate_inner(token, None) + } + + /// Claim a detached token and publish its first phase in one critical section. + /// Destructive operations use this to eliminate the validate→commit window. + pub fn validate_with_phase( + &self, + token: &OperationToken, + phase: OperationPhase, + ) -> Result<(), OperationError> { + self.validate_inner(token, Some(phase)) + } + + fn validate_inner( + &self, + token: &OperationToken, + phase: Option, + ) -> Result<(), OperationError> { let mut inner = self .inner .lock() @@ -270,6 +294,15 @@ impl OperationManager { ); } active.holders = active.holders.saturating_add(1); + if let Some(phase) = phase { + log::info!( + "claimed operation at phase kind={} phase={} token_prefix={}", + active.kind.as_str(), + phase.as_str(), + token_prefix(&token.0) + ); + active.phase = phase; + } return Ok(()); } } @@ -477,6 +510,71 @@ impl OperationManager { QuitPolicy::evaluate(force_quit, confirm_close, busy, phase, kind) } + /// Linearize an allowed or user-confirmed quit with the active phase. + /// + /// `prepare_exit` runs while the operation mutex is held when policy is + /// already `Allow`, or after an explicit confirmation. Callers use it to arm + /// platform cancellation latches and the force-exit flag before a worker can + /// advance to `Committing`. A worker that won the mutex first is observed as + /// blocked; a worker that advances afterward observes the final checkpoint. + pub fn prepare_quit( + &self, + confirm_close: bool, + confirmed: bool, + prepare_exit: impl FnOnce(), + ) -> QuitPolicy { + let Ok(mut inner) = self.inner.lock() else { + return QuitPolicy::evaluate( + false, + confirm_close, + true, + OperationPhase::Finishing, + None, + ); + }; + let _ = self.reclaim_stale_detached(&mut inner); + + let (busy, phase, kind) = if let Some(active) = inner.active.as_ref() { + (true, active.phase, Some(active.kind)) + } else { + let other_busy = match Self::lock_file_mut(&mut inner) { + Ok(lock_file) => match Fs4FileExt::try_lock(lock_file) { + Ok(()) => { + let _ = Fs4FileExt::unlock(lock_file); + false + } + Err(TryLockError::WouldBlock) => true, + Err(TryLockError::Error(_)) => false, + }, + Err(_) => false, + }; + if other_busy { + (true, OperationPhase::Finishing, None) + } else { + (false, OperationPhase::Idle, None) + } + }; + let policy = QuitPolicy::evaluate(false, confirm_close, busy, phase, kind); + let should_prepare = matches!(policy, QuitPolicy::Allow) + || (confirmed && matches!(policy, QuitPolicy::Confirm)); + if should_prepare { + // An armed detached token has not entered its command yet. Remove it + // under this same mutex so a delayed validate cannot start destructive + // work after force-exit preparation. Claimed workers keep their lease + // and must honor the phase-linearized abort checkpoint instead. + let abandon_unclaimed = inner + .active + .as_ref() + .is_some_and(|active| active.detached && !active.claimed); + if abandon_unclaimed { + let _ = Self::unlock_lock_file(&mut inner); + inner.active.take(); + } + prepare_exit(); + } + policy + } + /// Snapshot of the local active operation, for frontend reattach after /// renderer reload / remount. `None` when free (or only a cross-process lock). pub fn snapshot(&self) -> Option { @@ -500,6 +598,48 @@ impl OperationManager { }) } + /// Run a platform cancel signal while the active lease is locked and still + /// known to be interruptible. This is the linearization point between an + /// operation ending and a new one beginning: a process-global engine latch + /// can never be armed for operation A after operation B has taken its lease. + pub fn request_cancellation( + &self, + token: &OperationToken, + request: impl FnOnce() -> bool, + ) -> bool { + let Ok(inner) = self.inner.lock() else { + return false; + }; + if !inner + .active + .as_ref() + .is_some_and(|active| active.token == token.0 && active.phase.interruptible()) + { + return false; + } + request() + } + + /// Signal pause and mark the same active lease while holding the operation + /// mutex. Without this critical section a late pause for operation A could + /// observe the process-global downloader of newly-started operation B. + pub fn request_pause(&self, token: &OperationToken, request: impl FnOnce() -> bool) -> bool { + let Ok(mut inner) = self.inner.lock() else { + return false; + }; + let Some(active) = inner.active.as_mut() else { + return false; + }; + if active.token != token.0 || !active.phase.interruptible() { + return false; + } + let requested = request(); + if requested { + active.paused = true; + } + requested + } + /// Record the terminal outcome before the lease is released. Keeping a /// small token-keyed history lets a replacement renderer distinguish a /// true no-mutation failure from an ambiguous platform/mutation error. @@ -696,6 +836,7 @@ impl OperationManager { active.kind.as_str() ); Self::unlock_lock_file(inner)?; + clear_cancel_latches(); inner.active.take(); } Ok(()) @@ -712,6 +853,22 @@ impl OperationManager { } } +#[cfg(test)] +pub(crate) static CANCEL_LATCH_TEST_LOCK: Mutex<()> = Mutex::new(()); + +fn clear_cancel_latches() { + // The abort latches are process-global. Production OperationManager access is + // serialized by `inner`, while Rust's test harness may run unrelated managers + // and direct latch tests concurrently in one process. Share a test-only lock + // with those direct tests so a guard drop cannot clear their asserted value. + #[cfg(test)] + let _test_guard = CANCEL_LATCH_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + crate::app::mac_update::clear_update_abort(); + crate::app::win_update::clear_win_update_abort(); +} + fn token_prefix(token: &str) -> &str { token.get(..8).unwrap_or(token) } @@ -1074,6 +1231,227 @@ mod tests { let _ = fs::remove_dir_all(path.parent().unwrap()); } + #[test] + fn cancellation_signal_runs_only_inside_a_live_interruptible_lease() { + use crate::app::op_phase::OperationPhase; + use std::sync::atomic::{AtomicUsize, Ordering}; + + let path = lock_path("cancel-linearization"); + let manager = OperationManager::new(path.clone()); + let calls = AtomicUsize::new(0); + let missing = OperationToken("missing".to_string()); + assert!(!manager.request_cancellation(&missing, || { + calls.fetch_add(1, Ordering::SeqCst); + true + })); + + let guard = manager.begin(OperationKind::Update).unwrap(); + let owner = guard.token().clone(); + assert!(manager.request_cancellation(&owner, || { + calls.fetch_add(1, Ordering::SeqCst); + true + })); + manager + .set_phase(guard.token(), OperationPhase::Committing) + .unwrap(); + assert!(!manager.request_cancellation(&owner, || { + calls.fetch_add(1, Ordering::SeqCst); + true + })); + assert_eq!(calls.load(Ordering::SeqCst), 1); + + drop(guard); + let _ = fs::remove_dir_all(path.parent().unwrap()); + } + + #[test] + fn pause_signal_and_paused_marker_share_the_same_live_lease() { + use crate::app::op_phase::OperationPhase; + use std::sync::atomic::{AtomicUsize, Ordering}; + + let path = lock_path("pause-linearization"); + let manager = OperationManager::new(path.clone()); + let calls = AtomicUsize::new(0); + let missing = OperationToken("missing".to_string()); + assert!(!manager.request_pause(&missing, || { + calls.fetch_add(1, Ordering::SeqCst); + true + })); + + let guard = manager.begin(OperationKind::Update).unwrap(); + let owner = guard.token().clone(); + assert!(!manager.request_pause(&owner, || { + calls.fetch_add(1, Ordering::SeqCst); + false + })); + assert!(!manager.snapshot().unwrap().paused); + assert!(manager.request_pause(&owner, || { + calls.fetch_add(1, Ordering::SeqCst); + true + })); + assert!(manager.snapshot().unwrap().paused); + + manager + .set_phase(guard.token(), OperationPhase::Committing) + .unwrap(); + assert!(!manager.request_pause(&owner, || { + calls.fetch_add(1, Ordering::SeqCst); + true + })); + assert_eq!(calls.load(Ordering::SeqCst), 2); + + drop(guard); + let _ = fs::remove_dir_all(path.parent().unwrap()); + } + + #[test] + fn stale_stop_token_cannot_target_the_next_operation() { + use std::sync::atomic::{AtomicUsize, Ordering}; + + let path = lock_path("stale-stop-token"); + let manager = OperationManager::new(path.clone()); + let first = manager.begin(OperationKind::Update).unwrap(); + let stale = first.token().clone(); + drop(first); + + let second = manager.begin(OperationKind::Install).unwrap(); + let calls = AtomicUsize::new(0); + assert!(!manager.request_cancellation(&stale, || { + calls.fetch_add(1, Ordering::SeqCst); + true + })); + assert!(!manager.request_pause(&stale, || { + calls.fetch_add(1, Ordering::SeqCst); + true + })); + assert_eq!(calls.load(Ordering::SeqCst), 0); + assert!(!manager.snapshot().unwrap().paused); + + assert!(manager.request_pause(second.token(), || { + calls.fetch_add(1, Ordering::SeqCst); + true + })); + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert!(manager.snapshot().unwrap().paused); + + drop(second); + let _ = fs::remove_dir_all(path.parent().unwrap()); + } + + #[test] + fn confirmed_quit_preparation_linearizes_before_commit_transition() { + use crate::app::op_phase::{OperationPhase, QuitPolicy}; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::{Arc, Barrier}; + + let path = lock_path("confirmed-quit-linearization"); + let manager = OperationManager::new(path.clone()); + let guard = manager.begin(OperationKind::Install).unwrap(); + manager + .set_phase(guard.token(), OperationPhase::Verifying) + .unwrap(); + + let attempted = Arc::new(AtomicBool::new(false)); + let completed = Arc::new(AtomicBool::new(false)); + let prepared = Arc::new(AtomicBool::new(false)); + let start = Arc::new(Barrier::new(2)); + let worker_manager = manager.clone(); + let worker_token = guard.token().clone(); + let worker_attempted = Arc::clone(&attempted); + let worker_completed = Arc::clone(&completed); + let worker_start = Arc::clone(&start); + let worker = std::thread::spawn(move || { + worker_start.wait(); + worker_attempted.store(true, Ordering::SeqCst); + worker_manager + .set_phase(&worker_token, OperationPhase::Committing) + .unwrap(); + worker_completed.store(true, Ordering::SeqCst); + }); + + let prepared_for_quit = Arc::clone(&prepared); + let policy = manager.prepare_quit(true, true, || { + start.wait(); + while !attempted.load(Ordering::SeqCst) { + std::thread::yield_now(); + } + assert!( + !completed.load(Ordering::SeqCst), + "phase transition must stay behind confirmed-quit preparation" + ); + prepared_for_quit.store(true, Ordering::SeqCst); + }); + assert_eq!(policy, QuitPolicy::Confirm); + assert!(prepared.load(Ordering::SeqCst)); + worker.join().unwrap(); + assert!(completed.load(Ordering::SeqCst)); + + let blocked_preparation = AtomicBool::new(false); + let blocked = manager.prepare_quit(true, true, || { + blocked_preparation.store(true, Ordering::SeqCst); + }); + assert!(matches!(blocked, QuitPolicy::Block { .. })); + assert!(!blocked_preparation.load(Ordering::SeqCst)); + + let needs_confirmation = AtomicBool::new(false); + manager + .set_phase(guard.token(), OperationPhase::Verifying) + .unwrap(); + let confirm = manager.prepare_quit(true, false, || { + needs_confirmation.store(true, Ordering::SeqCst); + }); + assert_eq!(confirm, QuitPolicy::Confirm); + assert!(!needs_confirmation.load(Ordering::SeqCst)); + + let automatic_preparation = AtomicBool::new(false); + let allow = manager.prepare_quit(false, false, || { + automatic_preparation.store(true, Ordering::SeqCst); + }); + assert_eq!(allow, QuitPolicy::Allow); + assert!(automatic_preparation.load(Ordering::SeqCst)); + + drop(guard); + let _ = fs::remove_dir_all(path.parent().unwrap()); + } + + #[test] + fn quit_abandons_unclaimed_destructive_token_or_commit_claim_blocks_quit() { + use crate::app::op_phase::{OperationPhase, QuitPolicy}; + use std::sync::atomic::{AtomicBool, Ordering}; + + let path = lock_path("quit-vs-destructive-claim"); + let manager = OperationManager::new(path.clone()); + + // Quit wins: the still-unclaimed token is removed, so its delayed command + // cannot validate and begin destructive work after force-exit preparation. + let abandoned = manager.begin_detached(OperationKind::Uninstall).unwrap(); + let prepared = AtomicBool::new(false); + assert_eq!( + manager.prepare_quit(false, false, || prepared.store(true, Ordering::SeqCst)), + QuitPolicy::Allow + ); + assert!(prepared.load(Ordering::SeqCst)); + assert!(matches!( + manager.validate_with_phase(&abandoned, OperationPhase::Committing), + Err(OperationError::InvalidToken) + )); + assert!(!manager.is_busy()); + + // Destructive claim wins: validation and Committing are atomic, so quit + // observes the point of no return and does not run its preparation closure. + let committed = manager.begin_detached(OperationKind::Uninstall).unwrap(); + manager + .validate_with_phase(&committed, OperationPhase::Committing) + .unwrap(); + let should_not_prepare = AtomicBool::new(false); + let blocked = manager.prepare_quit(false, false, || { + should_not_prepare.store(true, Ordering::SeqCst); + }); + assert!(matches!(blocked, QuitPolicy::Block { .. })); + assert!(!should_not_prepare.load(Ordering::SeqCst)); + manager.end(committed).unwrap(); + } + #[test] fn terminal_completion_uses_mutation_evidence_independently_of_quit_phase() { use crate::app::op_phase::OperationPhase; diff --git a/src-tauri/src/app/win_update.rs b/src-tauri/src/app/win_update.rs index d90c66e..627a6eb 100644 --- a/src-tauri/src/app/win_update.rs +++ b/src-tauri/src/app/win_update.rs @@ -7,7 +7,7 @@ //! verification into staging. Non-destructive; it does not install yet. use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use serde::{Deserialize, Serialize}; @@ -581,7 +581,7 @@ pub fn stage_windows_update_with_install_mode_and_network( // (Nesting under perform's guard can't lose a reachable cancel: once the // download completes the UI is in the "finishing" state with cancel disabled, // so no cancel lands during stage's post-download verify.) - let _abort_guard = WinAbortGuard; + let _abort_guard = WinAbortGuard::new(); let report = plan_windows_update_with_install_mode_and_network( endpoints, settings, @@ -909,7 +909,7 @@ pub fn auto_stage_windows_update_with_install_mode_and_network( /// cancel flag can't reach. Reset on op end via `WinAbortGuard`, not at entry. static WIN_UPDATE_ABORT: AtomicBool = AtomicBool::new(false); -fn clear_win_update_abort() { +pub(crate) fn clear_win_update_abort() { WIN_UPDATE_ABORT.store(false, Ordering::SeqCst); } @@ -917,16 +917,30 @@ fn clear_win_update_abort() { /// DROP (not at entry) keeps the cancel race-free: a cancel landing between the /// UI showing its button and the op reaching its first checkpoint isn't wiped, so /// the checkpoint observes it; the next op still starts clean. The cancel command -/// doesn't hold the op lock, so this startup window is real. Owned by both +/// now validates the owning token under the op lock, but the worker can still be +/// between entry and its first checkpoint. Owned by both /// `perform` and `stage` (so background `auto_stage` and the standalone -/// `win_stage_update` can't leak a set latch into the next op). The perform→stage -/// nesting is harmless: clears are idempotent, and the only window the inner clear -/// could touch (stage's post-download verify) has the UI cancel already disabled. +/// `win_stage_update` can't leak a set latch into the next op). Nested guards are +/// reference-counted: an inner stage return cannot clear a quit/cancel owned by +/// the still-running outer perform operation. struct WinAbortGuard; +static WIN_ABORT_GUARD_DEPTH: AtomicUsize = AtomicUsize::new(0); + +impl WinAbortGuard { + fn new() -> Self { + WIN_ABORT_GUARD_DEPTH.fetch_add(1, Ordering::SeqCst); + Self + } +} + impl Drop for WinAbortGuard { fn drop(&mut self) { - clear_win_update_abort(); + let previous = WIN_ABORT_GUARD_DEPTH.fetch_sub(1, Ordering::SeqCst); + debug_assert!(previous > 0, "WinAbortGuard depth underflow"); + if previous == 1 { + clear_win_update_abort(); + } } } @@ -1048,7 +1062,7 @@ pub fn perform_windows_update_with_install_mode_network_and_phase( // Reset the latch when THIS perform ends (not at stage entry) so a cancel // racing the op's startup isn't wiped, and `auto_stage` never clears it. See // WinAbortGuard. - let _abort_guard = WinAbortGuard; + let _abort_guard = WinAbortGuard::new(); set_phase(OperationPhase::Preparing); if !confirm { return Err(AppError::Internal( @@ -1098,12 +1112,14 @@ pub fn perform_windows_update_with_install_mode_network_and_phase( }); } - // Honor a cancel one last time before closing Codex or beginning install - // preparation. A fully-cached MSIX can skip the download loop (so its cancel - // flag never arms) yet still reach here. Keep the operation pre-commit until - // the selected route actually mutates the installed application. + // Point of no return. Honor a cancel one last time BEFORE closing Codex or + // sideloading — closes the gap after staging where a fully-cached MSIX skips + // the download loop (so its cancel flag never arms) yet still reaches here. + // Completion classification remains independent from this quit phase and + // changes only when the evidence hook records a real or ambiguous mutation. + set_phase(OperationPhase::Committing); + // Linearized with every native/window quit path by OperationManager. check_win_update_abort()?; - set_phase(OperationPhase::Applying); if stage.route == "portable-fallback" { log::warn!("Windows route changed to portable fallback from_route=msix-sideload to_route=portable-fallback"); @@ -1962,17 +1978,27 @@ mod tests { #[test] fn win_abort_guard_preserves_a_startup_race_cancel_and_resets_on_drop() { + let _serial = crate::app::oplock::CANCEL_LATCH_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); // Mirrors the macOS guard test: a cancel landing before `perform` reaches // its first checkpoint must survive the guard's creation (no entry-clear) // and still be observed; the guard resets the latch on drop so the next // op — and background auto_stage — start clean. WIN_UPDATE_ABORT.store(true, Ordering::SeqCst); { - let _guard = WinAbortGuard; + let outer = WinAbortGuard::new(); + let inner = WinAbortGuard::new(); assert!( check_win_update_abort().is_err(), "guard creation must not wipe a pending cancel" ); + drop(inner); + assert!( + check_win_update_abort().is_err(), + "a nested guard must not clear the owning operation's cancel" + ); + drop(outer); } assert!( check_win_update_abort().is_ok(), diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 5fa3c43..ad16d40 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -14,8 +14,9 @@ use crate::app::disk::available_space; use crate::app::logging::redact_url; use crate::app::mac_update::{ cancel_macos_download, detect_existing_install_at_path as detect_macos_install_at_path, - discard_macos_download, install_macos_with_network, mac_adopt_path as adopt_macos_path, - pause_macos_download, perform_macos_update_with_network_and_phase, + discard_macos_download, install_macos_with_network_and_phase, + mac_adopt_path as adopt_macos_path, pause_macos_download, + perform_macos_update_with_network_and_phase, plan_macos_update_with_network, retry_macos_ancillary, stage_macos_update_with_network, uninstall_macos, InstalledCodex, MacInstallStatus, MacPerformReport, MacStageReport, MacUninstallReport, MacUpdateReport, PerformExpectation, @@ -320,6 +321,23 @@ impl DetachedGuard { }) } + fn validate_with_phase( + state: &ManagerState, + token: OperationToken, + phase: OperationPhase, + ) -> Result { + let operations = state.operations.clone(); + operations + .validate_with_phase(&token, phase) + .map_err(destructive_token_error)?; + Ok(Self { + completion_tracking: false, + operations, + succeeded: false, + token: Some(token), + }) + } + fn mark_succeeded(&mut self) { self.succeeded = true; } @@ -831,6 +849,8 @@ pub async fn mac_install( let network = mac_network_config_for_settings()?; let progress_token = token.clone(); let progress_ops = ops.clone(); + let phase_token = token.clone(); + let phase_ops = ops.clone(); tauri::async_runtime::spawn_blocking(move || { let report = move |p: crate::app::mac_update::DownloadProgress| { emit_op_download_progress( @@ -843,7 +863,10 @@ pub async fn mac_install( p.source, ); }; - install_macos_with_network(&report, &network) + let phase_hook = |phase: OperationPhase| { + let _ = phase_ops.set_phase(&phase_token, phase); + }; + install_macos_with_network_and_phase(&report, &network, Some(&phase_hook)) }) .await .map_err(|e| AppError::Internal(format!("join: {e}")))? @@ -853,26 +876,31 @@ pub async fn mac_install( /// macOS-only: request pausing an active package download. /// Partial bytes are left in place for the next resume-capable run. #[tauri::command] -pub fn mac_pause_download(state: State<'_, ManagerState>) -> Result { +pub fn mac_pause_download( + state: State<'_, ManagerState>, + operation_id: String, +) -> Result { if !cfg!(target_os = "macos") { return Err(AppError::UnsupportedPlatform.into()); } - if let Some(snap) = state.operations.snapshot() { - let _ = state - .operations - .set_paused(&OperationToken(snap.id), true); - } - Ok(pause_macos_download()) + Ok(state + .operations + .request_pause(&OperationToken(operation_id), pause_macos_download)) } /// macOS-only: request cancellation of an active package download. /// Partial bytes are discarded. #[tauri::command] -pub fn mac_cancel_download() -> Result { +pub fn mac_cancel_download( + state: State<'_, ManagerState>, + operation_id: String, +) -> Result { if !cfg!(target_os = "macos") { return Err(AppError::UnsupportedPlatform.into()); } - Ok(cancel_macos_download()) + Ok(state + .operations + .request_cancellation(&OperationToken(operation_id), cancel_macos_download)) } /// macOS-only: discard a PAUSED download. After a pause the curl process is gone @@ -1115,7 +1143,9 @@ pub fn retry_ancillary( "清除用户数据需要破坏性令牌（先 arm_destructive uninstall）".to_string(), ) })?; - RetryGuard::Detached(DetachedGuard::validate(&state, token)?) + let guard = + DetachedGuard::validate_with_phase(&state, token, OperationPhase::Committing)?; + RetryGuard::Detached(guard) } else { RetryGuard::Scoped(begin_guard(&state, OperationKind::Adopt)?) }; @@ -1210,9 +1240,17 @@ pub fn get_operation_completion( #[tauri::command] pub fn confirm_quit(app: tauri::AppHandle, state: State<'_, ManagerState>) -> Result<(), CommandError> { let confirm_close = crate::app::settings_store::AppSettings::load().confirm_close; - // Evaluate as if force_quit is not yet set so a point-of-no-return phase - // still blocks even after the user clicks the confirm dialog. - let policy = state.operations.quit_policy(false, confirm_close); + // Decide and arm exit under the SAME operation mutex used by phase changes. + // If the worker already reached commit this returns Block. Otherwise both app + // abort latches and force_quit are set before it can cross that boundary, so + // the final pre-commit checkpoint observes the cancellation. + let policy = state.operations.prepare_quit(confirm_close, true, || { + let _ = cancel_macos_download(); + let _ = cancel_windows_download(); + state + .force_quit + .store(true, std::sync::atomic::Ordering::SeqCst); + }); if let QuitPolicy::Block { phase, reason_code, @@ -1228,12 +1266,6 @@ pub fn confirm_quit(app: tauri::AppHandle, state: State<'_, ManagerState>) -> Re let _ = app.emit("app://quit-blocked", &policy); return Err(AppError::Busy(reason.clone()).into()); } - // Interruptible phases: best-effort cancel so partial downloads settle cleanly. - let _ = codex_mac_engine::cancel_active_download(); - let _ = codex_win_engine::cancel_active_download(); - state - .force_quit - .store(true, std::sync::atomic::Ordering::SeqCst); app.exit(0); Ok(()) } @@ -1380,7 +1412,10 @@ pub async fn mac_uninstall( if !confirm { return Err(AppError::Internal("拒绝执行：卸载必须带显式 confirm".to_string()).into()); } - let _op = DetachedGuard::validate(&state, token)?; + let _op = DetachedGuard::validate_with_phase(&state, token, OperationPhase::Committing)?; + // Uninstall has no resumable cancellation protocol. Treat the whole worker + // as point-of-no-return so every native/window quit path blocks until the + // removal and ancillary bookkeeping have settled. tauri::async_runtime::spawn_blocking(move || uninstall_macos(keep_codex_home)) .await .map_err(|e| AppError::Internal(format!("join: {e}")))? @@ -1431,26 +1466,31 @@ pub async fn win_auto_stage_update( /// Windows-only: request pausing an active background/manual download. /// Partial bytes are left in place for the next resume-capable staging run. #[tauri::command] -pub fn win_pause_download(state: State<'_, ManagerState>) -> Result { +pub fn win_pause_download( + state: State<'_, ManagerState>, + operation_id: String, +) -> Result { if !matches!(state.target.os, OperatingSystem::Windows) { return Err(AppError::UnsupportedPlatform.into()); } - if let Some(snap) = state.operations.snapshot() { - let _ = state - .operations - .set_paused(&OperationToken(snap.id), true); - } - Ok(pause_windows_download()) + Ok(state + .operations + .request_pause(&OperationToken(operation_id), pause_windows_download)) } /// Windows-only: request cancellation of an active background/manual download. /// Partial bytes are discarded. #[tauri::command] -pub fn win_cancel_download(state: State<'_, ManagerState>) -> Result { +pub fn win_cancel_download( + state: State<'_, ManagerState>, + operation_id: String, +) -> Result { if !matches!(state.target.os, OperatingSystem::Windows) { return Err(AppError::UnsupportedPlatform.into()); } - Ok(cancel_windows_download()) + Ok(state + .operations + .request_cancellation(&OperationToken(operation_id), cancel_windows_download)) } /// Windows-only: discard a PAUSED download. Drops the cached `.part` left for @@ -1857,7 +1897,8 @@ pub async fn win_uninstall( AppError::Internal("拒绝执行：Windows 卸载必须带显式 confirm".to_string()).into(), ); } - let _op = DetachedGuard::validate(&state, token)?; + let _op = DetachedGuard::validate_with_phase(&state, token, OperationPhase::Committing)?; + // Remove-AppxPackage / portable tree removal must not be killed mid-call. let settings = windows_domain_settings_for_persisted(&state); tauri::async_runtime::spawn_blocking(move || { uninstall_windows_codex(&settings, confirm, purge_user_data) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 0198ff9..4dc015b 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -22,9 +22,18 @@ fn confirm_close_enabled() -> bool { fn quit_policy_for(app: &tauri::AppHandle) -> QuitPolicy { let state = app.state::(); let force = state.force_quit.load(Ordering::SeqCst); + if force { + return QuitPolicy::Allow; + } state .operations - .quit_policy(force, confirm_close_enabled()) + .prepare_quit(confirm_close_enabled(), false, || { + // Arm both platform latches while the operation phase mutex is still + // held. Only the active platform has work; the other store is harmless. + let _ = crate::app::mac_update::cancel_macos_download(); + let _ = crate::app::win_update::cancel_windows_download(); + state.force_quit.store(true, Ordering::SeqCst); + }) } /// Apply a quit policy decision for window/menu/exit paths. diff --git a/src/app/errorCopy.ts b/src/app/errorCopy.ts index 0cd0f69..754f320 100644 --- a/src/app/errorCopy.ts +++ b/src/app/errorCopy.ts @@ -84,6 +84,22 @@ export function messageFailure( return { code, message, detail: null, recoverable }; } +/** + * Add action-specific recovery copy without throwing away the backend / bridge + * diagnostic. The contextual code is intentionally stable so callers can + * alter controls for a known state without parsing localized prose. + */ +export function contextualFailure( + cause: unknown, + t: TFn, + message: string, + code: string, + recoverable = true, +): FailureSurface { + const failure = resolveFailure(cause, t); + return { ...failure, code, message, recoverable }; +} + /** True when the failure is a connectivity class (DNS / TLS / timeout). */ export function isConnectivityFailure(cause: unknown): boolean { const code = errorCode(cause); diff --git a/src/app/i18n.tsx b/src/app/i18n.tsx index c6a79b3..424426e 100644 --- a/src/app/i18n.tsx +++ b/src/app/i18n.tsx @@ -128,6 +128,10 @@ const ZH = { "progress.cancelPending": "正在取消…", "progress.cancelled": "下载已取消。", "progress.cannotCancel": "当前阶段不能取消。", + "progress.stopRejected": "{action}请求被后端拒绝。任务仍在继续，可重试。", + "progress.stopDeliveryFailed": "{action}请求未送达。任务仍在继续，可重试。", + "progress.stopUninterruptible": "任务已进入不可中断的安装阶段，请等待完成。", + "progress.discardFailed": "取消未完成。下载仍处于暂停状态；你可以继续下载或重试取消。", "progress.resume": "继续", "progress.paused.title": "下载已暂停", "progress.finishing": "下载完成,正在安装,请勿关闭", @@ -411,6 +415,10 @@ const EN: Record = { "progress.cancelPending": "Cancelling…", "progress.cancelled": "Download cancelled.", "progress.cannotCancel": "This phase cannot be cancelled.", + "progress.stopRejected": "The backend rejected the {action} request. The task is still running; try again.", + "progress.stopDeliveryFailed": "The {action} request was not delivered. The task is still running; try again.", + "progress.stopUninterruptible": "The task has entered an uninterruptible install phase. Wait for it to finish.", + "progress.discardFailed": "Cancellation did not complete. The download remains paused; resume it or retry cancellation.", "progress.resume": "Resume", "progress.paused.title": "Download paused", "progress.finishing": "Download complete — installing, don't close", @@ -691,6 +699,10 @@ const FR: Record = { "progress.cancelPending": "Annulation…", "progress.cancelled": "Téléchargement annulé.", "progress.cannotCancel": "Cette étape ne peut pas être annulée.", + "progress.stopRejected": "Le moteur a refusé la demande « {action} ». La tâche continue ; réessayez.", + "progress.stopDeliveryFailed": "La demande « {action} » n’a pas été transmise. La tâche continue ; réessayez.", + "progress.stopUninterruptible": "La tâche a atteint une phase d’installation impossible à interrompre. Attendez la fin.", + "progress.discardFailed": "L’annulation n’a pas abouti. Le téléchargement reste en pause ; reprenez-le ou réessayez.", "progress.resume": "Reprendre", "progress.paused.title": "Téléchargement en pause", "progress.finishing": "Téléchargement terminé — installation, ne pas fermer", @@ -972,6 +984,10 @@ const ZH_TW: Record = { "progress.cancelPending": "正在取消…", "progress.cancelled": "下載已取消。", "progress.cannotCancel": "目前階段不能取消。", + "progress.stopRejected": "後端拒絕了{action}要求。工作仍在繼續，可重試。", + "progress.stopDeliveryFailed": "{action}要求未送達。工作仍在繼續，可重試。", + "progress.stopUninterruptible": "工作已進入無法中斷的安裝階段，請等待完成。", + "progress.discardFailed": "取消未完成。下載仍處於暫停狀態；你可以繼續下載或重試取消。", "progress.resume": "繼續", "progress.paused.title": "下載已暫停", "progress.finishing": "下載完成,正在安裝,請勿關閉", @@ -1263,6 +1279,10 @@ const DE: Record = { "progress.cancelPending": "Abbrechen…", "progress.cancelled": "Download abgebrochen.", "progress.cannotCancel": "Diese Phase kann nicht abgebrochen werden.", + "progress.stopRejected": "Das Backend hat die Anfrage „{action}“ abgelehnt. Der Vorgang läuft weiter; versuchen Sie es erneut.", + "progress.stopDeliveryFailed": "Die Anfrage „{action}“ wurde nicht übermittelt. Der Vorgang läuft weiter; versuchen Sie es erneut.", + "progress.stopUninterruptible": "Der Vorgang befindet sich in einer nicht unterbrechbaren Installationsphase. Warten Sie, bis sie abgeschlossen ist.", + "progress.discardFailed": "Der Abbruch wurde nicht abgeschlossen. Der Download bleibt pausiert; setzen Sie ihn fort oder versuchen Sie den Abbruch erneut.", "progress.resume": "Fortsetzen", "progress.paused.title": "Download pausiert", "progress.finishing": "Download abgeschlossen – wird installiert, nicht schließen", @@ -1554,6 +1574,10 @@ const KO: Record = { "progress.cancelPending": "취소 중…", "progress.cancelled": "다운로드가 취소되었습니다.", "progress.cannotCancel": "이 단계는 취소할 수 없습니다.", + "progress.stopRejected": "백엔드가 {action} 요청을 거부했습니다. 작업은 계속 진행 중이며 다시 시도할 수 있습니다.", + "progress.stopDeliveryFailed": "{action} 요청이 전달되지 않았습니다. 작업은 계속 진행 중이며 다시 시도할 수 있습니다.", + "progress.stopUninterruptible": "작업이 중단할 수 없는 설치 단계에 들어갔습니다. 완료될 때까지 기다려 주세요.", + "progress.discardFailed": "취소가 완료되지 않았습니다. 다운로드는 일시 중지 상태입니다. 계속하거나 취소를 다시 시도하세요.", "progress.resume": "계속", "progress.paused.title": "다운로드 일시 중지됨", "progress.finishing": "다운로드 완료 — 설치 중, 닫지 마세요", @@ -1837,6 +1861,10 @@ const JA: Record = { "progress.cancelPending": "キャンセル中…", "progress.cancelled": "ダウンロードをキャンセルしました。", "progress.cannotCancel": "この段階はキャンセルできません。", + "progress.stopRejected": "バックエンドが{action}要求を拒否しました。処理は続行中です。再試行できます。", + "progress.stopDeliveryFailed": "{action}要求を送信できませんでした。処理は続行中です。再試行できます。", + "progress.stopUninterruptible": "処理は中断できないインストール段階に入りました。完了までお待ちください。", + "progress.discardFailed": "キャンセルが完了しませんでした。ダウンロードは一時停止したままです。再開するか、キャンセルを再試行してください。", "progress.resume": "再開", "progress.paused.title": "ダウンロードを一時停止しました", "progress.finishing": "ダウンロード完了 — インストール中です。閉じないでください", @@ -2108,6 +2136,10 @@ const RU: Record = { "progress.cancelPending": "Отмена…", "progress.cancelled": "Загрузка отменена.", "progress.cannotCancel": "Этот этап нельзя отменить.", + "progress.stopRejected": "Серверная часть отклонила запрос «{action}». Операция продолжается; повторите попытку.", + "progress.stopDeliveryFailed": "Запрос «{action}» не был доставлен. Операция продолжается; повторите попытку.", + "progress.stopUninterruptible": "Операция перешла к непрерываемому этапу установки. Дождитесь завершения.", + "progress.discardFailed": "Отмена не завершена. Загрузка остаётся на паузе; продолжите её или повторите отмену.", "progress.resume": "Продолжить", "progress.paused.title": "Загрузка приостановлена", "progress.finishing": "Загрузка завершена — установка, не закрывайте", @@ -2379,6 +2411,10 @@ const AR: Record = { "progress.cancelPending": "جارٍ الإلغاء…", "progress.cancelled": "تم إلغاء التنزيل.", "progress.cannotCancel": "لا يمكن إلغاء هذه المرحلة.", + "progress.stopRejected": "رفضت الخدمة طلب {action}. لا تزال المهمة قيد التنفيذ؛ حاول مجددًا.", + "progress.stopDeliveryFailed": "لم يتم تسليم طلب {action}. لا تزال المهمة قيد التنفيذ؛ حاول مجددًا.", + "progress.stopUninterruptible": "دخلت المهمة مرحلة تثبيت لا يمكن مقاطعتها. انتظر حتى تكتمل.", + "progress.discardFailed": "لم يكتمل الإلغاء. لا يزال التنزيل متوقفًا مؤقتًا؛ تابعه أو أعد محاولة الإلغاء.", "progress.resume": "استئناف", "progress.paused.title": "تم إيقاف التنزيل مؤقتًا", "progress.finishing": "اكتمل التنزيل — جارٍ التثبيت، لا تغلق النافذة", @@ -2650,6 +2686,10 @@ const ES: Record = { "progress.cancelPending": "Cancelando…", "progress.cancelled": "Descarga cancelada.", "progress.cannotCancel": "Esta fase no se puede cancelar.", + "progress.stopRejected": "El motor rechazó la solicitud «{action}». La tarea sigue en curso; vuelve a intentarlo.", + "progress.stopDeliveryFailed": "La solicitud «{action}» no se entregó. La tarea sigue en curso; vuelve a intentarlo.", + "progress.stopUninterruptible": "La tarea ha entrado en una fase de instalación que no se puede interrumpir. Espera a que termine.", + "progress.discardFailed": "La cancelación no se completó. La descarga sigue en pausa; reanúdala o vuelve a cancelar.", "progress.resume": "Reanudar", "progress.paused.title": "Descarga en pausa", "progress.finishing": "Descarga completa — instalando, no cierres", @@ -2921,6 +2961,10 @@ const PT_BR: Record = { "progress.cancelPending": "Cancelando…", "progress.cancelled": "Download cancelado.", "progress.cannotCancel": "Esta fase não pode ser cancelada.", + "progress.stopRejected": "O mecanismo recusou a solicitação “{action}”. A tarefa continua em execução; tente novamente.", + "progress.stopDeliveryFailed": "A solicitação “{action}” não foi entregue. A tarefa continua em execução; tente novamente.", + "progress.stopUninterruptible": "A tarefa entrou em uma fase de instalação que não pode ser interrompida. Aguarde a conclusão.", + "progress.discardFailed": "O cancelamento não foi concluído. O download permanece pausado; retome-o ou tente cancelar novamente.", "progress.resume": "Retomar", "progress.paused.title": "Download pausado", "progress.finishing": "Download concluído — instalando, não feche", diff --git a/src/app/views/Home.test.tsx b/src/app/views/Home.test.tsx index c988105..338bbce 100644 --- a/src/app/views/Home.test.tsx +++ b/src/app/views/Home.test.tsx @@ -12,6 +12,7 @@ import type { MacInstallStatus, MacPerformReport, MacUpdateReport, + OperationSnapshot, UpdatePlan, } from "../../shared/types"; import { DEFAULT_SETTINGS } from "../../shared/types"; @@ -83,6 +84,15 @@ const REPORT_UPTODATE: MacUpdateReport = { const STATUS_MANAGED: MacInstallStatus = { installed: INSTALLED, status: "managed" }; const STATUS_NONE: MacInstallStatus = { installed: null, status: "none" }; +const ACTIVE_OPERATION: OperationSnapshot = { + id: "op-active", + kind: "update", + phase: "downloading", + progress: { downloaded: 10, total: 100, source: "mirror.example" }, + paused: false, + cancellable: true, + interruptible: true, +}; const PERFORM_OK: MacPerformReport = { upToDate: false, @@ -127,6 +137,7 @@ describe("MacHome state machine", () => { api.macPauseDownload.mockResolvedValue(true); api.macCancelDownload.mockResolvedValue(true); api.macDiscardDownload.mockResolvedValue(undefined); + api.getOperationSnapshot.mockResolvedValue(null); }); it("offers install when nothing is detected", async () => { @@ -245,7 +256,14 @@ describe("MacHome state machine", () => { // Bytes arrive → the pause button becomes actionable. await waitFor(() => expect(onProgress).toBeDefined()); act(() => { - onProgress?.({ payload: { downloaded: 512, total: 1024, source: "mirror.example" } }); + onProgress?.({ + payload: { + downloaded: 512, + total: 1024, + source: "mirror.example", + operationId: "op-active", + }, + }); }); // The progress bar exposes progressbar semantics to assistive tech. The @@ -260,6 +278,7 @@ describe("MacHome state machine", () => { await waitFor(() => expect(pause).toBeEnabled()); await user.click(pause); await waitFor(() => expect(api.macPauseDownload).toHaveBeenCalledTimes(1)); + expect(api.macPauseDownload).toHaveBeenCalledWith("op-active"); // The backend acknowledges the pause by failing the in-flight perform. act(() => rejectPerform?.(new Error("download cancelled"))); @@ -287,7 +306,11 @@ describe("MacHome state machine", () => { renderHome(); await user.click(await screen.findByRole("button", { name: /立即更新/ })); await waitFor(() => expect(onProgress).toBeDefined()); - act(() => onProgress?.({ payload: { downloaded: 10, total: 100, source: "s" } })); + act(() => + onProgress?.({ + payload: { downloaded: 10, total: 100, source: "s", operationId: "op-active" }, + }), + ); await user.click(await screen.findByRole("button", { name: /^暂停$/ })); act(() => rejectPerform?.(new Error("download cancelled"))); await screen.findByText("下载已暂停"); @@ -296,4 +319,101 @@ describe("MacHome state machine", () => { await waitFor(() => expect(api.macDiscardDownload).toHaveBeenCalledTimes(1)); expect(await screen.findByText("下载已取消。")).toBeInTheDocument(); }); + + it.each([ + { intent: "pause" as const, outcome: "false" as const }, + { intent: "pause" as const, outcome: "reject" as const }, + { intent: "cancel" as const, outcome: "false" as const }, + { intent: "cancel" as const, outcome: "reject" as const }, + ])( + "keeps the macOS progress flow recoverable when $intent returns $outcome", + async ({ intent, outcome }) => { + const user = userEvent.setup(); + api.getSettings.mockResolvedValue(settings({ askBefore: false })); + api.macPerformUpdate.mockImplementationOnce(() => new Promise(() => {})); + + let onProgress: ((event: { payload: DownloadProgress }) => void) | undefined; + listenMock.mockImplementation((event: string, cb: unknown) => { + if (event === "mac://download-progress") onProgress = cb as typeof onProgress; + return Promise.resolve(() => {}); + }); + + const stop = intent === "pause" ? api.macPauseDownload : api.macCancelDownload; + if (outcome === "false") { + stop.mockResolvedValue(false); + } else { + stop.mockRejectedValue(new Error("invoke bridge unavailable")); + } + + renderHome(); + await user.click(await screen.findByRole("button", { name: /立即更新/ })); + if (intent === "pause") { + await waitFor(() => expect(onProgress).toBeDefined()); + act(() => + onProgress?.({ + payload: { downloaded: 10, total: 100, source: "mirror.example" }, + }), + ); + } + + const action = intent === "pause" ? "暂停" : "取消"; + const button = await screen.findByRole("button", { name: action }); + await waitFor(() => expect(button).toBeEnabled()); + api.getOperationSnapshot.mockResolvedValue(ACTIVE_OPERATION); + await user.click(button); + + const expected = + outcome === "false" + ? `${action}请求被后端拒绝。任务仍在继续，可重试。` + : `${action}请求未送达。任务仍在继续，可重试。`; + expect(await screen.findByRole("alert")).toHaveTextContent(expected); + expect(screen.getByText("正在更新…")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: action })).toBeEnabled(); + + // The same control becomes actionable again rather than staying pending. + await user.click(screen.getByRole("button", { name: action })); + await waitFor(() => expect(stop).toHaveBeenCalledTimes(2)); + }, + ); + + it("keeps the macOS paused screen and both recovery actions when discard rejects", async () => { + const user = userEvent.setup(); + api.getSettings.mockResolvedValue(settings({ askBefore: false })); + api.macDiscardDownload.mockRejectedValueOnce(new Error("cache locked")); + + let rejectPerform: ((cause: unknown) => void) | undefined; + let onProgress: ((event: { payload: DownloadProgress }) => void) | undefined; + listenMock.mockImplementation((event: string, cb: unknown) => { + if (event === "mac://download-progress") onProgress = cb as typeof onProgress; + return Promise.resolve(() => {}); + }); + api.macPerformUpdate.mockImplementationOnce( + () => new Promise((_resolve, reject) => (rejectPerform = reject)), + ); + + renderHome(); + await user.click(await screen.findByRole("button", { name: /立即更新/ })); + await waitFor(() => expect(onProgress).toBeDefined()); + act(() => + onProgress?.({ + payload: { downloaded: 10, total: 100, source: "s", operationId: "op-active" }, + }), + ); + await user.click(await screen.findByRole("button", { name: /^暂停$/ })); + act(() => rejectPerform?.(new Error("download cancelled"))); + await screen.findByText("下载已暂停"); + + await user.click(screen.getByRole("button", { name: "取消" })); + expect(await screen.findByRole("alert")).toHaveTextContent( + "取消未完成。下载仍处于暂停状态；你可以继续下载或重试取消。", + ); + expect(screen.getByText("下载已暂停")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "继续" })).toBeEnabled(); + expect(screen.getByRole("button", { name: "取消" })).toBeEnabled(); + + // The failed discard is retryable; only the successful retry leaves pause. + await user.click(screen.getByRole("button", { name: "取消" })); + await waitFor(() => expect(api.macDiscardDownload).toHaveBeenCalledTimes(2)); + expect(await screen.findByText("下载已取消。")).toBeInTheDocument(); + }); }); diff --git a/src/app/views/Home.tsx b/src/app/views/Home.tsx index 71be6e2..13eaae9 100644 --- a/src/app/views/Home.tsx +++ b/src/app/views/Home.tsx @@ -13,7 +13,12 @@ import type { MacUpdateReport, } from "../../shared/types"; import { DEFAULT_SETTINGS } from "../../shared/types"; -import { resolveFailure, userErrorMessage, type FailureSurface } from "../errorCopy"; +import { + contextualFailure, + resolveFailure, + userErrorMessage, + type FailureSurface, +} from "../errorCopy"; import { Icon, CodexGlyph } from "../icons"; import { useI18n, dirOf, type TKey } from "../i18n"; import { Ring, TopBar, ResultBanner, ErrorHero, FailureBanner, StatusBanner } from "../components"; @@ -76,6 +81,8 @@ function MacHome({ onOpenSettings }: { onOpenSettings: () => void }) { // A paused download: the progress screen stays up (not routed home) offering // 〔继续〕/〔取消〕. `dl` is the byte snapshot captured at the moment of pause. const [paused, setPaused] = useState(null); + const [pausedDiscardBusy, setPausedDiscardBusy] = useState(false); + const pausedDiscardBusyRef = useRef(false); const scopeRef = useRef(null); const confirmTitleId = useId(); const confirmBodyId = useId(); @@ -99,9 +106,9 @@ function MacHome({ onOpenSettings }: { onOpenSettings: () => void }) { resetStop, } = useDownloadProgress({ eventName: "mac://download-progress", - pauseDownload: () => managerApi.macPauseDownload(), - cancelDownload: () => managerApi.macCancelDownload(), - cannotCancelMessage: t("progress.cannotCancel"), + pauseDownload: (operationId) => managerApi.macPauseDownload(operationId), + cancelDownload: (operationId) => managerApi.macCancelDownload(operationId), + getOperationSnapshot: () => managerApi.getOperationSnapshot(), onError: setActionError, }); @@ -344,6 +351,7 @@ function MacHome({ onOpenSettings }: { onOpenSettings: () => void }) { // stopped instead of at 0. const resumeDownload = useCallback(() => { const kind = paused?.kind; + setActionError(null); setPaused(null); if (kind === "install") void runInstall(); else void runPerform(); @@ -353,15 +361,29 @@ function MacHome({ onOpenSettings }: { onOpenSettings: () => void }) { // cached partial and route home. (An in-flight cancel is handled by // requestDownloadStop instead.) const cancelPausedDownload = useCallback(async () => { - setPaused(null); + if (pausedDiscardBusyRef.current) return; + pausedDiscardBusyRef.current = true; + setActionError(null); + setPausedDiscardBusy(true); try { // Only claim "已取消" once the cached partial is actually gone — otherwise // a failed discard would leave a `.part` that the next update silently // resumes, contradicting the cancel. await managerApi.macDiscardDownload(); + setPaused(null); setNotice(t("progress.cancelled")); } catch (cause) { - setActionError(resolveFailure(cause, t)); + setActionError( + contextualFailure( + cause, + t, + t("progress.discardFailed"), + "paused_discard_failed", + ), + ); + } finally { + pausedDiscardBusyRef.current = false; + setPausedDiscardBusy(false); } }, [t]); @@ -553,7 +575,8 @@ function MacHome({ onOpenSettings }: { onOpenSettings: () => void }) { dlSpeed={dlSpeed} installing={paused ? paused.kind === "install" : busy === "install"} downloadStop={downloadStop} - downloadStopBusy={downloadStopBusy} + downloadStopBusy={downloadStopBusy || pausedDiscardBusy} + failure={actionError} onResume={resumeDownload} onPause={() => void requestDownloadStop("pause")} onCancel={() => { diff --git a/src/app/views/ProgressScreen.tsx b/src/app/views/ProgressScreen.tsx index 4ded0f7..38d098c 100644 --- a/src/app/views/ProgressScreen.tsx +++ b/src/app/views/ProgressScreen.tsx @@ -1,9 +1,10 @@ import type { RefObject } from "react"; import type { DownloadProgress } from "../../shared/types"; +import type { FailureSurface } from "../errorCopy"; import { Icon } from "../icons"; import { useI18n } from "../i18n"; -import { Ring, TopBar } from "../components"; +import { FailureBanner, Ring, TopBar } from "../components"; import { mib } from "../format"; export type DownloadStopIntent = "pause" | "cancel"; @@ -29,6 +30,7 @@ export function ProgressScreen({ installing, downloadStop, downloadStopBusy, + failure, onResume, onPause, onCancel, @@ -46,6 +48,7 @@ export function ProgressScreen({ installing: boolean; downloadStop: DownloadStopIntent | null; downloadStopBusy: boolean; + failure: FailureSurface | null; onResume: () => void; onPause: () => void; onCancel: () => void; @@ -62,12 +65,16 @@ export function ProgressScreen({ // mac, sideload/extract on Windows). Say so and drop the dead buttons rather // than leave them greyed for no visible reason. const finishing = !paused && Boolean(snap && snap.total > 0 && snap.downloaded >= snap.total); + const uninterruptible = failure?.code === "download_stop_uninterruptible"; // Pause only makes sense mid-transfer; cancel is the "abandon" out and works // through the preparing phase too (a backend abort checkpoint honors it), but // not once the install has begun. const canPause = - !paused && Boolean(dl && dl.total > 0 && dl.downloaded < dl.total) && !downloadStopBusy; - const canCancel = !paused && !finishing && !downloadStopBusy; + !paused && + !uninterruptible && + Boolean(dl && dl.total > 0 && dl.downloaded < dl.total) && + !downloadStopBusy; + const canCancel = !paused && !finishing && !uninterruptible && !downloadStopBusy; const phase = paused ? t("progress.paused.title") @@ -81,6 +88,7 @@ export function ProgressScreen({
+ {failure ? : null}
@@ -123,7 +131,7 @@ export function ProgressScreen({ ) : null}
{paused ? ( - @@ -136,7 +144,7 @@ export function ProgressScreen({