Pass Console reference to handlers directly

Better reflects the borrowing model

Author: lionel-

Cargo.lock

@@ -10,6 +10,9 @@ edition.workspace = true
 license.workspace = true
 rust-version.workspace = true
 
+[features]
+testing = []
+
 [lints]
 workspace = true
 
@@ -73,6 +76,7 @@ winsafe.workspace = true
 yaml-rust.workspace = true
 
 [dev-dependencies]
+ark = { path = ".", features = ["testing"] }
 ark_test.workspace = true
 assert_matches.workspace = true
 insta.workspace = true

crates/ark/Cargo.toml

@@ -21,9 +21,7 @@ use stdext::result::ResultExt;
 use crate::console::Console;
 
 /// Context provided to `CommHandler` methods, giving access to the outgoing
-/// channel, close-request mechanism, and the Console singleton (via
-/// `console()`). Through the Console, handlers can reach runtime state such
-/// as the graphics device context.
+/// channel and close-request mechanism.
 #[derive(Debug)]
 pub struct CommHandlerContext {
     pub outgoing_tx: CommOutgoingTx,
@@ -59,11 +57,6 @@ impl CommHandlerContext {
         };
         self.outgoing_tx.send(CommMsg::Data(json)).log_err();
     }
-
-    /// Access the Console singleton (the R runtime).
-    pub(crate) fn console(&self) -> &Console {
-        Console::get()
-    }
 }
 
 /// Trait for comm handlers that run synchronously on the R thread.
@@ -79,18 +72,24 @@ pub trait CommHandler: Debug {
 
     /// Initialise handler state on the R thread (initial scan, first event,
     /// etc.). Default is no-op.
-    fn handle_open(&mut self, _ctx: &CommHandlerContext) {}
+    fn handle_open(&mut self, _ctx: &CommHandlerContext, _console: &Console) {}
 
     /// Handle an incoming message (RPC or data).
-    fn handle_msg(&mut self, msg: CommMsg, ctx: &CommHandlerContext);
+    fn handle_msg(&mut self, msg: CommMsg, ctx: &CommHandlerContext, console: &Console);
 
     /// Handle comm close. Default is no-op.
-    fn handle_close(&mut self, _ctx: &CommHandlerContext) {}
+    fn handle_close(&mut self, _ctx: &CommHandlerContext, _console: &Console) {}
 
     /// Called when the environment changes. The `event` indicates what
     /// triggered the change so handlers can decide whether to react.
     /// Default is no-op.
-    fn handle_environment(&mut self, _event: &EnvironmentChanged, _ctx: &CommHandlerContext) {}
+    fn handle_environment(
+        &mut self,
+        _event: &EnvironmentChanged,
+        _ctx: &CommHandlerContext,
+        _console: &Console,
+    ) {
+    }
 }
 
 /// Why the environment changed.

crates/ark/src/comm_handler.rs

@@ -177,7 +177,7 @@ thread_local! {
     pub static CONSOLE: RefCell<UnsafeCell<Console>> = panic!("Must access `CONSOLE` from the R thread");
 }
 
-pub(crate) struct Console {
+pub struct Console {
     pub(crate) positron_ns: Option<RObject>,
 
     kernel_request_rx: Receiver<KernelRequest>,

crates/ark/src/console.rs

@@ -25,7 +25,7 @@ impl Console {
             log::warn!("Received message for unknown registered comm {comm_id}");
             return;
         };
-        comm.handler.handle_msg(msg, &comm.ctx);
+        comm.handler.handle_msg(msg, &comm.ctx, Console::get());
         self.drain_closed();
     }
 
@@ -34,7 +34,7 @@ impl Console {
             log::warn!("Received close for unknown registered comm {comm_id}");
             return;
         };
-        comm.handler.handle_close(&comm.ctx);
+        comm.handler.handle_close(&comm.ctx, Console::get());
     }
 
     /// Register a backend-initiated comm on the R thread.
@@ -57,7 +57,7 @@ impl Console {
         );
 
         let ctx = CommHandlerContext::new(comm.outgoing_tx.clone(), self.comm_event_tx.clone());
-        handler.handle_open(&ctx);
+        handler.handle_open(&ctx, Console::get());
 
         self.comms
             .insert(comm_id.clone(), ConsoleComm { handler, ctx });
@@ -82,13 +82,13 @@ impl Console {
         mut handler: Box<dyn CommHandler>,
     ) {
         let ctx = CommHandlerContext::new(outgoing_tx, self.comm_event_tx.clone());
-        handler.handle_open(&ctx);
+        handler.handle_open(&ctx, Console::get());
 
         if comm_name == UI_COMM_NAME {
             if let Some(old_id) = self.ui_comm_id.take() {
                 log::info!("Replacing an existing UI comm.");
                 if let Some(mut old) = self.comm_remove(&old_id) {
-                    old.handler.handle_close(&old.ctx);
+                    old.handler.handle_close(&old.ctx, Console::get());
                 }
             }
             self.ui_comm_id = Some(comm_id.clone());
@@ -99,7 +99,8 @@ impl Console {
 
     pub(super) fn comm_notify_environment_changed(&mut self, event: &EnvironmentChanged) {
         for (_, comm) in self.comms.iter_mut() {
-            comm.handler.handle_environment(event, &comm.ctx);
+            comm.handler
+                .handle_environment(event, &comm.ctx, Console::get());
         }
         self.drain_closed();
     }

crates/ark/src/console/console_comm.rs

@@ -663,10 +663,66 @@ impl Console {
     /// Access a reference to the singleton instance of this struct
     ///
     /// SAFETY: Accesses must occur after `Console::start()` initializes it.
-    pub(crate) fn get() -> &'static Self {
+    pub fn get() -> &'static Self {
         Console::get_mut()
     }
 
+    /// Install a minimal stopgap `Console` in the thread-local for unit tests
+    /// that need a `&Console` (e.g. to pass to `CommHandler` methods) but
+    /// don't go through the full `Console::start()` path.
+    ///
+    /// All internal channels are created with their counterpart immediately
+    /// dropped, so the Console is inert: it never processes requests, tasks,
+    /// or kernel messages. Any attempt to send to IOPub or other channels
+    /// will return a disconnected error, which surfaces problems early
+    /// rather than silently hanging.
+    ///
+    /// For tests that exercise real Console behaviour (execution, comms
+    /// lifecycle, IOPub output), prefer full integration tests with
+    /// `DummyArkFrontend` which spins up a real Console.
+    ///
+    /// Idempotent per thread. Does not set `R_INIT`, so the `r_task()` escape
+    /// hatch for unit tests continues to work.
+    #[cfg(feature = "testing")]
+    pub fn test_init() {
+        use std::cell::Cell;
+        thread_local! {
+            static INITIALIZED: Cell<bool> = const { Cell::new(false) };
+        }
+
+        INITIALIZED.with(|init| {
+            if init.get() {
+                return;
+            }
+            init.set(true);
+
+            let (_, tasks_interrupt_rx) = crossbeam::channel::unbounded();
+            let (_, tasks_idle_rx) = crossbeam::channel::unbounded();
+            let (_, tasks_idle_any_rx) = crossbeam::channel::unbounded();
+            let (comm_event_tx, _) = crossbeam::channel::unbounded();
+            let (r_request_tx, r_request_rx) = crossbeam::channel::unbounded();
+            let (stdin_request_tx, _) = crossbeam::channel::unbounded();
+            let (_, stdin_reply_rx) = crossbeam::channel::unbounded();
+            let (iopub_tx, _) = crossbeam::channel::unbounded();
+            let (_, kernel_request_rx) = crossbeam::channel::unbounded();
+            let dap = Dap::new_shared(r_request_tx);
+
+            CONSOLE.set(UnsafeCell::new(Console::new(
+                tasks_interrupt_rx,
+                tasks_idle_rx,
+                tasks_idle_any_rx,
+                comm_event_tx,
+                r_request_rx,
+                stdin_request_tx,
+                stdin_reply_rx,
+                iopub_tx,
+                kernel_request_rx,
+                dap,
+                SessionMode::Console,
+            )));
+        });
+    }
+
     /// Access a mutable reference to the singleton instance of this struct
     ///
     /// SAFETY: Accesses must occur after `Console::start()` initializes it.

crates/ark/src/console/console_repl.rs

@@ -481,13 +481,18 @@ impl CommHandler for RDataExplorer {
         serde_json::json!({ "title": self.title, "inline_only": inline_only })
     }
 
-    fn handle_msg(&mut self, msg: CommMsg, ctx: &CommHandlerContext) {
+    fn handle_msg(&mut self, msg: CommMsg, ctx: &CommHandlerContext, _console: &Console) {
         handle_rpc_request(&ctx.outgoing_tx, DATA_EXPLORER_COMM_NAME, msg, |req| {
             self.handle_rpc(req, ctx)
         });
     }
 
-    fn handle_environment(&mut self, event: &EnvironmentChanged, ctx: &CommHandlerContext) {
+    fn handle_environment(
+        &mut self,
+        event: &EnvironmentChanged,
+        ctx: &CommHandlerContext,
+        _console: &Console,
+    ) {
         let EnvironmentChanged::Execution { .. } = event else {
             return;
         };

crates/ark/src/data_explorer/r_data_explorer.rs

@@ -9,6 +9,8 @@ use std::sync::Once;
 
 use tree_sitter::Point;
 
+#[cfg(feature = "testing")]
+use crate::console::Console;
 use crate::modules;
 use crate::modules::ARK_ENVS;
 
@@ -20,6 +22,10 @@ pub fn r_test_init() {
         // Initialize the positron module so tests can use them.
         modules::initialize().unwrap();
     });
+    // Per-thread: install a minimal Console singleton so that unit tests
+    // can pass `&Console` to comm handlers via `Console::get()`.
+    #[cfg(feature = "testing")]
+    Console::test_init();
 }
 
 pub fn point_from_cursor(x: &str) -> (String, Point) {

crates/ark/src/fixtures/utils.rs

@@ -900,15 +900,15 @@ impl CommHandler for PlotComm {
         self.open_data.clone()
     }
 
-    fn handle_msg(&mut self, msg: CommMsg, ctx: &CommHandlerContext) {
-        let dc = ctx.console().device_context();
+    fn handle_msg(&mut self, msg: CommMsg, ctx: &CommHandlerContext, console: &Console) {
+        let dc = console.device_context();
         handle_rpc_request(&ctx.outgoing_tx, PLOT_COMM_NAME, msg, |req| {
             dc.handle_rpc(req, &self.id)
         });
     }
 
-    fn handle_close(&mut self, ctx: &CommHandlerContext) {
-        ctx.console().device_context().on_plot_closed(&self.id);
+    fn handle_close(&mut self, _ctx: &CommHandlerContext, console: &Console) {
+        console.device_context().on_plot_closed(&self.id);
     }
 }
 

crates/ark/src/plots/graphics_device.rs

@@ -50,7 +50,7 @@ pub struct UiComm {
 }
 
 impl CommHandler for UiComm {
-    fn handle_open(&mut self, ctx: &CommHandlerContext) {
+    fn handle_open(&mut self, ctx: &CommHandlerContext, _console: &Console) {
         // Set initial console width from the comm_open data, if provided.
         if let Some(width) = self.comm_open_data.console_width {
             if let Err(err) = RFunction::from(".ps.rpc.setConsoleWidth")
@@ -69,13 +69,18 @@ impl CommHandler for UiComm {
         self.refresh(&input_prompt, &continuation_prompt, ctx);
     }
 
-    fn handle_msg(&mut self, msg: CommMsg, ctx: &CommHandlerContext) {
+    fn handle_msg(&mut self, msg: CommMsg, ctx: &CommHandlerContext, _console: &Console) {
         handle_rpc_request(&ctx.outgoing_tx, UI_COMM_NAME, msg, |req| {
             self.handle_rpc(req)
         });
     }
 
-    fn handle_environment(&mut self, event: &EnvironmentChanged, ctx: &CommHandlerContext) {
+    fn handle_environment(
+        &mut self,
+        event: &EnvironmentChanged,
+        ctx: &CommHandlerContext,
+        _console: &Console,
+    ) {
         let EnvironmentChanged::Execution {
             input_prompt,
             continuation_prompt,
@@ -307,7 +312,7 @@ mod tests {
                 }))
                 .unwrap(),
             };
-            handler.handle_msg(msg, &ctx);
+            handler.handle_msg(msg, &ctx, Console::get());
 
             // Assert that the console width changed
             let new_width: i32 = harp::get_option("width").try_into().unwrap();
@@ -323,7 +328,7 @@ mod tests {
                 }))
                 .unwrap(),
             };
-            handler.handle_msg(msg, &ctx);
+            handler.handle_msg(msg, &ctx, Console::get());
 
             old_width
         });
@@ -369,7 +374,7 @@ mod tests {
                 }))
                 .unwrap(),
             };
-            handler.handle_msg(msg, &ctx);
+            handler.handle_msg(msg, &ctx, Console::get());
         });
 
         let response = iopub_rx.recv_comm_msg();