diff --git a/apps/soundcraft-cli/src/main.rs b/apps/soundcraft-cli/src/main.rs index 46271c9..2d8861c 100644 --- a/apps/soundcraft-cli/src/main.rs +++ b/apps/soundcraft-cli/src/main.rs @@ -187,7 +187,7 @@ fn finish(e: &mut Engine, args: &[String]) -> ExitCode { } } if let Some(path) = arg_value(args, "--save") { - match e.execute("session.save", &json!({"path": path})) { + match e.execute("session.save", &json!({"path": path, "replace": true})) { Ok(v) => outln!("{v}"), Err(err) => return fail(err), } diff --git a/crates/automation/src/mcp.rs b/crates/automation/src/mcp.rs index 1b38dcc..b3d21ab 100644 --- a/crates/automation/src/mcp.rs +++ b/crates/automation/src/mcp.rs @@ -54,7 +54,12 @@ pub fn tool_definitions() -> Vec { &[], ), tool("open_session", "Open a .scraft session file.", json!({"path": {"type": "string"}}), &["path"]), - tool("save_session", "Save the session (path required the first time).", json!({"path": {"type": "string"}}), &[]), + tool( + "save_session", + "Save the session (path required the first time; replace: true to save over another existing file).", + json!({"path": {"type": "string"}, "replace": {"type": "boolean"}}), + &[], + ), tool( "import_audio", "Import an audio file (WAV, AIFF, FLAC, MP3, OGG, AAC/M4A, ALAC, CAF) onto a new or existing track.", diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index d58af41..8c91d0a 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -15,32 +15,37 @@ pub fn specs() -> Vec { Ok(json!({"name": e.session().name, "tracks": e.session().tracks.len(), "missing": missing})) }), cmd!(noundo "session.close", "Close Session", ["File"], Some("Cmd+Shift+W"), "{}", always, |e, _| { e.replace_session(Session::default()); e.path = None; Ok(json!({})) }), - cmd!(noundo "session.save", "Save Session", ["File"], Some("Cmd+S"), "{path?}", always, |e, p| { + cmd!(noundo "session.save", "Save Session", ["File"], Some("Cmd+S"), "{path?, replace?: false}", always, |e, p| { let path = str_param(p, "path").map(str::to_string).or_else(|| e.path.clone()).ok_or_else(|| bad("session.save", "no path yet: pass `path` (Save As)"))?; + refuse_replacing(e, "session.save", &crate::io::session_file(&path), p)?; let written = crate::io::save_session(e, &path)?; Ok(json!({"path": path, "audio_files_written": written})) }), - cmd!(noundo "session.save_as", "Save Session As...", ["File"], Some("Cmd+Shift+S"), "{path}", always, |e, p| { + cmd!(noundo "session.save_as", "Save Session As...", ["File"], Some("Cmd+Shift+S"), "{path, replace?: false}", always, |e, p| { let path = str_param(p, "path").ok_or_else(|| bad("session.save_as", "`path` required"))?.to_string(); + refuse_replacing(e, "session.save_as", &crate::io::session_file(&path), p)?; let written = crate::io::save_session(e, &path)?; Ok(json!({"path": path, "audio_files_written": written})) }), - cmd!(noundo "session.save_copy", "Save Session Copy In...", ["File"], None, "{path}", always, |e, p| { + cmd!(noundo "session.save_copy", "Save Session Copy In...", ["File"], None, "{path, replace?: false}", always, |e, p| { let path = str_param(p, "path").ok_or_else(|| bad("session.save_copy", "`path` required"))?.to_string(); + refuse_replacing(e, "session.save_copy", &crate::io::session_file(&path), p)?; let keep = e.path.clone(); let written = crate::io::save_session(e, &path)?; e.path = keep; Ok(json!({"path": path, "audio_files_written": written})) }), - cmd!(noundo "session.save_template", "Save As Template...", ["File"], None, "{path}", always, |e, p| { + cmd!(noundo "session.save_template", "Save As Template...", ["File"], None, "{path, replace?: false}", always, |e, p| { let path = str_param(p, "path").ok_or_else(|| bad("session.save_template", "`path` required"))?.to_string(); + let file = crate::io::session_file(&path); + refuse_replacing(e, "session.save_template", &file, p)?; let mut s = e.session().clone(); for t in &mut s.tracks { for pl in &mut t.playlists { pl.clips.clear() } } s.sources.clear(); s.videos.clear(); let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?; - std::fs::write(&path, text).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; - Ok(json!({"path": path})) + std::fs::write(&file, text).map_err(|err| EngineError::Io(format!("{}: {err}", file.display())))?; + Ok(json!({"path": file})) }), cmd!(noundo "session.revert", "Revert Session to Saved...", ["File"], None, "{}", always, |e, _| { let path = e.path.clone().ok_or_else(|| bad("session.revert", "the session has not been saved"))?; @@ -115,8 +120,9 @@ pub fn specs() -> Vec { let n = crate::io::export_clips(e, &ids, &dir)?; Ok(json!({"written": n})) }), - cmd!(noundo "file.export_selected_tracks_as_session", "Selected Tracks as New Session...", ["File", "Export"], None, "{path, tracks?}", has_selection, |e, p| { + cmd!(noundo "file.export_selected_tracks_as_session", "Selected Tracks as New Session...", ["File", "Export"], None, "{path, tracks?, replace?: false}", has_selection, |e, p| { let path = str_param(p, "path").ok_or_else(|| bad("file.export_selected_tracks_as_session", "`path` required"))?.to_string(); + refuse_replacing(e, "file.export_selected_tracks_as_session", &crate::io::session_file(&path), p)?; let keep = tracks_required(e, "file.export_selected_tracks_as_session", p)?; let mut s = e.session().clone(); s.tracks.retain(|t| keep.contains(&t.id) || t.kind == soundcraft_model::TrackKind::Master); @@ -180,6 +186,14 @@ pub fn specs() -> Vec { ] } +/// Writing a session over an existing file needs `replace: true` (see `io::replaces`). +fn refuse_replacing(e: &Engine, id: &str, file: &std::path::Path, p: &Value) -> Result<()> { + if crate::io::replaces(e, id, file) && !bool_or(p, "replace", false) { + return Err(bad(id, format!("{} already exists: pass `replace: true` to replace it", file.display()))); + } + Ok(()) +} + fn new_session(e: &mut Engine, p: &Value) -> Result { let name = str_param(p, "name").unwrap_or("Untitled").to_string(); let sr = @@ -380,6 +394,7 @@ mod tests { fn session_open_reports_missing_media() { let mut e = crate::demo::demo_engine(); let dir = std::env::temp_dir().join(format!("soundcraft-open-missing-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); let path = dir.join("Gone.scraft").to_string_lossy().into_owned(); e.execute("session.save_as", &json!({"path": path})).unwrap(); let mut ok = Engine::default(); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index a563387..655955e 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -474,8 +474,7 @@ pub fn save_copy(e: &Engine, path: &str) -> Result { /// Write `session` to `path`; returns the session with updated media paths, the number of audio /// files written and the final path. fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Session, usize, PathBuf)> { - let p = PathBuf::from(path); - let p = if p.extension().is_none() { p.with_extension(soundcraft_model::SESSION_EXTENSION) } else { p }; + let p = session_file(path); let dir = p.parent().map(Path::to_path_buf).unwrap_or_default(); let audio_dir = dir.join("Audio Files"); let mut s = session.clone(); @@ -519,6 +518,7 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess s.name = stem.to_string(); } let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?; + std::fs::create_dir_all(&dir).map_err(|err| EngineError::Io(format!("{}: {err}", dir.display())))?; // Atomic-ish write: temp file then rename. let tmp = p.with_extension("scraft.tmp"); std::fs::write(&tmp, text).map_err(|err| EngineError::Io(format!("{}: {err}", tmp.display())))?; @@ -526,6 +526,25 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess Ok((s, written, p)) } +/// The file a session saved to `path` goes to: `path`, with `.scraft` added when it has no +/// extension and isn't an existing file. +pub fn session_file(path: &str) -> PathBuf { + let p = PathBuf::from(path); + if p.extension().is_none() && !p.is_file() { p.with_extension(soundcraft_model::SESSION_EXTENSION) } else { p } +} + +/// True when `command` would write a session over the existing `file`. Save, Save As and Save +/// Copy In may write the open session's own file; other commands never ask. +pub fn replaces(e: &Engine, command: &str, file: &Path) -> bool { + match command { + "session.save" | "session.save_as" | "session.save_copy" => { + file.exists() && !e.path.as_deref().is_some_and(|own| same_path(Path::new(own), file)) + } + "session.save_template" | "file.export_selected_tracks_as_session" => file.exists(), + _ => false, + } +} + fn same_path(a: &Path, b: &Path) -> bool { let full = |p: &Path| if p.as_os_str().is_empty() { Path::new(".").canonicalize() } else { p.canonicalize() }; a == b || matches!((full(a), full(b)), (Ok(x), Ok(y)) if x == y) @@ -745,6 +764,34 @@ mod save_tests { assert_eq!(open_session(&mut r, &path("B/Moved.scraft")).unwrap(), vec![dir.join("A/Audio Files/Audio 1_02.wav").display().to_string()]); let _ = std::fs::remove_dir_all(&dir); } + + #[test] + fn saving_over_another_session_needs_replace() { + let dir = std::env::temp_dir().join(format!("soundcraft-replace-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let path = dir.join("Song.scraft").to_string_lossy().into_owned(); + let mut first = with_take(0.5); + first.execute("session.save_as", &json!({"path": path})).unwrap(); + first.execute("session.save", &json!({})).unwrap(); + first.execute("session.save_as", &json!({"path": dir.join("Song")})).unwrap(); + assert!(first.execute("session.save_template", &json!({"path": path})).is_err()); + let mut second = with_take(-0.25); + let track = second.session().tracks[0].id.0; + for id in ["session.save", "session.save_as", "session.save_copy", "session.save_template", "file.export_selected_tracks_as_session"] { + let err = second.execute(id, &json!({"path": path, "tracks": [track]})).unwrap_err(); + assert!(err.to_string().contains("`replace: true`"), "{id}: {err}"); + } + assert_eq!(std::fs::read_dir(dir.join("Audio Files")).unwrap().count(), 1); + Engine::default().execute("session.save_as", &json!({"path": dir.join("New/New.scraft")})).unwrap(); + std::fs::copy(&path, dir.join("Song")).unwrap(); + let mut bare = Engine::default(); + open_session(&mut bare, &dir.join("Song").to_string_lossy()).unwrap(); + bare.execute("session.save", &json!({})).unwrap(); + second.execute("session.save_as", &json!({"path": path, "replace": true})).unwrap(); + let r = reopen(&path); + assert_eq!(r.session().pool.get(r.session().sources[0].id).unwrap().buffer.channels[0][0], -0.25); + let _ = std::fs::remove_dir_all(&dir); + } } #[cfg(test)] diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index d212634..d49b2af 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -303,9 +303,17 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { { *path = p; } - if buttons(ui, "OK", enter) { + let file = soundcraft_engine::io::session_file(path); + let replaces = soundcraft_engine::io::replaces(&app.engine, cmd, &file); + if replaces { + ui.label(format!("“{}” already exists. Do you want to replace it?", file.file_name().unwrap_or_default().to_string_lossy())); + } + if buttons(ui, if replaces { "Replace" } else { "OK" }, enter && !replaces) { let mut v = json!({}); v[key.as_str()] = json!(path); + if replaces { + v["replace"] = json!(true); + } action = Some((cmd.clone(), v)); } } diff --git a/crates/ui-egui/tests/save_as_replace.rs b/crates/ui-egui/tests/save_as_replace.rs new file mode 100644 index 0000000..717e2a6 --- /dev/null +++ b/crates/ui-egui/tests/save_as_replace.rs @@ -0,0 +1,38 @@ +//! Save As over another session asks first, and Enter alone never replaces it. +use egui::vec2; +use egui_kittest::{Harness, kittest::Queryable}; +use serde_json::json; +use soundcraft_ui_egui::{Services, SoundApp, dialogs::Dialog}; + +#[test] +fn saving_over_another_session_takes_the_replace_button() { + let dir = std::env::temp_dir().join(format!("soundcraft-ui-replace-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("Song.scraft"); + soundcraft_engine::Engine::default().execute("session.save_as", &json!({"path": path})).unwrap(); + let other = std::fs::read(&path).unwrap(); + let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); + app.dialogs.open = Some(Dialog::PathPrompt { + cmd: "session.save_as".into(), + title: "Save Session As".into(), + path: path.to_string_lossy().into(), + key: "path".into(), + }); + let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_ui_state( + |ui, app: &mut SoundApp| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }, + app, + ); + h.run_steps(3); + h.key_press(egui::Key::Enter); + h.run_steps(3); + assert_eq!(std::fs::read(&path).unwrap(), other); + h.get_by_label("Replace").click(); + h.run_steps(3); + assert_ne!(std::fs::read(&path).unwrap(), other); + assert_eq!(h.state().engine.path, Some(path.to_string_lossy().into_owned())); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/docs/mcp.md b/docs/mcp.md index 2949871..deb026d 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -24,7 +24,7 @@ claude mcp add soundcraft -- soundcraft-cli mcp --demo | `execute` | Run one command with JSON params. | | `batch` | Run several commands in order (`keep_going` optional). | | `inspect_session` | Verify work: tracks, clips, mixer, plugins, sends, automation, markers, selection; each `sources[]` entry includes `loaded` (decoded audio in the pool). | -| `new_session`, `open_session`, `save_session` | Session files (`.scraft`, audio in `Audio Files/`). `open_session` returns `missing`: the media files that could not be loaded (empty when all are found). | +| `new_session`, `open_session`, `save_session` | Session files (`.scraft`, audio in `Audio Files/`). `open_session` returns `missing`: the media files that could not be loaded (empty when all are found). `save_session` won't save over another existing file unless `replace` is true. | | `import_audio` | WAV, AIFF, FLAC, MP3, OGG, AAC/M4A, ALAC, CAF onto a new or existing track. | | `bounce_mix` | Render to WAV/AIFF/FLAC; returns peak dBFS and integrated LUFS. | | `parity` | Feature-parity report. |