From 883e703b02e89b16632d2887ba8320d40b4fe1e0 Mon Sep 17 00:00:00 2001 From: Sebastian <106036+schoeller@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:49:57 +0200 Subject: [PATCH] Fix V2/V3 messaging ABI-break --- crates/ocs_plugin_api/src/host.rs | 16 ++- crates/ocs_plugin_api/src/process.rs | 128 +++++++++++++++---- crates/ocs_plugin_api/src/process/manager.rs | 13 ++ crates/ocs_plugin_api/src/process/v4.rs | 26 +++- src/app/update/mod.rs | 22 ++++ src/plugin/registry.rs | 1 - src/ui/window/plugin_manager.rs | 11 +- 7 files changed, 179 insertions(+), 38 deletions(-) diff --git a/crates/ocs_plugin_api/src/host.rs b/crates/ocs_plugin_api/src/host.rs index 66582f10..35d28360 100644 --- a/crates/ocs_plugin_api/src/host.rs +++ b/crates/ocs_plugin_api/src/host.rs @@ -395,12 +395,6 @@ pub trait HostApi { /// Add an entity to the active document, returning its handle. fn add_entity(&mut self, entity: EntityType) -> Handle; - /// Add multiple entities to the active document, returning their handles. - /// The default implementation calls [`add_entity`](Self::add_entity) for each - /// entity; hosts should override it for batch efficiency. - fn add_entities(&mut self, entities: Vec) -> Vec { - entities.into_iter().map(|e| self.add_entity(e)).collect() - } /// Replace the existing entity that carries `entity`'s handle, preserving /// its identity (handle and owning block). Returns `false` when no entity /// has that handle. This is the sanctioned way to commit in-place edits @@ -519,6 +513,16 @@ pub trait HostApi { fn close_document_view_v4(&mut self, tab_id: u64) { let _ = tab_id; } + + // ── Batch entities (added after API v4; appended at the very end so older + // plugins compiled without it keep stable vtable indices) ──────────────── + + /// Add multiple entities to the active document, returning their handles. + /// The default implementation calls [`add_entity`](Self::add_entity) for each + /// entity; hosts should override it for batch efficiency. + fn add_entities(&mut self, entities: Vec) -> Vec { + entities.into_iter().map(|e| self.add_entity(e)).collect() + } } /// Simplified, read-only entity kind exposed by [`DocumentReader`]. diff --git a/crates/ocs_plugin_api/src/process.rs b/crates/ocs_plugin_api/src/process.rs index 6b5170c2..239a6392 100644 --- a/crates/ocs_plugin_api/src/process.rs +++ b/crates/ocs_plugin_api/src/process.rs @@ -27,6 +27,33 @@ mod manager; mod v4; pub use manager::{DispatchResult, NotificationHandler, PluginManager}; +/// A line emitted by a plugin process on stdout or stderr. +#[derive(Debug, Clone)] +pub struct PluginIoLine { + /// Which stream the line came from. + pub source: IoStream, + /// Plugin id that produced the line. + pub plugin_id: String, + /// Text content without the trailing newline. + pub text: String, +} + +/// Stream source for a [`PluginIoLine`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum IoStream { + Stdout, + Stderr, +} + +impl std::fmt::Display for IoStream { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + IoStream::Stdout => write!(f, "stdout"), + IoStream::Stderr => write!(f, "stderr"), + } + } +} + /// A dummy `HostApi` used for V4 paths that do not supply a real host surface /// (e.g., interactive events). Nested plugin requests receive safe defaults. struct NullHost; @@ -226,6 +253,7 @@ pub struct PluginProcess { id: String, manifest: OwnedPluginManifest, ribbon: Vec, + io_lines: Mutex>>, } impl PluginProcess { @@ -252,28 +280,15 @@ impl PluginProcess { // Create the listener before spawning so the runner can connect immediately. let listener = ListenerOptions::new().name(socket_name_ref).create_sync()?; - let mut child = Command::new(&runner_path) + let child = Command::new(&runner_path) .arg("--ocs-plugin-runner") .arg(&socket_name) .arg(cdylib_path) .env(PLUGIN_TOKEN_ENV, &token) .stdin(Stdio::null()) - .stdout(Stdio::null()) + .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn()?; - let last_stderr: Arc> = Arc::new(Mutex::new(String::new())); - if let Some(stderr) = child.stderr.take() { - let last_stderr = Arc::clone(&last_stderr); - std::thread::spawn(move || { - use std::io::{BufRead, BufReader}; - for line in BufReader::new(stderr).lines().map_while(Result::ok) { - if let Ok(mut guard) = last_stderr.lock() { - *guard = line.clone(); - } - eprintln!("[runner stderr] {line}"); - } - }); - } let child = Mutex::new(Some(child)); // Accept the runner connection with a timeout so a hung/crashed runner @@ -282,6 +297,7 @@ impl PluginProcess { std::thread::spawn(move || { let _ = tx.send(listener.accept()); }); + let last_stderr: Arc> = Arc::new(Mutex::new(String::new())); let stream = match rx.recv_timeout(spawn_timeout()) { Ok(Ok(stream)) => { vlog!("[plugin] runner connected"); @@ -384,6 +400,43 @@ impl PluginProcess { ); let id = manifest.id.clone(); + + // Start forwarding plugin stdout/stderr to the host UI. + let (io_tx, io_rx) = mpsc::channel::(); + if let Some(child_ref) = child.lock().unwrap_or_else(|e| e.into_inner()).as_mut() { + let plugin_id_stdout = id.clone(); + if let Some(stdout) = child_ref.stdout.take() { + let io_tx = io_tx.clone(); + std::thread::spawn(move || { + use std::io::{BufRead, BufReader}; + for line in BufReader::new(stdout).lines().map_while(Result::ok) { + let _ = io_tx.send(PluginIoLine { + source: IoStream::Stdout, + plugin_id: plugin_id_stdout.clone(), + text: line, + }); + } + }); + } + let plugin_id_stderr = id.clone(); + if let Some(stderr) = child_ref.stderr.take() { + let last_stderr = Arc::clone(&last_stderr); + std::thread::spawn(move || { + use std::io::{BufRead, BufReader}; + for line in BufReader::new(stderr).lines().map_while(Result::ok) { + if let Ok(mut guard) = last_stderr.lock() { + *guard = line.clone(); + } + let _ = io_tx.send(PluginIoLine { + source: IoStream::Stderr, + plugin_id: plugin_id_stderr.clone(), + text: line, + }); + } + }); + } + } + Ok(Self { stream, v4, @@ -391,6 +444,7 @@ impl PluginProcess { id, manifest, ribbon, + io_lines: Mutex::new(Some(io_rx)), }) } @@ -406,6 +460,22 @@ impl PluginProcess { &self.ribbon } + /// Drain any stdout/stderr lines that have accumulated from the plugin + /// runner. This should be called regularly by the host (e.g., alongside + /// [`drain_requests`]) so plugin `println!` / `eprintln!` output appears in + /// the host command line. + pub fn drain_io(&self) -> Vec { + let mut rx = self.io_lines.lock().unwrap_or_else(|e| e.into_inner()); + let Some(rx) = rx.as_mut() else { + return Vec::new(); + }; + let mut out = Vec::new(); + while let Ok(line) = rx.try_recv() { + out.push(line); + } + out + } + pub fn dispatch( &self, host: &mut dyn HostApi, @@ -1045,6 +1115,19 @@ mod timeout_tests { struct DummyHost { doc: CadDocument, + push_info_messages: StdMutex>, + } + + impl DummyHost { + fn new(doc: CadDocument) -> Self { + Self { + doc, + push_info_messages: StdMutex::new(Vec::new()), + } + } + fn take_push_info(&self) -> Vec { + std::mem::take(&mut *self.push_info_messages.lock().unwrap()) + } } impl HostApi for DummyHost { @@ -1075,7 +1158,9 @@ mod timeout_tests { } fn push_undo(&mut self, _label: &str) {} fn set_dirty(&mut self) {} - fn push_info(&mut self, _msg: &str) {} + fn push_info(&mut self, msg: &str) { + self.push_info_messages.lock().unwrap().push(msg.to_string()); + } fn push_output(&mut self, _msg: &str) {} fn push_error(&mut self, _msg: &str) {} fn start_interactive(&mut self, _command: Box) {} @@ -1164,6 +1249,7 @@ mod timeout_tests { id: "test.plugin".to_string(), manifest: fake_manifest(), ribbon: vec![], + io_lines: Mutex::new(None), }; (process, runner_stream) } @@ -1189,9 +1275,7 @@ mod timeout_tests { let _ = recv::(&mut peer); }); - let mut host = DummyHost { - doc: CadDocument::default(), - }; + let mut host = DummyHost::new(CadDocument::default()); let start = Instant::now(); let result = process.dispatch(&mut host, "HANG", &mut |_| {}); let elapsed = start.elapsed(); @@ -1254,12 +1338,12 @@ mod timeout_tests { .expect("send final response"); }); - let mut host = DummyHost { - doc: CadDocument::default(), - }; + let mut host = DummyHost::new(CadDocument::default()); let result = process.dispatch(&mut host, "NESTED", &mut |_| {}); assert!(result.expect("dispatch succeeds")); assert!(process.is_alive(), "process should still be alive"); + let infos = host.take_push_info(); + assert_eq!(infos, vec!["hello".to_string()], "push_info should be delivered to host"); // Clean up the helper child so it does not outlive the test. if let Some(mut child) = process diff --git a/crates/ocs_plugin_api/src/process/manager.rs b/crates/ocs_plugin_api/src/process/manager.rs index 54f69030..319be98c 100644 --- a/crates/ocs_plugin_api/src/process/manager.rs +++ b/crates/ocs_plugin_api/src/process/manager.rs @@ -193,6 +193,19 @@ impl PluginManager { } } + /// Drain any plugin stdout/stderr lines that have accumulated across all + /// loaded plugins. Errors are logged; this is best-effort. + pub fn drain_io(&self) -> Vec { + let mut out = Vec::new(); + for p in &self.plugins { + if !p.process.is_alive() { + continue; + } + out.extend(p.process.drain_io()); + } + out + } + /// Begin asynchronous shutdown of every plugin process. /// /// Kills every child synchronously on the calling thread and moves the diff --git a/crates/ocs_plugin_api/src/process/v4.rs b/crates/ocs_plugin_api/src/process/v4.rs index 1df2675d..a9cfabd0 100644 --- a/crates/ocs_plugin_api/src/process/v4.rs +++ b/crates/ocs_plugin_api/src/process/v4.rs @@ -367,7 +367,19 @@ mod tests { } } - struct DummyHost; + struct DummyHost { + push_info_messages: StdMutex>, + } + impl DummyHost { + fn new() -> Self { + Self { + push_info_messages: StdMutex::new(Vec::new()), + } + } + fn take_push_info(&self) -> Vec { + std::mem::take(&mut *self.push_info_messages.lock().unwrap()) + } + } impl HostApi for DummyHost { fn tab_index(&self) -> usize { 0 @@ -404,7 +416,9 @@ mod tests { } fn push_undo(&mut self, _label: &str) {} fn set_dirty(&mut self) {} - fn push_info(&mut self, _msg: &str) {} + fn push_info(&mut self, msg: &str) { + self.push_info_messages.lock().unwrap().push(msg.to_string()); + } fn push_output(&mut self, _msg: &str) {} fn push_error(&mut self, _msg: &str) {} fn start_interactive(&mut self, _command: Box) {} @@ -489,7 +503,7 @@ mod tests { } }); - let mut host = DummyHost; + let mut host = DummyHost::new(); let resp = conn .call(&mut host, HostRequest::Dispatch { cmd: "HELLO".to_string() }, &mut |_| {}) .unwrap(); @@ -541,11 +555,13 @@ mod tests { } }); - let mut host = DummyHost; + let mut host = DummyHost::new(); let resp = conn .call(&mut host, HostRequest::Dispatch { cmd: "NESTED".to_string() }, &mut |_| {}) .unwrap(); assert!(matches!(resp, HostResponse::Bool(true))); + let infos = host.take_push_info(); + assert_eq!(infos, vec!["nested".to_string()], "push_info should be delivered to host"); runner.join().unwrap(); restore_test_env(); } @@ -731,7 +747,7 @@ mod tests { } }); - let mut host = DummyHost; + let mut host = DummyHost::new(); let result = conn .execute_code(&mut host, 1, CommandSource::Editor, "1+1") .expect("execute_code should succeed"); diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index 71f1f393..bc43fd08 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -313,6 +313,28 @@ impl OpenCADStudio { crate::plugin::external::with_manager(|mgr| { mgr.drain_requests(&mut host, &mut |_| {}); }); + crate::plugin::external::with_manager(|mgr| { + for line in mgr.drain_io() { + // Skip internal runner/plugin/REPL trace noise when + // verbose logging is enabled; real plugin stdout/stderr + // and non-trace runner errors still pass through. + if line.text.starts_with("[runner]") + || line.text.starts_with("[plugin] ") + || line.text.starts_with("[python-repl]") + { + continue; + } + let prefixed = format!("[{} {}] {}", line.plugin_id, line.source, line.text); + match line.source { + ocs_plugin_api::process::IoStream::Stderr => { + self.command_line.push_error(&prefixed); + } + _ => { + self.command_line.push_output(&prefixed); + } + } + } + }); } Task::none() } diff --git a/src/plugin/registry.rs b/src/plugin/registry.rs index fed97df6..9137bb99 100644 --- a/src/plugin/registry.rs +++ b/src/plugin/registry.rs @@ -62,7 +62,6 @@ pub(crate) fn try_dispatch(app: &mut OpenCADStudio, tab: usize, cmd: &str) -> bo manager.dispatch(&mut host, cmd, |id| disabled.contains(id)) }) }; - for id in result.dead_plugins { app.push_plugin_error(&format!("Plugin '{id}' process died; skipping dispatch")); } diff --git a/src/ui/window/plugin_manager.rs b/src/ui/window/plugin_manager.rs index df4dd4dc..6a212cc2 100644 --- a/src/ui/window/plugin_manager.rs +++ b/src/ui/window/plugin_manager.rs @@ -218,8 +218,7 @@ fn external_card<'a>( disabled: bool, selected: bool, ) -> Element<'a, Message> { - let failed_old_api = - load_error.is_some() && p.api_version != ocs_plugin_api::API_VERSION; + let failed_old_api = load_error.is_some() && !p.api_compatible(); let (status, kind) = if loaded && disabled { (t!("Disabled"), StatusKind::Muted) } else if loaded { @@ -369,7 +368,9 @@ fn install_controls<'a>( .into() }; let action = match selected_api { - Some(api_version) if api_version == ocs_plugin_api::API_VERSION => { + Some(api_version) + if ocs_plugin_api::manifest::host_accepts_plugin_version(api_version) => + { pill_button( t!("Install"), Message::PluginInstall(repo_s.clone()), @@ -845,7 +846,9 @@ pub fn view_window<'a>( let compatible_tags = releases .iter() .filter(|release| { - release.api_version == ocs_plugin_api::API_VERSION + ocs_plugin_api::manifest::host_accepts_plugin_version( + release.api_version, + ) }) .map(|release| release.tag.clone()) .collect::>();