Merge main into #801

apps/figcraft/src/recovery.rs

@@ -120,8 +120,7 @@ impl Recovery {
         (0..10).find_map(|n| {
             let dir = root.join(if n == 0 { pid.to_string() } else { format!("{pid}-{n}") });
             std::fs::create_dir_all(&dir).ok()?;
-            let lock = File::options().write(true).create(true).truncate(false).open(dir.join(LOCK)).ok()?;
-            lock.try_lock().ok()?;
+            let lock = lock_folder(&dir).ok()?;
             Some(Self { dir, _lock: lock, written: HashMap::new(), next: 0.0, waiting: Vec::new() })
         })
     }
@@ -293,6 +292,17 @@ fn remove_kept(w: Waiting) {
 /// The extension of the note beside the copy of a document that is one of your online files.
 const ONLINE: &str = "online";
 
+/// How long the startup waits, once, for folder locks that may free themselves (#334).
+#[cfg(unix)]
+const LOCK_WAIT: std::time::Duration = std::time::Duration::from_millis(100);
+
+/// Lock the folder `dir` (`WouldBlock` while another process holds it).
+fn lock_folder(dir: &Path) -> Result<File, std::fs::TryLockError> {
+    let lock = File::options().write(true).create(true).truncate(false).open(dir.join(LOCK)).map_err(std::fs::TryLockError::Error)?;
+    lock.try_lock()?;
+    Ok(lock)
+}
+
 /// The copy at `file` is of one of your online files: join that file again as its owner, keeping
 /// the copy to compare with what arrives. The session, or `None` when the copy isn't one (or its
 /// file isn't recorded any more, or is open already): then it opens as any other copy.
@@ -362,6 +372,7 @@ pub fn recover_all(root: &Path, s: &mut Session) -> Recovered {
             }
         }
     };
+    let (mut free, mut held) = (Vec::new(), Vec::new());
     for e in entries.flatten() {
         let path = e.path();
         if !path.is_dir() {
@@ -369,10 +380,22 @@ pub fn recover_all(root: &Path, s: &mut Session) -> Recovered {
             continue;
         }
         // Still running (another window of the app), or not ours to take.
-        let Ok(lock) = File::options().write(true).create(true).truncate(false).open(path.join(LOCK)) else { continue };
-        if lock.try_lock().is_err() {
-            continue;
+        match lock_folder(&path) {
+            Ok(lock) => free.push((path, lock)),
+            Err(std::fs::TryLockError::WouldBlock) => held.push(path),
+            Err(_) => {}
         }
+    }
+    // A child between its `fork` and its `exec` shares the run's open descriptors, and a `flock`
+    // lasts while any of them is open, so an ended run's folder can look held for a moment. Wait
+    // once, however many folders are held, and try those again (#334). Windows doesn't pass the
+    // lock to child processes.
+    #[cfg(unix)]
+    if !held.is_empty() {
+        std::thread::sleep(LOCK_WAIT);
+        free.extend(held.into_iter().filter_map(|path| lock_folder(&path).ok().map(|lock| (path, lock))));
+    }
+    for (path, lock) in free {
         let lock = Arc::new(lock);
         let files: Vec<PathBuf> = std::fs::read_dir(&path).into_iter().flatten().flatten().map(|f| f.path()).collect();
         for f in &files {

apps/figcraft/src/recovery_tests.rs

@@ -203,6 +203,29 @@ fn a_run_whose_folder_name_is_still_held_takes_another() {
     let _ = std::fs::remove_dir_all(&root);
 }
 
+/// #334: a lock that frees itself is not a run that is still going. A child between its `fork` and
+/// its `exec` inherits the run's open descriptors and a `flock` lasts while any descriptor for the
+/// file is open, so a crashed run's copies must still come back once that child has exec'd.
+#[cfg(unix)]
+#[test]
+fn a_lock_freed_a_moment_later_is_not_a_run_that_is_still_going() {
+    let root = root("fork-window");
+    crashed(&edited(), &root);
+    // Hold the crashed run's folder lock on a second descriptor, the way a child forked before its
+    // exec holds it, and free it shortly after.
+    let lock = std::fs::File::options().write(true).create(true).truncate(false).open(root.join("crashed").join("lock")).unwrap();
+    lock.try_lock().unwrap();
+    let freed = std::thread::spawn(move || {
+        std::thread::sleep(std::time::Duration::from_millis(10));
+        drop(lock);
+    });
+    // The next start waits for that lock rather than reading the run as still going.
+    let mut next = Session::empty();
+    assert_eq!(recover(&root, &mut next), 1, "the copies come back once the lock frees itself");
+    freed.join().unwrap();
+    let _ = std::fs::remove_dir_all(&root);
+}
+
 /// The rectangles in `doc`.
 fn rectangles(doc: &figcraft_engine::doc::Document) -> usize {
     doc.nodes().filter(|n| matches!(n.kind, figcraft_engine::doc::NodeKind::Rectangle)).count()

crates/collab/src/change.rs

@@ -87,8 +87,9 @@ macro_rules! node_props {
     ($($variant:ident => $field:ident),* $(,)?) => {
         /// A node property: the tree position (`parent`, `position`) or one saved field of
         /// [`Node`]. List-valued fields (fills, text runs, vector networks…) are one property each,
-        /// replaced whole (three-way merges line fills, strokes and effects up item by item, see
-        /// [`merge`](crate::merge)). New properties go at the end (the binary form numbers them).
+        /// replaced whole (three-way merges line the lists whose items stand on their own, such as
+        /// fills, up item by item, see [`merge`](crate::merge)). New properties go at the end (the
+        /// binary form numbers them).
         #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
         #[serde(rename_all = "camelCase")]
         pub enum Prop {

crates/collab/src/merge.rs

@@ -4,12 +4,13 @@
 //!
 //! A conflict is the same property (or document-level entry) changed on both sides to different
 //! values, or a layer deleted on one side while the other side edited it or something inside it
-//! (including adding layers to it or moving layers into or out of it). Fills, strokes and effects
-//! merge item by item: both sides changing different items of the list (or one adding items at its
-//! end) is no conflict, only the same item changed both ways or the list shortened on a side
-//! while the other changed it. Nothing that conflicts is
-//! merged until it is resolved: each conflict is resolved for one side, and the other side's
-//! changes to it are left out. Everything else from both sides is kept.
+//! (including adding layers to it or moving layers into or out of it). The layer lists whose items
+//! stand on their own (fills, strokes, effects, layout grids, export settings, guides, prototype
+//! interactions, accessibility annotations and Dev Mode resources) merge item by item: both sides
+//! changing different items of the list (or one adding items at its end) is no conflict, only the
+//! same item changed both ways or the list shortened on a side while the other changed it. Nothing
+//! that conflicts is merged until it is resolved: each conflict is resolved for one side, and the
+//! other side's changes to it are left out. Everything else from both sides is kept.
 
 use std::collections::{BTreeMap, BTreeSet};
 use std::fmt;
@@ -159,8 +160,23 @@ fn by_key(changes: &[Change]) -> BTreeMap<Key, Vec<usize>> {
     out
 }
 
-/// List properties whose items merge one by one.
-const PER_ITEM: &[Prop] = &[Prop::Fills, Prop::Strokes, Prop::Effects];
+/// List properties whose items merge one by one: the items stand on their own, so both sides
+/// changing different ones (or one side adding at its end) is no conflict. Sets and keyed maps
+/// (`bound`, `varModes`, `overrides`, `propRefs`, `pluginData`) are not here: their items line up
+/// by key, not by position.
+const PER_ITEM: &[Prop] = &[
+    Prop::Fills,
+    Prop::Strokes,
+    Prop::Effects,
+    Prop::Grids,
+    Prop::Exports,
+    Prop::Guides,
+    Prop::Interactions,
+    Prop::Annotations,
+    // Not `CodeLinks`: there is one per language, and lining them up by position could leave two
+    // for one language (the second hidden), so they merge whole.
+    Prop::DevResources,
+];
 
 /// Both sides' writes `b` and `m` of list property `key` merged item by item against the base
 /// (see [`merge_items`]); `None` when `key` isn't such a list or the writes conflict.

crates/collab/src/merge_list_tests.rs

@@ -1,8 +1,10 @@
-//! Fills, strokes and effects merge item by item: different items changed on each side merge,
-//! the same item changed both ways conflicts.
+//! The layer lists whose items stand on their own (fills, strokes, effects, layout grids, export
+//! settings, guides, prototype interactions, accessibility annotations and Dev Mode resources)
+//! merge item by item: different items changed on each side merge, the same item changed both ways
+//! conflicts.
 
 use figcraft_color::{Color, Paint};
-use figcraft_doc::{Document, Effect, NodeId, NodeKind};
+use figcraft_doc::{Document, Effect, LayoutGrid, NodeId, NodeKind, proto};
 use serde_json::{Value, json};
 
 use crate::merge::{Key, Side, merge_items, plan};
@@ -100,19 +102,113 @@ fn the_same_item_changed_both_ways_conflicts() {
 }
 
 #[test]
-fn other_lists_still_conflict_as_a_whole() {
-    // Docs silent: lists other than fills, strokes and effects (export settings here) still
-    // conflict whole when both sides change them.
+fn the_other_item_lists_merge_one_by_one_too() {
+    // Layout grids, export settings, guides, accessibility annotations and Dev Mode resources are
+    // item lists like fills: both sides changing different items merges.
     let (mut base, _, _, r) = files();
     let export = |suffix: &str| figcraft_doc::ExportSetting { suffix: suffix.into(), ..Default::default() };
-    base.get_mut(r).unwrap().exports = vec![export("@1"), export("@2")];
+    let guide = |vertical: bool, offset: f64| figcraft_doc::Guide { vertical, offset };
+    let annot = |label: &str| figcraft_doc::Annotation { label: label.into(), properties: Vec::new(), category: None };
+    let grid = |size: f64| LayoutGrid { size, ..Default::default() };
+    let res = |url: &str| figcraft_doc::DevResource { url: url.into(), name: String::new() };
+    let n = base.get_mut(r).unwrap();
+    n.exports = vec![export("@1"), export("@2")];
+    n.guides = vec![guide(true, 10.0), guide(false, 20.0)];
+    n.annotations = vec![annot("a1"), annot("a2")];
+    n.grids = vec![grid(10.0), grid(20.0)];
+    n.dev_resources = vec![res("https://a.test"), res("https://b.test")];
     let (mut branch, mut main) = (base.clone(), base.clone());
     branch.set_client_id(base.highest_id_space() + 1);
-    branch.get_mut(r).unwrap().exports[0] = export("@b");
-    main.get_mut(r).unwrap().exports[1] = export("@m");
+    // The branch changes the first item of each list; main the second.
+    let b = branch.get_mut(r).unwrap();
+    (b.exports[0].suffix, b.guides[0].offset, b.annotations[0].label, b.grids[0].size, b.dev_resources[0].url) =
+        ("@b".into(), 11.0, "b1".into(), 12.0, "https://branch.test".into());
+    let m = main.get_mut(r).unwrap();
+    (m.exports[1].suffix, m.guides[1].offset, m.annotations[1].label, m.grids[1].size, m.dev_resources[1].url) =
+        ("@m".into(), 22.0, "m2".into(), 24.0, "https://main.test".into());
+    both_ways(&base, &branch, &main, |doc| {
+        let n = doc.node(r).unwrap();
+        let suffixes: Vec<&str> = n.exports.iter().map(|e| e.suffix.as_str()).collect();
+        assert_eq!(suffixes, ["@b", "@m"], "export settings");
+        let offsets: Vec<f64> = n.guides.iter().map(|g| g.offset).collect();
+        assert_eq!(offsets, [11.0, 22.0], "guides");
+        let labels: Vec<&str> = n.annotations.iter().map(|a| a.label.as_str()).collect();
+        assert_eq!(labels, ["b1", "m2"], "annotations");
+        let sizes: Vec<f64> = n.grids.iter().map(|g| g.size).collect();
+        assert_eq!(sizes, [12.0, 24.0], "layout grids");
+        let urls: Vec<&str> = n.dev_resources.iter().map(|d| d.url.as_str()).collect();
+        assert_eq!(urls, ["https://branch.test", "https://main.test"], "dev resources");
+    });
+}
+
+#[test]
+fn the_same_item_of_another_list_changed_both_ways_conflicts() {
+    let (mut base, _, _, r) = files();
+    let link = |language: &str| figcraft_doc::CodeLink { language: language.into(), ..Default::default() };
+    let n = base.get_mut(r).unwrap();
+    n.exports = vec![
+        figcraft_doc::ExportSetting { suffix: "@1".into(), ..Default::default() },
+        figcraft_doc::ExportSetting { suffix: "@2".into(), ..Default::default() },
+    ];
+    n.code_links = vec![link("react"), link("vue")];
+    let (mut branch, mut main) = (base.clone(), base.clone());
+    branch.set_client_id(base.highest_id_space() + 1);
+    branch.get_mut(r).unwrap().exports[0].suffix = "@b".into();
+    main.get_mut(r).unwrap().exports[0].suffix = "@m".into();
+    // Code links are one per language, not an item list: different links changed still conflict.
+    branch.get_mut(r).unwrap().code_links[0].name = "B".into();
+    main.get_mut(r).unwrap().code_links[1].name = "M".into();
     let p = plan(&base, &branch, &main).unwrap();
-    let keys: Vec<String> = p.conflicts.iter().map(|c| c.key.to_string()).collect();
-    assert_eq!(keys, [format!("{r}/exports")]);
+    let mut keys: Vec<String> = p.conflicts.iter().map(|c| c.key.to_string()).collect();
+    keys.sort();
+    let mut want = [format!("{r}/codeLinks"), format!("{r}/exports")];
+    want.sort();
+    assert_eq!(keys, want);
+}
+
+#[test]
+fn interactions_merge_item_by_item() {
+    let ia = |duration_ms: u32| proto::Interaction {
+        trigger: Default::default(),
+        actions: vec![proto::Action::Back],
+        transition: Default::default(),
+        duration_ms,
+        easing: Default::default(),
+        delay_ms: 0,
+    };
+    let durations = |doc: &Document, r: NodeId| doc.node(r).unwrap().interactions.iter().map(|i| i.duration_ms).collect::<Vec<_>>();
+    let (mut base, _, _, r) = files();
+    base.get_mut(r).unwrap().interactions = vec![ia(100), ia(200)];
+    let fork = || {
+        let (mut branch, main) = (base.clone(), base.clone());
+        branch.set_client_id(base.highest_id_space() + 1);
+        (branch, main)
+    };
+    // The branch changes the first interaction and adds one; main changes the second.
+    let (mut branch, mut main) = fork();
+    branch.get_mut(r).unwrap().interactions[0].duration_ms = 150;
+    branch.get_mut(r).unwrap().interactions.push(ia(300));
+    main.get_mut(r).unwrap().interactions[1].duration_ms = 250;
+    both_ways(&base, &branch, &main, |doc| assert_eq!(durations(doc, r), [150, 250, 300]));
+    // The branch deletes one while main leaves them alone.
+    let (mut branch, mut main) = fork();
+    branch.get_mut(r).unwrap().interactions.remove(0);
+    main.get_mut(r).unwrap().fills[0] = red();
+    both_ways(&base, &branch, &main, |doc| assert_eq!(durations(doc, r), [200]));
+    // The same interaction changed both ways, or one deleted while the other side changed the
+    // list, conflicts.
+    for delete in [false, true] {
+        let (mut branch, mut main) = fork();
+        if delete {
+            branch.get_mut(r).unwrap().interactions.remove(0);
+        } else {
+            branch.get_mut(r).unwrap().interactions[1].duration_ms = 220;
+        }
+        main.get_mut(r).unwrap().interactions[1].duration_ms = 250;
+        let p = plan(&base, &branch, &main).unwrap();
+        let keys: Vec<String> = p.conflicts.iter().map(|c| c.key.to_string()).collect();
+        assert_eq!(keys, [format!("{r}/interactions")], "delete: {delete}");
+    }
 }
 
 #[test]

docs/mcp.md

@@ -978,10 +978,11 @@ branches). Driven with `run_command`:
   name, property, branch, main, deletedBy?}]}`. `layers` are the changed layers with their property
   changes formatted like Inspect (`fill #2563EB → #DC2626`); `document` lists styles, variables,
   comments and file settings. A conflict is the same property changed on both sides to different
-  values (key `<layer id>/<property>`, e.g. `4/fills`; fills, strokes and effects compare item by
-  item, so changing different items of one list on each side, or adding items at its end on one
-  side, merges without a conflict; a layer's parent and position are one
-  property, `parent`), a layer deleted on one side while the other edited it or what's inside it
+  values (key `<layer id>/<property>`, e.g. `4/fills`; fills, strokes, effects, layout grids,
+  export settings, guides, prototype interactions, annotations and Dev Mode resources compare item
+  by item, so changing different items of one list on each side, or adding items at its end on one
+  side, merges without a conflict; other lists, such as code links, compare whole; a layer's parent
+  and position are one property, `parent`), a layer deleted on one side while the other edited it or what's inside it
   (`<layer id>/layer`, with `deletedBy`), or a document-level entry or setting changed on both sides
   (`style/<id>`, `variable/<id>`, `comment/<id>`, `doc/<property>`…).
 - `branch.merge {id?, resolutions?}` merges the branch into the main file as one undo step,

docs/translating.md

@@ -15,12 +15,12 @@ Simplified.
 `pref.language {language}` does the same from the CLI, MCP and the control channel.
 
 The catalogs: English (`en`, the source), and Spanish (`es`, with the Latin American overlay
-`es-419`), French (`fr`), German (`de`), Dutch (`nl`), Brazilian Portuguese (`pt-BR`), Japanese (`ja`),
-Korean (`ko`), Simplified Chinese (`zh-Hans`), Czech (`cs`) and Indonesian (`id`), machine-drafted
-and waiting for native speakers' review. Russian (`ru`, Русский) was reviewed and approved by
-VanTan; its catalog keeps Frame, Dev Mode and Smart animate as familiar tool names and uses « »
-quotes. Turkish (`tr`) was machine-drafted and reviewed by a native speaker, and Swedish (`sv`)
-translates every UI string, command label and menu wording.
+`es-419`), French (`fr`), German (`de`), Dutch (`nl`), Brazilian Portuguese (`pt-BR`), Japanese
+(`ja`), Korean (`ko`), Simplified Chinese (`zh-Hans`), Czech (`cs`), Indonesian (`id`) and Finnish
+(`fi`), machine-drafted and waiting for native speakers' review. Russian (`ru`, Русский) was
+reviewed and approved by VanTan; its catalog keeps Frame, Dev Mode and Smart animate as familiar
+tool names and uses « » quotes. Turkish (`tr`) was machine-drafted and reviewed by a native speaker,
+and Swedish (`sv`) translates every UI string, command label and menu wording.
 
 ## The catalogs