codex: address PR review feedback (#14225)

This commit is contained in:
pakrym-oai
2026-03-11 10:01:28 -07:00
parent 9d3d72b821
commit 0fd60a9b6b
5 changed files with 109 additions and 47 deletions
+15 -15
View File
@@ -1,19 +1,19 @@
load("@rules_cc//cc:defs.bzl", "cc_library")
load("//:defs.bzl", "codex_rust_crate")
load("@rules_rust//rust:defs.bzl", "rust_library")
codex_rust_crate(
rust_library(
name = "code-mode",
crate_name = "codex_code_mode",
compile_data = ["src/code_mode_bridge.js"],
deps_extra = select({
"@platforms//os:linux": [":libc_stack_end_shim"],
"//conditions:default": [],
}),
)
cc_library(
name = "libc_stack_end_shim",
srcs = ["src/libc_stack_end_shim.cc"],
target_compatible_with = ["@platforms//os:linux"],
visibility = ["//visibility:private"],
crate_root = "src/bazel_stub.rs",
srcs = [
"src/api.rs",
"src/bazel_stub.rs",
],
deps = [
"@crates//:serde",
"@crates//:serde_json",
],
rustc_env = {
"BAZEL_PACKAGE": "codex-rs/code-mode",
},
visibility = ["//visibility:public"],
)
+31
View File
@@ -0,0 +1,31 @@
use std::collections::HashMap;
use serde::Serialize;
use serde_json::Value as JsonValue;
pub type ToolCallHandler =
dyn FnMut(String, Option<JsonValue>) -> Result<JsonValue, String> + Send + 'static;
#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize)]
#[serde(rename_all = "snake_case")]
pub enum ToolKind {
Function,
Freeform,
}
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub struct EnabledTool {
pub tool_name: String,
pub namespace: Vec<String>,
pub name: String,
pub kind: ToolKind,
}
#[derive(Debug)]
pub struct ExecutionResult {
pub content_items: Vec<JsonValue>,
pub stored_values: HashMap<String, JsonValue>,
pub max_output_tokens_per_exec_call: usize,
pub success: bool,
pub error_text: Option<String>,
}
+37
View File
@@ -0,0 +1,37 @@
use std::collections::HashMap;
use serde_json::Value as JsonValue;
mod api;
pub use api::EnabledTool;
pub use api::ExecutionResult;
pub use api::ToolCallHandler;
pub use api::ToolKind;
const BAZEL_BUILD: bool = option_env!("BAZEL_PACKAGE").is_some();
const BAZEL_UNSUPPORTED_REASON: &str = "code_mode is unavailable in Bazel builds";
pub const fn is_supported() -> bool {
!BAZEL_BUILD
}
pub fn unsupported_reason() -> Option<&'static str> {
if BAZEL_BUILD {
Some(BAZEL_UNSUPPORTED_REASON)
} else {
None
}
}
pub fn execute(
code: String,
enabled_tools: Vec<EnabledTool>,
stored_values: HashMap<String, JsonValue>,
on_tool_call: Box<ToolCallHandler>,
) -> Result<ExecutionResult, String> {
let _ = code;
let _ = enabled_tools;
let _ = stored_values;
let _ = on_tool_call;
Err(BAZEL_UNSUPPORTED_REASON.to_string())
}
+6 -32
View File
@@ -1,34 +1,8 @@
use std::collections::HashMap;
use serde::Serialize;
use serde_json::Value as JsonValue;
pub type ToolCallHandler =
dyn FnMut(String, Option<JsonValue>) -> Result<JsonValue, String> + Send + 'static;
#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize)]
#[serde(rename_all = "snake_case")]
pub enum ToolKind {
Function,
Freeform,
}
#[derive(Clone, Debug, Eq, PartialEq, Serialize)]
pub struct EnabledTool {
pub tool_name: String,
pub namespace: Vec<String>,
pub name: String,
pub kind: ToolKind,
}
#[derive(Debug)]
pub struct ExecutionResult {
pub content_items: Vec<JsonValue>,
pub stored_values: HashMap<String, JsonValue>,
pub max_output_tokens_per_exec_call: usize,
pub success: bool,
pub error_text: Option<String>,
}
mod api;
pub use api::EnabledTool;
pub use api::ExecutionResult;
pub use api::ToolCallHandler;
pub use api::ToolKind;
const MUSL_UNSUPPORTED_REASON: &str = "code_mode is unavailable on musl Linux";
@@ -54,7 +28,7 @@ pub use imp::execute;
pub fn execute(
code: String,
enabled_tools: Vec<EnabledTool>,
stored_values: HashMap<String, JsonValue>,
stored_values: std::collections::HashMap<String, serde_json::Value>,
on_tool_call: Box<ToolCallHandler>,
) -> Result<ExecutionResult, String> {
let _ = code;
+20
View File
@@ -24,6 +24,14 @@ use std::fs;
use std::time::Duration;
use wiremock::MockServer;
macro_rules! skip_on_bazel {
($result:expr) => {
if option_env!("BAZEL_PACKAGE").is_some() {
return $result;
}
};
}
fn custom_tool_output_items(req: &ResponsesRequest, call_id: &str) -> Vec<Value> {
req.custom_tool_call_output(call_id)
.get("output")
@@ -157,6 +165,7 @@ async fn run_code_mode_turn_with_rmcp(
#[cfg_attr(windows, ignore = "no exec_command on Windows")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_return_exec_command_output() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -203,6 +212,7 @@ add_content(JSON.stringify(await exec_command({ cmd: "printf code_mode_exec_mark
#[cfg_attr(windows, ignore = "no exec_command on Windows")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_truncate_final_result_with_configured_budget() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -248,6 +258,7 @@ Total\ output\ lines:\ 1\n
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_returns_accumulated_output_when_script_fails() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -291,6 +302,7 @@ Error:\ boom\n
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_output_serialized_text_via_openai_code_mode_module() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -320,6 +332,7 @@ output_text({ json: true });
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_surfaces_output_text_stringify_errors() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -363,6 +376,7 @@ output_text(circular);
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_output_images_via_openai_code_mode_module() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -415,6 +429,7 @@ output_image("data:image/png;base64,AAA");
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_apply_patch_via_nested_tool() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -456,6 +471,7 @@ async fn code_mode_can_apply_patch_via_nested_tool() -> Result<()> {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_print_structured_mcp_tool_result_fields() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -496,6 +512,7 @@ contentLength=0"
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_access_namespaced_mcp_tool_from_flat_tools_namespace() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -534,6 +551,7 @@ isError=false"
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_print_content_only_mcp_tool_result_fields() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -579,6 +597,7 @@ isError=false"
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_print_error_mcp_tool_result_fields() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
@@ -619,6 +638,7 @@ structuredContent=null"
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_can_store_and_load_values_across_turns() -> Result<()> {
skip_on_bazel!(Ok(()));
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;