Merge main into fix/issue-97-folder-undo

# Conflicts:
#	crates/engine/src/lib.rs
#	docs/parity.md
#	docs/whats-new.md

apps/lightcraft/src/control_server.rs

@@ -1,14 +1,28 @@
 //! Loopback JSON-lines control server: one request per line, one reply per line.
 //! This is the transport the MCP server (`lightcraft-cli mcp --connect`) wraps.
+//!
+//! The server only ever reads requests: a line that is not a JSON object with a string `method`
+//! (an HTTP request line from a browser's cross-origin `fetch`, a stray `nc`, binary junk) gets
+//! one error reply and the connection is closed, so nothing after it is executed. Lines are
+//! bounded ([`MAX_LINE`]) and so is the number of open connections ([`MAX_CONNECTIONS`]); junk
+//! never reaches the UI thread. See `docs/control-protocol.md`.
 
-use std::io::{BufRead, BufReader, Write};
+use std::io::{BufRead, BufReader, Read, Write};
 use std::net::{TcpListener, TcpStream};
+use std::sync::Arc;
+use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::mpsc::{Receiver, Sender, channel};
 use std::time::Duration;
 
 use lightcraft_ui_egui::ControlRequest;
 use serde_json::{Value, json};
 
+/// Longest accepted request line (bytes, without the newline). Requests are small JSON objects;
+/// anything longer is refused and the connection closed.
+pub const MAX_LINE: usize = 4 * 1024 * 1024;
+/// Connections served at once; further ones get an error line and are closed.
+pub const MAX_CONNECTIONS: usize = 16;
+
 pub fn start(port: u16, ctx: egui::Context) -> Receiver<ControlRequest> {
     let (tx, rx) = channel::<ControlRequest>();
     let listener = match TcpListener::bind(("127.0.0.1", port)) {
@@ -19,44 +33,235 @@ pub fn start(port: u16, ctx: egui::Context) -> Receiver<ControlRequest> {
         }
     };
     eprintln!("lightcraft: control server listening on 127.0.0.1:{port}");
-    std::thread::spawn(move || {
-        for stream in listener.incoming().flatten() {
-            let tx = tx.clone();
-            let ctx = ctx.clone();
-            std::thread::spawn(move || serve(stream, tx, ctx));
-        }
-    });
+    std::thread::spawn(move || accept_loop(listener, tx, ctx));
     rx
 }
 
-fn serve(stream: TcpStream, tx: Sender<ControlRequest>, ctx: egui::Context) {
-    let Ok(read) = stream.try_clone() else { return };
-    let mut out = stream;
-    for line in BufReader::new(read).lines() {
-        let Ok(line) = line else { break };
-        if line.trim().is_empty() {
+fn accept_loop(listener: TcpListener, tx: Sender<ControlRequest>, ctx: egui::Context) {
+    let open = Arc::new(AtomicUsize::new(0));
+    for mut stream in listener.incoming().flatten() {
+        if open.fetch_add(1, Ordering::SeqCst) >= MAX_CONNECTIONS {
+            open.fetch_sub(1, Ordering::SeqCst);
+            let _ = writeln!(stream, "{}", json!({"ok": false, "error": "too many control connections"}));
             continue;
         }
-        let reply = match serde_json::from_str::<Value>(&line) {
-            Ok(msg) => {
-                let id = msg.get("id").cloned().unwrap_or(Value::Null);
-                let method = msg.get("method").and_then(Value::as_str).unwrap_or("").to_string();
-                let params = msg.get("params").cloned().unwrap_or(json!({}));
-                let (req, rrx) = ControlRequest::new(method, params);
-                if tx.send(req).is_err() {
-                    break;
-                }
-                ctx.request_repaint();
-                let mut r = rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"}));
-                if let Some(o) = r.as_object_mut() {
-                    o.insert("id".into(), id);
+        let tx = tx.clone();
+        let ctx = ctx.clone();
+        let open = Arc::clone(&open);
+        std::thread::spawn(move || {
+            serve(stream, &tx, &ctx);
+            open.fetch_sub(1, Ordering::SeqCst);
+        });
+    }
+}
+
+fn serve(stream: TcpStream, tx: &Sender<ControlRequest>, ctx: &egui::Context) {
+    let Ok(read) = stream.try_clone() else { return };
+    serve_lines(read, stream, |method, params| {
+        let (req, rrx) = ControlRequest::new(method, params);
+        tx.send(req).ok()?;
+        ctx.request_repaint();
+        Some(rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})))
+    });
+}
+
+/// One parsed line.
+enum Line {
+    Request {
+        id: Value,
+        method: String,
+        params: Value,
+    },
+    Blank,
+    /// Not a request: reply with this error and close the connection.
+    Reject(String),
+}
+
+fn parse_line(bytes: &[u8]) -> Line {
+    let Ok(text) = std::str::from_utf8(bytes) else {
+        return Line::Reject("not a control request (invalid UTF-8); closing the connection".into());
+    };
+    let text = text.trim();
+    if text.is_empty() {
+        return Line::Blank;
+    }
+    let msg = match serde_json::from_str::<Value>(text) {
+        Ok(v) => v,
+        Err(e) => return Line::Reject(format!("bad JSON ({e}); expected one JSON request object per line; closing the connection")),
+    };
+    let Some(obj) = msg.as_object() else {
+        return Line::Reject("not a control request (expected a JSON object); closing the connection".into());
+    };
+    let Some(method) = obj.get("method").and_then(Value::as_str) else {
+        return Line::Reject("not a control request (missing string `method`); closing the connection".into());
+    };
+    Line::Request {
+        id: obj.get("id").cloned().unwrap_or(Value::Null),
+        method: method.to_string(),
+        params: obj.get("params").cloned().unwrap_or(json!({})),
+    }
+}
+
+/// Read one `\n`-terminated line of at most `max` bytes into `buf` (newline stripped).
+/// `Ok(false)` at end of stream; `Err` when the line is too long or the read fails.
+fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec<u8>, max: usize) -> Result<bool, String> {
+    buf.clear();
+    // `take` bounds the bytes buffered for one line: a peer can't make us allocate more than `max`.
+    let limit = u64::try_from(max).unwrap_or(u64::MAX).saturating_add(1);
+    let n = reader.take(limit).read_until(b'\n', buf).map_err(|e| e.to_string())?;
+    if n == 0 {
+        return Ok(false);
+    }
+    if buf.last() == Some(&b'\n') {
+        buf.pop();
+        if buf.last() == Some(&b'\r') {
+            buf.pop();
+        }
+        return Ok(true);
+    }
+    if buf.len() > max {
+        return Err(format!("request line longer than {max} bytes; closing the connection"));
+    }
+    // Last line without a newline, then end of stream.
+    Ok(true)
+}
+
+/// Serve requests from `read`, answering on `out`, until the peer closes, sends something that is
+/// not a request, or `handle` returns `None` (app gone). `handle` gets `(method, params)` and
+/// returns the reply object (the request's `id` is added here).
+fn serve_lines<R: Read, W: Write>(read: R, mut out: W, mut handle: impl FnMut(String, Value) -> Option<Value>) {
+    let mut reader = BufReader::new(read);
+    let mut buf = Vec::new();
+    loop {
+        let reject = match read_bounded_line(&mut reader, &mut buf, MAX_LINE) {
+            Ok(false) => return,
+            Err(e) => e,
+            Ok(true) => match parse_line(&buf) {
+                Line::Blank => continue,
+                Line::Reject(e) => e,
+                Line::Request { id, method, params } => {
+                    let Some(mut r) = handle(method, params) else { return };
+                    if let Some(o) = r.as_object_mut() {
+                        o.insert("id".into(), id);
+                    }
+                    if writeln!(out, "{r}").and_then(|()| out.flush()).is_err() {
+                        return;
+                    }
+                    continue;
                 }
-                r
-            }
-            Err(e) => json!({"ok": false, "error": format!("bad JSON: {e}")}),
+            },
         };
-        if writeln!(out, "{reply}").is_err() {
-            break;
+        let _ = writeln!(out, "{}", json!({"ok": false, "error": reject})).and_then(|()| out.flush());
+        return;
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn run(input: &[u8]) -> (Vec<Value>, Vec<String>) {
+        let mut out = Vec::new();
+        let mut calls = Vec::new();
+        serve_lines(input, &mut out, |m, _p| {
+            calls.push(m);
+            Some(json!({"ok": true, "result": null}))
+        });
+        let replies = String::from_utf8(out).unwrap().lines().map(|l| serde_json::from_str::<Value>(l).unwrap()).collect();
+        (replies, calls)
+    }
+
+    #[test]
+    fn requests_are_answered_in_order_with_ids() {
+        let (r, calls) = run(b"{\"id\":1,\"method\":\"a\"}\n\n{\"id\":2,\"method\":\"b\",\"params\":{}}\r\n{\"method\":\"c\"}");
+        assert_eq!(calls, ["a", "b", "c"]);
+        assert_eq!(r.len(), 3);
+        assert_eq!(r[0]["id"], 1);
+        assert_eq!(r[1]["id"], 2);
+        assert_eq!(r[2]["id"], Value::Null);
+    }
+
+    #[test]
+    fn http_request_closes_before_the_body_runs() {
+        // A browser's cross-origin `fetch` POST: the request line is rejected and the connection
+        // closed, so the JSON body is never executed.
+        let http = b"POST / HTTP/1.1\r\nHost: 127.0.0.1:7980\r\nContent-Type: text/plain\r\n\r\n\
+            {\"id\":1,\"method\":\"engine.execute\",\"params\":{\"command\":\"folder.move\"}}\n";
+        let (r, calls) = run(http);
+        assert!(calls.is_empty(), "nothing may run: {calls:?}");
+        assert_eq!(r.len(), 1);
+        assert_eq!(r[0]["ok"], false);
+        assert!(r[0]["error"].as_str().unwrap().contains("closing the connection"));
+    }
+
+    #[test]
+    fn non_request_json_closes_the_connection() {
+        for junk in [
+            &b"[1,2]\n{\"method\":\"a\"}\n"[..],
+            b"{\"id\":1}\n{\"method\":\"a\"}\n",
+            b"{\"method\":5}\n{\"method\":\"a\"}\n",
+            b"\xff\xfe\n{\"method\":\"a\"}\n",
+        ] {
+            let (r, calls) = run(junk);
+            assert!(calls.is_empty());
+            assert_eq!(r.len(), 1);
+            assert_eq!(r[0]["ok"], false);
         }
+    }
+
+    #[test]
+    fn a_request_before_junk_still_runs() {
+        let (r, calls) = run(b"{\"method\":\"a\"}\nGET / HTTP/1.1\n{\"method\":\"b\"}\n");
+        assert_eq!(calls, ["a"]);
+        assert_eq!(r.len(), 2);
+        assert_eq!(r[1]["ok"], false);
+    }
+
+    #[test]
+    fn overlong_line_is_refused_without_buffering_it_all() {
+        // An endless line without a newline: reading stops after MAX_LINE + 1 bytes.
+        let endless = std::io::repeat(b'x');
+        let mut out = Vec::new();
+        let mut ran = false;
+        serve_lines(endless, &mut out, |_, _| {
+            ran = true;
+            None
+        });
+        assert!(!ran);
+        let reply: Value = serde_json::from_slice(out.strip_suffix(b"\n").unwrap()).unwrap();
+        assert!(reply["error"].as_str().unwrap().contains("longer than"));
+    }
+
+    #[test]
+    fn line_at_the_limit_is_accepted() {
+        let mut buf = Vec::new();
+        let line = vec![b'a'; 10];
+        let mut input = line.clone();
+        input.push(b'\n');
+        assert_eq!(read_bounded_line(&mut &input[..], &mut buf, 10), Ok(true));
+        assert_eq!(buf, line);
+        assert!(read_bounded_line(&mut &[b'a'; 11][..], &mut buf, 10).is_err());
+    }
+
+    #[test]
+    fn tcp_connection_is_closed_after_junk() {
+        // End to end over a real loopback socket (the transport the app uses).
+        let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap();
+        let addr = listener.local_addr().unwrap();
+        let server = std::thread::spawn(move || {
+            let (s, _) = listener.accept().unwrap();
+            let read = s.try_clone().unwrap();
+            serve_lines(read, s, |_, _| Some(json!({"ok": true})));
+        });
+        let mut c = TcpStream::connect(addr).unwrap();
+        c.set_read_timeout(Some(Duration::from_secs(10))).unwrap();
+        c.write_all(b"{\"id\":7,\"method\":\"x\"}\nhello\n{\"method\":\"y\"}\n").unwrap();
+        let mut text = String::new();
+        c.read_to_string(&mut text).unwrap(); // returns at EOF: the server closed the connection
+        server.join().unwrap();
+        let lines: Vec<&str> = text.lines().collect();
+        assert_eq!(lines.len(), 2, "{text}");
+        assert!(lines[0].contains("\"id\":7"));
+        assert!(lines[1].contains("closing the connection"));
     }
 }

crates/engine/src/cmd/browse.rs

@@ -166,7 +166,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Rename Folder",
             [],
             None,
-            "{path, name} — rename a folder on disk (its files and sidecars go along) and relink the photos in it; not an undo step → {path, relinked}",
+            "{path, name} — rename a folder on disk (its files and sidecars go along) and relink the photos in it; one undo step (undo renames it back, refused if the old name is taken) → {path, relinked}",
             always,
             |s, p| {
                 let from = str_param(p, "path").ok_or_else(|| bad("folder.rename", "missing path"))?.trim_end_matches(['/', '\\']).to_string();
@@ -184,7 +184,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Move Folder",
             [],
             None,
-            "{path, into: destination folder} — move a folder on disk into another and relink the photos in it; not an undo step → {path, relinked}",
+            "{path, into: destination folder} — move a folder on disk into another and relink the photos in it; one undo step (undo moves it back, refused if the old place is taken) → {path, relinked}",
             always,
             |s, p| {
                 let from = str_param(p, "path").ok_or_else(|| bad("folder.move", "missing path"))?.trim_end_matches(['/', '\\']).to_string();
@@ -217,58 +217,85 @@ pub fn specs() -> Vec<CommandSpec> {
     ]
 }
 
-/// Rename or move a folder on disk (everything in it goes along, sidecars included) and relink
-/// the photos inside. Like a rename in the file manager it is not an undo step: undoing would
-/// point the photos back at a folder that no longer exists.
-pub(crate) fn move_folder(s: &mut crate::Session, from: &str, to: &str) -> std::result::Result<usize, String> {
-    use std::path::Path;
+/// Rename `from` to `to` on disk (a folder; everything in it goes along). Never overwrites:
+/// refused when `to` exists (a case-only rename on a case-insensitive file system goes through
+/// a temporary name). Missing parents of `to` are created.
+pub(crate) fn rename_folder_on_disk(from: &str, to: &str) -> std::result::Result<(), String> {
     let (src, dst) = (Path::new(from), Path::new(to));
     if !src.is_dir() {
-        return Err(format!("{from} is not a folder"));
+        return Err(format!("{from} is not a folder (moved or deleted?)"));
     }
-    if dst.exists() {
+    let case_only = from != to && from.to_lowercase() == to.to_lowercase();
+    if dst.exists() && !case_only {
         return Err(format!("{to} already exists"));
     }
     if dst.starts_with(src) {
         return Err("a folder can't move into itself".into());
     }
-    if s.library.as_ref().is_some_and(|l| l.dir.starts_with(src) || src.starts_with(&l.dir)) {
-        return Err("the library's own folders can't be moved here".into());
-    }
     if let Some(parent) = dst.parent() {
         std::fs::create_dir_all(parent).map_err(|e| format!("{}: {e}", parent.display()))?;
     }
-    std::fs::rename(src, dst).map_err(|e| format!("{from} → {to}: {e}"))?;
-    let mut n = 0;
-    let ops: Vec<lightcraft_catalog::Op> = s
+    if case_only {
+        let tmp = src.with_file_name(format!(".lc-rename-{}", std::process::id()));
+        if tmp.exists() {
+            return Err(format!("{} already exists", tmp.display()));
+        }
+        std::fs::rename(src, &tmp).map_err(|e| format!("{from} → {to}: {e}"))?;
+        return std::fs::rename(&tmp, dst).map_err(|e| {
+            let _ = std::fs::rename(&tmp, src);
+            format!("{from} → {to}: {e}")
+        });
+    }
+    std::fs::rename(src, dst).map_err(|e| format!("{from} → {to}: {e}"))
+}
+
+/// After a folder moved from `from` to `to`: a browsed folder at or below it follows.
+pub(crate) fn follow_folder(s: &mut Session, from: &str, to: &str) {
+    if let Some(b) = &mut s.browse
+        && let Ok(rel) = Path::new(&b.path).strip_prefix(from)
+    {
+        b.path = Path::new(to).join(rel).to_string_lossy().to_string();
+        let f = b.path.clone();
+        s.filter.folder = Some(f);
+    }
+}
+
+/// Rename or move a folder on disk (everything in it goes along, sidecars included) and relink
+/// the photos inside, as one undo step: undo renames the folder back and restores the photos'
+/// paths, redo repeats the move — each refused (and reported) when the destination is taken.
+pub(crate) fn move_folder(s: &mut Session, from: &str, to: &str) -> std::result::Result<usize, String> {
+    let (src, dst) = (Path::new(from), Path::new(to));
+    if !src.is_dir() {
+        return Err(format!("{from} is not a folder"));
+    }
+    if s.library.as_ref().is_some_and(|l| l.dir.starts_with(src) || src.starts_with(&l.dir)) {
+        return Err("the library's own folders can't be moved here".into());
+    }
+    rename_folder_on_disk(from, to)?;
+    let ops: Vec<Op> = s
         .catalog
         .photos()
         .filter_map(|p| match &p.source {
             lightcraft_catalog::Source::File { path } => Path::new(path).strip_prefix(src).ok().map(|rel| (p.id, p.file_name.clone(), dst.join(rel))),
             _ => None,
         })
-        .map(|(id, file_name, np)| {
-            n += 1;
-            lightcraft_catalog::Op::Relink {
-                id,
-                file_name,
-                source: lightcraft_catalog::Source::File { path: np.to_string_lossy().to_string() },
-                format: None,
-            }
+        .map(|(id, file_name, np)| Op::Relink {
+            id,
+            file_name,
+            source: lightcraft_catalog::Source::File { path: np.to_string_lossy().to_string() },
+            format: None,
         })
         .collect();
-    for op in ops {
-        if s.catalog.apply(op.clone()).is_ok() {
-            s.pending_log.push(op);
-        }
-    }
-    // a browsed folder at or below it follows
-    if let Some(b) = &mut s.browse
-        && let Ok(rel) = Path::new(&b.path).strip_prefix(src)
-    {
-        b.path = dst.join(rel).to_string_lossy().to_string();
-        let f = b.path.clone();
-        s.filter.folder = Some(f);
+    let n = ops.len();
+    let name = Path::new(to).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default();
+    let renamed = src.parent() == dst.parent();
+    let label = if renamed { format!("Rename Folder to “{name}”") } else { format!("Move Folder “{name}”") };
+    let folder = crate::FolderMove { from: to.to_string(), to: from.to_string() };
+    if let Err(e) = s.commit_with_folder(&label, Op::Batch { ops }, folder) {
+        // the catalog didn't take it: put the folder back where the photos still point
+        let back = rename_folder_on_disk(to, from).err().map(|b| format!("; moving it back failed too: {b}")).unwrap_or_default();
+        return Err(format!("{e}{back}"));
     }
+    follow_folder(s, from, to);
     Ok(n)
 }

crates/engine/src/cmd/mod.rs

@@ -5,7 +5,7 @@
 //! `edit.*` (undo/redo), and queries (`catalog.query`, `photo.inspect`, `develop.get`…).
 
 mod before;
-mod browse;
+pub(crate) mod browse;
 mod color;
 pub(crate) mod convert;
 mod cull;

crates/engine/src/lib.rs

@@ -71,6 +71,22 @@ pub type Result<T> = std::result::Result<T, EngineError>;
 pub struct UndoEntry {
     pub label: String,
     pub op: Op,
+    /// A folder to rename on disk (`from` → `to`) before `op` is applied: Rename / Move Folder
+    /// (whose `op` relinks the photos inside). Never overwrites; refused when `to` exists.
+    pub folder: Option<FolderMove>,
+}
+
+/// A folder renamed or moved on disk as part of an undo step.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct FolderMove {
+    pub from: String,
+    pub to: String,
+}
+
+impl FolderMove {
+    fn reversed(&self) -> Self {
+        Self { from: self.to.clone(), to: self.from.clone() }
+    }
 }
 
 /// An in-progress slider drag / brush stroke: one undo step when it ends.
@@ -305,7 +321,7 @@ impl Session {
         let fwd = op.clone();
         let inv = self.catalog.apply(op)?;
         self.pending_log.push(fwd);
-        self.undo.push(UndoEntry { label: label.to_string(), op: inv });
+        self.undo.push(UndoEntry { label: label.to_string(), op: inv, folder: None });
         if self.undo.len() > 1000 {
             self.undo.remove(0);
         }
@@ -313,6 +329,16 @@ impl Session {
         Ok(())
     }
 
+    /// [`Session::commit`] for an op that goes with a folder already renamed on disk; the undo
+    /// step renames it back (`folder` is the undo direction: current place → old place).
+    pub(crate) fn commit_with_folder(&mut self, label: &str, op: Op, folder: FolderMove) -> Result<()> {
+        self.commit(label, op)?;
+        if let Some(e) = self.undo.last_mut() {
+            e.folder = Some(folder);
+        }
+        Ok(())
+    }
+
     /// Leaving photo `id` after editing it: keep its settings as an automatic version (when they
     /// differ from its latest version; at most [`AUTO_VERSIONS`] auto versions, oldest dropped).
     /// Saved with the library but not an undo step.
@@ -343,9 +369,13 @@ impl Session {
         if n < 2 || n > self.undo.len() {
             return;
         }
+        // a step that moves a folder on disk stays on its own
+        if self.undo[self.undo.len() - n..].iter().any(|e| e.folder.is_some()) {
+            return;
+        }
         let tail = self.undo.split_off(self.undo.len() - n);
         let ops = tail.into_iter().rev().map(|e| e.op).collect();
-        self.undo.push(UndoEntry { label: label.to_string(), op: Op::Batch { ops } });
+        self.undo.push(UndoEntry { label: label.to_string(), op: Op::Batch { ops }, folder: None });
     }
 
     /// Apply without recording undo (interactive previews).
@@ -356,7 +386,7 @@ impl Session {
 
     pub fn undo_step(&mut self) -> Result<String> {
         let e = self.undo.pop().ok_or_else(|| EngineError::Other("nothing to undo".into()))?;
-        let redo = match self.apply_with_files(&e.op) {
+        let redo = match self.apply_with_files(&e.op, e.folder.as_ref()) {
             Ok(r) => r,
             Err(err) => {
                 self.undo.push(e);
@@ -364,13 +394,13 @@ impl Session {
             }
         };
         self.pending_log.push(e.op);
-        self.redo.push(UndoEntry { label: e.label.clone(), op: redo });
+        self.redo.push(UndoEntry { label: e.label.clone(), op: redo, folder: e.folder.as_ref().map(FolderMove::reversed) });
         Ok(e.label)
     }
 
     pub fn redo_step(&mut self) -> Result<String> {
         let e = self.redo.pop().ok_or_else(|| EngineError::Other("nothing to redo".into()))?;
-        let undo = match self.apply_with_files(&e.op) {
+        let undo = match self.apply_with_files(&e.op, e.folder.as_ref()) {
             Ok(r) => r,
             Err(err) => {
                 self.redo.push(e);
@@ -378,26 +408,44 @@ impl Session {
             }
         };
         self.pending_log.push(e.op);
-        self.undo.push(UndoEntry { label: e.label.clone(), op: undo });
+        self.undo.push(UndoEntry { label: e.label.clone(), op: undo, folder: e.folder.as_ref().map(FolderMove::reversed) });
         Ok(e.label)
     }
 
-    /// Apply an undo/redo op, first moving the files its renames imply (all or nothing). Files
-    /// that can't be moved back after a failure are reported and the library follows them (a
-    /// logged change outside the undo history), so none goes missing.
-    fn apply_with_files(&mut self, op: &Op) -> Result<Op> {
+    /// Apply an undo/redo op, first renaming the step's folder and moving the files its renames
+    /// imply (all or nothing). Files that can't be moved back after a failure are reported and the
+    /// library follows them (a logged change outside the undo history), so none goes missing; the
+    /// folder is only moved back when no file was left behind at its new path.
+    fn apply_with_files(&mut self, op: &Op, folder: Option<&FolderMove>) -> Result<Op> {
         let fs = rename::RealFs;
+        if let Some(f) = folder {
+            cmd::browse::rename_folder_on_disk(&f.from, &f.to).map_err(|e| EngineError::Other(format!("can't move the folder back: {e}")))?;
+        }
+        let undo_folder = || {
+            if let Some(f) = folder {
+                let _ = cmd::browse::rename_folder_on_disk(&f.to, &f.from);
+            }
+        };
         let moves = self.file_moves(op);
         if let Err(e) = rename::move_all(&fs, &moves) {
+            if e.stuck.is_empty() {
+                undo_folder();
+            }
             return Err(EngineError::Other(format!("can't move the files back: {}", self.follow_stuck(op, e, false))));
         }
         match self.catalog.apply(op.clone()) {
-            Ok(inv) => Ok(inv),
+            Ok(inv) => {
+                if let Some(f) = folder {
+                    cmd::browse::follow_folder(self, &f.from, &f.to);
+                }
+                Ok(inv)
+            }
             Err(e) => {
                 let back: Vec<(String, String)> = moves.iter().rev().map(|(a, b)| (b.clone(), a.clone())).collect();
                 if let Err(be) = rename::move_all(&fs, &back) {
                     return Err(EngineError::Other(format!("{e}; the files could not all be moved back: {}", be.message)));
                 }
+                undo_folder();
                 Err(e.into())
             }
         }

crates/engine/src/tests_import.rs

@@ -518,6 +518,70 @@ fn folders_rename_and_move_with_their_photos() {
     let _ = std::fs::remove_dir_all(&root);
 }
 
+/// Rename / Move Folder are undo steps (issue #97): undo renames the folder back on disk (files and
+/// sidecars along) and restores the photos' paths, redo repeats it; a destination taken in the
+/// meantime is never overwritten — the step is refused, reported and stays on the stack.
+#[test]
+fn folder_rename_and_move_undo_and_redo_on_disk() {
+    let root = temp_dir("folders-undo");
+    let a = root.join("Trip");
+    write_png(&a.join("one.png"), 3);
+    std::fs::write(a.join("one.xmp"), "<x:xmpmeta xmlns:x=\"adobe:ns:meta/\"/>").unwrap();
+    let mut s = Session::new().with_fs();
+    s.execute("library.import", &json!({"paths": [a.to_string_lossy()]})).unwrap();
+    let id = s.active().unwrap();
+    let path = |s: &Session| match &s.catalog.photo(id).unwrap().source {
+        lightcraft_catalog::Source::File { path } => path.clone(),
+        _ => panic!(),
+    };
+    let undo0 = s.undo.len();
+    let b = root.join("Italy");
+    s.execute("folder.rename", &json!({"path": a.to_string_lossy(), "name": "Italy"})).unwrap();
+    assert_eq!(s.undo.len(), undo0 + 1, "one undo step");
+    assert!(s.undo.last().unwrap().label.contains("Rename Folder"));
+    assert!(b.join("one.png").exists() && !a.exists());
+
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert!(a.join("one.png").exists() && a.join("one.xmp").exists() && !b.exists(), "renamed back on disk");
+    assert_eq!(path(&s), a.join("one.png").to_string_lossy());
+    s.media.forget(id);
+    assert!(s.render_now(id, 16, 16).is_ok());
+
+    s.execute("edit.redo", &json!({})).unwrap();
+    assert!(b.join("one.png").exists() && b.join("one.xmp").exists() && !a.exists());
+    assert_eq!(path(&s), b.join("one.png").to_string_lossy());
+
+    // the old name is taken now: undo is refused, nothing is overwritten, the step stays
+    std::fs::create_dir_all(&a).unwrap();
+    std::fs::write(a.join("keep.txt"), "mine").unwrap();
+    let undo_n = s.undo.len();
+    let e = s.execute("edit.undo", &json!({})).unwrap_err().to_string();
+    assert!(e.contains("already exists"), "{e}");
+    assert_eq!(s.undo.len(), undo_n, "the step stays on the stack");
+    assert_eq!(std::fs::read_to_string(a.join("keep.txt")).unwrap(), "mine");
+    assert!(b.join("one.png").exists());
+    assert_eq!(path(&s), b.join("one.png").to_string_lossy());
+    std::fs::remove_dir_all(&a).unwrap();
+
+    // move into another folder, undo, redo
+    let into = root.join("Archive");
+    s.execute("folder.move", &json!({"path": b.to_string_lossy(), "into": into.to_string_lossy()})).unwrap();
+    let moved = into.join("Italy");
+    assert_eq!(path(&s), moved.join("one.png").to_string_lossy());
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert!(b.join("one.png").exists() && !moved.exists());
+    assert_eq!(path(&s), b.join("one.png").to_string_lossy());
+    // the redo target is taken (even by an empty folder): refused, nothing replaced
+    std::fs::create_dir_all(&moved).unwrap();
+    assert!(s.execute("edit.redo", &json!({})).is_err());
+    assert!(b.join("one.png").exists());
+    std::fs::remove_dir(&moved).unwrap();
+    s.execute("edit.redo", &json!({})).unwrap();
+    assert!(moved.join("one.png").exists() && moved.join("one.xmp").exists() && !b.exists());
+    assert_eq!(path(&s), moved.join("one.png").to_string_lossy());
+    let _ = std::fs::remove_dir_all(&root);
+}
+
 /// Assisted culling: a burst (same scene, seconds apart) of a sharp and two soft shots, plus an
 /// unrelated photo: the burst is grouped, the sharp one is its best, blurry shots can be rejected.
 #[test]

docs/control-protocol.md

@@ -9,7 +9,16 @@
 ← {"id": 2, "ok": false, "error": "unknown command `nope`"}
 ```
 
-Requests are answered on the UI thread between frames (timeout 60 s). The MCP server's connect
+Requests are answered on the UI thread between frames (timeout 60 s).
+
+**Only requests are read.** Every line must be a JSON object with a string `method` (`id` and `params` are
+optional; blank lines are skipped). Anything else — text that isn't JSON, a JSON array or number, an object without
+`method`, invalid UTF-8, or a line longer than 4 MiB — gets one error reply
+(`{"ok": false, "error": "… closing the connection"}`) and the server **closes the connection**; nothing sent after
+it on that connection runs. This keeps an HTTP request (for example a web page's cross-origin `fetch` to
+`127.0.0.1:<port>`) from smuggling a command in its body: its request line is rejected first. Junk never reaches the
+UI thread, and at most 16 connections are served at once (further ones get an error line and are closed). Clients
+that hit an error reply should reconnect. The port has no authentication, so only enable it when you need it. The MCP server's connect
 mode ([mcp.md](mcp.md)) is a thin layer over this channel. Implementation:
 `crates/ui-egui/src/control.rs` (methods) and `apps/lightcraft/src/control_server.rs` (transport).
 

docs/parity.md

@@ -714,7 +714,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06-
 | 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-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; like the file manager, not an undo step) |
+| 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-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), Quick Collection and target album (B in the grids adds / removes the selection; album menu ▸ Set as Target Album; the target is marked +) |
 | LRC-LIB-SMARTCOLL | Smart-collection rules | P1 | ✅ | `cmd:album.createSmart`, `cmd:album.setRules`, `cmd:album.ruleFields`, `cmd:dialog.smartAlbum`, `crates/catalog/src/rules.rs`, `crates/ui-egui/src/panels/rules_editor.rs` | rule editor: match all / any / none, nested groups (⌥+ or + Group), 26 fields (rating, flag, label, type, edits, keywords, any text, filename, format, title, caption, camera, lens, location, creator, copyright, copyright status, capture / import / edit date, ISO, aperture, focal length, megapixels, GPS, virtual copy, album) with text / number / date / in-the-last operators; live match count; also usable as a library filter (`ruleSet`) |
 | LRC-LIB-PUBLISH | Publish services | P2 | ⬜ | | |

docs/whats-new.md

@@ -39,6 +39,9 @@
   written by a newer LightCraft is refused with "this library was written by a newer version of LightCraft" and left
   untouched — older versions no longer read part of it as a damaged log. Once this version has opened a library,
   LightCraft 0.2.0 and older refuse it ("unsupported format … v2").
+- The control port (`--control`) closes a connection as soon as it receives anything that isn't a JSON request
+  (issue #94): an HTTP request from a web page can no longer carry a command in its body. Lines are capped at
+  4 MiB and connections at 16.
 - Canon CR2 photos from the EOS 7D, 50D, 60D, 550D, 600D, 1200D, 1300D, 5D Mark II and 1D Mark IV (and other
   models whose sensor starts on a green-blue row) no longer come out magenta (issue #85): the colour-filter
   layout is read from each file instead of assumed.
@@ -65,6 +68,9 @@
   so none shows as missing. Renaming one of a raw + JPEG pair copies their shared `IMG_0001.xmp` instead of taking
   it away from the other (issue #92). Find Missing Photos also finds renamed files by their content, prefers a content match
   over a same-name same-size look-alike, and skips (and reports) photos it can't tell apart instead of guessing.
+- Rename Folder… and Move Folder To… (Local) can be undone and redone (issue #97): the folder moves back on disk with
+  its photos and XMP sidecars, and the photos point at it again. If something now occupies the old place, the undo
+  is refused and nothing is overwritten.
 - Smart albums with a rule editor: match all / any / none, nested groups, 26 fields.
 - Quick Collection and target album (B in the grid), keyword sets (⌥1–⌥9), colour-label sets.
 - Colour-label filter with several labels at once; expandable folder tree in Local.