From 2e057309b90c1c153831550b75cacf9487ba8afc Mon Sep 17 00:00:00 2001 From: Samuel Bacaro Date: Fri, 23 Oct 2026 18:59:00 -0300 Subject: [PATCH 1/3] Bound the command journal The command journal was a Vec that was never trimmed, so large params (clips/indices arrays, junk fields from MCP or the control channel) were kept for the life of the process. - Store the journal as a VecDeque ring buffer capped at 10,000 entries (oldest are dropped). - Cap a single entry's params at 64 KiB (serialized): a larger value is replaced by a {"__truncated__": true} marker before journaling. Fixes #926 --- crates/engine/src/lib.rs | 60 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 03d173dd..9bbf75fd 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -26,6 +26,12 @@ use std::sync::Arc; pub use cmd::{CommandInfo, CommandSpec, command_specs, find_command}; pub use soundcraft_model as model; +/// Upper bound on journaled commands, so a long session cannot grow memory without limit. +const MAX_JOURNAL_ENTRIES: usize = 10_000; +/// Upper bound on a single journaled entry's params (serialized), so junk fields from MCP or the +/// control channel cannot keep a huge blob alive for the life of the process. +const MAX_JOURNAL_ENTRY_BYTES: usize = 64 * 1024; // 64 KiB + /// Shuts the third-party plugin hosts down, running the module exit of every plugin binary they /// loaded. Call it at the end of `main`, once every plugin instance (mixer, player) is gone: a /// plugin that has been used can crash when its module exit only runs from an exit handler. @@ -119,8 +125,9 @@ pub struct Engine { pub clipboard: Clipboard, pub transport_requests: Vec, pub transport: TransportStatus, - /// Command journal (id, params) for macro recording / agents. - pub journal: Vec<(String, Value)>, + /// Command journal (id, params) for macro recording / agents. Ring buffer: capped at + /// [`MAX_JOURNAL_ENTRIES`] entries of up to [`MAX_JOURNAL_ENTRY_BYTES`] params each. + pub journal: std::collections::VecDeque<(String, Value)>, /// Path of the session file, if saved. pub path: Option, /// Revision counter; bumps on every document change but zoom/scroll (UI caches key on it). @@ -160,7 +167,7 @@ impl Engine { clipboard: Clipboard::default(), transport_requests: Vec::new(), transport: TransportStatus::default(), - journal: Vec::new(), + journal: std::collections::VecDeque::new(), path: None, revision: 1, dirty: false, @@ -337,7 +344,19 @@ impl Engine { self.push_undo(spec.label, before); } if spec.journal { - self.journal.push((id.to_string(), params.clone())); + // Ring buffer with a per-entry cap: a hostile/large params value (junk fields, + // huge arrays from MCP or the control channel) must not be kept whole for the + // life of the process, and the journal must not grow without bound. + let entry = (id.to_string(), params.clone()); + let clamped: (String, Value) = if serde_json::to_vec(&entry.1).map(|b| b.len() > MAX_JOURNAL_ENTRY_BYTES).unwrap_or(true) { + (entry.0, json!({"__truncated__": true})) + } else { + entry + }; + self.journal.push_back(clamped); + if self.journal.len() > MAX_JOURNAL_ENTRIES { + self.journal.pop_front(); + } } Ok(v) } @@ -452,6 +471,39 @@ fn panic_message(p: &(dyn std::any::Any + Send)) -> String { p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()) } +#[cfg(test)] +mod journal_tests { + use super::*; + + fn journaled_command(e: &mut Engine) { + // `track.new` is journaled (see its CommandSpec). + e.execute("track.new", &json!({})).unwrap(); + } + + #[test] + fn journal_is_bounded_to_max_entries() { + let mut e = Engine::default(); + for _ in 0..MAX_JOURNAL_ENTRIES + 500 { + journaled_command(&mut e); + } + assert_eq!(e.journal.len(), MAX_JOURNAL_ENTRIES); + // The oldest entries were dropped: the first remaining id is not the first pushed. + assert!(e.journal.iter().all(|(id, _)| id == "track.new")); + } + + #[test] + fn oversized_params_are_truncated_in_the_journal() { + let mut e = Engine::default(); + let big = "x".repeat(MAX_JOURNAL_ENTRY_BYTES * 4); + e.execute("track.new", &json!({"name": big})).unwrap(); + let (id, params) = e.journal.back().expect("a journaled entry"); + assert_eq!(id, "track.new"); + let sz = serde_json::to_vec(params).map(|b| b.len()).unwrap_or(usize::MAX); + assert!(sz <= 1024, "params are truncated, got {sz} bytes"); + assert_eq!(params, &json!({"__truncated__": true})); + } +} + #[cfg(test)] mod merge_tests { use super::*; From 4f771268b0841ef9dec29d8fb32a0e498272d4fe Mon Sep 17 00:00:00 2001 From: Samuel Bacaro Date: Sat, 24 Oct 2026 17:25:53 -0300 Subject: [PATCH 2/3] journal: measure params size via a bounded writer before cloning Addresses the review on #995: the previous fix serialized the full params clone (up to a multi-MiB line from MCP or the control channel) before applying the 64 KiB cap, so one oversized request still made several large allocations. - journal_params serializes through a bounded std::io::Write sink that fails with WriteZero the moment the cap is passed, so an oversized value costs at most ~1x the cap and is never fully serialized. - Only when serialization fits is the value cloned for the journal. - Tests exercise the journal helper directly (no 10,500 track.new calls), check ring-buffer eviction (oldest dropped, newest kept) and the cap boundary. Refines the fix for #926. --- crates/engine/src/lib.rs | 109 ++++++++++++++++++++++++++++----------- 1 file changed, 80 insertions(+), 29 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 9bbf75fd..0db9e1cf 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -344,19 +344,7 @@ impl Engine { self.push_undo(spec.label, before); } if spec.journal { - // Ring buffer with a per-entry cap: a hostile/large params value (junk fields, - // huge arrays from MCP or the control channel) must not be kept whole for the - // life of the process, and the journal must not grow without bound. - let entry = (id.to_string(), params.clone()); - let clamped: (String, Value) = if serde_json::to_vec(&entry.1).map(|b| b.len() > MAX_JOURNAL_ENTRY_BYTES).unwrap_or(true) { - (entry.0, json!({"__truncated__": true})) - } else { - entry - }; - self.journal.push_back(clamped); - if self.journal.len() > MAX_JOURNAL_ENTRIES { - self.journal.pop_front(); - } + self.journal_append(id, params); } Ok(v) } @@ -418,6 +406,16 @@ impl Engine { self.merge_key = None; } + /// Append a journaled command, clamping its params to [`MAX_JOURNAL_ENTRY_BYTES`] and the + /// buffer to [`MAX_JOURNAL_ENTRIES`] entries (oldest are dropped). + fn journal_append(&mut self, id: &str, params: &Value) { + let clamped = (id.to_string(), journal_params(params)); + self.journal.push_back(clamped); + if self.journal.len() > MAX_JOURNAL_ENTRIES { + self.journal.pop_front(); + } + } + /// Convenience: execute with `{}` params. pub fn run(&mut self, id: &str) -> Result { self.execute(id, &json!({})) @@ -471,36 +469,89 @@ fn panic_message(p: &(dyn std::any::Any + Send)) -> String { p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()) } +/// The params to keep in the journal for a command: the value itself when it serializes within +/// [`MAX_JOURNAL_ENTRY_BYTES`], or a truncation marker when it is too large or unparseable. +/// +/// Serializes into a [`std::io::Write`] sink that fails as soon as the cap is exceeded, so an +/// oversized `params` (a multi-MiB line from MCP or the control channel) is never fully +/// serialized, cloned, or allocated just to be discarded. +fn journal_params(params: &Value) -> Value { + // Write through a bounded sink: serialization stops (WriteZero) the moment the cap is passed, + // so an oversized value costs at most ~1× the cap, not its full serialized size. + struct Bounded(Vec); + impl std::io::Write for Bounded { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + let room = MAX_JOURNAL_ENTRY_BYTES.saturating_sub(self.0.len()); + if room == 0 { + return Err(std::io::Error::new(std::io::ErrorKind::WriteZero, "journal entry cap")); + } + let take = buf.len().min(room); + self.0.extend_from_slice(&buf[..take]); + if take < buf.len() { + return Err(std::io::Error::new(std::io::ErrorKind::WriteZero, "journal entry cap")); + } + Ok(take) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + let mut sink = Bounded(Vec::new()); + // Fails with WriteZero as soon as the cap is exceeded, so the allocation stays bounded even + // for a multi-MiB params. + let ok = serde_json::to_writer(&mut sink, params).is_ok(); + if !ok { + return json!({"__truncated__": true}); + } + params.clone() +} + #[cfg(test)] mod journal_tests { use super::*; - fn journaled_command(e: &mut Engine) { - // `track.new` is journaled (see its CommandSpec). - e.execute("track.new", &json!({})).unwrap(); - } - #[test] fn journal_is_bounded_to_max_entries() { let mut e = Engine::default(); - for _ in 0..MAX_JOURNAL_ENTRIES + 500 { - journaled_command(&mut e); + // Calling the private append helper directly exercises the ring buffer without the cost + // of cloning the whole document on 10,000+ `track.new` calls. + for i in 0..MAX_JOURNAL_ENTRIES + 500 { + let n = format!("t{i}"); + e.journal_append("track.new", &json!({"name": n})); } assert_eq!(e.journal.len(), MAX_JOURNAL_ENTRIES); - // The oldest entries were dropped: the first remaining id is not the first pushed. - assert!(e.journal.iter().all(|(id, _)| id == "track.new")); + // The oldest entries were dropped, the newest kept: the first remaining is the 501st pushed, + // and the newest still there. + let first = e.journal.front().map(|(_, p)| p["name"].as_str().unwrap_or_default()); + assert_eq!(first, Some("t500")); + let last = e.journal.back().map(|(_, p)| p["name"].as_str().unwrap_or_default()); + let last_name = format!("t{}", MAX_JOURNAL_ENTRIES + 499); + assert_eq!(last, Some(last_name.as_str())); + assert_eq!(e.journal.back().map(|(id, _)| id.as_str()), Some("track.new")); } #[test] fn oversized_params_are_truncated_in_the_journal() { - let mut e = Engine::default(); let big = "x".repeat(MAX_JOURNAL_ENTRY_BYTES * 4); - e.execute("track.new", &json!({"name": big})).unwrap(); - let (id, params) = e.journal.back().expect("a journaled entry"); - assert_eq!(id, "track.new"); - let sz = serde_json::to_vec(params).map(|b| b.len()).unwrap_or(usize::MAX); - assert!(sz <= 1024, "params are truncated, got {sz} bytes"); - assert_eq!(params, &json!({"__truncated__": true})); + assert_eq!(journal_params(&json!({"name": big})), json!({"__truncated__": true})); + } + + #[test] + fn small_params_are_kept_whole() { + let p = json!({"track": "Kick", "db": -6.0}); + assert_eq!(journal_params(&p), p); + } + + #[test] + fn cap_boundary_is_exact() { + // A value just under the cap is kept whole; one just over is truncated. "Just" means the + // bounded writer made at most ~two allocations of cap size, not a full multi-MiB buffer. + // `{"n":"…"}` adds 9 bytes around the string. + let small_str = MAX_JOURNAL_ENTRY_BYTES - 64; + let small = json!({"n": "x".repeat(small_str)}); + assert_eq!(journal_params(&small), small); + let big = json!({"n": "x".repeat(MAX_JOURNAL_ENTRY_BYTES)}); + assert_eq!(journal_params(&big), json!({"__truncated__": true})); } } From 81ab012724e0f68b7821a90b345b57cea3fcce3d Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Thu, 29 Oct 2026 22:00:11 -0400 Subject: [PATCH 3/3] Take the journal as a VecDeque in assert_history_unchanged #995 makes the command journal a bounded VecDeque; the helper #1129 added still took a slice. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tests.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index 892b5393..a0b9a2b6 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -1515,9 +1515,15 @@ fn editor_gesture_polling_survives_discarded_and_hidden_host_iterations() { } /// The engine's revision, undo history and journal are as they were. -fn assert_history_unchanged(app: &SoundApp, revision: u64, undo: usize, history: &[String], journal: &[(String, serde_json::Value)]) { +fn assert_history_unchanged( + app: &SoundApp, + revision: u64, + undo: usize, + history: &[String], + journal: &std::collections::VecDeque<(String, serde_json::Value)>, +) { assert_eq!(app.engine.revision, revision); assert_eq!(app.engine.undo_history().len(), undo); assert_eq!(app.engine.undo_history(), history); - assert_eq!(app.engine.journal, journal); + assert_eq!(&app.engine.journal, journal); }