mirror of
https://github.com/openai/codex.git
synced 2026-03-24 09:06:33 +03:00
Compare commits
1 Commits
dev/cc/ref
...
cconger/js
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
cab6e60b08 |
@@ -122,6 +122,9 @@ pub(crate) async fn apply_role_to_config(
|
||||
codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(),
|
||||
main_execve_wrapper_exe: config.main_execve_wrapper_exe.clone(),
|
||||
js_repl_node_path: config.js_repl_node_path.clone(),
|
||||
experimental_js_repl_runtime_command: config
|
||||
.experimental_js_repl_runtime_command
|
||||
.clone(),
|
||||
..Default::default()
|
||||
},
|
||||
config.codex_home.clone(),
|
||||
|
||||
@@ -283,7 +283,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::js_repl::validate_js_repl_runtime;
|
||||
use crate::tools::network_approval::NetworkApprovalService;
|
||||
use crate::tools::network_approval::build_blocked_request_observer;
|
||||
use crate::tools::network_approval::build_network_policy_decider;
|
||||
@@ -383,17 +383,21 @@ impl Codex {
|
||||
}
|
||||
|
||||
if config.features.enabled(Feature::JsRepl)
|
||||
&& let Err(err) = resolve_compatible_node(config.js_repl_node_path.as_deref()).await
|
||||
&& let Err(err) = validate_js_repl_runtime(
|
||||
config.experimental_js_repl_runtime_command.as_deref(),
|
||||
config.js_repl_node_path.as_deref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
let _ = config.features.disable(Feature::JsRepl);
|
||||
let _ = config.features.disable(Feature::JsReplToolsOnly);
|
||||
let message = if config.features.enabled(Feature::JsRepl) {
|
||||
format!(
|
||||
"`js_repl` remains enabled because enterprise requirements pin it on, but the configured Node runtime is unavailable or incompatible. {err}"
|
||||
"`js_repl` remains enabled because enterprise requirements pin it on, but the configured experimental js_repl runtime is unavailable or invalid. {err}"
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"Disabled `js_repl` for this session because the configured Node runtime is unavailable or incompatible. {err}"
|
||||
"Disabled `js_repl` for this session because the configured experimental js_repl runtime is unavailable or invalid. {err}"
|
||||
)
|
||||
};
|
||||
warn!("{message}");
|
||||
@@ -1582,9 +1586,10 @@ impl Session {
|
||||
Self::build_model_client_beta_features_header(config.as_ref()),
|
||||
),
|
||||
};
|
||||
let js_repl = Arc::new(JsReplHandle::with_node_path(
|
||||
let js_repl = Arc::new(JsReplHandle::with_runtime_config(
|
||||
config.js_repl_node_path.clone(),
|
||||
config.js_repl_node_module_dirs.clone(),
|
||||
config.experimental_js_repl_runtime_command.clone(),
|
||||
));
|
||||
|
||||
let sess = Arc::new(Session {
|
||||
|
||||
@@ -2105,9 +2105,10 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
Session::build_model_client_beta_features_header(config.as_ref()),
|
||||
),
|
||||
};
|
||||
let js_repl = Arc::new(JsReplHandle::with_node_path(
|
||||
let js_repl = Arc::new(JsReplHandle::with_runtime_config(
|
||||
config.js_repl_node_path.clone(),
|
||||
config.js_repl_node_module_dirs.clone(),
|
||||
config.experimental_js_repl_runtime_command.clone(),
|
||||
));
|
||||
|
||||
let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&per_turn_config));
|
||||
@@ -2512,9 +2513,10 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
|
||||
Session::build_model_client_beta_features_header(config.as_ref()),
|
||||
),
|
||||
};
|
||||
let js_repl = Arc::new(JsReplHandle::with_node_path(
|
||||
let js_repl = Arc::new(JsReplHandle::with_runtime_config(
|
||||
config.js_repl_node_path.clone(),
|
||||
config.js_repl_node_module_dirs.clone(),
|
||||
config.experimental_js_repl_runtime_command.clone(),
|
||||
));
|
||||
|
||||
let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&per_turn_config));
|
||||
|
||||
@@ -3048,6 +3048,7 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
||||
main_execve_wrapper_exe: None,
|
||||
js_repl_node_path: None,
|
||||
js_repl_node_module_dirs: Vec::new(),
|
||||
experimental_js_repl_runtime_command: None,
|
||||
zsh_path: None,
|
||||
hide_agent_reasoning: false,
|
||||
show_raw_agent_reasoning: false,
|
||||
@@ -3183,6 +3184,7 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
main_execve_wrapper_exe: None,
|
||||
js_repl_node_path: None,
|
||||
js_repl_node_module_dirs: Vec::new(),
|
||||
experimental_js_repl_runtime_command: None,
|
||||
zsh_path: None,
|
||||
hide_agent_reasoning: false,
|
||||
show_raw_agent_reasoning: false,
|
||||
@@ -3316,6 +3318,7 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
||||
main_execve_wrapper_exe: None,
|
||||
js_repl_node_path: None,
|
||||
js_repl_node_module_dirs: Vec::new(),
|
||||
experimental_js_repl_runtime_command: None,
|
||||
zsh_path: None,
|
||||
hide_agent_reasoning: false,
|
||||
show_raw_agent_reasoning: false,
|
||||
@@ -3435,6 +3438,7 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
main_execve_wrapper_exe: None,
|
||||
js_repl_node_path: None,
|
||||
js_repl_node_module_dirs: Vec::new(),
|
||||
experimental_js_repl_runtime_command: None,
|
||||
zsh_path: None,
|
||||
hide_agent_reasoning: false,
|
||||
show_raw_agent_reasoning: false,
|
||||
@@ -3985,6 +3989,41 @@ allow_login_shell = false
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_loads_experimental_js_repl_runtime_command_from_toml() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cfg: ConfigToml = toml::from_str(
|
||||
r#"
|
||||
model = "gpt-5.1"
|
||||
experimental_js_repl_runtime_command = ["/usr/bin/env", "poliwhirl-codex-kernel"]
|
||||
"#,
|
||||
)
|
||||
.expect("TOML deserialization should succeed for experimental js_repl runtime command");
|
||||
|
||||
assert_eq!(
|
||||
cfg.experimental_js_repl_runtime_command,
|
||||
Some(vec![
|
||||
"/usr/bin/env".to_string(),
|
||||
"poliwhirl-codex-kernel".to_string(),
|
||||
])
|
||||
);
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
cfg,
|
||||
ConfigOverrides::default(),
|
||||
codex_home.path().to_path_buf(),
|
||||
)?;
|
||||
|
||||
assert_eq!(
|
||||
config.experimental_js_repl_runtime_command,
|
||||
Some(vec![
|
||||
"/usr/bin/env".to_string(),
|
||||
"poliwhirl-codex-kernel".to_string(),
|
||||
])
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_loads_mcp_oauth_callback_url_from_toml() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
@@ -420,6 +420,10 @@ pub struct Config {
|
||||
/// Ordered list of directories to search for Node modules in `js_repl`.
|
||||
pub js_repl_node_module_dirs: Vec<PathBuf>,
|
||||
|
||||
/// Experimental / do not use. Optional external stdio command used instead
|
||||
/// of the built-in Node kernel for `js_repl`.
|
||||
pub experimental_js_repl_runtime_command: Option<Vec<String>>,
|
||||
|
||||
/// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution.
|
||||
pub zsh_path: Option<PathBuf>,
|
||||
|
||||
@@ -1162,6 +1166,11 @@ pub struct ConfigToml {
|
||||
/// Ordered list of directories to search for Node modules in `js_repl`.
|
||||
pub js_repl_node_module_dirs: Option<Vec<AbsolutePathBuf>>,
|
||||
|
||||
/// Experimental / do not use. Optional external stdio command used instead
|
||||
/// of the built-in Node kernel for `js_repl`.
|
||||
#[schemars(skip)]
|
||||
pub experimental_js_repl_runtime_command: Option<Vec<String>>,
|
||||
|
||||
/// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution.
|
||||
pub zsh_path: Option<AbsolutePathBuf>,
|
||||
|
||||
@@ -1680,6 +1689,7 @@ pub struct ConfigOverrides {
|
||||
pub main_execve_wrapper_exe: Option<PathBuf>,
|
||||
pub js_repl_node_path: Option<PathBuf>,
|
||||
pub js_repl_node_module_dirs: Option<Vec<PathBuf>>,
|
||||
pub experimental_js_repl_runtime_command: Option<Vec<String>>,
|
||||
pub zsh_path: Option<PathBuf>,
|
||||
pub base_instructions: Option<String>,
|
||||
pub developer_instructions: Option<String>,
|
||||
@@ -1844,6 +1854,7 @@ impl Config {
|
||||
main_execve_wrapper_exe,
|
||||
js_repl_node_path: js_repl_node_path_override,
|
||||
js_repl_node_module_dirs: js_repl_node_module_dirs_override,
|
||||
experimental_js_repl_runtime_command: experimental_js_repl_runtime_command_override,
|
||||
zsh_path: zsh_path_override,
|
||||
base_instructions,
|
||||
developer_instructions,
|
||||
@@ -2247,6 +2258,9 @@ impl Config {
|
||||
.map(|dirs| dirs.into_iter().map(Into::into).collect::<Vec<PathBuf>>())
|
||||
})
|
||||
.unwrap_or_default();
|
||||
let experimental_js_repl_runtime_command = experimental_js_repl_runtime_command_override
|
||||
.or(config_profile.experimental_js_repl_runtime_command)
|
||||
.or(cfg.experimental_js_repl_runtime_command);
|
||||
let zsh_path = zsh_path_override
|
||||
.or(config_profile.zsh_path.map(Into::into))
|
||||
.or(cfg.zsh_path.map(Into::into));
|
||||
@@ -2409,6 +2423,7 @@ impl Config {
|
||||
main_execve_wrapper_exe,
|
||||
js_repl_node_path,
|
||||
js_repl_node_module_dirs,
|
||||
experimental_js_repl_runtime_command,
|
||||
zsh_path,
|
||||
|
||||
hide_agent_reasoning: cfg.hide_agent_reasoning.unwrap_or(false),
|
||||
|
||||
@@ -39,6 +39,10 @@ pub struct ConfigProfile {
|
||||
pub js_repl_node_path: Option<AbsolutePathBuf>,
|
||||
/// Ordered list of directories to search for Node modules in `js_repl`.
|
||||
pub js_repl_node_module_dirs: Option<Vec<AbsolutePathBuf>>,
|
||||
/// Experimental / do not use. Optional external stdio command used instead
|
||||
/// of the built-in Node kernel for `js_repl`.
|
||||
#[schemars(skip)]
|
||||
pub experimental_js_repl_runtime_command: Option<Vec<String>>,
|
||||
/// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution.
|
||||
pub zsh_path: Option<AbsolutePathBuf>,
|
||||
/// Deprecated: ignored. Use `model_instructions_file`.
|
||||
|
||||
@@ -64,6 +64,7 @@ const JS_REPL_TOOL_RESPONSE_TEXT_PREVIEW_MAX_BYTES: usize = 512;
|
||||
pub(crate) struct JsReplHandle {
|
||||
node_path: Option<PathBuf>,
|
||||
node_module_dirs: Vec<PathBuf>,
|
||||
runtime_command: Option<Vec<String>>,
|
||||
cell: OnceCell<Arc<JsReplManager>>,
|
||||
}
|
||||
|
||||
@@ -74,13 +75,23 @@ impl fmt::Debug for JsReplHandle {
|
||||
}
|
||||
|
||||
impl JsReplHandle {
|
||||
#[cfg(test)]
|
||||
pub(crate) fn with_node_path(
|
||||
node_path: Option<PathBuf>,
|
||||
node_module_dirs: Vec<PathBuf>,
|
||||
) -> Self {
|
||||
Self::with_runtime_config(node_path, node_module_dirs, None)
|
||||
}
|
||||
|
||||
pub(crate) fn with_runtime_config(
|
||||
node_path: Option<PathBuf>,
|
||||
node_module_dirs: Vec<PathBuf>,
|
||||
runtime_command: Option<Vec<String>>,
|
||||
) -> Self {
|
||||
Self {
|
||||
node_path,
|
||||
node_module_dirs,
|
||||
runtime_command,
|
||||
cell: OnceCell::new(),
|
||||
}
|
||||
}
|
||||
@@ -88,7 +99,12 @@ impl JsReplHandle {
|
||||
pub(crate) async fn manager(&self) -> Result<Arc<JsReplManager>, FunctionCallError> {
|
||||
self.cell
|
||||
.get_or_try_init(|| async {
|
||||
JsReplManager::new(self.node_path.clone(), self.node_module_dirs.clone()).await
|
||||
JsReplManager::new(
|
||||
self.node_path.clone(),
|
||||
self.node_module_dirs.clone(),
|
||||
self.runtime_command.clone(),
|
||||
)
|
||||
.await
|
||||
})
|
||||
.await
|
||||
.cloned()
|
||||
@@ -307,6 +323,7 @@ fn with_model_kernel_failure_message(
|
||||
pub struct JsReplManager {
|
||||
node_path: Option<PathBuf>,
|
||||
node_module_dirs: Vec<PathBuf>,
|
||||
runtime_command: Option<Vec<String>>,
|
||||
tmp_dir: tempfile::TempDir,
|
||||
kernel: Arc<Mutex<Option<KernelState>>>,
|
||||
exec_lock: Arc<tokio::sync::Semaphore>,
|
||||
@@ -317,6 +334,7 @@ impl JsReplManager {
|
||||
async fn new(
|
||||
node_path: Option<PathBuf>,
|
||||
node_module_dirs: Vec<PathBuf>,
|
||||
runtime_command: Option<Vec<String>>,
|
||||
) -> Result<Arc<Self>, FunctionCallError> {
|
||||
let tmp_dir = tempfile::tempdir().map_err(|err| {
|
||||
FunctionCallError::RespondToModel(format!("failed to create js_repl temp dir: {err}"))
|
||||
@@ -325,6 +343,7 @@ impl JsReplManager {
|
||||
let manager = Arc::new(Self {
|
||||
node_path,
|
||||
node_module_dirs,
|
||||
runtime_command,
|
||||
tmp_dir,
|
||||
kernel: Arc::new(Mutex::new(None)),
|
||||
exec_lock: Arc::new(tokio::sync::Semaphore::new(1)),
|
||||
@@ -808,12 +827,22 @@ impl JsReplManager {
|
||||
turn: Arc<TurnContext>,
|
||||
thread_id: Option<ThreadId>,
|
||||
) -> Result<KernelState, String> {
|
||||
let node_path = resolve_compatible_node(self.node_path.as_deref()).await?;
|
||||
|
||||
let kernel_path = self
|
||||
.write_kernel_script()
|
||||
.await
|
||||
.map_err(|err| err.to_string())?;
|
||||
let (program, args) = if let Some(runtime_command) = self.runtime_command.as_deref() {
|
||||
resolve_external_runtime_command(runtime_command)?
|
||||
} else {
|
||||
let node_path = resolve_compatible_node(self.node_path.as_deref()).await?;
|
||||
let kernel_path = self
|
||||
.write_kernel_script()
|
||||
.await
|
||||
.map_err(|err| err.to_string())?;
|
||||
(
|
||||
node_path.to_string_lossy().to_string(),
|
||||
vec![
|
||||
"--experimental-vm-modules".to_string(),
|
||||
kernel_path.to_string_lossy().to_string(),
|
||||
],
|
||||
)
|
||||
};
|
||||
|
||||
let mut env = create_env(&turn.shell_environment_policy, thread_id);
|
||||
env.insert(
|
||||
@@ -831,11 +860,8 @@ impl JsReplManager {
|
||||
}
|
||||
|
||||
let spec = CommandSpec {
|
||||
program: node_path.to_string_lossy().to_string(),
|
||||
args: vec![
|
||||
"--experimental-vm-modules".to_string(),
|
||||
kernel_path.to_string_lossy().to_string(),
|
||||
],
|
||||
program,
|
||||
args,
|
||||
cwd: turn.cwd.clone(),
|
||||
env,
|
||||
expiration: ExecExpiration::DefaultTimeout,
|
||||
@@ -900,7 +926,7 @@ impl JsReplManager {
|
||||
|
||||
let mut child = cmd
|
||||
.spawn()
|
||||
.map_err(|err| format!("failed to start Node runtime: {err}"))?;
|
||||
.map_err(|err| format!("failed to start js_repl runtime: {err}"))?;
|
||||
let stdout = child
|
||||
.stdout
|
||||
.take()
|
||||
@@ -1698,6 +1724,44 @@ async fn ensure_node_version(node_path: &Path) -> Result<(), String> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn resolve_external_runtime_command(command: &[String]) -> Result<(String, Vec<String>), String> {
|
||||
let Some((program, args)) = command.split_first() else {
|
||||
return Err("js_repl runtime command is empty".to_string());
|
||||
};
|
||||
if program.trim().is_empty() {
|
||||
return Err("js_repl runtime command program is empty".to_string());
|
||||
}
|
||||
|
||||
let program_path = Path::new(program);
|
||||
if program_path.is_absolute() {
|
||||
if !program_path.exists() {
|
||||
return Err(format!(
|
||||
"js_repl runtime command not found: {}",
|
||||
program_path.display()
|
||||
));
|
||||
}
|
||||
} else if which::which(program).is_err() {
|
||||
return Err(format!(
|
||||
"js_repl runtime command not found on PATH: {program}"
|
||||
));
|
||||
}
|
||||
|
||||
Ok((program.clone(), args.to_vec()))
|
||||
}
|
||||
|
||||
pub(crate) async fn validate_js_repl_runtime(
|
||||
runtime_command: Option<&[String]>,
|
||||
node_path: Option<&Path>,
|
||||
) -> Result<(), String> {
|
||||
if let Some(runtime_command) = runtime_command {
|
||||
let _ = resolve_external_runtime_command(runtime_command)?;
|
||||
Ok(())
|
||||
} else {
|
||||
let _ = resolve_compatible_node(node_path).await?;
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn resolve_compatible_node(config_path: Option<&Path>) -> Result<PathBuf, String> {
|
||||
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()
|
||||
@@ -1773,6 +1837,41 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn external_runtime_command_requires_program() {
|
||||
let err = resolve_external_runtime_command(&[]).expect_err("empty command should fail");
|
||||
assert_eq!(err, "js_repl runtime command is empty");
|
||||
|
||||
let err = resolve_external_runtime_command(&["".to_string()])
|
||||
.expect_err("blank program should fail");
|
||||
assert_eq!(err, "js_repl runtime command program is empty");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn external_runtime_command_accepts_existing_absolute_path() {
|
||||
let current_exe = std::env::current_exe().expect("resolve current executable");
|
||||
let command = vec![
|
||||
current_exe.to_string_lossy().to_string(),
|
||||
"--version".to_string(),
|
||||
];
|
||||
let (program, args) =
|
||||
resolve_external_runtime_command(&command).expect("command should validate");
|
||||
|
||||
assert_eq!(program, current_exe.to_string_lossy());
|
||||
assert_eq!(args, vec!["--version".to_string()]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn validate_js_repl_runtime_prefers_external_command_over_node_checks() {
|
||||
let current_exe = std::env::current_exe().expect("resolve current executable");
|
||||
let command = vec![current_exe.to_string_lossy().to_string()];
|
||||
let missing_node = Path::new("/definitely/missing/node");
|
||||
|
||||
validate_js_repl_runtime(Some(&command), Some(missing_node))
|
||||
.await
|
||||
.expect("external command should bypass node validation");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn truncate_utf8_prefix_by_bytes_preserves_character_boundaries() {
|
||||
let input = "aé🙂z";
|
||||
@@ -1932,7 +2031,7 @@ mod tests {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn reset_waits_for_exec_lock_before_clearing_exec_tool_calls() {
|
||||
let manager = JsReplManager::new(None, Vec::new())
|
||||
let manager = JsReplManager::new(None, Vec::new(), None)
|
||||
.await
|
||||
.expect("manager should initialize");
|
||||
let permit = manager
|
||||
@@ -2106,7 +2205,7 @@ mod tests {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn reset_clears_inflight_exec_tool_calls_without_waiting() {
|
||||
let manager = JsReplManager::new(None, Vec::new())
|
||||
let manager = JsReplManager::new(None, Vec::new(), None)
|
||||
.await
|
||||
.expect("manager should initialize");
|
||||
let exec_id = Uuid::new_v4().to_string();
|
||||
@@ -2139,7 +2238,7 @@ mod tests {
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn reset_aborts_inflight_exec_tool_tasks() {
|
||||
let manager = JsReplManager::new(None, Vec::new())
|
||||
let manager = JsReplManager::new(None, Vec::new(), None)
|
||||
.await
|
||||
.expect("manager should initialize");
|
||||
let exec_id = Uuid::new_v4().to_string();
|
||||
|
||||
@@ -311,6 +311,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
|
||||
main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(),
|
||||
js_repl_node_path: None,
|
||||
js_repl_node_module_dirs: None,
|
||||
experimental_js_repl_runtime_command: None,
|
||||
zsh_path: None,
|
||||
base_instructions: None,
|
||||
developer_instructions: None,
|
||||
|
||||
Reference in New Issue
Block a user