diff --git a/crates/amalthea/src/comm/ui_comm.rs b/crates/amalthea/src/comm/ui_comm.rs index 7ab9fcca87..6a443babd4 100644 --- a/crates/amalthea/src/comm/ui_comm.rs +++ b/crates/amalthea/src/comm/ui_comm.rs @@ -359,6 +359,19 @@ pub struct ExecuteCodeParams { pub allow_incomplete: bool, } +/// Parameters for the ExecuteCodeInNewSession method. +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +pub struct ExecuteCodeInNewSessionParams { + /// The language ID of the code to execute + pub language_id: String, + + /// The code to execute + pub code: String, + + /// The name of the new session + pub session_name: String, +} + /// Parameters for the OpenWorkspace method. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct OpenWorkspaceParams { @@ -537,6 +550,18 @@ pub enum UiFrontendRequest { #[serde(rename = "execute_code")] ExecuteCode(ExecuteCodeParams), + /// Execute code in a new session + /// + /// Start a new console session in the background, without making it the + /// foreground session, and queue code in it. When a session sends the + /// request, the new session uses that session's runtime, and the request + /// fails if that session is for another language. Otherwise the new + /// session uses the preferred runtime for the language. Returns once the + /// code is queued. The code must be complete; it is run as-is without + /// completeness checks. + #[serde(rename = "execute_code_in_new_session")] + ExecuteCodeInNewSession(ExecuteCodeInNewSessionParams), + /// Path to the workspace folder /// /// Returns the path to the workspace folder, or first folder if there are @@ -596,6 +621,8 @@ pub enum UiFrontendReply { /// Reply for the execute_code method (no result) ExecuteCodeReply(), + ExecuteCodeInNewSessionReply(String), + /// The path to the workspace folder WorkspaceFolderReply(Option), @@ -710,6 +737,7 @@ pub fn ui_frontend_reply_from_value( UiFrontendRequest::ExecuteCommand(_) => Ok(UiFrontendReply::ExecuteCommandReply()), UiFrontendRequest::EvaluateWhenClause(_) => Ok(UiFrontendReply::EvaluateWhenClauseReply(serde_json::from_value(reply)?)), UiFrontendRequest::ExecuteCode(_) => Ok(UiFrontendReply::ExecuteCodeReply()), + UiFrontendRequest::ExecuteCodeInNewSession(_) => Ok(UiFrontendReply::ExecuteCodeInNewSessionReply(serde_json::from_value(reply)?)), UiFrontendRequest::WorkspaceFolder => Ok(UiFrontendReply::WorkspaceFolderReply(serde_json::from_value(reply)?)), UiFrontendRequest::ModifyEditorSelections(_) => Ok(UiFrontendReply::ModifyEditorSelectionsReply()), UiFrontendRequest::LastActiveEditorContext(_) => Ok(UiFrontendReply::LastActiveEditorContextReply(serde_json::from_value(reply)?)), diff --git a/crates/ark/src/modules/positron/frontend-methods.R b/crates/ark/src/modules/positron/frontend-methods.R index 93d5de0af1..c26f05a7bd 100644 --- a/crates/ark/src/modules/positron/frontend-methods.R +++ b/crates/ark/src/modules/positron/frontend-methods.R @@ -73,6 +73,21 @@ .ps.Call("ps_ui_execute_code", code, focus) } +#' @export +.ps.ui.executeCodeInNewSession <- function(code, sessionName) { + tryCatch( + parse(text = code), + error = function(e) { + stop( + "Can't run code in a new session because it does not parse: ", + conditionMessage(e), + call. = FALSE + ) + } + ) + .ps.Call("ps_ui_execute_code_in_new_session", code, sessionName) +} + #' @export .ps.ui.showMessage <- function(message) { .ps.Call("ps_ui_show_message", message) diff --git a/crates/ark/src/modules/rstudio/stubs.R b/crates/ark/src/modules/rstudio/stubs.R index ee920531d7..c8706f091a 100644 --- a/crates/ark/src/modules/rstudio/stubs.R +++ b/crates/ark/src/modules/rstudio/stubs.R @@ -77,3 +77,73 @@ PACKAGE = "(embedding)" )) } + +#' @export +.rs.api.runScriptJob <- function( + path, + name = NULL, + encoding = "unknown", + workingDir = NULL, + importEnv = FALSE, + exportEnv = "" +) { + if (missing(path)) { + stop("Must specify path to R script to run.", call. = FALSE) + } + check_string <- function(x, arg) { + if (!is_string(x)) { + stop("`", arg, "` must be a single string.", call. = FALSE) + } + } + check_string(path, "path") + if (!is.null(name)) { + check_string(name, "name") + } + check_string(encoding, "encoding") + if (!is.null(workingDir)) { + check_string(workingDir, "workingDir") + } + if (!file.exists(path) || dir.exists(path)) { + stop("The R script '", path, "' does not exist.", call. = FALSE) + } + path <- normalizePath(path, winslash = "/") + + if (is.null(name) || !nzchar(name)) { + name <- basename(path) + } + if (is.null(workingDir)) { + workingDir <- dirname(path) + } + if (!dir.exists(workingDir)) { + stop( + "The working directory '", + workingDir, + "' does not exist.", + call. = FALSE + ) + } + workingDir <- normalizePath(workingDir, winslash = "/") + + if (isTRUE(importEnv)) { + warning( + "`importEnv = TRUE` is not supported in Positron and is ignored.", + call. = FALSE + ) + } + if (is_string(exportEnv) && nzchar(exportEnv)) { + warning( + "`exportEnv` is not supported in Positron and is ignored.", + call. = FALSE + ) + } + + # Braces make one expression so a failed setwd() stops the job before source() + code <- sprintf( + "{\n setwd(%s)\n source(%s, encoding = %s)\n}", + deparse(workingDir), + deparse(path), + deparse(encoding) + ) + + invisible(.ps.ui.executeCodeInNewSession(code, name)) +} diff --git a/crates/ark/src/ui/methods.rs b/crates/ark/src/ui/methods.rs index 558229439f..fcf0a366c7 100644 --- a/crates/ark/src/ui/methods.rs +++ b/crates/ark/src/ui/methods.rs @@ -8,6 +8,7 @@ use amalthea::comm::ui_comm::AskForPasswordParams; use amalthea::comm::ui_comm::DebugSleepParams; use amalthea::comm::ui_comm::EvaluateWhenClauseParams; +use amalthea::comm::ui_comm::ExecuteCodeInNewSessionParams; use amalthea::comm::ui_comm::ExecuteCodeParams; use amalthea::comm::ui_comm::ExecuteCommandParams; use amalthea::comm::ui_comm::LastActiveEditorContextParams; @@ -200,6 +201,23 @@ pub unsafe extern "C-unwind" fn ps_ui_execute_code( Ok(out.sexp) } +#[harp::register] +pub unsafe extern "C-unwind" fn ps_ui_execute_code_in_new_session( + code: SEXP, + session_name: SEXP, +) -> anyhow::Result { + let params = ExecuteCodeInNewSessionParams { + language_id: String::from("r"), + code: RObject::view(code).try_into()?, + session_name: RObject::view(session_name).try_into()?, + }; + + let out = Console::get() + .try_ui_comm()? + .call_frontend_method(UiFrontendRequest::ExecuteCodeInNewSession(params))?; + Ok(out.sexp) +} + #[harp::register] pub unsafe extern "C-unwind" fn ps_ui_evaluate_when_clause( when_clause: SEXP, diff --git a/crates/ark/tests/integration/rstudioapi.rs b/crates/ark/tests/integration/rstudioapi.rs index 7f793192e9..3e681cb31f 100644 --- a/crates/ark/tests/integration/rstudioapi.rs +++ b/crates/ark/tests/integration/rstudioapi.rs @@ -1,5 +1,12 @@ +use amalthea::comm::base_comm::JsonRpcReply; +use amalthea::comm::base_comm::JsonRpcResult; +use amalthea::comm::ui_comm::ExecuteCodeInNewSessionParams; +use amalthea::comm::ui_comm::UiFrontendRequest; use amalthea::fixtures::dummy_frontend::ExecuteRequestOptions; +use amalthea::wire::jupyter_message::Message; use ark_test::DummyArkFrontend; +use assert_matches::assert_matches; +use serde_json::Value; #[test] fn test_get_version() { @@ -83,3 +90,324 @@ fn has_rstudioapi(frontend: &DummyArkFrontend) -> bool { fn report_skipped(f: &str) { println!("Skipping `{f}()`. rstudioapi is not installed."); } + +/// Mirrors R's `deparse()` of a string: double-quoted, with `"` and `\\` +/// escaped. Single quotes are left alone. +fn deparse_str(x: &str) -> String { + format!("\"{}\"", x.replace('\\', "\\\\").replace('"', "\\\"")) +} + +/// Executes `code` (which must call `rstudioapi::jobRunScript()`), asserts the +/// `execute_code_in_new_session` request, replies with a session ID, and +/// finishes the execution. Returns the request params. +fn run_job( + frontend: &DummyArkFrontend, + comm_id: &str, + code: &str, +) -> ExecuteCodeInNewSessionParams { + let options = ExecuteRequestOptions { + allow_stdin: true, + ..Default::default() + }; + frontend.send_execute_request(code, options); + frontend.recv_iopub_busy(); + frontend.recv_iopub_execute_input(); + + let request = assert_matches!(frontend.recv_stdin(), Message::CommRequest(msg) => msg.content); + let params = assert_matches!( + request, + UiFrontendRequest::ExecuteCodeInNewSession(params) => params + ); + + frontend.send_stdin(JsonRpcReply::Result(JsonRpcResult { + result: Value::from("job-1"), + })); + + // The result is invisible, so there is no execute result + frontend.recv_ui_prompt_state(comm_id); + frontend.recv_iopub_idle(); + frontend.recv_shell_execute_reply(); + params +} + +/// Executes `code` and asserts that it errors with `needle` without sending a +/// comm request. +fn run_job_error(frontend: &DummyArkFrontend, comm_id: &str, code: &str, needle: &str) { + let options = ExecuteRequestOptions { + allow_stdin: true, + ..Default::default() + }; + frontend.send_execute_request(code, options); + frontend.recv_iopub_busy(); + frontend.recv_iopub_execute_input(); + assert!(frontend.recv_iopub_execute_error().contains(needle)); + frontend.recv_ui_prompt_state(comm_id); + frontend.recv_iopub_idle(); + frontend.recv_shell_execute_reply_exception(); +} + +/// Absolute path with forward slashes, as R's `normalizePath(winslash = "/")` +/// returns it. On Windows `canonicalize()` yields verbatim paths (`\\?\C:\...`, +/// `\\?\UNC\server\...`), which R does not, so strip that prefix. +fn normalized(path: &std::path::Path) -> String { + let path = path.canonicalize().unwrap().to_string_lossy().to_string(); + let path = if let Some(unc) = path.strip_prefix(r"\\?\UNC\") { + format!(r"\\{unc}") + } else { + path.strip_prefix(r"\\?\").unwrap_or(&path).to_string() + }; + path.replace('\\', "/") +} + +fn write_script(dir: &std::path::Path, name: &str) -> String { + let path = dir.join(name); + std::fs::write(&path, "1\n").unwrap(); + normalized(&path) +} + +#[test] +fn test_job_run_script_sources_in_new_session() { + let frontend = DummyArkFrontend::lock(); + if !has_rstudioapi(&frontend) { + report_skipped("test_job_run_script_sources_in_new_session"); + return; + } + + let comm_id = frontend.open_ui_comm(); + + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().join("with space's"); + std::fs::create_dir(&dir).unwrap(); + let path = write_script(&dir, "job.R"); + let dir = normalized(&dir); + + let code = format!("rstudioapi::jobRunScript({})", deparse_str(&path)); + let params = run_job(&frontend, &comm_id, &code); + + assert_eq!( + UiFrontendRequest::ExecuteCodeInNewSession(params), + UiFrontendRequest::ExecuteCodeInNewSession(ExecuteCodeInNewSessionParams { + language_id: String::from("r"), + code: format!( + "{{\n setwd({})\n source({}, encoding = \"unknown\")\n}}", + deparse_str(&dir), + deparse_str(&path) + ), + session_name: String::from("job.R"), + }) + ); +} + +#[test] +fn test_job_run_script_uses_name_and_working_dir() { + let frontend = DummyArkFrontend::lock(); + if !has_rstudioapi(&frontend) { + report_skipped("test_job_run_script_uses_name_and_working_dir"); + return; + } + + let comm_id = frontend.open_ui_comm(); + + let tmp = tempfile::tempdir().unwrap(); + let path = write_script(tmp.path(), "job.R"); + let other = tempfile::tempdir().unwrap(); + let other_dir = normalized(other.path()); + + let code = format!( + "rstudioapi::jobRunScript({}, name = \"targets\", workingDir = {})", + deparse_str(&path), + deparse_str(&other_dir) + ); + let params = run_job(&frontend, &comm_id, &code); + + assert_eq!(params.session_name, "targets"); + assert!(params + .code + .starts_with(&format!("{{\n setwd({})", deparse_str(&other_dir)))); +} + +#[test] +fn test_job_run_script_relative_paths_use_callers_wd() { + let frontend = DummyArkFrontend::lock(); + if !has_rstudioapi(&frontend) { + report_skipped("test_job_run_script_relative_paths_use_callers_wd"); + return; + } + + let comm_id = frontend.open_ui_comm(); + + let tmp = tempfile::tempdir().unwrap(); + std::fs::create_dir(tmp.path().join("sub")).unwrap(); + let root = write_script(tmp.path(), "a.R"); + let root = root.strip_suffix("/a.R").unwrap().to_string(); + + // Restore the working directory so later tests are unaffected + let code = format!( + "local({{ + old <- setwd({}) + on.exit(setwd(old)) + rstudioapi::jobRunScript('a.R', workingDir = 'sub') + }})", + deparse_str(&root) + ); + let params = run_job(&frontend, &comm_id, &code); + + assert_eq!( + params.code, + format!( + "{{\n setwd({})\n source({}, encoding = \"unknown\")\n}}", + deparse_str(&format!("{root}/sub")), + deparse_str(&format!("{root}/a.R")) + ) + ); +} + +#[test] +fn test_job_run_script_missing_working_dir_errors() { + let frontend = DummyArkFrontend::lock(); + if !has_rstudioapi(&frontend) { + report_skipped("test_job_run_script_missing_working_dir_errors"); + return; + } + + let comm_id = frontend.open_ui_comm(); + + let tmp = tempfile::tempdir().unwrap(); + let path = write_script(tmp.path(), "job.R"); + let missing = format!("{}/nope", path.strip_suffix("/job.R").unwrap()); + + let code = format!( + "rstudioapi::jobRunScript({}, workingDir = {})", + deparse_str(&path), + deparse_str(&missing) + ); + run_job_error(&frontend, &comm_id, &code, "working directory"); +} + +#[test] +fn test_job_run_script_missing_file_errors() { + let frontend = DummyArkFrontend::lock(); + if !has_rstudioapi(&frontend) { + report_skipped("test_job_run_script_missing_file_errors"); + return; + } + + let comm_id = frontend.open_ui_comm(); + + run_job_error( + &frontend, + &comm_id, + "rstudioapi::jobRunScript('does-not-exist.R')", + "does-not-exist.R", + ); +} + +#[test] +fn test_run_script_job_shim_missing_file_errors() { + let frontend = DummyArkFrontend::lock(); + let comm_id = frontend.open_ui_comm(); + + // Calls the shim directly, so rstudioapi's own wrapper cannot fail first. + // A comm request would break the error flow `run_job_error` expects. + run_job_error( + &frontend, + &comm_id, + ".rs.api.runScriptJob('does-not-exist.R')", + "does not exist", + ); +} + +#[test] +fn test_run_script_job_shim_rejects_non_scalar_inputs() { + let frontend = DummyArkFrontend::lock(); + let comm_id = frontend.open_ui_comm(); + + // Each call fails validation before any comm request is sent. + for (code, needle) in [ + (".rs.api.runScriptJob(c('a.R', 'b.R'))", "`path`"), + (".rs.api.runScriptJob(NA)", "`path`"), + (".rs.api.runScriptJob('a.R', name = 1)", "`name`"), + ] { + run_job_error(&frontend, &comm_id, code, needle); + } +} + +#[test] +fn test_execute_code_in_new_session_unparseable_code_errors() { + let frontend = DummyArkFrontend::lock(); + let comm_id = frontend.open_ui_comm(); + + // A comm request would break the error flow `run_job_error` expects. + run_job_error( + &frontend, + &comm_id, + ".ps.ui.executeCodeInNewSession('2 +', 'x')", + "does not parse", + ); +} + +#[test] +fn test_job_run_script_warns_on_import_env() { + let frontend = DummyArkFrontend::lock(); + if !has_rstudioapi(&frontend) { + report_skipped("test_job_run_script_warns_on_import_env"); + return; + } + + let comm_id = frontend.open_ui_comm(); + + let tmp = tempfile::tempdir().unwrap(); + let path = write_script(tmp.path(), "job.R"); + + let code = format!( + "rstudioapi::jobRunScript({}, importEnv = TRUE)", + deparse_str(&path) + ); + let options = ExecuteRequestOptions { + allow_stdin: true, + ..Default::default() + }; + frontend.send_execute_request(&code, options); + frontend.recv_iopub_busy(); + frontend.recv_iopub_execute_input(); + + assert_matches!(frontend.recv_stdin(), Message::CommRequest(msg) => { + assert_matches!(msg.content, UiFrontendRequest::ExecuteCodeInNewSession(_)); + }); + frontend.send_stdin(JsonRpcReply::Result(JsonRpcResult { + result: Value::from("job-1"), + })); + + frontend.assert_stream_stderr_contains("importEnv"); + frontend.recv_ui_prompt_state(&comm_id); + frontend.recv_iopub_idle(); + frontend.recv_shell_execute_reply(); +} + +#[test] +fn test_run_script_job_shim_empty_name_falls_back_to_file_name() { + let frontend = DummyArkFrontend::lock(); + let comm_id = frontend.open_ui_comm(); + + let tmp = tempfile::tempdir().unwrap(); + let path = write_script(tmp.path(), "job.R"); + + let code = format!(".rs.api.runScriptJob({}, name = \"\")", deparse_str(&path)); + let params = run_job(&frontend, &comm_id, &code); + assert_eq!(params.session_name, "job.R"); +} + +#[test] +fn test_run_script_job_shim_accepts_vector_export_env() { + let frontend = DummyArkFrontend::lock(); + let comm_id = frontend.open_ui_comm(); + + let tmp = tempfile::tempdir().unwrap(); + let path = write_script(tmp.path(), "job.R"); + + let code = format!( + ".rs.api.runScriptJob({}, exportEnv = c(\"a\", \"b\"))", + deparse_str(&path) + ); + run_job(&frontend, &comm_id, &code); +}