From 40ab71a9852d36745542a7c9367cc67af6fb2df9 Mon Sep 17 00:00:00 2001 From: Curtis 'Fjord' Hawthorne Date: Wed, 25 Feb 2026 17:14:51 -0800 Subject: [PATCH] Disable js_repl when Node is incompatible at startup (#12824) ## Summary - validate `js_repl` Node compatibility during session startup when the experiment is enabled - if Node is missing or too old, disable `js_repl` and `js_repl_tools_only` for the session before tools and instructions are built - surface that startup disablement to users through the existing startup warning flow instead of only logging it - reuse the same compatibility check in js_repl kernel startup so startup gating and runtime behavior stay aligned - add a regression test that verifies the warning is emitted and that the first advertised tool list omits `js_repl` and `js_repl_reset` when Node is incompatible ## Why Today `js_repl` can be advertised based only on the feature flag, then fail later when the kernel starts. That makes the available tool list inaccurate at the start of a conversation, and users do not get a clear explanation for why the tool is unavailable. This change makes tool availability reflect real startup checks, keeps the advertised tool set stable for the lifetime of the session, and gives users a visible warning when `js_repl` is disabled. ## Testing - `just fmt` - `cargo test -p codex-core --test all js_repl_is_not_advertised_when_startup_node_is_incompatible` --- codex-rs/core/src/codex.rs | 13 ++++ codex-rs/core/src/tools/js_repl/mod.rs | 13 +++- codex-rs/core/tests/suite/js_repl.rs | 104 +++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 6566f0848..b2145afae 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -251,6 +251,7 @@ use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::handlers::SEARCH_TOOL_BM25_TOOL_NAME; use crate::tools::js_repl::JsReplHandle; +use crate::tools::js_repl::resolve_compatible_node; use crate::tools::network_approval::NetworkApprovalService; use crate::tools::network_approval::build_blocked_request_observer; use crate::tools::network_approval::build_network_policy_decider; @@ -341,6 +342,18 @@ impl Codex { config.features.disable(Feature::Collab); } + if config.features.enabled(Feature::JsRepl) + && let Err(err) = resolve_compatible_node(config.js_repl_node_path.as_deref()).await + { + let message = format!( + "Disabled `js_repl` for this session because the configured Node runtime is unavailable or incompatible. {err}" + ); + warn!("{message}"); + config.features.disable(Feature::JsRepl); + config.features.disable(Feature::JsReplToolsOnly); + config.startup_warnings.push(message); + } + let allowed_skills_for_implicit_invocation = loaded_skills.allowed_skills_for_implicit_invocation(); let user_instructions = diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 066cb8258..aaa1ab29d 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -556,10 +556,7 @@ impl JsReplManager { turn: Arc, thread_id: Option, ) -> Result { - let node_path = resolve_node(self.node_path.as_deref()).ok_or_else(|| { - "Node runtime not found; install Node or set CODEX_JS_REPL_NODE_PATH".to_string() - })?; - ensure_node_version(&node_path).await?; + let node_path = resolve_compatible_node(self.node_path.as_deref()).await?; let kernel_path = self .write_kernel_script() @@ -1321,6 +1318,14 @@ async fn ensure_node_version(node_path: &Path) -> Result<(), String> { Ok(()) } +pub(crate) async fn resolve_compatible_node(config_path: Option<&Path>) -> Result { + let node_path = resolve_node(config_path).ok_or_else(|| { + "Node runtime not found; install Node or set CODEX_JS_REPL_NODE_PATH".to_string() + })?; + ensure_node_version(&node_path).await?; + Ok(node_path) +} + pub(crate) fn resolve_node(config_path: Option<&Path>) -> Option { if let Some(path) = std::env::var_os("CODEX_JS_REPL_NODE_PATH") { let p = PathBuf::from(path); diff --git a/codex-rs/core/tests/suite/js_repl.rs b/codex-rs/core/tests/suite/js_repl.rs index ba2b52fd6..31dbe6e60 100644 --- a/codex-rs/core/tests/suite/js_repl.rs +++ b/codex-rs/core/tests/suite/js_repl.rs @@ -2,6 +2,7 @@ use anyhow::Result; use codex_core::features::Feature; +use codex_protocol::protocol::EventMsg; use core_test_support::responses; use core_test_support::responses::ResponseMock; use core_test_support::responses::ResponsesRequest; @@ -12,6 +13,12 @@ use core_test_support::responses::ev_response_created; use core_test_support::responses::sse; use core_test_support::skip_if_no_network; use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event_match; +use std::fs; +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt; +use std::path::Path; +use tempfile::tempdir; use wiremock::MockServer; fn custom_tool_output_text_and_success( @@ -24,6 +31,45 @@ fn custom_tool_output_text_and_success( (output.unwrap_or_default(), success) } +fn tool_names(body: &serde_json::Value) -> Vec { + body["tools"] + .as_array() + .expect("tools array should be present") + .iter() + .map(|tool| { + tool.get("name") + .and_then(|value| value.as_str()) + .or_else(|| tool.get("type").and_then(|value| value.as_str())) + .expect("tool should have a name or type") + .to_string() + }) + .collect() +} + +fn write_too_old_node_script(dir: &Path) -> Result { + #[cfg(windows)] + { + let path = dir.join("old-node.cmd"); + fs::write(&path, "@echo off\r\necho v0.0.1\r\n")?; + Ok(path) + } + + #[cfg(unix)] + { + let path = dir.join("old-node.sh"); + fs::write(&path, "#!/bin/sh\necho v0.0.1\n")?; + let mut permissions = fs::metadata(&path)?.permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&path, permissions)?; + Ok(path) + } + + #[cfg(not(any(unix, windows)))] + { + anyhow::bail!("unsupported platform for js_repl test fixture"); + } +} + async fn run_js_repl_turn( server: &MockServer, prompt: &str, @@ -54,6 +100,64 @@ async fn run_js_repl_turn( Ok(second_mock) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn js_repl_is_not_advertised_when_startup_node_is_incompatible() -> Result<()> { + skip_if_no_network!(Ok(())); + if std::env::var_os("CODEX_JS_REPL_NODE_PATH").is_some() { + return Ok(()); + } + + let server = responses::start_mock_server().await; + let temp = tempdir()?; + let old_node = write_too_old_node_script(temp.path())?; + + let mut builder = test_codex().with_config(move |config| { + config.features.enable(Feature::JsRepl); + config.js_repl_node_path = Some(old_node); + }); + let test = builder.build(&server).await?; + let warning = wait_for_event_match(&test.codex, |event| match event { + EventMsg::Warning(ev) if ev.message.contains("Disabled `js_repl` for this session") => { + Some(ev.message.clone()) + } + _ => None, + }) + .await; + assert!( + warning.contains("Node runtime"), + "warning should explain the Node compatibility issue: {warning}" + ); + + let request_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-1"), + ]), + ) + .await; + + test.submit_turn("hello").await?; + + let body = request_mock.single_request().body_json(); + let tools = tool_names(&body); + assert!( + !tools.iter().any(|tool| tool == "js_repl"), + "js_repl should be omitted when startup validation fails: {tools:?}" + ); + assert!( + !tools.iter().any(|tool| tool == "js_repl_reset"), + "js_repl_reset should be omitted when startup validation fails: {tools:?}" + ); + let instructions = body["instructions"].as_str().unwrap_or_default(); + assert!( + !instructions.contains("## JavaScript REPL (Node)"), + "startup instructions should not mention js_repl when it is disabled: {instructions}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn js_repl_persists_top_level_bindings_and_supports_tla() -> Result<()> { skip_if_no_network!(Ok(()));