Merge pull request #369 from storytold/claude/fix-171-unlocked-library-warning

Fix #171: a library that can't be locked opens with a warning the user sees

apps/lightcraft-cli/src/main.rs

@@ -102,6 +102,14 @@ static ALLOC: dhat::Alloc = dhat::Alloc;
 
 /// Why `--library DIR` can't be opened; for a library open in another program, how to work with
 /// that one instead (issue #99).
+/// Print the open library's warnings (an unlockable library, damaged settings files) on stderr:
+/// stdout carries command results / MCP protocol messages only.
+fn library_warnings(session: &mut Session, who: &str) {
+    for w in session.take_library_warnings() {
+        eprintln!("{who}: warning: {w}");
+    }
+}
+
 fn library_error(dir: &str, e: lightcraft_engine::EngineError) -> String {
     match e {
         lightcraft_engine::EngineError::LibraryInUse(why) => format!(
@@ -317,6 +325,7 @@ fn mcp(args: &[String]) -> Result<(), String> {
                     let mut h = Headless::default();
                     let r = h.session.open_library(dir, demo).map_err(|e| library_error(dir, e))?;
                     eprintln!("lightcraft-cli mcp: opened library {dir} ({r:?})");
+                    library_warnings(&mut h.session, "lightcraft-cli mcp");
                     h
                 }
                 None if demo => Headless::demo(),
@@ -553,6 +562,7 @@ fn run(args: &[String]) -> Result<(), String> {
                     Some(dir) => {
                         let mut h = Headless::default();
                         h.session.open_library(dir, demo).map_err(|e| library_error(dir, e))?;
+                        library_warnings(&mut h.session, "lightcraft-cli");
                         h
                     }
                     None if demo => Headless::demo(),

crates/engine/src/library.rs

@@ -395,7 +395,7 @@ impl Session {
             forgot_local: None,
             presets_written,
             view_written,
-            settings_warnings: settings.warnings,
+            settings_warnings: unlocked_warning(lock.as_ref()).into_iter().chain(settings.warnings).collect(),
             warnings_reported: 0,
             blocked: settings.blocked,
             lock: lock.take(),
@@ -570,7 +570,8 @@ impl Session {
         lib.files.write_atomic("prefs.json", &v).map_err(|e| EngineError::Other(format!("prefs: {e}")))
     }
 
-    /// Settings-file warnings of the open library not handed out yet (the UI shows each once).
+    /// Warnings about the open library not handed out yet: its settings files, and a lock that
+    /// couldn't be taken (the UI shows each once; the CLI and MCP print them on stderr).
     pub fn take_library_warnings(&mut self) -> Vec<String> {
         let Some(lib) = self.library.as_mut() else { return vec![] };
         let new = lib.settings_warnings[lib.warnings_reported..].to_vec();
@@ -625,3 +626,15 @@ impl Session {
         Ok(())
     }
 }
+
+/// The warning for a library opened without its one-program-at-a-time lock (issue #171): its
+/// file system can't lock `catalog.lock` (some network shares). It opens anyway, as documented —
+/// refusing would lock the user out — but the user must learn that a second program could open it.
+fn unlocked_warning(lock: Option<&LibraryLock>) -> Option<String> {
+    lock.filter(|l| !l.held()).map(|_| {
+        "This library could not be locked (its catalog.lock file can't be locked where it is stored, e.g. on some network \
+         shares), so it is open without protection against a second program: use it in one LightCraft app or command at \
+         a time, or changes made in one of them can be lost."
+            .to_string()
+    })
+}

crates/engine/src/tests_settings_files.rs

@@ -95,3 +95,28 @@ fn unreadable_settings_are_not_overwritten() {
     assert_eq!(files.files.get("presets.json").unwrap(), presets);
     assert_eq!(files.files.get("prefs.json").unwrap(), prefs);
 }
+
+/// Issue #171: a library whose `catalog.lock` can't be locked (stood in for here by a directory
+/// of that name, which takes the same path as a file system without locks) still opens, and the
+/// user is told it is unprotected — in `library.info` and once through the library warnings the
+/// app's notices and the CLI show. A normally locked library says nothing.
+#[test]
+fn an_unlockable_library_opens_with_a_warning() {
+    let lib = temp_dir("unlocked");
+    std::fs::create_dir_all(lib.join("catalog.lock")).unwrap();
+    let mut s = Session::new().with_fs();
+    s.open_library(&lib, false).unwrap();
+    let info = s.execute("library.info", &json!({})).unwrap();
+    let listed = info["settingsWarnings"].as_array().cloned().unwrap_or_default();
+    assert!(listed.iter().any(|w| w.as_str().is_some_and(|w| w.contains("without protection against a second program"))), "{info}");
+    let warnings = s.take_library_warnings();
+    assert_eq!(warnings.len(), 1, "{warnings:?}");
+    drop(s);
+    // the same library with a lockable lock file: no warning
+    std::fs::remove_dir_all(lib.join("catalog.lock")).unwrap();
+    let mut s = Session::new().with_fs();
+    s.open_library(&lib, false).unwrap();
+    assert!(s.take_library_warnings().is_empty());
+    drop(s);
+    let _ = std::fs::remove_dir_all(&lib);
+}

docs/parity.md

@@ -729,7 +729,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06-
 | LRC-LIB-SURVEY | Survey view | P2 | ✅ | `cmd:view.survey` | N |
 | LRC-LIB-REFVIEW | Reference view | P2 | ✅ | `cmd:view.reference`, `cmd:photo.setReference`, `crates/ui-egui/src/panels/compare.rs` (`show_reference`) | View ▸ Reference View (⇧R): the reference photo beside the active one (edited with the Edit panel); photo menu ▸ Set as Reference Photo |
 | LRC-LIB-CATALOG-PANEL | Catalog sets | P1 | ✅ | `cmd:library.source` (`missing`) | all, recently added, picks, recently deleted, missing photos (when any) |
-| LRC-LIB-CATALOG-LOCK | Catalog open in one program at a time | P1 | ✅ | `cmd:app.openLibrary`, `crates/catalog/src/lock.rs`, `crates/engine/src/library.rs` (`open_library`) | exclusive OS lock on `catalog.lock` for the session (app and `lightcraft-cli --library`); a second opener is refused with who has it (`catalog.lock.owner`: program, process, computer); released by the OS on a crash, so never stale; file systems without locks open unprotected with a warning; no read-only mode; the browser build (OPFS) has no guard across tabs yet. A library that can't be opened at launch (locked, unreadable, newer format, drive missing) shows a window — Try Again, Choose Another Library…, Continue Without Saving (then a banner; nothing written), Quit — instead of a silent demo session (`crates/ui-egui/src/panels/library_problem.rs`) |
+| LRC-LIB-CATALOG-LOCK | Catalog open in one program at a time | P1 | ✅ | `cmd:app.openLibrary`, `crates/catalog/src/lock.rs`, `crates/engine/src/library.rs` (`open_library`) | exclusive OS lock on `catalog.lock` for the session (app and `lightcraft-cli --library`); a second opener is refused with who has it (`catalog.lock.owner`: program, process, computer); released by the OS on a crash, so never stale; file systems without locks open unprotected with a warning (the app's notices, `library.info` → `settingsWarnings`, and the CLI's stderr; issue #171); no read-only mode; the browser build (OPFS) has no guard across tabs yet. A library that can't be opened at launch (locked, unreadable, newer format, drive missing) shows a window — Try Again, Choose Another Library…, Continue Without Saving (then a banner; nothing written), Quit — instead of a silent demo session (`crates/ui-egui/src/panels/library_problem.rs`) |
 | LRC-LIB-FOLDERS | Disk folder tree | P1 | ✅ | `cmd:library.browse`, `cmd:folder.rename`, `cmd:folder.move`, `crates/ui-egui/src/panels/left.rs` (`local_section`, `folder_tree`) | standard places + the browsed folder + Browse Folder…, each an expandable tree of subfolders, breadcrumb navigation; context menu: Rename Folder…, Move Folder To…, Show in Finder (on disk, photos relinked; one undo step each: undo / redo move the folder back and forth, refused when the destination is taken) |
 | LRC-LIB-LIBFOLDERS | Library folders | P1 | ✅ | `cmd:library.folders`, `cmd:library.source` (`libraryFolder`), `cmd:library.filter` (`libraryFolder`), `cmd:library.removeFolder`, `cmd:folder.rename`, `cmd:folder.move`, `crates/catalog/src/folders.rs`, `crates/ui-egui/src/panels/left.rs` (`folders_section`, `folder_menu_for_library`, `row_menu`), `crates/ui-egui/src/tests_panels.rs` | sidebar Folders: the folders the library's photos were imported from, with photo counts (subfolders included), as another way to explore the library next to By Date and Keywords; Local stays the way to browse any folder on disk. Each disk is a row of its own and starts open: a drive letter, a Windows share (`\\server\share`, `\\?\UNC\…`, `\\wsl$\…`), a macOS share or disk (`/Volumes/tokyo`), a Linux mount (`/media`, `/run/media`, `/mnt`, gvfs `smb-share:…`, `/net/<host>`), NFS (`/Network/Servers/<host>`), else “This Computer” — told apart from the stored paths alone, so an offline share lists like a mounted one and the sidebar never waits on a network. Only folders that hold imported photos are listed (browsed, deleted and demo photos don't count); every folder is a row of its own (`Users` → `me` → `Pictures`), and a folder that holds nothing itself and leads to one folder starts open; every level indents one step more than its parent however deep it goes: the whole sidebar (Albums, Local, By Date, Folders, Keywords) scrolls sideways when its widest row is wider than the panel (rows are measured one frame behind; Create Album and the counts stay at the visible edge); a name that does not fit ends in an ellipsis at the count (the full name on hover; a row asks for room for at most 140 pt of its name, so depth, not one long name, makes the sidebar scroll), as does the grid title; selection and hover bars end at the panel's edge; click makes the folder the source, like an album or a Local folder (its photos and those of the folders inside it fill the grid, titled “photos/travel”; the filter bar narrows it further; `library.source` `libraryFolder` + `path`; kept with the library's view), and its row is always opened into view; the shown folder follows a rename or move and their undo (engine-tested; the sidebar test covers rename), is dropped back to All Photos when it is removed or no longer holds a photo, and a smart album made from the view keeps it, also when its rules are edited (the editor has no folder field). Only a folder that shows exactly its own photos can be shown (not the startup disk, not `/Volumes`-like folders above disks, not an empty one). A row that would cover other disks' photos too (the startup disk, `/Volumes`, `/mnt`…) only opens. Right-click (row or triangle): Rename Folder…, Move Folder To…, Show in Finder (on disk, photos relinked, as in Local) and Remove from Library… (asks; moves the folder's photos to Recently Deleted, one undo step, no file touched); a disk row offers Remove Disk from Library… (the command needs `disk: true` for a whole disk or a folder spanning several). Limits: names are compared as written (macOS `/Volumes/tokyo` vs `/Volumes/TOKYO` are two disks; `/Volumes/Macintosh HD/…` and `/…` two rows); no disk capacity / online status; no “include subfolders” switch (a folder always includes the ones inside it); creating a folder and moving photos between folders are not here |
 | LRC-LIB-COLLECTIONS | Collections & sets | P1 | ✅ | `cmd:album.create`, `cmd:album.createSmart`, `cmd:album.toggleTarget`, `cmd:album.setTarget`, `cmd:album.clearQuick` | albums + folders (sets), smart albums (rule editor; rules incl. File Path — the whole string, either separator, case-insensitive), Quick Collection and target album (B in the grids adds / removes the selection; album menu ▸ Set as Target Album; the target is marked +) |