Merge remote-tracking branch 'origin/main' into codex/pr23585-resolve

# Conflicts:
#	codex-rs/core/src/session/turn.rs
This commit is contained in:
Roy Han
2026-05-24 20:56:38 -07:00
68 changed files with 4598 additions and 739 deletions
+5
View File
@@ -7,6 +7,11 @@
"format": "tar.gz",
"path": "codex-zsh/bin/zsh"
},
"macos-x86_64": {
"name": "codex-zsh-x86_64-apple-darwin.tar.gz",
"format": "tar.gz",
"path": "codex-zsh/bin/zsh"
},
"linux-x86_64": {
"name": "codex-zsh-x86_64-unknown-linux-musl.tar.gz",
"format": "tar.gz",
+4
View File
@@ -69,6 +69,10 @@ jobs:
fail-fast: false
matrix:
include:
- runner: macos-15-large
target: x86_64-apple-darwin
variant: macos-15
archive_name: codex-zsh-x86_64-apple-darwin.tar.gz
- runner: macos-15-xlarge
target: aarch64-apple-darwin
variant: macos-15
+1
View File
@@ -30,6 +30,7 @@ In the codex-rs folder where the rust code lives:
- Prefer private modules and explicitly exported public crate API.
- If you change `ConfigToml` or nested config types, run `just write-config-schema` to update `codex-rs/core/config.schema.json`.
- When working with MCP tool calls, prefer using `codex-rs/codex-mcp/src/mcp_connection_manager.rs` to handle mutation of tools and tool calls. Aim to minimize the footprint of changes and leverage existing abstractions rather than plumbing code through multiple levels of function calls.
- Do not call `reset_client_session` unnecessarily; let the incremental check logic decide whether to reuse the previous request.
- If you change Rust dependencies (`Cargo.toml` or `Cargo.lock`), run `just bazel-lock-update` from the
repo root to refresh `MODULE.bazel.lock`, and include that lockfile update in the same change.
- After dependency changes, run `just bazel-lock-check` from the repo root so lockfile drift is caught
+9
View File
@@ -2236,6 +2236,7 @@ dependencies = [
"codex-exec-server",
"codex-execpolicy",
"codex-features",
"codex-git-utils",
"codex-install-context",
"codex-login",
"codex-mcp",
@@ -2260,7 +2261,9 @@ dependencies = [
"codex-windows-sandbox",
"crossterm",
"http 1.4.0",
"insta",
"libc",
"os_info",
"owo-colors",
"predicates",
"pretty_assertions",
@@ -2269,12 +2272,16 @@ dependencies = [
"serde_json",
"sqlx",
"supports-color 3.0.2",
"sys-locale",
"tempfile",
"tokio",
"toml 0.9.11+spec-1.1.0",
"tracing",
"tracing-appender",
"tracing-subscriber",
"unicode-segmentation",
"which 8.0.0",
"windows-sys 0.52.0",
]
[[package]]
@@ -2507,6 +2514,7 @@ dependencies = [
"codex-feedback",
"codex-git-utils",
"codex-hooks",
"codex-install-context",
"codex-login",
"codex-mcp",
"codex-memories-read",
@@ -3770,6 +3778,7 @@ dependencies = [
"codex-features",
"codex-protocol",
"codex-utils-absolute-path",
"codex-utils-cargo-bin",
"codex-utils-output-truncation",
"codex-utils-pty",
"codex-utils-string",
@@ -64,6 +64,8 @@ const REMOTE_CONTROL_WEBSOCKET_PONG_TIMEOUT: std::time::Duration =
std::time::Duration::from_secs(60);
const REMOTE_CONTROL_ACCOUNT_ID_RETRY_INTERVAL: std::time::Duration =
std::time::Duration::from_secs(1);
const REMOTE_CONTROL_RECONNECT_BACKOFF_CAP: std::time::Duration =
std::time::Duration::from_secs(30);
struct BoundedOutboundBuffer {
buffer_by_stream: HashMap<(ClientId, StreamId), VecDeque<ServerEnvelope>>,
@@ -514,12 +516,23 @@ impl RemoteControlWebsocket {
} else {
self.status_publisher
.publish_status(RemoteControlConnectionStatus::Errored);
let reconnect_attempt = self.reconnect_attempt.saturating_add(1);
let (reconnect_delay, reconnect_backoff_reset) =
next_reconnect_delay(&mut self.reconnect_attempt);
warn!(
"failed to connect to app-server remote control websocket: {}, err: {}",
remote_control_target.websocket_url, err
websocket_url = %remote_control_target.websocket_url,
error = %err,
reconnect_attempt,
reconnect_delay = ?reconnect_delay,
reconnect_backoff_reset,
"failed to connect to app-server remote control websocket"
);
let reconnect_delay = backoff(self.reconnect_attempt);
self.reconnect_attempt += 1;
if reconnect_backoff_reset {
info!(
reconnect_backoff_cap = ?REMOTE_CONTROL_RECONNECT_BACKOFF_CAP,
"reset app-server remote control websocket reconnect backoff after cap"
);
}
reconnect_delay
};
tokio::select! {
@@ -1036,6 +1049,17 @@ pub(crate) async fn load_remote_control_auth(
})
}
fn next_reconnect_delay(reconnect_attempt: &mut u64) -> (std::time::Duration, bool) {
let reconnect_delay = backoff(*reconnect_attempt).min(REMOTE_CONTROL_RECONNECT_BACKOFF_CAP);
let reconnect_backoff_reset = reconnect_delay == REMOTE_CONTROL_RECONNECT_BACKOFF_CAP;
*reconnect_attempt = if reconnect_backoff_reset {
0
} else {
(*reconnect_attempt).saturating_add(1)
};
(reconnect_delay, reconnect_backoff_reset)
}
pub(super) async fn connect_remote_control_websocket(
remote_control_target: &RemoteControlTarget,
state_db: Option<&StateRuntime>,
@@ -1326,6 +1350,26 @@ mod tests {
const TEST_HTTP_ACCEPT_TIMEOUT: Duration = Duration::from_secs(5);
const TEST_INSTALLATION_ID: &str = "11111111-1111-4111-8111-111111111111";
#[test]
fn next_reconnect_delay_resets_after_cap() {
let mut reconnect_attempt = 9;
let (reconnect_delay, reconnect_backoff_reset) =
next_reconnect_delay(&mut reconnect_attempt);
assert_eq!(reconnect_delay, REMOTE_CONTROL_RECONNECT_BACKOFF_CAP);
assert!(reconnect_backoff_reset);
assert_eq!(reconnect_attempt, 0);
let (reconnect_delay, reconnect_backoff_reset) =
next_reconnect_delay(&mut reconnect_attempt);
assert!(reconnect_delay >= Duration::from_millis(180));
assert!(reconnect_delay <= Duration::from_millis(220));
assert!(!reconnect_backoff_reset);
assert_eq!(reconnect_attempt, 1);
}
fn remote_control_status_channel() -> (
RemoteControlStatusPublisher,
watch::Receiver<RemoteControlStatusChangedNotification>,
@@ -173,6 +173,20 @@ impl McpProcess {
.await
}
pub async fn new_with_program_and_env(
codex_home: &Path,
program: &Path,
env_overrides: &[(&str, Option<&str>)],
) -> anyhow::Result<Self> {
Self::new_with_program_env_and_args(
codex_home,
program,
env_overrides,
&[DISABLE_PLUGIN_STARTUP_TASKS_ARG],
)
.await
}
async fn new_with_env_and_args(
codex_home: &Path,
env_overrides: &[(&str, Option<&str>)],
@@ -180,6 +194,15 @@ impl McpProcess {
) -> anyhow::Result<Self> {
let program = codex_utils_cargo_bin::cargo_bin("codex-app-server")
.context("should find binary for codex-app-server")?;
Self::new_with_program_env_and_args(codex_home, &program, env_overrides, args).await
}
async fn new_with_program_env_and_args(
codex_home: &Path,
program: &Path,
env_overrides: &[(&str, Option<&str>)],
args: &[&str],
) -> anyhow::Result<Self> {
let mut cmd = Command::new(program);
cmd.stdin(Stdio::piped());
@@ -37,6 +37,7 @@ use core_test_support::skip_if_no_network;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
use std::path::Path;
use std::path::PathBuf;
use tempfile::TempDir;
use tokio::time::timeout;
@@ -94,10 +95,9 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> {
(Feature::UnifiedExec, false),
(Feature::ShellSnapshot, false),
]),
&zsh_path,
)?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace, &zsh_path).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let start_id = mcp
@@ -162,7 +162,7 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> {
};
assert_eq!(id, "call-zsh-fork");
assert_eq!(status, CommandExecutionStatus::InProgress);
assert!(command.starts_with(&zsh_path.display().to_string()));
assert!(command.starts_with(&command_packaged_zsh_path(&codex_home).display().to_string()));
assert!(command.contains("/bin/sh -c"));
assert!(command.contains("sleep 0.01"));
assert!(command.contains(&release_marker.display().to_string()));
@@ -213,10 +213,9 @@ async fn turn_start_shell_zsh_fork_exec_approval_decline_v2() -> Result<()> {
(Feature::UnifiedExec, false),
(Feature::ShellSnapshot, false),
]),
&zsh_path,
)?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace, &zsh_path).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let start_id = mcp
@@ -346,10 +345,9 @@ async fn turn_start_shell_zsh_fork_exec_approval_cancel_v2() -> Result<()> {
(Feature::UnifiedExec, false),
(Feature::ShellSnapshot, false),
]),
&zsh_path,
)?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace, &zsh_path).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let start_id = mcp
@@ -505,10 +503,9 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2()
(Feature::UnifiedExec, false),
(Feature::ShellSnapshot, false),
]),
&zsh_path,
)?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace).await?;
let mut mcp = create_zsh_test_mcp_process(&codex_home, &workspace, &zsh_path).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let start_id = mcp
@@ -603,7 +600,8 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2()
);
approved_subcommand_strings.push(approval_command.to_string());
}
let is_parent_approval = approval_command.contains(&zsh_path.display().to_string())
let is_parent_approval = approval_command
.contains(&command_packaged_zsh_path(&codex_home).display().to_string())
&& (approval_command.contains(&shell_command)
|| (has_first_file && has_second_file)
|| approval_command.contains(&parent_shell_hint));
@@ -738,9 +736,58 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2()
Ok(())
}
async fn create_zsh_test_mcp_process(codex_home: &Path, zdotdir: &Path) -> Result<McpProcess> {
async fn create_zsh_test_mcp_process(
codex_home: &Path,
zdotdir: &Path,
zsh_path: &Path,
) -> Result<McpProcess> {
let app_server = create_test_package_app_server(codex_home, zsh_path)?;
let zdotdir = zdotdir.to_string_lossy().into_owned();
McpProcess::new_with_env(codex_home, &[("ZDOTDIR", Some(zdotdir.as_str()))]).await
McpProcess::new_with_program_and_env(
codex_home,
&app_server,
&[("ZDOTDIR", Some(zdotdir.as_str()))],
)
.await
}
fn create_test_package_app_server(codex_home: &Path, zsh_path: &Path) -> Result<PathBuf> {
let package_dir = codex_home.join("test-package");
let bin_dir = package_dir.join("bin");
let package_zsh_path = packaged_zsh_path(codex_home);
let Some(zsh_bin_dir) = package_zsh_path.parent() else {
anyhow::bail!("packaged zsh path should have parent");
};
std::fs::create_dir_all(&bin_dir)?;
std::fs::create_dir_all(zsh_bin_dir)?;
std::fs::write(package_dir.join("codex-package.json"), "{}")?;
let app_server = bin_dir.join("codex-app-server");
copy_with_permissions(
&codex_utils_cargo_bin::cargo_bin("codex-app-server")?,
&app_server,
)?;
copy_with_permissions(zsh_path, &package_zsh_path)?;
Ok(app_server)
}
fn packaged_zsh_path(codex_home: &Path) -> PathBuf {
codex_home
.join("test-package")
.join("codex-resources")
.join("zsh")
.join("bin")
.join("zsh")
}
fn command_packaged_zsh_path(codex_home: &Path) -> PathBuf {
let path = packaged_zsh_path(codex_home);
std::fs::canonicalize(&path).unwrap_or(path)
}
fn copy_with_permissions(source: &Path, destination: &Path) -> std::io::Result<()> {
std::fs::copy(source, destination)?;
std::fs::set_permissions(destination, std::fs::metadata(source)?.permissions())
}
fn create_config_toml(
@@ -748,7 +795,6 @@ fn create_config_toml(
server_uri: &str,
approval_policy: &str,
feature_flags: &BTreeMap<Feature, bool>,
zsh_path: &Path,
) -> std::io::Result<()> {
let mut features = BTreeMap::from([(Feature::RemoteModels, false)]);
for (feature, enabled) in feature_flags {
@@ -774,7 +820,6 @@ fn create_config_toml(
model = "mock-model"
approval_policy = "{approval_policy}"
sandbox_mode = "read-only"
zsh_path = "{zsh_path}"
model_provider = "mock_provider"
@@ -787,9 +832,7 @@ base_url = "{server_uri}/v1"
wire_api = "responses"
request_max_retries = 0
stream_max_retries = 0
"#,
approval_policy = approval_policy,
zsh_path = zsh_path.display()
"#
),
)
}
+10
View File
@@ -37,6 +37,7 @@ codex-exec = { workspace = true }
codex-exec-server = { workspace = true }
codex-execpolicy = { workspace = true }
codex-features = { workspace = true }
codex-git-utils = { workspace = true }
codex-install-context = { workspace = true }
codex-login = { workspace = true }
codex-memories-write = { workspace = true }
@@ -59,11 +60,13 @@ codex-utils-path = { workspace = true }
crossterm = { workspace = true }
http = { workspace = true }
libc = { workspace = true }
os_info = { workspace = true }
owo-colors = { workspace = true }
regex-lite = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
supports-color = { workspace = true }
sys-locale = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = [
"io-std",
@@ -78,14 +81,21 @@ toml = { workspace = true }
tracing = { workspace = true }
tracing-appender = { workspace = true }
tracing-subscriber = { workspace = true }
unicode-segmentation = { workspace = true }
which = { workspace = true }
[target.'cfg(target_os = "windows")'.dependencies]
codex_windows_sandbox = { package = "codex-windows-sandbox", path = "../windows-sandbox-rs" }
windows-sys = { version = "0.52", features = [
"Win32_Foundation",
"Win32_System_Console",
] }
[dev-dependencies]
assert_cmd = { workspace = true }
assert_matches = { workspace = true }
codex-utils-cargo-bin = { workspace = true }
insta = { workspace = true }
predicates = { workspace = true }
pretty_assertions = { workspace = true }
sqlx = { workspace = true }
+140 -1
View File
@@ -66,12 +66,16 @@ use serde::Serialize;
use supports_color::Stream;
mod background;
mod git;
mod output;
mod progress;
mod runtime;
mod system;
mod title;
mod updates;
use background::background_server_check;
use git::git_check;
use output::HumanOutputOptions;
use output::redact_detail;
use output::render_human_report;
@@ -79,6 +83,8 @@ use progress::DoctorProgress;
use progress::doctor_progress;
use runtime::runtime_check;
use runtime::search_check;
use system::system_check;
use title::terminal_title_check;
use updates::updates_check;
const OPENAI_BETA_HEADER: &str = "OpenAI-Beta";
@@ -330,6 +336,7 @@ async fn build_report(
) -> DoctorReport {
let progress = doctor_progress(command.json);
let mut checks = Vec::new();
checks.push(run_sync_check("system", progress.clone(), system_check));
checks.push(run_sync_check("installation", progress.clone(), || {
installation_check(!command.summary)
}));
@@ -352,6 +359,8 @@ async fn build_report(
mcp_check,
sandbox_check,
terminal_check,
git_check,
terminal_title_check,
state_check,
background_server_check,
reachability_check,
@@ -376,6 +385,12 @@ async fn build_report(
terminal_check(command.no_color)
})
},
run_async_check("git", progress.clone(), git_check(config.cwd.as_path())),
async {
run_sync_check("terminal title", progress.clone(), || {
terminal_title_check(config)
})
},
run_async_check("state", progress.clone(), state_check(config)),
async {
run_sync_check("app-server", progress.clone(), || {
@@ -397,6 +412,8 @@ async fn build_report(
mcp_check,
sandbox_check,
terminal_check,
git_check,
terminal_title_check,
state_check,
background_server_check,
reachability_check,
@@ -404,7 +421,18 @@ async fn build_report(
}
Err(err) => {
let reachability_plan = default_reachability_plan();
let (config_check, network_check, terminal_check, state_check, reachability_check) = tokio::join!(
let fallback_cwd = interactive
.cwd
.clone()
.unwrap_or_else(|| env::current_dir().unwrap_or_else(|_| PathBuf::from(".")));
let (
config_check,
network_check,
terminal_check,
git_check,
state_check,
reachability_check,
) = tokio::join!(
async {
run_sync_check("config", progress.clone(), || {
DoctorCheck::new(
@@ -423,6 +451,7 @@ async fn build_report(
terminal_check(command.no_color)
})
},
run_async_check("git", progress.clone(), git_check(fallback_cwd.as_path())),
async { run_sync_check("state", progress.clone(), fallback_state_check) },
run_async_check(
"provider reachability",
@@ -434,6 +463,7 @@ async fn build_report(
config_check,
network_check,
terminal_check,
git_check,
state_check,
reachability_check,
]);
@@ -1034,6 +1064,7 @@ fn config_check(config: &Config) -> DoctorCheck {
let status = if config.startup_warnings.is_empty() {
CheckStatus::Ok
} else {
push_startup_warning_counts(&mut details, &config.startup_warnings);
details.extend(
config
.startup_warnings
@@ -1046,6 +1077,23 @@ fn config_check(config: &Config) -> DoctorCheck {
DoctorCheck::new("config.load", "config", status, "config loaded").details(details)
}
fn push_startup_warning_counts(details: &mut Vec<String>, warnings: &[String]) {
details.push(format!("startup warnings: {}", warnings.len()));
for (label, needle) in [
("startup warning skills", "skill"),
("startup warning hooks", "hook"),
("startup warning plugins", "plugin"),
("startup warning MCP", "mcp"),
("startup warning deprecated", "deprecated"),
] {
let count = warnings
.iter()
.filter(|warning| warning.to_ascii_lowercase().contains(needle))
.count();
details.push(format!("{label}: {count}"));
}
}
fn feature_flag_details(config: &Config, details: &mut Vec<String>) {
let features = config.features.get();
let enabled_features = FEATURES
@@ -1579,6 +1627,7 @@ struct TerminalCheckInputs {
stream_supports_color: bool,
terminal_size: Result<(u16, u16), String>,
tmux_details: Vec<String>,
windows_console_details: Vec<String>,
}
impl TerminalCheckInputs {
@@ -1592,6 +1641,7 @@ impl TerminalCheckInputs {
} else {
Vec::new()
};
let windows_console_details = windows_console_details();
Self {
info,
env,
@@ -1603,6 +1653,7 @@ impl TerminalCheckInputs {
stream_supports_color: supports_color::on(Stream::Stdout).is_some(),
terminal_size,
tmux_details,
windows_console_details,
}
}
@@ -1619,6 +1670,51 @@ fn terminal_check(no_color_flag: bool) -> DoctorCheck {
terminal_check_from_inputs(TerminalCheckInputs::detect(no_color_flag))
}
#[cfg(windows)]
fn windows_console_details() -> Vec<String> {
use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE;
use windows_sys::Win32::System::Console::ENABLE_VIRTUAL_TERMINAL_PROCESSING;
use windows_sys::Win32::System::Console::GetConsoleCP;
use windows_sys::Win32::System::Console::GetConsoleMode;
use windows_sys::Win32::System::Console::GetConsoleOutputCP;
use windows_sys::Win32::System::Console::GetStdHandle;
use windows_sys::Win32::System::Console::STD_ERROR_HANDLE;
use windows_sys::Win32::System::Console::STD_OUTPUT_HANDLE;
let mut details = Vec::new();
details.push(format!("console input code page: {}", unsafe {
GetConsoleCP()
}));
details.push(format!("console output code page: {}", unsafe {
GetConsoleOutputCP()
}));
details.push(console_mode_detail("stdout console mode", unsafe {
GetStdHandle(STD_OUTPUT_HANDLE)
}));
details.push(console_mode_detail("stderr console mode", unsafe {
GetStdHandle(STD_ERROR_HANDLE)
}));
fn console_mode_detail(label: &str, handle: isize) -> String {
if handle == 0 || handle == INVALID_HANDLE_VALUE {
return format!("{label}: unavailable");
}
let mut mode = 0_u32;
if unsafe { GetConsoleMode(handle, &mut mode) } == 0 {
return format!("{label}: unavailable");
}
let vt_enabled = mode & ENABLE_VIRTUAL_TERMINAL_PROCESSING != 0;
format!("{label}: 0x{mode:08x} (VT processing: {vt_enabled})")
}
details
}
#[cfg(not(windows))]
fn windows_console_details() -> Vec<String> {
Vec::new()
}
fn terminal_check_from_inputs(inputs: TerminalCheckInputs) -> DoctorCheck {
let info = &inputs.info;
let name = info.name;
@@ -1652,6 +1748,7 @@ fn terminal_check_from_inputs(inputs: TerminalCheckInputs) -> DoctorCheck {
}
push_presence_env_values(&mut details, &inputs, REMOTE_TERMINAL_ENV_VARS);
details.extend(inputs.tmux_details.iter().cloned());
details.extend(inputs.windows_console_details.iter().cloned());
let locale_warning = locale.as_deref().is_some_and(is_non_utf8_locale);
let mut issues = Vec::new();
@@ -3030,6 +3127,31 @@ mod tests {
);
}
#[test]
fn startup_warning_counts_group_known_sources() {
let warnings = vec![
"Skipped loading 2 skill(s) due to invalid SKILL.md files.".to_string(),
"[features].codex_hooks is deprecated. Use [features].hooks instead.".to_string(),
"plugin example failed to load".to_string(),
"MCP server example failed to start".to_string(),
];
let mut details = Vec::new();
push_startup_warning_counts(&mut details, &warnings);
assert_eq!(
details,
vec![
"startup warnings: 4",
"startup warning skills: 1",
"startup warning hooks: 1",
"startup warning plugins: 1",
"startup warning MCP: 1",
"startup warning deprecated: 1",
]
);
}
#[test]
fn config_overrides_from_interactive_preserves_global_options() {
let interactive = TuiCli::parse_from([
@@ -3723,6 +3845,7 @@ mod tests {
stream_supports_color: true,
terminal_size: Ok((120, 40)),
tmux_details: Vec::new(),
windows_console_details: Vec::new(),
}
}
@@ -3856,6 +3979,22 @@ mod tests {
);
}
#[test]
fn terminal_check_includes_windows_console_details() {
let mut inputs = terminal_inputs();
inputs
.windows_console_details
.push("stdout console mode: 0x00000004 (VT processing: true)".to_string());
let check = terminal_check_from_inputs(inputs);
assert!(
check
.details
.contains(&"stdout console mode: 0x00000004 (VT processing: true)".to_string())
);
}
#[test]
fn terminal_check_keeps_tmux_probe_failures_non_fatal() {
let mut inputs = terminal_inputs();
+378
View File
@@ -0,0 +1,378 @@
use std::collections::BTreeSet;
use std::path::Path;
use std::path::PathBuf;
use std::process::Output;
use std::time::Duration;
use codex_git_utils::get_git_repo_root;
use tokio::process::Command;
use tokio::time::timeout;
use super::CheckStatus;
use super::DoctorCheck;
use super::DoctorIssue;
const GIT_COMMAND_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 2);
#[derive(Clone, Debug, Default, Eq, PartialEq)]
struct GitCheckInputs {
selected_git: Option<PathBuf>,
git_candidates: Vec<PathBuf>,
git_version: Option<String>,
git_exec_path: Option<String>,
git_build_options: Option<String>,
repo_root: Option<PathBuf>,
git_entry: Option<String>,
branch: Option<String>,
core_fsmonitor: Option<String>,
}
pub(super) async fn git_check(cwd: &Path) -> DoctorCheck {
let selected_git = which::which("git").ok();
let git_candidates = git_candidates();
let repo_root = get_git_repo_root(cwd);
let (git_version, git_exec_path, git_build_options, branch, core_fsmonitor) =
if let Some(git_path) = selected_git.as_deref() {
let (version, exec_path, build_options, branch, fsmonitor) = tokio::join!(
git_output(git_path, cwd, &["--version"]),
git_output(git_path, cwd, &["--exec-path"]),
git_output(git_path, cwd, &["version", "--build-options"]),
git_output(git_path, cwd, &["rev-parse", "--abbrev-ref", "HEAD"]),
git_output(git_path, cwd, &["config", "--get", "core.fsmonitor"]),
);
(version, exec_path, build_options, branch, fsmonitor)
} else {
(None, None, None, None, None)
};
git_check_from_inputs(GitCheckInputs {
selected_git,
git_candidates,
git_version,
git_exec_path,
git_build_options,
git_entry: repo_root.as_deref().map(git_entry_summary),
repo_root,
branch,
core_fsmonitor,
})
}
fn git_check_from_inputs(inputs: GitCheckInputs) -> DoctorCheck {
let mut details = Vec::new();
match inputs.selected_git.as_deref() {
Some(path) => details.push(format!("selected git: {}", path.display())),
None => details.push("selected git: not found".to_string()),
}
details.push(format!("PATH git entries: {}", inputs.git_candidates.len()));
for (index, path) in inputs.git_candidates.iter().enumerate() {
details.push(format!("PATH git #{}: {}", index + 1, path.display()));
}
push_optional_detail(&mut details, "git version", inputs.git_version.as_deref());
push_optional_detail(
&mut details,
"git exec path",
inputs.git_exec_path.as_deref(),
);
push_optional_detail(
&mut details,
"git build options",
inputs.git_build_options.as_deref(),
);
match inputs.repo_root.as_deref() {
Some(root) => {
details.push("repo detected: true".to_string());
details.push(format!("repo root: {}", root.display()));
}
None => details.push("repo detected: false".to_string()),
}
push_optional_detail(&mut details, ".git entry", inputs.git_entry.as_deref());
push_optional_detail(
&mut details,
"git branch",
normalized_branch(inputs.branch.as_deref()),
);
push_optional_detail(
&mut details,
"core.fsmonitor",
inputs
.core_fsmonitor
.as_deref()
.filter(|value| !value.is_empty()),
);
let mut check = DoctorCheck::new(
"git.environment",
"git",
CheckStatus::Ok,
git_summary(&inputs),
)
.details(details);
if inputs.selected_git.is_some() && inputs.git_version.is_none() {
check.status = CheckStatus::Warning;
check.summary = "Git executable found but could not be run".to_string();
check = check.issue(
DoctorIssue::new(
CheckStatus::Warning,
"Git executable was found on PATH but did not return a version",
)
.expected("git --version succeeds")
.remedy("Fix the selected Git executable or PATH so Codex can inspect Git metadata.")
.field("git version")
.field("selected git"),
);
} else if inputs.selected_git.is_none() && inputs.repo_root.is_some() {
check.status = CheckStatus::Warning;
check.summary = "Git repository detected but git executable was not found".to_string();
check = check.issue(
DoctorIssue::new(
CheckStatus::Warning,
"Git repository detected but git executable was not found",
)
.expected("git available on PATH")
.remedy("Install Git or fix PATH so Codex can inspect repository metadata.")
.field("selected git"),
);
} else if let Some(cause) =
old_windows_git_warning(inputs.git_version.as_deref(), cfg!(windows))
{
check.status = CheckStatus::Warning;
check.summary = cause.clone();
check = check.issue(
DoctorIssue::new(CheckStatus::Warning, cause)
.measured(inputs.git_version.unwrap_or_else(|| "unknown".to_string()))
.expected("current Git for Windows")
.remedy(
"Update Git for Windows or the bundled Git executable Codex resolves first.",
)
.field("git version")
.field("selected git"),
);
}
check
}
fn git_summary(inputs: &GitCheckInputs) -> String {
match inputs.git_version.as_deref() {
Some(version) => version.to_string(),
None if inputs.selected_git.is_some() => {
"git executable found; version unavailable".to_string()
}
None => "git executable not found".to_string(),
}
}
fn push_optional_detail(details: &mut Vec<String>, label: &str, value: Option<&str>) {
if let Some(value) = value {
details.push(format!("{label}: {value}"));
}
}
fn normalized_branch(branch: Option<&str>) -> Option<&str> {
match branch {
Some("HEAD") => Some("detached HEAD"),
Some(value) if !value.is_empty() => Some(value),
_ => None,
}
}
fn git_candidates() -> Vec<PathBuf> {
let Ok(candidates) = which::which_all("git") else {
return Vec::new();
};
let mut seen = BTreeSet::new();
candidates
.filter(|candidate| seen.insert(candidate.clone()))
.collect()
}
async fn git_output(git_path: &Path, cwd: &Path, args: &[&str]) -> Option<String> {
let mut command = Command::new(git_path);
command
.env("GIT_OPTIONAL_LOCKS", "0")
.args(args)
.current_dir(cwd)
.kill_on_drop(true);
let output = timeout(GIT_COMMAND_TIMEOUT, command.output())
.await
.ok()?
.ok()?;
command_output_text(output)
}
fn command_output_text(output: Output) -> Option<String> {
if !output.status.success() {
return None;
}
let stdout = String::from_utf8_lossy(&output.stdout);
let normalized = stdout
.lines()
.map(str::trim)
.filter(|line| !line.is_empty())
.collect::<Vec<_>>()
.join("; ");
if normalized.is_empty() {
None
} else {
Some(normalized)
}
}
fn git_entry_summary(repo_root: &Path) -> String {
let entry = repo_root.join(".git");
match std::fs::metadata(&entry) {
Ok(metadata) if metadata.is_dir() => "directory".to_string(),
Ok(metadata) if metadata.is_file() => std::fs::read_to_string(&entry)
.ok()
.and_then(|contents| {
contents
.strip_prefix("gitdir:")
.map(str::trim)
.map(|path| format!("file -> {path}"))
})
.unwrap_or_else(|| "file".to_string()),
Ok(_) => "other".to_string(),
Err(err) if err.kind() == std::io::ErrorKind::NotFound => "missing".to_string(),
Err(err) => format!("unreadable ({err})"),
}
}
fn old_windows_git_warning(version: Option<&str>, is_windows: bool) -> Option<String> {
if !is_windows {
return None;
}
let version = version?;
if version.to_ascii_lowercase().contains("msysgit") {
return Some("old msysgit installation may corrupt Windows TUI rendering".to_string());
}
let parsed = parse_git_version(version)?;
if parsed.major < 2 || (parsed.major == 2 && parsed.minor <= 34) {
return Some("old Git for Windows may corrupt Windows TUI rendering".to_string());
}
None
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
struct ParsedGitVersion {
major: u32,
minor: u32,
patch: u32,
}
fn parse_git_version(version: &str) -> Option<ParsedGitVersion> {
let version = version.strip_prefix("git version ")?;
let numeric = version
.split_whitespace()
.next()?
.split(".windows.")
.next()
.unwrap_or(version);
let mut parts = numeric.split('.');
Some(ParsedGitVersion {
major: parts.next()?.parse().ok()?,
minor: parts.next()?.parse().ok()?,
patch: parts.next().unwrap_or("0").parse().ok()?,
})
}
#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;
use super::*;
#[test]
fn parses_git_for_windows_version() {
assert_eq!(
parse_git_version("git version 2.34.1.windows.1"),
Some(ParsedGitVersion {
major: 2,
minor: 34,
patch: 1,
})
);
assert_eq!(
parse_git_version("git version 2.54.0.windows.1"),
Some(ParsedGitVersion {
major: 2,
minor: 54,
patch: 0,
})
);
}
#[test]
fn classifies_old_windows_git() {
assert_eq!(
old_windows_git_warning(
Some("git version 2.34.1.windows.1"),
/*is_windows*/ true
)
.as_deref(),
Some("old Git for Windows may corrupt Windows TUI rendering")
);
assert_eq!(
old_windows_git_warning(
Some("git version 2.54.0.windows.1"),
/*is_windows*/ true
),
None
);
assert_eq!(
old_windows_git_warning(
Some("git version 2.34.1.windows.1"),
/*is_windows*/ false
),
None
);
}
#[test]
fn warns_when_git_repo_has_no_git_executable() {
let check = git_check_from_inputs(GitCheckInputs {
repo_root: Some(PathBuf::from("/repo")),
..GitCheckInputs::default()
});
assert_eq!(check.status, CheckStatus::Warning);
assert_eq!(
check.summary,
"Git repository detected but git executable was not found"
);
}
#[test]
fn warns_when_selected_git_cannot_report_version() {
let check = git_check_from_inputs(GitCheckInputs {
selected_git: Some(PathBuf::from("/usr/bin/git")),
repo_root: Some(PathBuf::from("/repo")),
..GitCheckInputs::default()
});
assert_eq!(check.status, CheckStatus::Warning);
assert_eq!(check.summary, "Git executable found but could not be run");
}
#[test]
fn reports_git_candidates_and_repo_metadata() {
let check = git_check_from_inputs(GitCheckInputs {
selected_git: Some(PathBuf::from("/usr/bin/git")),
git_candidates: vec![PathBuf::from("/usr/bin/git"), PathBuf::from("/opt/bin/git")],
git_version: Some("git version 2.54.0".to_string()),
git_exec_path: Some("/usr/libexec/git-core".to_string()),
repo_root: Some(PathBuf::from("/repo")),
git_entry: Some("directory".to_string()),
branch: Some("main".to_string()),
core_fsmonitor: Some("false".to_string()),
..GitCheckInputs::default()
});
assert_eq!(check.status, CheckStatus::Ok);
assert!(check.details.contains(&"PATH git entries: 2".to_string()));
assert!(check.details.contains(&"git branch: main".to_string()));
assert!(check.details.contains(&"core.fsmonitor: false".to_string()));
}
}
+84 -5
View File
@@ -25,7 +25,9 @@ const SEPARATOR_WIDTH: usize = 61;
const GROUPS: &[OutputGroup] = &[
OutputGroup {
title: "Environment",
keys: &["runtime", "install", "search", "terminal", "state"],
keys: &[
"system", "runtime", "install", "search", "git", "terminal", "title", "state",
],
},
OutputGroup {
title: "Configuration",
@@ -611,12 +613,15 @@ fn auth_reachability_note(report: &DoctorReport) -> Option<DoctorNote> {
None
}
fn display_summary(check: &DoctorCheck, _options: HumanOutputOptions) -> String {
fn display_summary(check: &DoctorCheck, options: HumanOutputOptions) -> String {
match check.category.as_str() {
"system" => system_summary(check),
"runtime" => runtime_summary(check),
"install" if check.status == CheckStatus::Ok => "consistent".to_string(),
"search" => search_summary(check),
"git" => git_summary(check),
"terminal" => terminal_summary(check),
"title" => title_summary(check, options),
"state" => state_summary(check),
"config" if check.status == CheckStatus::Ok => "loaded".to_string(),
"mcp" => mcp_summary(check),
@@ -628,6 +633,10 @@ fn display_summary(check: &DoctorCheck, _options: HumanOutputOptions) -> String
}
}
fn system_summary(check: &DoctorCheck) -> String {
detail::detail_value(check, "os language").unwrap_or_else(|| check.summary.clone())
}
fn runtime_summary(check: &DoctorCheck) -> String {
if detail::detail_value(check, "current executable")
.is_some_and(|path| path.contains("/target/debug/"))
@@ -649,6 +658,12 @@ fn search_summary(check: &DoctorCheck) -> String {
}
}
fn git_summary(check: &DoctorCheck) -> String {
detail::detail_value(check, "git version")
.or_else(|| detail::detail_value(check, "selected git"))
.unwrap_or_else(|| check.summary.clone())
}
fn terminal_summary(check: &DoctorCheck) -> String {
let mut parts = Vec::new();
if let Some(terminal) = detail::detail_value(check, "terminal") {
@@ -668,6 +683,19 @@ fn terminal_summary(check: &DoctorCheck) -> String {
}
}
fn title_summary(check: &DoctorCheck, options: HumanOutputOptions) -> String {
let source = detail::detail_value(check, "terminal title source");
let project = detail::detail_value(check, "terminal title project value");
match (source, project) {
(Some(source), Some(project)) => {
let separator = if options.ascii { " | " } else { " · " };
format!("{source}{separator}project {project}")
}
(Some(source), None) => source,
_ => check.summary.clone(),
}
}
fn state_summary(check: &DoctorCheck) -> String {
let databases_ok = [
"state DB integrity",
@@ -1096,6 +1124,14 @@ mod tests {
fn sample_report() -> DoctorReport {
let checks = vec![
DoctorCheck::new(
"system.environment",
"system",
CheckStatus::Ok,
"OS language en-US",
)
.detail("os: macOS 15.0")
.detail("os language: en-US"),
DoctorCheck::new(
"runtime.provenance",
"runtime",
@@ -1114,12 +1150,30 @@ mod tests {
CheckStatus::Ok,
"search is OK (bundled)",
),
DoctorCheck::new(
"git.environment",
"git",
CheckStatus::Ok,
"git version 2.54.0",
)
.detail("selected git: /usr/bin/git")
.detail("git version: git version 2.54.0")
.detail("repo detected: true"),
DoctorCheck::new(
"terminal.env",
"terminal",
CheckStatus::Warning,
"narrow terminal",
),
DoctorCheck::new(
"terminal.title",
"title",
CheckStatus::Ok,
"terminal title default",
)
.detail("terminal title source: default")
.detail("terminal title items: activity, project-name")
.detail("terminal title project value: codex"),
DoctorCheck::new(
"state.paths",
"state",
@@ -1187,11 +1241,22 @@ Notes
─────────────────────────────────────────────────────────────
Environment
✓ system en-US
os macOS 15.0
OS language en-US
✓ runtime running local build on darwin-arm64
✓ install consistent
managed by npm: no · bun: no · package root —
✓ search search is OK (bundled)
✓ git git version 2.54.0
selected git /usr/bin/git
version git version 2.54.0
repo detected true
⚠ terminal narrow terminal
✓ title default · project codex
title source default
title items activity, project-name
project value codex
✓ state state paths inspectable
Configuration
@@ -1210,7 +1275,7 @@ Background Server
✓ app-server background server is not running
{}
9 ok · 2 notes · 1 warn · 1 fail failed
12 ok · 2 notes · 1 warn · 1 fail failed
--summary compact output --all expand truncated lists
--json redacted report
@@ -1220,6 +1285,14 @@ Background Server
assert_eq!(rendered, expected);
}
#[test]
fn render_human_report_snapshot_covers_environment_rows() {
insta::assert_snapshot!(
"doctor_human_report_environment_rows",
render_human_report(&sample_report(), detailed_no_color_unicode_options())
);
}
#[test]
fn render_human_report_supports_summary_output_without_color() {
let rendered = render_human_report(&sample_report(), summary_no_color_unicode_options());
@@ -1233,10 +1306,13 @@ Notes
─────────────────────────────────────────────────────────────
Environment
✓ system en-US
✓ runtime running local build on darwin-arm64
✓ install consistent
✓ search search is OK (bundled)
✓ git git version 2.54.0
⚠ terminal narrow terminal
✓ title default · project codex
✓ state state paths inspectable
Configuration
@@ -1254,7 +1330,7 @@ Background Server
✓ app-server background server is not running
{}
9 ok · 2 notes · 1 warn · 1 fail failed
12 ok · 2 notes · 1 warn · 1 fail failed
Run codex doctor without --summary for detailed diagnostics.
--all expand truncated lists --json redacted report
@@ -1285,10 +1361,13 @@ Notes
-------------------------------------------------------------
Environment
[ok] system en-US
[ok] runtime running local build on darwin-arm64
[ok] install consistent
[ok] search search is OK (bundled)
[ok] git git version 2.54.0
[!!] terminal narrow terminal
[ok] title default | project codex
[ok] state state paths inspectable
Configuration
@@ -1306,7 +1385,7 @@ Background Server
[ok] app-server background server is not running
{}
9 ok | 2 notes | 1 warn | 1 fail failed
12 ok | 2 notes | 1 warn | 1 fail failed
Run codex doctor without --summary for detailed diagnostics.
--all expand truncated lists --json redacted report
+118
View File
@@ -37,8 +37,11 @@ struct ParsedDetail {
pub(super) fn detail_lines(check: &DoctorCheck, options: HumanOutputOptions) -> Vec<HumanDetail> {
let parsed = parsed_details(check);
let details = match check.category.as_str() {
"system" => system_details(&parsed),
"runtime" => runtime_details(&parsed),
"install" => install_details(&parsed, options),
"git" => git_details(&parsed, options),
"title" => title_details(&parsed),
"config" => config_details(&parsed, options),
"state" => state_details(&parsed),
_ => generic_details(&parsed),
@@ -52,6 +55,30 @@ pub(super) fn detail_lines(check: &DoctorCheck, options: HumanOutputOptions) ->
details
}
fn system_details(parsed: &[ParsedDetail]) -> Vec<HumanDetail> {
let mut out = Vec::new();
push_row_if_present(&mut out, parsed, "os", "os");
push_row_if_present(&mut out, parsed, "os language", "OS language");
push_row_if_present(&mut out, parsed, "LC_ALL", "LC_ALL");
push_row_if_present(&mut out, parsed, "LC_CTYPE", "LC_CTYPE");
push_row_if_present(&mut out, parsed, "LANG", "LANG");
push_remaining(
&mut out,
parsed,
&[
"os",
"os type",
"os version",
"os language",
"LC_ALL",
"LC_CTYPE",
"LANG",
],
&[],
);
out
}
pub(super) fn detail_value(check: &DoctorCheck, label: &str) -> Option<String> {
parsed_details(check)
.into_iter()
@@ -232,6 +259,97 @@ fn install_details(parsed: &[ParsedDetail], options: HumanOutputOptions) -> Vec<
out
}
fn git_details(parsed: &[ParsedDetail], options: HumanOutputOptions) -> Vec<HumanDetail> {
let mut out = Vec::new();
push_row_if_present(&mut out, parsed, "selected git", "selected git");
push_row_if_present(&mut out, parsed, "git version", "version");
push_row_if_present(&mut out, parsed, "git exec path", "exec path");
push_row_if_present(&mut out, parsed, "repo detected", "repo detected");
push_row_if_present(&mut out, parsed, "repo root", "repo root");
push_row_if_present(&mut out, parsed, ".git entry", ".git entry");
push_row_if_present(&mut out, parsed, "git branch", "branch");
push_row_if_present(&mut out, parsed, "core.fsmonitor", "core.fsmonitor");
let path_entries = numbered_values(parsed, "PATH git #");
if !path_entries.is_empty() {
let total = path_entries.len();
let shown = if options.show_all {
total
} else {
total.min(3)
};
out.push(HumanDetail::Row {
label: format!("PATH entries ({total})"),
value: path_entries[0].clone(),
expected: None,
});
out.extend(
path_entries
.iter()
.skip(1)
.take(shown.saturating_sub(1))
.cloned()
.map(HumanDetail::Continuation),
);
if shown < total {
out.push(HumanDetail::Continuation(
"… (full list with --all)".to_string(),
));
}
}
push_remaining(
&mut out,
parsed,
&[
"selected git",
"PATH git entries",
"git version",
"git exec path",
"git build options",
"repo detected",
"repo root",
".git entry",
"git branch",
"core.fsmonitor",
],
&["PATH git #"],
);
out
}
fn title_details(parsed: &[ParsedDetail]) -> Vec<HumanDetail> {
let mut out = Vec::new();
push_row_if_present(&mut out, parsed, "terminal title source", "title source");
push_row_if_present(&mut out, parsed, "terminal title items", "title items");
push_row_if_present(&mut out, parsed, "terminal title activity", "activity item");
push_row_if_present(
&mut out,
parsed,
"terminal title project source",
"project source",
);
push_row_if_present(
&mut out,
parsed,
"terminal title project value",
"project value",
);
push_remaining(
&mut out,
parsed,
&[
"terminal title source",
"terminal title items",
"terminal title activity",
"terminal title project source",
"terminal title project value",
],
&[],
);
out
}
fn config_details(parsed: &[ParsedDetail], options: HumanOutputOptions) -> Vec<HumanDetail> {
let mut out = Vec::new();
if let Some(model) = value(parsed, "model") {
@@ -0,0 +1,50 @@
---
source: cli/src/doctor/output.rs
expression: "render_human_report(&sample_report(), detailed_no_color_unicode_options())"
---
Codex Doctor v0.0.0
Notes
⚠ terminal narrow terminal
✗ auth token expired - Run `codex login`.
─────────────────────────────────────────────────────────────
Environment
✓ system en-US
os macOS 15.0
OS language en-US
✓ runtime running local build on darwin-arm64
✓ install consistent
managed by npm: no · bun: no · package root —
✓ search search is OK (bundled)
✓ git git version 2.54.0
selected git /usr/bin/git
version git version 2.54.0
repo detected true
⚠ terminal narrow terminal
✓ title default · project codex
title source default
title items activity, project-name
project value codex
✓ state state paths inspectable
Configuration
✗ auth token expired — Run `codex login`.
OPENAI_API_KEY present
Updates
✓ updates update configuration is locally consistent
Connectivity
✓ network network environment readable
✓ websocket Responses WebSocket handshake succeeded
✓ reachability active provider endpoints are reachable over HTTP
Background Server
✓ app-server background server is not running
─────────────────────────────────────────────────────────────
12 ok · 2 notes · 1 warn · 1 fail failed
--summary compact output --all expand truncated lists
--json redacted report
+112
View File
@@ -0,0 +1,112 @@
use std::collections::BTreeMap;
use std::env;
use super::DoctorCheck;
use super::LOCALE_ENV_VARS;
#[derive(Clone, Debug, Default, Eq, PartialEq)]
struct SystemCheckInputs {
os: String,
os_type: String,
os_version: String,
os_language: Option<String>,
locale_env: BTreeMap<String, String>,
}
impl SystemCheckInputs {
fn detect() -> Self {
let info = os_info::get();
let locale_env = LOCALE_ENV_VARS
.iter()
.filter_map(|name| {
env::var(name)
.ok()
.map(|value| ((*name).to_string(), value))
})
.collect();
Self {
os: info.to_string(),
os_type: info.os_type().to_string(),
os_version: info.version().to_string(),
os_language: sys_locale::get_locale(),
locale_env,
}
}
}
pub(super) fn system_check() -> DoctorCheck {
system_check_from_inputs(SystemCheckInputs::detect())
}
fn system_check_from_inputs(inputs: SystemCheckInputs) -> DoctorCheck {
let mut details = vec![
format!("os: {}", inputs.os),
format!("os type: {}", inputs.os_type),
format!("os version: {}", inputs.os_version),
];
if let Some(language) = inputs.os_language.as_deref() {
details.push(format!("os language: {language}"));
} else {
details.push("os language: unavailable".to_string());
}
for name in LOCALE_ENV_VARS {
if let Some(value) = inputs.locale_env.get(*name) {
details.push(format!("{name}: {value}"));
}
}
let summary = inputs
.os_language
.as_deref()
.map(|language| format!("OS language {language}"))
.unwrap_or_else(|| "OS language unavailable".to_string());
DoctorCheck::new(
"system.environment",
"system",
super::CheckStatus::Ok,
summary,
)
.details(details)
}
#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;
use super::*;
#[test]
fn system_check_reports_os_language_and_locale_env() {
let mut locale_env = BTreeMap::new();
locale_env.insert("LANG".to_string(), "en_US.UTF-8".to_string());
let check = system_check_from_inputs(SystemCheckInputs {
os: "macOS 15.0".to_string(),
os_type: "macos".to_string(),
os_version: "15.0".to_string(),
os_language: Some("en-US".to_string()),
locale_env,
});
assert_eq!(check.summary, "OS language en-US");
assert!(check.details.contains(&"os language: en-US".to_string()));
assert!(check.details.contains(&"LANG: en_US.UTF-8".to_string()));
}
#[test]
fn system_check_handles_missing_os_language() {
let check = system_check_from_inputs(SystemCheckInputs {
os: "Linux".to_string(),
os_type: "linux".to_string(),
os_version: "unknown".to_string(),
os_language: None,
locale_env: BTreeMap::new(),
});
assert_eq!(check.summary, "OS language unavailable");
assert!(
check
.details
.contains(&"os language: unavailable".to_string())
);
}
}
+405
View File
@@ -0,0 +1,405 @@
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;
use codex_config::ConfigLayerSource;
use codex_config::ConfigLayerStackOrdering;
use codex_core::config::Config;
use codex_git_utils::get_git_repo_root;
use unicode_segmentation::UnicodeSegmentation;
use super::CheckStatus;
use super::DoctorCheck;
const DEFAULT_TERMINAL_TITLE_ITEMS: &[&str] = &["activity", "project-name"];
const PROJECT_TITLE_MAX_CHARS: usize = 24;
#[derive(Clone, Debug, Eq, PartialEq)]
struct TerminalTitleInputs {
configured_items: Option<Vec<String>>,
cwd: PathBuf,
project_root: Option<ProjectTitleRoot>,
}
#[derive(Clone, Debug, Eq, PartialEq)]
struct ProjectTitleRoot {
source: &'static str,
path: PathBuf,
}
pub(super) fn terminal_title_check(config: &Config) -> DoctorCheck {
terminal_title_check_from_inputs(TerminalTitleInputs {
configured_items: config.tui_terminal_title.clone(),
cwd: config.cwd.to_path_buf(),
project_root: terminal_title_project_root(config, &config.cwd),
})
}
fn terminal_title_check_from_inputs(inputs: TerminalTitleInputs) -> DoctorCheck {
let (source, items, invalid_items) = match inputs.configured_items {
Some(items) if items.is_empty() => ("disabled", Vec::new(), Vec::new()),
Some(items) => {
let (items, invalid_items) = parse_terminal_title_items(items);
("configured", items, invalid_items)
}
None => (
"default",
DEFAULT_TERMINAL_TITLE_ITEMS
.iter()
.map(ToString::to_string)
.collect(),
Vec::new(),
),
};
let mut details = vec![
format!("terminal title source: {source}"),
format!(
"terminal title items: {}",
if items.is_empty() {
"none".to_string()
} else {
items.join(", ")
}
),
format!("terminal title activity: {}", activity_enabled(&items)),
];
if !invalid_items.is_empty() {
details.push(format!(
"terminal title invalid items: {}",
invalid_items.join(", ")
));
}
if project_title_selected(&items) {
let (project_source, project_value) =
project_title_candidate(inputs.project_root, &inputs.cwd);
details.push(format!("terminal title project source: {project_source}"));
if let Some(project_value) = project_value {
details.push(format!("terminal title project value: {project_value}"));
}
}
let status = if invalid_items.is_empty() {
CheckStatus::Ok
} else {
CheckStatus::Warning
};
let summary = if invalid_items.is_empty() {
format!("terminal title {source}")
} else {
format!("terminal title {source} with invalid items")
};
let mut check = DoctorCheck::new("terminal.title", "title", status, summary).details(details);
if !invalid_items.is_empty() {
check = check.issue(
super::DoctorIssue::new(
CheckStatus::Warning,
"terminal title configuration contains unknown item identifiers",
)
.measured(invalid_items.join(", "))
.expected("known terminal title item identifiers")
.remedy("Remove or replace the unknown entries in [tui].terminal_title.")
.field("terminal title invalid items"),
);
}
check
}
fn parse_terminal_title_items(items: Vec<String>) -> (Vec<String>, Vec<String>) {
let mut invalid = Vec::new();
let mut invalid_seen = HashSet::new();
let mut parsed = Vec::new();
for item in items {
match terminal_title_item_id(&item) {
Some(id) => parsed.push(id.to_string()),
None => {
if invalid_seen.insert(item.clone()) {
invalid.push(format!(r#""{item}""#));
}
}
}
}
(parsed, invalid)
}
fn terminal_title_item_id(item: &str) -> Option<&'static str> {
match item {
"app-name" => Some("app-name"),
"project-name" | "project" => Some("project-name"),
"current-dir" => Some("current-dir"),
"activity" | "spinner" => Some("activity"),
"run-state" | "status" => Some("run-state"),
"thread-title" | "thread" => Some("thread-title"),
"git-branch" => Some("git-branch"),
"context-remaining" => Some("context-remaining"),
"context-used" | "context-usage" => Some("context-used"),
"five-hour-limit" => Some("five-hour-limit"),
"weekly-limit" => Some("weekly-limit"),
"codex-version" => Some("codex-version"),
"used-tokens" => Some("used-tokens"),
"total-input-tokens" => Some("total-input-tokens"),
"total-output-tokens" => Some("total-output-tokens"),
"thread-id" | "session-id" => Some("thread-id"),
"fast-mode" => Some("fast-mode"),
"model" | "model-name" => Some("model"),
"model-with-reasoning" => Some("model-with-reasoning"),
"task-progress" => Some("task-progress"),
_ => None,
}
}
fn activity_enabled(items: &[String]) -> bool {
items
.iter()
.any(|item| item == "activity" || item == "spinner")
}
fn project_title_selected(items: &[String]) -> bool {
items
.iter()
.any(|item| item == "project-name" || item == "project")
}
fn terminal_title_project_root(config: &Config, cwd: &Path) -> Option<ProjectTitleRoot> {
if let Some(repo_root) = get_git_repo_root(cwd) {
return Some(ProjectTitleRoot {
source: "git repo root",
path: repo_root,
});
}
config
.config_layer_stack
.get_layers(
ConfigLayerStackOrdering::LowestPrecedenceFirst,
/*include_disabled*/ true,
)
.iter()
.find_map(|layer| match &layer.name {
ConfigLayerSource::Project { dot_codex_folder } => dot_codex_folder
.as_path()
.parent()
.map(|root| ProjectTitleRoot {
source: "project config",
path: root.to_path_buf(),
}),
_ => None,
})
}
fn project_title_candidate(
project_root: Option<ProjectTitleRoot>,
cwd: &Path,
) -> (&'static str, Option<String>) {
if let Some(project_root) = project_root {
return (
project_root.source,
Some(truncate_title_part(path_display_name(&project_root.path))),
);
}
("cwd", Some(truncate_title_part(path_display_name(cwd))))
}
fn path_display_name(path: &Path) -> String {
path.file_name()
.map(|name| name.to_string_lossy().to_string())
.unwrap_or_else(|| path.display().to_string())
}
fn truncate_title_part(value: String) -> String {
let mut graphemes = value.graphemes(true);
let head = graphemes
.by_ref()
.take(PROJECT_TITLE_MAX_CHARS)
.collect::<String>();
if graphemes.next().is_none() || PROJECT_TITLE_MAX_CHARS <= 3 {
return head;
}
let mut truncated = head
.graphemes(true)
.take(PROJECT_TITLE_MAX_CHARS - 3)
.collect::<String>();
truncated.push_str("...");
truncated
}
#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;
use super::*;
#[test]
fn terminal_title_reports_default_items_and_git_project_name() {
let check = terminal_title_check_from_inputs(TerminalTitleInputs {
configured_items: None,
cwd: PathBuf::from("/repo/subdir"),
project_root: Some(ProjectTitleRoot {
source: "git repo root",
path: PathBuf::from("/repo"),
}),
});
assert_eq!(check.summary, "terminal title default");
assert!(
check
.details
.contains(&"terminal title items: activity, project-name".to_string())
);
assert!(
check
.details
.contains(&"terminal title project source: git repo root".to_string())
);
assert!(
check
.details
.contains(&"terminal title project value: repo".to_string())
);
}
#[test]
fn terminal_title_reports_disabled_configuration() {
let check = terminal_title_check_from_inputs(TerminalTitleInputs {
configured_items: Some(Vec::new()),
cwd: PathBuf::from("/workspace"),
project_root: None,
});
assert_eq!(check.summary, "terminal title disabled");
assert!(
check
.details
.contains(&"terminal title items: none".to_string())
);
assert!(
check
.details
.contains(&"terminal title activity: false".to_string())
);
assert!(
!check
.details
.iter()
.any(|detail| detail.starts_with("terminal title project "))
);
}
#[test]
fn terminal_title_reports_project_config_fallback() {
let check = terminal_title_check_from_inputs(TerminalTitleInputs {
configured_items: Some(vec!["project".to_string()]),
cwd: PathBuf::from("/workspace/project/subdir"),
project_root: Some(ProjectTitleRoot {
source: "project config",
path: PathBuf::from("/workspace/project"),
}),
});
assert_eq!(check.summary, "terminal title configured");
assert!(
check
.details
.contains(&"terminal title project source: project config".to_string())
);
assert!(
check
.details
.contains(&"terminal title project value: project".to_string())
);
}
#[test]
fn terminal_title_omits_project_when_project_item_is_not_selected() {
let check = terminal_title_check_from_inputs(TerminalTitleInputs {
configured_items: Some(vec!["model".to_string()]),
cwd: PathBuf::from("/workspace/project"),
project_root: Some(ProjectTitleRoot {
source: "project config",
path: PathBuf::from("/workspace/project"),
}),
});
assert_eq!(check.summary, "terminal title configured");
assert!(
!check
.details
.iter()
.any(|detail| detail.starts_with("terminal title project "))
);
}
#[test]
fn terminal_title_warns_for_invalid_configured_items() {
let check = terminal_title_check_from_inputs(TerminalTitleInputs {
configured_items: Some(vec![
"project".to_string(),
"bogus".to_string(),
"activity".to_string(),
"bogus".to_string(),
]),
cwd: PathBuf::from("/workspace/project"),
project_root: Some(ProjectTitleRoot {
source: "project config",
path: PathBuf::from("/workspace/project"),
}),
});
assert_eq!(check.status, CheckStatus::Warning);
assert_eq!(
check.summary,
"terminal title configured with invalid items"
);
assert!(
check
.details
.contains(&"terminal title items: project-name, activity".to_string())
);
assert!(
check
.details
.contains(&r#"terminal title invalid items: "bogus""#.to_string())
);
assert_eq!(check.issues.len(), 1);
}
#[test]
fn terminal_title_warns_when_all_configured_items_are_invalid() {
let check = terminal_title_check_from_inputs(TerminalTitleInputs {
configured_items: Some(vec!["bogus".to_string()]),
cwd: PathBuf::from("/workspace/project"),
project_root: None,
});
assert_eq!(check.status, CheckStatus::Warning);
assert!(
check
.details
.contains(&"terminal title items: none".to_string())
);
assert!(
check
.details
.contains(&r#"terminal title invalid items: "bogus""#.to_string())
);
}
#[test]
fn terminal_title_project_value_uses_tui_truncation_shape() {
let check = terminal_title_check_from_inputs(TerminalTitleInputs {
configured_items: Some(vec!["project".to_string()]),
cwd: PathBuf::from("/workspace/abcdefghijklmnopqrstuvwxyz"),
project_root: Some(ProjectTitleRoot {
source: "project config",
path: PathBuf::from("/workspace/abcdefghijklmnopqrstuvwxyz"),
}),
});
assert!(
check
.details
.contains(&"terminal title project value: abcdefghijklmnopqrstu...".to_string())
);
}
}
+2 -1
View File
@@ -157,7 +157,8 @@ pub(super) fn store_callback(
}
};
if let Some(state) = scope.get_slot_mut::<RuntimeState>() {
state.stored_values.insert(key, serialized);
state.stored_values.insert(key.clone(), serialized.clone());
state.stored_value_writes.insert(key, serialized);
}
}
+18 -21
View File
@@ -35,7 +35,6 @@ pub struct ExecuteRequest {
pub tool_call_id: String,
pub enabled_tools: Vec<ToolDefinition>,
pub source: String,
pub stored_values: HashMap<String, JsonValue>,
pub yield_time_ms: Option<u64>,
pub max_output_tokens: Option<usize>,
}
@@ -116,7 +115,6 @@ pub enum RuntimeResponse {
Result {
cell_id: String,
content_items: Vec<FunctionCallOutputContentItem>,
stored_values: HashMap<String, JsonValue>,
error_text: Option<String>,
},
}
@@ -182,12 +180,13 @@ pub(crate) enum RuntimeEvent {
text: String,
},
Result {
stored_values: HashMap<String, JsonValue>,
stored_value_writes: HashMap<String, JsonValue>,
error_text: Option<String>,
},
}
pub(crate) fn spawn_runtime(
stored_values: HashMap<String, JsonValue>,
request: ExecuteRequest,
event_tx: mpsc::UnboundedSender<RuntimeEvent>,
pending_mode: PendingRuntimeMode,
@@ -214,7 +213,7 @@ pub(crate) fn spawn_runtime(
tool_call_id: request.tool_call_id,
enabled_tools,
source: request.source,
stored_values: request.stored_values,
stored_values,
};
thread::spawn(move || {
@@ -248,6 +247,7 @@ pub(super) struct RuntimeState {
pending_tool_calls: HashMap<String, v8::Global<v8::PromiseResolver>>,
pending_timeouts: HashMap<u64, timers::ScheduledTimeout>,
stored_values: HashMap<String, JsonValue>,
stored_value_writes: HashMap<String, JsonValue>,
enabled_tools: Vec<EnabledToolMetadata>,
next_tool_call_id: u64,
next_timeout_id: u64,
@@ -259,7 +259,7 @@ pub(super) struct RuntimeState {
pub(super) enum CompletionState {
Pending,
Completed {
stored_values: HashMap<String, JsonValue>,
stored_value_writes: HashMap<String, JsonValue>,
error_text: Option<String>,
},
}
@@ -305,6 +305,7 @@ fn run_runtime(
pending_tool_calls: HashMap::new(),
pending_timeouts: HashMap::new(),
stored_values: config.stored_values,
stored_value_writes: HashMap::new(),
enabled_tools: config.enabled_tools,
next_tool_call_id: 1,
next_timeout_id: 1,
@@ -330,10 +331,10 @@ fn run_runtime(
match module_loader::completion_state(scope, pending_promise.as_ref()) {
CompletionState::Completed {
stored_values,
stored_value_writes,
error_text,
} => {
send_result(&event_tx, stored_values, error_text);
send_result(&event_tx, stored_value_writes, error_text);
return;
}
CompletionState::Pending => {}
@@ -375,10 +376,10 @@ fn run_runtime(
scope.perform_microtask_checkpoint();
match module_loader::completion_state(scope, pending_promise.as_ref()) {
CompletionState::Completed {
stored_values,
stored_value_writes,
error_text,
} => {
send_result(&event_tx, stored_values, error_text);
send_result(&event_tx, stored_value_writes, error_text);
return;
}
CompletionState::Pending => {}
@@ -422,21 +423,21 @@ fn capture_scope_send_error(
event_tx: &mpsc::UnboundedSender<RuntimeEvent>,
error_text: Option<String>,
) {
let stored_values = scope
let stored_value_writes = scope
.get_slot::<RuntimeState>()
.map(|state| state.stored_values.clone())
.map(|state| state.stored_value_writes.clone())
.unwrap_or_default();
send_result(event_tx, stored_values, error_text);
send_result(event_tx, stored_value_writes, error_text);
}
fn send_result(
event_tx: &mpsc::UnboundedSender<RuntimeEvent>,
stored_values: HashMap<String, JsonValue>,
stored_value_writes: HashMap<String, JsonValue>,
error_text: Option<String>,
) {
let _ = event_tx.send(RuntimeEvent::Result {
stored_values,
stored_value_writes,
error_text,
});
}
@@ -463,7 +464,6 @@ mod tests {
tool_call_id: "call_1".to_string(),
enabled_tools: Vec::new(),
source: source.to_string(),
stored_values: HashMap::new(),
yield_time_ms: Some(1),
max_output_tokens: None,
}
@@ -473,6 +473,7 @@ mod tests {
async fn terminate_execution_stops_cpu_bound_module() {
let (event_tx, mut event_rx) = mpsc::unbounded_channel();
let (_runtime_tx, _runtime_control_tx, runtime_terminate_handle) = spawn_runtime(
HashMap::new(),
execute_request("while (true) {}"),
event_tx,
PendingRuntimeMode::Continue,
@@ -491,14 +492,9 @@ mod tests {
.await
.unwrap()
.unwrap();
let RuntimeEvent::Result {
stored_values,
error_text,
} = result_event
else {
let RuntimeEvent::Result { error_text, .. } = result_event else {
panic!("expected runtime result after termination");
};
assert_eq!(stored_values, HashMap::new());
assert!(error_text.is_some());
assert!(
@@ -513,6 +509,7 @@ mod tests {
async fn pending_mode_freezes_runtime_commands_until_resume() {
let (event_tx, mut event_rx) = mpsc::unbounded_channel();
let (runtime_tx, runtime_control_tx, _runtime_terminate_handle) = spawn_runtime(
HashMap::new(),
execute_request(
r#"
await new Promise((resolve) => setTimeout(resolve, 60_000));
@@ -104,14 +104,14 @@ pub(super) fn completion_state(
scope: &mut v8::PinScope<'_, '_>,
pending_promise: Option<&v8::Global<v8::Promise>>,
) -> CompletionState {
let stored_values = scope
let stored_value_writes = scope
.get_slot::<RuntimeState>()
.map(|state| state.stored_values.clone())
.map(|state| state.stored_value_writes.clone())
.unwrap_or_default();
let Some(pending_promise) = pending_promise else {
return CompletionState::Completed {
stored_values,
stored_value_writes,
error_text: None,
};
};
@@ -120,7 +120,7 @@ pub(super) fn completion_state(
match promise.state() {
v8::PromiseState::Pending => CompletionState::Pending,
v8::PromiseState::Fulfilled => CompletionState::Completed {
stored_values,
stored_value_writes,
error_text: None,
},
v8::PromiseState::Rejected => {
@@ -131,7 +131,7 @@ pub(super) fn completion_state(
Some(value_to_error_text(scope, result))
};
CompletionState::Completed {
stored_values,
stored_value_writes,
error_text,
}
}
+10 -26
View File
@@ -73,14 +73,10 @@ impl CodeModeService {
}
}
pub async fn stored_values(&self) -> HashMap<String, JsonValue> {
async fn stored_values(&self) -> HashMap<String, JsonValue> {
self.inner.stored_values.lock().await.clone()
}
pub async fn replace_stored_values(&self, values: HashMap<String, JsonValue>) {
*self.inner.stored_values.lock().await = values;
}
/// Reserves the runtime cell id for a future `execute` request.
///
/// The runtime can issue nested tool calls before the first `execute`
@@ -138,6 +134,7 @@ impl CodeModeService {
let cell_id = request.cell_id.clone();
let (event_tx, event_rx) = mpsc::unbounded_channel();
let (control_tx, control_rx) = mpsc::unbounded_channel();
let stored_values = self.stored_values().await;
let (runtime_tx, runtime_control_tx, runtime_terminate_handle) = {
let mut sessions = self.inner.sessions.lock().await;
if sessions.contains_key(&cell_id) {
@@ -145,7 +142,7 @@ impl CodeModeService {
}
let (runtime_tx, runtime_control_tx, runtime_terminate_handle) =
spawn_runtime(request, event_tx, pending_mode)?;
spawn_runtime(stored_values, request, event_tx, pending_mode)?;
// Keep the session registry locked through insertion so a
// caller-owned cell id cannot race with another execute and replace
@@ -349,7 +346,6 @@ enum SessionResponseSender {
struct PendingResult {
content_items: Vec<FunctionCallOutputContentItem>,
stored_values: HashMap<String, JsonValue>,
error_text: Option<String>,
}
@@ -366,7 +362,6 @@ fn missing_cell_response(cell_id: String) -> RuntimeResponse {
error_text: Some(format!("exec cell {cell_id} not found")),
cell_id,
content_items: Vec::new(),
stored_values: HashMap::new(),
}
}
@@ -374,7 +369,6 @@ fn pending_result_response(cell_id: &str, result: PendingResult) -> RuntimeRespo
RuntimeResponse::Result {
cell_id: cell_id.to_string(),
content_items: result.content_items,
stored_values: result.stored_values,
error_text: result.error_text,
}
}
@@ -476,7 +470,6 @@ async fn run_session_control(
if pending_result.is_none() {
let result = PendingResult {
content_items: std::mem::take(&mut content_items),
stored_values: HashMap::new(),
error_text: Some("exec runtime ended unexpectedly".to_string()),
};
if send_or_buffer_result(
@@ -551,7 +544,7 @@ async fn run_session_control(
.await;
}
RuntimeEvent::Result {
stored_values,
stored_value_writes,
error_text,
} => {
yield_timer = None;
@@ -565,9 +558,13 @@ async fn run_session_control(
}
break;
}
inner
.stored_values
.lock()
.await
.extend(stored_value_writes);
let result = PendingResult {
content_items: std::mem::take(&mut content_items),
stored_values,
error_text,
};
if send_or_buffer_result(
@@ -715,7 +712,6 @@ mod tests {
tool_call_id: "call_1".to_string(),
enabled_tools: Vec::new(),
source: source.to_string(),
stored_values: HashMap::new(),
yield_time_ms: Some(1),
max_output_tokens: None,
}
@@ -752,7 +748,6 @@ mod tests {
content_items: vec![FunctionCallOutputContentItem::InputText {
text: "before".to_string(),
}],
stored_values: HashMap::new(),
error_text: None,
}
);
@@ -778,7 +773,6 @@ mod tests {
content_items: vec![FunctionCallOutputContentItem::InputText {
text: "done".to_string(),
}],
stored_values: HashMap::new(),
error_text: None,
})
);
@@ -1108,7 +1102,6 @@ text("done");
content_items: vec![FunctionCallOutputContentItem::InputText {
text: "done".to_string(),
}],
stored_values: HashMap::new(),
error_text: None,
}
))
@@ -1135,7 +1128,6 @@ text("done");
content_items: vec![FunctionCallOutputContentItem::InputText {
text: "false".to_string(),
}],
stored_values: HashMap::new(),
error_text: None,
}
);
@@ -1175,7 +1167,6 @@ text(value);
content_items: vec![FunctionCallOutputContentItem::InputText {
text: "jeudi 2 janvier \u{e0} 03:04:05".to_string(),
}],
stored_values: HashMap::new(),
error_text: None,
}
);
@@ -1214,7 +1205,6 @@ text(formatter.format(new Date("2025-01-02T03:04:05Z")));
content_items: vec![FunctionCallOutputContentItem::InputText {
text: "jeudi 2 janvier \u{e0} 03:04:05".to_string(),
}],
stored_values: HashMap::new(),
error_text: None,
}
);
@@ -1257,7 +1247,6 @@ text(JSON.stringify(returnsUndefined));
text: "[true,true,true]".to_string(),
},
],
stored_values: HashMap::new(),
error_text: None,
}
);
@@ -1292,7 +1281,6 @@ image({
image_url: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==".to_string(),
detail: Some(crate::ImageDetail::Original),
}],
stored_values: HashMap::new(),
error_text: None,
}
);
@@ -1328,7 +1316,6 @@ image(
image_url: "https://example.com/image.jpg".to_string(),
detail: Some(crate::ImageDetail::Original),
}],
stored_values: HashMap::new(),
error_text: None,
}
);
@@ -1366,7 +1353,6 @@ image(
image_url: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==".to_string(),
detail: Some(crate::ImageDetail::High),
}],
stored_values: HashMap::new(),
error_text: None,
}
);
@@ -1396,7 +1382,6 @@ image({
RuntimeResponse::Result {
cell_id: "1".to_string(),
content_items: Vec::new(),
stored_values: HashMap::new(),
error_text: Some("image detail must be one of: high, original".to_string()),
}
);
@@ -1433,7 +1418,6 @@ image({
RuntimeResponse::Result {
cell_id: "1".to_string(),
content_items: Vec::new(),
stored_values: HashMap::new(),
error_text: Some(
"image expects a non-empty image URL string, an object with image_url and optional detail, or a raw MCP image block".to_string(),
),
@@ -1459,7 +1443,6 @@ image({
WaitOutcome::MissingCell(RuntimeResponse::Result {
cell_id: "missing".to_string(),
content_items: Vec::new(),
stored_values: HashMap::new(),
error_text: Some("exec cell missing not found".to_string()),
})
);
@@ -1473,6 +1456,7 @@ image({
let (initial_response_tx, initial_response_rx) = oneshot::channel();
let (runtime_event_tx, _runtime_event_rx) = mpsc::unbounded_channel();
let (runtime_tx, runtime_control_tx, runtime_terminate_handle) = spawn_runtime(
HashMap::new(),
ExecuteRequest {
source: "await new Promise(() => {})".to_string(),
yield_time_ms: None,
+4
View File
@@ -2,6 +2,7 @@ use crate::TransportError;
use crate::error::ApiError;
use crate::rate_limits::parse_promo_message;
use crate::rate_limits::parse_rate_limit_for_limit;
use crate::rate_limits::parse_rate_limit_reached_type;
use base64::Engine;
use chrono::DateTime;
use chrono::Utc;
@@ -85,6 +86,8 @@ pub fn map_api_error(err: ApiError) -> CodexErr {
parse_rate_limit_for_limit(map, limit_id.as_deref())
});
let promo_message = headers.as_ref().and_then(parse_promo_message);
let rate_limit_reached_type =
headers.as_ref().and_then(parse_rate_limit_reached_type);
let resets_at = err
.error
.resets_at
@@ -94,6 +97,7 @@ pub fn map_api_error(err: ApiError) -> CodexErr {
resets_at,
rate_limits: rate_limits.map(Box::new),
promo_message,
rate_limit_reached_type,
});
} else if err.error.error_type.as_deref() == Some("usage_not_included") {
return CodexErr::UsageNotIncluded;
@@ -194,6 +194,37 @@ fn map_api_error_does_not_fallback_limit_name_to_limit_id() {
);
}
#[test]
fn map_api_error_ignores_unparseable_rate_limit_reached_type_headers() {
let values = [
http::HeaderValue::from_static("future_rate_limit_reached_type"),
http::HeaderValue::from_bytes(&[0xff]).expect("valid opaque header value"),
];
for value in values {
let mut headers = HeaderMap::new();
headers.insert("x-codex-rate-limit-reached-type", value);
let body = serde_json::json!({
"error": {
"type": "usage_limit_reached",
"plan_type": "pro",
}
})
.to_string();
let err = map_api_error(ApiError::Transport(TransportError::Http {
status: http::StatusCode::TOO_MANY_REQUESTS,
url: Some("http://example.com/v1/responses".to_string()),
headers: Some(headers),
body: Some(body),
}));
let CodexErr::UsageLimitReached(usage_limit) = err else {
panic!("expected CodexErr::UsageLimitReached, got {err:?}");
};
assert_eq!(usage_limit.rate_limit_reached_type, None);
}
}
#[test]
fn map_api_error_extracts_identity_auth_details_from_headers() {
let mut headers = HeaderMap::new();
+8
View File
@@ -1,5 +1,6 @@
use codex_protocol::account::PlanType;
use codex_protocol::protocol::CreditsSnapshot;
use codex_protocol::protocol::RateLimitReachedType;
use codex_protocol::protocol::RateLimitSnapshot;
use codex_protocol::protocol::RateLimitWindow;
use http::HeaderMap;
@@ -178,6 +179,13 @@ pub fn parse_promo_message(headers: &HeaderMap) -> Option<String> {
.map(std::string::ToString::to_string)
}
pub(crate) fn parse_rate_limit_reached_type(headers: &HeaderMap) -> Option<RateLimitReachedType> {
parse_header_str(headers, "x-codex-rate-limit-reached-type")?
.trim()
.parse()
.ok()
}
fn parse_rate_limit_window(
headers: &HeaderMap,
used_percent_header: &str,
-3
View File
@@ -291,9 +291,6 @@ pub struct ConfigToml {
#[schemars(skip)]
pub js_repl_node_module_dirs: Option<Vec<AbsolutePathBuf>>,
/// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution.
pub zsh_path: Option<AbsolutePathBuf>,
/// Profile to use from the `profiles` map.
pub profile: Option<String>,
-2
View File
@@ -48,8 +48,6 @@ pub struct ConfigProfile {
/// Deprecated: ignored.
#[schemars(skip)]
pub js_repl_node_module_dirs: Option<Vec<AbsolutePathBuf>>,
/// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution.
pub zsh_path: Option<AbsolutePathBuf>,
pub experimental_compact_prompt_file: Option<AbsolutePathBuf>,
pub include_permissions_instructions: Option<bool>,
pub include_apps_instructions: Option<bool>,
+1
View File
@@ -6,6 +6,7 @@ pub mod marketplace;
pub mod marketplace_add;
pub mod marketplace_remove;
pub mod marketplace_upgrade;
mod plugin_bundle_archive;
pub mod remote;
pub mod remote_bundle;
pub mod remote_legacy;
@@ -0,0 +1,315 @@
use flate2::Compression;
use flate2::read::GzDecoder;
use flate2::write::GzEncoder;
use std::fmt;
use std::fs;
use std::io;
use std::io::Read;
use std::io::Write;
use std::path::Path;
use std::path::PathBuf;
use tar::Archive;
#[derive(Debug, thiserror::Error)]
pub(crate) enum PluginBundlePackError {
#[error("invalid plugin path `{path}`: {reason}")]
InvalidPluginPath { path: PathBuf, reason: String },
#[error("plugin archive would be {bytes} bytes, exceeding maximum size of {max_bytes} bytes")]
ArchiveTooLarge { bytes: usize, max_bytes: usize },
#[error("failed to archive plugin bundle: {source}")]
Io {
#[source]
source: io::Error,
},
}
#[derive(Debug, thiserror::Error)]
pub(crate) enum PluginBundleUnpackError {
#[error(
"plugin bundle extracted size would be {bytes} bytes, exceeding maximum total size of {max_bytes} bytes"
)]
ExtractedBundleTooLarge { bytes: u64, max_bytes: u64 },
#[error("{context}: {source}")]
Io {
context: &'static str,
#[source]
source: io::Error,
},
#[error("{0}")]
InvalidBundle(String),
}
impl PluginBundleUnpackError {
fn io(context: &'static str, source: io::Error) -> Self {
Self::Io { context, source }
}
}
pub(crate) fn pack_plugin_bundle_tar_gz(
plugin_path: &Path,
max_bytes: usize,
) -> Result<Vec<u8>, PluginBundlePackError> {
if !plugin_path.is_dir() {
return Err(PluginBundlePackError::InvalidPluginPath {
path: plugin_path.to_path_buf(),
reason: "expected a plugin directory".to_string(),
});
}
if !plugin_path.join(".codex-plugin/plugin.json").is_file() {
return Err(PluginBundlePackError::InvalidPluginPath {
path: plugin_path.to_path_buf(),
reason: "missing .codex-plugin/plugin.json".to_string(),
});
}
let encoder = GzEncoder::new(SizeLimitedBuffer::new(max_bytes), Compression::default());
let mut archive = tar::Builder::new(encoder);
append_plugin_tree(&mut archive, plugin_path, plugin_path).map_err(archive_io_error)?;
let encoder = archive.into_inner().map_err(archive_io_error)?;
encoder
.finish()
.map(SizeLimitedBuffer::into_inner)
.map_err(archive_io_error)
}
fn append_plugin_tree<W: Write>(
archive: &mut tar::Builder<W>,
plugin_root: &Path,
current: &Path,
) -> io::Result<()> {
let mut entries = fs::read_dir(current)?.collect::<Result<Vec<_>, io::Error>>()?;
entries.sort_by_key(fs::DirEntry::file_name);
for entry in entries {
let path = entry.path();
let file_type = entry.file_type()?;
let relative_path = path.strip_prefix(plugin_root).map_err(|err| {
io::Error::other(format!(
"failed to compute plugin archive path for `{}`: {err}",
path.display()
))
})?;
if file_type.is_dir() {
archive.append_dir(relative_path, &path)?;
append_plugin_tree(archive, plugin_root, &path)?;
} else if file_type.is_file() {
archive.append_path_with_name(&path, relative_path)?;
} else {
return Err(io::Error::other(format!(
"unsupported plugin archive entry type: {}",
path.display()
)));
}
}
Ok(())
}
fn archive_io_error(source: io::Error) -> PluginBundlePackError {
if let Some(limit) = source
.get_ref()
.and_then(|err| err.downcast_ref::<ArchiveSizeLimitExceeded>())
{
return PluginBundlePackError::ArchiveTooLarge {
bytes: limit.bytes,
max_bytes: limit.max_bytes,
};
}
PluginBundlePackError::Io { source }
}
pub(crate) fn unpack_plugin_bundle_tar_gz(
bytes: &[u8],
destination: &Path,
max_total_bytes: u64,
) -> Result<(), PluginBundleUnpackError> {
fs::create_dir_all(destination).map_err(|source| {
PluginBundleUnpackError::io(
"failed to create plugin bundle extraction directory",
source,
)
})?;
let archive = GzDecoder::new(std::io::Cursor::new(bytes));
let mut archive = Archive::new(archive);
unpack_plugin_bundle_tar(&mut archive, destination, max_total_bytes)
}
fn unpack_plugin_bundle_tar<R: Read>(
archive: &mut Archive<R>,
destination: &Path,
max_total_bytes: u64,
) -> Result<(), PluginBundleUnpackError> {
let mut extracted_bytes = 0u64;
let entries = archive.entries().map_err(|source| {
PluginBundleUnpackError::io("failed to read plugin bundle tar", source)
})?;
for entry in entries {
let mut entry = entry.map_err(|source| {
PluginBundleUnpackError::io("failed to read plugin bundle tar entry", source)
})?;
let entry_type = entry.header().entry_type();
let entry_size = entry.size();
let entry_path = entry
.path()
.map_err(|source| {
PluginBundleUnpackError::io("failed to read plugin bundle tar entry path", source)
})?
.into_owned();
let output_path = checked_tar_output_path(destination, &entry_path)?;
if entry_type.is_dir() {
fs::create_dir_all(&output_path).map_err(|source| {
PluginBundleUnpackError::io("failed to create plugin bundle directory", source)
})?;
continue;
}
if entry_type.is_file() {
enforce_total_extracted_size(entry_size, &mut extracted_bytes, max_total_bytes)?;
let Some(parent) = output_path.parent() else {
return Err(PluginBundleUnpackError::InvalidBundle(format!(
"plugin bundle output path has no parent: {}",
output_path.display()
)));
};
fs::create_dir_all(parent).map_err(|source| {
PluginBundleUnpackError::io("failed to create plugin bundle directory", source)
})?;
entry.unpack(&output_path).map_err(|source| {
PluginBundleUnpackError::io("failed to unpack plugin bundle entry", source)
})?;
continue;
}
if entry_type.is_hard_link() || entry_type.is_symlink() {
return Err(PluginBundleUnpackError::InvalidBundle(format!(
"plugin bundle tar entry `{}` is a link",
entry_path.display()
)));
}
return Err(PluginBundleUnpackError::InvalidBundle(format!(
"plugin bundle tar entry `{}` has unsupported type {:?}",
entry_path.display(),
entry_type
)));
}
Ok(())
}
fn checked_tar_output_path(
destination: &Path,
entry_name: &Path,
) -> Result<PathBuf, PluginBundleUnpackError> {
let mut output_path = destination.to_path_buf();
let mut has_component = false;
for component in entry_name.components() {
match component {
std::path::Component::Normal(component) => {
has_component = true;
output_path.push(component);
}
std::path::Component::CurDir => {}
std::path::Component::ParentDir
| std::path::Component::RootDir
| std::path::Component::Prefix(_) => {
return Err(PluginBundleUnpackError::InvalidBundle(format!(
"plugin bundle tar entry `{}` escapes extraction root",
entry_name.display()
)));
}
}
}
if !has_component {
return Err(PluginBundleUnpackError::InvalidBundle(
"plugin bundle tar entry has an empty path".to_string(),
));
}
Ok(output_path)
}
fn enforce_total_extracted_size(
entry_size: u64,
extracted_bytes: &mut u64,
max_total_bytes: u64,
) -> Result<(), PluginBundleUnpackError> {
let next_total = extracted_bytes.checked_add(entry_size).ok_or(
PluginBundleUnpackError::ExtractedBundleTooLarge {
bytes: u64::MAX,
max_bytes: max_total_bytes,
},
)?;
if next_total > max_total_bytes {
return Err(PluginBundleUnpackError::ExtractedBundleTooLarge {
bytes: next_total,
max_bytes: max_total_bytes,
});
}
*extracted_bytes = next_total;
Ok(())
}
struct SizeLimitedBuffer {
bytes: Vec<u8>,
max_bytes: usize,
}
impl SizeLimitedBuffer {
fn new(max_bytes: usize) -> Self {
Self {
bytes: Vec::new(),
max_bytes,
}
}
fn into_inner(self) -> Vec<u8> {
self.bytes
}
}
impl Write for SizeLimitedBuffer {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
let next_len = self.bytes.len().checked_add(buf.len()).ok_or_else(|| {
io::Error::other(ArchiveSizeLimitExceeded {
bytes: usize::MAX,
max_bytes: self.max_bytes,
})
})?;
if next_len > self.max_bytes {
return Err(io::Error::other(ArchiveSizeLimitExceeded {
bytes: next_len,
max_bytes: self.max_bytes,
}));
}
self.bytes.extend_from_slice(buf);
Ok(buf.len())
}
fn flush(&mut self) -> io::Result<()> {
Ok(())
}
}
#[derive(Debug)]
struct ArchiveSizeLimitExceeded {
bytes: usize,
max_bytes: usize,
}
impl fmt::Display for ArchiveSizeLimitExceeded {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"archive would be {} bytes, exceeding maximum size of {} bytes",
self.bytes, self.max_bytes
)
}
}
impl std::error::Error for ArchiveSizeLimitExceeded {}
+13 -136
View File
@@ -1,18 +1,15 @@
use super::*;
use crate::plugin_bundle_archive::PluginBundlePackError;
use crate::plugin_bundle_archive::pack_plugin_bundle_tar_gz;
use codex_login::CodexAuth;
use codex_login::default_client::build_reqwest_client;
use codex_utils_absolute_path::AbsolutePathBuf;
use flate2::Compression;
use flate2::write::GzEncoder;
use reqwest::RequestBuilder;
use reqwest::StatusCode;
use serde::Deserialize;
use serde::Serialize;
use std::collections::BTreeMap;
use std::fmt;
use std::fs;
use std::io;
use std::io::Write;
use std::path::Path;
use tracing::warn;
@@ -477,140 +474,20 @@ fn archive_plugin_for_upload_with_limit(
plugin_path: &Path,
max_bytes: usize,
) -> Result<Vec<u8>, RemotePluginCatalogError> {
if !plugin_path.is_dir() {
return Err(RemotePluginCatalogError::InvalidPluginPath {
pack_plugin_bundle_tar_gz(plugin_path, max_bytes).map_err(|err| match err {
PluginBundlePackError::InvalidPluginPath { path, reason } => {
RemotePluginCatalogError::InvalidPluginPath { path, reason }
}
PluginBundlePackError::ArchiveTooLarge { bytes, max_bytes } => {
RemotePluginCatalogError::ArchiveTooLarge { bytes, max_bytes }
}
PluginBundlePackError::Io { source } => RemotePluginCatalogError::Archive {
path: plugin_path.to_path_buf(),
reason: "expected a plugin directory".to_string(),
});
}
if !plugin_path.join(".codex-plugin/plugin.json").is_file() {
return Err(RemotePluginCatalogError::InvalidPluginPath {
path: plugin_path.to_path_buf(),
reason: "missing .codex-plugin/plugin.json".to_string(),
});
}
let encoder = GzEncoder::new(SizeLimitedBuffer::new(max_bytes), Compression::default());
let mut archive = tar::Builder::new(encoder);
append_plugin_tree(&mut archive, plugin_path, plugin_path)
.map_err(|source| archive_error(plugin_path, source))?;
let encoder = archive
.into_inner()
.map_err(|source| archive_error(plugin_path, source))?;
encoder
.finish()
.map(SizeLimitedBuffer::into_inner)
.map_err(|source| archive_error(plugin_path, source))
source,
},
})
}
fn append_plugin_tree<W: Write>(
archive: &mut tar::Builder<W>,
plugin_root: &Path,
current: &Path,
) -> io::Result<()> {
let mut entries = fs::read_dir(current)?.collect::<Result<Vec<_>, io::Error>>()?;
entries.sort_by_key(fs::DirEntry::file_name);
for entry in entries {
let path = entry.path();
let file_type = entry.file_type()?;
let relative_path = path.strip_prefix(plugin_root).map_err(|err| {
io::Error::other(format!(
"failed to compute plugin archive path for `{}`: {err}",
path.display()
))
})?;
if file_type.is_dir() {
archive.append_dir(relative_path, &path)?;
append_plugin_tree(archive, plugin_root, &path)?;
} else if file_type.is_file() {
archive.append_path_with_name(&path, relative_path)?;
} else {
return Err(io::Error::other(format!(
"unsupported plugin archive entry type: {}",
path.display()
)));
}
}
Ok(())
}
fn archive_error(plugin_path: &Path, source: io::Error) -> RemotePluginCatalogError {
if let Some(limit) = source
.get_ref()
.and_then(|err| err.downcast_ref::<ArchiveSizeLimitExceeded>())
{
return RemotePluginCatalogError::ArchiveTooLarge {
bytes: limit.bytes,
max_bytes: limit.max_bytes,
};
}
RemotePluginCatalogError::Archive {
path: plugin_path.to_path_buf(),
source,
}
}
struct SizeLimitedBuffer {
bytes: Vec<u8>,
max_bytes: usize,
}
impl SizeLimitedBuffer {
fn new(max_bytes: usize) -> Self {
Self {
bytes: Vec::new(),
max_bytes,
}
}
fn into_inner(self) -> Vec<u8> {
self.bytes
}
}
impl Write for SizeLimitedBuffer {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
let next_len = self.bytes.len().checked_add(buf.len()).ok_or_else(|| {
io::Error::other(ArchiveSizeLimitExceeded {
bytes: usize::MAX,
max_bytes: self.max_bytes,
})
})?;
if next_len > self.max_bytes {
return Err(io::Error::other(ArchiveSizeLimitExceeded {
bytes: next_len,
max_bytes: self.max_bytes,
}));
}
self.bytes.extend_from_slice(buf);
Ok(buf.len())
}
fn flush(&mut self) -> io::Result<()> {
Ok(())
}
}
#[derive(Debug)]
struct ArchiveSizeLimitExceeded {
bytes: usize,
max_bytes: usize,
}
impl fmt::Display for ArchiveSizeLimitExceeded {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"archive would be {} bytes, exceeding maximum size of {} bytes",
self.bytes, self.max_bytes
)
}
}
impl std::error::Error for ArchiveSizeLimitExceeded {}
async fn send_and_expect_status(
request: RequestBuilder,
url_for_error: &str,
@@ -326,6 +326,34 @@ fn archive_plugin_for_upload_places_manifest_at_archive_root() {
);
}
#[test]
fn archive_plugin_for_upload_round_trips_through_plugin_bundle_archive_with_long_paths() {
let temp_dir = TempDir::new().unwrap();
let plugin_path = write_test_plugin(temp_dir.path(), "demo-plugin");
let long_skill_path = Path::new("skills")
.join(["segment"; 40].join("/"))
.join("SKILL.md");
write_file(&plugin_path.join(&long_skill_path), "# Long path skill\n");
let archive_bytes = archive_plugin_for_upload(&plugin_path).unwrap();
let destination = TempDir::new().unwrap();
crate::plugin_bundle_archive::unpack_plugin_bundle_tar_gz(
&archive_bytes,
destination.path(),
/*max_total_bytes*/ 1024 * 1024,
)
.expect("extract shared plugin archive");
assert_eq!(
fs::read_to_string(destination.path().join(".codex-plugin/plugin.json")).unwrap(),
r#"{"name":"demo-plugin"}"#
);
assert_eq!(
fs::read_to_string(destination.path().join(long_skill_path)).unwrap(),
"# Long path skill\n"
);
}
#[tokio::test]
async fn save_remote_plugin_share_updates_existing_workspace_plugin() {
let codex_home = TempDir::new().unwrap();
+46 -163
View File
@@ -1,3 +1,5 @@
use crate::plugin_bundle_archive::PluginBundleUnpackError;
use crate::plugin_bundle_archive::unpack_plugin_bundle_tar_gz;
use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
use crate::store::PluginInstallResult;
use crate::store::PluginStore;
@@ -8,17 +10,14 @@ use codex_plugin::PluginId;
use codex_plugin::PluginIdError;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_plugins::find_plugin_manifest_path;
use flate2::read::GzDecoder;
use reqwest::Response;
use reqwest::StatusCode;
use serde_json::Value as JsonValue;
use std::fs;
use std::io;
use std::io::Read;
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
use tar::Archive;
use url::Host;
use url::Url;
@@ -542,146 +541,17 @@ fn extract_plugin_bundle_tar_gz_with_limits(
destination: &Path,
max_total_bytes: u64,
) -> Result<(), RemotePluginBundleInstallError> {
fs::create_dir_all(destination).map_err(|source| {
RemotePluginBundleInstallError::io(
"failed to create remote plugin bundle extraction directory",
source,
)
})?;
let archive = GzDecoder::new(std::io::Cursor::new(bytes));
let mut archive = Archive::new(archive);
extract_plugin_bundle_tar(&mut archive, destination, max_total_bytes)
}
fn extract_plugin_bundle_tar<R: Read>(
archive: &mut Archive<R>,
destination: &Path,
max_total_bytes: u64,
) -> Result<(), RemotePluginBundleInstallError> {
let mut extracted_bytes = 0u64;
let entries = archive.entries().map_err(|source| {
RemotePluginBundleInstallError::io("failed to read remote plugin bundle tar", source)
})?;
let entries = entries.raw(true);
for entry in entries {
let mut entry = entry.map_err(|source| {
RemotePluginBundleInstallError::io(
"failed to read remote plugin bundle tar entry",
source,
)
})?;
let entry_type = entry.header().entry_type();
let entry_size = entry.size();
let entry_path = entry.path().map_err(|source| {
RemotePluginBundleInstallError::io(
"failed to read remote plugin bundle tar entry path",
source,
)
})?;
let entry_path = entry_path.into_owned();
let output_path = checked_tar_output_path(destination, &entry_path)?;
if entry_type.is_dir() {
fs::create_dir_all(&output_path).map_err(|source| {
RemotePluginBundleInstallError::io(
"failed to create remote plugin bundle directory",
source,
)
})?;
continue;
unpack_plugin_bundle_tar_gz(bytes, destination, max_total_bytes).map_err(|err| match err {
PluginBundleUnpackError::ExtractedBundleTooLarge { bytes, max_bytes } => {
RemotePluginBundleInstallError::ExtractedBundleTooLarge { bytes, max_bytes }
}
if entry_type.is_file() {
enforce_total_extracted_size(entry_size, &mut extracted_bytes, max_total_bytes)?;
let Some(parent) = output_path.parent() else {
return Err(RemotePluginBundleInstallError::InvalidBundle(format!(
"remote plugin bundle output path has no parent: {}",
output_path.display()
)));
};
fs::create_dir_all(parent).map_err(|source| {
RemotePluginBundleInstallError::io(
"failed to create remote plugin bundle directory",
source,
)
})?;
entry.unpack(&output_path).map_err(|source| {
RemotePluginBundleInstallError::io(
"failed to unpack remote plugin bundle entry",
source,
)
})?;
continue;
PluginBundleUnpackError::Io { context, source } => {
RemotePluginBundleInstallError::io(context, source)
}
if entry_type.is_hard_link() || entry_type.is_symlink() {
return Err(RemotePluginBundleInstallError::InvalidBundle(format!(
"remote plugin bundle tar entry `{}` is a link",
entry_path.display()
)));
PluginBundleUnpackError::InvalidBundle(message) => {
RemotePluginBundleInstallError::InvalidBundle(message)
}
return Err(RemotePluginBundleInstallError::InvalidBundle(format!(
"remote plugin bundle tar entry `{}` has unsupported type {:?}",
entry_path.display(),
entry_type
)));
}
Ok(())
}
fn checked_tar_output_path(
destination: &Path,
entry_name: &Path,
) -> Result<PathBuf, RemotePluginBundleInstallError> {
let mut output_path = destination.to_path_buf();
let mut has_component = false;
for component in entry_name.components() {
match component {
std::path::Component::Normal(component) => {
has_component = true;
output_path.push(component);
}
std::path::Component::CurDir => {}
std::path::Component::ParentDir
| std::path::Component::RootDir
| std::path::Component::Prefix(_) => {
return Err(RemotePluginBundleInstallError::InvalidBundle(format!(
"remote plugin bundle tar entry `{}` escapes extraction root",
entry_name.display()
)));
}
}
}
if !has_component {
return Err(RemotePluginBundleInstallError::InvalidBundle(
"remote plugin bundle tar entry has an empty path".to_string(),
));
}
Ok(output_path)
}
fn enforce_total_extracted_size(
entry_size: u64,
extracted_bytes: &mut u64,
max_total_bytes: u64,
) -> Result<(), RemotePluginBundleInstallError> {
let next_total = extracted_bytes.checked_add(entry_size).ok_or(
RemotePluginBundleInstallError::ExtractedBundleTooLarge {
bytes: u64::MAX,
max_bytes: max_total_bytes,
},
)?;
if next_total > max_total_bytes {
return Err(RemotePluginBundleInstallError::ExtractedBundleTooLarge {
bytes: next_total,
max_bytes: max_total_bytes,
});
}
*extracted_bytes = next_total;
Ok(())
})
}
fn find_extracted_plugin_root(
@@ -706,6 +576,7 @@ mod tests {
use flate2::Compression;
use flate2::write::GzEncoder;
use pretty_assertions::assert_eq;
use std::io::Write;
use tempfile::tempdir;
const REMOTE_PLUGIN_ID: &str = "plugins~Plugin_00000000000000000000000000000000";
@@ -830,7 +701,7 @@ mod tests {
)
.expect_err("invalid tar.gz should be rejected");
assert!(format!("{err}").contains("failed to read remote plugin bundle tar"));
assert!(format!("{err}").contains("failed to read plugin bundle tar"));
}
#[test]
@@ -961,8 +832,11 @@ mod tests {
#[test]
fn extraction_rejects_tar_path_traversal() {
let destination = tempdir().expect("tempdir");
let err = checked_tar_output_path(destination.path(), Path::new("../evil.txt"))
.expect_err("tar path traversal should be rejected");
let err = extract_plugin_bundle_tar_gz(
&tar_gz_bytes_with_raw_path("../evil.txt", b"evil", /*mode*/ 0o644),
destination.path(),
)
.expect_err("tar path traversal should be rejected");
assert!(format!("{err}").contains("escapes extraction root"));
}
@@ -987,20 +861,20 @@ mod tests {
}
#[test]
fn extraction_rejects_pax_metadata_entries() {
fn extraction_supports_gnu_long_name_entries() {
let destination = tempdir().expect("tempdir");
let err = extract_plugin_bundle_tar_gz(
&tar_gz_bytes_with_entry_type(
tar::EntryType::XHeader,
"PaxHeaders.0/linear",
b"18 path=linear\n",
/*mode*/ 0o644,
),
let long_path = format!("{}/file.txt", ["segment"; 40].join("/"));
extract_plugin_bundle_tar_gz(
&tar_gz_bytes(&[(long_path.as_str(), b"long", /*mode*/ 0o644)]),
destination.path(),
)
.expect_err("pax metadata entries should be rejected");
.expect("extract bundle with GNU long name entry");
assert!(format!("{err}").contains("unsupported type"));
assert_eq!(
std::fs::read(destination.path().join(long_path)).expect("read extracted file"),
b"long"
);
}
#[cfg(unix)]
@@ -1051,16 +925,25 @@ mod tests {
finish_tar_gz(tar)
}
fn tar_gz_bytes_with_entry_type(
entry_type: tar::EntryType,
path: &str,
contents: &[u8],
mode: u32,
) -> Vec<u8> {
let encoder = GzEncoder::new(Vec::new(), Compression::default());
let mut tar = tar::Builder::new(encoder);
append_tar_entry(&mut tar, entry_type, path, contents, mode);
finish_tar_gz(tar)
fn tar_gz_bytes_with_raw_path(path: &str, contents: &[u8], mode: u32) -> Vec<u8> {
let mut header = tar::Header::new_gnu();
header.set_entry_type(tar::EntryType::Regular);
header.set_size(contents.len() as u64);
header.set_mode(mode);
header.as_mut_bytes()[..path.len()].copy_from_slice(path.as_bytes());
header.set_cksum();
let mut encoder = GzEncoder::new(Vec::new(), Compression::default());
encoder
.write_all(header.as_bytes())
.expect("write tar header");
encoder.write_all(contents).expect("write tar contents");
let padding = (512 - (contents.len() % 512)) % 512;
encoder
.write_all(&vec![0; padding])
.expect("write tar padding");
encoder.write_all(&[0; 1024]).expect("write tar terminator");
encoder.finish().expect("finish gzip")
}
fn append_tar_entry<W: std::io::Write>(
+1
View File
@@ -47,6 +47,7 @@ codex-shell-command = { workspace = true }
codex-execpolicy = { workspace = true }
codex-git-utils = { workspace = true }
codex-hooks = { workspace = true }
codex-install-context = { workspace = true }
codex-network-proxy = { workspace = true }
codex-otel = { workspace = true }
codex-plugin = { workspace = true }
-16
View File
@@ -711,14 +711,6 @@
}
],
"default": null
},
"zsh_path": {
"allOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
}
],
"description": "Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution."
}
},
"type": "object"
@@ -4986,14 +4978,6 @@
],
"default": null,
"description": "Windows-specific configuration."
},
"zsh_path": {
"allOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
}
],
"description": "Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution."
}
},
"title": "ConfigToml",
+1 -1
View File
@@ -938,7 +938,7 @@ impl Drop for ModelClientSession {
}
impl ModelClientSession {
pub(crate) fn reset_websocket_session(&mut self) {
fn reset_websocket_session(&mut self) {
self.websocket_session.connection = None;
self.websocket_session.last_request = None;
self.websocket_session.last_response_rx = None;
+19
View File
@@ -4470,6 +4470,25 @@ async fn add_dir_override_extends_workspace_writable_roots() -> std::io::Result<
Ok(())
}
#[tokio::test]
async fn default_zsh_path_sets_runtime_zsh_path() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let default_zsh_path = codex_home.path().join("packaged-zsh");
let config = Config::load_from_base_config_with_overrides(
ConfigToml::default(),
ConfigOverrides {
default_zsh_path: Some(default_zsh_path.abs()),
..Default::default()
},
codex_home.abs(),
)
.await?;
assert_eq!(config.zsh_path, Some(default_zsh_path));
Ok(())
}
#[tokio::test]
async fn sqlite_home_defaults_to_codex_home_for_workspace_write() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
+13 -3
View File
@@ -67,6 +67,7 @@ use codex_features::FeaturesToml;
use codex_features::MultiAgentV2ConfigToml;
use codex_features::NetworkProxyConfigToml;
use codex_git_utils::resolve_root_git_project_for_trust;
use codex_install_context::InstallContext;
use codex_login::AuthManagerConfig;
use codex_mcp::McpConfig;
use codex_memories_read::memory_root;
@@ -1392,11 +1393,18 @@ impl Config {
.effective_config()
.try_into()
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?;
let default_zsh_path = refreshed_config
.zsh_path
.clone()
.map(AbsolutePathBuf::try_from)
.transpose()?;
Self::load_config_with_layer_stack(
LOCAL_FS.as_ref(),
cfg,
ConfigOverrides {
cwd: Some(self.cwd.to_path_buf()),
default_zsh_path,
..Default::default()
},
refreshed_config.codex_home.clone(),
@@ -2128,7 +2136,7 @@ pub struct ConfigOverrides {
pub codex_self_exe: Option<PathBuf>,
pub codex_linux_sandbox_exe: Option<PathBuf>,
pub main_execve_wrapper_exe: Option<PathBuf>,
pub zsh_path: Option<PathBuf>,
pub default_zsh_path: Option<AbsolutePathBuf>,
pub base_instructions: Option<String>,
pub developer_instructions: Option<String>,
pub personality: Option<Personality>,
@@ -2480,7 +2488,7 @@ impl Config {
codex_self_exe,
codex_linux_sandbox_exe,
main_execve_wrapper_exe,
zsh_path: zsh_path_override,
default_zsh_path,
base_instructions,
developer_instructions,
personality,
@@ -3199,7 +3207,9 @@ impl Config {
)
.await?;
let compact_prompt = compact_prompt.or(file_compact_prompt);
let zsh_path = zsh_path_override.or(cfg.zsh_path.map(Into::into));
let zsh_path = default_zsh_path
.or_else(|| InstallContext::current().bundled_zsh_path())
.map(AbsolutePathBuf::into_path_buf);
let review_model = override_review_model.or(cfg.review_model);
@@ -82,7 +82,7 @@ async fn restricted_read_implicitly_allows_helper_executables() -> std::io::Resu
},
ConfigOverrides {
cwd: Some(cwd.clone()),
zsh_path: Some(zsh_path.clone()),
default_zsh_path: Some(AbsolutePathBuf::try_from(zsh_path.clone())?),
main_execve_wrapper_exe: Some(execve_wrapper),
..Default::default()
},
+2 -2
View File
@@ -826,13 +826,13 @@ impl Session {
} else if use_zsh_fork_shell {
let zsh_path = config.zsh_path.as_ref().ok_or_else(|| {
anyhow::anyhow!(
"zsh fork feature enabled, but `zsh_path` is not configured; set `zsh_path` in config.toml"
"zsh fork feature enabled, but no packaged zsh fork is available for this install"
)
})?;
let zsh_path = zsh_path.to_path_buf();
shell::get_shell(shell::ShellType::Zsh, Some(&zsh_path)).ok_or_else(|| {
anyhow::anyhow!(
"zsh fork feature enabled, but zsh_path `{}` is not usable; set `zsh_path` to a valid zsh executable",
"zsh fork feature enabled, but packaged zsh fork `{}` is not usable",
zsh_path.display()
)
})?
+2 -2
View File
@@ -4319,7 +4319,7 @@ async fn absolute_cwd_update_with_turn_environment_is_allowed() {
}
#[tokio::test]
async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
let codex_home = tempfile::tempdir().expect("create temp dir");
let mut config = build_test_config(codex_home.path()).await;
config
@@ -4420,7 +4420,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
Err(err) => err,
};
let msg = format!("{err:#}");
assert!(msg.contains("zsh fork feature enabled, but `zsh_path` is not configured"));
assert!(msg.contains("zsh fork feature enabled, but no packaged zsh fork is available"));
}
// todo: use online model info
+62 -79
View File
@@ -150,7 +150,7 @@ pub(crate) async fn run_turn(
// new user message are recorded. Estimate pending incoming items (context
// diffs/full reinjection + user input) and trigger compaction preemptively
// when they would push the thread over the compaction threshold.
let pre_sampling_compact = match run_pre_sampling_compact(
if let Err(err) = run_pre_sampling_compact(
&sess,
&turn_context,
&mut client_session,
@@ -158,24 +158,25 @@ pub(crate) async fn run_turn(
)
.await
{
Ok(pre_sampling_compact) => pre_sampling_compact,
Err(AutoCompactRunError::NoProgressLimitReached) => return RunTurnResult::Terminal,
Err(AutoCompactRunError::Compact(err)) => {
if err.to_codex_protocol_error() == CodexErrorInfo::UsageLimitExceeded
&& let Err(err) = sess
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
turn_context: turn_context.as_ref(),
})
.await
{
warn!("failed to usage-limit active goal after usage-limit error: {err}");
return match err {
AutoCompactRunError::NoProgressLimitReached => {
error!("Failed to run pre-sampling compact");
RunTurnResult::Terminal
}
error!("Failed to run pre-sampling compact");
return RunTurnResult::Continue(None);
}
};
if pre_sampling_compact.reset_client_session {
client_session.reset_websocket_session();
AutoCompactRunError::Compact(err) => {
if err.to_codex_protocol_error() == CodexErrorInfo::UsageLimitExceeded
&& let Err(err) = sess
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
turn_context: turn_context.as_ref(),
})
.await
{
warn!("failed to usage-limit active goal after usage-limit error: {err}");
}
error!("Failed to run pre-sampling compact");
RunTurnResult::Continue(None)
}
};
}
sess.record_context_updates_and_set_reference_context_item(turn_context.as_ref())
@@ -314,7 +315,7 @@ pub(crate) async fn run_turn(
// as long as compaction works well in getting us way below the token limit, we shouldn't worry about being in an infinite loop.
if token_limit_reached && needs_follow_up {
let reset_client_session = match run_auto_compact(
if let Err(err) = run_auto_compact(
&sess,
&turn_context,
&mut client_session,
@@ -325,27 +326,26 @@ pub(crate) async fn run_turn(
)
.await
{
Ok(reset_client_session) => reset_client_session,
Err(AutoCompactRunError::NoProgressLimitReached) => {
return RunTurnResult::Terminal;
}
Err(AutoCompactRunError::Compact(err)) => {
if err.to_codex_protocol_error() == CodexErrorInfo::UsageLimitExceeded
&& let Err(err) = sess
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
turn_context: turn_context.as_ref(),
})
.await
{
warn!(
"failed to usage-limit active goal after usage-limit error: {err}"
);
match err {
AutoCompactRunError::NoProgressLimitReached => {
return RunTurnResult::Terminal;
}
AutoCompactRunError::Compact(err) => {
if err.to_codex_protocol_error()
== CodexErrorInfo::UsageLimitExceeded
&& let Err(err) = sess
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
turn_context: turn_context.as_ref(),
})
.await
{
warn!(
"failed to usage-limit active goal after usage-limit error: {err}"
);
}
return RunTurnResult::Continue(None);
}
return RunTurnResult::Continue(None);
}
};
if reset_client_session {
client_session.reset_websocket_session();
}
can_drain_pending_input = !model_needs_follow_up;
continue;
@@ -680,10 +680,6 @@ async fn track_turn_resolved_config_analytics(
});
}
struct PreSamplingCompactResult {
reset_client_session: bool,
}
#[derive(Debug)]
struct AutoCompactTokenStatus {
// Full active context usage, independent of the configured auto-compact scope.
@@ -827,19 +823,18 @@ async fn run_pre_sampling_compact(
turn_context: &Arc<TurnContext>,
client_session: &mut ModelClientSession,
auto_compact_limiter: &mut AutoCompactTurnLimiter,
) -> Result<PreSamplingCompactResult, AutoCompactRunError> {
let mut pre_sampling_compacted = maybe_run_previous_model_inline_compact(
) -> Result<(), AutoCompactRunError> {
maybe_run_previous_model_inline_compact(
sess,
turn_context,
client_session,
auto_compact_limiter,
)
.await?;
let mut reset_client_session = pre_sampling_compacted;
let token_status = auto_compact_token_status(sess.as_ref(), turn_context.as_ref()).await;
// Compact if the configured auto-compaction budget or usable context window is exhausted.
if token_status.token_limit_reached {
reset_client_session |= run_auto_compact(
run_auto_compact(
sess,
turn_context,
client_session,
@@ -849,27 +844,22 @@ async fn run_pre_sampling_compact(
CompactionPhase::PreTurn,
)
.await?;
pre_sampling_compacted = true;
}
Ok(PreSamplingCompactResult {
reset_client_session: pre_sampling_compacted && reset_client_session,
})
Ok(())
}
/// Runs pre-sampling compaction against the previous model when switching to a smaller
/// context-window model.
///
/// Returns `Ok(true)` when compaction ran successfully, `Ok(false)` when compaction was skipped
/// because the model/context-window preconditions were not met, and `Err(_)` only when compaction
/// was attempted and failed.
/// Returns `Err(_)` only when compaction was attempted and failed.
async fn maybe_run_previous_model_inline_compact(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
client_session: &mut ModelClientSession,
auto_compact_limiter: &mut AutoCompactTurnLimiter,
) -> Result<bool, AutoCompactRunError> {
) -> Result<(), AutoCompactRunError> {
let Some(previous_turn_settings) = sess.previous_turn_settings().await else {
return Ok(false);
return Ok(());
};
let previous_model_turn_context = Arc::new(
turn_context
@@ -878,10 +868,10 @@ async fn maybe_run_previous_model_inline_compact(
);
let Some(old_context_window) = previous_model_turn_context.model_context_window() else {
return Ok(false);
return Ok(());
};
let Some(new_context_window) = turn_context.model_context_window() else {
return Ok(false);
return Ok(());
};
let active_context_tokens = sess.get_total_token_usage().await;
let previous_model_limit_reached = match turn_context
@@ -902,7 +892,7 @@ async fn maybe_run_previous_model_inline_compact(
&& previous_model_turn_context.model_info.slug != turn_context.model_info.slug
&& old_context_window > new_context_window;
if should_run {
let _ = run_auto_compact(
run_auto_compact(
sess,
&previous_model_turn_context,
client_session,
@@ -912,9 +902,8 @@ async fn maybe_run_previous_model_inline_compact(
CompactionPhase::PreTurn,
)
.await?;
return Ok(true);
}
Ok(false)
Ok(())
}
async fn run_auto_compact(
@@ -925,12 +914,11 @@ async fn run_auto_compact(
initial_context_injection: InitialContextInjection,
reason: CompactionReason,
phase: CompactionPhase,
) -> Result<bool, AutoCompactRunError> {
) -> Result<(), AutoCompactRunError> {
auto_compact_limiter
.ensure_can_dispatch(sess.as_ref(), turn_context.as_ref(), reason, phase)
.await?;
let reset_client_session = if should_use_remote_compact_task(turn_context.provider.info()) {
if should_use_remote_compact_task(turn_context.provider.info()) {
if turn_context.features.enabled(Feature::RemoteCompactionV2) {
run_inline_remote_auto_compact_task_v2(
Arc::clone(sess),
@@ -941,18 +929,16 @@ async fn run_auto_compact(
phase,
)
.await?;
false
} else {
run_inline_remote_auto_compact_task(
Arc::clone(sess),
Arc::clone(turn_context),
initial_context_injection,
reason,
phase,
)
.await?;
true
return Ok(());
}
run_inline_remote_auto_compact_task(
Arc::clone(sess),
Arc::clone(turn_context),
initial_context_injection,
reason,
phase,
)
.await?;
} else {
run_inline_auto_compact_task(
Arc::clone(sess),
@@ -962,13 +948,10 @@ async fn run_auto_compact(
phase,
)
.await?;
true
};
}
let token_status = auto_compact_token_status(sess.as_ref(), turn_context.as_ref()).await;
auto_compact_limiter.record_successful_compaction(&token_status);
Ok(reset_client_session)
Ok(())
}
pub(super) fn collect_explicit_app_ids_from_skill_items(
@@ -38,12 +38,6 @@ impl CodeModeExecuteHandler {
let exec = ExecContext { session, turn };
let enabled_tools =
codex_tools::collect_code_mode_tool_definitions(&self.nested_tool_specs);
let stored_values = exec
.session
.services
.code_mode_service
.stored_values()
.await;
// Allocate before starting V8 so the trace can create the parent
// CodeCell before model-authored JavaScript issues nested tool calls.
let runtime_cell_id = exec.session.services.code_mode_service.allocate_cell_id();
@@ -67,7 +61,6 @@ impl CodeModeExecuteHandler {
tool_call_id: call_id,
enabled_tools,
source: args.code,
stored_values,
yield_time_ms: args.yield_time_ms,
max_output_tokens: args.max_output_tokens,
})
-17
View File
@@ -66,17 +66,6 @@ impl CodeModeService {
}
}
pub(crate) async fn stored_values(&self) -> std::collections::HashMap<String, JsonValue> {
self.inner.stored_values().await
}
pub(crate) async fn replace_stored_values(
&self,
values: std::collections::HashMap<String, JsonValue>,
) {
self.inner.replace_stored_values(values).await;
}
pub(crate) fn allocate_cell_id(&self) -> String {
self.inner.allocate_cell_id()
}
@@ -182,17 +171,11 @@ pub(super) async fn handle_runtime_response(
}
RuntimeResponse::Result {
content_items,
stored_values,
error_text,
..
} => {
let mut content_items = into_function_call_output_content_items(content_items);
sanitize_runtime_image_detail(exec.turn.as_ref(), &mut content_items);
exec.session
.services
.code_mode_service
.replace_stored_values(stored_values)
.await;
let success = error_text.is_none();
if let Some(error_text) = error_text {
content_items.push(FunctionCallOutputContentItem::InputText {
@@ -2,9 +2,12 @@ use serde::Deserialize;
use crate::function_tool::FunctionCallError;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolExecutor;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
@@ -110,4 +113,22 @@ impl ToolExecutor<ToolInvocation> for CodeModeWaitHandler {
}
}
impl CoreToolRuntime for CodeModeWaitHandler {}
impl CoreToolRuntime for CodeModeWaitHandler {
fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
// Code-mode `wait` is runtime control for an existing code cell, not a
// standalone user action. Tool calls made from code mode still flow
// through normal dispatch, but hooks should not block or rewrite the
// wait loop itself.
None
}
fn post_tool_use_payload(
&self,
_invocation: &ToolInvocation,
_result: &dyn ToolOutput,
) -> Option<PostToolUsePayload> {
// The wait result feeds code-mode control flow, so do not let
// PostToolUse replace it with model-facing hook feedback.
None
}
}
@@ -4,17 +4,12 @@ use codex_tools::ConversationHistory;
use codex_tools::ToolCall as ExtensionToolCall;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use serde_json::Value;
use crate::function_tool::FunctionCallError;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::flat_tool_name;
use crate::tools::hook_names::HookToolName;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolExecutor;
pub(crate) struct ExtensionToolAdapter(Arc<dyn codex_tools::ToolExecutor<ExtensionToolCall>>);
@@ -23,13 +18,6 @@ impl ExtensionToolAdapter {
pub(crate) fn new(executor: Arc<dyn codex_tools::ToolExecutor<ExtensionToolCall>>) -> Self {
Self(executor)
}
fn arguments_from_payload<'a>(&self, payload: &'a ToolPayload) -> Option<&'a str> {
let ToolPayload::Function { arguments } = payload else {
return None;
};
Some(arguments)
}
}
#[async_trait::async_trait]
@@ -60,30 +48,7 @@ impl ToolExecutor<ToolInvocation> for ExtensionToolAdapter {
impl CoreToolRuntime for ExtensionToolAdapter {
fn matches_kind(&self, payload: &ToolPayload) -> bool {
self.arguments_from_payload(payload).is_some()
}
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
let arguments = self.arguments_from_payload(&invocation.payload)?;
Some(PreToolUsePayload {
tool_name: HookToolName::new(flat_tool_name(&self.tool_name()).into_owned()),
tool_input: extension_tool_hook_input(arguments),
})
}
fn post_tool_use_payload(
&self,
invocation: &ToolInvocation,
result: &dyn ToolOutput,
) -> Option<PostToolUsePayload> {
let arguments = self.arguments_from_payload(&invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::new(flat_tool_name(&self.tool_name()).into_owned()),
tool_use_id: invocation.call_id.clone(),
tool_input: extension_tool_hook_input(arguments),
tool_response: result
.post_tool_use_response(&invocation.call_id, &invocation.payload)?,
})
matches!(payload, ToolPayload::Function { .. })
}
}
@@ -100,14 +65,6 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
}
}
fn extension_tool_hook_input(arguments: &str) -> Value {
if arguments.trim().is_empty() {
return Value::Object(serde_json::Map::new());
}
serde_json::from_str(arguments).unwrap_or_else(|_| Value::String(arguments.to_string()))
}
#[cfg(test)]
mod tests {
use std::sync::Arc;
+3 -70
View File
@@ -9,10 +9,7 @@ use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::flat_tool_name;
use crate::tools::hook_names::HookToolName;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolExecutor;
use crate::tools::registry::ToolTelemetryTags;
use crate::tools::tool_search_entry::ToolSearchInfo;
@@ -23,8 +20,6 @@ use codex_tools::ToolName;
use codex_tools::ToolSearchSourceInfo;
use codex_tools::ToolSpec;
use codex_tools::mcp_tool_to_responses_api_tool;
use serde_json::Map;
use serde_json::Value;
pub struct McpHandler {
tool_info: ToolInfo,
@@ -143,58 +138,6 @@ impl CoreToolRuntime for McpHandler {
tags
})
}
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
let ToolPayload::Function { arguments } = &invocation.payload else {
return None;
};
Some(PreToolUsePayload {
tool_name: HookToolName::new(self.tool_name().to_string()),
tool_input: mcp_hook_tool_input(arguments),
})
}
fn with_updated_hook_input(
&self,
mut invocation: ToolInvocation,
updated_input: Value,
) -> Result<ToolInvocation, FunctionCallError> {
invocation.payload = match invocation.payload {
ToolPayload::Function { .. } => ToolPayload::Function {
arguments: serde_json::to_string(&updated_input).map_err(|err| {
FunctionCallError::RespondToModel(format!(
"failed to serialize rewritten MCP arguments: {err}"
))
})?,
},
payload => {
return Err(FunctionCallError::RespondToModel(format!(
"tool {} does not support hook input rewriting for payload {payload:?}",
self.tool_name()
)));
}
};
Ok(invocation)
}
fn post_tool_use_payload(
&self,
invocation: &ToolInvocation,
result: &dyn crate::tools::context::ToolOutput,
) -> Option<PostToolUsePayload> {
let ToolPayload::Function { .. } = &invocation.payload else {
return None;
};
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::new(self.tool_name().to_string()),
tool_use_id: invocation.call_id.clone(),
tool_input: result.post_tool_use_input(&invocation.payload)?,
tool_response,
})
}
}
fn create_tool_spec(tool_info: &ToolInfo) -> Result<ToolSpec, serde_json::Error> {
@@ -223,14 +166,6 @@ fn create_tool_spec(tool_info: &ToolInfo) -> Result<ToolSpec, serde_json::Error>
}))
}
fn mcp_hook_tool_input(raw_arguments: &str) -> Value {
if raw_arguments.trim().is_empty() {
return Value::Object(Map::new());
}
serde_json::from_str(raw_arguments).unwrap_or_else(|_| Value::String(raw_arguments.to_string()))
}
fn build_mcp_search_text(info: &ToolInfo) -> String {
let tool_name = info.canonical_tool_name();
let mut schema_properties = info
@@ -288,6 +223,9 @@ mod tests {
use super::*;
use crate::session::tests::make_session_and_context;
use crate::tools::context::ToolCallSource;
use crate::tools::hook_names::HookToolName;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::turn_diff_tracker::TurnDiffTracker;
use pretty_assertions::assert_eq;
use serde_json::json;
@@ -447,11 +385,6 @@ mod tests {
);
}
#[test]
fn mcp_hook_tool_input_defaults_empty_args_to_object() {
assert_eq!(mcp_hook_tool_input(" "), json!({}));
}
#[test]
fn mcp_read_only_hint_supports_parallel_calls_without_server_opt_in() {
let mut read_only_info = tool_info("foo", "mcp__foo__", "read");
@@ -5,6 +5,7 @@ use crate::tools::context::boxed_tool_output;
use crate::tools::handlers::parse_arguments;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolExecutor;
use crate::unified_exec::WriteStdinRequest;
use codex_protocol::protocol::EventMsg;
@@ -101,11 +102,20 @@ impl CoreToolRuntime for WriteStdinHandler {
matches!(payload, ToolPayload::Function { .. })
}
fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
// `write_stdin` is transport for an existing exec session. Empty writes
// are background polls, and non-empty writes continue a command that
// already ran PreToolUse as Bash, so do not emit a second pre hook here.
None
}
fn post_tool_use_payload(
&self,
invocation: &ToolInvocation,
result: &dyn crate::tools::context::ToolOutput,
) -> Option<PostToolUsePayload> {
// A `write_stdin` poll can observe final completion for the original
// `exec_command`; emit that command's matching Bash PostToolUse.
post_unified_exec_tool_use_payload(invocation, result)
}
}
+12
View File
@@ -38,6 +38,18 @@ impl HookToolName {
}
}
/// Returns the hook identity for spawning sub-agents.
///
/// The serialized name remains `spawn_agent`, while `Agent` is accepted as
/// a matcher alias for compatibility with hook configurations that describe
/// sub-agent creation using Claude Code-style names.
pub(crate) fn spawn_agent() -> Self {
Self {
name: "spawn_agent".to_string(),
matcher_aliases: vec!["Agent".to_string()],
}
}
/// Returns the hook identity historically used for shell-like tools.
pub(crate) fn bash() -> Self {
Self::new("Bash")
+111 -15
View File
@@ -19,6 +19,7 @@ use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::flat_tool_name;
use crate::tools::handlers::multi_agents_spec::MULTI_AGENT_V1_NAMESPACE;
use crate::tools::hook_names::HookToolName;
use crate::tools::lifecycle::notify_tool_finish;
use crate::tools::lifecycle::notify_tool_start;
@@ -26,6 +27,7 @@ use crate::tools::tool_dispatch_trace::ToolDispatchTrace;
use crate::tools::tool_search_entry::ToolSearchInfo;
use crate::util::error_or_panic;
use codex_extension_api::ToolCallOutcome;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::protocol::EventMsg;
use codex_tools::ToolName;
@@ -64,14 +66,47 @@ pub(crate) trait CoreToolRuntime: ToolExecutor<ToolInvocation> {
fn post_tool_use_payload(
&self,
_invocation: &ToolInvocation,
_result: &dyn ToolOutput,
invocation: &ToolInvocation,
result: &dyn ToolOutput,
) -> Option<PostToolUsePayload> {
None
let ToolPayload::Function { arguments } = &invocation.payload else {
return None;
};
Some(PostToolUsePayload {
tool_name: function_hook_tool_name(invocation),
tool_use_id: result.post_tool_use_id(&invocation.call_id),
tool_input: result
.post_tool_use_input(&invocation.payload)
.unwrap_or_else(|| function_hook_tool_input(arguments)),
tool_response: result
.post_tool_use_response(&invocation.call_id, &invocation.payload)
.or_else(|| {
// Most function tools can expose their model-facing output
// as the hook response. Outputs with a more stable hook
// contract should override post_tool_use_response above.
let ResponseInputItem::FunctionCallOutput {
output: FunctionCallOutputPayload { body, .. },
..
} = result.to_response_item(&invocation.call_id, &invocation.payload)
else {
return None;
};
serde_json::to_value(body).ok()
})?,
})
}
fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
None
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
let ToolPayload::Function { arguments } = &invocation.payload else {
return None;
};
Some(PreToolUsePayload {
tool_name: function_hook_tool_name(invocation),
tool_input: function_hook_tool_input(arguments),
})
}
/// Rebuilds a tool invocation from hook-facing `tool_input`.
@@ -80,12 +115,25 @@ pub(crate) trait CoreToolRuntime: ToolExecutor<ToolInvocation> {
/// hook contract they expose from `pre_tool_use_payload`.
fn with_updated_hook_input(
&self,
_invocation: ToolInvocation,
_updated_input: Value,
invocation: ToolInvocation,
updated_input: Value,
) -> Result<ToolInvocation, FunctionCallError> {
Err(FunctionCallError::RespondToModel(
"tool does not support hook input rewriting".to_string(),
))
let ToolPayload::Function { .. } = &invocation.payload else {
return Err(FunctionCallError::RespondToModel(
"hook input rewrite received unsupported function tool payload".to_string(),
));
};
let arguments = serde_json::to_string(&updated_input).map_err(|err| {
FunctionCallError::RespondToModel(format!(
"failed to serialize rewritten {} arguments: {err}",
flat_tool_name(&invocation.tool_name)
))
})?;
Ok(ToolInvocation {
payload: ToolPayload::Function { arguments },
..invocation
})
}
/// Creates an optional consumer for streamed tool argument diffs.
@@ -133,6 +181,29 @@ impl AnyToolResult {
}
}
struct PostToolUseFeedbackOutput {
original: Box<dyn ToolOutput>,
model_visible: FunctionToolOutput,
}
impl ToolOutput for PostToolUseFeedbackOutput {
fn log_preview(&self) -> String {
self.original.log_preview()
}
fn success_for_logging(&self) -> bool {
self.original.success_for_logging()
}
fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem {
self.model_visible.to_response_item(call_id, payload)
}
fn code_mode_result(&self, payload: &ToolPayload) -> Value {
self.original.code_mode_result(payload)
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct PreToolUsePayload {
/// Hook-facing tool name model.
@@ -538,11 +609,15 @@ impl ToolRegistry {
};
if let Some(replacement_text) = replacement_text {
let mut guard = response_cell.lock().await;
if let Some(result) = guard.as_mut() {
result.result = Box::new(FunctionToolOutput::from_text(
replacement_text,
/*success*/ None,
));
if let Some(mut result) = guard.take() {
result.result = Box::new(PostToolUseFeedbackOutput {
original: result.result,
model_visible: FunctionToolOutput::from_text(
replacement_text,
/*success*/ None,
),
});
*guard = Some(result);
}
}
}
@@ -618,6 +693,27 @@ async fn handle_any_tool(
})
}
fn function_hook_tool_name(invocation: &ToolInvocation) -> HookToolName {
if invocation.tool_name.name == "spawn_agent"
&& matches!(
invocation.tool_name.namespace.as_deref(),
None | Some(MULTI_AGENT_V1_NAMESPACE)
)
{
return HookToolName::spawn_agent();
}
HookToolName::new(flat_tool_name(&invocation.tool_name).into_owned())
}
fn function_hook_tool_input(arguments: &str) -> Value {
if arguments.trim().is_empty() {
return Value::Object(serde_json::Map::new());
}
serde_json::from_str(arguments).unwrap_or_else(|_| Value::String(arguments.to_string()))
}
fn unsupported_tool_call_message(payload: &ToolPayload, tool_name: &ToolName) -> String {
match payload {
ToolPayload::Custom { .. } => format!("unsupported custom tool call: {tool_name}"),
+192
View File
@@ -172,6 +172,198 @@ fn handler_looks_up_namespaced_aliases_explicitly() {
);
}
#[tokio::test]
async fn function_tools_expose_default_hook_payloads_and_rewrites() -> anyhow::Result<()> {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let tool_name = codex_tools::ToolName::namespaced("functions.", "echo");
let handler = TestHandler {
tool_name: tool_name.clone(),
};
let invocation = ToolInvocation {
payload: ToolPayload::Function {
arguments: serde_json::json!({ "message": "hello" }).to_string(),
},
..test_invocation(Arc::new(session), Arc::new(turn), "call-1", tool_name)
};
let output =
crate::tools::context::FunctionToolOutput::from_text("echoed".to_string(), Some(true));
assert_eq!(
handler.pre_tool_use_payload(&invocation),
Some(PreToolUsePayload {
tool_name: HookToolName::new("functions.echo"),
tool_input: serde_json::json!({ "message": "hello" }),
})
);
assert_eq!(
handler.post_tool_use_payload(&invocation, &output),
Some(PostToolUsePayload {
tool_name: HookToolName::new("functions.echo"),
tool_use_id: "call-1".to_string(),
tool_input: serde_json::json!({ "message": "hello" }),
tool_response: serde_json::json!("echoed"),
})
);
let invocation = handler
.with_updated_hook_input(invocation, serde_json::json!({ "message": "rewritten" }))?;
let ToolPayload::Function { arguments } = invocation.payload else {
panic!("generic rewritten function payload should remain function-shaped");
};
assert_eq!(
serde_json::from_str::<serde_json::Value>(&arguments)?,
serde_json::json!({ "message": "rewritten" })
);
Ok(())
}
#[tokio::test]
async fn function_hook_input_defaults_empty_arguments_to_object() {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let tool_name = codex_tools::ToolName::plain("echo");
let handler = TestHandler {
tool_name: tool_name.clone(),
};
let invocation = ToolInvocation {
payload: ToolPayload::Function {
arguments: " ".to_string(),
},
..test_invocation(Arc::new(session), Arc::new(turn), "call-1", tool_name)
};
assert_eq!(
handler.pre_tool_use_payload(&invocation),
Some(PreToolUsePayload {
tool_name: HookToolName::new("echo"),
tool_input: serde_json::json!({}),
})
);
}
#[tokio::test]
async fn spawn_agent_function_tools_use_agent_matcher_alias() {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let session = Arc::new(session);
let turn = Arc::new(turn);
let hook_payloads = [
codex_tools::ToolName::plain("spawn_agent"),
codex_tools::ToolName::namespaced(MULTI_AGENT_V1_NAMESPACE, "spawn_agent"),
]
.into_iter()
.map(|tool_name| {
let handler = TestHandler {
tool_name: tool_name.clone(),
};
let invocation = ToolInvocation {
payload: ToolPayload::Function {
arguments: serde_json::json!({ "message": "inspect this repo" }).to_string(),
},
..test_invocation(Arc::clone(&session), Arc::clone(&turn), "call-1", tool_name)
};
handler.pre_tool_use_payload(&invocation)
})
.collect::<Vec<_>>();
assert_eq!(
hook_payloads,
vec![
Some(PreToolUsePayload {
tool_name: HookToolName::spawn_agent(),
tool_input: serde_json::json!({ "message": "inspect this repo" }),
}),
Some(PreToolUsePayload {
tool_name: HookToolName::spawn_agent(),
tool_input: serde_json::json!({ "message": "inspect this repo" }),
}),
]
);
}
#[tokio::test]
async fn code_mode_wait_does_not_expose_default_hook_payloads() {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let output = crate::tools::context::FunctionToolOutput::from_text("ok".to_string(), Some(true));
let wait = crate::tools::handlers::CodeModeWaitHandler;
let wait_invocation = test_invocation(
Arc::new(session),
Arc::new(turn),
"wait-call",
wait.tool_name(),
);
assert_eq!(wait.pre_tool_use_payload(&wait_invocation), None);
assert_eq!(wait.post_tool_use_payload(&wait_invocation, &output), None);
}
#[tokio::test]
async fn write_stdin_does_not_expose_default_pre_tool_use_payload() {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let write_stdin = crate::tools::handlers::WriteStdinHandler;
let invocation = test_invocation(
Arc::new(session),
Arc::new(turn),
"write-stdin-call",
write_stdin.tool_name(),
);
assert_eq!(write_stdin.pre_tool_use_payload(&invocation), None);
}
#[test]
fn post_tool_use_feedback_output_keeps_code_mode_result_typed() {
let result = AnyToolResult {
call_id: "call-1".to_string(),
payload: ToolPayload::Function {
arguments: "{}".to_string(),
},
result: Box::new(PostToolUseFeedbackOutput {
original: Box::new(codex_tools::JsonToolOutput::new(
serde_json::json!({ "typed": true }),
)),
model_visible: crate::tools::context::FunctionToolOutput::from_text(
"hook feedback".to_string(),
/*success*/ None,
),
}),
post_tool_use_payload: None,
};
assert_eq!(
result.into_response(),
ResponseInputItem::FunctionCallOutput {
call_id: "call-1".to_string(),
output: codex_protocol::models::FunctionCallOutputPayload::from_text(
"hook feedback".to_string()
),
}
);
let result = AnyToolResult {
call_id: "call-1".to_string(),
payload: ToolPayload::Function {
arguments: "{}".to_string(),
},
result: Box::new(PostToolUseFeedbackOutput {
original: Box::new(codex_tools::JsonToolOutput::new(
serde_json::json!({ "typed": true }),
)),
model_visible: crate::tools::context::FunctionToolOutput::from_text(
"hook feedback".to_string(),
/*success*/ None,
),
}),
post_tool_use_payload: None,
};
assert_eq!(
result.code_mode_result(),
serde_json::json!({ "typed": true })
);
}
#[tokio::test]
async fn dispatch_notifies_tool_lifecycle_contributors() -> anyhow::Result<()> {
let (mut session, turn) = crate::session::tests::make_session_and_context().await;
+153
View File
@@ -1359,6 +1359,159 @@ text("session b done");
Ok(())
}
#[cfg_attr(windows, ignore = "no exec_command on Windows")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_concurrent_cells_merge_only_the_stored_values_they_write() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
let mut builder = test_codex().with_config(move |config| {
let _ = config.features.enable(Feature::CodeMode);
});
let test = builder.build(&server).await?;
let first_gate = test.workspace_path("code-mode-first-store.ready");
let first_wait = wait_for_file_source(&first_gate)?;
responses::mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-1"),
ev_custom_tool_call(
"call-init",
"exec",
r#"
store("a", 1);
store("b", 2);
"#,
),
ev_completed("resp-1"),
]),
)
.await;
responses::mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-1", "initialized"),
ev_completed("resp-2"),
]),
)
.await;
test.submit_turn("initialize stored values").await?;
let first_code = format!(
r#"
store("a", 3);
yield_control();
{first_wait}
"#
);
responses::mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-3"),
ev_custom_tool_call("call-first", "exec", &first_code),
ev_completed("resp-3"),
]),
)
.await;
let first_started = responses::mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-2", "first pending"),
ev_completed("resp-4"),
]),
)
.await;
test.submit_turn("start first store").await?;
let first_request = first_started.single_request();
let first_items = custom_tool_output_items(&first_request, "call-first");
let first_cell_id = extract_running_cell_id(text_item(&first_items, /*index*/ 0));
responses::mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-5"),
ev_custom_tool_call("call-second", "exec", r#"store("b", 4);"#),
ev_completed("resp-5"),
]),
)
.await;
responses::mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-3", "second complete"),
ev_completed("resp-6"),
]),
)
.await;
test.submit_turn("write the second key").await?;
fs::write(&first_gate, "ready")?;
responses::mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-7"),
responses::ev_function_call(
"call-wait",
"wait",
&serde_json::to_string(&serde_json::json!({
"cell_id": first_cell_id,
"yield_time_ms": 1_000,
}))?,
),
ev_completed("resp-7"),
]),
)
.await;
responses::mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-4", "first completed"),
ev_completed("resp-8"),
]),
)
.await;
test.submit_turn("complete the first store").await?;
responses::mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-9"),
ev_custom_tool_call(
"call-check",
"exec",
r#"text(JSON.stringify({ a: load("a"), b: load("b") }));"#,
),
ev_completed("resp-9"),
]),
)
.await;
let check_response = responses::mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-5", "checked"),
ev_completed("resp-10"),
]),
)
.await;
test.submit_turn("check merged stored values").await?;
let check_request = check_response.single_request();
let stored_values: Value = serde_json::from_str(
&custom_tool_output_last_non_empty_text(&check_request, "call-check")
.expect("checking stored values should emit JSON"),
)?;
assert_eq!(stored_values, serde_json::json!({ "a": 3, "b": 4 }));
Ok(())
}
#[cfg_attr(windows, ignore = "no exec_command on Windows")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_wait_can_terminate_and_continue() -> Result<()> {
+86 -94
View File
@@ -3463,42 +3463,35 @@ async fn pre_tool_use_blocks_apply_patch_with_write_alias() -> Result<()> {
}
#[tokio::test]
async fn pre_tool_use_does_not_fire_for_plan_tool() -> Result<()> {
async fn pre_tool_use_blocks_local_function_tool_before_execution() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "pretooluse-update-plan";
let args = serde_json::json!({
"plan": [{
"step": "watch the tide",
"status": "pending",
}]
});
let call_id = "pretooluse-local-function-tool";
let args = serde_json::json!({});
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
core_test_support::responses::ev_function_call(
call_id,
"update_plan",
&serde_json::to_string(&args)?,
),
ev_function_call(call_id, "test_sync_tool", &serde_json::to_string(&args)?),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "plan updated"),
ev_assistant_message("msg-1", "local function hook blocked it"),
ev_completed("resp-2"),
]),
],
)
.await;
let reason = "blocked local function pre hook";
let mut builder = test_codex()
.with_model("test-gpt-5.1-codex")
.with_pre_build_hook(|home| {
if let Err(error) =
write_pre_tool_use_hook(home, /*matcher*/ None, "json_deny", "should not fire")
write_pre_tool_use_hook(home, Some("^test_sync_tool$"), "json_deny", reason)
{
panic!("failed to write pre tool use hook test fixture: {error}");
}
@@ -3506,7 +3499,8 @@ async fn pre_tool_use_does_not_fire_for_plan_tool() -> Result<()> {
.with_config(trust_discovered_hooks);
let test = builder.build(&server).await?;
test.submit_turn("update the plan").await?;
test.submit_turn("call the local function tool with the pre hook")
.await?;
let requests = responses.requests();
assert_eq!(requests.len(), 2);
@@ -3514,17 +3508,85 @@ async fn pre_tool_use_does_not_fire_for_plan_tool() -> Result<()> {
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("update plan output string");
.expect("blocked local function tool output string");
assert!(
!output.contains("should not fire"),
"non-shell tool output should not be blocked by PreToolUse",
output.contains(&format!(
"Tool call blocked by PreToolUse hook: {reason}. Tool: test_sync_tool"
)),
"blocked local function output should surface the hook reason and tool name",
);
let hook_log_path = test.codex_home_path().join("pre_tool_use_hook_log.jsonl");
assert!(
!hook_log_path.exists(),
"plan tool should not trigger pre tool use hooks",
);
let hook_inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())?;
assert_eq!(hook_inputs.len(), 1);
assert_eq!(hook_inputs[0]["hook_event_name"], "PreToolUse");
assert_eq!(hook_inputs[0]["tool_name"], "test_sync_tool");
assert_eq!(hook_inputs[0]["tool_use_id"], call_id);
assert_eq!(hook_inputs[0]["tool_input"], args);
Ok(())
}
#[tokio::test]
async fn pre_tool_use_rewrites_local_function_tool_before_execution() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "pretooluse-local-function-tool-rewrite";
let original_args = serde_json::json!({
"barrier": {
"id": "pretooluse-local-function-invalid-barrier",
"participants": 0,
}
});
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(
call_id,
"test_sync_tool",
&serde_json::to_string(&original_args)?,
),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "local function hook rewrote it"),
ev_completed("resp-2"),
]),
],
)
.await;
let updated_input = serde_json::json!({});
let mut builder = test_codex()
.with_model("test-gpt-5.1-codex")
.with_pre_build_hook(move |home| {
if let Err(error) =
write_updating_pre_tool_use_hook(home, "^test_sync_tool$", &updated_input)
{
panic!("failed to write updating pre tool use hook test fixture: {error}");
}
})
.with_config(trust_discovered_hooks);
let test = builder.build(&server).await?;
test.submit_turn("call the local function tool with the pre hook rewrite")
.await?;
let requests = responses.requests();
assert_eq!(requests.len(), 2);
let output_item = requests[1].function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("rewritten local function tool output string");
assert_eq!(output, "ok");
let hook_inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())?;
assert_eq!(hook_inputs.len(), 1);
assert_eq!(hook_inputs[0]["tool_input"], original_args);
Ok(())
}
@@ -4133,73 +4195,3 @@ async fn post_tool_use_records_apply_patch_context_with_edit_alias() -> Result<(
Ok(())
}
#[tokio::test]
async fn post_tool_use_does_not_fire_for_plan_tool() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "posttooluse-update-plan";
let args = serde_json::json!({
"plan": [{
"step": "watch the tide",
"status": "pending",
}]
});
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
core_test_support::responses::ev_function_call(
call_id,
"update_plan",
&serde_json::to_string(&args)?,
),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "plan updated"),
ev_completed("resp-2"),
]),
],
)
.await;
let mut builder = test_codex()
.with_pre_build_hook(|home| {
if let Err(error) = write_post_tool_use_hook(
home,
/*matcher*/ None,
"decision_block",
"should not fire",
) {
panic!("failed to write post tool use hook test fixture: {error}");
}
})
.with_config(trust_discovered_hooks);
let test = builder.build(&server).await?;
test.submit_turn("update the plan").await?;
let requests = responses.requests();
assert_eq!(requests.len(), 2);
let output_item = requests[1].function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("update plan output string");
assert!(
!output.contains("should not fire"),
"non-shell tool output should not be affected by PostToolUse",
);
let hook_log_path = test.codex_home_path().join("post_tool_use_hook_log.jsonl");
assert!(
!hook_log_path.exists(),
"plan tool should not trigger post tool use hooks",
);
Ok(())
}
+1 -1
View File
@@ -416,7 +416,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
codex_self_exe: arg0_paths.codex_self_exe.clone(),
codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe.clone(),
main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe.clone(),
zsh_path: None,
default_zsh_path: None,
base_instructions: None,
developer_instructions: None,
personality: None,
+35
View File
@@ -11,6 +11,7 @@ const PATH_DIRNAME: &str = "codex-path";
const RELEASES_DIRNAME: &str = "releases";
const RESOURCES_DIRNAME: &str = "codex-resources";
const STANDALONE_PACKAGES_DIRNAME: &str = "standalone";
const ZSH_DIRNAME: &str = "zsh";
static INSTALL_CONTEXT: OnceLock<InstallContext> = OnceLock::new();
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -166,6 +167,18 @@ impl InstallContext {
None
}
pub fn bundled_zsh_path(&self) -> Option<AbsolutePathBuf> {
if cfg!(windows) {
None
} else {
self.bundled_resource(zsh_resource_path())
}
}
pub fn bundled_zsh_bin_dir(&self) -> Option<AbsolutePathBuf> {
self.bundled_zsh_path()?.parent()
}
}
impl CodexPackageLayout {
@@ -260,6 +273,10 @@ fn default_rg_command() -> PathBuf {
}
}
fn zsh_resource_path() -> PathBuf {
PathBuf::from(ZSH_DIRNAME).join(BIN_DIRNAME).join("zsh")
}
#[cfg(test)]
mod tests {
use super::*;
@@ -345,6 +362,11 @@ mod tests {
fs::write(&exe_path, "")?;
fs::write(resources_dir.join(TEST_RESOURCE_NAME), "")?;
fs::write(path_dir.join(default_rg_command()), "")?;
if !cfg!(windows) {
let zsh_path = resources_dir.join(zsh_resource_path());
fs::create_dir_all(zsh_path.parent().expect("zsh path should have parent"))?;
fs::write(&zsh_path, "")?;
}
let canonical_package_dir =
AbsolutePathBuf::from_absolute_path(package_dir.path().canonicalize()?)?;
let canonical_bin_dir = AbsolutePathBuf::from_absolute_path(bin_dir.canonicalize()?)?;
@@ -382,6 +404,19 @@ mod tests {
context.bundled_resource(TEST_RESOURCE_NAME),
Some(canonical_resources_dir.join(TEST_RESOURCE_NAME))
);
if cfg!(windows) {
assert_eq!(context.bundled_zsh_path(), None);
assert_eq!(context.bundled_zsh_bin_dir(), None);
} else {
assert_eq!(
context.bundled_zsh_path(),
Some(canonical_resources_dir.join(zsh_resource_path()))
);
assert_eq!(
context.bundled_zsh_bin_dir(),
Some(canonical_resources_dir.join(ZSH_DIRNAME).join(BIN_DIRNAME))
);
}
Ok(())
}
+34
View File
@@ -7,6 +7,7 @@ use crate::exec_output::ExecToolCallOutput;
use crate::network_policy::NetworkPolicyDecisionPayload;
use crate::protocol::CodexErrorInfo;
use crate::protocol::ErrorEvent;
use crate::protocol::RateLimitReachedType;
use crate::protocol::RateLimitSnapshot;
use crate::protocol::TruncationPolicy;
use chrono::DateTime;
@@ -451,6 +452,7 @@ pub struct UsageLimitReachedError {
pub resets_at: Option<DateTime<Utc>>,
pub rate_limits: Option<Box<RateLimitSnapshot>>,
pub promo_message: Option<String>,
pub rate_limit_reached_type: Option<RateLimitReachedType>,
}
impl std::fmt::Display for UsageLimitReachedError {
@@ -470,6 +472,38 @@ impl std::fmt::Display for UsageLimitReachedError {
);
}
if let Some(rate_limit_reached_type) = self.rate_limit_reached_type {
match rate_limit_reached_type {
RateLimitReachedType::WorkspaceOwnerCreditsDepleted => {
return write!(
f,
"Your workspace is out of credits. Add credits to continue."
);
}
RateLimitReachedType::WorkspaceMemberCreditsDepleted => {
return write!(
f,
"Your workspace is out of credits. Ask your workspace owner to refill in order to continue."
);
}
RateLimitReachedType::WorkspaceOwnerUsageLimitReached => {
return write!(
f,
"You hit your spend cap set in your workspace. Increase your spend cap to continue."
);
}
RateLimitReachedType::WorkspaceMemberUsageLimitReached => {
return write!(
f,
"You hit your spend cap set by the owner of your workspace. Ask an owner to increase your spend cap to continue."
);
}
RateLimitReachedType::RateLimitReached => {
// Generic limits intentionally use the existing promo or plan copy below.
}
}
}
if let Some(promo_message) = &self.promo_message {
return write!(
f,
+54
View File
@@ -56,6 +56,7 @@ fn usage_limit_reached_error_formats_plus_plan() {
resets_at: None,
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
assert_eq!(
err.to_string(),
@@ -63,6 +64,44 @@ fn usage_limit_reached_error_formats_plus_plan() {
);
}
#[test]
fn usage_limit_reached_error_formats_rate_limit_reached_types() {
let cases = [
(
RateLimitReachedType::RateLimitReached,
"You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again later.",
),
(
RateLimitReachedType::WorkspaceOwnerCreditsDepleted,
"Your workspace is out of credits. Add credits to continue.",
),
(
RateLimitReachedType::WorkspaceMemberCreditsDepleted,
"Your workspace is out of credits. Ask your workspace owner to refill in order to continue.",
),
(
RateLimitReachedType::WorkspaceOwnerUsageLimitReached,
"You hit your spend cap set in your workspace. Increase your spend cap to continue.",
),
(
RateLimitReachedType::WorkspaceMemberUsageLimitReached,
"You hit your spend cap set by the owner of your workspace. Ask an owner to increase your spend cap to continue.",
),
];
for (rate_limit_reached_type, expected) in cases {
let err = UsageLimitReachedError {
plan_type: Some(PlanType::Known(KnownPlan::Plus)),
resets_at: None,
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: Some(rate_limit_reached_type),
};
assert_eq!(err.to_string(), expected);
}
}
#[test]
fn server_overloaded_maps_to_protocol() {
let err = CodexErr::ServerOverloaded;
@@ -177,6 +216,7 @@ fn usage_limit_reached_error_formats_free_plan() {
resets_at: None,
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
assert_eq!(
err.to_string(),
@@ -191,6 +231,7 @@ fn usage_limit_reached_error_formats_go_plan() {
resets_at: None,
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
assert_eq!(
err.to_string(),
@@ -205,6 +246,7 @@ fn usage_limit_reached_error_formats_default_when_none() {
resets_at: None,
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
assert_eq!(
err.to_string(),
@@ -223,6 +265,7 @@ fn usage_limit_reached_error_formats_team_plan() {
resets_at: Some(resets_at),
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
let expected = format!(
"You've hit your usage limit. To get more access now, send a request to your admin or try again at {expected_time}."
@@ -238,6 +281,7 @@ fn usage_limit_reached_error_formats_business_plan_without_reset() {
resets_at: None,
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
assert_eq!(
err.to_string(),
@@ -252,6 +296,7 @@ fn usage_limit_reached_error_formats_self_serve_business_usage_based_plan() {
resets_at: None,
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
assert_eq!(
err.to_string(),
@@ -266,6 +311,7 @@ fn usage_limit_reached_error_formats_enterprise_cbp_usage_based_plan() {
resets_at: None,
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
assert_eq!(
err.to_string(),
@@ -280,6 +326,7 @@ fn usage_limit_reached_error_formats_default_for_other_plans() {
resets_at: None,
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
assert_eq!(
err.to_string(),
@@ -298,6 +345,7 @@ fn usage_limit_reached_error_formats_pro_plan_with_reset() {
resets_at: Some(resets_at),
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
let expected = format!(
"You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at {expected_time}."
@@ -324,6 +372,7 @@ fn usage_limit_reached_error_hides_upsell_for_non_codex_limit_name() {
"Visit https://chatgpt.com/codex/settings/usage to purchase more credits"
.to_string(),
),
rate_limit_reached_type: None,
};
let expected = format!(
"You've hit your usage limit for codex_other. Switch to another model now, or try again at {expected_time}."
@@ -343,6 +392,7 @@ fn usage_limit_reached_includes_minutes_when_available() {
resets_at: Some(resets_at),
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
let expected = format!("You've hit your usage limit. Try again at {expected_time}.");
assert_eq!(err.to_string(), expected);
@@ -482,6 +532,7 @@ fn usage_limit_reached_includes_hours_and_minutes() {
resets_at: Some(resets_at),
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
let expected = format!(
"You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at {expected_time}."
@@ -502,6 +553,7 @@ fn usage_limit_reached_includes_days_hours_minutes() {
resets_at: Some(resets_at),
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
let expected = format!("You've hit your usage limit. Try again at {expected_time}.");
assert_eq!(err.to_string(), expected);
@@ -519,6 +571,7 @@ fn usage_limit_reached_less_than_minute() {
resets_at: Some(resets_at),
rate_limits: Some(Box::new(rate_limit_snapshot())),
promo_message: None,
rate_limit_reached_type: None,
};
let expected = format!("You've hit your usage limit. Try again at {expected_time}.");
assert_eq!(err.to_string(), expected);
@@ -538,6 +591,7 @@ fn usage_limit_reached_with_promo_message() {
promo_message: Some(
"To continue using Codex, start a free trial of <PLAN> today".to_string(),
),
rate_limit_reached_type: None,
};
let expected = format!(
"You've hit your usage limit. To continue using Codex, start a free trial of <PLAN> today, or try again at {expected_time}."
+15
View File
@@ -2000,6 +2000,21 @@ pub enum RateLimitReachedType {
WorkspaceMemberUsageLimitReached,
}
impl FromStr for RateLimitReachedType {
type Err = String;
fn from_str(value: &str) -> Result<Self, Self::Err> {
match value {
"rate_limit_reached" => Ok(Self::RateLimitReached),
"workspace_owner_credits_depleted" => Ok(Self::WorkspaceOwnerCreditsDepleted),
"workspace_member_credits_depleted" => Ok(Self::WorkspaceMemberCreditsDepleted),
"workspace_owner_usage_limit_reached" => Ok(Self::WorkspaceOwnerUsageLimitReached),
"workspace_member_usage_limit_reached" => Ok(Self::WorkspaceMemberUsageLimitReached),
other => Err(format!("unknown rate limit reached type: {other}")),
}
}
}
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)]
pub struct RateLimitWindow {
/// Percentage (0-100) of the window that has been consumed.
+1
View File
@@ -31,6 +31,7 @@ tracing = { workspace = true }
urlencoding = { workspace = true }
[dev-dependencies]
codex-utils-cargo-bin = { workspace = true }
pretty_assertions = { workspace = true }
[lib]
@@ -0,0 +1,85 @@
{
"source": "standard/google_calendar/tools",
"tools": [
{
"name": "google_calendar_create_event",
"description": "Create a calendar event.",
"input_schema": {
"type": "object",
"properties": {
"event": {
"$ref": "#/definitions/Event"
},
"notify_attendees": {
"type": "boolean",
"description": "Whether attendees should receive notifications."
}
},
"required": [
"event"
],
"definitions": {
"Event": {
"type": "object",
"description": "Calendar event payload.",
"properties": {
"title": {
"type": "string",
"description": "Event title."
},
"start": {
"anyOf": [
{
"$ref": "#/definitions/DateTime"
},
{
"type": "null"
}
]
}
}
},
"DateTime": {
"type": "object",
"description": "Calendar date-time.",
"properties": {
"dateTime": {
"type": "string",
"format": "date-time",
"description": "RFC3339 date-time."
},
"timeZone": {
"type": "string",
"enum": [
"UTC",
"America/Los_Angeles"
],
"description": "IANA time zone."
}
}
},
"UnusedCalendarResource": {
"type": "string",
"description": "Unreachable calendar resource."
}
}
},
"expected_preserved": [
{
"pointer": "/properties/event/$ref",
"value": "#/definitions/Event"
},
{
"pointer": "/definitions/DateTime/properties/timeZone/enum/1",
"value": "America/Los_Angeles"
}
],
"expected_pruned": [
"/definitions/UnusedCalendarResource"
],
"expected_dropped_fields": [
"/definitions/DateTime/properties/dateTime/format"
]
}
]
}
@@ -0,0 +1,65 @@
{
"source": "standard/google_drive/tools",
"tools": [
{
"name": "google_drive_copy_file",
"description": "Copy a Google Drive file.",
"input_schema": {
"type": "object",
"properties": {
"file": {
"description": "File selector.",
"oneOf": [
{
"type": "string",
"description": "A Drive file ID."
},
{
"type": "object",
"properties": {
"shared_drive_id": {
"type": "string",
"description": "Shared drive identifier."
}
}
}
]
},
"metadata": {
"description": "Optional copy metadata.",
"allOf": [
{
"type": "object",
"description": "Base metadata.",
"properties": {
"source": {
"type": "string",
"description": "Metadata source."
}
}
}
]
},
"title": {
"type": "string",
"description": "Copied file title."
}
},
"required": [
"file"
]
},
"expected_preserved": [
{
"pointer": "/properties/title/description",
"value": "Copied file title."
}
],
"expected_pruned": [],
"expected_dropped_fields": [
"/properties/file/oneOf",
"/properties/metadata/allOf"
]
}
]
}
@@ -0,0 +1,90 @@
{
"source": "golden/microsoft_outlook_email/tools",
"tools": [
{
"name": "microsoft_outlook_email_send",
"description": "Send an Outlook email.",
"input_schema": {
"type": "object",
"properties": {
"message_id": {
"$ref": "#/$defs/Message/properties/id"
},
"attachment": {
"type": "array",
"items": {
"$ref": "#/$defs/Attachment"
}
}
},
"$defs": {
"Message": {
"type": "object",
"description": "Outlook message.",
"properties": {
"id": {
"type": "string",
"description": "Message identifier."
},
"sender": {
"$ref": "#/$defs/EmailAddress"
},
"importance": {
"type": "string",
"enum": [
"low",
"normal",
"high"
],
"description": "Message importance."
}
}
},
"Attachment": {
"type": "object",
"description": "Email attachment.",
"properties": {
"name": {
"type": "string",
"description": "Attachment file name."
}
}
},
"EmailAddress": {
"type": "object",
"description": "Email address object.",
"properties": {
"address": {
"type": "string",
"description": "SMTP address."
}
}
},
"GiantUnusedPayload": {
"type": "object",
"description": "Representative unreachable Outlook payload.",
"properties": {
"opaque": {
"type": "string"
}
}
}
}
},
"expected_preserved": [
{
"pointer": "/properties/message_id/$ref",
"value": "#/$defs/Message/properties/id"
},
{
"pointer": "/$defs/Message/properties/importance/enum/2",
"value": "high"
}
],
"expected_pruned": [
"/$defs/GiantUnusedPayload"
],
"expected_dropped_fields": []
}
]
}
@@ -0,0 +1,72 @@
{
"source": "golden/notion/tools",
"tools": [
{
"name": "notion_fetch_page",
"description": "Fetch a Notion page.",
"input_schema": {
"type": "object",
"properties": {
"page": {
"$ref": "#/$defs/Page%20Ref"
},
"filter": {
"type": "object",
"additionalProperties": {
"$ref": "#/$defs/Filter"
}
},
"content_mode": {
"description": "How much content to return.",
"anyOf": [
{
"type": "string",
"enum": [
"summary",
"full"
]
},
{
"type": "null"
}
]
}
},
"$defs": {
"Page Ref": {
"type": "string",
"description": "Notion page ID or URL."
},
"Filter": {
"type": "object",
"description": "Filter object.",
"properties": {
"created_by": {
"type": "string",
"description": "Creator user ID."
}
}
},
"UnusedDatabase": {
"type": "object",
"description": "Unreachable Notion database schema."
}
}
},
"expected_preserved": [
{
"pointer": "/properties/page/$ref",
"value": "#/$defs/Page%20Ref"
},
{
"pointer": "/properties/content_mode/anyOf/0/enum/0",
"value": "summary"
}
],
"expected_pruned": [
"/$defs/UnusedDatabase"
],
"expected_dropped_fields": []
}
]
}
@@ -0,0 +1,75 @@
{
"source": "standard/slack/tools",
"tools": [
{
"name": "slack_schedule_message",
"description": "Schedule a Slack message.",
"input_schema": {
"type": "object",
"properties": {
"channel_id": {
"type": "string",
"description": "Slack channel ID."
},
"message": {
"type": "string",
"description": "Message text."
},
"post_at": {
"type": "integer",
"description": "Unix timestamp for delivery."
},
"thread_ts": {
"description": "Optional parent thread timestamp.",
"anyOf": [
{
"$ref": "#/$defs/SlackTs"
},
{
"type": "null"
}
]
}
},
"required": [
"channel_id",
"message",
"post_at"
],
"additionalProperties": false,
"$defs": {
"SlackTs": {
"type": "string",
"description": "Slack timestamp string.",
"pattern": "^[0-9]+[.][0-9]+$"
},
"UnusedPayload": {
"type": "object",
"description": "Large unreachable Slack payload.",
"properties": {
"debug": {
"type": "string"
}
}
}
}
},
"expected_preserved": [
{
"pointer": "/properties/thread_ts/anyOf/0/$ref",
"value": "#/$defs/SlackTs"
},
{
"pointer": "/$defs/SlackTs/description",
"value": "Slack timestamp string."
}
],
"expected_pruned": [
"/$defs/UnusedPayload"
],
"expected_dropped_fields": [
"/$defs/SlackTs/pattern"
]
}
]
}
@@ -0,0 +1,224 @@
use codex_tools::ToolName;
use codex_tools::mcp_tool_to_responses_api_tool;
use pretty_assertions::assert_eq;
use serde::Deserialize;
use serde::de::DeserializeOwned;
use serde_json::Value;
use serde_json::json;
use std::fs;
use std::sync::Arc;
const FIXTURE_PATHS: [&str; 5] = [
"tests/fixtures/json_schema_policy/slack.json",
"tests/fixtures/json_schema_policy/google_calendar.json",
"tests/fixtures/json_schema_policy/google_drive.json",
"tests/fixtures/json_schema_policy/notion.json",
"tests/fixtures/json_schema_policy/microsoft_outlook_email.json",
];
const OVERSIZED_NOTION_CREATE_PAGE_SCHEMA_PATH: &str =
"tests/fixtures/json_schema_policy/oversized_notion_create_page_input_schema.json";
#[derive(Debug, Deserialize)]
struct FixtureFile {
source: String,
tools: Vec<FixtureTool>,
}
#[derive(Debug, Deserialize)]
struct FixtureTool {
name: String,
description: String,
input_schema: Value,
#[serde(default)]
expected_preserved: Vec<ExpectedValue>,
#[serde(default)]
expected_pruned: Vec<String>,
#[serde(default)]
expected_dropped_fields: Vec<String>,
}
#[derive(Debug, Deserialize)]
struct ExpectedValue {
pointer: String,
value: Value,
}
#[test]
fn json_schema_policy_fixtures_convert_to_responses_tools() {
for fixture in FIXTURE_PATHS.into_iter().map(load_fixture::<FixtureFile>) {
for fixture_tool in &fixture.tools {
let responses_tool = convert_fixture_tool(&fixture, fixture_tool);
let parameters = serde_json::to_value(&responses_tool.parameters)
.expect("responses parameters should serialize");
let expected_fields = [
(
"preserve the tool name",
json!(fixture_tool.name),
json!(responses_tool.name),
),
(
"preserve the tool description",
json!(fixture_tool.description),
json!(responses_tool.description),
),
(
"remain a strict:false tool",
json!(false),
json!(responses_tool.strict),
),
(
"produce object-shaped parameters",
json!("object"),
parameters.get("type").cloned().unwrap_or(Value::Null),
),
];
for (message, expected, actual) in expected_fields {
assert_eq!(actual, expected, "{} should {message}", fixture_tool.name);
}
assert!(
parameters.get("properties").is_some_and(Value::is_object),
"{} should produce a parameters.properties object",
fixture_tool.name
);
for expected in &fixture_tool.expected_preserved {
assert_eq!(
parameters.pointer(&expected.pointer),
Some(&expected.value),
"{} should preserve {}",
fixture_tool.name,
expected.pointer
);
}
for pointer in &fixture_tool.expected_pruned {
assert!(
parameters.pointer(pointer).is_none(),
"{} should prune unreachable definition {pointer}",
fixture_tool.name
);
}
for pointer in &fixture_tool.expected_dropped_fields {
assert!(
fixture_tool.input_schema.pointer(pointer).is_some(),
"{} fixture should contain expected dropped field {pointer}",
fixture_tool.name
);
assert!(
parameters.pointer(pointer).is_none(),
"{} should drop field {pointer} after JsonSchema conversion",
fixture_tool.name
);
}
}
}
}
#[test]
fn json_schema_policy_oversized_golden_schema_triggers_compaction() {
let fixture: FixtureFile = load_fixture(OVERSIZED_NOTION_CREATE_PAGE_SCHEMA_PATH);
let fixture_tool = fixture
.tools
.first()
.expect("oversized fixture should contain a tool");
let input_bytes = compact_json_len(&fixture_tool.input_schema);
let responses_tool = convert_fixture_tool(&fixture, fixture_tool);
let parameters =
serde_json::to_value(&responses_tool.parameters).expect("responses parameters serialize");
let output_bytes = compact_json_len(&parameters);
assert!(
output_bytes < input_bytes,
"compaction should reduce schema size from {input_bytes} bytes"
);
let absent_pointers = [
("/description", "drop root description"),
("/properties/parent/description", "drop nested descriptions"),
(
"/$defs",
"drop root definitions after stripping descriptions is insufficient",
),
];
for (pointer, message) in absent_pointers {
assert!(
parameters.pointer(pointer).is_none(),
"oversized schema should {message}"
);
}
let expected_values = [
(
"/properties/parent",
json!({}),
"rewrite local refs before dropping root definitions",
),
(
"/properties/children/items",
json!({}),
"rewrite nested local refs before dropping root definitions",
),
(
"/properties/markdown/type",
json!("string"),
"retain top-level argument shape",
),
(
"/properties/properties/type",
json!("object"),
"retain object argument shape",
),
];
for (pointer, expected, message) in expected_values {
assert_eq!(
parameters.pointer(pointer),
Some(&expected),
"oversized schema should {message}"
);
}
}
fn load_fixture<T: DeserializeOwned>(path: &str) -> T {
let path = codex_utils_cargo_bin::find_resource!(path)
.unwrap_or_else(|err| panic!("resolve fixture {path}: {err}"));
let fixture = fs::read_to_string(&path)
.unwrap_or_else(|err| panic!("read fixture {}: {err}", path.display()));
serde_json::from_str(&fixture)
.unwrap_or_else(|err| panic!("parse fixture {}: {err}", path.display()))
}
fn convert_fixture_tool(
fixture: &FixtureFile,
fixture_tool: &FixtureTool,
) -> codex_tools::ResponsesApiTool {
let name = &fixture_tool.name;
let input_schema = fixture_tool
.input_schema
.as_object()
.unwrap_or_else(|| panic!("{name} input_schema should be an object"))
.clone();
let tool = rmcp::model::Tool {
name: name.to_string().into(),
title: None,
description: Some(fixture_tool.description.clone().into()),
input_schema: Arc::new(input_schema),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
};
mcp_tool_to_responses_api_tool(&ToolName::namespaced(&fixture.source, name), &tool)
.unwrap_or_else(|err| panic!("convert {name} from {}: {err}", fixture.source))
}
fn compact_json_len(value: &Value) -> usize {
serde_json::to_vec(value)
.unwrap_or_else(|err| panic!("serialize compact JSON: {err}"))
.len()
}
+7
View File
@@ -13,6 +13,7 @@ The builder creates a canonical Codex package directory:
│ └── <entrypoint>[.exe]
├── codex-resources
│ ├── bwrap # Linux only
│ ├── zsh/bin/zsh # supported Unix targets only
│ ├── codex-command-runner.exe # Windows only
│ └── codex-windows-sandbox-setup.exe # Windows only
└── codex-path
@@ -67,3 +68,9 @@ DotSlash manifest at `scripts/codex_package/rg`. Downloaded archives are cached
under `$TMPDIR/codex-package/<target>-rg` and are reused only after the recorded
size and SHA-256 digest have been verified. Pass `--rg-bin` to use a local
ripgrep executable instead.
The patched zsh fork used by `shell_zsh_fork` is fetched from the DotSlash
manifest at `scripts/codex_package/codex-zsh` when the selected target has a
matching prebuilt artifact. Downloaded archives are cached under
`$TMPDIR/codex-package/<target>-zsh` and installed at
`codex-resources/zsh/bin/zsh`.
+3 -1
View File
@@ -15,6 +15,7 @@ from .targets import TARGET_SPECS
from .targets import PackageInputs
from .targets import default_target
from .targets import resolve_input_path
from .zsh import resolve_zsh_bin
from .version import read_workspace_version
@@ -161,13 +162,14 @@ def main() -> int:
inputs = PackageInputs(
entrypoint_bin=source_outputs.entrypoint_bin,
rg_bin=resolve_rg_bin(spec, args.rg_bin),
zsh_bin=resolve_zsh_bin(spec),
bwrap_bin=source_outputs.bwrap_bin,
codex_command_runner_bin=source_outputs.codex_command_runner_bin,
codex_windows_sandbox_setup_bin=source_outputs.codex_windows_sandbox_setup_bin,
)
prepare_package_dir(package_dir, force=args.force)
build_package_dir(package_dir, version, variant, spec, inputs)
validate_package_dir(package_dir, variant, spec)
validate_package_dir(package_dir, variant, spec, include_zsh=inputs.zsh_bin is not None)
for archive_output in args.archive_output:
archive_path = archive_output.resolve()
+55
View File
@@ -0,0 +1,55 @@
#!/usr/bin/env dotslash
{
"name": "codex-zsh",
"platforms": {
"macos-aarch64": {
"size": 358776,
"hash": "sha256",
"digest": "49dec9832379688c9090666694a3449502ac5ebd4d76b9ffde1d0999cd088205",
"format": "tar.gz",
"path": "codex-zsh/bin/zsh",
"providers": [
{
"url": "https://github.com/openai/codex/releases/download/rust-v0.134.0-alpha.3/codex-zsh-aarch64-apple-darwin.tar.gz"
}
]
},
"macos-x86_64": {
"size": 391161,
"hash": "sha256",
"digest": "0246fd4703bb540ae74f13ec739ca365ebd607df44a1f21e335eb7a421352923",
"format": "tar.gz",
"path": "codex-zsh/bin/zsh",
"providers": [
{
"url": "https://github.com/openai/codex/releases/download/rust-v0.134.0-alpha.3/codex-zsh-x86_64-apple-darwin.tar.gz"
}
]
},
"linux-x86_64": {
"size": 433412,
"hash": "sha256",
"digest": "e7f760998c9644448d2cb8a821084a0134d6820c14b284bd7f39d7f32262ad40",
"format": "tar.gz",
"path": "codex-zsh/bin/zsh",
"providers": [
{
"url": "https://github.com/openai/codex/releases/download/rust-v0.134.0-alpha.3/codex-zsh-x86_64-unknown-linux-musl.tar.gz"
}
]
},
"linux-aarch64": {
"size": 411655,
"hash": "sha256",
"digest": "8d50cff5dfabc97e37e1138513da022d9247a802c2ff996ab91654bf1892c7d5",
"format": "tar.gz",
"path": "codex-zsh/bin/zsh",
"providers": [
{
"url": "https://github.com/openai/codex/releases/download/rust-v0.134.0-alpha.3/codex-zsh-aarch64-unknown-linux-musl.tar.gz"
}
]
}
}
}
+16 -1
View File
@@ -8,6 +8,7 @@ from pathlib import Path
from .targets import PackageInputs
from .targets import PackageVariant
from .targets import TargetSpec
from .zsh import ZSH_RESOURCE_PATH
LAYOUT_VERSION = 1
@@ -50,6 +51,13 @@ def build_package_dir(
)
copy_executable(inputs.rg_bin, path_dir / spec.rg_name, is_windows=spec.is_windows)
if inputs.zsh_bin is not None:
copy_executable(
inputs.zsh_bin,
resources_dir / ZSH_RESOURCE_PATH,
is_windows=False,
)
if inputs.bwrap_bin is not None:
copy_executable(inputs.bwrap_bin, resources_dir / "bwrap", is_windows=False)
@@ -83,6 +91,8 @@ def validate_package_dir(
package_dir: Path,
variant: PackageVariant,
spec: TargetSpec,
*,
include_zsh: bool,
) -> None:
required_dirs = [
Path("bin"),
@@ -122,6 +132,11 @@ def validate_package_dir(
]
executable_files = list(required_files)
if include_zsh:
zsh_path = Path("codex-resources") / ZSH_RESOURCE_PATH
required_files.append(zsh_path)
executable_files.append(zsh_path)
if spec.is_linux:
required_files.append(Path("codex-resources") / "bwrap")
executable_files.append(Path("codex-resources") / "bwrap")
@@ -148,7 +163,7 @@ def validate_package_dir(
def copy_executable(src: Path, dest: Path, *, is_windows: bool) -> None:
dest.parent.mkdir(parents=True, exist_ok=True)
shutil.copy2(src, dest)
shutil.copyfile(src, dest)
if not is_windows:
mode = dest.stat().st_mode
dest.chmod(mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
+1
View File
@@ -40,6 +40,7 @@ class PackageVariant:
class PackageInputs:
entrypoint_bin: Path
rg_bin: Path
zsh_bin: Path | None
bwrap_bin: Path | None
codex_command_runner_bin: Path | None
codex_windows_sandbox_setup_bin: Path | None
+22
View File
@@ -0,0 +1,22 @@
"""Fetch the patched zsh fork used by shell_zsh_fork."""
from pathlib import Path
from .dotslash import fetch_dotslash_executable
from .targets import REPO_ROOT
from .targets import TargetSpec
ZSH_MANIFEST = REPO_ROOT / "scripts" / "codex_package" / "codex-zsh"
ZSH_RESOURCE_PATH = Path("zsh") / "bin" / "zsh"
def resolve_zsh_bin(spec: TargetSpec) -> Path | None:
return fetch_dotslash_executable(
spec,
manifest_path=ZSH_MANIFEST,
artifact_label="codex-zsh",
cache_key=f"{spec.target}-zsh",
dest_name="zsh",
missing_ok=True,
)