From 8c788d5b8775b53a77a109e37319a94420261e82 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:07:37 +0200 Subject: [PATCH 1/6] docs(session): describe background explicit operation phases --- docs/CONFIG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/CONFIG.md b/docs/CONFIG.md index bfe2fe2f..8a61c9f0 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -2128,6 +2128,12 @@ The overlay Session panel lives in the top strip's overflow **"Session..."** pop - Recent session rows reopen other named sessions. If a recent target is missing, Wayscriber removes that stale catalog entry after the failed open. - `Manager` opens the configurator. Overlay Open/Save As dialogs use `zenity` or `kdialog`. +Open, Save As, and Clear run their disk phases on the persistence worker while +event dispatch continues. One command runs at a time, after any pending autosave +finishes. If you edit while a captured phase is pending, the edits stay live and +the target switch is refused; retry the session operation with the updated +canvas. + The configurator Session tab also shows recent named sessions from the catalog, recorded when named-session targets are opened or saved from the CLI, daemon, or overlay. It can rename catalog display labels, reveal file locations, and forget catalog metadata without touching files. Duplicate, Move, and Clear are disabled while an overlay, manually started daemon, or background service is active. Session overrides and recovery: From c88d568c53747bc2645c79bd31ef7f0fac689a76 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:19:10 +0200 Subject: [PATCH 2/6] refactor(session): advance explicit commands without blocking event dispatch --- .../backend/event_loop/session_save.rs | 34 +- src/backend/wayland/session.rs | 6 +- src/backend/wayland/session/runtime.rs | 289 +---------- .../wayland/session/runtime/transaction.rs | 253 ++++++++++ .../session/runtime/transaction/phases.rs | 328 +++++++++++++ src/backend/wayland/session/tests.rs | 458 +++++++++++++++--- src/backend/wayland/state.rs | 2 + src/backend/wayland/state/core/init.rs | 1 + .../wayland/state/core/output/transition.rs | 10 +- src/backend/wayland/state/core/session.rs | 261 +++++----- .../wayland/state/toolbar/events/session.rs | 176 ++++--- src/input/state/actions/action_dispatch.rs | 2 + src/input/state/actions/key_press/mod.rs | 4 + .../state/actions/key_press/text_input.rs | 1 + src/input/state/core/ime.rs | 1 + src/input/state/core/session.rs | 15 + src/input/state/core/session_flags.rs | 10 + src/input/state/core/toolbar/apply/mod.rs | 2 + src/input/state/mouse/motion.rs | 2 + src/input/state/mouse/press.rs | 2 + src/input/state/mouse/release/mod.rs | 2 + 21 files changed, 1313 insertions(+), 546 deletions(-) create mode 100644 src/backend/wayland/session/runtime/transaction.rs create mode 100644 src/backend/wayland/session/runtime/transaction/phases.rs diff --git a/src/backend/wayland/backend/event_loop/session_save.rs b/src/backend/wayland/backend/event_loop/session_save.rs index 6ae5a744..4a6c6c33 100644 --- a/src/backend/wayland/backend/event_loop/session_save.rs +++ b/src/backend/wayland/backend/event_loop/session_save.rs @@ -37,6 +37,12 @@ pub(super) fn persist_session(state: &mut WaylandState) -> Result<(), anyhow::Er state.session.target_epoch() ); } + if let Err(error) = state.finish_pending_session_command() { + if let Some(transaction) = state.session_transaction.take() { + state.fail_session_command(transaction.command(), &error); + } + log::warn!("Explicit session command failed during shutdown: {error:#}"); + } let save_result = persist_final_session(state); let worker_failed = !state.persistence.is_healthy(); let shutdown_result = state.persistence.shutdown(state.session.target_epoch()); @@ -187,6 +193,11 @@ fn persist_final_session(state: &mut WaylandState) -> Result<(), anyhow::Error> } pub(super) fn autosave_timeout(state: &WaylandState, now: Instant) -> Option { + if state.session_transaction.is_some() { + // A queued command may outlive a failed autosave completion. Admit it + // on the next tick even when no worker wake remains outstanding. + return (!state.persistence.is_active()).then_some(Duration::ZERO); + } let autosave = scheduled_autosave_timeout( &state.session, state.session_options(), @@ -214,8 +225,13 @@ fn scheduled_autosave_timeout( } pub(super) fn autosave_if_due(state: &mut WaylandState, now: Instant) -> Result<(), anyhow::Error> { - drain_persistence_completion(state)?; + let completion_result = drain_persistence_completion(state); observe_input_dirty(state, now); + state.poll_pending_session_command(); + completion_result?; + if state.session_transaction.is_some() { + return Ok(()); + } if !state.persistence.is_healthy() { return Ok(()); @@ -412,7 +428,13 @@ impl PersistenceCompletionRuntime for WaylandState { fn try_receive_persistence_completion( &mut self, ) -> Result, anyhow::Error> { - self.persistence.try_receive() + let result = self.persistence.try_receive(); + if let Err(error) = &result + && let Some(transaction) = self.session_transaction.take() + { + self.fail_session_command(transaction.command(), error); + } + result } fn apply_persistence_completion( @@ -460,6 +482,14 @@ fn apply_persistence_completion( completion: PersistenceCompletion, ) -> Result<(), anyhow::Error> { observe_input_dirty(state, Instant::now()); + if state + .session_transaction + .as_ref() + .is_some_and(|transaction| transaction.request_id == Some(completion.id)) + { + state.complete_session_command(completion); + return Ok(()); + } let id = completion.id; let save_result: Result = match completion.result { Ok(PersistenceOutcome::Save(save)) => Ok(save), diff --git a/src/backend/wayland/session.rs b/src/backend/wayland/session.rs index 2932f9aa..9905e61d 100644 --- a/src/backend/wayland/session.rs +++ b/src/backend/wayland/session.rs @@ -586,7 +586,11 @@ pub(in crate::backend::wayland) use persistence::{ RequestId, SaveCompletion, SaveStrategy, SubmitFailure, }; -pub(in crate::backend::wayland) use runtime::SessionTransaction; +pub(in crate::backend::wayland) use runtime::{ + ExplicitSessionTransaction, SessionCommand, SessionCommandReport, SessionTransaction, + TransactionStep, +}; +#[cfg(test)] pub(in crate::backend::wayland) use runtime::{ RuntimeClearSessionReport, RuntimeClearToolStateReport, RuntimeOpenSessionReport, RuntimeSaveAsSessionReport, diff --git a/src/backend/wayland/session/runtime.rs b/src/backend/wayland/session/runtime.rs index a39316c0..a055115d 100644 --- a/src/backend/wayland/session/runtime.rs +++ b/src/backend/wayland/session/runtime.rs @@ -40,293 +40,12 @@ pub(in crate::backend::wayland) struct SessionTransaction<'a> { pub input_state: &'a mut InputState, pub measurer: &'a crate::draw::TextMeasurer, pub session: &'a mut SessionState, - pub persistence: &'a mut PersistenceController, } -impl SessionTransaction<'_> { - fn run(&mut self, operation: PersistenceOperation) -> Result { - self.persistence.run(self.session.target_epoch(), operation) - } - - pub(in crate::backend::wayland) fn open_named_session_runtime( - &mut self, - target_path: &Path, - ) -> Result { - let current_options = self - .session - .options() - .cloned() - .ok_or_else(|| anyhow!("cannot open session without active session options"))?; - let previous_path = current_options.session_file_path(); - - let validation = self.run(PersistenceOperation::ValidateNamedOpen { - path: target_path.to_path_buf(), - })?; - accept_open_preflight(self.session, validation)?; - - let saved_current = self.save_current_before_explicit_target_change(¤t_options)?; - let mut candidate_options = current_options; - candidate_options.set_named_file_target(target_path.to_path_buf()); - candidate_options.force_resume_persistence(); - - let outcome = self.run(PersistenceOperation::LoadNamedCandidate { - options: candidate_options.clone(), - })?; - let PersistenceOutcome::Load(load_outcome) = outcome else { - return Err(anyhow!("unexpected named-session load outcome")); - }; - let candidate_snapshot = named_candidate_snapshot(load_outcome, &candidate_options)?; - let loaded_board_data = candidate_snapshot.has_board_data(); - stored_session::apply_snapshot_replacing_boards( - self.input_state, - self.measurer, - candidate_snapshot, - &candidate_options, - )?; - self.input_state - .set_session_preflight_options(Some(candidate_options.clone())); - self.input_state.clear_session_dirty(); - let opened_path = candidate_options.session_file_path(); - self.session - .commit_runtime_open(candidate_options.clone(), loaded_board_data); - - match self.run(PersistenceOperation::RecordNamedOpened { - options: candidate_options, - }) { - Ok(PersistenceOutcome::Unit) => {} - Ok(other) => log::warn!( - "Named session opened, but catalog worker returned an unexpected outcome: {other:?}" - ), - Err(err) => log::warn!( - "Named session opened, but recording it in the recent-session catalog failed: {err:#}" - ), - } - - Ok(RuntimeOpenSessionReport { - previous_path, - opened_path, - saved_current, - loaded_board_data, - }) - } - - pub(in crate::backend::wayland) fn save_named_session_as_runtime( - &mut self, - target_path: &Path, - overwrite: SaveAsOverwrite, - ) -> Result { - let current_options = self - .session - .options() - .cloned() - .ok_or_else(|| anyhow!("cannot save session as without active session options"))?; - let previous_path = current_options.session_file_path(); - let mut target_options = current_options.clone(); - target_options.set_named_file_target(target_path.to_path_buf()); - target_options.force_resume_persistence(); - let preflight = self.run(PersistenceOperation::SaveAsOverwritePreflight { - current_path: previous_path.clone(), - options: target_options.clone(), - })?; - match accept_save_as_preflight(self.session, preflight, overwrite, target_path)? { - SaveAsPreflightDecision::SameTarget => { - let saved = self.save_current_before_explicit_target_change(¤t_options)?; - return Ok(RuntimeSaveAsSessionReport { - previous_path: previous_path.clone(), - saved_path: previous_path, - switched_target: false, - saved, - saved_board_data: self.session.has_loaded_board_data(), - outcome: None, - written_size: None, - }); - } - SaveAsPreflightDecision::SwitchTarget => {} - } - - let snapshot = self - .input_state - .with_active_interaction_canceled_for_capture_with(self.measurer, |input_state| { - stored_session::snapshot_from_input(input_state, &target_options) - }) - .ok_or_else(|| anyhow!("Save Session As has no session data to write"))?; - let outcome = self.run(PersistenceOperation::SaveAs { - snapshot, - options: target_options.clone(), - overwrite, - })?; - let PersistenceOutcome::SaveAs { - report, - committed_board_data, - } = outcome - else { - return Err(anyhow!("unexpected Save As worker outcome")); - }; - - self.input_state - .set_session_preflight_options(Some(target_options.clone())); - let _ = self.input_state.take_session_dirty(); - self.input_state.clear_session_dirty(); - let saved_path = target_options.session_file_path(); - self.session - .commit_runtime_save_as(target_options, Instant::now(), committed_board_data); - - Ok(RuntimeSaveAsSessionReport { - previous_path, - saved_path, - switched_target: true, - saved: true, - saved_board_data: committed_board_data, - outcome: Some(report.outcome), - written_size: Some(report.written_size), - }) - } - - pub(in crate::backend::wayland) fn save_named_session_as_requires_overwrite( - &mut self, - target_path: &Path, - ) -> Result { - let current_options = self - .session - .options() - .cloned() - .ok_or_else(|| anyhow!("cannot save session as without active session options"))?; - let previous_path = current_options.session_file_path(); - let mut target_options = current_options; - target_options.set_named_file_target(target_path.to_path_buf()); - target_options.force_resume_persistence(); - let outcome = self.run(PersistenceOperation::SaveAsOverwritePreflight { - current_path: previous_path, - options: target_options, - })?; - let PersistenceOutcome::SaveAsPreflight { - same_target, - overwrite_required, - } = outcome - else { - return Err(anyhow!("unexpected Save As preflight outcome")); - }; - Ok(!same_target && overwrite_required) - } - - pub(in crate::backend::wayland) fn clear_current_session_runtime( - &mut self, - ) -> Result { - let options = self - .session - .options() - .cloned() - .ok_or_else(|| anyhow!("cannot clear session without active session options"))?; - let cleared_path = options.session_file_path(); - let empty_snapshot = SessionSnapshot { - active_board_id: self.input_state.board_id().to_string(), - boards: Vec::new(), - tool_state: None, - }; - let outcome = self.run(PersistenceOperation::Save { - snapshot: empty_snapshot.clone(), - options: options.clone(), - strategy: SaveStrategy::Normal, - contentless_clear_boundary: true, - })?; - let PersistenceOutcome::Save(save) = outcome else { - return Err(anyhow!("unexpected clear-session worker outcome")); - }; - if !save.committed() { - return Err(anyhow!( - "current session clear did not write a committed clear boundary" - )); - } - stored_session::apply_snapshot_replacing_boards( - self.input_state, - self.measurer, - empty_snapshot, - &options, - )?; - self.input_state - .set_session_preflight_options(Some(options)); - let _ = self.input_state.take_session_dirty(); - self.input_state.clear_session_dirty(); - self.session.commit_runtime_clear(Instant::now()); - Ok(RuntimeClearSessionReport { - cleared_path, - persisted: true, - }) - } - - pub(in crate::backend::wayland) fn clear_saved_tool_state_runtime( - &mut self, - default_tool_state: ToolStateSnapshot, - ) -> Result { - let (session_path, outcome) = if let Some(options) = self.session.options().cloned() { - let path = options.session_file_path(); - let outcome = self.run(PersistenceOperation::ClearToolState { options })?; - let PersistenceOutcome::ToolStateCleared(outcome) = outcome else { - return Err(anyhow!("unexpected clear-tool-state worker outcome")); - }; - (Some(path), Some(outcome)) - } else { - (None, None) - }; - stored_session::apply_tool_state_snapshot( - self.input_state, - self.measurer, - default_tool_state, - ); - self.input_state.mark_session_dirty(); - self.session.record_input_dirty(Instant::now(), true); - - Ok(RuntimeClearToolStateReport { - session_path, - outcome, - }) - } - - fn save_current_before_explicit_target_change( - &mut self, - options: &SessionOptions, - ) -> Result { - if !self.input_state.is_session_dirty() && !self.session.is_dirty() { - return Ok(false); - } - let snapshot = self - .input_state - .with_active_interaction_canceled_for_capture_with(self.measurer, |input_state| { - stored_session::snapshot_from_input(input_state, options) - }); - let snapshot = if let Some(snapshot) = snapshot { - snapshot - } else if session_persistence_enabled(options) { - SessionSnapshot { - active_board_id: self.input_state.board_id().to_string(), - boards: Vec::new(), - tool_state: None, - } - } else { - return Err(anyhow!( - "current session has unsaved changes but persistence is disabled" - )); - }; - let outcome = self.run(PersistenceOperation::Save { - snapshot, - options: options.clone(), - strategy: SaveStrategy::Normal, - contentless_clear_boundary: self.session.has_loaded_board_data(), - })?; - let PersistenceOutcome::Save(save) = outcome else { - return Err(anyhow!("unexpected save-before-target-change outcome")); - }; - if !save.committed() { - return Err(anyhow!( - "current session had unsaved changes but no session file was written" - )); - } - let _ = self.input_state.take_session_dirty(); - self.session - .mark_saved(Instant::now(), save.committed_board_data); - Ok(true) - } -} +mod transaction; +pub(in crate::backend::wayland) use transaction::{ + ExplicitSessionTransaction, SessionCommand, SessionCommandReport, TransactionStep, +}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum SaveAsPreflightDecision { diff --git a/src/backend/wayland/session/runtime/transaction.rs b/src/backend/wayland/session/runtime/transaction.rs new file mode 100644 index 00000000..70c00ea8 --- /dev/null +++ b/src/backend/wayland/session/runtime/transaction.rs @@ -0,0 +1,253 @@ +//! Explicit session commands advance only when their disk phase completes. +use super::*; + +mod phases; + +#[derive(Debug)] +pub(in crate::backend::wayland) enum SessionCommand { + Open(PathBuf), + SaveAs(PathBuf, SaveAsOverwrite), + CheckOverwrite(PathBuf), + Clear, + ClearTools(Box), + Inspect, + Forget(PathBuf), +} + +pub(in crate::backend::wayland) enum SessionCommandReport { + Open(RuntimeOpenSessionReport), + SaveAs(RuntimeSaveAsSessionReport), + Overwrite(PathBuf, bool), + Clear(RuntimeClearSessionReport), + ClearTools(RuntimeClearToolStateReport), + Inspection(stored_session::SessionInspection), + Forgotten(PathBuf, bool), +} + +pub(in crate::backend::wayland) enum TransactionStep { + Work(Box), + Complete(Box), +} + +#[derive(Debug, Clone, Copy)] +enum Phase { + Start, + OpenPreflight, + SaveCurrent, + Load, + RecordOpen, + SaveAsPreflight, + SaveAs, + Clear, + ClearTools, + Inspect, + Forget, +} + +pub(in crate::backend::wayland) struct ExplicitSessionTransaction { + command: SessionCommand, + phase: Phase, + current: Option, + target: Option, + epoch: u64, + generation: Option, + interaction: Option<(u64, bool)>, + saved_current: bool, + loaded_board_data: bool, + pub request_id: Option, +} + +impl ExplicitSessionTransaction { + pub fn new(command: SessionCommand, epoch: u64, interaction: (u64, bool)) -> Self { + Self { + command, + phase: Phase::Start, + current: None, + target: None, + epoch, + generation: None, + interaction: Some(interaction), + saved_current: false, + loaded_board_data: false, + request_id: None, + } + } + + pub fn command(&self) -> &SessionCommand { + &self.command + } + + pub fn has_committed_open(&self) -> bool { + matches!(self.phase, Phase::RecordOpen) + } + + pub fn advance( + &mut self, + context: &mut SessionTransaction<'_>, + result: Option>, + ) -> Result { + if self.epoch != context.session.target_epoch() { + return Err(anyhow!( + "session target changed while the command was pending" + )); + } + let outcome = result.transpose()?; + if self + .generation + .is_some_and(|generation| generation != context.session.edit_generation()) + { + return Err(anyhow!( + "session was edited while the command was pending; retry the command" + )); + } + + if let Some((revision, active)) = self.interaction { + let (current_revision, current_active) = + context.input_state.session_interaction_state(); + if revision != current_revision || (!active && current_active) { + return Err(anyhow!( + "input interaction changed while the session command was pending; retry the command" + )); + } + } + + match self.phase { + Phase::Start => self.complete_start(context, outcome), + Phase::OpenPreflight => self.complete_open_preflight(context, outcome), + Phase::SaveCurrent => self.complete_save_current(context, outcome), + Phase::Load => self.complete_load(context, outcome), + Phase::RecordOpen => self.complete_record_open(context, outcome), + Phase::SaveAsPreflight => self.complete_save_as_preflight(context, outcome), + Phase::SaveAs => self.complete_save_as(context, outcome), + Phase::Clear => self.complete_clear(context, outcome), + Phase::ClearTools => self.complete_clear_tools(context, outcome), + Phase::Inspect => self.complete_inspect(context, outcome), + Phase::Forget => self.complete_forget(context, outcome), + } + } + + // Catalog bookkeeping is best effort after a successful open. + pub fn accept_catalog_failure(&self, error: &anyhow::Error) -> Option { + if matches!(self.phase, Phase::RecordOpen) { + log::warn!("Named session opened, but catalog update failed: {error:#}"); + Some(SessionCommandReport::Open(self.open_report())) + } else { + None + } + } + + fn work(&mut self, phase: Phase, operation: PersistenceOperation) -> Result { + self.phase = phase; + Ok(TransactionStep::Work(Box::new(operation))) + } + + fn capture_input_generation(&mut self, context: &SessionTransaction<'_>) { + self.generation = Some(context.session.edit_generation()); + } + + fn save_current_or_continue( + &mut self, + context: &mut SessionTransaction<'_>, + ) -> Result { + if !context.input_state.is_session_dirty() && !context.session.is_dirty() { + return self.after_current_save(context); + } + let options = self + .current + .as_ref() + .expect("target change has current options") + .clone(); + let snapshot = context + .input_state + .with_active_interaction_canceled_for_capture_with(context.measurer, |input| { + stored_session::snapshot_from_input(input, &options) + }); + let snapshot = if let Some(snapshot) = snapshot { + snapshot + } else if session_persistence_enabled(&options) { + empty_snapshot(context.input_state) + } else { + return Err(anyhow!( + "current session has unsaved changes but persistence is disabled" + )); + }; + self.capture_input_generation(context); + self.work( + Phase::SaveCurrent, + PersistenceOperation::Save { + snapshot, + options, + strategy: SaveStrategy::Normal, + contentless_clear_boundary: context.session.has_loaded_board_data(), + }, + ) + } + + fn after_current_save( + &mut self, + context: &mut SessionTransaction<'_>, + ) -> Result { + match &self.command { + SessionCommand::Open(_) => { + self.capture_input_generation(context); + self.work( + Phase::Load, + PersistenceOperation::LoadNamedCandidate { + options: self.target.as_ref().expect("open target").clone(), + }, + ) + } + SessionCommand::SaveAs(_, _) => Ok(TransactionStep::Complete(Box::new( + SessionCommandReport::SaveAs(RuntimeSaveAsSessionReport { + previous_path: self.current_path(), + saved_path: self.current_path(), + switched_target: false, + saved: self.saved_current, + saved_board_data: context.session.has_loaded_board_data(), + outcome: None, + written_size: None, + }), + ))), + _ => unreachable!(), + } + } + + fn current_path(&self) -> PathBuf { + self.current + .as_ref() + .expect("command has current options") + .session_file_path() + } + fn open_report(&self) -> RuntimeOpenSessionReport { + RuntimeOpenSessionReport { + previous_path: self.current_path(), + opened_path: self + .target + .as_ref() + .expect("open target") + .session_file_path(), + saved_current: self.saved_current, + loaded_board_data: self.loaded_board_data, + } + } + fn apply_default_tools( + &self, + context: &mut SessionTransaction<'_>, + defaults: ToolStateSnapshot, + ) { + stored_session::apply_tool_state_snapshot(context.input_state, context.measurer, defaults); + context.input_state.mark_session_dirty(); + context.session.record_input_dirty(Instant::now(), true); + } +} + +fn empty_snapshot(input: &InputState) -> SessionSnapshot { + SessionSnapshot { + active_board_id: input.board_id().to_string(), + boards: Vec::new(), + tool_state: None, + } +} +fn required_outcome(outcome: Option) -> Result { + outcome.ok_or_else(|| anyhow!("session command phase requires a completion")) +} diff --git a/src/backend/wayland/session/runtime/transaction/phases.rs b/src/backend/wayland/session/runtime/transaction/phases.rs new file mode 100644 index 00000000..99e80d0a --- /dev/null +++ b/src/backend/wayland/session/runtime/transaction/phases.rs @@ -0,0 +1,328 @@ +use super::*; + +impl ExplicitSessionTransaction { + pub(super) fn complete_start( + &mut self, + context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + if outcome.is_some() { + return Err(anyhow!( + "unexpected completion before session command started" + )); + } + self.current = context.session.options().cloned(); + match &self.command { + SessionCommand::ClearTools(defaults) if self.current.is_none() => { + let defaults = defaults.clone(); + self.apply_default_tools(context, *defaults); + Ok(TransactionStep::Complete(Box::new( + SessionCommandReport::ClearTools(RuntimeClearToolStateReport { + session_path: None, + outcome: None, + }), + ))) + } + SessionCommand::Forget(path) => self.work( + Phase::Forget, + PersistenceOperation::ForgetNamedSessionByPath { path: path.clone() }, + ), + _ => { + let options = self + .current + .clone() + .ok_or_else(|| anyhow!("no active persisted session target"))?; + match &self.command { + SessionCommand::Open(path) => { + let mut target = options; + target.set_named_file_target(path.clone()); + target.force_resume_persistence(); + self.target = Some(target); + self.work( + Phase::OpenPreflight, + PersistenceOperation::ValidateNamedOpen { path: path.clone() }, + ) + } + SessionCommand::SaveAs(path, _) | SessionCommand::CheckOverwrite(path) => { + let current_path = options.session_file_path(); + let mut target = options; + target.set_named_file_target(path.clone()); + target.force_resume_persistence(); + self.target = Some(target.clone()); + self.work( + Phase::SaveAsPreflight, + PersistenceOperation::SaveAsOverwritePreflight { + current_path, + options: target, + }, + ) + } + SessionCommand::Clear => { + self.capture_input_generation(context); + self.work( + Phase::Clear, + PersistenceOperation::Save { + snapshot: empty_snapshot(context.input_state), + options, + strategy: SaveStrategy::Normal, + contentless_clear_boundary: true, + }, + ) + } + SessionCommand::ClearTools(_) => { + self.capture_input_generation(context); + self.work( + Phase::ClearTools, + PersistenceOperation::ClearToolState { options }, + ) + } + SessionCommand::Inspect => { + self.work(Phase::Inspect, PersistenceOperation::Inspect { options }) + } + SessionCommand::Forget(_) => unreachable!(), + } + } + } + } + + pub(super) fn complete_open_preflight( + &mut self, + context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + accept_open_preflight(context.session, required_outcome(outcome)?)?; + self.save_current_or_continue(context) + } + + pub(super) fn complete_save_current( + &mut self, + context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + let PersistenceOutcome::Save(save) = required_outcome(outcome)? else { + return Err(anyhow!("unexpected save-before-target-change outcome")); + }; + if !save.committed() { + return Err(anyhow!( + "current session had unsaved changes but no session file was written" + )); + } + context.input_state.clear_session_dirty(); + context + .session + .mark_saved(Instant::now(), save.committed_board_data); + self.saved_current = true; + self.after_current_save(context) + } + + pub(super) fn complete_load( + &mut self, + context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + let PersistenceOutcome::Load(load) = required_outcome(outcome)? else { + return Err(anyhow!("unexpected named-session load outcome")); + }; + let target = self.target.as_ref().expect("open has target"); + let snapshot = named_candidate_snapshot(load, target)?; + self.loaded_board_data = snapshot.has_board_data(); + stored_session::apply_snapshot_replacing_boards( + context.input_state, + context.measurer, + snapshot, + target, + )?; + context + .input_state + .set_session_preflight_options(Some(target.clone())); + context.input_state.clear_session_dirty(); + context + .session + .commit_runtime_open(target.clone(), self.loaded_board_data); + self.epoch = context.session.target_epoch(); + // The open is committed. Later edits belong to the opened target; + // a catalog completion must never roll that state back. + self.generation = None; + self.interaction = None; + self.work( + Phase::RecordOpen, + PersistenceOperation::RecordNamedOpened { + options: target.clone(), + }, + ) + } + + pub(super) fn complete_record_open( + &mut self, + _context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + if !matches!(required_outcome(outcome)?, PersistenceOutcome::Unit) { + log::warn!("Named session opened, but catalog returned an unexpected outcome"); + } + Ok(TransactionStep::Complete(Box::new( + SessionCommandReport::Open(self.open_report()), + ))) + } + + pub(super) fn complete_save_as_preflight( + &mut self, + context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + let outcome = required_outcome(outcome)?; + if let SessionCommand::CheckOverwrite(path) = &self.command { + let PersistenceOutcome::SaveAsPreflight { + same_target, + overwrite_required, + } = outcome + else { + return Err(anyhow!("unexpected Save As preflight outcome")); + }; + return Ok(TransactionStep::Complete(Box::new( + SessionCommandReport::Overwrite(path.clone(), !same_target && overwrite_required), + ))); + } + let SessionCommand::SaveAs(path, overwrite) = &self.command else { + unreachable!() + }; + match accept_save_as_preflight(context.session, outcome, *overwrite, path)? { + SaveAsPreflightDecision::SameTarget => self.save_current_or_continue(context), + SaveAsPreflightDecision::SwitchTarget => { + let overwrite = *overwrite; + let options = self.target.as_ref().expect("save as has target").clone(); + let snapshot = context + .input_state + .with_active_interaction_canceled_for_capture_with(context.measurer, |input| { + stored_session::snapshot_from_input(input, &options) + }) + .ok_or_else(|| anyhow!("Save Session As has no session data to write"))?; + self.capture_input_generation(context); + self.work( + Phase::SaveAs, + PersistenceOperation::SaveAs { + snapshot, + options, + overwrite, + }, + ) + } + } + } + + pub(super) fn complete_save_as( + &mut self, + context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + let PersistenceOutcome::SaveAs { + report, + committed_board_data, + } = required_outcome(outcome)? + else { + return Err(anyhow!("unexpected Save As worker outcome")); + }; + let target = self.target.as_ref().expect("save as has target").clone(); + let saved_path = target.session_file_path(); + context + .input_state + .set_session_preflight_options(Some(target.clone())); + context.input_state.clear_session_dirty(); + context + .session + .commit_runtime_save_as(target, Instant::now(), committed_board_data); + Ok(TransactionStep::Complete(Box::new( + SessionCommandReport::SaveAs(RuntimeSaveAsSessionReport { + previous_path: self.current_path(), + saved_path, + switched_target: true, + saved: true, + saved_board_data: committed_board_data, + outcome: Some(report.outcome), + written_size: Some(report.written_size), + }), + ))) + } + + pub(super) fn complete_clear( + &mut self, + context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + let PersistenceOutcome::Save(save) = required_outcome(outcome)? else { + return Err(anyhow!("unexpected clear-session worker outcome")); + }; + if !save.committed() { + return Err(anyhow!( + "current session clear did not write a committed clear boundary" + )); + } + let options = self.current.as_ref().expect("clear has options"); + stored_session::apply_snapshot_replacing_boards( + context.input_state, + context.measurer, + empty_snapshot(context.input_state), + options, + )?; + context + .input_state + .set_session_preflight_options(Some(options.clone())); + context.input_state.clear_session_dirty(); + context.session.commit_runtime_clear(Instant::now()); + Ok(TransactionStep::Complete(Box::new( + SessionCommandReport::Clear(RuntimeClearSessionReport { + cleared_path: options.session_file_path(), + persisted: true, + }), + ))) + } + + pub(super) fn complete_clear_tools( + &mut self, + context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + let PersistenceOutcome::ToolStateCleared(outcome) = required_outcome(outcome)? else { + return Err(anyhow!("unexpected clear-tool-state worker outcome")); + }; + let SessionCommand::ClearTools(defaults) = &self.command else { + unreachable!() + }; + self.apply_default_tools(context, *defaults.clone()); + Ok(TransactionStep::Complete(Box::new( + SessionCommandReport::ClearTools(RuntimeClearToolStateReport { + session_path: Some(self.current_path()), + outcome: Some(outcome), + }), + ))) + } + + pub(super) fn complete_inspect( + &mut self, + _context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + let PersistenceOutcome::Inspection(inspection) = required_outcome(outcome)? else { + return Err(anyhow!("unexpected inspection outcome")); + }; + Ok(TransactionStep::Complete(Box::new( + SessionCommandReport::Inspection(inspection), + ))) + } + + pub(super) fn complete_forget( + &mut self, + _context: &mut SessionTransaction<'_>, + outcome: Option, + ) -> Result { + let PersistenceOutcome::CatalogForgotten(forgotten) = required_outcome(outcome)? else { + return Err(anyhow!("unexpected forget outcome")); + }; + let SessionCommand::Forget(path) = &self.command else { + unreachable!() + }; + Ok(TransactionStep::Complete(Box::new( + SessionCommandReport::Forgotten(path.clone(), forgotten), + ))) + } +} diff --git a/src/backend/wayland/session/tests.rs b/src/backend/wayland/session/tests.rs index 7f0e917c..78c16d6e 100644 --- a/src/backend/wayland/session/tests.rs +++ b/src/backend/wayland/session/tests.rs @@ -2008,86 +2008,434 @@ fn output_transition_deferral_moves_deadline_forward() { ); } +fn drive_session_command( + input: &mut InputState, + measurer: &crate::draw::TextMeasurer, + session: &mut SessionState, + command: SessionCommand, +) -> Result { + let mut persistence = PersistenceController::start_for_test()?; + let mut transaction = ExplicitSessionTransaction::new( + command, + session.target_epoch(), + input.session_interaction_state(), + ); + let mut result = None; + loop { + if let Some(Err(error)) = result.as_ref() + && let Some(report) = transaction.accept_catalog_failure(error) + { + return Ok(report); + } + let step = transaction.advance( + &mut SessionTransaction { + input_state: input, + measurer, + session, + }, + result, + )?; + match step { + TransactionStep::Work(operation) => { + result = Some(persistence.run(session.target_epoch(), *operation)) + } + TransactionStep::Complete(report) => return Ok(*report), + } + } +} + fn open_named_session_runtime( - input_state: &mut InputState, + input: &mut InputState, measurer: &crate::draw::TextMeasurer, - session_state: &mut SessionState, - target_path: &Path, + session: &mut SessionState, + target: &Path, _now: Instant, ) -> Result { - let mut persistence = PersistenceController::start_for_test()?; - SessionTransaction { - input_state, + let SessionCommandReport::Open(report) = drive_session_command( + input, measurer, - session: session_state, - persistence: &mut persistence, - } - .open_named_session_runtime(target_path) + session, + SessionCommand::Open(target.to_path_buf()), + )? + else { + panic!("unexpected report") + }; + Ok(report) } - fn save_named_session_as_runtime( - input_state: &mut InputState, + input: &mut InputState, measurer: &crate::draw::TextMeasurer, - session_state: &mut SessionState, - target_path: &Path, + session: &mut SessionState, + target: &Path, overwrite: stored_session::SaveAsOverwrite, _now: Instant, ) -> Result { - let mut persistence = PersistenceController::start_for_test()?; - SessionTransaction { - input_state, + let SessionCommandReport::SaveAs(report) = drive_session_command( + input, measurer, - session: session_state, - persistence: &mut persistence, - } - .save_named_session_as_runtime(target_path, overwrite) + session, + SessionCommand::SaveAs(target.to_path_buf(), overwrite), + )? + else { + panic!("unexpected report") + }; + Ok(report) } - fn save_named_session_as_requires_overwrite( - session_state: &mut SessionState, - target_path: &Path, + session: &mut SessionState, + target: &Path, ) -> Result { - let mut input_state = test_input_state(); - let measurer = crate::draw::TextMeasurer::default(); - let mut persistence = PersistenceController::start_for_test()?; - SessionTransaction { - input_state: &mut input_state, - measurer: &measurer, - session: session_state, - persistence: &mut persistence, - } - .save_named_session_as_requires_overwrite(target_path) + let mut input = test_input_state(); + let SessionCommandReport::Overwrite(_, required) = drive_session_command( + &mut input, + &crate::draw::TextMeasurer::default(), + session, + SessionCommand::CheckOverwrite(target.to_path_buf()), + )? + else { + panic!("unexpected report") + }; + Ok(required) } - fn clear_current_session_runtime( - input_state: &mut InputState, + input: &mut InputState, measurer: &crate::draw::TextMeasurer, - session_state: &mut SessionState, + session: &mut SessionState, _now: Instant, ) -> Result { - let mut persistence = PersistenceController::start_for_test()?; - SessionTransaction { - input_state, - measurer, - session: session_state, - persistence: &mut persistence, - } - .clear_current_session_runtime() + let SessionCommandReport::Clear(report) = + drive_session_command(input, measurer, session, SessionCommand::Clear)? + else { + panic!("unexpected report") + }; + Ok(report) } - fn clear_saved_tool_state_runtime( - input_state: &mut InputState, + input: &mut InputState, measurer: &crate::draw::TextMeasurer, - session_state: &mut SessionState, - default_tool_state: stored_session::ToolStateSnapshot, + session: &mut SessionState, + defaults: stored_session::ToolStateSnapshot, _now: Instant, ) -> Result { - let mut persistence = PersistenceController::start_for_test()?; - SessionTransaction { - input_state, + let SessionCommandReport::ClearTools(report) = drive_session_command( + input, measurer, - session: session_state, - persistence: &mut persistence, + session, + SessionCommand::ClearTools(Box::new(defaults)), + )? + else { + panic!("unexpected report") + }; + Ok(report) +} + +#[test] +fn explicit_clear_keeps_dispatch_available_and_preserves_edits_during_blocked_disk_work() { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "blocked-clear"); + stored_session::save_snapshot(&sample_snapshot(), &options).unwrap(); + let lock = std::fs::OpenOptions::new() + .read(true) + .write(true) + .open(options.lock_file_path()) + .unwrap(); + crate::durable_io::lock_exclusive(&lock).unwrap(); + let mut input = test_input_state(); + add_line(&mut input, 51); + let measurer = crate::draw::TextMeasurer::default(); + let mut session = SessionState::new(Some(options.clone())); + let mut persistence = PersistenceController::start_for_test().unwrap(); + let mut transaction = ExplicitSessionTransaction::new( + SessionCommand::Clear, + session.target_epoch(), + input.session_interaction_state(), + ); + let TransactionStep::Work(operation) = transaction + .advance( + &mut SessionTransaction { + input_state: &mut input, + measurer: &measurer, + session: &mut session, + }, + None, + ) + .unwrap() + else { + panic!("clear must request a disk phase") + }; + let id = persistence + .try_submit(session.target_epoch(), *operation) + .unwrap(); + transaction.request_id = Some(id); + + // The real worker cannot acquire its disk lock. Input can still change, + // and polling completion does not wait for that lock to be released. + assert!(persistence.try_receive().unwrap().is_none()); + add_line(&mut input, 99); + input.mark_session_dirty(); + session.record_input_dirty(Instant::now(), input.take_session_dirty()); + assert_eq!(input.boards.active_frame().shapes.len(), 2); + drop(lock); + + let completion = persistence.wait_for_completion().unwrap().unwrap(); + assert_eq!(completion.id, id); + let result = transaction.advance( + &mut SessionTransaction { + input_state: &mut input, + measurer: &measurer, + session: &mut session, + }, + Some(completion.result), + ); + assert!(matches!(result, Err(error) if error.to_string().contains("edited while"))); + assert_eq!(input.boards.active_frame().shapes.len(), 2); + assert!(session.is_dirty()); + assert_eq!( + session.options().unwrap().session_file_path(), + options.session_file_path() + ); + let snapshot = stored_session::snapshot_from_input(&input, &options).unwrap(); + let outcome = persistence + .run( + session.target_epoch(), + PersistenceOperation::Save { + snapshot, + options: options.clone(), + strategy: SaveStrategy::Normal, + contentless_clear_boundary: false, + }, + ) + .unwrap(); + assert!(matches!(outcome, PersistenceOutcome::Save(save) if save.committed())); + let LoadSnapshotOutcome::Loaded(snapshot) = + stored_session::load_snapshot_with_outcome(&options).unwrap() + else { + panic!("new edits must remain persistable") + }; + assert_eq!(snapshot.boards[0].pages.pages[0].shapes.len(), 2); +} + +#[test] +fn explicit_open_rejects_a_completion_after_target_epoch_changes() { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "epoch-source"); + let target = named_options(temp.path(), "epoch-candidate"); + stored_session::save_snapshot(&sample_snapshot(), &target).unwrap(); + let mut input = test_input_state(); + add_line(&mut input, 91); + let measurer = crate::draw::TextMeasurer::default(); + let mut session = SessionState::new(Some(options)); + let mut persistence = PersistenceController::start_for_test().unwrap(); + let mut transaction = ExplicitSessionTransaction::new( + SessionCommand::Open(target.session_file_path()), + session.target_epoch(), + input.session_interaction_state(), + ); + let TransactionStep::Work(operation) = transaction + .advance( + &mut SessionTransaction { + input_state: &mut input, + measurer: &measurer, + session: &mut session, + }, + None, + ) + .unwrap() + else { + panic!("open must request validation") + }; + let outcome = persistence.run(session.target_epoch(), *operation); + let replacement = named_options(temp.path(), "epoch-new-target"); + session.commit_runtime_open(replacement.clone(), false); + + let result = transaction.advance( + &mut SessionTransaction { + input_state: &mut input, + measurer: &measurer, + session: &mut session, + }, + Some(outcome), + ); + assert!(matches!(result, Err(error) if error.to_string().contains("target changed"))); + assert_eq!(input.boards.active_frame().shapes.len(), 1); + assert_eq!( + session.options().unwrap().session_file_path(), + replacement.session_file_path() + ); +} + +#[cfg(unix)] +#[test] +fn runtime_open_validates_target_before_saving_dirty_current_session() { + let temp = crate::test_temp::tempdir().unwrap(); + let current = named_options(temp.path(), "unsaved-source"); + let candidate = named_options(temp.path(), "symlink-candidate"); + let destination = temp.path().join("keep-unrelated-bytes"); + std::fs::write(&destination, b"unrelated content").unwrap(); + symlink(&destination, candidate.session_file_path()).unwrap(); + let mut input = test_input_state(); + add_line(&mut input, 63); + input.mark_session_dirty(); + let mut session = SessionState::new(Some(current.clone())); + + let error = open_named_session_runtime( + &mut input, + &crate::draw::TextMeasurer::default(), + &mut session, + &candidate.session_file_path(), + Instant::now(), + ) + .unwrap_err(); + + assert!(error.to_string().contains("symlink"), "{error:#}"); + assert!( + !current.session_file_path().exists(), + "invalid target must be rejected before saving the source" + ); + assert!(input.is_session_dirty()); + assert_eq!(input.boards.active_frame().shapes.len(), 1); + assert_eq!( + session.options().unwrap().session_file_path(), + current.session_file_path() + ); + assert_eq!(std::fs::read(destination).unwrap(), b"unrelated content"); +} + +#[test] +fn pending_session_handoffs_preserve_new_unfinished_pointer_and_text_drafts() { + fn begin_draft(input: &mut InputState, measurer: &crate::draw::TextMeasurer, kind: &str) { + if kind == "pointer" { + input.on_mouse_press(crate::input::MouseButton::Left, 80, 80); + input.on_mouse_motion(90, 90); + assert!(matches!(input.state, DrawingState::Drawing { .. })); + } else { + let ui_engine = crate::ui_text::UiTextEngine::default(); + input.handle_action_with_resources( + crate::input::state::InputTextResources { + measurer, + ui_engine: &ui_engine, + }, + Action::EnterTextMode, + ); + input.on_key_press(crate::input::Key::Char('x')); + assert!( + matches!(&input.state, DrawingState::TextInput { buffer, .. } if buffer == "x") + ); + } + assert!( + !input.is_session_dirty(), + "unfinished drafts have not entered history" + ); + } + + for (command_kind, pending_phase) in [ + ("open", "admission"), + ("open", "preflight"), + ("open", "snapshot"), + ("clear", "admission"), + ("clear", "snapshot"), + ("save_as", "admission"), + ("save_as", "preflight"), + ("save_as", "snapshot"), + ] { + for draft_kind in ["pointer", "text"] { + let temp = crate::test_temp::tempdir().unwrap(); + let current = named_options(temp.path(), "draft-source"); + let target = named_options(temp.path(), "draft-target"); + if command_kind == "open" { + stored_session::save_snapshot(&sample_snapshot(), &target).unwrap(); + } + let mut input = test_input_state(); + add_line(&mut input, 61); + let measurer = crate::draw::TextMeasurer::default(); + let mut session = SessionState::new(Some(current.clone())); + let mut worker = PersistenceController::start_for_test().unwrap(); + let command = match command_kind { + "open" => SessionCommand::Open(target.session_file_path()), + "clear" => SessionCommand::Clear, + _ => SessionCommand::SaveAs( + target.session_file_path(), + stored_session::SaveAsOverwrite::Deny, + ), + }; + let mut transaction = ExplicitSessionTransaction::new( + command, + session.target_epoch(), + input.session_interaction_state(), + ); + let result = if pending_phase == "admission" { + // This is the same retained admission used while an autosave + // owns the worker; starting it later must keep its identity. + begin_draft(&mut input, &measurer, draft_kind); + transaction.advance( + &mut SessionTransaction { + input_state: &mut input, + measurer: &measurer, + session: &mut session, + }, + None, + ) + } else { + let TransactionStep::Work(mut operation) = transaction + .advance( + &mut SessionTransaction { + input_state: &mut input, + measurer: &measurer, + session: &mut session, + }, + None, + ) + .unwrap() + else { + panic!("expected disk work") + }; + if pending_phase == "snapshot" && command_kind != "clear" { + let preflight = worker.run(session.target_epoch(), *operation); + let TransactionStep::Work(next) = transaction + .advance( + &mut SessionTransaction { + input_state: &mut input, + measurer: &measurer, + session: &mut session, + }, + Some(preflight), + ) + .unwrap() + else { + panic!("expected snapshot work") + }; + operation = next; + } + begin_draft(&mut input, &measurer, draft_kind); + let completion = worker.run(session.target_epoch(), *operation); + transaction.advance( + &mut SessionTransaction { + input_state: &mut input, + measurer: &measurer, + session: &mut session, + }, + Some(completion), + ) + }; + + assert!( + matches!(result, Err(error) if error.to_string().contains("interaction changed")), + "{command_kind}/{pending_phase}/{draft_kind}" + ); + assert_eq!( + session.options().unwrap().session_file_path(), + current.session_file_path() + ); + assert_eq!(input.boards.active_frame().shapes.len(), 1); + if draft_kind == "pointer" { + assert!( + matches!(&input.state, DrawingState::Drawing { points, .. } if points.last() == Some(&(90,90))) + ); + } else { + assert!( + matches!(&input.state, DrawingState::TextInput { buffer, .. } if buffer == "x") + ); + } + } } - .clear_saved_tool_state_runtime(default_tool_state) } diff --git a/src/backend/wayland/state.rs b/src/backend/wayland/state.rs index 3ecdb561..49feebb5 100644 --- a/src/backend/wayland/state.rs +++ b/src/backend/wayland/state.rs @@ -236,6 +236,8 @@ pub(super) struct WaylandState { // Session persistence pub(super) session: SessionState, pub(super) persistence: crate::backend::wayland::session::PersistenceController, + pub(super) session_transaction: + Option, session_dialog: self::toolbar::SessionFileDialogController, pub(super) durable_action_finish: Option, pub(super) durable_action_retry_at: Option, diff --git a/src/backend/wayland/state/core/init.rs b/src/backend/wayland/state/core/init.rs index ede9c925..f2441dfd 100644 --- a/src/backend/wayland/state/core/init.rs +++ b/src/backend/wayland/state/core/init.rs @@ -158,6 +158,7 @@ impl WaylandState { tablet: super::super::tablet_runtime::TabletState::new(tablet_manager, tablet_settings), session: SessionState::new(session_options), session_config_failed, + session_transaction: None, persistence, input_hud: super::super::input_hud::InputHudRuntime::new(runtime_wake.clone()), session_dialog: super::super::toolbar::SessionFileDialogController::new(runtime_wake), diff --git a/src/backend/wayland/state/core/output/transition.rs b/src/backend/wayland/state/core/output/transition.rs index b1af4928..41e6b27a 100644 --- a/src/backend/wayland/state/core/output/transition.rs +++ b/src/backend/wayland/state/core/output/transition.rs @@ -21,7 +21,8 @@ impl WaylandState { .is_some_and(|pending| { pending.physical_output_identity == physical_output_identity }); - let interaction_active = session_save::should_defer_for_interaction(self); + let interaction_active = + self.session_transaction.is_some() || session_save::should_defer_for_interaction(self); let input_dirty = self.input_state.is_session_dirty(); let live_source_resolution_pending = self .session @@ -122,7 +123,7 @@ impl WaylandState { self.session.cancel_pending_output_transition(); return Ok(true); } - if session_save::should_defer_for_interaction(self) { + if self.session_transaction.is_some() || session_save::should_defer_for_interaction(self) { self.session .defer_output_transition(now, session_save::interaction_defer_interval()); log::debug!("Deferring pending output transition while interaction is active"); @@ -179,7 +180,8 @@ impl WaylandState { let _ = self.session.resolve_live_source_resolution(false, false); return false; } - let interaction_active = session_save::should_defer_for_interaction(self); + let interaction_active = + self.session_transaction.is_some() || session_save::should_defer_for_interaction(self); if !live_source_reconciliation_ready( true, self.session.pending_output_transition().is_some(), @@ -203,7 +205,7 @@ impl WaylandState { physical_output_identity: Option, reason: &str, ) -> anyhow::Result<()> { - if session_save::should_defer_for_interaction(self) { + if self.session_transaction.is_some() || session_save::should_defer_for_interaction(self) { return Err(anyhow::anyhow!( "output transition became ineligible because an interaction started" )); diff --git a/src/backend/wayland/state/core/session.rs b/src/backend/wayland/state/core/session.rs index 4509f814..5be2008f 100644 --- a/src/backend/wayland/state/core/session.rs +++ b/src/backend/wayland/state/core/session.rs @@ -1,153 +1,148 @@ -use crate::input::state::{Toast, ToastPriority}; -use std::path::{Path, PathBuf}; - use anyhow::{Result, anyhow}; +use std::time::Instant; use super::super::*; use crate::backend::wayland::{ backend::event_loop::session_save, session::{ - PersistenceOperation, PersistenceOutcome, RuntimeClearSessionReport, - RuntimeClearToolStateReport, RuntimeOpenSessionReport, RuntimeSaveAsSessionReport, + ExplicitSessionTransaction, PersistenceCompletion, SessionCommand, SessionCommandReport, + SessionTransaction, TransactionStep, }, }; -use crate::session::{ - self as stored_session, ClearToolStateOutcome, SaveAsOverwrite, ToolStateSnapshot, -}; +use crate::session::ToolStateSnapshot; impl WaylandState { - pub(in crate::backend::wayland) fn open_named_session_runtime( + /// Admit one explicit command. An existing autosave finishes first without + /// waiting in dispatch; the command captures its live input when admitted. + pub(in crate::backend::wayland) fn start_session_command( &mut self, - target_path: &Path, - ) -> Result { - session_save::persistence_barrier(self)?; - let result = crate::backend::wayland::session::SessionTransaction { - input_state: &mut self.input_state, - measurer: self.render.text_measurer(), - session: &mut self.session, - persistence: &mut self.persistence, + command: SessionCommand, + ) -> Result<()> { + if self.session_transaction.is_some() { + return Err(anyhow!("another session command is already pending")); } - .open_named_session_runtime(target_path); - if result.is_ok() { - self.refresh_runtime_ui_config_seeds(); + if matches!( + command, + SessionCommand::Clear | SessionCommand::ClearTools(_) + ) { + ensure_destructive_session_config_available(self.session_config_failed)?; } - result - } - - pub(in crate::backend::wayland) fn save_named_session_as_runtime( - &mut self, - target_path: &Path, - overwrite: SaveAsOverwrite, - ) -> Result { - session_save::persistence_barrier(self)?; - - crate::backend::wayland::session::SessionTransaction { - input_state: &mut self.input_state, - measurer: self.render.text_measurer(), - session: &mut self.session, - persistence: &mut self.persistence, + if !self.persistence.is_healthy() { + return Err(anyhow!("session persistence worker is unhealthy")); } - .save_named_session_as_runtime(target_path, overwrite) + self.session_transaction = Some(ExplicitSessionTransaction::new( + command, + self.session.target_epoch(), + self.input_state.session_interaction_state(), + )); + self.poll_pending_session_command(); + Ok(()) } - pub(in crate::backend::wayland) fn save_named_session_as_requires_overwrite( - &mut self, - target_path: &Path, - ) -> Result { - session_save::persistence_barrier(self)?; - - crate::backend::wayland::session::SessionTransaction { - input_state: &mut self.input_state, - measurer: self.render.text_measurer(), - session: &mut self.session, - persistence: &mut self.persistence, + pub(in crate::backend::wayland) fn poll_pending_session_command(&mut self) { + if self.persistence.is_active() { + return; } - .save_named_session_as_requires_overwrite(target_path) + let Some(transaction) = self.session_transaction.take() else { + return; + }; + self.advance_session_command(transaction, None); } - pub(in crate::backend::wayland) fn clear_current_session_runtime( + pub(in crate::backend::wayland) fn complete_session_command( &mut self, - ) -> Result { - ensure_destructive_session_config_available(self.session_config_failed)?; - session_save::persistence_barrier(self)?; - let result = crate::backend::wayland::session::SessionTransaction { - input_state: &mut self.input_state, - measurer: self.render.text_measurer(), - session: &mut self.session, - persistence: &mut self.persistence, - } - .clear_current_session_runtime(); - if result.is_ok() { - self.refresh_runtime_ui_config_seeds(); + completion: PersistenceCompletion, + ) { + let Some(transaction) = self.session_transaction.take() else { + return; + }; + if transaction.request_id != Some(completion.id) { + self.fail_session_command( + transaction.command(), + &anyhow!("explicit session completion identity mismatch"), + ); + return; } - result + self.advance_session_command(transaction, Some(completion.result)); } - pub(in crate::backend::wayland) fn clear_saved_tool_state_runtime( + fn advance_session_command( &mut self, - ) -> Result { - ensure_destructive_session_config_available(self.session_config_failed)?; - let default_tool_state = ToolStateSnapshot::from_config(&self.config); - session_save::persistence_barrier(self)?; - - crate::backend::wayland::session::SessionTransaction { - input_state: &mut self.input_state, - measurer: self.render.text_measurer(), - session: &mut self.session, - persistence: &mut self.persistence, - } - .clear_saved_tool_state_runtime(default_tool_state) - } - - pub(in crate::backend::wayland) fn handle_clear_saved_tool_state_action(&mut self) { - match self.clear_saved_tool_state_runtime() { - Ok(report) => { - let message = clear_tool_state_runtime_message(&report); - log::info!("{message}"); - self.input_state - .push_toast(ToastPriority::Info, "session", Toast::info(message)); + mut transaction: ExplicitSessionTransaction, + result: Option>, + ) { + session_save::observe_input_dirty(self, Instant::now()); + // A catalog failure follows an already committed open. It is reported + // independently, without reverting the canvas or its current edits. + let step = if let Some(Err(error)) = result.as_ref() + && let Some(report) = transaction.accept_catalog_failure(error) + { + Ok(TransactionStep::Complete(Box::new(report))) + } else { + transaction.advance( + &mut SessionTransaction { + input_state: &mut self.input_state, + measurer: self.render.text_measurer(), + session: &mut self.session, + }, + result, + ) + }; + match step { + Ok(TransactionStep::Work(operation)) => { + // Applying an open refreshes consumer seeds before catalog work. + if transaction.has_committed_open() { + self.refresh_runtime_ui_config_seeds(); + } + match self + .persistence + .try_submit(self.session.target_epoch(), *operation) + { + Ok(id) => { + transaction.request_id = Some(id); + self.session_transaction = Some(transaction); + } + Err(failure) => { + let error = anyhow!("failed to submit session command: {}", failure.error); + if let Some(report) = transaction.accept_catalog_failure(&error) { + self.finish_session_command(report); + } else { + self.fail_session_command(transaction.command(), &error); + } + } + } } - Err(err) => { - let message = format!("Failed to reset tool defaults: {err:#}"); - log::warn!("{message}"); - self.input_state.push_toast( - ToastPriority::Critical, - "session", - Toast::error(message), - ); + Ok(TransactionStep::Complete(report)) => { + if matches!( + *report, + SessionCommandReport::Open(_) | SessionCommandReport::Clear(_) + ) { + self.refresh_runtime_ui_config_seeds(); + } + self.finish_session_command(*report); } + Err(error) => self.fail_session_command(transaction.command(), &error), } } - pub(in crate::backend::wayland) fn inspect_active_session( - &mut self, - ) -> Result { - let options = self - .session_options() - .cloned() - .ok_or_else(|| anyhow!("no active persisted session target"))?; - let outcome = session_save::run_persistence_operation( - self, - PersistenceOperation::Inspect { options }, - )?; - let PersistenceOutcome::Inspection(inspection) = outcome else { - return Err(anyhow!("unexpected session-inspection worker outcome")); - }; - Ok(inspection) + pub(in crate::backend::wayland) fn handle_clear_saved_tool_state_action(&mut self) { + let command = + SessionCommand::ClearTools(Box::new(ToolStateSnapshot::from_config(&self.config))); + if let Err(error) = self.start_session_command(command) { + self.report_session_command_error("Failed to reset tool defaults", &error); + } } - pub(in crate::backend::wayland) fn forget_named_session_by_path( - &mut self, - path: PathBuf, - ) -> Result { - let outcome = session_save::run_persistence_operation( - self, - PersistenceOperation::ForgetNamedSessionByPath { path }, - )?; - let PersistenceOutcome::CatalogForgotten(forgotten) = outcome else { - return Err(anyhow!("unexpected catalog-forget worker outcome")); - }; - Ok(forgotten) + /// Shutdown may wait for durable work; normal dispatch never calls this. + pub(in crate::backend::wayland) fn finish_pending_session_command(&mut self) -> Result<()> { + while self.session_transaction.is_some() { + if self.persistence.is_active() { + session_save::persistence_barrier(self)?; + } else { + self.poll_pending_session_command(); + } + } + Ok(()) } } @@ -160,37 +155,13 @@ fn ensure_destructive_session_config_available(section_failed: bool) -> Result<( Ok(()) } -fn clear_tool_state_runtime_message(report: &RuntimeClearToolStateReport) -> String { - match report.outcome { - Some(ClearToolStateOutcome::Cleared { - preserved_board_data: true, - }) => { - "Tool defaults reset from config. Saved boards and history were preserved.".to_string() - } - Some(ClearToolStateOutcome::Cleared { - preserved_board_data: false, - }) => "Tool defaults reset from config. No board data was present.".to_string(), - Some(ClearToolStateOutcome::NoToolState) => { - "Tool defaults reset from config. No saved tool state was stored.".to_string() - } - Some(ClearToolStateOutcome::NoSession) => { - "Tool defaults reset from config. No saved session file was present.".to_string() - } - None => "Tool defaults reset from config for this run. No active session file to edit." - .to_string(), - } -} - #[cfg(test)] mod tests { use super::*; - #[test] fn destructive_session_actions_fail_closed_after_session_config_fallback() { - let err = ensure_destructive_session_config_available(true) - .expect_err("default-derived session paths must not be mutated"); + let err = ensure_destructive_session_config_available(true).unwrap_err(); assert!(format!("{err:#}").contains("refusing to modify saved session data")); - ensure_destructive_session_config_available(false) - .expect("a successfully loaded session section permits mutations"); + ensure_destructive_session_config_available(false).unwrap(); } } diff --git a/src/backend/wayland/state/toolbar/events/session.rs b/src/backend/wayland/state/toolbar/events/session.rs index ab8c0b52..24579810 100644 --- a/src/backend/wayland/state/toolbar/events/session.rs +++ b/src/backend/wayland/state/toolbar/events/session.rs @@ -1,4 +1,5 @@ use super::*; +use crate::backend::wayland::session::{SessionCommand, SessionCommandReport}; use crate::input::state::{Toast, ToastPriority}; use crate::session::catalog; use anyhow::{Context, Error as AnyhowError, Result, anyhow}; @@ -294,28 +295,9 @@ impl WaylandState { fn handle_toolbar_open_session_path(&mut self, path: &Path) { self.clear_toolbar_save_as_overwrite_prompt(); - match self.open_named_session_runtime(path) { - Ok(report) => self.set_session_toolbar_info(format!( - "Opened session {}", - session_display_name(&report.opened_path) - )), - Err(err) if missing_session_error_matches_path(path, &err) => { - match self.forget_named_session_by_path(path.to_path_buf()) { - Ok(true) => self.set_session_toolbar_error(format!( - "Session file missing; removed from recent sessions: {}", - session_display_name(path) - )), - Ok(false) => self.set_session_toolbar_error(format!( - "Session file missing; no recent-session entry matched: {}", - session_display_name(path) - )), - Err(catalog_err) => self.set_session_toolbar_error(format!( - "Session file missing and recent-session cleanup failed for {}: {catalog_err:#}", - session_display_name(path) - )), - } - } - Err(err) => self.set_session_toolbar_error(format!("Open session failed: {err:#}")), + let command = SessionCommand::Open(path.to_path_buf()); + if let Err(error) = self.start_session_command(command) { + self.report_session_command_error("Open session failed", &error); } } @@ -372,20 +354,8 @@ impl WaylandState { } fn handle_selected_save_as_path(&mut self, path: PathBuf) { - match self.save_named_session_as_requires_overwrite(&path) { - Ok(true) => { - self.input_state.set_pending_save_as_overwrite(path.clone()); - self.set_session_toolbar_info(format!( - "Replace existing session {}?", - session_display_name(&path) - )); - } - Ok(false) => { - self.commit_toolbar_save_session_as(&path, crate::session::SaveAsOverwrite::Deny) - } - Err(err) => { - self.set_session_toolbar_error(format!("Save session failed: {err:#}")); - } + if let Err(error) = self.start_session_command(SessionCommand::CheckOverwrite(path)) { + self.report_session_command_error("Save session failed", &error); } } @@ -419,39 +389,137 @@ impl WaylandState { path: &Path, overwrite: crate::session::SaveAsOverwrite, ) { - match self.save_named_session_as_runtime(path, overwrite) { - Ok(report) => { - self.clear_toolbar_save_as_overwrite_prompt(); - self.set_session_toolbar_info(format!( - "Saved session as {}", - session_display_name(&report.saved_path) - )); - } - Err(err) => { - self.clear_toolbar_save_as_overwrite_prompt(); - self.set_session_toolbar_error(format!("Save session failed: {err:#}")); - } + if let Err(error) = + self.start_session_command(SessionCommand::SaveAs(path.to_path_buf(), overwrite)) + { + self.clear_toolbar_save_as_overwrite_prompt(); + self.report_session_command_error("Save session failed", &error); } } fn handle_toolbar_session_info(&mut self) { - match self.inspect_active_session() { - Ok(inspection) => self.set_session_toolbar_info(session_info_summary(&inspection)), - Err(err) => self.set_session_toolbar_error(format!("Session info failed: {err:#}")), + if let Err(error) = self.start_session_command(SessionCommand::Inspect) { + self.report_session_command_error("Session info failed", &error); } } fn handle_toolbar_clear_session(&mut self) { self.clear_toolbar_save_as_overwrite_prompt(); - match self.clear_current_session_runtime() { - Ok(report) => self.set_session_toolbar_info(format!( + if let Err(error) = self.start_session_command(SessionCommand::Clear) { + self.report_session_command_error("Clear session failed", &error); + } + } + + pub(in crate::backend::wayland) fn finish_session_command( + &mut self, + report: SessionCommandReport, + ) { + match report { + SessionCommandReport::Open(report) => self.set_session_toolbar_info(format!( + "Opened session {}", + session_display_name(&report.opened_path) + )), + SessionCommandReport::SaveAs(report) => { + self.clear_toolbar_save_as_overwrite_prompt(); + self.set_session_toolbar_info(format!( + "Saved session as {}", + session_display_name(&report.saved_path) + )); + } + SessionCommandReport::Overwrite(path, required) => { + if required { + self.input_state.set_pending_save_as_overwrite(path.clone()); + self.set_session_toolbar_info(format!( + "Replace existing session {}?", + session_display_name(&path) + )); + } else { + self.commit_toolbar_save_session_as( + &path, + crate::session::SaveAsOverwrite::Deny, + ); + } + } + SessionCommandReport::Clear(report) => self.set_session_toolbar_info(format!( "Cleared session {}", session_display_name(&report.cleared_path) )), - Err(err) => self.set_session_toolbar_error(format!("Clear session failed: {err:#}")), + SessionCommandReport::ClearTools(report) => { + let message = match report.outcome { + Some(crate::session::ClearToolStateOutcome::Cleared { + preserved_board_data: true, + }) => { + "Tool defaults reset from config. Saved boards and history were preserved." + } + Some(crate::session::ClearToolStateOutcome::Cleared { + preserved_board_data: false, + }) => "Tool defaults reset from config. No board data was present.", + Some(crate::session::ClearToolStateOutcome::NoToolState) => { + "Tool defaults reset from config. No saved tool state was stored." + } + Some(crate::session::ClearToolStateOutcome::NoSession) => { + "Tool defaults reset from config. No saved session file was present." + } + None => { + "Tool defaults reset from config for this run. No active session file to edit." + } + }; + self.set_session_toolbar_info(message); + } + SessionCommandReport::Inspection(inspection) => { + self.set_session_toolbar_info(session_info_summary(&inspection)) + } + SessionCommandReport::Forgotten(path, forgotten) => { + self.set_session_toolbar_error(format!( + "Session file missing; {}: {}", + if forgotten { + "removed from recent sessions" + } else { + "no recent-session entry matched" + }, + session_display_name(&path) + )) + } } } + pub(in crate::backend::wayland) fn fail_session_command( + &mut self, + command: &SessionCommand, + error: &AnyhowError, + ) { + if let SessionCommand::Open(path) = command + && missing_session_error_matches_path(path, error) + { + if let Err(catalog_error) = + self.start_session_command(SessionCommand::Forget(path.clone())) + { + self.report_session_command_error( + "Session file missing and recent-session cleanup failed", + &catalog_error, + ); + } + return; + } + let prefix = match command { + SessionCommand::Open(_) => "Open session failed", + SessionCommand::SaveAs(..) | SessionCommand::CheckOverwrite(_) => "Save session failed", + SessionCommand::Clear => "Clear session failed", + SessionCommand::ClearTools(_) => "Failed to reset tool defaults", + SessionCommand::Inspect => "Session info failed", + SessionCommand::Forget(_) => "Session file missing and recent-session cleanup failed", + }; + self.report_session_command_error(prefix, error); + } + + pub(in crate::backend::wayland) fn report_session_command_error( + &mut self, + prefix: &str, + error: &AnyhowError, + ) { + self.set_session_toolbar_error(format!("{prefix}: {error:#}")); + } + fn clear_toolbar_save_as_overwrite_prompt(&mut self) -> bool { let cleared = self.input_state.clear_pending_save_as_overwrite().is_some(); if cleared { diff --git a/src/input/state/actions/action_dispatch.rs b/src/input/state/actions/action_dispatch.rs index 13746c4e..c0f5b5b7 100644 --- a/src/input/state/actions/action_dispatch.rs +++ b/src/input/state/actions/action_dispatch.rs @@ -12,7 +12,9 @@ impl InputState { resources: crate::input::state::InputTextResources<'_>, action: Action, ) { + self.note_session_interaction_activity(); let _ = interaction::route_action_with_resources(self, resources, action); + self.note_session_interaction_activity(); } /// Handle an action for a control that sits away from the pointer, such as diff --git a/src/input/state/actions/key_press/mod.rs b/src/input/state/actions/key_press/mod.rs index 06e7d053..baa4cda4 100644 --- a/src/input/state/actions/key_press/mod.rs +++ b/src/input/state/actions/key_press/mod.rs @@ -57,7 +57,9 @@ impl InputState { resources: crate::input::state::InputTextResources<'_>, key: Key, ) { + self.note_session_interaction_activity(); let _ = interaction::route_key_press_with_resources(self, resources, key); + self.note_session_interaction_activity(); } pub fn on_key_repeat(&mut self, key: Key) { @@ -77,6 +79,8 @@ impl InputState { resources: crate::input::state::InputTextResources<'_>, key: Key, ) { + self.note_session_interaction_activity(); let _ = interaction::route_key_repeat_with_resources(self, resources, key); + self.note_session_interaction_activity(); } } diff --git a/src/input/state/actions/key_press/text_input.rs b/src/input/state/actions/key_press/text_input.rs index 7c170ddd..fb06a3b5 100644 --- a/src/input/state/actions/key_press/text_input.rs +++ b/src/input/state/actions/key_press/text_input.rs @@ -250,6 +250,7 @@ impl InputState { ) -> bool { let changed = self.text_editing.insert_text(&mut self.state, text); if changed { + self.note_session_interaction_activity(); self.needs_redraw = true; self.update_text_preview_dirty_from_editor_with(measurer); } diff --git a/src/input/state/core/ime.rs b/src/input/state/core/ime.rs index bf8389a1..f3dbe0bd 100644 --- a/src/input/state/core/ime.rs +++ b/src/input/state/core/ime.rs @@ -388,6 +388,7 @@ impl InputState { } pub(crate) fn ime_apply_done_with(&mut self, measurer: &crate::draw::TextMeasurer) -> bool { + self.note_session_interaction_activity(); let changed = self.text_editing.apply_ime_done(&mut self.state); if changed { self.needs_redraw = true; diff --git a/src/input/state/core/session.rs b/src/input/state/core/session.rs index 4ec864bf..576b0d12 100644 --- a/src/input/state/core/session.rs +++ b/src/input/state/core/session.rs @@ -151,6 +151,21 @@ impl InputState { || self.radial_menu_is_size_dragging() } + /// Transient edits have their own revision because they become session-dirty + /// only on release or text finalization. Disk completions must preserve them. + pub(crate) fn session_interaction_state(&self) -> (u64, bool) { + ( + self.session_flags.interaction_revision(), + self.has_cancelable_session_capture_interaction(), + ) + } + + pub(crate) fn note_session_interaction_activity(&mut self) { + if self.has_cancelable_session_capture_interaction() { + self.session_flags.note_interaction_activity(); + } + } + fn has_cancelable_session_capture_interaction(&self) -> bool { self.has_active_pointer_interaction() || matches!(self.state, DrawingState::TextInput { .. }) diff --git a/src/input/state/core/session_flags.rs b/src/input/state/core/session_flags.rs index 157ade9e..660f8398 100644 --- a/src/input/state/core/session_flags.rs +++ b/src/input/state/core/session_flags.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; #[derive(Debug, Clone)] pub(in crate::input::state) struct SessionFlags { dirty: bool, + interaction_revision: u64, preflight_options: Option, pending_save_as_overwrite: Option, last_capture_path: Option, @@ -15,12 +16,21 @@ impl SessionFlags { pub(in crate::input::state) fn new() -> Self { Self { dirty: false, + interaction_revision: 0, preflight_options: None, pending_save_as_overwrite: None, last_capture_path: None, } } + pub(in crate::input::state) const fn interaction_revision(&self) -> u64 { + self.interaction_revision + } + + pub(in crate::input::state) fn note_interaction_activity(&mut self) { + self.interaction_revision = self.interaction_revision.wrapping_add(1); + } + pub(in crate::input::state) const fn is_dirty(&self) -> bool { self.dirty } diff --git a/src/input/state/core/toolbar/apply/mod.rs b/src/input/state/core/toolbar/apply/mod.rs index 0a25fd7b..4d8e7f50 100644 --- a/src/input/state/core/toolbar/apply/mod.rs +++ b/src/input/state/core/toolbar/apply/mod.rs @@ -43,7 +43,9 @@ impl InputState { // can delete the arrow outright — after which the release finds no // shape and drops the bend without a trace. self.finish_active_arrow_bend(); + self.note_session_interaction_activity(); let changed = self.apply_toolbar_event_inner_with_resources(resources, event); + self.note_session_interaction_activity(); self.note_toolbar_shortcut_slow_path(coach_action, changed); changed } diff --git a/src/input/state/mouse/motion.rs b/src/input/state/mouse/motion.rs index 2c888227..a596dad9 100644 --- a/src/input/state/mouse/motion.rs +++ b/src/input/state/mouse/motion.rs @@ -56,7 +56,9 @@ impl InputState { ScreenPoint::new(screen_x, screen_y), CanvasPoint::new(canvas_x, canvas_y), ); + self.note_session_interaction_activity(); let _ = route_pointer_motion(self, resources.measurer, PointerMotion::new(points)); + self.note_session_interaction_activity(); } } diff --git a/src/input/state/mouse/press.rs b/src/input/state/mouse/press.rs index 237b1310..6b326034 100644 --- a/src/input/state/mouse/press.rs +++ b/src/input/state/mouse/press.rs @@ -160,7 +160,9 @@ impl InputState { ScreenPoint::new(screen_x, screen_y), CanvasPoint::new(canvas_x, canvas_y), ); + self.note_session_interaction_activity(); let _ = route_pointer_press(self, resources, PointerPress::new(button, points)); + self.note_session_interaction_activity(); } pub(in crate::input::state) fn tool_for_button_press( diff --git a/src/input/state/mouse/release/mod.rs b/src/input/state/mouse/release/mod.rs index e4a5fe89..4933343d 100644 --- a/src/input/state/mouse/release/mod.rs +++ b/src/input/state/mouse/release/mod.rs @@ -73,7 +73,9 @@ impl InputState { ScreenPoint::new(screen_x, screen_y), CanvasPoint::new(canvas_x, canvas_y), ); + self.note_session_interaction_activity(); let _ = route_pointer_release(self, resources, PointerRelease::new(button, points)); + self.note_session_interaction_activity(); } pub(in crate::input::state) fn handle_color_picker_popup_release_at( From f8a16ee9fe3c0b7a382044c145cf89a6ee1021f5 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Fri, 2 Oct 2026 11:27:02 +0200 Subject: [PATCH 3/6] test(session): exercise production command lifecycle through a shared runtime driver --- .../backend/event_loop/session_save.rs | 55 +- src/backend/wayland/session.rs | 1 + src/backend/wayland/session/driver.rs | 215 +++++++ src/backend/wayland/session/driver/tests.rs | 548 ++++++++++++++++++ src/backend/wayland/session/persistence.rs | 61 ++ src/backend/wayland/session/tests.rs | 57 +- src/backend/wayland/state/core/session.rs | 198 ++----- 7 files changed, 913 insertions(+), 222 deletions(-) create mode 100644 src/backend/wayland/session/driver.rs create mode 100644 src/backend/wayland/session/driver/tests.rs diff --git a/src/backend/wayland/backend/event_loop/session_save.rs b/src/backend/wayland/backend/event_loop/session_save.rs index 4a6c6c33..d7c74b70 100644 --- a/src/backend/wayland/backend/event_loop/session_save.rs +++ b/src/backend/wayland/backend/event_loop/session_save.rs @@ -2,7 +2,7 @@ use super::super::super::state::WaylandState; use crate::{ backend::wayland::session::{ self as runtime_session, PersistenceCompletion, PersistenceOperation, PersistenceOutcome, - SaveCompletion, SaveStrategy, SessionState, SubmitFailure, + SaveStrategy, SessionState, SubmitFailure, }, session, session::SaveSnapshotReport, @@ -37,12 +37,13 @@ pub(super) fn persist_session(state: &mut WaylandState) -> Result<(), anyhow::Er state.session.target_epoch() ); } - if let Err(error) = state.finish_pending_session_command() { - if let Some(transaction) = state.session_transaction.take() { - state.fail_session_command(transaction.command(), &error); - } - log::warn!("Explicit session command failed during shutdown: {error:#}"); - } + runtime_session::driver::persist_after_pending_commands( + state, + persist_final_session_and_shutdown, + ) +} + +fn persist_final_session_and_shutdown(state: &mut WaylandState) -> Result<(), anyhow::Error> { let save_result = persist_final_session(state); let worker_failed = !state.persistence.is_healthy(); let shutdown_result = state.persistence.shutdown(state.session.target_epoch()); @@ -477,47 +478,23 @@ pub(in crate::backend::wayland) fn drain_persistence_completion_for_runtime( Ok(()) } -fn apply_persistence_completion( +pub(in crate::backend::wayland) fn apply_persistence_completion( state: &mut WaylandState, completion: PersistenceCompletion, ) -> Result<(), anyhow::Error> { - observe_input_dirty(state, Instant::now()); - if state - .session_transaction - .as_ref() - .is_some_and(|transaction| transaction.request_id == Some(completion.id)) - { - state.complete_session_command(completion); - return Ok(()); - } - let id = completion.id; - let save_result: Result = match completion.result { - Ok(PersistenceOutcome::Save(save)) => Ok(save), - Ok(other) => Err(anyhow::anyhow!( - "unexpected asynchronous persistence outcome: {other:?}" - )), - Err(err) => Err(err), - }; - let completed_at = Instant::now(); - let committed = state - .session - .complete_autosave(id, completed_at, &save_result)?; - match save_result { - Ok(save) if committed => { + let execution_time = completion.execution_time; + match runtime_session::driver::route_session_completion(state, completion) { + Ok(Some(save)) => { log_session_save_result( SessionSaveReason::Autosave, save.report.as_ref(), - completion.execution_time, + execution_time, ); notify_session_save_report(state, save.report.as_ref()); } - Ok(_) => { - let err = anyhow::anyhow!("autosave worker completed without writing session data"); - handle_autosave_failure(state, completed_at, &err); - return Err(err); - } + Ok(None) => {} Err(err) => { - handle_autosave_failure(state, completed_at, &err); + handle_autosave_failure(state, Instant::now(), &err); return Err(err); } } @@ -534,7 +511,7 @@ fn handle_autosave_failure(state: &mut WaylandState, now: Instant, err: &anyhow: } } -fn handle_persistence_transport_failure( +pub(in crate::backend::wayland) fn handle_persistence_transport_failure( state: &mut WaylandState, now: Instant, err: &anyhow::Error, diff --git a/src/backend/wayland/session.rs b/src/backend/wayland/session.rs index 9905e61d..3ea2280f 100644 --- a/src/backend/wayland/session.rs +++ b/src/backend/wayland/session.rs @@ -578,6 +578,7 @@ fn autosave_active(options: &SessionOptions) -> bool { && (options.any_enabled() || options.restore_tool_state || options.persist_history) } +pub(in crate::backend::wayland) mod driver; mod persistence; mod runtime; diff --git a/src/backend/wayland/session/driver.rs b/src/backend/wayland/session/driver.rs new file mode 100644 index 00000000..066f589d --- /dev/null +++ b/src/backend/wayland/session/driver.rs @@ -0,0 +1,215 @@ +//! Live explicit-command orchestration, shared by Wayland and headless runtime tests. +use anyhow::{Result, anyhow}; +use std::time::Instant; + +use super::{ + ExplicitSessionTransaction, PersistenceCompletion, PersistenceController, PersistenceOutcome, + SaveCompletion, SessionCommand, SessionCommandReport, SessionTransaction, TransactionStep, +}; + +/// The driver owns ordering and identity; adapters own UI publication and autosave feedback. +pub(in crate::backend::wayland) trait SessionCommandRuntime { + fn session_context(&mut self) -> SessionTransaction<'_>; + fn pending_command(&mut self) -> &mut Option; + fn persistence(&mut self) -> &mut PersistenceController; + fn session_config_failed(&self) -> bool; + fn refresh_session_ui_seeds(&mut self); + fn finish_session_command(&mut self, report: SessionCommandReport); + fn fail_session_command(&mut self, command: &SessionCommand, error: &anyhow::Error); + fn apply_session_completion(&mut self, completion: PersistenceCompletion) -> Result<()>; + fn session_transport_failed(&mut self, error: &anyhow::Error); +} + +pub(in crate::backend::wayland) fn observe_input_dirty(runtime: &mut impl SessionCommandRuntime) { + let context = runtime.session_context(); + let dirty = context.input_state.take_session_dirty(); + context.session.record_input_dirty(Instant::now(), dirty); +} + +pub(in crate::backend::wayland) fn start_session_command( + runtime: &mut impl SessionCommandRuntime, + command: SessionCommand, +) -> Result<()> { + if runtime.pending_command().is_some() { + return Err(anyhow!("another session command is already pending")); + } + if matches!( + command, + SessionCommand::Clear | SessionCommand::ClearTools(_) + ) && runtime.session_config_failed() + { + return Err(anyhow!( + "config.toml [session] could not be read; refusing to modify saved session data that default settings may mistarget - fix the section and retry" + )); + } + if !runtime.persistence().is_healthy() { + return Err(anyhow!("session persistence worker is unhealthy")); + } + + let context = runtime.session_context(); + let transaction = ExplicitSessionTransaction::new( + command, + context.session.target_epoch(), + context.input_state.session_interaction_state(), + ); + *runtime.pending_command() = Some(transaction); + poll_pending_session_command(runtime); + Ok(()) +} + +pub(in crate::backend::wayland) fn poll_pending_session_command( + runtime: &mut impl SessionCommandRuntime, +) { + if runtime.persistence().is_active() { + return; + } + let Some(transaction) = runtime.pending_command().take() else { + return; + }; + advance_session_command(runtime, transaction, None); +} + +pub(in crate::backend::wayland) fn complete_session_command( + runtime: &mut impl SessionCommandRuntime, + completion: PersistenceCompletion, +) { + let Some(transaction) = runtime.pending_command().take() else { + return; + }; + if transaction.request_id != Some(completion.id) { + runtime.fail_session_command( + transaction.command(), + &anyhow!("explicit session completion identity mismatch"), + ); + return; + } + advance_session_command(runtime, transaction, Some(completion.result)); +} + +fn advance_session_command( + runtime: &mut impl SessionCommandRuntime, + mut transaction: ExplicitSessionTransaction, + result: Option>, +) { + observe_input_dirty(runtime); + // Catalog failure follows an already committed open; never roll back that canvas. + let step = if let Some(Err(error)) = result.as_ref() + && let Some(report) = transaction.accept_catalog_failure(error) + { + Ok(TransactionStep::Complete(Box::new(report))) + } else { + transaction.advance(&mut runtime.session_context(), result) + }; + + match step { + Ok(TransactionStep::Work(operation)) => { + if transaction.has_committed_open() { + runtime.refresh_session_ui_seeds(); + } + let epoch = runtime.session_context().session.target_epoch(); + match runtime.persistence().try_submit(epoch, *operation) { + Ok(id) => { + transaction.request_id = Some(id); + *runtime.pending_command() = Some(transaction); + } + Err(failure) => { + let error = anyhow!("failed to submit session command: {}", failure.error); + if let Some(report) = transaction.accept_catalog_failure(&error) { + runtime.finish_session_command(report); + } else { + runtime.fail_session_command(transaction.command(), &error); + } + } + } + } + Ok(TransactionStep::Complete(report)) => { + if matches!( + *report, + SessionCommandReport::Open(_) | SessionCommandReport::Clear(_) + ) { + runtime.refresh_session_ui_seeds(); + } + runtime.finish_session_command(*report); + } + Err(error) => runtime.fail_session_command(transaction.command(), &error), + } +} + +/// Route autosave receipts separately from commands queued behind them. Once a +/// command has submitted work, its completion must pass the explicit identity gate. +pub(in crate::backend::wayland) fn route_session_completion( + runtime: &mut impl SessionCommandRuntime, + completion: PersistenceCompletion, +) -> Result> { + observe_input_dirty(runtime); + if runtime + .pending_command() + .as_ref() + .is_some_and(|command| command.request_id.is_some()) + { + complete_session_command(runtime, completion); + return Ok(None); + } + + let save_result = match completion.result { + Ok(PersistenceOutcome::Save(save)) => Ok(save), + Ok(other) => Err(anyhow!( + "unexpected asynchronous persistence outcome: {other:?}" + )), + Err(error) => Err(error), + }; + let committed = runtime.session_context().session.complete_autosave( + completion.id, + Instant::now(), + &save_result, + )?; + let save = save_result?; + if !committed { + return Err(anyhow!( + "autosave worker completed without writing session data" + )); + } + Ok(Some(save)) +} + +/// Deliberate durability barrier, never called from normal dispatch. +pub(in crate::backend::wayland) fn finish_pending_session_command( + runtime: &mut impl SessionCommandRuntime, +) -> Result<()> { + while runtime.pending_command().is_some() { + if runtime.persistence().is_active() { + let completion = match runtime.persistence().wait_for_completion() { + Ok(Some(completion)) => completion, + Ok(None) => return Err(anyhow!("active persistence request had no completion")), + Err(error) => { + runtime.session_transport_failed(&error); + return Err(error); + } + }; + runtime.apply_session_completion(completion)?; + } else { + poll_pending_session_command(runtime); + } + if !runtime.persistence().is_healthy() { + return Err(anyhow!("session persistence worker is unhealthy")); + } + } + Ok(()) +} + +/// Shutdown must clear a failed command before attempting final persistence. +pub(in crate::backend::wayland) fn persist_after_pending_commands( + runtime: &mut R, + persist: impl FnOnce(&mut R) -> Result, +) -> Result { + if let Err(error) = finish_pending_session_command(runtime) { + if let Some(transaction) = runtime.pending_command().take() { + runtime.fail_session_command(transaction.command(), &error); + } + log::warn!("Explicit session command failed during shutdown: {error:#}"); + } + persist(runtime) +} + +#[cfg(test)] +pub(super) mod tests; diff --git a/src/backend/wayland/session/driver/tests.rs b/src/backend/wayland/session/driver/tests.rs new file mode 100644 index 00000000..91e34d43 --- /dev/null +++ b/src/backend/wayland/session/driver/tests.rs @@ -0,0 +1,548 @@ +use super::*; +use crate::backend::wayland::session::{ + PersistenceOperation, SaveStrategy, SessionState, + tests::{add_line, loaded_line_x2, named_options, sample_snapshot, test_input_state}, +}; +use crate::{config::Config, draw::TextMeasurer, input::InputState, session as stored_session}; + +pub(in crate::backend::wayland::session) struct CommandRuntime<'a> { + pub input: &'a mut InputState, + measurer: &'a TextMeasurer, + pub session: &'a mut SessionState, + pub persistence: PersistenceController, + pending: Option, + config: Config, + config_failed: bool, + reports: Vec, + errors: Vec, + seed_refreshes: usize, + autosaves: usize, +} + +impl<'a> CommandRuntime<'a> { + pub fn new( + input: &'a mut InputState, + measurer: &'a TextMeasurer, + session: &'a mut SessionState, + persistence: PersistenceController, + ) -> Self { + Self { + input, + measurer, + session, + persistence, + pending: None, + config: Config::default(), + config_failed: false, + reports: Vec::new(), + errors: Vec::new(), + seed_refreshes: 0, + autosaves: 0, + } + } + + pub fn into_result(mut self) -> Result { + if let Some(error) = self.errors.pop() { + return Err(error); + } + self.reports + .pop() + .ok_or_else(|| anyhow!("command did not publish a terminal report")) + } + + fn receive(&mut self) { + let completion = self.persistence.wait_for_completion().unwrap().unwrap(); + self.apply_session_completion(completion).unwrap(); + } +} + +impl SessionCommandRuntime for CommandRuntime<'_> { + fn session_context(&mut self) -> SessionTransaction<'_> { + SessionTransaction { + input_state: self.input, + measurer: self.measurer, + session: self.session, + } + } + fn pending_command(&mut self) -> &mut Option { + &mut self.pending + } + fn persistence(&mut self) -> &mut PersistenceController { + &mut self.persistence + } + fn session_config_failed(&self) -> bool { + self.config_failed + } + fn refresh_session_ui_seeds(&mut self) { + self.seed_refreshes += 1; + self.input + .boards + .sync_pin_seeds_from_config(&self.config.resolved_boards()); + } + fn finish_session_command(&mut self, report: SessionCommandReport) { + self.reports.push(report); + } + fn fail_session_command(&mut self, _: &SessionCommand, error: &anyhow::Error) { + self.errors.push(anyhow!("{error:#}")); + } + fn session_transport_failed(&mut self, _: &anyhow::Error) { + self.session.restore_in_flight_autosave(); + } + + fn apply_session_completion(&mut self, completion: PersistenceCompletion) -> Result<()> { + if route_session_completion(self, completion)?.is_some() { + self.autosaves += 1; + } + Ok(()) + } +} + +#[test] +fn admission_rejects_each_guard_without_submitting_or_retargeting() { + for case in ["pending", "unhealthy", "clear-config", "tools-config"] { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "current"); + let mut input = test_input_state(); + let mut session = SessionState::new(Some(options.clone())); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + let mut worker = Some(worker); + let command = match case { + "pending" => { + start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); + worker.as_ref().unwrap().complete_next(); + // Receipt is held before runtime delivery; the original identity must survive. + SessionCommand::Clear + } + "unhealthy" => { + drop(worker.take()); + assert!( + runtime + .persistence + .try_submit( + 0, + PersistenceOperation::HasArtifacts { + options: options.clone() + } + ) + .is_err() + ); + SessionCommand::Inspect + } + "clear-config" => { + runtime.config_failed = true; + SessionCommand::Clear + } + "tools-config" => { + runtime.config_failed = true; + SessionCommand::ClearTools(Box::new( + stored_session::ToolStateSnapshot::from_config(&runtime.config), + )) + } + _ => unreachable!(), + }; + let before = runtime + .pending + .as_ref() + .and_then(|pending| pending.request_id); + let error = start_session_command(&mut runtime, command) + .unwrap_err() + .to_string(); + let expected = match case { + "pending" => "already pending", + "unhealthy" => "unhealthy", + _ => "refusing to modify saved session data", + }; + assert!(error.contains(expected), "{case}: {error}"); + assert_eq!( + runtime + .pending + .as_ref() + .and_then(|pending| pending.request_id), + before + ); + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + options.session_file_path() + ); + assert!(runtime.reports.is_empty()); + if let Some(worker) = worker { + assert!(!worker.has_request()); + } + if case == "pending" { + runtime.receive(); + assert!(matches!( + runtime.reports.as_slice(), + [SessionCommandReport::Inspection(_)] + )); + } + } +} + +#[test] +fn explicit_command_waits_for_autosave_receipt_and_survives_dispatch_work() { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "current"); + let mut input = test_input_state(); + let mut session = SessionState::new(Some(options.clone())); + session.record_input_dirty(Instant::now(), true); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + let window = runtime.session.prepare_autosave_submission().unwrap(); + let autosave_id = runtime + .persistence + .try_submit( + 0, + PersistenceOperation::Save { + snapshot: sample_snapshot(), + options, + strategy: SaveStrategy::Autosave, + contentless_clear_boundary: false, + }, + ) + .unwrap(); + runtime + .session + .commit_autosave_submission(autosave_id, window); + start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); + poll_pending_session_command(&mut runtime); + assert_eq!(runtime.pending.as_ref().unwrap().request_id, None); + assert!(runtime.reports.is_empty()); + assert_eq!(runtime.session.edit_generation(), 1); + // The worker has received no completion yet; unrelated live input still progresses. + add_line(runtime.input, 77); + runtime.input.mark_session_dirty(); + worker.complete_next(); + runtime.receive(); + assert_eq!(runtime.autosaves, 1); + assert_eq!(runtime.pending.as_ref().unwrap().request_id, None); + assert!(!worker.has_request()); + assert!(runtime.session.is_dirty()); + poll_pending_session_command(&mut runtime); + let explicit_id = runtime.pending.as_ref().unwrap().request_id.unwrap(); + assert_ne!(explicit_id, autosave_id); + worker.complete_next(); + runtime.receive(); + assert!(runtime.pending.is_none()); + assert!(matches!( + runtime.reports.as_slice(), + [SessionCommandReport::Inspection(_)] + )); +} + +#[test] +fn live_completion_gate_rejects_a_different_request_identity() { + let mut input = test_input_state(); + let mut session = SessionState::new(None); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + // No active session makes Inspect complete inline; use a disk-bound command instead. + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "current"); + *runtime.session = SessionState::new(Some(options)); + start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); + worker.complete_next(); + let mut completion = runtime.persistence.wait_for_completion().unwrap().unwrap(); + completion.id.sequence += 1; + runtime.apply_session_completion(completion).unwrap(); + assert!(runtime.pending.is_none()); + assert!(runtime.reports.is_empty()); + assert!(runtime.errors[0].to_string().contains("identity mismatch")); + assert!(!worker.has_request()); +} + +#[test] +fn advance_observes_live_edits_and_finalizes_stale_destructive_work() { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "clear"); + stored_session::save_snapshot(&sample_snapshot(), &options).unwrap(); + let mut input = test_input_state(); + add_line(&mut input, 51); + let mut session = SessionState::new(Some(options)); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + start_session_command(&mut runtime, SessionCommand::Clear).unwrap(); + add_line(runtime.input, 88); + runtime.input.mark_session_dirty(); + worker.complete_next(); + let completion = runtime.persistence.wait_for_completion().unwrap().unwrap(); + // Exercise advance's own dirty observation, not the completion router's observation. + complete_session_command(&mut runtime, completion); + assert!(runtime.pending.is_none()); + assert_eq!(runtime.input.boards.active_frame().shapes.len(), 2); + assert!(runtime.session.is_dirty()); + assert_eq!(runtime.session.edit_generation(), 1); + assert!(runtime.errors[0].to_string().contains("edited while")); + assert!(runtime.reports.is_empty()); +} + +#[test] +fn deferred_submission_rejection_is_terminal() { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "current"); + let mut input = test_input_state(); + let mut session = SessionState::new(Some(options.clone())); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + runtime + .persistence + .try_submit(0, PersistenceOperation::HasArtifacts { options }) + .unwrap(); + start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); + worker.complete_next(); + runtime.persistence.wait_for_completion().unwrap().unwrap(); + drop(worker); + poll_pending_session_command(&mut runtime); + assert!(runtime.pending.is_none()); + assert!(runtime.reports.is_empty()); + assert!(runtime.errors[0].to_string().contains("failed to submit")); +} + +#[test] +fn advance_failure_publishes_error_and_retires_the_command() { + let mut input = test_input_state(); + let mut session = SessionState::new(None); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); + assert!(runtime.pending.is_none()); + assert!(runtime.reports.is_empty()); + assert!( + runtime.errors[0] + .to_string() + .contains("no active persisted session target") + ); + assert!(!worker.has_request()); +} + +#[test] +fn open_refreshes_consumer_seeds_before_catalog_work_and_at_completion() { + for catalog in ["success", "failure", "rejected"] { + let temp = crate::test_temp::tempdir().unwrap(); + let current = named_options(temp.path(), "current"); + let target = named_options(temp.path(), "target"); + stored_session::save_snapshot(&sample_snapshot(), &target).unwrap(); + let mut input = test_input_state(); + let mut session = SessionState::new(Some(current)); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + runtime.config.boards = Some(runtime.input.boards.to_config()); + for item in &mut runtime.config.boards.as_mut().unwrap().items { + item.pinned = true; + } + start_session_command( + &mut runtime, + SessionCommand::Open(target.session_file_path()), + ) + .unwrap(); + worker.complete_next(); // open preflight + runtime.receive(); + worker.complete_next(); // candidate load, no dirty current source + let completion = runtime.persistence.wait_for_completion().unwrap().unwrap(); + let worker = if catalog == "rejected" { + drop(worker); + None + } else { + Some(worker) + }; + runtime.apply_session_completion(completion).unwrap(); + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + target.session_file_path() + ); + assert_eq!(runtime.input.boards.active_frame().shapes.len(), 1); + assert!( + runtime + .input + .boards + .to_config() + .items + .iter() + .all(|item| item.pinned) + ); + assert_eq!(runtime.seed_refreshes, 1); + if let Some(worker) = worker { + // Make a changed authored seed observable on the terminal refresh too. + for item in &mut runtime.config.boards.as_mut().unwrap().items { + item.pinned = false; + } + if catalog == "failure" { + worker.respond_with(|_| Err(anyhow!("controlled catalog error"))); + } else { + worker.complete_next(); + } + runtime.receive(); + assert_eq!(runtime.seed_refreshes, 2); + assert!( + runtime + .input + .boards + .to_config() + .items + .iter() + .all(|item| !item.pinned) + ); + } + assert!(runtime.pending.is_none()); + assert!(runtime.errors.is_empty()); + assert!(matches!( + runtime.reports.as_slice(), + [SessionCommandReport::Open(_)] + )); + } +} + +#[test] +fn clear_completion_refreshes_consumer_seeds() { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "clear"); + let mut input = test_input_state(); + add_line(&mut input, 51); + let mut session = SessionState::new(Some(options)); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + runtime.config.boards = Some(runtime.input.boards.to_config()); + for item in &mut runtime.config.boards.as_mut().unwrap().items { + item.pinned = true; + } + start_session_command(&mut runtime, SessionCommand::Clear).unwrap(); + worker.complete_next(); + runtime.receive(); + assert!(runtime.input.boards.active_frame().shapes.is_empty()); + assert!( + runtime + .input + .boards + .to_config() + .items + .iter() + .all(|item| item.pinned) + ); + assert!(matches!( + runtime.reports.as_slice(), + [SessionCommandReport::Clear(_)] + )); +} + +#[test] +fn shutdown_drains_save_as_and_open_at_every_disk_phase_before_final_save() { + for (open, phases) in [(false, 2), (true, 4)] { + for completed in 0..phases { + let temp = crate::test_temp::tempdir().unwrap(); + let current = named_options(temp.path(), "current"); + let target = named_options(temp.path(), "target"); + if open { + stored_session::save_snapshot(&sample_snapshot(), &target).unwrap(); + } + let mut input = test_input_state(); + add_line(&mut input, 51); + input.mark_session_dirty(); + let mut session = SessionState::new(Some(current.clone())); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + let command = if open { + SessionCommand::Open(target.session_file_path()) + } else { + SessionCommand::SaveAs( + target.session_file_path(), + stored_session::SaveAsOverwrite::Deny, + ) + }; + start_session_command(&mut runtime, command).unwrap(); + for _ in 0..completed { + worker.complete_next(); + runtime.receive(); + } + assert!(runtime.pending.is_some()); + std::thread::scope(|scope| { + scope.spawn(move || { + for _ in completed..phases { + worker.complete_next(); + } + }); + persist_after_pending_commands(&mut runtime, |runtime| { + assert!(runtime.pending.is_none()); + assert!(runtime.errors.is_empty()); + assert_eq!(runtime.reports.len(), 1); + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + target.session_file_path() + ); + add_line(runtime.input, 99); + runtime.input.mark_session_dirty(); + let snapshot = runtime + .input + .snapshot_for_persistence_with(&measurer, &target) + .unwrap(); + stored_session::save_snapshot(&snapshot, runtime.session.options().unwrap())?; + Ok(()) + }) + .unwrap(); + }); + assert!(runtime.pending.is_none()); + assert!(runtime.errors.is_empty()); + assert_eq!(runtime.reports.len(), 1); + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + target.session_file_path() + ); + assert_eq!(loaded_line_x2(&target), if open { 42 } else { 51 }); + let loaded = stored_session::load_snapshot(&target).unwrap().unwrap(); + assert_eq!(loaded.boards[0].pages.pages[0].shapes.len(), 2); + if open { + assert_eq!(loaded_line_x2(¤t), 51); + } + } + } +} + +#[test] +fn shutdown_cleans_up_pending_work_after_worker_disconnect_and_when_ready_to_poll() { + for disconnect in [true, false] { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "current"); + let mut input = test_input_state(); + let mut session = SessionState::new(Some(options.clone())); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + // Queue explicit work behind an active worker; delivering the unrelated receipt + // leaves the command ready to poll rather than already submitted. + runtime + .persistence + .try_submit(0, PersistenceOperation::HasArtifacts { options }) + .unwrap(); + start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); + if disconnect { + drop(worker); + persist_after_pending_commands(&mut runtime, |runtime| { + assert!(runtime.pending.is_none()); + assert!(runtime.errors[0].to_string().contains("disconnected")); + Ok(()) + }) + .unwrap(); + } else { + worker.complete_next(); + runtime.persistence.wait_for_completion().unwrap().unwrap(); + std::thread::scope(|scope| { + scope.spawn(move || worker.complete_next()); + persist_after_pending_commands(&mut runtime, |runtime| { + assert!(runtime.pending.is_none()); + assert_eq!(runtime.reports.len(), 1); + Ok(()) + }) + .unwrap(); + }); + } + assert!(runtime.pending.is_none()); + } +} diff --git a/src/backend/wayland/session/persistence.rs b/src/backend/wayland/session/persistence.rs index 631c7e2b..3392fa09 100644 --- a/src/backend/wayland/session/persistence.rs +++ b/src/backend/wayland/session/persistence.rs @@ -214,6 +214,27 @@ impl PersistenceController { Self::start(wake.handle()) } + #[cfg(test)] + pub(in crate::backend::wayland) fn controlled_for_test() -> (Self, ControlledPersistenceWorker) + { + let (request_tx, requests) = mpsc::sync_channel(1); + let (completions, completion_rx) = mpsc::sync_channel(1); + ( + Self { + request_tx: Some(request_tx), + completion_rx, + worker: None, + active_id: None, + next_sequence: 0, + healthy: true, + }, + ControlledPersistenceWorker { + requests, + completions, + }, + ) + } + pub(in crate::backend::wayland) fn is_active(&self) -> bool { self.active_id.is_some() } @@ -433,6 +454,46 @@ impl Drop for PersistenceController { } } +/// Channel peer for driver tests: it controls delivery, not admission or phase ordering. +#[cfg(test)] +pub(in crate::backend::wayland) struct ControlledPersistenceWorker { + requests: Receiver, + completions: SyncSender, +} + +#[cfg(test)] +impl ControlledPersistenceWorker { + pub(in crate::backend::wayland) fn complete_next(&self) { + self.respond_with(execute); + } + + pub(in crate::backend::wayland) fn respond_with( + &self, + result: impl FnOnce(PersistenceOperation) -> Result, + ) { + let request = self + .requests + .recv_timeout(Duration::from_secs(5)) + .expect("driver submitted work"); + let completion = PersistenceCompletion { + id: request.id, + result: result(request.operation), + queue_wait: Duration::ZERO, + execution_time: Duration::ZERO, + worker_thread_id: thread::current().id(), + finished_at: Instant::now(), + }; + self.completions.send(completion).unwrap(); + } + + pub(in crate::backend::wayland) fn has_request(&self) -> bool { + match self.requests.try_recv() { + Err(TryRecvError::Empty) => false, + other => panic!("unexpected worker request: {other:?}"), + } + } +} + fn worker_main( request_rx: Receiver, completion_tx: SyncSender, diff --git a/src/backend/wayland/session/tests.rs b/src/backend/wayland/session/tests.rs index 78c16d6e..18e14af7 100644 --- a/src/backend/wayland/session/tests.rs +++ b/src/backend/wayland/session/tests.rs @@ -101,7 +101,7 @@ fn can_create_probe(parent: &Path) -> bool { } } -fn test_input_state() -> InputState { +pub(super) fn test_input_state() -> InputState { let mut action_map = HashMap::new(); action_map.insert(Shortcut::parse("Escape").unwrap(), Action::Exit); crate::input::state::test_support::TestInputStateBuilder::default() @@ -112,7 +112,7 @@ fn test_input_state() -> InputState { .build() } -fn add_line(input: &mut InputState, x2: i32) -> ShapeId { +pub(super) fn add_line(input: &mut InputState, x2: i32) -> ShapeId { input.boards.active_frame_mut().add_shape(Shape::Line { x1: 0, y1: 0, @@ -141,7 +141,7 @@ fn board_shape_count(input: &InputState, id: &str) -> usize { .len() } -fn sample_snapshot() -> stored_session::SessionSnapshot { +pub(super) fn sample_snapshot() -> stored_session::SessionSnapshot { snapshot_for_board("transparent", 42) } @@ -212,7 +212,7 @@ fn sample_tool_state() -> stored_session::ToolStateSnapshot { } } -fn named_options(base: &Path, name: &str) -> SessionOptions { +pub(super) fn named_options(base: &Path, name: &str) -> SessionOptions { let mut options = SessionOptions::new(base.join("configured"), name); options.persist_transparent = true; options.set_named_file_target(base.join(format!("{name}.wayscriber-session"))); @@ -236,7 +236,7 @@ fn assert_no_candidate_sidecars(options: &SessionOptions) { } } -fn loaded_line_x2(options: &SessionOptions) -> i32 { +pub(super) fn loaded_line_x2(options: &SessionOptions) -> i32 { let outcome = stored_session::load_snapshot_with_outcome(options).expect("load session"); let LoadSnapshotOutcome::Loaded(snapshot) = outcome else { panic!("expected loaded snapshot, got {outcome:?}"); @@ -334,7 +334,7 @@ fn runtime_save_as_rejects_existing_target_without_confirmation() { .map(SessionOptions::session_file_path), Some(current_options.session_file_path()) ); - assert!(input.is_session_dirty()); + assert!(session_state.is_dirty()); assert_eq!( std::fs::read(target_options.session_file_path()).expect("target unchanged"), before @@ -374,7 +374,7 @@ fn runtime_save_as_rejects_existing_sidecar_without_confirmation() { .map(SessionOptions::session_file_path), Some(current_options.session_file_path()) ); - assert!(input.is_session_dirty()); + assert!(session_state.is_dirty()); assert!(target_options.clear_marker_file_path().exists()); assert!(!target_options.session_file_path().exists()); } @@ -652,7 +652,6 @@ fn runtime_clear_persistence_failure_leaves_live_session_unchanged() { assert!(format!("{err:#}").contains("symlink"), "{err:#}"); assert_eq!(input.boards.active_frame().shapes.len(), 1); - assert!(input.is_session_dirty()); assert!(session_state.is_dirty()); assert!(session_state.has_loaded_board_data()); assert_eq!( @@ -1421,7 +1420,7 @@ fn runtime_open_current_save_failure_preserves_active_selection_move() { panic!("expected line"); }; assert_eq!((*x1, *y1, *x2, *y2), (100, 0, 109, 10)); - assert!(input.is_session_dirty()); + assert!(session_state.is_dirty()); assert_eq!( std::fs::read(¤t_target).expect("current target bytes"), b"preserve current target" @@ -1669,8 +1668,7 @@ fn runtime_open_current_save_failure_aborts_before_candidate_load() { .map(SessionOptions::session_file_path), Some(current_options.session_file_path()) ); - assert!(input.is_session_dirty()); - assert!(!session_state.is_dirty()); + assert!(session_state.is_dirty()); assert_eq!( std::fs::read(¤t_target).expect("current target bytes"), b"preserve current target" @@ -2014,34 +2012,15 @@ fn drive_session_command( session: &mut SessionState, command: SessionCommand, ) -> Result { - let mut persistence = PersistenceController::start_for_test()?; - let mut transaction = ExplicitSessionTransaction::new( - command, - session.target_epoch(), - input.session_interaction_state(), + let mut runtime = super::driver::tests::CommandRuntime::new( + input, + measurer, + session, + PersistenceController::start_for_test()?, ); - let mut result = None; - loop { - if let Some(Err(error)) = result.as_ref() - && let Some(report) = transaction.accept_catalog_failure(error) - { - return Ok(report); - } - let step = transaction.advance( - &mut SessionTransaction { - input_state: input, - measurer, - session, - }, - result, - )?; - match step { - TransactionStep::Work(operation) => { - result = Some(persistence.run(session.target_epoch(), *operation)) - } - TransactionStep::Complete(report) => return Ok(*report), - } - } + super::driver::start_session_command(&mut runtime, command)?; + super::driver::finish_pending_session_command(&mut runtime)?; + runtime.into_result() } fn open_named_session_runtime( @@ -2292,7 +2271,7 @@ fn runtime_open_validates_target_before_saving_dirty_current_session() { !current.session_file_path().exists(), "invalid target must be rejected before saving the source" ); - assert!(input.is_session_dirty()); + assert!(session.is_dirty()); assert_eq!(input.boards.active_frame().shapes.len(), 1); assert_eq!( session.options().unwrap().session_file_path(), diff --git a/src/backend/wayland/state/core/session.rs b/src/backend/wayland/state/core/session.rs index 5be2008f..7f96d66f 100644 --- a/src/backend/wayland/state/core/session.rs +++ b/src/backend/wayland/state/core/session.rs @@ -1,167 +1,77 @@ -use anyhow::{Result, anyhow}; -use std::time::Instant; +use anyhow::Result; use super::super::*; -use crate::backend::wayland::{ - backend::event_loop::session_save, - session::{ - ExplicitSessionTransaction, PersistenceCompletion, SessionCommand, SessionCommandReport, - SessionTransaction, TransactionStep, - }, +use crate::backend::wayland::session::{ + ExplicitSessionTransaction, PersistenceCompletion, PersistenceController, SessionCommand, + SessionCommandReport, SessionTransaction, + driver::{self, SessionCommandRuntime}, }; use crate::session::ToolStateSnapshot; -impl WaylandState { - /// Admit one explicit command. An existing autosave finishes first without - /// waiting in dispatch; the command captures its live input when admitted. - pub(in crate::backend::wayland) fn start_session_command( - &mut self, - command: SessionCommand, - ) -> Result<()> { - if self.session_transaction.is_some() { - return Err(anyhow!("another session command is already pending")); - } - if matches!( - command, - SessionCommand::Clear | SessionCommand::ClearTools(_) - ) { - ensure_destructive_session_config_available(self.session_config_failed)?; +impl SessionCommandRuntime for WaylandState { + fn session_context(&mut self) -> SessionTransaction<'_> { + SessionTransaction { + input_state: &mut self.input_state, + measurer: self.render.text_measurer(), + session: &mut self.session, } - if !self.persistence.is_healthy() { - return Err(anyhow!("session persistence worker is unhealthy")); - } - self.session_transaction = Some(ExplicitSessionTransaction::new( - command, - self.session.target_epoch(), - self.input_state.session_interaction_state(), - )); - self.poll_pending_session_command(); - Ok(()) } - pub(in crate::backend::wayland) fn poll_pending_session_command(&mut self) { - if self.persistence.is_active() { - return; - } - let Some(transaction) = self.session_transaction.take() else { - return; - }; - self.advance_session_command(transaction, None); + fn pending_command(&mut self) -> &mut Option { + &mut self.session_transaction } - pub(in crate::backend::wayland) fn complete_session_command( - &mut self, - completion: PersistenceCompletion, - ) { - let Some(transaction) = self.session_transaction.take() else { - return; - }; - if transaction.request_id != Some(completion.id) { - self.fail_session_command( - transaction.command(), - &anyhow!("explicit session completion identity mismatch"), - ); - return; - } - self.advance_session_command(transaction, Some(completion.result)); + fn persistence(&mut self) -> &mut PersistenceController { + &mut self.persistence } - fn advance_session_command( - &mut self, - mut transaction: ExplicitSessionTransaction, - result: Option>, - ) { - session_save::observe_input_dirty(self, Instant::now()); - // A catalog failure follows an already committed open. It is reported - // independently, without reverting the canvas or its current edits. - let step = if let Some(Err(error)) = result.as_ref() - && let Some(report) = transaction.accept_catalog_failure(error) - { - Ok(TransactionStep::Complete(Box::new(report))) - } else { - transaction.advance( - &mut SessionTransaction { - input_state: &mut self.input_state, - measurer: self.render.text_measurer(), - session: &mut self.session, - }, - result, - ) - }; - match step { - Ok(TransactionStep::Work(operation)) => { - // Applying an open refreshes consumer seeds before catalog work. - if transaction.has_committed_open() { - self.refresh_runtime_ui_config_seeds(); - } - match self - .persistence - .try_submit(self.session.target_epoch(), *operation) - { - Ok(id) => { - transaction.request_id = Some(id); - self.session_transaction = Some(transaction); - } - Err(failure) => { - let error = anyhow!("failed to submit session command: {}", failure.error); - if let Some(report) = transaction.accept_catalog_failure(&error) { - self.finish_session_command(report); - } else { - self.fail_session_command(transaction.command(), &error); - } - } - } - } - Ok(TransactionStep::Complete(report)) => { - if matches!( - *report, - SessionCommandReport::Open(_) | SessionCommandReport::Clear(_) - ) { - self.refresh_runtime_ui_config_seeds(); - } - self.finish_session_command(*report); - } - Err(error) => self.fail_session_command(transaction.command(), &error), - } + fn session_config_failed(&self) -> bool { + self.session_config_failed } - pub(in crate::backend::wayland) fn handle_clear_saved_tool_state_action(&mut self) { - let command = - SessionCommand::ClearTools(Box::new(ToolStateSnapshot::from_config(&self.config))); - if let Err(error) = self.start_session_command(command) { - self.report_session_command_error("Failed to reset tool defaults", &error); - } + fn refresh_session_ui_seeds(&mut self) { + self.refresh_runtime_ui_config_seeds(); } - /// Shutdown may wait for durable work; normal dispatch never calls this. - pub(in crate::backend::wayland) fn finish_pending_session_command(&mut self) -> Result<()> { - while self.session_transaction.is_some() { - if self.persistence.is_active() { - session_save::persistence_barrier(self)?; - } else { - self.poll_pending_session_command(); - } - } - Ok(()) + fn finish_session_command(&mut self, report: SessionCommandReport) { + WaylandState::finish_session_command(self, report); } -} -fn ensure_destructive_session_config_available(section_failed: bool) -> Result<()> { - if section_failed { - return Err(anyhow!( - "config.toml [session] could not be read; refusing to modify saved session data that default settings may mistarget - fix the section and retry" - )); + fn fail_session_command(&mut self, command: &SessionCommand, error: &anyhow::Error) { + WaylandState::fail_session_command(self, command, error); + } + + fn session_transport_failed(&mut self, error: &anyhow::Error) { + crate::backend::wayland::backend::event_loop::session_save::handle_persistence_transport_failure( + self, std::time::Instant::now(), error, + ); + } + + fn apply_session_completion(&mut self, completion: PersistenceCompletion) -> Result<()> { + crate::backend::wayland::backend::event_loop::session_save::apply_persistence_completion( + self, completion, + ) } - Ok(()) } -#[cfg(test)] -mod tests { - use super::*; - #[test] - fn destructive_session_actions_fail_closed_after_session_config_fallback() { - let err = ensure_destructive_session_config_available(true).unwrap_err(); - assert!(format!("{err:#}").contains("refusing to modify saved session data")); - ensure_destructive_session_config_available(false).unwrap(); +impl WaylandState { + /// Admit without waiting; an outstanding autosave completes before this command advances. + pub(in crate::backend::wayland) fn start_session_command( + &mut self, + command: SessionCommand, + ) -> Result<()> { + driver::start_session_command(self, command) + } + + pub(in crate::backend::wayland) fn poll_pending_session_command(&mut self) { + driver::poll_pending_session_command(self); + } + + pub(in crate::backend::wayland) fn handle_clear_saved_tool_state_action(&mut self) { + let command = + SessionCommand::ClearTools(Box::new(ToolStateSnapshot::from_config(&self.config))); + if let Err(error) = self.start_session_command(command) { + self.report_session_command_error("Failed to reset tool defaults", &error); + } } } From 6cd5c37d956581ffe89e0fbf22d13a888844e600 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Fri, 2 Oct 2026 12:21:00 +0200 Subject: [PATCH 4/6] test(session): verify real UI seeds and shutdown persistence across pending phases --- docs/CONFIG.md | 6 + .../backend/event_loop/session_save.rs | 76 +-- .../event_loop/session_save/notifications.rs | 2 +- src/backend/wayland/runtime_ui_state.rs | 14 + .../wayland/runtime_ui_state/seed_refresh.rs | 70 +++ .../wayland/runtime_ui_state/wayland.rs | 61 +-- src/backend/wayland/session/driver.rs | 126 +++-- src/backend/wayland/session/driver/tests.rs | 438 +++++++++++++++--- .../driver/tests/lifecycle_regressions.rs | 212 +++++++++ src/backend/wayland/session/persistence.rs | 300 +----------- .../session/persistence/test_support.rs | 63 +++ .../wayland/session/persistence/worker.rs | 230 +++++++++ src/backend/wayland/session/runtime.rs | 3 +- .../wayland/session/runtime/transaction.rs | 16 +- src/backend/wayland/session/tests.rs | 4 +- src/backend/wayland/state.rs | 2 +- src/backend/wayland/state/core/session.rs | 20 +- .../wayland/state/toolbar/events/session.rs | 14 +- src/session/catalog.rs | 64 +-- src/session/catalog/hooks.rs | 77 +++ 20 files changed, 1241 insertions(+), 557 deletions(-) create mode 100644 src/backend/wayland/runtime_ui_state/seed_refresh.rs create mode 100644 src/backend/wayland/session/driver/tests/lifecycle_regressions.rs create mode 100644 src/backend/wayland/session/persistence/test_support.rs create mode 100644 src/backend/wayland/session/persistence/worker.rs create mode 100644 src/session/catalog/hooks.rs diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 8a61c9f0..0acbaff4 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -2128,6 +2128,12 @@ The overlay Session panel lives in the top strip's overflow **"Session..."** pop - Recent session rows reopen other named sessions. If a recent target is missing, Wayscriber removes that stale catalog entry after the failed open. - `Manager` opens the configurator. Overlay Open/Save As dialogs use `zenity` or `kdialog`. +Recent-session catalog updates after Open are best effort. If recording a +successfully opened session fails, the opened canvas stays active and an +overlay toast reports the catalog error. Save As and ordinary saves also keep +catalog bookkeeping best effort, but catalog errors on those paths are logged +rather than shown in a toast; a successful session-file write remains successful. + Open, Save As, and Clear run their disk phases on the persistence worker while event dispatch continues. One command runs at a time, after any pending autosave finishes. If you edit while a captured phase is pending, the edits stay live and diff --git a/src/backend/wayland/backend/event_loop/session_save.rs b/src/backend/wayland/backend/event_loop/session_save.rs index d7c74b70..3c4c6fc3 100644 --- a/src/backend/wayland/backend/event_loop/session_save.rs +++ b/src/backend/wayland/backend/event_loop/session_save.rs @@ -15,13 +15,17 @@ mod notifications; pub(super) use notifications::notify_session_failure; #[cfg(test)] +pub(in crate::backend::wayland) use notifications::record_autosave_failure; +#[cfg(not(test))] +use notifications::record_autosave_failure; +#[cfg(test)] use notifications::record_autosave_success; #[cfg(test)] use notifications::{ SessionSaveNotification, pending_save_notifications, session_save_notification_text, }; use notifications::{ - notify_persistence_worker_failure, notify_session_save_report, record_autosave_failure, + notify_persistence_worker_failure, notify_session_save_report, show_persistence_worker_failure_toast, show_session_failure_toast, }; @@ -354,35 +358,9 @@ fn snapshot_or_empty( }) } -pub(in crate::backend::wayland) fn observe_input_dirty(state: &mut WaylandState, now: Instant) { - let input_dirty = state.input_state.take_session_dirty(); - state.session.record_input_dirty(now, input_dirty); -} - -pub(in crate::backend::wayland) fn persistence_barrier( - state: &mut WaylandState, -) -> Result<(), anyhow::Error> { - observe_input_dirty(state, Instant::now()); - if state.persistence.is_active() { - let completion = match state.persistence.wait_for_completion() { - Ok(Some(completion)) => completion, - Ok(None) => { - return Err(anyhow::anyhow!( - "active persistence request had no completion" - )); - } - Err(err) => { - handle_persistence_transport_failure(state, Instant::now(), &err); - return Err(err); - } - }; - apply_persistence_completion(state, completion)?; - } - if !state.persistence.is_healthy() { - return Err(anyhow::anyhow!("session persistence worker is unhealthy")); - } - Ok(()) -} +pub(in crate::backend::wayland) use runtime_session::driver::{ + observe_input_dirty, persistence_barrier, +}; pub(in crate::backend::wayland) fn run_persistence_operation( state: &mut WaylandState, @@ -442,7 +420,7 @@ impl PersistenceCompletionRuntime for WaylandState { &mut self, completion: PersistenceCompletion, ) -> Result<(), anyhow::Error> { - apply_persistence_completion(self, completion) + runtime_session::driver::apply_session_completion(self, completion) } fn persistence_session_options(&self) -> Option { @@ -478,30 +456,24 @@ pub(in crate::backend::wayland) fn drain_persistence_completion_for_runtime( Ok(()) } -pub(in crate::backend::wayland) fn apply_persistence_completion( +pub(in crate::backend::wayland) fn report_autosave_success( state: &mut WaylandState, - completion: PersistenceCompletion, -) -> Result<(), anyhow::Error> { - let execution_time = completion.execution_time; - match runtime_session::driver::route_session_completion(state, completion) { - Ok(Some(save)) => { - log_session_save_result( - SessionSaveReason::Autosave, - save.report.as_ref(), - execution_time, - ); - notify_session_save_report(state, save.report.as_ref()); - } - Ok(None) => {} - Err(err) => { - handle_autosave_failure(state, Instant::now(), &err); - return Err(err); - } - } - Ok(()) + save: runtime_session::SaveCompletion, + execution_time: Duration, +) { + log_session_save_result( + SessionSaveReason::Autosave, + save.report.as_ref(), + execution_time, + ); + notify_session_save_report(state, save.report.as_ref()); } -fn handle_autosave_failure(state: &mut WaylandState, now: Instant, err: &anyhow::Error) { +pub(in crate::backend::wayland) fn handle_autosave_failure( + state: &mut WaylandState, + now: Instant, + err: &anyhow::Error, +) { let Some(options) = state.session_options().cloned() else { return; }; diff --git a/src/backend/wayland/backend/event_loop/session_save/notifications.rs b/src/backend/wayland/backend/event_loop/session_save/notifications.rs index ded2d720..a02f64a7 100644 --- a/src/backend/wayland/backend/event_loop/session_save/notifications.rs +++ b/src/backend/wayland/backend/event_loop/session_save/notifications.rs @@ -141,7 +141,7 @@ pub(super) fn record_autosave_success( } } -pub(super) fn record_autosave_failure( +pub(in crate::backend::wayland) fn record_autosave_failure( session_state: &mut SessionState, now: Instant, options: &session::SessionOptions, diff --git a/src/backend/wayland/runtime_ui_state.rs b/src/backend/wayland/runtime_ui_state.rs index 734f340f..8440554f 100644 --- a/src/backend/wayland/runtime_ui_state.rs +++ b/src/backend/wayland/runtime_ui_state.rs @@ -24,9 +24,15 @@ mod lifecycle; mod live_state; mod positions; mod rollback; +mod seed_refresh; mod seeds; mod wayland; +#[cfg(test)] +pub(in crate::backend::wayland) use seed_refresh::{ + RuntimeUiSeedRefresh, SeedRefreshContext, refresh_runtime_ui_config_seeds, +}; + use live_state::{ apply_live_board_state, apply_live_toolbar_positions, apply_live_toolbar_state, apply_persisted_top_display_mode, runtime_preview_authority, top_display_mode_values, @@ -49,6 +55,14 @@ pub(in crate::backend::wayland) struct ToolbarPositionSnapshot { pub top: (f64, f64), } +impl ToolbarPositionSnapshot { + fn from_chrome(chrome: &crate::backend::wayland::state::ToolbarChrome) -> Self { + Self { + top: chrome.top_offset(), + } + } +} + #[derive(Debug)] pub(in crate::backend::wayland) enum ToolbarRuntimeFinish { KeepPreview, diff --git a/src/backend/wayland/runtime_ui_state/seed_refresh.rs b/src/backend/wayland/runtime_ui_state/seed_refresh.rs new file mode 100644 index 00000000..bd5695a3 --- /dev/null +++ b/src/backend/wayland/runtime_ui_state/seed_refresh.rs @@ -0,0 +1,70 @@ +//! Shared seed reconciliation and publication; only protocol drag teardown is adapter-owned. +use super::{ToolbarPositionSnapshot, ToolbarRuntimeState}; +use crate::backend::wayland::{ + state::{ToolbarChrome, ToolbarDrag}, + toolbar::ToolbarSurfaceManager, +}; +use crate::{config::Config, draw::TextMeasurer, input::InputState, ui_text::UiTextEngine}; + +pub(in crate::backend::wayland) struct SeedRefreshContext<'a> { + pub config: &'a Config, + pub input: &'a mut InputState, + pub engine: &'a UiTextEngine, + pub measurer: &'a TextMeasurer, + pub runtime: Option<&'a mut ToolbarRuntimeState>, + pub drag: &'a mut ToolbarDrag, + pub chrome: &'a mut ToolbarChrome, + pub toolbar: &'a mut ToolbarSurfaceManager, +} + +pub(in crate::backend::wayland) trait RuntimeUiSeedRefresh { + fn seed_refresh_context(&mut self) -> SeedRefreshContext<'_>; + fn cancel_position_drags(&mut self); +} + +pub(in crate::backend::wayland) fn refresh_runtime_ui_config_seeds( + owner: &mut impl RuntimeUiSeedRefresh, +) { + let (positions, refresh) = { + let context = owner.seed_refresh_context(); + + context + .input + .boards + .sync_pin_seeds_from_config(&context.config.resolved_boards()); + + let Some(runtime) = context.runtime else { + return; + }; + + let mut positions = ToolbarPositionSnapshot::from_chrome(context.chrome); + let refresh = runtime.refresh_config_seeds( + context.engine, + context.measurer, + context.config, + context.input, + &mut positions, + ); + if !refresh.applied { + return; + } + + if refresh.item_drag_aborted { + context.input.clear_toolbar_item_drag(); + context.drag.set_item_dragging(false); + } + + (positions, refresh) + }; + + if refresh.position_drag_aborted { + owner.cancel_position_drags(); + } + + // Seed reconciliation owns preview rollback; protocol drag teardown remains adapter-owned. + let context = owner.seed_refresh_context(); + context.chrome.set_top_offset(positions.top); + context.toolbar.mark_dirty(); + context.input.dirty_tracker.mark_full(); + context.input.needs_redraw = true; +} diff --git a/src/backend/wayland/runtime_ui_state/wayland.rs b/src/backend/wayland/runtime_ui_state/wayland.rs index 1849e1a1..c109b414 100644 --- a/src/backend/wayland/runtime_ui_state/wayland.rs +++ b/src/backend/wayland/runtime_ui_state/wayland.rs @@ -1,14 +1,32 @@ use super::super::state::{MoveDragKind, WaylandState}; +#[cfg(not(test))] +use super::seed_refresh::{ + RuntimeUiSeedRefresh, SeedRefreshContext, refresh_runtime_ui_config_seeds, +}; use super::*; +impl RuntimeUiSeedRefresh for WaylandState { + fn seed_refresh_context(&mut self) -> SeedRefreshContext<'_> { + SeedRefreshContext { + config: &self.config, + input: &mut self.input_state, + engine: self.render.ui_text(), + measurer: self.render.text_measurer(), + runtime: self.preferences.runtime_ui_mut().state_mut(), + drag: &mut self.toolbar_drag, + chrome: &mut self.toolbar_chrome, + toolbar: &mut self.toolbar, + } + } + + fn cancel_position_drags(&mut self) { + self.cancel_toolbar_move_drag(); + self.cancel_gtk_toolbar_drag_lifecycle(); + } +} impl WaylandState { pub(in crate::backend::wayland) fn toolbar_position_snapshot(&self) -> ToolbarPositionSnapshot { - ToolbarPositionSnapshot { - top: ( - self.toolbar_chrome.top_offset().0, - self.toolbar_chrome.top_offset().1, - ), - } + ToolbarPositionSnapshot::from_chrome(&self.toolbar_chrome) } pub(in crate::backend::wayland) fn apply_toolbar_runtime_finish( @@ -331,36 +349,7 @@ impl WaylandState { /// daemon, but keeping this boundary complete prevents a future /// same-process reload from committing an old drag under new seeds. pub(in crate::backend::wayland) fn refresh_runtime_ui_config_seeds(&mut self) { - let configured_boards = self.config.resolved_boards(); - self.input_state - .boards - .sync_pin_seeds_from_config(&configured_boards); - let mut positions = self.toolbar_position_snapshot(); - let Some(runtime) = self.preferences.runtime_ui_mut().state_mut() else { - return; - }; - let refresh = runtime.refresh_config_seeds( - self.render.ui_text(), - self.render.text_measurer(), - &self.config, - &mut self.input_state, - &mut positions, - ); - if !refresh.applied { - return; - } - if refresh.item_drag_aborted { - self.input_state.clear_toolbar_item_drag(); - self.toolbar_drag.set_item_dragging(false); - } - if refresh.position_drag_aborted { - self.cancel_toolbar_move_drag(); - self.cancel_gtk_toolbar_drag_lifecycle(); - } - self.toolbar_chrome.set_top_offset(positions.top); - self.toolbar.mark_dirty(); - self.input_state.dirty_tracker.mark_full(); - self.input_state.needs_redraw = true; + refresh_runtime_ui_config_seeds(self); } pub(in crate::backend::wayland) fn apply_board_runtime_ui_action( diff --git a/src/backend/wayland/session/driver.rs b/src/backend/wayland/session/driver.rs index 066f589d..35354647 100644 --- a/src/backend/wayland/session/driver.rs +++ b/src/backend/wayland/session/driver.rs @@ -1,6 +1,6 @@ //! Live explicit-command orchestration, shared by Wayland and headless runtime tests. use anyhow::{Result, anyhow}; -use std::time::Instant; +use std::time::{Duration, Instant}; use super::{ ExplicitSessionTransaction, PersistenceCompletion, PersistenceController, PersistenceOutcome, @@ -16,14 +16,18 @@ pub(in crate::backend::wayland) trait SessionCommandRuntime { fn refresh_session_ui_seeds(&mut self); fn finish_session_command(&mut self, report: SessionCommandReport); fn fail_session_command(&mut self, command: &SessionCommand, error: &anyhow::Error); - fn apply_session_completion(&mut self, completion: PersistenceCompletion) -> Result<()>; + fn autosave_succeeded(&mut self, save: SaveCompletion, execution_time: Duration); + fn autosave_failed(&mut self, error: &anyhow::Error); fn session_transport_failed(&mut self, error: &anyhow::Error); } -pub(in crate::backend::wayland) fn observe_input_dirty(runtime: &mut impl SessionCommandRuntime) { +pub(in crate::backend::wayland) fn observe_input_dirty( + runtime: &mut impl SessionCommandRuntime, + now: Instant, +) { let context = runtime.session_context(); let dirty = context.input_state.take_session_dirty(); - context.session.record_input_dirty(Instant::now(), dirty); + context.session.record_input_dirty(now, dirty); } pub(in crate::backend::wayland) fn start_session_command( @@ -63,9 +67,11 @@ pub(in crate::backend::wayland) fn poll_pending_session_command( if runtime.persistence().is_active() { return; } + let Some(transaction) = runtime.pending_command().take() else { return; }; + advance_session_command(runtime, transaction, None); } @@ -83,6 +89,7 @@ pub(in crate::backend::wayland) fn complete_session_command( ); return; } + advance_session_command(runtime, transaction, Some(completion.result)); } @@ -91,14 +98,14 @@ fn advance_session_command( mut transaction: ExplicitSessionTransaction, result: Option>, ) { - observe_input_dirty(runtime); + observe_input_dirty(runtime, Instant::now()); + // Catalog failure follows an already committed open; never roll back that canvas. - let step = if let Some(Err(error)) = result.as_ref() - && let Some(report) = transaction.accept_catalog_failure(error) - { - Ok(TransactionStep::Complete(Box::new(report))) - } else { - transaction.advance(&mut runtime.session_context(), result) + let step = match result { + Some(Err(error)) if transaction.has_committed_open() => Ok(TransactionStep::Complete( + Box::new(transaction.catalog_failure_report(error)), + )), + result => transaction.advance(&mut runtime.session_context(), result), }; match step { @@ -106,6 +113,7 @@ fn advance_session_command( if transaction.has_committed_open() { runtime.refresh_session_ui_seeds(); } + let epoch = runtime.session_context().session.target_epoch(); match runtime.persistence().try_submit(epoch, *operation) { Ok(id) => { @@ -113,9 +121,10 @@ fn advance_session_command( *runtime.pending_command() = Some(transaction); } Err(failure) => { - let error = anyhow!("failed to submit session command: {}", failure.error); - if let Some(report) = transaction.accept_catalog_failure(&error) { - runtime.finish_session_command(report); + let error = anyhow::Error::new(failure.error) + .context("failed to submit session command"); + if transaction.has_committed_open() { + runtime.finish_session_command(transaction.catalog_failure_report(error)); } else { runtime.fail_session_command(transaction.command(), &error); } @@ -129,26 +138,30 @@ fn advance_session_command( ) { runtime.refresh_session_ui_seeds(); } + runtime.finish_session_command(*report); } Err(error) => runtime.fail_session_command(transaction.command(), &error), } } -/// Route autosave receipts separately from commands queued behind them. Once a -/// command has submitted work, its completion must pass the explicit identity gate. -pub(in crate::backend::wayland) fn route_session_completion( +/// Apply a receipt and publish its feedback. Ownership errors return before +/// autosave failure bookkeeping; only an owned failed save incurs retry backoff. +pub(in crate::backend::wayland) fn apply_session_completion( runtime: &mut impl SessionCommandRuntime, completion: PersistenceCompletion, -) -> Result> { - observe_input_dirty(runtime); +) -> Result<()> { + observe_input_dirty(runtime, Instant::now()); + + let execution_time = completion.execution_time; + if runtime .pending_command() .as_ref() .is_some_and(|command| command.request_id.is_some()) { complete_session_command(runtime, completion); - return Ok(None); + return Ok(()); } let save_result = match completion.result { @@ -163,37 +176,63 @@ pub(in crate::backend::wayland) fn route_session_completion( Instant::now(), &save_result, )?; - let save = save_result?; - if !committed { - return Err(anyhow!( - "autosave worker completed without writing session data" - )); - } - Ok(Some(save)) + + let write_result = save_result.and_then(|save| { + if committed { + Ok(save) + } else { + Err(anyhow!( + "autosave worker completed without writing session data" + )) + } + }); + + let save = match write_result { + Ok(save) => save, + Err(error) => { + runtime.autosave_failed(&error); + return Err(error); + } + }; + + runtime.autosave_succeeded(save, execution_time); + + Ok(()) } /// Deliberate durability barrier, never called from normal dispatch. +pub(in crate::backend::wayland) fn persistence_barrier( + runtime: &mut impl SessionCommandRuntime, +) -> Result<()> { + observe_input_dirty(runtime, Instant::now()); + + if runtime.persistence().is_active() { + let completion = match runtime.persistence().wait_for_completion() { + Ok(Some(completion)) => completion, + Ok(None) => return Err(anyhow!("active persistence request had no completion")), + Err(error) => { + runtime.session_transport_failed(&error); + return Err(error); + } + }; + apply_session_completion(runtime, completion)?; + } + + if !runtime.persistence().is_healthy() { + return Err(anyhow!("session persistence worker is unhealthy")); + } + + Ok(()) +} + pub(in crate::backend::wayland) fn finish_pending_session_command( runtime: &mut impl SessionCommandRuntime, ) -> Result<()> { while runtime.pending_command().is_some() { - if runtime.persistence().is_active() { - let completion = match runtime.persistence().wait_for_completion() { - Ok(Some(completion)) => completion, - Ok(None) => return Err(anyhow!("active persistence request had no completion")), - Err(error) => { - runtime.session_transport_failed(&error); - return Err(error); - } - }; - runtime.apply_session_completion(completion)?; - } else { - poll_pending_session_command(runtime); - } - if !runtime.persistence().is_healthy() { - return Err(anyhow!("session persistence worker is unhealthy")); - } + poll_pending_session_command(runtime); + persistence_barrier(runtime)?; } + Ok(()) } @@ -208,6 +247,7 @@ pub(in crate::backend::wayland) fn persist_after_pending_commands { config_failed: bool, reports: Vec, errors: Vec, - seed_refreshes: usize, + ui: Option, + ui_engine: crate::ui_text::UiTextEngine, + chrome: ToolbarChrome, + drag: ToolbarDrag, + toolbar: ToolbarSurfaceManager, autosaves: usize, + autosave_failures: usize, } impl<'a> CommandRuntime<'a> { @@ -36,8 +54,13 @@ impl<'a> CommandRuntime<'a> { config_failed: false, reports: Vec::new(), errors: Vec::new(), - seed_refreshes: 0, + ui: None, + ui_engine: crate::ui_text::UiTextEngine::default(), + chrome: ToolbarChrome::new(true, (0.0, 0.0)), + drag: ToolbarDrag::new(), + toolbar: ToolbarSurfaceManager::new(), autosaves: 0, + autosave_failures: 0, } } @@ -50,12 +73,69 @@ impl<'a> CommandRuntime<'a> { .ok_or_else(|| anyhow!("command did not publish a terminal report")) } + fn enable_ui_runtime(&mut self, path: &std::path::Path) { + let wake = RuntimeWakeSource::new().unwrap(); + self.ui = Some( + ToolbarRuntimeState::start(&self.config, self.input, path, wake.handle()).unwrap(), + ); + } + + fn submit_autosave( + &mut self, + snapshot: stored_session::SessionSnapshot, + options: stored_session::SessionOptions, + ) -> RequestId { + let window = self.session.prepare_autosave_submission().unwrap(); + let epoch = self.session.target_epoch(); + let id = self + .persistence + .try_submit( + epoch, + PersistenceOperation::Save { + snapshot, + options, + strategy: SaveStrategy::Autosave, + contentless_clear_boundary: false, + }, + ) + .unwrap(); + + self.session.commit_autosave_submission(id, window); + + id + } + + fn apply_session_completion(&mut self, completion: PersistenceCompletion) -> Result<()> { + apply_session_completion(self, completion) + } + fn receive(&mut self) { let completion = self.persistence.wait_for_completion().unwrap().unwrap(); self.apply_session_completion(completion).unwrap(); } } +impl RuntimeUiSeedRefresh for CommandRuntime<'_> { + fn seed_refresh_context(&mut self) -> SeedRefreshContext<'_> { + SeedRefreshContext { + config: &self.config, + input: self.input, + engine: &self.ui_engine, + measurer: self.measurer, + runtime: self.ui.as_mut(), + drag: &mut self.drag, + chrome: &mut self.chrome, + toolbar: &mut self.toolbar, + } + } + + fn cancel_position_drags(&mut self) { + self.drag.end_move(); + self.drag.set_preview_active(false); + self.drag.cancel_gtk(); + } +} + impl SessionCommandRuntime for CommandRuntime<'_> { fn session_context(&mut self) -> SessionTransaction<'_> { SessionTransaction { @@ -64,42 +144,62 @@ impl SessionCommandRuntime for CommandRuntime<'_> { session: self.session, } } + fn pending_command(&mut self) -> &mut Option { &mut self.pending } + fn persistence(&mut self) -> &mut PersistenceController { &mut self.persistence } + fn session_config_failed(&self) -> bool { self.config_failed } + fn refresh_session_ui_seeds(&mut self) { - self.seed_refreshes += 1; - self.input - .boards - .sync_pin_seeds_from_config(&self.config.resolved_boards()); + refresh_runtime_ui_config_seeds(self); } + fn finish_session_command(&mut self, report: SessionCommandReport) { self.reports.push(report); } + fn fail_session_command(&mut self, _: &SessionCommand, error: &anyhow::Error) { self.errors.push(anyhow!("{error:#}")); } + fn session_transport_failed(&mut self, _: &anyhow::Error) { self.session.restore_in_flight_autosave(); } - fn apply_session_completion(&mut self, completion: PersistenceCompletion) -> Result<()> { - if route_session_completion(self, completion)?.is_some() { - self.autosaves += 1; - } - Ok(()) + fn autosave_succeeded(&mut self, _: SaveCompletion, _: Duration) { + self.autosaves += 1; + } + + fn autosave_failed(&mut self, _: &anyhow::Error) { + let options = self.session.options().unwrap().clone(); + record_autosave_failure(self.session, Instant::now(), &options); + self.autosave_failures += 1; } } #[test] fn admission_rejects_each_guard_without_submitting_or_retargeting() { - for case in ["pending", "unhealthy", "clear-config", "tools-config"] { + #[derive(Clone, Copy, Debug, PartialEq, Eq)] + enum Guard { + Pending, + Unhealthy, + ClearConfig, + ToolsConfig, + } + + for case in [ + Guard::Pending, + Guard::Unhealthy, + Guard::ClearConfig, + Guard::ToolsConfig, + ] { let temp = crate::test_temp::tempdir().unwrap(); let options = named_options(temp.path(), "current"); let mut input = test_input_state(); @@ -108,14 +208,15 @@ fn admission_rejects_each_guard_without_submitting_or_retargeting() { let (persistence, worker) = PersistenceController::controlled_for_test(); let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); let mut worker = Some(worker); + let command = match case { - "pending" => { + Guard::Pending => { start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); worker.as_ref().unwrap().complete_next(); // Receipt is held before runtime delivery; the original identity must survive. SessionCommand::Clear } - "unhealthy" => { + Guard::Unhealthy => { drop(worker.take()); assert!( runtime @@ -130,18 +231,18 @@ fn admission_rejects_each_guard_without_submitting_or_retargeting() { ); SessionCommand::Inspect } - "clear-config" => { + Guard::ClearConfig => { runtime.config_failed = true; SessionCommand::Clear } - "tools-config" => { + Guard::ToolsConfig => { runtime.config_failed = true; SessionCommand::ClearTools(Box::new( stored_session::ToolStateSnapshot::from_config(&runtime.config), )) } - _ => unreachable!(), }; + let before = runtime .pending .as_ref() @@ -150,11 +251,12 @@ fn admission_rejects_each_guard_without_submitting_or_retargeting() { .unwrap_err() .to_string(); let expected = match case { - "pending" => "already pending", - "unhealthy" => "unhealthy", + Guard::Pending => "already pending", + Guard::Unhealthy => "unhealthy", _ => "refusing to modify saved session data", }; - assert!(error.contains(expected), "{case}: {error}"); + + assert!(error.contains(expected), "{case:?}: {error}"); assert_eq!( runtime .pending @@ -170,7 +272,7 @@ fn admission_rejects_each_guard_without_submitting_or_retargeting() { if let Some(worker) = worker { assert!(!worker.has_request()); } - if case == "pending" { + if case == Guard::Pending { runtime.receive(); assert!(matches!( runtime.reports.as_slice(), @@ -190,27 +292,15 @@ fn explicit_command_waits_for_autosave_receipt_and_survives_dispatch_work() { let measurer = TextMeasurer::default(); let (persistence, worker) = PersistenceController::controlled_for_test(); let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); - let window = runtime.session.prepare_autosave_submission().unwrap(); - let autosave_id = runtime - .persistence - .try_submit( - 0, - PersistenceOperation::Save { - snapshot: sample_snapshot(), - options, - strategy: SaveStrategy::Autosave, - contentless_clear_boundary: false, - }, - ) - .unwrap(); - runtime - .session - .commit_autosave_submission(autosave_id, window); + + let autosave_id = runtime.submit_autosave(sample_snapshot(), options); + start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); poll_pending_session_command(&mut runtime); assert_eq!(runtime.pending.as_ref().unwrap().request_id, None); assert!(runtime.reports.is_empty()); assert_eq!(runtime.session.edit_generation(), 1); + // The worker has received no completion yet; unrelated live input still progresses. add_line(runtime.input, 77); runtime.input.mark_session_dirty(); @@ -220,9 +310,11 @@ fn explicit_command_waits_for_autosave_receipt_and_survives_dispatch_work() { assert_eq!(runtime.pending.as_ref().unwrap().request_id, None); assert!(!worker.has_request()); assert!(runtime.session.is_dirty()); + poll_pending_session_command(&mut runtime); let explicit_id = runtime.pending.as_ref().unwrap().request_id.unwrap(); assert_ne!(explicit_id, autosave_id); + worker.complete_next(); runtime.receive(); assert!(runtime.pending.is_none()); @@ -243,11 +335,13 @@ fn live_completion_gate_rejects_a_different_request_identity() { let temp = crate::test_temp::tempdir().unwrap(); let options = named_options(temp.path(), "current"); *runtime.session = SessionState::new(Some(options)); + start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); worker.complete_next(); let mut completion = runtime.persistence.wait_for_completion().unwrap().unwrap(); completion.id.sequence += 1; runtime.apply_session_completion(completion).unwrap(); + assert!(runtime.pending.is_none()); assert!(runtime.reports.is_empty()); assert!(runtime.errors[0].to_string().contains("identity mismatch")); @@ -272,6 +366,7 @@ fn advance_observes_live_edits_and_finalizes_stale_destructive_work() { let completion = runtime.persistence.wait_for_completion().unwrap().unwrap(); // Exercise advance's own dirty observation, not the completion router's observation. complete_session_command(&mut runtime, completion); + assert!(runtime.pending.is_none()); assert_eq!(runtime.input.boards.active_frame().shapes.len(), 2); assert!(runtime.session.is_dirty()); @@ -298,6 +393,7 @@ fn deferred_submission_rejection_is_terminal() { runtime.persistence.wait_for_completion().unwrap().unwrap(); drop(worker); poll_pending_session_command(&mut runtime); + assert!(runtime.pending.is_none()); assert!(runtime.reports.is_empty()); assert!(runtime.errors[0].to_string().contains("failed to submit")); @@ -311,6 +407,7 @@ fn advance_failure_publishes_error_and_retires_the_command() { let (persistence, worker) = PersistenceController::controlled_for_test(); let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); + assert!(runtime.pending.is_none()); assert!(runtime.reports.is_empty()); assert!( @@ -321,18 +418,98 @@ fn advance_failure_publishes_error_and_retires_the_command() { assert!(!worker.has_request()); } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum CatalogOutcome { + Success, + Failure, + Rejected, +} + +impl CatalogOutcome { + fn assert_terminal_report( + self, + runtime: &CommandRuntime<'_>, + target: &stored_session::SessionOptions, + root: &std::path::Path, + ) { + assert!(runtime.pending.is_none()); + assert!(runtime.errors.is_empty()); + let [SessionCommandReport::Open(report)] = runtime.reports.as_slice() else { + panic!("expected committed Open report"); + }; + assert_eq!(report.catalog_error.is_some(), self != Self::Success); + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + target.session_file_path() + ); + assert_eq!(runtime.input.boards.active_frame().shapes.len(), 1); + assert!(matches!( + runtime.input.boards.active_frame().shapes[0].shape, + crate::draw::Shape::Line { x2: 42, .. } + )); + assert_eq!(loaded_line_x2(target), 42); + // Catalog I/O failure leaves the worker usable; rejected transport marks it unhealthy. + assert_eq!(runtime.persistence.is_healthy(), self != Self::Rejected); + + match self { + Self::Failure => { + let error = report.catalog_error.as_ref().unwrap(); + assert!( + format!("{error:#}").contains("failed to create session catalog directory") + ); + assert!(error.downcast_ref::().is_some()); + assert_eq!( + std::fs::read(root.join("wayscriber")).unwrap(), + b"catalog blocked" + ); + } + Self::Success => { + let entries = stored_session::catalog::recent_sessions().unwrap(); + assert_eq!(entries.len(), 1); + assert!(stored_session::catalog::session_paths_match( + std::path::Path::new(&entries[0].path), + &target.session_file_path() + )); + assert!(entries[0].last_opened_at_millis.is_some()); + } + Self::Rejected => { + assert!(matches!( + report + .catalog_error + .as_ref() + .unwrap() + .downcast_ref::(), + Some(SubmitError::Disconnected) + )); + } + } + } +} + #[test] fn open_refreshes_consumer_seeds_before_catalog_work_and_at_completion() { - for catalog in ["success", "failure", "rejected"] { + for catalog in [ + CatalogOutcome::Success, + CatalogOutcome::Failure, + CatalogOutcome::Rejected, + ] { let temp = crate::test_temp::tempdir().unwrap(); let current = named_options(temp.path(), "current"); let target = named_options(temp.path(), "target"); stored_session::save_snapshot(&sample_snapshot(), &target).unwrap(); + let _env = EnvGuard::set_xdg_data_home(temp.path()); + if catalog == CatalogOutcome::Failure { + // A regular file blocks catalog directory creation, without relying on uid or modes. + std::fs::write(temp.path().join("wayscriber"), b"catalog blocked").unwrap(); + } + let mut input = test_input_state(); let mut session = SessionState::new(Some(current)); let measurer = TextMeasurer::default(); let (persistence, worker) = PersistenceController::controlled_for_test(); let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + runtime.enable_ui_runtime(&temp.path().join("runtime-ui.toml")); + runtime.config.ui.toolbar.top_offset = 64.0; runtime.config.boards = Some(runtime.input.boards.to_config()); for item in &mut runtime.config.boards.as_mut().unwrap().items { item.pinned = true; @@ -342,17 +519,19 @@ fn open_refreshes_consumer_seeds_before_catalog_work_and_at_completion() { SessionCommand::Open(target.session_file_path()), ) .unwrap(); + worker.complete_next(); // open preflight runtime.receive(); worker.complete_next(); // candidate load, no dirty current source let completion = runtime.persistence.wait_for_completion().unwrap().unwrap(); - let worker = if catalog == "rejected" { + let worker = if catalog == CatalogOutcome::Rejected { drop(worker); None } else { Some(worker) }; runtime.apply_session_completion(completion).unwrap(); + assert_eq!( runtime.session.options().unwrap().session_file_path(), target.session_file_path() @@ -367,19 +546,18 @@ fn open_refreshes_consumer_seeds_before_catalog_work_and_at_completion() { .iter() .all(|item| item.pinned) ); - assert_eq!(runtime.seed_refreshes, 1); + assert_eq!(runtime.chrome.top_offset(), (64.0, 0.0)); + if let Some(worker) = worker { - // Make a changed authored seed observable on the terminal refresh too. + // Make changed consumer-visible seeds observable on the terminal refresh too. + runtime.config.ui.toolbar.top_offset = 96.0; for item in &mut runtime.config.boards.as_mut().unwrap().items { item.pinned = false; } - if catalog == "failure" { - worker.respond_with(|_| Err(anyhow!("controlled catalog error"))); - } else { - worker.complete_next(); - } + worker.complete_next(); runtime.receive(); - assert_eq!(runtime.seed_refreshes, 2); + + assert_eq!(runtime.chrome.top_offset(), (96.0, 0.0)); assert!( runtime .input @@ -390,12 +568,8 @@ fn open_refreshes_consumer_seeds_before_catalog_work_and_at_completion() { .all(|item| !item.pinned) ); } - assert!(runtime.pending.is_none()); - assert!(runtime.errors.is_empty()); - assert!(matches!( - runtime.reports.as_slice(), - [SessionCommandReport::Open(_)] - )); + + catalog.assert_terminal_report(&runtime, &target, temp.path()); } } @@ -409,14 +583,19 @@ fn clear_completion_refreshes_consumer_seeds() { let measurer = TextMeasurer::default(); let (persistence, worker) = PersistenceController::controlled_for_test(); let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + runtime.enable_ui_runtime(&temp.path().join("runtime-ui.toml")); + runtime.config.ui.toolbar.top_offset = 64.0; runtime.config.boards = Some(runtime.input.boards.to_config()); for item in &mut runtime.config.boards.as_mut().unwrap().items { item.pinned = true; } + start_session_command(&mut runtime, SessionCommand::Clear).unwrap(); worker.complete_next(); runtime.receive(); + assert!(runtime.input.boards.active_frame().shapes.is_empty()); + assert_eq!(runtime.chrome.top_offset(), (64.0, 0.0)); assert!( runtime .input @@ -463,6 +642,7 @@ fn shutdown_drains_save_as_and_open_at_every_disk_phase_before_final_save() { runtime.receive(); } assert!(runtime.pending.is_some()); + std::thread::scope(|scope| { scope.spawn(move || { for _ in completed..phases { @@ -495,6 +675,7 @@ fn shutdown_drains_save_as_and_open_at_every_disk_phase_before_final_save() { runtime.session.options().unwrap().session_file_path(), target.session_file_path() ); + assert_eq!(loaded_line_x2(&target), if open { 42 } else { 51 }); let loaded = stored_session::load_snapshot(&target).unwrap().unwrap(); assert_eq!(loaded.boards[0].pages.pages[0].shapes.len(), 2); @@ -505,6 +686,152 @@ fn shutdown_drains_save_as_and_open_at_every_disk_phase_before_final_save() { } } +#[test] +fn shutdown_starts_ready_to_poll_open_and_save_as_after_an_autosave_receipt() { + for open in [true, false] { + let temp = crate::test_temp::tempdir().unwrap(); + let current = named_options(temp.path(), "current"); + let target = named_options(temp.path(), "target"); + if open { + stored_session::save_snapshot(&sample_snapshot(), &target).unwrap(); + } + let mut input = test_input_state(); + add_line(&mut input, 51); + input.mark_session_dirty(); + let mut session = SessionState::new(Some(current.clone())); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + observe_input_dirty(&mut runtime, Instant::now()); + let snapshot = runtime + .input + .snapshot_for_persistence_with(&measurer, ¤t) + .unwrap(); + + runtime.submit_autosave(snapshot, current.clone()); + let command = if open { + SessionCommand::Open(target.session_file_path()) + } else { + SessionCommand::SaveAs( + target.session_file_path(), + stored_session::SaveAsOverwrite::Deny, + ) + }; + start_session_command(&mut runtime, command).unwrap(); + worker.complete_next(); + runtime.receive(); + assert!(!runtime.persistence.is_active()); + assert_eq!(runtime.pending.as_ref().unwrap().request_id, None); + assert!(runtime.reports.is_empty()); + + std::thread::scope(|scope| { + scope.spawn(move || { + for _ in 0..if open { 3 } else { 2 } { + worker.complete_next(); + } + }); + persist_after_pending_commands(&mut runtime, |runtime| { + assert!(runtime.pending.is_none()); + assert!(runtime.errors.is_empty()); + assert_eq!(runtime.reports.len(), 1); + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + target.session_file_path() + ); + let snapshot = runtime + .input + .snapshot_for_persistence_with(&measurer, &target) + .unwrap(); + stored_session::save_snapshot(&snapshot, runtime.session.options().unwrap())?; + Ok(()) + }) + .unwrap(); + }); + assert_eq!(loaded_line_x2(¤t), 51); + assert_eq!(loaded_line_x2(&target), if open { 42 } else { 51 }); + } +} + +#[test] +fn shutdown_reports_open_and_save_as_phase_failures_before_final_persistence() { + for (open, phases) in [(false, 2), (true, 4)] { + for completed in 0..phases { + let temp = crate::test_temp::tempdir().unwrap(); + let current = named_options(temp.path(), "current"); + let target = named_options(temp.path(), "target"); + if open { + stored_session::save_snapshot(&sample_snapshot(), &target).unwrap(); + } + let mut input = test_input_state(); + add_line(&mut input, 51); + input.mark_session_dirty(); + let mut session = SessionState::new(Some(current.clone())); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + let command = if open { + SessionCommand::Open(target.session_file_path()) + } else { + SessionCommand::SaveAs( + target.session_file_path(), + stored_session::SaveAsOverwrite::Deny, + ) + }; + start_session_command(&mut runtime, command).unwrap(); + for _ in 0..completed { + worker.complete_next(); + runtime.receive(); + } + + worker.respond_with(|_| Err(anyhow!("controlled shutdown phase failure"))); + // The controller remains active with a receipt ready; shutdown must poll it. + persist_after_pending_commands(&mut runtime, |runtime| { + assert!(runtime.pending.is_none()); + let catalog_failure = open && completed == 3; + let active = if catalog_failure { &target } else { ¤t }; + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + active.session_file_path() + ); + if catalog_failure { + assert!(runtime.errors.is_empty()); + let [SessionCommandReport::Open(report)] = runtime.reports.as_slice() else { + panic!("expected committed Open report"); + }; + assert!( + report + .catalog_error + .as_ref() + .unwrap() + .to_string() + .contains("controlled shutdown phase failure") + ); + } else { + assert!(runtime.reports.is_empty()); + assert_eq!(runtime.errors.len(), 1); + assert!( + runtime.errors[0] + .to_string() + .contains("controlled shutdown phase failure") + ); + } + + let snapshot = runtime + .input + .snapshot_for_persistence_with(&measurer, active) + .unwrap(); + stored_session::save_snapshot(&snapshot, active)?; + assert_eq!( + loaded_line_x2(active), + if catalog_failure { 42 } else { 51 } + ); + Ok(()) + }) + .unwrap(); + } + } +} + #[test] fn shutdown_cleans_up_pending_work_after_worker_disconnect_and_when_ready_to_poll() { for disconnect in [true, false] { @@ -522,6 +849,7 @@ fn shutdown_cleans_up_pending_work_after_worker_disconnect_and_when_ready_to_pol .try_submit(0, PersistenceOperation::HasArtifacts { options }) .unwrap(); start_session_command(&mut runtime, SessionCommand::Inspect).unwrap(); + if disconnect { drop(worker); persist_after_pending_commands(&mut runtime, |runtime| { diff --git a/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs b/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs new file mode 100644 index 00000000..a7bbc695 --- /dev/null +++ b/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs @@ -0,0 +1,212 @@ +use super::*; +use crate::backend::wayland::{runtime_ui_state::ToolbarPositionSnapshot, state::MoveDragKind}; + +#[test] +fn autosave_ownership_errors_do_not_publish_failures_or_delay_retry() { + #[derive(Clone, Copy, Debug)] + enum Receipt { + MissingTicket, + WrongTicket, + WrongEpoch, + WriteFailure, + } + + for receipt in [ + Receipt::MissingTicket, + Receipt::WrongTicket, + Receipt::WrongEpoch, + Receipt::WriteFailure, + ] { + let temp = crate::test_temp::tempdir().unwrap(); + let mut options = named_options(temp.path(), "current"); + options.autosave_enabled = true; + options.autosave_idle = Duration::from_millis(1); + options.autosave_interval = Duration::from_millis(1); + options.autosave_failure_backoff = Duration::from_secs(60); + + let started = Instant::now(); + let mut input = test_input_state(); + let mut session = SessionState::new(Some(options.clone())); + session.record_input_dirty(started, true); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + + runtime.submit_autosave(sample_snapshot(), options.clone()); + if matches!(receipt, Receipt::MissingTicket) { + assert!(runtime.session.restore_in_flight_autosave()); + } + + if matches!(receipt, Receipt::WriteFailure) { + worker.respond_with(|_| Err(anyhow!("controlled write failure"))); + } else { + worker.complete_next(); + } + + let mut completion = runtime.persistence.wait_for_completion().unwrap().unwrap(); + match receipt { + Receipt::WrongTicket => completion.id.sequence += 1, + // Defensive branch: real target changes also clear the in-flight ticket. + // Keep receipt and ticket equal so only the session-epoch guard rejects it. + Receipt::WrongEpoch => runtime.session.target_epoch += 1, + _ => {} + } + + assert!(runtime.apply_session_completion(completion).is_err()); + + let owned_failure = matches!(receipt, Receipt::WriteFailure); + assert_eq!( + runtime.autosave_failures, + usize::from(owned_failure), + "{receipt:?}" + ); + assert_eq!(runtime.autosaves, 0); + assert!(runtime.session.is_dirty()); + assert!(runtime.persistence.is_healthy()); + assert_eq!( + runtime + .session + .autosave_due(Instant::now() + Duration::from_millis(2), &options), + !owned_failure, + "{receipt:?}" + ); + } +} + +#[test] +fn shutdown_caller_retires_a_command_queued_behind_an_owned_failed_autosave() { + let temp = crate::test_temp::tempdir().unwrap(); + let current = named_options(temp.path(), "current"); + let target = named_options(temp.path(), "target"); + stored_session::save_snapshot(&sample_snapshot(), &target).unwrap(); + + let mut input = test_input_state(); + add_line(&mut input, 51); + let mut session = SessionState::new(Some(current.clone())); + session.record_input_dirty(Instant::now(), true); + let measurer = TextMeasurer::default(); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + + runtime.submit_autosave(sample_snapshot(), current.clone()); + + start_session_command( + &mut runtime, + SessionCommand::Open(target.session_file_path()), + ) + .unwrap(); + assert_eq!(runtime.pending.as_ref().unwrap().request_id, None); + + worker.respond_with(|_| Err(anyhow!("controlled autosave failure before open"))); + persist_after_pending_commands(&mut runtime, |runtime| { + assert!(runtime.pending.is_none()); + assert!(runtime.reports.is_empty()); + assert_eq!(runtime.errors.len(), 1); + assert!( + runtime.errors[0] + .to_string() + .contains("controlled autosave failure before open") + ); + assert!(runtime.persistence.is_healthy()); + assert!(runtime.session.is_dirty()); + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + current.session_file_path() + ); + + runtime.input.mark_session_dirty(); + let snapshot = runtime + .input + .snapshot_for_persistence_with(&measurer, ¤t) + .unwrap(); + stored_session::save_snapshot(&snapshot, runtime.session.options().unwrap())?; + Ok(()) + }) + .unwrap(); + + assert!(!worker.has_request()); + assert_eq!(loaded_line_x2(¤t), 51); + assert_eq!(loaded_line_x2(&target), 42); +} + +#[test] +fn session_seed_refresh_aborts_changed_drags_and_publishes_live_chrome_positions() { + #[derive(Clone, Copy, Debug)] + enum Drag { + Item, + Builtin, + Gtk, + } + + for drag in [Drag::Item, Drag::Builtin, Drag::Gtk] { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "current"); + + let mut input = test_input_state(); + add_line(&mut input, 51); + + let mut session = SessionState::new(Some(options)); + let measurer = TextMeasurer::default(); + let (persistence, _worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + + runtime.enable_ui_runtime(&temp.path().join("runtime-ui.toml")); + + let group = crate::config::ToolbarItemOrderGroup::TopTools; + let pen = crate::config::toolbar_item_ids::TOP_TOOL_PEN; + + match drag { + Drag::Item => { + assert!( + runtime + .ui + .as_mut() + .unwrap() + .begin_item_drag(group, runtime.input) + ); + assert!(runtime.input.start_toolbar_item_drag(group, pen)); + runtime.drag.set_item_dragging(true); + + assert!( + runtime + .config + .ui + .toolbar + .items + .move_item_to_index(group, pen, 8) + ); + } + Drag::Builtin | Drag::Gtk => { + assert!(runtime.ui.as_mut().unwrap().begin_position_drag( + MoveDragKind::Top, + ToolbarPositionSnapshot { top: (0.0, 0.0) } + )); + if matches!(drag, Drag::Builtin) { + runtime + .drag + .begin_move(MoveDragKind::Top, (0.0, 0.0), false, (0.0, 0.0)); + } else { + runtime + .drag + .begin_gtk_preview(crate::toolbar_gtk::GtkToolbarKind::Top, 0.0); + } + runtime.chrome.set_top_offset((80.0, 100.0)); + } + } + + runtime.config.ui.toolbar.top_offset = 64.0; + runtime.config.ui.toolbar.top_offset_y = 32.0; + runtime.input.needs_redraw = false; + + runtime.refresh_session_ui_seeds(); + + assert_eq!(runtime.chrome.top_offset(), (64.0, 32.0), "{drag:?}"); + assert!(!runtime.drag.item_dragging()); + assert!(!runtime.drag.is_moving()); + assert_eq!(runtime.drag.gtk_preview_kind(), None); + assert!(runtime.input.needs_redraw); + if matches!(drag, Drag::Item) { + assert!(runtime.input.start_toolbar_item_drag(group, pen)); + } + } +} diff --git a/src/backend/wayland/session/persistence.rs b/src/backend/wayland/session/persistence.rs index 3392fa09..ee3d1c06 100644 --- a/src/backend/wayland/session/persistence.rs +++ b/src/backend/wayland/session/persistence.rs @@ -1,4 +1,6 @@ -use std::path::{Path, PathBuf}; +#[cfg(test)] +use std::path::Path; +use std::path::PathBuf; use std::sync::mpsc::{self, Receiver, SyncSender, TryRecvError, TrySendError}; use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant}; @@ -9,10 +11,16 @@ use crate::backend::wayland::backend::runtime_wake::RuntimeWakeHandle; #[cfg(test)] use crate::backend::wayland::backend::runtime_wake::RuntimeWakeSource; use crate::session::{ - self, ClearToolStateOutcome, LoadSnapshotOutcome, SaveAsOverwrite, SaveSnapshotOutcome, - SaveSnapshotReport, SessionInspection, SessionOptions, SessionSnapshot, + self, ClearToolStateOutcome, LoadSnapshotOutcome, SaveAsOverwrite, SaveSnapshotReport, + SessionInspection, SessionOptions, SessionSnapshot, }; +mod worker; +use worker::worker_main; + +#[cfg(test)] +use worker::save_as_preflight_after_validation; + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(in crate::backend::wayland) struct RequestId { pub(in crate::backend::wayland) target_epoch: u64, @@ -214,27 +222,6 @@ impl PersistenceController { Self::start(wake.handle()) } - #[cfg(test)] - pub(in crate::backend::wayland) fn controlled_for_test() -> (Self, ControlledPersistenceWorker) - { - let (request_tx, requests) = mpsc::sync_channel(1); - let (completions, completion_rx) = mpsc::sync_channel(1); - ( - Self { - request_tx: Some(request_tx), - completion_rx, - worker: None, - active_id: None, - next_sequence: 0, - healthy: true, - }, - ControlledPersistenceWorker { - requests, - completions, - }, - ) - } - pub(in crate::backend::wayland) fn is_active(&self) -> bool { self.active_id.is_some() } @@ -454,278 +441,17 @@ impl Drop for PersistenceController { } } -/// Channel peer for driver tests: it controls delivery, not admission or phase ordering. #[cfg(test)] -pub(in crate::backend::wayland) struct ControlledPersistenceWorker { - requests: Receiver, - completions: SyncSender, -} - -#[cfg(test)] -impl ControlledPersistenceWorker { - pub(in crate::backend::wayland) fn complete_next(&self) { - self.respond_with(execute); - } - - pub(in crate::backend::wayland) fn respond_with( - &self, - result: impl FnOnce(PersistenceOperation) -> Result, - ) { - let request = self - .requests - .recv_timeout(Duration::from_secs(5)) - .expect("driver submitted work"); - let completion = PersistenceCompletion { - id: request.id, - result: result(request.operation), - queue_wait: Duration::ZERO, - execution_time: Duration::ZERO, - worker_thread_id: thread::current().id(), - finished_at: Instant::now(), - }; - self.completions.send(completion).unwrap(); - } - - pub(in crate::backend::wayland) fn has_request(&self) -> bool { - match self.requests.try_recv() { - Err(TryRecvError::Empty) => false, - other => panic!("unexpected worker request: {other:?}"), - } - } -} - -fn worker_main( - request_rx: Receiver, - completion_tx: SyncSender, - wake: RuntimeWakeHandle, -) { - let publisher = PersistenceCompletionPublisher::new(completion_tx, wake); - while let Ok(request) = request_rx.recv() { - let PersistenceRequest { - id, - queued_at, - operation, - } = request; - let queue_wait = queued_at.elapsed(); - let label = operation.label(); - let shutdown = matches!(operation, PersistenceOperation::Shutdown); - let started = Instant::now(); - log::debug!("Persistence worker starting {label} request {id:?}"); - let result = execute(operation); - let execution_time = started.elapsed(); - let worker_thread_id = thread::current().id(); - let finished_at = Instant::now(); - if !publisher.publish(PersistenceCompletion { - id, - result, - queue_wait, - execution_time, - worker_thread_id, - finished_at, - }) { - break; - } - if shutdown { - break; - } - } -} - -struct PersistenceCompletionPublisher { - completion_tx: Option>, - wake: RuntimeWakeHandle, -} - -impl PersistenceCompletionPublisher { - fn new(completion_tx: SyncSender, wake: RuntimeWakeHandle) -> Self { - Self { - completion_tx: Some(completion_tx), - wake, - } - } - - fn publish(&self, completion: PersistenceCompletion) -> bool { - let Some(completion_tx) = self.completion_tx.as_ref() else { - return false; - }; - if completion_tx.send(completion).is_err() { - return false; - } - if let Err(err) = self.wake.wake() { - log::error!("Failed to wake runtime after persistence completion: {err}"); - return false; - } - true - } -} - -impl Drop for PersistenceCompletionPublisher { - fn drop(&mut self) { - // Close the completion channel before waking. The event loop can therefore - // observe disconnect immediately even when the worker unwinds without a - // completion packet. - self.completion_tx.take(); - if let Err(err) = self.wake.wake() { - log::error!("Failed to wake runtime after persistence worker exit: {err}"); - } - } -} - -fn execute(operation: PersistenceOperation) -> Result { - match operation { - PersistenceOperation::Save { - snapshot, - options, - strategy, - contentless_clear_boundary, - } => { - log_snapshot_summary(&snapshot, &options, strategy); - let snapshot_board_data = snapshot.has_board_data(); - let report = match strategy { - SaveStrategy::Autosave => { - session::save_snapshot_autosave_with_report_and_clear_boundary( - &snapshot, - &options, - contentless_clear_boundary, - )? - } - SaveStrategy::Normal => session::save_snapshot_with_report_and_clear_boundary( - &snapshot, - &options, - contentless_clear_boundary, - )?, - }; - let committed_board_data = report.as_ref().is_some_and(|report| { - !matches!(report.outcome, SaveSnapshotOutcome::ClearedEmpty) && snapshot_board_data - }); - Ok(PersistenceOutcome::Save(SaveCompletion { - report, - committed_board_data, - })) - } - PersistenceOperation::SaveAs { - snapshot, - options, - overwrite, - } => { - let snapshot_board_data = snapshot.has_board_data(); - let report = session::save_snapshot_as_with_report(&snapshot, &options, overwrite)?; - let committed_board_data = - !matches!(report.outcome, SaveSnapshotOutcome::ClearedEmpty) && snapshot_board_data; - session::catalog::record_named_session_saved(&options); - Ok(PersistenceOutcome::SaveAs { - report, - committed_board_data, - }) - } - PersistenceOperation::LoadConfigured { options } => Ok(PersistenceOutcome::Load( - session::load_snapshot_with_outcome(&options)?, - )), - PersistenceOperation::LoadNamedCandidate { options } => Ok(PersistenceOutcome::Load( - session::load_named_session_candidate(&options)?, - )), - PersistenceOperation::Inspect { options } => Ok(PersistenceOutcome::Inspection( - session::inspect_session(&options)?, - )), - PersistenceOperation::SaveAsOverwritePreflight { - current_path, - options, - } => { - // Validation must run before identity matching: identity canonicalization follows - // symlinks, while named foreground targets must reject them. - let target_path = options.session_file_path(); - session::validate_named_session_file_for_foreground(&target_path)?; - let (same_target, overwrite_required) = - save_as_preflight_after_validation(¤t_path, &target_path, || { - session::save_snapshot_as_requires_overwrite(&options) - })?; - Ok(PersistenceOutcome::SaveAsPreflight { - same_target, - overwrite_required, - }) - } - PersistenceOperation::ValidateNamedOpen { path } => { - session::validate_named_session_file_for_open(&path)?; - Ok(PersistenceOutcome::Unit) - } - PersistenceOperation::ClearToolState { options } => Ok( - PersistenceOutcome::ToolStateCleared(session::clear_tool_state(&options)?), - ), - PersistenceOperation::HasArtifacts { options } => Ok(PersistenceOutcome::HasArtifacts( - super::has_session_artifact(&options), - )), - PersistenceOperation::RecordNamedOpened { options } => { - session::catalog::record_named_session_opened(&options); - Ok(PersistenceOutcome::Unit) - } - PersistenceOperation::ForgetNamedSessionByPath { path } => Ok( - PersistenceOutcome::CatalogForgotten(session::catalog::forget_session_by_path(&path)?), - ), - #[cfg(test)] - PersistenceOperation::PanicForTest => { - panic!("intentional persistence worker panic for disconnect testing") - } - PersistenceOperation::Shutdown => Ok(PersistenceOutcome::Unit), - } -} - -fn save_as_preflight_after_validation( - current_path: &Path, - target_path: &Path, - discover_overwrite: impl FnOnce() -> Result, -) -> Result<(bool, bool)> { - let same_target = session::catalog::session_paths_match(current_path, target_path); - if same_target { - return Ok((true, false)); - } - Ok((false, discover_overwrite()?)) -} +mod test_support; fn assert_send_static() {} -fn log_snapshot_summary( - snapshot: &SessionSnapshot, - options: &SessionOptions, - strategy: SaveStrategy, -) { - let mut boards = 0usize; - let mut pages = 0usize; - let mut shapes = 0usize; - let mut undo_entries = 0usize; - let mut redo_entries = 0usize; - let mut visible_image_shapes = 0usize; - let mut visible_image_bytes = 0usize; - let mut max_history_depth = 0usize; - for board in &snapshot.boards { - boards += 1; - pages += board.pages.pages.len(); - for frame in &board.pages.pages { - let undo = frame.undo_stack_len(); - let redo = frame.redo_stack_len(); - shapes += frame.shapes.len(); - undo_entries += undo; - redo_entries += redo; - max_history_depth = max_history_depth.max(undo.max(redo)); - for drawn in &frame.shapes { - if let crate::draw::Shape::Image { data, .. } = &drawn.shape { - visible_image_shapes += 1; - visible_image_bytes = visible_image_bytes.saturating_add(data.bytes.len()); - } - } - } - } - log::info!( - "Persistence worker snapshot diagnostics for {} ({strategy:?}): boards={boards}, pages={pages}, shapes={shapes}, undo_entries={undo_entries}, redo_entries={redo_entries}, max_history_depth={max_history_depth}, visible_images={visible_image_shapes} ({visible_image_bytes} bytes), tool_state={}", - options.session_file_path().display(), - snapshot.tool_state.is_some() - ); -} - #[cfg(test)] mod tests { use std::os::fd::AsRawFd; use super::*; + use crate::session::SaveSnapshotOutcome; #[test] fn request_and_completion_payloads_are_send_and_static() { diff --git a/src/backend/wayland/session/persistence/test_support.rs b/src/backend/wayland/session/persistence/test_support.rs new file mode 100644 index 00000000..b3eb7502 --- /dev/null +++ b/src/backend/wayland/session/persistence/test_support.rs @@ -0,0 +1,63 @@ +//! Controlled channel peer for session driver tests, not a second command driver. +use super::{worker::execute, *}; + +impl PersistenceController { + pub(in crate::backend::wayland) fn controlled_for_test() -> (Self, ControlledPersistenceWorker) + { + let (request_tx, requests) = mpsc::sync_channel(1); + let (completions, completion_rx) = mpsc::sync_channel(1); + + ( + Self { + request_tx: Some(request_tx), + completion_rx, + worker: None, + active_id: None, + next_sequence: 0, + healthy: true, + }, + ControlledPersistenceWorker { + requests, + completions, + }, + ) + } +} + +pub(in crate::backend::wayland) struct ControlledPersistenceWorker { + requests: Receiver, + completions: SyncSender, +} + +impl ControlledPersistenceWorker { + pub(in crate::backend::wayland) fn complete_next(&self) { + self.respond_with(execute); + } + + pub(in crate::backend::wayland) fn respond_with( + &self, + result: impl FnOnce(PersistenceOperation) -> Result, + ) { + let request = self + .requests + .recv_timeout(Duration::from_secs(5)) + .expect("driver submitted work"); + let completion = PersistenceCompletion { + id: request.id, + result: result(request.operation), + queue_wait: Duration::ZERO, + execution_time: Duration::ZERO, + worker_thread_id: thread::current().id(), + finished_at: Instant::now(), + }; + + self.completions.send(completion).unwrap(); + } + + pub(in crate::backend::wayland) fn has_request(&self) -> bool { + match self.requests.try_recv() { + Err(TryRecvError::Empty) => false, + other => panic!("unexpected worker request: {other:?}"), + } + } +} diff --git a/src/backend/wayland/session/persistence/worker.rs b/src/backend/wayland/session/persistence/worker.rs new file mode 100644 index 00000000..3726285f --- /dev/null +++ b/src/backend/wayland/session/persistence/worker.rs @@ -0,0 +1,230 @@ +//! Background disk operations and completion-before-wake publication. +use std::path::Path; + +use super::*; +use crate::session::SaveSnapshotOutcome; + +pub(super) fn worker_main( + request_rx: Receiver, + completion_tx: SyncSender, + wake: RuntimeWakeHandle, +) { + let publisher = PersistenceCompletionPublisher::new(completion_tx, wake); + while let Ok(request) = request_rx.recv() { + let PersistenceRequest { + id, + queued_at, + operation, + } = request; + let queue_wait = queued_at.elapsed(); + let label = operation.label(); + let shutdown = matches!(operation, PersistenceOperation::Shutdown); + let started = Instant::now(); + log::debug!("Persistence worker starting {label} request {id:?}"); + let result = execute(operation); + let execution_time = started.elapsed(); + let worker_thread_id = thread::current().id(); + let finished_at = Instant::now(); + if !publisher.publish(PersistenceCompletion { + id, + result, + queue_wait, + execution_time, + worker_thread_id, + finished_at, + }) { + break; + } + if shutdown { + break; + } + } +} + +struct PersistenceCompletionPublisher { + completion_tx: Option>, + wake: RuntimeWakeHandle, +} + +impl PersistenceCompletionPublisher { + fn new(completion_tx: SyncSender, wake: RuntimeWakeHandle) -> Self { + Self { + completion_tx: Some(completion_tx), + wake, + } + } + + fn publish(&self, completion: PersistenceCompletion) -> bool { + let Some(completion_tx) = self.completion_tx.as_ref() else { + return false; + }; + if completion_tx.send(completion).is_err() { + return false; + } + if let Err(err) = self.wake.wake() { + log::error!("Failed to wake runtime after persistence completion: {err}"); + return false; + } + true + } +} + +impl Drop for PersistenceCompletionPublisher { + fn drop(&mut self) { + // Close the completion channel before waking. The event loop can therefore + // observe disconnect immediately even when the worker unwinds without a + // completion packet. + self.completion_tx.take(); + if let Err(err) = self.wake.wake() { + log::error!("Failed to wake runtime after persistence worker exit: {err}"); + } + } +} + +pub(super) fn execute(operation: PersistenceOperation) -> Result { + match operation { + PersistenceOperation::Save { + snapshot, + options, + strategy, + contentless_clear_boundary, + } => { + log_snapshot_summary(&snapshot, &options, strategy); + let snapshot_board_data = snapshot.has_board_data(); + let report = match strategy { + SaveStrategy::Autosave => { + session::save_snapshot_autosave_with_report_and_clear_boundary( + &snapshot, + &options, + contentless_clear_boundary, + )? + } + SaveStrategy::Normal => session::save_snapshot_with_report_and_clear_boundary( + &snapshot, + &options, + contentless_clear_boundary, + )?, + }; + let committed_board_data = report.as_ref().is_some_and(|report| { + !matches!(report.outcome, SaveSnapshotOutcome::ClearedEmpty) && snapshot_board_data + }); + Ok(PersistenceOutcome::Save(SaveCompletion { + report, + committed_board_data, + })) + } + PersistenceOperation::SaveAs { + snapshot, + options, + overwrite, + } => { + let snapshot_board_data = snapshot.has_board_data(); + let report = session::save_snapshot_as_with_report(&snapshot, &options, overwrite)?; + let committed_board_data = + !matches!(report.outcome, SaveSnapshotOutcome::ClearedEmpty) && snapshot_board_data; + session::catalog::record_named_session_saved(&options); + Ok(PersistenceOutcome::SaveAs { + report, + committed_board_data, + }) + } + PersistenceOperation::LoadConfigured { options } => Ok(PersistenceOutcome::Load( + session::load_snapshot_with_outcome(&options)?, + )), + PersistenceOperation::LoadNamedCandidate { options } => Ok(PersistenceOutcome::Load( + session::load_named_session_candidate(&options)?, + )), + PersistenceOperation::Inspect { options } => Ok(PersistenceOutcome::Inspection( + session::inspect_session(&options)?, + )), + PersistenceOperation::SaveAsOverwritePreflight { + current_path, + options, + } => { + // Validation must run before identity matching: identity canonicalization follows + // symlinks, while named foreground targets must reject them. + let target_path = options.session_file_path(); + session::validate_named_session_file_for_foreground(&target_path)?; + let (same_target, overwrite_required) = + save_as_preflight_after_validation(¤t_path, &target_path, || { + session::save_snapshot_as_requires_overwrite(&options) + })?; + Ok(PersistenceOutcome::SaveAsPreflight { + same_target, + overwrite_required, + }) + } + PersistenceOperation::ValidateNamedOpen { path } => { + session::validate_named_session_file_for_open(&path)?; + Ok(PersistenceOutcome::Unit) + } + PersistenceOperation::ClearToolState { options } => Ok( + PersistenceOutcome::ToolStateCleared(session::clear_tool_state(&options)?), + ), + PersistenceOperation::HasArtifacts { options } => Ok(PersistenceOutcome::HasArtifacts( + super::super::has_session_artifact(&options), + )), + PersistenceOperation::RecordNamedOpened { options } => { + session::catalog::try_record_named_session_opened(&options)?; + Ok(PersistenceOutcome::Unit) + } + PersistenceOperation::ForgetNamedSessionByPath { path } => Ok( + PersistenceOutcome::CatalogForgotten(session::catalog::forget_session_by_path(&path)?), + ), + #[cfg(test)] + PersistenceOperation::PanicForTest => { + panic!("intentional persistence worker panic for disconnect testing") + } + PersistenceOperation::Shutdown => Ok(PersistenceOutcome::Unit), + } +} + +pub(super) fn save_as_preflight_after_validation( + current_path: &Path, + target_path: &Path, + discover_overwrite: impl FnOnce() -> Result, +) -> Result<(bool, bool)> { + let same_target = session::catalog::session_paths_match(current_path, target_path); + if same_target { + return Ok((true, false)); + } + Ok((false, discover_overwrite()?)) +} + +fn log_snapshot_summary( + snapshot: &SessionSnapshot, + options: &SessionOptions, + strategy: SaveStrategy, +) { + let mut boards = 0usize; + let mut pages = 0usize; + let mut shapes = 0usize; + let mut undo_entries = 0usize; + let mut redo_entries = 0usize; + let mut visible_image_shapes = 0usize; + let mut visible_image_bytes = 0usize; + let mut max_history_depth = 0usize; + for board in &snapshot.boards { + boards += 1; + pages += board.pages.pages.len(); + for frame in &board.pages.pages { + let undo = frame.undo_stack_len(); + let redo = frame.redo_stack_len(); + shapes += frame.shapes.len(); + undo_entries += undo; + redo_entries += redo; + max_history_depth = max_history_depth.max(undo.max(redo)); + for drawn in &frame.shapes { + if let crate::draw::Shape::Image { data, .. } = &drawn.shape { + visible_image_shapes += 1; + visible_image_bytes = visible_image_bytes.saturating_add(data.bytes.len()); + } + } + } + } + log::info!( + "Persistence worker snapshot diagnostics for {} ({strategy:?}): boards={boards}, pages={pages}, shapes={shapes}, undo_entries={undo_entries}, redo_entries={redo_entries}, max_history_depth={max_history_depth}, visible_images={visible_image_shapes} ({visible_image_bytes} bytes), tool_state={}", + options.session_file_path().display(), + snapshot.tool_state.is_some() + ); +} diff --git a/src/backend/wayland/session/runtime.rs b/src/backend/wayland/session/runtime.rs index a055115d..5a2f2bb2 100644 --- a/src/backend/wayland/session/runtime.rs +++ b/src/backend/wayland/session/runtime.rs @@ -2,12 +2,13 @@ use super::*; use crate::session::{SaveAsOverwrite, SessionSnapshot, ToolStateSnapshot}; #[allow(dead_code)] -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug)] pub(in crate::backend::wayland) struct RuntimeOpenSessionReport { pub previous_path: PathBuf, pub opened_path: PathBuf, pub saved_current: bool, pub loaded_board_data: bool, + pub catalog_error: Option, } #[allow(dead_code)] diff --git a/src/backend/wayland/session/runtime/transaction.rs b/src/backend/wayland/session/runtime/transaction.rs index 70c00ea8..218dd98d 100644 --- a/src/backend/wayland/session/runtime/transaction.rs +++ b/src/backend/wayland/session/runtime/transaction.rs @@ -127,13 +127,12 @@ impl ExplicitSessionTransaction { } // Catalog bookkeeping is best effort after a successful open. - pub fn accept_catalog_failure(&self, error: &anyhow::Error) -> Option { - if matches!(self.phase, Phase::RecordOpen) { - log::warn!("Named session opened, but catalog update failed: {error:#}"); - Some(SessionCommandReport::Open(self.open_report())) - } else { - None - } + pub fn catalog_failure_report(&self, error: anyhow::Error) -> SessionCommandReport { + debug_assert!(self.has_committed_open()); + let mut report = self.open_report(); + report.catalog_error = Some(error); + + SessionCommandReport::Open(report) } fn work(&mut self, phase: Phase, operation: PersistenceOperation) -> Result { @@ -218,6 +217,7 @@ impl ExplicitSessionTransaction { .expect("command has current options") .session_file_path() } + fn open_report(&self) -> RuntimeOpenSessionReport { RuntimeOpenSessionReport { previous_path: self.current_path(), @@ -228,8 +228,10 @@ impl ExplicitSessionTransaction { .session_file_path(), saved_current: self.saved_current, loaded_board_data: self.loaded_board_data, + catalog_error: None, } } + fn apply_default_tools( &self, context: &mut SessionTransaction<'_>, diff --git a/src/backend/wayland/session/tests.rs b/src/backend/wayland/session/tests.rs index 18e14af7..6544b513 100644 --- a/src/backend/wayland/session/tests.rs +++ b/src/backend/wayland/session/tests.rs @@ -15,14 +15,14 @@ use std::sync::MutexGuard; #[cfg(unix)] use std::os::unix::fs::{PermissionsExt, symlink}; -struct EnvGuard { +pub(super) struct EnvGuard { _guard: MutexGuard<'static, ()>, catalog_hooks: Option, xdg_data_home: Option, } impl EnvGuard { - fn set_xdg_data_home(path: &Path) -> Self { + pub(super) fn set_xdg_data_home(path: &Path) -> Self { let guard = crate::test_env::lock(); let catalog_hooks = std::env::var_os(CATALOG_HOOKS_TEST_ENV); let xdg_data_home = std::env::var_os(XDG_DATA_HOME_ENV); diff --git a/src/backend/wayland/state.rs b/src/backend/wayland/state.rs index 49feebb5..42fa36d5 100644 --- a/src/backend/wayland/state.rs +++ b/src/backend/wayland/state.rs @@ -42,7 +42,7 @@ pub(in crate::backend::wayland) use self::core::overlay::{ OverlaySuppression, OverlaySuppressionKeyboardPolicy, }; pub(in crate::backend::wayland) use self::region_capture::WindowSnapDirection; -pub(in crate::backend::wayland) use self::toolbar::MoveDragKind; +pub(in crate::backend::wayland) use self::toolbar::{MoveDragKind, ToolbarChrome, ToolbarDrag}; use super::{ RuntimeOperationController, RuntimeOperationIdSource, capture::{CapturePreflightRequest, CaptureState, PendingPdfExport}, diff --git a/src/backend/wayland/state/core/session.rs b/src/backend/wayland/state/core/session.rs index 7f96d66f..954dd642 100644 --- a/src/backend/wayland/state/core/session.rs +++ b/src/backend/wayland/state/core/session.rs @@ -1,12 +1,16 @@ use anyhow::Result; use super::super::*; +use crate::backend::wayland::backend::event_loop::session_save::{ + handle_autosave_failure, handle_persistence_transport_failure, report_autosave_success, +}; use crate::backend::wayland::session::{ - ExplicitSessionTransaction, PersistenceCompletion, PersistenceController, SessionCommand, + ExplicitSessionTransaction, PersistenceController, SaveCompletion, SessionCommand, SessionCommandReport, SessionTransaction, driver::{self, SessionCommandRuntime}, }; use crate::session::ToolStateSnapshot; +use std::time::{Duration, Instant}; impl SessionCommandRuntime for WaylandState { fn session_context(&mut self) -> SessionTransaction<'_> { @@ -42,15 +46,15 @@ impl SessionCommandRuntime for WaylandState { } fn session_transport_failed(&mut self, error: &anyhow::Error) { - crate::backend::wayland::backend::event_loop::session_save::handle_persistence_transport_failure( - self, std::time::Instant::now(), error, - ); + handle_persistence_transport_failure(self, Instant::now(), error); + } + + fn autosave_succeeded(&mut self, save: SaveCompletion, execution_time: Duration) { + report_autosave_success(self, save, execution_time); } - fn apply_session_completion(&mut self, completion: PersistenceCompletion) -> Result<()> { - crate::backend::wayland::backend::event_loop::session_save::apply_persistence_completion( - self, completion, - ) + fn autosave_failed(&mut self, error: &anyhow::Error) { + handle_autosave_failure(self, Instant::now(), error); } } diff --git a/src/backend/wayland/state/toolbar/events/session.rs b/src/backend/wayland/state/toolbar/events/session.rs index 24579810..b459ed3a 100644 --- a/src/backend/wayland/state/toolbar/events/session.rs +++ b/src/backend/wayland/state/toolbar/events/session.rs @@ -415,10 +415,16 @@ impl WaylandState { report: SessionCommandReport, ) { match report { - SessionCommandReport::Open(report) => self.set_session_toolbar_info(format!( - "Opened session {}", - session_display_name(&report.opened_path) - )), + SessionCommandReport::Open(report) => { + let name = session_display_name(&report.opened_path); + if let Some(error) = report.catalog_error { + self.set_session_toolbar_error(format!( + "Opened session {name}; recent-session catalog update failed: {error:#}" + )); + } else { + self.set_session_toolbar_info(format!("Opened session {name}")); + } + } SessionCommandReport::SaveAs(report) => { self.clear_toolbar_save_as_overwrite_prompt(); self.set_session_toolbar_info(format!( diff --git a/src/session/catalog.rs b/src/session/catalog.rs index fc4f56bf..959d3eb9 100644 --- a/src/session/catalog.rs +++ b/src/session/catalog.rs @@ -7,16 +7,13 @@ use std::path::{Component, Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; -#[cfg(test)] -use crate::env_vars::CATALOG_HOOKS_TEST_ENV; - use super::lock::{lock_exclusive, open_runtime_lock_file, unlock}; -use super::options::SessionOptions; - +mod hooks; mod identity; -#[cfg(test)] -use identity::normalize_exact_path; +pub(crate) use hooks::{ + record_named_session_opened, record_named_session_saved, try_record_named_session_opened, +}; pub use identity::{CatalogPathIdentity, session_path_identity, session_paths_match}; use identity::{ display_name_for_path, entry_matches_identity, optional_path_to_string, path_to_string, @@ -202,59 +199,6 @@ pub fn move_session_path_by_id(id: &str, target_path: &Path) -> Result bool { - let Some(raw) = std::env::var_os(CATALOG_HOOKS_TEST_ENV) else { - return false; - }; - if raw.is_empty() || raw == std::ffi::OsStr::new("1") { - return true; - } - normalize_exact_path(path).starts_with(normalize_exact_path(Path::new(&raw))) -} - impl CatalogFile { fn upsert( &mut self, diff --git a/src/session/catalog/hooks.rs b/src/session/catalog/hooks.rs new file mode 100644 index 00000000..b884971d --- /dev/null +++ b/src/session/catalog/hooks.rs @@ -0,0 +1,77 @@ +//! Catalog integration policy for snapshot loading and committed runtime operations. +use anyhow::Result; +use log::warn; + +use super::{CatalogEvent, upsert_session_event}; +use crate::session::options::SessionOptions; + +#[cfg(test)] +use { + super::identity::normalize_exact_path, crate::env_vars::CATALOG_HOOKS_TEST_ENV, std::path::Path, +}; + +/// Record a committed named Open, preserving failures for runtime feedback. +pub(crate) fn try_record_named_session_opened(options: &SessionOptions) -> Result<()> { + if !options.is_named_file() { + return Ok(()); + } + + let path = options.session_file_path(); + + #[cfg(test)] + if !test_catalog_hooks_enabled_for_path(&path) { + return Ok(()); + } + + upsert_session_event(&path, CatalogEvent::Opened)?; + + Ok(()) +} + +/// Snapshot loading remains best effort: a catalog failure must not reject loaded data. +pub(crate) fn record_named_session_opened(options: &SessionOptions) { + if let Err(err) = try_record_named_session_opened(options) { + let path = options.session_file_path(); + warn!( + "Failed to update named session catalog after opening {}: {}", + path.display(), + err, + ); + } +} + +/// Saves keep successful disk writes successful even when catalog bookkeeping fails. +pub(crate) fn record_named_session_saved(options: &SessionOptions) { + if !options.is_named_file() { + return; + } + + let path = options.session_file_path(); + + #[cfg(test)] + if !test_catalog_hooks_enabled_for_path(&path) { + return; + } + + if path.is_file() + && let Err(err) = upsert_session_event(&path, CatalogEvent::Saved) + { + warn!( + "Failed to update named session catalog after saving {}: {}", + path.display(), + err, + ); + } +} + +#[cfg(test)] +fn test_catalog_hooks_enabled_for_path(path: &Path) -> bool { + let Some(raw) = std::env::var_os(CATALOG_HOOKS_TEST_ENV) else { + return false; + }; + if raw.is_empty() || raw == std::ffi::OsStr::new("1") { + return true; + } + + normalize_exact_path(path).starts_with(normalize_exact_path(Path::new(&raw))) +} From 95c41a884e271f43717feaa4f216192280d8ffea Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:58:37 +0200 Subject: [PATCH 5/6] fix(session): preserve slider previews across pending commands --- .../driver/tests/lifecycle_regressions.rs | 106 ++++++++++++++++++ src/input/state/core/session.rs | 1 + 2 files changed, 107 insertions(+) diff --git a/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs b/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs index a7bbc695..0d12c06b 100644 --- a/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs +++ b/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs @@ -1,6 +1,112 @@ use super::*; use crate::backend::wayland::{runtime_ui_state::ToolbarPositionSnapshot, state::MoveDragKind}; +#[test] +fn pending_destructive_commands_preserve_live_slider_edits_and_undo() { + use crate::input::state::{PropertiesPanelHit, SelectionPropertyKind}; + + for open in [false, true] { + let temp = crate::test_temp::tempdir().unwrap(); + let options = named_options(temp.path(), "current"); + let target = named_options(temp.path(), "candidate"); + stored_session::save_snapshot(&sample_snapshot(), &options).unwrap(); + stored_session::save_snapshot(&sample_snapshot(), &target).unwrap(); + let mut input = test_input_state(); + let id = add_line(&mut input, 51); + let original = input.boards.active_frame().shape(id).unwrap().shape.clone(); + let depth = input.boards.active_frame().undo_stack_len(); + let measurer = TextMeasurer::default(); + let mut session = SessionState::new(Some(options.clone())); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + let command = if open { + SessionCommand::Open(target.session_file_path()) + } else { + SessionCommand::Clear + }; + + start_session_command(&mut runtime, command).unwrap(); + if open { + worker.complete_next(); // preflight, followed by the held candidate load + runtime.receive(); + } + runtime.input.set_selection(vec![id]); + assert!(runtime.input.show_properties_panel_with(&measurer)); + let surface = cairo::ImageSurface::create(cairo::Format::ARgb32, 1, 1).unwrap(); + let ctx = cairo::Context::new(&surface).unwrap(); + runtime + .input + .update_properties_panel_layout(&ctx, 1280, 800); + let panel = runtime.input.properties_panel().unwrap(); + let row = panel + .entries + .iter() + .position(|entry| entry.kind == SelectionPropertyKind::Thickness) + .unwrap(); + let track = runtime + .input + .properties_panel_layout() + .unwrap() + .hit_rect(panel, PropertiesPanelHit::Slider(row)) + .unwrap(); + assert!(runtime.input.begin_properties_slider_drag_with( + &measurer, + row, + track.right() as i32 - 1 + )); + assert!( + !runtime.input.is_session_dirty(), + "preview has not committed yet" + ); + assert_eq!(runtime.input.boards.active_frame().undo_stack_len(), depth); + + worker.complete_next(); + runtime.receive(); + + let retained = runtime + .input + .boards + .active_frame() + .shape(id) + .expect("pending command must retain the edited shape"); + assert!(matches!( + retained.shape, + crate::draw::Shape::Line { thick: 50.0, .. } + )); + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + options.session_file_path() + ); + assert!(runtime.pending.is_none()); + assert!(runtime.reports.is_empty()); + assert!( + runtime.errors[0] + .to_string() + .contains("interaction changed") + ); + assert!(runtime.input.is_properties_slider_dragging()); + assert_eq!(runtime.input.boards.active_frame().undo_stack_len(), depth); + + runtime.input.finish_properties_slider_drag_with(&measurer); + assert!(runtime.input.is_session_dirty()); + assert_eq!( + runtime.input.boards.active_frame().undo_stack_len(), + depth + 1 + ); + runtime.input.handle_action_with_resources( + crate::input::state::InputTextResources { + measurer: &measurer, + ui_engine: &crate::ui_text::UiTextEngine::default(), + }, + crate::config::Action::Undo, + ); + assert_eq!( + runtime.input.boards.active_frame().shape(id).unwrap().shape, + original + ); + } +} + #[test] fn autosave_ownership_errors_do_not_publish_failures_or_delay_retry() { #[derive(Clone, Copy, Debug)] diff --git a/src/input/state/core/session.rs b/src/input/state/core/session.rs index 576b0d12..656eae59 100644 --- a/src/input/state/core/session.rs +++ b/src/input/state/core/session.rs @@ -149,6 +149,7 @@ impl InputState { || self.board_picker_is_page_dragging() || self.color_picker_popup_is_dragging() || self.radial_menu_is_size_dragging() + || self.is_properties_slider_dragging() } /// Transient edits have their own revision because they become session-dirty From 4ba9465d14dd5ac42d4817b101aadeb8d1f9a77c Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Sat, 3 Oct 2026 15:35:32 +0200 Subject: [PATCH 6/6] fix(session): recover retained state after rejected clears --- src/backend/wayland/session/driver.rs | 18 ++- src/backend/wayland/session/driver/tests.rs | 14 +- .../driver/tests/lifecycle_regressions.rs | 142 ++++++++++++++++++ 3 files changed, 172 insertions(+), 2 deletions(-) diff --git a/src/backend/wayland/session/driver.rs b/src/backend/wayland/session/driver.rs index 35354647..44cff107 100644 --- a/src/backend/wayland/session/driver.rs +++ b/src/backend/wayland/session/driver.rs @@ -100,6 +100,12 @@ fn advance_session_command( ) { observe_input_dirty(runtime, Instant::now()); + // A submitted board or tool-state clear can change disk before its receipt fails. + let clear_attempted = matches!( + transaction.command(), + SessionCommand::Clear | SessionCommand::ClearTools(_) + ) && result.is_some(); + // Catalog failure follows an already committed open; never roll back that canvas. let step = match result { Some(Err(error)) if transaction.has_committed_open() => Ok(TransactionStep::Complete( @@ -141,7 +147,17 @@ fn advance_session_command( runtime.finish_session_command(*report); } - Err(error) => runtime.fail_session_command(transaction.command(), &error), + Err(error) => { + // Resave retained state if a clear may have changed disk, without postponing + // already-dirty work or creating an undo entry for a persistence invalidation. + let context = runtime.session_context(); + let needs_recovery = clear_attempted && !context.session.is_dirty(); + context + .session + .record_input_dirty(Instant::now(), needs_recovery); + + runtime.fail_session_command(transaction.command(), &error); + } } } diff --git a/src/backend/wayland/session/driver/tests.rs b/src/backend/wayland/session/driver/tests.rs index 86393815..abd03273 100644 --- a/src/backend/wayland/session/driver/tests.rs +++ b/src/backend/wayland/session/driver/tests.rs @@ -576,7 +576,8 @@ fn open_refreshes_consumer_seeds_before_catalog_work_and_at_completion() { #[test] fn clear_completion_refreshes_consumer_seeds() { let temp = crate::test_temp::tempdir().unwrap(); - let options = named_options(temp.path(), "clear"); + let mut options = named_options(temp.path(), "clear"); + options.autosave_enabled = true; let mut input = test_input_state(); add_line(&mut input, 51); let mut session = SessionState::new(Some(options)); @@ -609,6 +610,17 @@ fn clear_completion_refreshes_consumer_seeds() { runtime.reports.as_slice(), [SessionCommandReport::Clear(_)] )); + assert!(!runtime.input.is_session_dirty()); + assert!(!runtime.session.is_dirty()); + let options = runtime.session.options().unwrap(); + assert!( + runtime + .session + .autosave_timeout(Instant::now(), options) + .is_none() + ); + assert!(options.clear_marker_file_path().exists()); + assert!(!options.session_file_path().exists()); } #[test] diff --git a/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs b/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs index 0d12c06b..1b8da484 100644 --- a/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs +++ b/src/backend/wayland/session/driver/tests/lifecycle_regressions.rs @@ -107,6 +107,148 @@ fn pending_destructive_commands_preserve_live_slider_edits_and_undo() { } } +fn assert_clean_saved_line( + runtime: &CommandRuntime<'_>, + options: &stored_session::SessionOptions, + x2: i32, +) { + let stored_session::LoadSnapshotOutcome::Loaded(snapshot) = + stored_session::load_snapshot_with_outcome(options).unwrap() + else { + panic!("expected saved session"); + }; + assert_eq!(snapshot.tool_state.unwrap().current_thickness, 11.0); + assert_eq!(runtime.input.thickness_for_active_tool(), 11.0); + assert!(options.session_file_path().exists()); + assert!(!options.clear_marker_file_path().exists()); + assert_eq!(loaded_line_x2(options), x2); + assert!(!runtime.input.is_session_dirty()); + assert!(!runtime.session.is_dirty()); + assert!( + runtime + .session + .autosave_timeout(Instant::now(), options) + .is_none() + ); +} + +#[test] +fn rejected_disk_clears_are_autosaved_after_an_uncommitted_gesture_is_canceled() { + for command in [ + SessionCommand::Clear, + SessionCommand::ClearTools(Box::new(stored_session::ToolStateSnapshot::from_config( + &Config::default(), + ))), + ] { + assert_rejected_disk_clear_recovers_after_canceled_gesture(command); + } +} + +fn assert_rejected_disk_clear_recovers_after_canceled_gesture(command: SessionCommand) { + let clears_boards = matches!(command, SessionCommand::Clear); + let temp = crate::test_temp::tempdir().unwrap(); + let mut options = named_options(temp.path(), "current"); + options.autosave_enabled = true; + options.autosave_idle = Duration::from_millis(10); + options.autosave_interval = Duration::from_secs(1); + let measurer = TextMeasurer::default(); + let mut input = test_input_state(); + let _ = input.set_thickness(11.0); + let id = add_line(&mut input, 42); + let original = input.boards.active_frame().shape(id).unwrap().shape.clone(); + let depth = input.boards.active_frame().undo_stack_len(); + let snapshot = input + .snapshot_for_persistence_with(&measurer, &options) + .unwrap(); + stored_session::save_snapshot(&snapshot, &options).unwrap(); + input.clear_session_dirty(); + let mut session = SessionState::new(Some(options.clone())); + session.mark_loaded(true); + session.mark_saved(Instant::now(), true); + let (persistence, worker) = PersistenceController::controlled_for_test(); + let mut runtime = CommandRuntime::new(&mut input, &measurer, &mut session, persistence); + assert_clean_saved_line(&runtime, &options, 42); + + start_session_command(&mut runtime, command).unwrap(); + runtime.input.on_mouse_press_with_canvas_and_resources( + crate::input::state::InputTextResources { + measurer: &measurer, + ui_engine: &crate::ui_text::UiTextEngine::default(), + }, + crate::input::MouseButton::Left, + 500, + 400, + 500, + 400, + ); + assert!(matches!( + runtime.input.state, + crate::input::DrawingState::Drawing { .. } + )); + assert!( + !runtime.input.is_session_dirty(), + "unfinished stroke has not committed" + ); + worker.complete_next(); + assert_disk_clear_applied(&options, clears_boards); + runtime.receive(); + assert!(runtime.pending.is_none()); + assert!(runtime.reports.is_empty()); + assert!( + runtime.errors[0] + .to_string() + .contains("interaction changed") + ); + + runtime.input.cancel_active_interaction_with(&measurer); + let now = Instant::now(); + observe_input_dirty(&mut runtime, now); + assert!(!runtime.input.has_active_pointer_interaction()); + assert_eq!( + runtime.input.boards.active_frame().shape(id).unwrap().shape, + original + ); + assert_eq!(runtime.input.boards.active_frame().undo_stack_len(), depth); + assert!( + runtime.session.is_dirty(), + "retained session state must be dirty after disk clear (clears_boards={clears_boards})" + ); + let delay = runtime + .session + .autosave_timeout(now, &options) + .expect("rejected disk clear must schedule recovery autosave"); + assert!(runtime.session.autosave_due(now + delay, &options)); + assert_eq!( + runtime.session.options().unwrap().session_file_path(), + options.session_file_path() + ); + + let snapshot = runtime + .input + .snapshot_for_persistence_with(&measurer, &options) + .unwrap(); + runtime.submit_autosave(snapshot, options.clone()); + worker.complete_next(); + runtime.receive(); + + assert_clean_saved_line(&runtime, &options, 42); +} + +fn assert_disk_clear_applied(options: &stored_session::SessionOptions, clears_boards: bool) { + if clears_boards { + assert!(!options.session_file_path().exists()); + assert!(options.clear_marker_file_path().exists()); + } else { + let stored_session::LoadSnapshotOutcome::Loaded(snapshot) = + stored_session::load_snapshot_with_outcome(options).unwrap() + else { + panic!("expected session after tool-state clear"); + }; + assert!(snapshot.tool_state.is_none()); + assert_eq!(loaded_line_x2(options), 42); + } +} + #[test] fn autosave_ownership_errors_do_not_publish_failures_or_delay_retry() { #[derive(Clone, Copy, Debug)]