Remove js_repl feature (#19410)

This commit is contained in:
Curtis 'Fjord' Hawthorne
2026-04-24 17:49:29 -07:00
committed by GitHub
parent cf02e9c052
commit 8a559e7938
63 changed files with 77 additions and 9261 deletions
+1 -2
View File
@@ -3,7 +3,6 @@ mod response_adapter;
mod wait_handler;
use std::collections::HashSet;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
@@ -62,7 +61,7 @@ pub(crate) struct CodeModeService {
}
impl CodeModeService {
pub(crate) fn new(_js_repl_node_path: Option<PathBuf>) -> Self {
pub(crate) fn new() -> Self {
Self {
inner: codex_code_mode::CodeModeService::new(),
}
-1
View File
@@ -33,7 +33,6 @@ pub type SharedTurnDiffTracker = Arc<Mutex<TurnDiffTracker>>;
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum ToolCallSource {
Direct,
JsRepl,
CodeMode {
/// Runtime cell that issued the nested tool request.
cell_id: String,
-300
View File
@@ -1,300 +0,0 @@
use serde_json::Value as JsonValue;
use std::sync::Arc;
use std::time::Duration;
use std::time::Instant;
use crate::function_tool::FunctionCallError;
use crate::tools::context::FunctionToolOutput;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
use crate::tools::events::ToolEmitter;
use crate::tools::events::ToolEventCtx;
use crate::tools::events::ToolEventFailure;
use crate::tools::events::ToolEventStage;
use crate::tools::handlers::parse_arguments;
use crate::tools::js_repl::JS_REPL_PRAGMA_PREFIX;
use crate::tools::js_repl::JsReplArgs;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
use codex_features::Feature;
use codex_protocol::exec_output::ExecToolCallOutput;
use codex_protocol::exec_output::StreamOutput;
use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::protocol::ExecCommandSource;
pub struct JsReplHandler;
pub struct JsReplResetHandler;
fn join_outputs(stdout: &str, stderr: &str) -> String {
if stdout.is_empty() {
stderr.to_string()
} else if stderr.is_empty() {
stdout.to_string()
} else {
format!("{stdout}\n{stderr}")
}
}
fn build_js_repl_exec_output(
output: &str,
error: Option<&str>,
duration: Duration,
) -> ExecToolCallOutput {
let stdout = output.to_string();
let stderr = error.unwrap_or("").to_string();
let aggregated_output = join_outputs(&stdout, &stderr);
ExecToolCallOutput {
exit_code: if error.is_some() { 1 } else { 0 },
stdout: StreamOutput::new(stdout),
stderr: StreamOutput::new(stderr),
aggregated_output: StreamOutput::new(aggregated_output),
duration,
timed_out: false,
}
}
async fn emit_js_repl_exec_begin(
session: &crate::session::session::Session,
turn: &crate::session::turn_context::TurnContext,
call_id: &str,
) {
let emitter = ToolEmitter::shell(
vec!["js_repl".to_string()],
turn.cwd.clone(),
ExecCommandSource::Agent,
/*freeform*/ false,
);
let ctx = ToolEventCtx::new(session, turn, call_id, /*turn_diff_tracker*/ None);
emitter.emit(ctx, ToolEventStage::Begin).await;
}
async fn emit_js_repl_exec_end(
session: &crate::session::session::Session,
turn: &crate::session::turn_context::TurnContext,
call_id: &str,
output: &str,
error: Option<&str>,
duration: Duration,
) {
let exec_output = build_js_repl_exec_output(output, error, duration);
let emitter = ToolEmitter::shell(
vec!["js_repl".to_string()],
turn.cwd.clone(),
ExecCommandSource::Agent,
/*freeform*/ false,
);
let ctx = ToolEventCtx::new(session, turn, call_id, /*turn_diff_tracker*/ None);
let stage = if error.is_some() {
ToolEventStage::Failure(ToolEventFailure::Output(exec_output))
} else {
ToolEventStage::Success(exec_output)
};
emitter.emit(ctx, stage).await;
}
impl ToolHandler for JsReplHandler {
type Output = FunctionToolOutput;
fn kind(&self) -> ToolKind {
ToolKind::Function
}
fn matches_kind(&self, payload: &ToolPayload) -> bool {
matches!(
payload,
ToolPayload::Function { .. } | ToolPayload::Custom { .. }
)
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
turn,
cancellation_token,
tracker,
payload,
call_id,
..
} = invocation;
if !session.features().enabled(Feature::JsRepl) {
return Err(FunctionCallError::RespondToModel(
"js_repl is disabled by feature flag".to_string(),
));
}
let args = match payload {
ToolPayload::Function { arguments } => parse_arguments(&arguments)?,
ToolPayload::Custom { input } => parse_freeform_args(&input)?,
_ => {
return Err(FunctionCallError::RespondToModel(
"js_repl expects custom or function payload".to_string(),
));
}
};
let manager = turn.js_repl.manager().await?;
let started_at = Instant::now();
emit_js_repl_exec_begin(session.as_ref(), turn.as_ref(), &call_id).await;
let result = manager
.execute_with_cancellation(
Arc::clone(&session),
Arc::clone(&turn),
cancellation_token,
tracker,
args,
)
.await;
let result = match result {
Ok(result) => result,
Err(err) => {
let message = err.to_string();
emit_js_repl_exec_end(
session.as_ref(),
turn.as_ref(),
&call_id,
"",
Some(&message),
started_at.elapsed(),
)
.await;
return Err(err);
}
};
let content = result.output;
let mut items = Vec::with_capacity(result.content_items.len() + 1);
if !content.is_empty() {
items.push(FunctionCallOutputContentItem::InputText {
text: content.clone(),
});
}
items.extend(result.content_items);
emit_js_repl_exec_end(
session.as_ref(),
turn.as_ref(),
&call_id,
&content,
/*error*/ None,
started_at.elapsed(),
)
.await;
if items.is_empty() {
Ok(FunctionToolOutput::from_text(content, Some(true)))
} else {
Ok(FunctionToolOutput::from_content(items, Some(true)))
}
}
}
impl ToolHandler for JsReplResetHandler {
type Output = FunctionToolOutput;
fn kind(&self) -> ToolKind {
ToolKind::Function
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
if !invocation.session.features().enabled(Feature::JsRepl) {
return Err(FunctionCallError::RespondToModel(
"js_repl is disabled by feature flag".to_string(),
));
}
let manager = invocation.turn.js_repl.manager().await?;
manager.reset().await?;
Ok(FunctionToolOutput::from_text(
"js_repl kernel reset".to_string(),
Some(true),
))
}
}
fn parse_freeform_args(input: &str) -> Result<JsReplArgs, FunctionCallError> {
if input.trim().is_empty() {
return Err(FunctionCallError::RespondToModel(
"js_repl expects raw JavaScript tool input (non-empty). Provide JS source text, optionally with first-line `// codex-js-repl: ...`."
.to_string(),
));
}
let mut args = JsReplArgs {
code: input.to_string(),
timeout_ms: None,
};
let mut lines = input.splitn(2, '\n');
let first_line = lines.next().unwrap_or_default();
let rest = lines.next().unwrap_or_default();
let trimmed = first_line.trim_start();
let Some(pragma) = trimmed.strip_prefix(JS_REPL_PRAGMA_PREFIX) else {
reject_json_or_quoted_source(&args.code)?;
return Ok(args);
};
let mut timeout_ms: Option<u64> = None;
let directive = pragma.trim();
if !directive.is_empty() {
for token in directive.split_whitespace() {
let (key, value) = token.split_once('=').ok_or_else(|| {
FunctionCallError::RespondToModel(format!(
"js_repl pragma expects space-separated key=value pairs (supported keys: timeout_ms); got `{token}`"
))
})?;
match key {
"timeout_ms" => {
if timeout_ms.is_some() {
return Err(FunctionCallError::RespondToModel(
"js_repl pragma specifies timeout_ms more than once".to_string(),
));
}
let parsed = value.parse::<u64>().map_err(|_| {
FunctionCallError::RespondToModel(format!(
"js_repl pragma timeout_ms must be an integer; got `{value}`"
))
})?;
timeout_ms = Some(parsed);
}
_ => {
return Err(FunctionCallError::RespondToModel(format!(
"js_repl pragma only supports timeout_ms; got `{key}`"
)));
}
}
}
}
if rest.trim().is_empty() {
return Err(FunctionCallError::RespondToModel(
"js_repl pragma must be followed by JavaScript source on subsequent lines".to_string(),
));
}
reject_json_or_quoted_source(rest)?;
args.code = rest.to_string();
args.timeout_ms = timeout_ms;
Ok(args)
}
fn reject_json_or_quoted_source(code: &str) -> Result<(), FunctionCallError> {
let trimmed = code.trim();
if trimmed.starts_with("```") {
return Err(FunctionCallError::RespondToModel(
"js_repl expects raw JavaScript source, not markdown code fences. Resend plain JS only (optional first line `// codex-js-repl: ...`)."
.to_string(),
));
}
let Ok(value) = serde_json::from_str::<JsonValue>(trimmed) else {
return Ok(());
};
match value {
JsonValue::Object(_) | JsonValue::String(_) => Err(FunctionCallError::RespondToModel(
"js_repl is a freeform tool and expects raw JavaScript source. Resend plain JS only (optional first line `// codex-js-repl: ...`); do not send JSON (`{\"code\":...}`), quoted code, or markdown fences."
.to_string(),
)),
_ => Ok(()),
}
}
#[cfg(test)]
#[path = "js_repl_tests.rs"]
mod tests;
@@ -1,90 +0,0 @@
use std::time::Duration;
use super::parse_freeform_args;
use crate::session::tests::make_session_and_context_with_rx;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecCommandSource;
use pretty_assertions::assert_eq;
#[test]
fn parse_freeform_args_without_pragma() {
let args = parse_freeform_args("console.log('ok');").expect("parse args");
assert_eq!(args.code, "console.log('ok');");
assert_eq!(args.timeout_ms, None);
}
#[test]
fn parse_freeform_args_with_pragma() {
let input = "// codex-js-repl: timeout_ms=15000\nconsole.log('ok');";
let args = parse_freeform_args(input).expect("parse args");
assert_eq!(args.code, "console.log('ok');");
assert_eq!(args.timeout_ms, Some(15_000));
}
#[test]
fn parse_freeform_args_rejects_unknown_key() {
let err = parse_freeform_args("// codex-js-repl: nope=1\nconsole.log('ok');")
.expect_err("expected error");
assert_eq!(
err.to_string(),
"js_repl pragma only supports timeout_ms; got `nope`"
);
}
#[test]
fn parse_freeform_args_rejects_reset_key() {
let err = parse_freeform_args("// codex-js-repl: reset=true\nconsole.log('ok');")
.expect_err("expected error");
assert_eq!(
err.to_string(),
"js_repl pragma only supports timeout_ms; got `reset`"
);
}
#[test]
fn parse_freeform_args_rejects_json_wrapped_code() {
let err = parse_freeform_args(r#"{"code":"await doThing()"}"#).expect_err("expected error");
assert_eq!(
err.to_string(),
"js_repl is a freeform tool and expects raw JavaScript source. Resend plain JS only (optional first line `// codex-js-repl: ...`); do not send JSON (`{\"code\":...}`), quoted code, or markdown fences."
);
}
#[tokio::test]
async fn emit_js_repl_exec_end_sends_event() {
let (session, turn, rx) = make_session_and_context_with_rx().await;
super::emit_js_repl_exec_end(
session.as_ref(),
turn.as_ref(),
"call-1",
"hello",
/*error*/ None,
Duration::from_millis(12),
)
.await;
let event = tokio::time::timeout(Duration::from_secs(5), async {
loop {
let event = rx.recv().await.expect("event");
if let EventMsg::ExecCommandEnd(end) = event.msg {
break end;
}
}
})
.await
.expect("timed out waiting for exec end");
assert_eq!(event.call_id, "call-1");
assert_eq!(event.turn_id, turn.sub_id);
assert_eq!(event.command, vec!["js_repl".to_string()]);
assert_eq!(event.cwd, turn.cwd);
assert_eq!(event.source, ExecCommandSource::Agent);
assert_eq!(event.interaction_input, None);
assert_eq!(event.stdout, "hello");
assert_eq!(event.stderr, "");
assert!(event.aggregated_output.contains("hello"));
assert_eq!(event.exit_code, 0);
assert_eq!(event.duration, Duration::from_millis(12));
assert!(event.formatted_output.contains("hello"));
assert!(!event.parsed_cmd.is_empty());
}
-3
View File
@@ -1,7 +1,6 @@
pub(crate) mod agent_jobs;
pub(crate) mod apply_patch;
mod dynamic;
mod js_repl;
mod list_dir;
mod mcp;
mod mcp_resource;
@@ -37,8 +36,6 @@ pub use apply_patch::ApplyPatchHandler;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::protocol::AskForApproval;
pub use dynamic::DynamicToolHandler;
pub use js_repl::JsReplHandler;
pub use js_repl::JsReplResetHandler;
pub use list_dir::ListDirHandler;
pub use mcp::McpHandler;
pub use mcp_resource::McpResourceHandler;
File diff suppressed because it is too large Load Diff
File diff suppressed because one or more lines are too long
File diff suppressed because it is too large Load Diff
File diff suppressed because it is too large Load Diff
-1
View File
@@ -3,7 +3,6 @@ pub(crate) mod context;
pub(crate) mod events;
pub(crate) mod handlers;
pub(crate) mod hook_names;
pub(crate) mod js_repl;
pub(crate) mod network_approval;
pub(crate) mod orchestrator;
pub(crate) mod parallel;
-12
View File
@@ -279,18 +279,6 @@ impl ToolRouter {
payload,
} = call;
let direct_js_repl_call = tool_name.namespace.is_none()
&& matches!(tool_name.name.as_str(), "js_repl" | "js_repl_reset");
if matches!(&source, ToolCallSource::Direct)
&& turn.tools_config.js_repl_tools_only
&& !direct_js_repl_call
{
return Err(FunctionCallError::RespondToModel(
"direct tool calls are disabled; use js_repl and codex.tool(...) instead"
.to_string(),
));
}
let invocation = ToolInvocation {
session,
turn,
-172
View File
@@ -1,187 +1,15 @@
use std::collections::HashSet;
use std::sync::Arc;
use crate::function_tool::FunctionCallError;
use crate::session::tests::make_session_and_context;
use crate::tools::context::ToolPayload;
use crate::turn_diff_tracker::TurnDiffTracker;
use codex_protocol::models::ResponseItem;
use codex_tools::ToolName;
use tokio_util::sync::CancellationToken;
use super::ToolCall;
use super::ToolCallSource;
use super::ToolRouter;
use super::ToolRouterParams;
#[tokio::test]
#[expect(
clippy::await_holding_invalid_type,
reason = "test builds a router from session-owned MCP manager state"
)]
async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> {
let (session, mut turn) = make_session_and_context().await;
turn.tools_config.js_repl_tools_only = true;
let session = Arc::new(session);
let turn = Arc::new(turn);
let mcp_tools = session
.services
.mcp_connection_manager
.read()
.await
.list_all_tools()
.await;
let deferred_mcp_tools = Some(mcp_tools.clone());
let router = ToolRouter::from_config(
&turn.tools_config,
ToolRouterParams {
deferred_mcp_tools,
mcp_tools: Some(mcp_tools),
unavailable_called_tools: Vec::new(),
parallel_mcp_server_names: HashSet::new(),
discoverable_tools: None,
dynamic_tools: turn.dynamic_tools.as_slice(),
},
);
let call = ToolCall {
tool_name: ToolName::plain("shell"),
call_id: "call-1".to_string(),
payload: ToolPayload::Function {
arguments: "{}".to_string(),
},
};
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
let err = router
.dispatch_tool_call_with_code_mode_result(
session,
turn,
CancellationToken::new(),
tracker,
call,
ToolCallSource::Direct,
)
.await
.err()
.expect("direct tool calls should be blocked");
let FunctionCallError::RespondToModel(message) = err else {
panic!("expected RespondToModel, got {err:?}");
};
assert!(message.contains("direct tool calls are disabled"));
Ok(())
}
#[tokio::test]
#[expect(
clippy::await_holding_invalid_type,
reason = "test builds a router from session-owned MCP manager state"
)]
async fn js_repl_tools_only_allows_js_repl_source_calls() -> anyhow::Result<()> {
let (session, mut turn) = make_session_and_context().await;
turn.tools_config.js_repl_tools_only = true;
let session = Arc::new(session);
let turn = Arc::new(turn);
let mcp_tools = session
.services
.mcp_connection_manager
.read()
.await
.list_all_tools()
.await;
let deferred_mcp_tools = Some(mcp_tools.clone());
let router = ToolRouter::from_config(
&turn.tools_config,
ToolRouterParams {
deferred_mcp_tools,
mcp_tools: Some(mcp_tools),
unavailable_called_tools: Vec::new(),
parallel_mcp_server_names: HashSet::new(),
discoverable_tools: None,
dynamic_tools: turn.dynamic_tools.as_slice(),
},
);
let call = ToolCall {
tool_name: ToolName::plain("shell"),
call_id: "call-2".to_string(),
payload: ToolPayload::Function {
arguments: "{}".to_string(),
},
};
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
let err = router
.dispatch_tool_call_with_code_mode_result(
session,
turn,
CancellationToken::new(),
tracker,
call,
ToolCallSource::JsRepl,
)
.await
.err()
.expect("shell call with empty args should fail");
let message = err.to_string();
assert!(
!message.contains("direct tool calls are disabled"),
"js_repl source should bypass direct-call policy gate"
);
Ok(())
}
#[tokio::test]
async fn js_repl_tools_only_blocks_namespaced_js_repl_tool() -> anyhow::Result<()> {
let (session, mut turn) = make_session_and_context().await;
turn.tools_config.js_repl_tools_only = true;
let session = Arc::new(session);
let turn = Arc::new(turn);
let router = ToolRouter::from_config(
&turn.tools_config,
ToolRouterParams {
deferred_mcp_tools: None,
mcp_tools: None,
unavailable_called_tools: Vec::new(),
parallel_mcp_server_names: HashSet::new(),
discoverable_tools: None,
dynamic_tools: turn.dynamic_tools.as_slice(),
},
);
let call = ToolCall {
tool_name: ToolName::namespaced("mcp__server__", "js_repl"),
call_id: "call-namespaced-js-repl".to_string(),
payload: ToolPayload::Mcp {
server: "server".to_string(),
tool: "js_repl".to_string(),
raw_arguments: "{}".to_string(),
},
};
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
let err = router
.dispatch_tool_call_with_code_mode_result(
session,
turn,
CancellationToken::new(),
tracker,
call,
ToolCallSource::Direct,
)
.await
.err()
.expect("namespaced js_repl calls should be blocked");
let FunctionCallError::RespondToModel(message) = err else {
panic!("expected RespondToModel, got {err:?}");
};
assert!(message.contains("direct tool calls are disabled"));
Ok(())
}
#[tokio::test]
#[expect(
clippy::await_holding_invalid_type,
-10
View File
@@ -80,8 +80,6 @@ pub(crate) fn build_specs_with_discoverable_tools(
use crate::tools::handlers::CodeModeExecuteHandler;
use crate::tools::handlers::CodeModeWaitHandler;
use crate::tools::handlers::DynamicToolHandler;
use crate::tools::handlers::JsReplHandler;
use crate::tools::handlers::JsReplResetHandler;
use crate::tools::handlers::ListDirHandler;
use crate::tools::handlers::McpHandler;
use crate::tools::handlers::McpResourceHandler;
@@ -167,8 +165,6 @@ pub(crate) fn build_specs_with_discoverable_tools(
let tool_suggest_handler = Arc::new(ToolSuggestHandler);
let code_mode_handler = Arc::new(CodeModeExecuteHandler);
let code_mode_wait_handler = Arc::new(CodeModeWaitHandler);
let js_repl_handler = Arc::new(JsReplHandler);
let js_repl_reset_handler = Arc::new(JsReplResetHandler);
let unavailable_tool_handler = Arc::new(UnavailableToolHandler);
let mut existing_spec_names = plan
.specs
@@ -212,12 +208,6 @@ pub(crate) fn build_specs_with_discoverable_tools(
ToolHandlerKind::FollowupTaskV2 => {
builder.register_handler(handler.name, Arc::new(FollowupTaskHandlerV2));
}
ToolHandlerKind::JsRepl => {
builder.register_handler(handler.name, js_repl_handler.clone());
}
ToolHandlerKind::JsReplReset => {
builder.register_handler(handler.name, js_repl_reset_handler.clone());
}
ToolHandlerKind::ListAgentsV2 => {
builder.register_handler(handler.name, Arc::new(ListAgentsHandlerV2));
}
@@ -71,7 +71,6 @@ fn tool_dispatch_invocation(invocation: &ToolInvocation) -> Option<ToolDispatchI
runtime_cell_id: cell_id.clone(),
runtime_tool_call_id: runtime_tool_call_id.clone(),
},
ToolCallSource::JsRepl => return None,
};
Some(ToolDispatchInvocation {
@@ -98,7 +97,6 @@ fn tool_dispatch_result(
ToolCallSource::CodeMode { .. } => Some(ToolDispatchResult::CodeModeResponse {
value: result.code_mode_result(payload),
}),
ToolCallSource::JsRepl => None,
}
}
@@ -129,11 +129,6 @@ async fn dispatch_lifecycle_trace_records_direct_and_code_mode_requesters() -> a
Ok(())
}
#[tokio::test]
async fn dispatch_lifecycle_trace_skips_noncanonical_boundaries() -> anyhow::Result<()> {
assert_dispatch_trace_skips(ToolCallSource::JsRepl).await
}
#[tokio::test]
async fn dispatch_lifecycle_trace_records_unsupported_tool_failures() -> anyhow::Result<()> {
let temp = TempDir::new()?;
@@ -234,35 +229,6 @@ async fn missing_code_mode_wait_traces_only_the_wait_tool_call() -> anyhow::Resu
Ok(())
}
async fn assert_dispatch_trace_skips(source: ToolCallSource) -> anyhow::Result<()> {
let temp = TempDir::new()?;
let (mut session, turn) = make_session_and_context().await;
attach_test_trace(&mut session, &turn, temp.path())?;
let registry = ToolRegistry::with_handler_for_test(
codex_tools::ToolName::plain("test_tool"),
Arc::new(TestHandler),
);
let session = Arc::new(session);
let turn = Arc::new(turn);
registry
.dispatch_any(test_invocation(
session,
turn,
"skipped-call",
"test_tool",
source,
"{}",
))
.await?;
let replayed = codex_rollout_trace::replay_bundle(single_bundle_dir(temp.path())?)?;
assert_eq!(replayed.tool_calls, Default::default());
Ok(())
}
fn test_invocation(
session: Arc<Session>,
turn: Arc<TurnContext>,