Guard and preserve behavior forms across document lifecycle

crates/oa-desktop/src/app.rs

@@ -12,6 +12,9 @@ use oa_core::{
 use serde_json::{Value, json};
 use std::{collections::BTreeSet, path::PathBuf, sync::Arc};
 
+#[cfg(test)]
+mod behavior_lifecycle_tests;
+
 #[derive(Debug, Clone)]
 pub enum LoadedSnapshot {
     Source(DiskProject),
@@ -351,7 +354,10 @@ impl App {
         self.draft.as_ref().is_some_and(Draft::dirty)
     }
     pub fn dirty(&self) -> bool {
-        self.document.dirty() || self.draft_dirty() || self.source_dirty()
+        self.document.dirty()
+            || self.draft_dirty()
+            || self.source_dirty()
+            || self.behavior.has_pending()
     }
     fn sync_source(&mut self) {
         self.source_workspace.accept(self.document.bundle());
@@ -435,6 +441,11 @@ impl App {
         }
     }
     pub fn apply_source(&mut self) -> bool {
+        if self.behavior.has_pending() {
+            self.status =
+                "Apply or cancel the behavior form before applying source changes.".into();
+            return false;
+        }
         if !self.source_dirty() {
             return true;
         }
@@ -465,6 +476,10 @@ impl App {
         }
     }
     pub fn apply(&mut self) -> bool {
+        if self.behavior.has_pending() {
+            self.status = "Apply or cancel the behavior form before changing the model.".into();
+            return false;
+        }
         if self.source_dirty() {
             self.status =
                 "Apply all source drafts or revert them before changing the model.".into();
@@ -540,6 +555,30 @@ impl App {
             }
         }
     }
+    fn apply_behavior(&mut self) -> bool {
+        if !self.behavior.has_pending() {
+            return true;
+        }
+        if self.source_dirty() || self.draft_dirty() {
+            self.status = "Property, source and behavior drafts are retained. Revert the other drafts before applying the behavior form.".into();
+            return false;
+        }
+        match self.behavior.prepare_pending(self.project()) {
+            Ok(Some(command)) => {
+                if self.execute(command) {
+                    self.behavior.acknowledge_commit();
+                    true
+                } else {
+                    false
+                }
+            }
+            Ok(None) => true,
+            Err(error) => {
+                self.status = error;
+                false
+            }
+        }
+    }
     fn reset(&mut self, project: Project, path: Option<PathBuf>) {
         let window_size = self.window_size;
         let canvas_size = self.canvas_size;
@@ -674,7 +713,7 @@ impl App {
         ) {
             self.model_menu = None;
         }
-        if self.source_dirty()
+        if (self.source_dirty() || self.behavior.has_pending())
             && matches!(
                 message,
                 Message::Field(..)
@@ -687,7 +726,8 @@ impl App {
             )
         {
             self.status =
-                "Apply all source drafts or revert them before editing properties.".into();
+                "Apply or cancel pending source and behavior drafts before editing properties."
+                    .into();
             return Task::none();
         }
         if self.busy
@@ -768,6 +808,35 @@ impl App {
             self.canvas_epoch = self.canvas_epoch.wrapping_add(1);
             self.selected_bend = None;
         }
+        if self.behavior.has_pending()
+            && matches!(
+                message,
+                Message::Page(page) if page != Page::Behavior
+            )
+        {
+            self.status = "Apply or cancel the behavior form before leaving this view.".into();
+            return Task::none();
+        }
+        if self.behavior.has_pending()
+            && matches!(
+                message,
+                Message::BrowseGroup(_)
+                    | Message::SourceEdit(_)
+                    | Message::SourceCreateFile
+                    | Message::SourceCloseFile(_)
+                    | Message::SourceUndo
+                    | Message::SourceRedo
+                    | Message::SourceRecover
+                    | Message::SourceLoadIncludes
+                    | Message::Menu(_)
+                    | Message::Connection(_)
+                    | Message::PortAnchor(_)
+                    | Message::Route(_)
+            )
+        {
+            self.status = "Apply or cancel the behavior form before changing another view.".into();
+            return Task::none();
+        }
         let previous = self.selected;
         let previous_layer = self.layer;
         let entering_catalog =
@@ -1297,14 +1366,22 @@ impl App {
             }
             Message::InspectorTab(tab) => self.inspector_tab = tab,
             Message::Behavior(event) => {
-                if event.is_submit() && !self.apply() {
+                if event.starts_form() && !self.behavior.has_pending() && !self.apply() {
+                    return Task::none();
+                }
+                if event.is_submit() && (self.source_dirty() || self.draft_dirty()) {
+                    self.status = "Revert the other drafts before applying the behavior form. Your inputs are retained.".into();
                     return Task::none();
                 }
                 let mut state = std::mem::take(&mut self.behavior);
                 let result = state.update(self.project(), event);
                 self.behavior = state;
                 match result {
-                    Ok(Some(command)) => return self.handle(Message::Command(command)),
+                    Ok(Some(command)) => {
+                        if self.execute(command) {
+                            self.behavior.acknowledge_commit();
+                        }
+                    }
                     Ok(None) => {}
                     Err(error) => self.status = error,
                 }
@@ -1440,6 +1517,9 @@ impl App {
                 }
             }
             Message::ConfirmDelete => {
+                if !self.delete_pending || self.behavior.has_pending() {
+                    return Task::none();
+                }
                 self.delete_pending = false;
                 self.execute(Command::Delete(self.selected));
             }
@@ -1858,7 +1938,7 @@ impl App {
                 return self.handle(Message::Save(false));
             }
             Message::Save(save_as) => {
-                if !self.apply_source() || !self.apply() {
+                if !self.apply_behavior() || !self.apply_source() || !self.apply() {
                     self.continue_after_save = false;
                     self.pending = None;
                     return Task::none();

crates/oa-desktop/src/app/behavior_lifecycle_tests.rs

@@ -0,0 +1,398 @@
+use super::*;
+use crate::behavior::{self, states};
+use oa_core::source_project::ProjectLimits;
+
+struct Fixture {
+    app: App,
+    machine: Id,
+    source: Id,
+    target: Id,
+    work: Id,
+}
+
+fn fixture() -> Fixture {
+    let mut project = Project::empty();
+    let owner = project
+        .add_element(Kind::System, Layer::Sa, "System", None)
+        .unwrap();
+    let machine = project
+        .add_element(
+            Kind::StateMachine,
+            Layer::Sa,
+            "Operating modes",
+            Some(owner),
+        )
+        .unwrap();
+    let region = project
+        .add_element(Kind::Region, Layer::Sa, "Operation", Some(machine))
+        .unwrap();
+    let source = project
+        .add_element(Kind::State, Layer::Sa, "Ready", Some(region))
+        .unwrap();
+    let target = project
+        .add_element(Kind::State, Layer::Sa, "Waiting", Some(region))
+        .unwrap();
+    let work = project
+        .add_element(Kind::Function, Layer::Sa, "Monitor connection", None)
+        .unwrap();
+    let mut app = App::with_project(project);
+    send(&mut app, Message::Select(source));
+    send(&mut app, Message::Page(Page::Behavior));
+    Fixture {
+        app,
+        machine,
+        source,
+        target,
+        work,
+    }
+}
+
+fn send(app: &mut App, message: Message) {
+    // Do not poll returned I/O tasks: these tests exercise the App transaction
+    // boundary without displaying a file dialog or writing to the filesystem.
+    let _ = app.update(message);
+}
+
+fn state_event(app: &mut App, event: states::Event) {
+    send(app, Message::Behavior(behavior::Event::States(event)));
+}
+
+fn choice(app: &App, id: Id) -> Choice {
+    Choice {
+        id,
+        label: app.project().named(id).into(),
+    }
+}
+
+fn start_transition(fixture: &mut Fixture, complete: bool) {
+    let Fixture {
+        app,
+        machine,
+        source,
+        target,
+        work,
+    } = fixture;
+    state_event(
+        app,
+        states::Event::Start(*machine, states::Tool::Transition, *source),
+    );
+    for event in [
+        states::Event::Name("Wait for recovery".into()),
+        states::Event::Trigger(choice(app, *work)),
+        states::Event::Effect(choice(app, *work)),
+        states::Event::TriggerNotes("A monitor result".into()),
+        states::Event::Guard("connection unavailable".into()),
+        states::Event::EffectNotes("Record the timeout".into()),
+        states::Event::Timed(true),
+        states::Event::TimeKind("At".into()),
+        states::Event::Expression("12:30 UTC".into()),
+    ] {
+        state_event(app, event);
+    }
+    if complete {
+        let target = choice(app, *target);
+        state_event(app, states::Event::Target(target));
+    }
+    assert_eq!(app.behavior.pending_owner(), Some(*machine));
+}
+
+fn assert_typed_transition(app: &App, source: Id, target: Id, work: Id) {
+    assert_eq!(app.project().relations.len(), 1);
+    let relation = app.project().relations.values().next().unwrap();
+    assert_eq!(relation.name, "Wait for recovery");
+    let RelationBody::StateTransition {
+        source: actual_source,
+        target: actual_target,
+        triggers,
+        effects,
+        trigger,
+        guard,
+        effect,
+        ..
+    } = &relation.body
+    else {
+        panic!("Expected the prepared state transition");
+    };
+    assert_eq!((*actual_source, *actual_target), (source, target));
+    assert_eq!(effects, &[work]);
+    assert_eq!(trigger, "A monitor result");
+    assert_eq!(guard, "connection unavailable");
+    assert_eq!(effect, "Record the timeout");
+    assert_eq!(triggers.len(), 2);
+    assert_eq!(triggers[0], work);
+    assert!(matches!(
+        app.project().elements[&triggers[1]].shared(),
+        Some(SharedBody::Event {
+            body: EventBody::Time { kind: TimeEventKind::At, expression }
+        }) if expression == "12:30 UTC"
+    ));
+    assert_eq!(
+        app.project()
+            .elements
+            .values()
+            .filter(|e| e.kind() == Kind::Event)
+            .count(),
+        1,
+        "Retry must not leave a time event from the rejected transaction"
+    );
+    assert_eq!(
+        source_project::compile_bundle(app.document.bundle(), LIMITS)
+            .unwrap()
+            .decoded
+            .project,
+        *app.project()
+    );
+}
+
+#[test]
+fn source_writeback_failure_keeps_typed_behavior_form_for_repair_and_one_undo() {
+    let mut fixture = fixture();
+    let bundle = fixture.app.document.bundle().clone();
+    let mut limits = ProjectLimits::default();
+    limits.model.max_source_bytes = bundle.files.values().map(String::len).sum::<usize>() + 2000;
+    fixture.app.document = Document::from_bundle(bundle.clone(), limits).unwrap();
+    fixture.app.sync();
+    let before = fixture.app.document.snapshot();
+    start_transition(&mut fixture, true);
+    let app = &mut fixture.app;
+    state_event(app, states::Event::Name("Transition".repeat(1024)));
+    let prepared = app
+        .behavior
+        .prepare_pending(app.project())
+        .unwrap()
+        .unwrap();
+    assert!(execute_batch(app.project(), &[prepared]).is_ok());
+
+    state_event(app, states::Event::Submit);
+    assert!(Arc::ptr_eq(&before, &app.document.snapshot()));
+    assert_eq!(app.source_workspace.bundle(), bundle);
+    assert_eq!(app.document.revision(), 0);
+    assert!(!app.document.can_undo());
+    assert!(app.behavior.has_pending());
+    assert!(app.dirty());
+    assert!(!app.source_dirty());
+
+    state_event(app, states::Event::Name("Wait for recovery".into()));
+    state_event(app, states::Event::Submit);
+    assert!(!app.behavior.has_pending());
+    assert_typed_transition(app, fixture.source, fixture.target, fixture.work);
+    assert_eq!(app.document.revision(), 1);
+    assert_eq!(app.source_workspace.bundle(), *app.document.bundle());
+    let generated_files: Vec<_> = app
+        .document
+        .bundle()
+        .files
+        .iter()
+        .filter(|(path, _)| !bundle.files.contains_key(*path))
+        .map(|(path, text)| (path.clone(), text.clone()))
+        .collect();
+    assert!(
+        !generated_files.is_empty(),
+        "The time event creates a source file"
+    );
+    send(app, Message::Undo);
+    assert_eq!(app.document.bundle(), &bundle);
+    assert_eq!(app.project(), &before.decoded.project);
+    assert_eq!(
+        source_workspace::reachable(&app.source_workspace.bundle()).unwrap(),
+        bundle
+    );
+    assert_eq!(
+        app.source_workspace.reachable,
+        bundle.files.keys().cloned().collect::<BTreeSet<_>>()
+    );
+    // Undo restores the accepted bundle exactly. Workspace retains clean buffers
+    // from removed includes so the user may inspect or explicitly close them.
+    for (path, text) in generated_files {
+        assert!(!app.source_workspace.reachable.contains(&path));
+        let detached = &app.source_workspace.files[&path];
+        assert_eq!(detached.text, text);
+        assert!(!detached.dirty());
+    }
+    assert!(!app.source_workspace.detached_dirty());
+    assert!(!app.document.can_undo());
+    assert!(!app.dirty());
+}
+
+#[test]
+fn incomplete_behavior_form_is_dirty_close_save_retains_it_and_cancel_allows_switching() {
+    let mut fixture = fixture();
+    let before = fixture.app.document.snapshot();
+    start_transition(&mut fixture, false);
+    let app = &mut fixture.app;
+    assert!(!app.document.dirty());
+    assert!(app.dirty());
+    assert!(app.title().contains(" *"));
+    let window = window::Id::unique();
+    send(app, Message::Request(Pending::Close(window)));
+    assert!(matches!(app.pending, Some(Pending::Close(id)) if id == window));
+    send(app, Message::SaveAndContinue);
+    assert!(app.behavior.has_pending());
+    assert!(Arc::ptr_eq(&before, &app.document.snapshot()));
+    assert!(app.pending.is_none());
+    assert!(!app.continue_after_save);
+    assert!(!app.busy);
+    assert!(app.file_prompt.is_none());
+    assert_eq!(app.page, Page::Behavior);
+
+    state_event(app, states::Event::Cancel);
+    assert!(!app.behavior.has_pending());
+    assert!(!app.dirty());
+    send(app, Message::Page(Page::Text));
+    assert_eq!(app.page, Page::Text);
+    send(app, Message::Select(fixture.target));
+    assert_eq!(app.selected, fixture.target);
+}
+
+#[test]
+fn save_prepares_behavior_before_opening_destination_and_cancel_keeps_one_undo() {
+    let mut fixture = fixture();
+    let before = fixture.app.document.snapshot();
+    start_transition(&mut fixture, true);
+    let app = &mut fixture.app;
+    send(app, Message::Save(true));
+    assert!(!app.behavior.has_pending());
+    assert_typed_transition(app, fixture.source, fixture.target, fixture.work);
+    assert_eq!(app.document.revision(), 1);
+    assert!(
+        app.document.dirty(),
+        "Choosing a destination is not a successful disk save"
+    );
+    assert!(app.file_prompt.is_some() || app.busy);
+    if app.file_prompt.is_some() {
+        send(app, Message::CancelFile);
+    } else {
+        send(app, Message::SavePath(None));
+    }
+    assert!(!app.busy);
+    assert!(app.document.dirty());
+    send(app, Message::Undo);
+    assert_eq!(app.document.bundle(), &before.bundle);
+    assert_eq!(app.project(), &before.decoded.project);
+    assert!(!app.document.can_undo());
+    assert!(!app.dirty());
+}
+
+#[test]
+fn pending_behavior_blocks_navigation_property_edits_and_model_history() {
+    let mut fixture = fixture();
+    let before = fixture.app.document.snapshot();
+    start_transition(&mut fixture, true);
+    let app = &mut fixture.app;
+    let properties = app.draft.as_ref().unwrap().materialize().unwrap();
+    for message in [
+        Message::Page(Page::Diagram),
+        Message::Page(Page::Catalog),
+        Message::Page(Page::Text),
+        Message::Page(Page::Guide),
+        Message::Layer(Layer::Oa),
+        Message::Select(fixture.target),
+        Message::Explore(Some(fixture.target)),
+        Message::BrowseGroup(crate::explorer::Group::Work),
+        Message::Field("/name".into(), "Overwrite properties".into()),
+        Message::Command(Command::Rename(fixture.source, "Overwrite model".into())),
+        Message::ConfirmDelete,
+        Message::Undo,
+        Message::Redo,
+        Message::ShortcutUndo(false),
+        Message::SourceForSelection,
+        Message::SourceNavigation(crate::source_navigation::Event::Open(false)),
+    ] {
+        send(app, message);
+        assert_eq!(app.page, Page::Behavior);
+        assert_eq!(app.selected, fixture.source);
+        assert_eq!(app.layer, Layer::Sa);
+        assert_eq!(app.behavior.pending_owner(), Some(fixture.machine));
+        assert!(Arc::ptr_eq(&before, &app.document.snapshot()));
+        assert_eq!(
+            app.draft.as_ref().unwrap().materialize().unwrap(),
+            properties
+        );
+        assert!(!app.source_dirty());
+    }
+}
+
+#[test]
+fn passive_behavior_canvas_events_do_not_apply_property_drafts() {
+    let mut fixture = fixture();
+    let app = &mut fixture.app;
+    let before = app.document.snapshot();
+    send(
+        app,
+        Message::Field("/name".into(), "Unapplied state name".into()),
+    );
+    assert!(app.draft_dirty());
+    for event in [
+        behavior::Event::States(states::Event::Viewport(iced::Size::new(900., 600.))),
+        behavior::Event::States(states::Event::Zoom(0.7)),
+        behavior::Event::States(states::Event::Fit),
+        behavior::Event::Topology(behavior::topology::Event::Viewport(iced::Size::new(
+            900., 600.,
+        ))),
+        behavior::Event::Topology(behavior::topology::Event::Zoom(0.7)),
+        behavior::Event::Topology(behavior::topology::Event::Fit),
+    ] {
+        send(app, Message::Behavior(event));
+        assert!(Arc::ptr_eq(&before, &app.document.snapshot()));
+        assert!(app.draft_dirty());
+        assert!(!app.behavior.has_pending());
+        assert_eq!(app.source_workspace.bundle(), before.bundle);
+        assert!(!app.document.can_undo());
+    }
+}
+
+#[test]
+fn pending_behavior_blocks_source_creation_and_source_history_replay() {
+    for redo in [false, true] {
+        let mut fixture = fixture();
+        let app = &mut fixture.app;
+        let baseline = app.source_workspace.bundle();
+        let edited = format!(
+            "{}\n// Source draft that must stay in history.\n",
+            app.source_workspace.text()
+        );
+        send(app, Message::SourceEdit(text_editor::Action::SelectAll));
+        send(
+            app,
+            Message::SourceEdit(text_editor::Action::Edit(text_editor::Edit::Paste(
+                Arc::new(edited),
+            ))),
+        );
+        send(
+            app,
+            if redo {
+                Message::SourceUndo
+            } else {
+                Message::RevertSource
+            },
+        );
+        assert!(!app.source_dirty());
+        start_transition(&mut fixture, true);
+        let app = &mut fixture.app;
+        send(
+            app,
+            if redo {
+                Message::SourceRedo
+            } else {
+                Message::SourceUndo
+            },
+        );
+        assert_eq!(
+            app.source_workspace.bundle(),
+            baseline,
+            "Source history must not create a competing draft"
+        );
+        send(app, Message::SourceNewPath("queued.arcadia".into()));
+        send(app, Message::SourceCreateFile);
+        assert_eq!(
+            app.source_workspace.bundle(),
+            baseline,
+            "A queued create-file action must not create a competing draft"
+        );
+        assert!(!app.source_dirty());
+        assert_eq!(app.behavior.pending_owner(), Some(fixture.machine));
+        state_event(app, states::Event::Submit);
+        assert!(!app.behavior.has_pending());
+        assert_typed_transition(app, fixture.source, fixture.target, fixture.work);
+    }
+}

crates/oa-desktop/src/behavior.rs

@@ -14,10 +14,10 @@ use iced::{
 };
 use oa_core::*;
 use std::collections::BTreeMap;
-mod scenario;
+pub(crate) mod scenario;
 mod state_scene;
-mod states;
-mod topology;
+pub(crate) mod states;
+pub(crate) mod topology;
 #[derive(Debug, Clone)]
 pub enum Event {
     Topology(topology::Event),
@@ -26,6 +26,13 @@ pub enum Event {
     Submit,
 }
 impl Event {
+    pub fn starts_form(&self) -> bool {
+        matches!(self,
+            Self::Topology(topology::Event::Start(..))
+                | Self::Scenario(scenario::Event::Start(..))
+                | Self::States(states::Event::Start(..) | states::Event::Edit(..))
+        )
+    }
     pub fn is_submit(&self) -> bool {
         matches!(
             self,
@@ -123,7 +130,7 @@ pub fn view(app: &App) -> Element<'_, Message> {
             label: format!("{} · {}", e.kind(), e.name),
         })
         .collect::<Vec<_>>();
-    let owner = selected_behavior(p, app.selected);
+    let owner = app.behavior.pending_owner().or_else(|| selected_behavior(p, app.selected));
     let selected = owner
         .and_then(|id| choices.iter().find(|c| c.id == id))
         .or(choices.first())