Merge remote-tracking branch 'origin/main' into jif/diff-based-phase-2

# Conflicts:
#	codex-rs/Cargo.lock
This commit is contained in:
jif-oai
2026-04-21 15:16:06 +01:00
455 changed files with 30996 additions and 16498 deletions
+3 -2
View File
@@ -5,7 +5,8 @@ use crate::agent::agent_status_from_event;
use crate::config::AgentRoleConfig;
use crate::config::Config;
use crate::config::ConfigBuilder;
use crate::contextual_user_message::SUBAGENT_NOTIFICATION_OPEN_TAG;
use crate::context::ContextualUserFragment;
use crate::context::SubagentNotification;
use assert_matches::assert_matches;
use codex_features::Feature;
use codex_login::CodexAuth;
@@ -127,7 +128,7 @@ fn has_subagent_notification(history_items: &[ResponseItem]) -> bool {
}
content.iter().any(|content_item| match content_item {
ContentItem::InputText { text } | ContentItem::OutputText { text } => {
text.contains(SUBAGENT_NOTIFICATION_OPEN_TAG)
SubagentNotification::matches_text(text)
}
ContentItem::InputImage { .. } => false,
})
+1 -1
View File
@@ -83,7 +83,7 @@ async fn apply_explorer_role_sets_model_and_adds_session_flags_layer() {
.await
.expect("explorer role should apply");
assert_eq!(config.model.as_deref(), Some("gpt-5.1-codex-mini"));
assert_eq!(config.model.as_deref(), Some("gpt-5.4-mini"));
assert_eq!(config.model_reasoning_effort, Some(ReasoningEffort::Medium));
assert_eq!(session_flags_layer_count(&config), before_layers + 1);
}
+12 -5
View File
@@ -22,7 +22,7 @@ use rand::TryRngCore;
use rand::rngs::OsRng;
use serde::Deserialize;
use serde::Serialize;
use tokio::sync::Mutex;
use tokio::sync::Semaphore;
use tracing::debug;
use tracing::info;
use tracing::warn;
@@ -42,7 +42,7 @@ pub(crate) struct AgentIdentityManager {
chatgpt_base_url: String,
feature_enabled: bool,
abom: AgentBillOfMaterials,
ensure_lock: Arc<Mutex<()>>,
ensure_lock: Arc<Semaphore>,
}
impl std::fmt::Debug for AgentIdentityManager {
@@ -110,7 +110,7 @@ impl AgentIdentityManager {
chatgpt_base_url: config.chatgpt_base_url.clone(),
feature_enabled: config.features.enabled(Feature::UseAgentIdentity),
abom: build_abom(session_source),
ensure_lock: Arc::new(Mutex::new(())),
ensure_lock: Arc::new(Semaphore::new(/*permits*/ 1)),
}
}
@@ -137,7 +137,11 @@ impl AgentIdentityManager {
auth: &CodexAuth,
binding: &AgentIdentityBinding,
) -> Result<StoredAgentIdentity> {
let _guard = self.ensure_lock.lock().await;
let _guard = self
.ensure_lock
.acquire()
.await
.map_err(|_| anyhow::anyhow!("agent identity ensure semaphore closed"))?;
if let Some(stored_identity) = self.load_stored_identity(auth, binding)? {
info!(
@@ -346,7 +350,7 @@ impl AgentIdentityManager {
chatgpt_base_url,
feature_enabled,
abom: build_abom(session_source),
ensure_lock: Arc::new(Mutex::new(())),
ensure_lock: Arc::new(Semaphore::new(/*permits*/ 1)),
}
}
@@ -408,6 +412,7 @@ impl StoredAgentIdentity {
agent_runtime_id: self.agent_runtime_id.clone(),
agent_private_key: self.private_key_pkcs8_base64.clone(),
registered_at: self.registered_at.clone(),
background_task_id: None,
}
}
@@ -668,6 +673,7 @@ mod tests {
agent_runtime_id: "agent_invalid".to_string(),
agent_private_key: "not-valid-base64".to_string(),
registered_at: "2026-01-01T00:00:00Z".to_string(),
background_task_id: None,
})
.expect("seed invalid identity");
@@ -707,6 +713,7 @@ mod tests {
agent_runtime_id: "agent_old".to_string(),
agent_private_key: stale_key.private_key_pkcs8_base64,
registered_at: "2026-01-01T00:00:00Z".to_string(),
background_task_id: None,
})
.expect("seed stale identity");
+11 -4
View File
@@ -1,5 +1,6 @@
use std::env;
use std::ffi::OsStr;
use std::path::PathBuf;
use std::sync::Arc;
use pretty_assertions::assert_eq;
@@ -16,6 +17,7 @@ use wiremock::matchers::path;
use super::*;
use crate::agent_identity::AgentIdentityManager;
use crate::agent_identity::RegisteredAgentTask;
use crate::context::ContextualUserFragment;
use crate::session::tests::make_session_and_context;
use chrono::Utc;
use codex_login::AuthCredentialsStoreMode;
@@ -143,11 +145,16 @@ async fn build_arc_monitor_request_includes_relevant_history_and_null_policies()
.await;
session
.record_into_history(
&[
crate::contextual_user_message::ENVIRONMENT_CONTEXT_FRAGMENT.into_message(
"<environment_context>\n<cwd>/tmp</cwd>\n</environment_context>".to_string(),
&[ContextualUserFragment::into(
crate::context::EnvironmentContext::new(
Some(PathBuf::from("/tmp")),
"zsh".to_string(),
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
/*subagents*/ None,
),
],
)],
&turn_context,
)
.await;
+4 -4
View File
@@ -13,7 +13,7 @@ fn serializes_text_verbosity_when_set() {
let input: Vec<ResponseItem> = vec![];
let tools: Vec<serde_json::Value> = vec![];
let req = ResponsesApiRequest {
model: "gpt-5.1".to_string(),
model: "gpt-5.4".to_string(),
instructions: "i".to_string(),
input,
tools,
@@ -57,7 +57,7 @@ fn serializes_text_schema_with_strict_format() {
.expect("text controls");
let req = ResponsesApiRequest {
model: "gpt-5.1".to_string(),
model: "gpt-5.4".to_string(),
instructions: "i".to_string(),
input,
tools,
@@ -95,7 +95,7 @@ fn omits_text_when_not_set() {
let input: Vec<ResponseItem> = vec![];
let tools: Vec<serde_json::Value> = vec![];
let req = ResponsesApiRequest {
model: "gpt-5.1".to_string(),
model: "gpt-5.4".to_string(),
instructions: "i".to_string(),
input,
tools,
@@ -118,7 +118,7 @@ fn omits_text_when_not_set() {
#[test]
fn serializes_flex_service_tier_when_set() {
let req = ResponsesApiRequest {
model: "gpt-5.1".to_string(),
model: "gpt-5.4".to_string(),
instructions: "i".to_string(),
input: vec![],
tools: vec![],
+6 -36
View File
@@ -26,15 +26,14 @@ use codex_protocol::user_input::UserInput;
use serde_json::Value;
use std::time::Duration;
use tokio::sync::Mutex;
use tokio::sync::oneshot;
use tokio::time::timeout;
use tokio_util::sync::CancellationToken;
use crate::config::Config;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::new_guardian_review_id;
use crate::guardian::review_approval_request_with_cancel;
use crate::guardian::routes_approval_to_guardian;
use crate::guardian::spawn_approval_request_review;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC;
@@ -457,7 +456,7 @@ async fn handle_exec_approval(
} = event;
let decision = if routes_approval_to_guardian(parent_ctx) {
let review_cancel = cancel_token.child_token();
let review_rx = spawn_guardian_review(
let review_rx = spawn_approval_request_review(
Arc::clone(parent_session),
Arc::clone(parent_ctx),
new_guardian_review_id(),
@@ -565,7 +564,7 @@ async fn handle_patch_approval(
})
.collect::<Vec<_>>()
.join("\n");
let review_rx = spawn_guardian_review(
let review_rx = spawn_approval_request_review(
Arc::clone(parent_session),
Arc::clone(parent_ctx),
new_guardian_review_id(),
@@ -686,7 +685,7 @@ async fn maybe_auto_review_mcp_request_user_input(
)
.await;
let review_cancel = cancel_token.child_token();
let review_rx = spawn_guardian_review(
let review_rx = spawn_approval_request_review(
Arc::clone(parent_session),
Arc::clone(parent_ctx),
new_guardian_review_id(),
@@ -730,36 +729,6 @@ async fn maybe_auto_review_mcp_request_user_input(
})
}
fn spawn_guardian_review(
session: Arc<Session>,
turn: Arc<TurnContext>,
review_id: String,
request: GuardianApprovalRequest,
retry_reason: Option<String>,
cancel_token: CancellationToken,
) -> oneshot::Receiver<ReviewDecision> {
let (tx, rx) = oneshot::channel();
std::thread::spawn(move || {
let Ok(runtime) = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
else {
let _ = tx.send(ReviewDecision::Denied);
return;
};
let decision = runtime.block_on(review_approval_request_with_cancel(
&session,
&turn,
review_id,
request,
retry_reason,
cancel_token,
));
let _ = tx.send(decision);
});
rx
}
async fn handle_request_permissions(
codex: &Codex,
parent_session: &Arc<Session>,
@@ -772,7 +741,8 @@ async fn handle_request_permissions(
reason: event.reason,
permissions: event.permissions,
};
let response_fut = parent_session.request_permissions(parent_ctx, call_id.clone(), args);
let response_fut =
parent_session.request_permissions(parent_ctx, call_id.clone(), args, cancel_token.clone());
let response =
await_request_permissions_with_cancel(response_fut, parent_session, &call_id, cancel_token)
.await;
+1
View File
@@ -198,6 +198,7 @@ fn should_use_remote_compact_task_for_azure_provider() {
env_key_instructions: None,
experimental_bearer_token: None,
auth: None,
aws: None,
wire_api: WireApi::Responses,
query_params: None,
http_headers: None,
+137 -24
View File
@@ -232,7 +232,7 @@ max_rollout_age_days = 42
max_rollouts_per_startup = 9
min_rollout_idle_hours = 24
extract_model = "gpt-5-mini"
consolidation_model = "gpt-5"
consolidation_model = "gpt-5.2"
"#;
let memories_cfg =
toml::from_str::<ConfigToml>(memories).expect("TOML deserialization should succeed");
@@ -247,7 +247,7 @@ consolidation_model = "gpt-5"
max_rollouts_per_startup: Some(9),
min_rollout_idle_hours: Some(24),
extract_model: Some("gpt-5-mini".to_string()),
consolidation_model: Some("gpt-5".to_string()),
consolidation_model: Some("gpt-5.2".to_string()),
}),
memories_cfg.memories
);
@@ -271,7 +271,7 @@ consolidation_model = "gpt-5"
max_rollouts_per_startup: 9,
min_rollout_idle_hours: 24,
extract_model: Some("gpt-5-mini".to_string()),
consolidation_model: Some("gpt-5".to_string()),
consolidation_model: Some("gpt-5.2".to_string()),
}
);
@@ -369,6 +369,108 @@ command = "print-token"
);
}
#[test]
fn rejects_provider_aws_for_custom_provider() {
let err = toml::from_str::<ConfigToml>(
r#"
[model_providers.custom]
name = "Custom Provider"
[model_providers.custom.aws]
profile = "codex-bedrock"
"#,
)
.unwrap_err();
assert!(
err.to_string().contains(
"model_providers.custom: provider aws is only supported for `amazon-bedrock`"
)
);
}
#[test]
fn accepts_amazon_bedrock_aws_profile_override() {
let cfg = toml::from_str::<ConfigToml>(
r#"
[model_providers.amazon-bedrock.aws]
profile = "codex-bedrock"
"#,
)
.expect("Amazon Bedrock AWS profile override should deserialize");
assert_eq!(
cfg.model_providers
.get("amazon-bedrock")
.and_then(|provider| provider.aws.as_ref())
.and_then(|aws| aws.profile.as_deref()),
Some("codex-bedrock")
);
}
#[tokio::test]
async fn load_config_applies_amazon_bedrock_aws_profile_override() {
let cfg = toml::from_str::<ConfigToml>(
r#"
model_provider = "amazon-bedrock"
[model_providers.amazon-bedrock.aws]
profile = "codex-bedrock"
"#,
)
.expect("Amazon Bedrock AWS profile override should deserialize");
let config = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
tempdir().expect("tempdir").abs(),
)
.await
.expect("load config");
assert_eq!(config.model_provider_id, "amazon-bedrock");
assert_eq!(
config
.model_provider
.aws
.as_ref()
.and_then(|aws| aws.profile.as_deref()),
Some("codex-bedrock")
);
}
#[tokio::test]
async fn load_config_rejects_unsupported_amazon_bedrock_overrides() {
let cfg = toml::from_str::<ConfigToml>(
r#"
model_provider = "amazon-bedrock"
[model_providers.amazon-bedrock]
name = "Custom Bedrock"
base_url = "https://bedrock.example.com/v1"
requires_openai_auth = true
supports_websockets = true
[model_providers.amazon-bedrock.aws]
profile = "codex-bedrock"
"#,
)
.expect("Amazon Bedrock unsupported overrides should deserialize");
let err = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
tempdir().expect("tempdir").abs(),
)
.await
.unwrap_err();
assert_eq!(err.kind(), std::io::ErrorKind::InvalidData);
assert!(err.to_string().contains(
"model_providers.amazon-bedrock only supports changing `aws.profile`; other non-default provider fields are not supported"
));
}
#[test]
fn config_toml_deserializes_model_availability_nux() {
let toml = r#"
@@ -2110,6 +2212,8 @@ async fn managed_config_overrides_oauth_store_mode() -> anyhow::Result<()> {
&Vec::new(),
overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
let cfg =
@@ -2244,6 +2348,8 @@ async fn managed_config_wins_over_cli_overrides() -> anyhow::Result<()> {
&[("model".to_string(), TomlValue::String("cli".to_string()))],
overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -3229,14 +3335,14 @@ async fn set_model_updates_defaults() -> anyhow::Result<()> {
let codex_home = TempDir::new()?;
ConfigEditsBuilder::new(codex_home.path())
.set_model(Some("gpt-5.1-codex"), Some(ReasoningEffort::High))
.set_model(Some("gpt-5.4"), Some(ReasoningEffort::High))
.apply()
.await?;
let serialized = tokio::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).await?;
let parsed: ConfigToml = toml::from_str(&serialized)?;
assert_eq!(parsed.model.as_deref(), Some("gpt-5.1-codex"));
assert_eq!(parsed.model.as_deref(), Some("gpt-5.4"));
assert_eq!(parsed.model_reasoning_effort, Some(ReasoningEffort::High));
Ok(())
@@ -3250,7 +3356,7 @@ async fn set_model_overwrites_existing_model() -> anyhow::Result<()> {
tokio::fs::write(
&config_path,
r#"
model = "gpt-5.1-codex"
model = "gpt-5.4"
model_reasoning_effort = "medium"
[profiles.dev]
@@ -3286,7 +3392,7 @@ async fn set_model_updates_profile() -> anyhow::Result<()> {
ConfigEditsBuilder::new(codex_home.path())
.with_profile(Some("dev"))
.set_model(Some("gpt-5.1-codex"), Some(ReasoningEffort::Medium))
.set_model(Some("gpt-5.4"), Some(ReasoningEffort::Medium))
.apply()
.await?;
@@ -3297,7 +3403,7 @@ async fn set_model_updates_profile() -> anyhow::Result<()> {
.get("dev")
.expect("profile should be created");
assert_eq!(profile.model.as_deref(), Some("gpt-5.1-codex"));
assert_eq!(profile.model.as_deref(), Some("gpt-5.4"));
assert_eq!(
profile.model_reasoning_effort,
Some(ReasoningEffort::Medium)
@@ -3319,7 +3425,7 @@ model = "gpt-4"
model_reasoning_effort = "medium"
[profiles.prod]
model = "gpt-5.1-codex"
model = "gpt-5.4"
"#,
)
.await?;
@@ -3348,7 +3454,7 @@ model = "gpt-5.1-codex"
.profiles
.get("prod")
.and_then(|profile| profile.model.as_deref()),
Some("gpt-5.1-codex"),
Some("gpt-5.4"),
);
Ok(())
@@ -3713,7 +3819,7 @@ async fn agent_role_file_metadata_overrides_config_toml_metadata() -> std::io::R
description = "Role metadata from file"
nickname_candidates = ["Hypatia"]
developer_instructions = "Research carefully"
model = "gpt-5"
model = "gpt-5.2"
"#,
)
.await?;
@@ -3775,7 +3881,7 @@ trust_level = "trusted"
r#"
name = "researcher"
description = "Role metadata from file"
model = "gpt-5"
model = "gpt-5.2"
"#,
)
.await?;
@@ -3785,7 +3891,7 @@ model = "gpt-5"
name = "reviewer"
description = "Review role"
developer_instructions = "Review carefully"
model = "gpt-5"
model = "gpt-5.2"
"#,
)
.await?;
@@ -3830,7 +3936,7 @@ async fn legacy_agent_role_config_file_allows_missing_developer_instructions() -
tokio::fs::write(
&role_config_path,
r#"
model = "gpt-5"
model = "gpt-5.2"
model_reasoning_effort = "high"
"#,
)
@@ -3882,7 +3988,7 @@ async fn agent_role_without_description_after_merge_is_dropped_with_warning() ->
&role_config_path,
r#"
developer_instructions = "Research carefully"
model = "gpt-5"
model = "gpt-5.2"
"#,
)
.await?;
@@ -4001,7 +4107,7 @@ async fn agent_role_file_name_takes_precedence_over_config_key() -> std::io::Res
name = "archivist"
description = "Role metadata from file"
developer_instructions = "Research carefully"
model = "gpt-5"
model = "gpt-5.2"
"#,
)
.await?;
@@ -4286,7 +4392,7 @@ nickname_candidates = ["Ada"]
home_agents_dir.join("researcher.toml"),
r#"
developer_instructions = "Research carefully"
model = "gpt-5"
model = "gpt-5.2"
"#,
)
.await?;
@@ -4319,7 +4425,7 @@ name = "writer"
description = "Writer role from file"
nickname_candidates = ["Sagan"]
developer_instructions = "Write carefully"
model = "gpt-5"
model = "gpt-5.2"
"#,
)
.await?;
@@ -4426,7 +4532,7 @@ config_file = "./agents/researcher.toml"
home_agents_dir.join("researcher.toml"),
r#"
developer_instructions = "Research carefully"
model = "gpt-5"
model = "gpt-5.2"
"#,
)
.await?;
@@ -4725,7 +4831,7 @@ approval_policy = "on-failure"
enabled = false
[profiles.gpt5]
model = "gpt-5.1"
model = "gpt-5.4"
model_provider = "openai"
approval_policy = "on-failure"
model_reasoning_effort = "high"
@@ -4753,6 +4859,7 @@ model_verbosity = "high"
env_key_instructions: None,
experimental_bearer_token: None,
auth: None,
aws: None,
query_params: None,
http_headers: None,
env_http_headers: None,
@@ -4887,6 +4994,7 @@ async fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
realtime: RealtimeConfig::default(),
experimental_realtime_ws_backend_prompt: None,
experimental_realtime_ws_startup_context: None,
experimental_thread_store_endpoint: None,
base_instructions: None,
developer_instructions: None,
guardian_policy_config: None,
@@ -5038,6 +5146,7 @@ async fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
realtime: RealtimeConfig::default(),
experimental_realtime_ws_backend_prompt: None,
experimental_realtime_ws_startup_context: None,
experimental_thread_store_endpoint: None,
base_instructions: None,
developer_instructions: None,
guardian_policy_config: None,
@@ -5187,6 +5296,7 @@ async fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
realtime: RealtimeConfig::default(),
experimental_realtime_ws_backend_prompt: None,
experimental_realtime_ws_startup_context: None,
experimental_thread_store_endpoint: None,
base_instructions: None,
developer_instructions: None,
guardian_policy_config: None,
@@ -5248,7 +5358,7 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
)
.await?;
let expected_gpt5_profile_config = Config {
model: Some("gpt-5.1".to_string()),
model: Some("gpt-5.4".to_string()),
review_model: None,
model_context_window: None,
model_auto_compact_token_limit: None,
@@ -5321,6 +5431,7 @@ async fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
realtime: RealtimeConfig::default(),
experimental_realtime_ws_backend_prompt: None,
experimental_realtime_ws_startup_context: None,
experimental_thread_store_endpoint: None,
base_instructions: None,
developer_instructions: None,
guardian_policy_config: None,
@@ -5375,6 +5486,7 @@ async fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset()
allowed_approval_policies: None,
allowed_approvals_reviewers: None,
allowed_sandbox_modes: None,
remote_sandbox_config: None,
allowed_web_search_modes: Some(vec![
crate::config_loader::WebSearchModeRequirement::Cached,
]),
@@ -5898,7 +6010,7 @@ fn config_toml_deserializes_mcp_oauth_callback_url() {
async fn config_loads_mcp_oauth_callback_port_from_toml() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let toml = r#"
model = "gpt-5.1"
model = "gpt-5.4"
mcp_oauth_callback_port = 5678
"#;
let cfg: ConfigToml =
@@ -5920,7 +6032,7 @@ async fn config_loads_allow_login_shell_from_toml() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let cfg: ConfigToml = toml::from_str(
r#"
model = "gpt-5.1"
model = "gpt-5.4"
allow_login_shell = false
"#,
)
@@ -5941,7 +6053,7 @@ allow_login_shell = false
async fn config_loads_mcp_oauth_callback_url_from_toml() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let toml = r#"
model = "gpt-5.1"
model = "gpt-5.4"
mcp_oauth_callback_url = "https://example.com/callback"
"#;
let cfg: ConfigToml =
@@ -6051,6 +6163,7 @@ async fn explicit_sandbox_mode_falls_back_when_disallowed_by_requirements() -> s
allowed_approval_policies: None,
allowed_approvals_reviewers: None,
allowed_sandbox_modes: Some(vec![crate::config_loader::SandboxModeRequirement::ReadOnly]),
remote_sandbox_config: None,
allowed_web_search_modes: None,
feature_requirements: None,
mcp_servers: None,
+14 -14
View File
@@ -18,14 +18,14 @@ fn blocking_set_model_top_level() {
codex_home,
/*profile*/ None,
&[ConfigEdit::SetModel {
model: Some("gpt-5.1-codex".to_string()),
model: Some("gpt-5.4".to_string()),
effort: Some(ReasoningEffort::High),
}],
)
.expect("persist");
let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
let expected = r#"model = "gpt-5.1-codex"
let expected = r#"model = "gpt-5.4"
model_reasoning_effort = "high"
"#;
assert_eq!(contents, expected);
@@ -197,7 +197,7 @@ fn blocking_set_model_writes_through_symlink_chain() {
codex_home,
/*profile*/ None,
&[ConfigEdit::SetModel {
model: Some("gpt-5.1-codex".to_string()),
model: Some("gpt-5.4".to_string()),
effort: Some(ReasoningEffort::High),
}],
)
@@ -207,7 +207,7 @@ fn blocking_set_model_writes_through_symlink_chain() {
assert!(meta.file_type().is_symlink());
let contents = std::fs::read_to_string(&target_path).expect("read target");
let expected = r#"model = "gpt-5.1-codex"
let expected = r#"model = "gpt-5.4"
model_reasoning_effort = "high"
"#;
assert_eq!(contents, expected);
@@ -230,7 +230,7 @@ fn blocking_set_model_replaces_symlink_on_cycle() {
codex_home,
/*profile*/ None,
&[ConfigEdit::SetModel {
model: Some("gpt-5.1-codex".to_string()),
model: Some("gpt-5.4".to_string()),
effort: None,
}],
)
@@ -240,7 +240,7 @@ fn blocking_set_model_replaces_symlink_on_cycle() {
assert!(!meta.file_type().is_symlink());
let contents = std::fs::read_to_string(&config_path).expect("read config");
let expected = r#"model = "gpt-5.1-codex"
let expected = r#"model = "gpt-5.4"
"#;
assert_eq!(contents, expected);
}
@@ -381,7 +381,7 @@ fn blocking_set_model_with_explicit_profile() {
std::fs::write(
codex_home.join(CONFIG_TOML_FILE),
r#"[profiles."team a"]
model = "gpt-5.1-codex"
model = "gpt-5.4"
"#,
)
.expect("seed");
@@ -536,8 +536,8 @@ existing = "value"
codex_home,
/*profile*/ None,
&[ConfigEdit::RecordModelMigrationSeen {
from: "gpt-5".to_string(),
to: "gpt-5.1".to_string(),
from: "gpt-5.2".to_string(),
to: "gpt-5.4".to_string(),
}],
)
.expect("persist");
@@ -547,7 +547,7 @@ existing = "value"
existing = "value"
[notice.model_migrations]
gpt-5 = "gpt-5.1"
"gpt-5.2" = "gpt-5.4"
"#;
assert_eq!(contents, expected);
}
@@ -1102,13 +1102,13 @@ async fn async_builder_set_model_persists() {
let codex_home = tmp.path().to_path_buf();
ConfigEditsBuilder::new(&codex_home)
.set_model(Some("gpt-5.1-codex"), Some(ReasoningEffort::High))
.set_model(Some("gpt-5.4"), Some(ReasoningEffort::High))
.apply()
.await
.expect("persist");
let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
let expected = r#"model = "gpt-5.1-codex"
let expected = r#"model = "gpt-5.4"
model_reasoning_effort = "high"
"#;
assert_eq!(contents, expected);
@@ -1130,11 +1130,11 @@ model_reasoning_effort = "low"
std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
assert_eq!(contents, initial_expected);
let updated_expected = r#"model = "gpt-5.1-codex"
let updated_expected = r#"model = "gpt-5.4"
model_reasoning_effort = "high"
"#;
ConfigEditsBuilder::new(codex_home)
.set_model(Some("gpt-5.1-codex"), Some(ReasoningEffort::High))
.set_model(Some("gpt-5.4"), Some(ReasoningEffort::High))
.apply_blocking()
.expect("persist update");
contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
+64 -7
View File
@@ -21,6 +21,7 @@ use crate::unified_exec::MIN_EMPTY_YIELD_TIME_MS;
use crate::windows_sandbox::WindowsSandboxLevelExt;
use crate::windows_sandbox::resolve_windows_sandbox_mode;
use crate::windows_sandbox::resolve_windows_sandbox_private_desktop;
use codex_config::ThreadConfigLoader;
use codex_config::config_toml::ConfigToml;
use codex_config::config_toml::ProjectConfig;
use codex_config::config_toml::RealtimeAudioConfig;
@@ -49,7 +50,7 @@ use codex_config::types::UriBasedFileOpener;
use codex_config::types::WindowsSandboxModeToml;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::LOCAL_FS;
use codex_features::Feature;
pub use codex_features::Feature;
use codex_features::FeatureConfigSource;
use codex_features::FeatureOverrides;
use codex_features::FeatureToml;
@@ -58,11 +59,13 @@ use codex_features::FeaturesToml;
use codex_features::MultiAgentV2ConfigToml;
use codex_git_utils::resolve_root_git_project_for_trust;
use codex_login::AuthManagerConfig;
use codex_login::BackgroundAgentTaskAuthMode;
use codex_mcp::McpConfig;
use codex_model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID;
use codex_model_provider_info::ModelProviderInfo;
use codex_model_provider_info::OLLAMA_CHAT_PROVIDER_REMOVED_ERROR;
use codex_model_provider_info::built_in_model_providers;
use codex_model_provider_info::merge_configured_model_providers;
use codex_models_manager::ModelsManagerConfig;
use codex_protocol::config_types::AltScreenMode;
use codex_protocol::config_types::ForcedLoginMethod;
@@ -89,6 +92,7 @@ use std::collections::HashMap;
use std::io::ErrorKind;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use crate::config::permissions::compile_permission_profile;
use crate::config::permissions::get_readable_roots_required_for_codex_runtime;
@@ -526,6 +530,10 @@ pub struct Config {
/// instructions inserted into developer messages when realtime becomes
/// active.
pub experimental_realtime_start_instructions: Option<String>,
/// Experimental / do not use. When set, app-server uses a remote thread
/// store at this endpoint instead of the local filesystem/SQLite store.
pub experimental_thread_store_endpoint: Option<String>,
/// When set, restricts ChatGPT login to a specific workspace identifier.
pub forced_chatgpt_workspace_id: Option<String>,
@@ -629,16 +637,43 @@ impl AuthManagerConfig for Config {
fn forced_chatgpt_workspace_id(&self) -> Option<String> {
self.forced_chatgpt_workspace_id.clone()
}
fn chatgpt_base_url(&self) -> Option<String> {
Some(self.chatgpt_base_url.clone())
}
fn background_agent_task_auth_mode(&self) -> BackgroundAgentTaskAuthMode {
BackgroundAgentTaskAuthMode::from_feature_enabled(
self.features.enabled(Feature::UseAgentIdentity),
)
}
}
#[derive(Debug, Clone, Default)]
#[derive(Clone)]
pub struct ConfigBuilder {
codex_home: Option<PathBuf>,
cli_overrides: Option<Vec<(String, TomlValue)>>,
harness_overrides: Option<ConfigOverrides>,
loader_overrides: Option<LoaderOverrides>,
cloud_requirements: CloudRequirementsLoader,
thread_config_loader: Option<Arc<dyn ThreadConfigLoader>>,
fallback_cwd: Option<PathBuf>,
host_name: Option<String>,
}
impl Default for ConfigBuilder {
fn default() -> Self {
Self {
codex_home: None,
cli_overrides: None,
harness_overrides: None,
loader_overrides: None,
cloud_requirements: CloudRequirementsLoader::default(),
thread_config_loader: None,
fallback_cwd: None,
host_name: codex_config::host_name(),
}
}
}
impl ConfigBuilder {
@@ -667,11 +702,24 @@ impl ConfigBuilder {
self
}
pub fn thread_config_loader(
mut self,
thread_config_loader: Arc<dyn ThreadConfigLoader>,
) -> Self {
self.thread_config_loader = Some(thread_config_loader);
self
}
pub fn fallback_cwd(mut self, fallback_cwd: Option<PathBuf>) -> Self {
self.fallback_cwd = fallback_cwd;
self
}
pub fn host_name(mut self, host_name: Option<String>) -> Self {
self.host_name = host_name;
self
}
pub async fn build(self) -> std::io::Result<Config> {
let Self {
codex_home,
@@ -679,7 +727,9 @@ impl ConfigBuilder {
harness_overrides,
loader_overrides,
cloud_requirements,
thread_config_loader,
fallback_cwd,
host_name,
} = self;
let codex_home = match codex_home {
Some(codex_home) => AbsolutePathBuf::from_absolute_path(codex_home)?,
@@ -701,6 +751,10 @@ impl ConfigBuilder {
&cli_overrides,
loader_overrides,
cloud_requirements,
thread_config_loader
.as_deref()
.unwrap_or(&codex_config::NoopThreadConfigLoader),
host_name.as_deref(),
)
.await?;
let merged_toml = config_layer_stack.effective_config();
@@ -879,6 +933,8 @@ pub async fn load_config_as_toml_with_cli_and_loader_overrides(
&cli_overrides,
loader_overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -1050,6 +1106,8 @@ pub async fn load_global_mcp_servers(
&cli_overrides,
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
let merged_toml = config_layer_stack.effective_config();
@@ -1789,11 +1847,9 @@ impl Config {
.clone()
.filter(|value| !value.is_empty());
let mut model_providers = built_in_model_providers(openai_base_url);
// Merge user-defined providers into the built-in list.
for (key, provider) in cfg.model_providers.into_iter() {
model_providers.entry(key).or_insert(provider);
}
let model_providers =
merge_configured_model_providers(built_in_model_providers(openai_base_url), cfg.model_providers)
.map_err(|message| std::io::Error::new(std::io::ErrorKind::InvalidData, message))?;
let model_provider_id = model_provider
.or(config_profile.model_provider)
@@ -2262,6 +2318,7 @@ impl Config {
experimental_realtime_ws_backend_prompt: cfg.experimental_realtime_ws_backend_prompt,
experimental_realtime_ws_startup_context: cfg.experimental_realtime_ws_startup_context,
experimental_realtime_start_instructions: cfg.experimental_realtime_start_instructions,
experimental_thread_store_endpoint: cfg.experimental_thread_store_endpoint,
forced_chatgpt_workspace_id,
forced_login_method,
include_apply_patch_tool: include_apply_patch_tool_flag,
+8
View File
@@ -115,6 +115,7 @@ pub struct ConfigService {
cli_overrides: Vec<(String, TomlValue)>,
loader_overrides: LoaderOverrides,
cloud_requirements: CloudRequirementsLoader,
host_name: Option<String>,
}
impl ConfigService {
@@ -123,12 +124,14 @@ impl ConfigService {
cli_overrides: Vec<(String, TomlValue)>,
loader_overrides: LoaderOverrides,
cloud_requirements: CloudRequirementsLoader,
host_name: Option<String>,
) -> Self {
Self {
codex_home,
cli_overrides,
loader_overrides,
cloud_requirements,
host_name,
}
}
@@ -138,6 +141,7 @@ impl ConfigService {
cli_overrides: Vec::new(),
loader_overrides: LoaderOverrides::default(),
cloud_requirements: CloudRequirementsLoader::default(),
host_name: codex_config::host_name(),
}
}
@@ -148,6 +152,7 @@ impl ConfigService {
Vec::new(),
LoaderOverrides::without_managed_config_for_tests(),
CloudRequirementsLoader::default(),
/*host_name*/ None,
)
}
@@ -166,6 +171,7 @@ impl ConfigService {
.loader_overrides(self.loader_overrides.clone())
.fallback_cwd(Some(cwd.to_path_buf()))
.cloud_requirements(self.cloud_requirements.clone())
.host_name(self.host_name.clone())
.build()
.await
.map_err(|err| {
@@ -431,6 +437,8 @@ impl ConfigService {
&self.cli_overrides,
self.loader_overrides.clone(),
self.cloud_requirements.clone(),
&codex_config::NoopThreadConfigLoader,
self.host_name.as_deref(),
)
.await
}
+11 -3
View File
@@ -62,7 +62,7 @@ X-Doc = "42"
async fn write_value_preserves_comments_and_order() -> Result<()> {
let tmp = tempdir().expect("tempdir");
let original = r#"# Codex user configuration
model = "gpt-5"
model = "gpt-5.2"
approval_policy = "on-request"
[notice]
@@ -88,7 +88,7 @@ unified_exec = true
let updated = std::fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).expect("read config");
let expected = r#"# Codex user configuration
model = "gpt-5"
model = "gpt-5.2"
approval_policy = "on-request"
[notice]
@@ -223,6 +223,7 @@ async fn read_includes_origins_and_layers() {
vec![],
LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()),
CloudRequirementsLoader::default(),
/*host_name*/ None,
);
let response = service
@@ -301,6 +302,7 @@ writable_roots = ["~/code"]
vec![],
loader_overrides,
CloudRequirementsLoader::default(),
/*host_name*/ None,
);
let response = service
@@ -341,6 +343,7 @@ async fn write_value_reports_override() {
vec![],
LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()),
CloudRequirementsLoader::default(),
/*host_name*/ None,
);
let result = service
@@ -390,7 +393,7 @@ async fn version_conflict_rejected() {
.write_value(ConfigValueWriteParams {
file_path: Some(tmp.path().join(CONFIG_TOML_FILE).display().to_string()),
key_path: "model".to_string(),
value: serde_json::json!("gpt-5"),
value: serde_json::json!("gpt-5.2"),
merge_strategy: MergeStrategy::Replace,
expected_version: Some("sha256:bogus".to_string()),
})
@@ -440,6 +443,7 @@ async fn invalid_user_value_rejected_even_if_overridden_by_managed() {
vec![],
LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()),
CloudRequirementsLoader::default(),
/*host_name*/ None,
);
let error = service
@@ -507,6 +511,7 @@ async fn write_value_rejects_feature_requirement_conflict() {
..Default::default()
}))
}),
/*host_name*/ None,
);
let error = service
@@ -553,6 +558,7 @@ async fn write_value_rejects_profile_feature_requirement_conflict() {
..Default::default()
}))
}),
/*host_name*/ None,
);
let error = service
@@ -603,6 +609,7 @@ async fn read_reports_managed_overrides_user_and_session_flags() {
cli_overrides,
LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()),
CloudRequirementsLoader::default(),
/*host_name*/ None,
);
let response = service
@@ -656,6 +663,7 @@ async fn write_value_reports_managed_override() {
vec![],
LoaderOverrides::with_managed_config_path_for_tests(managed_path.clone()),
CloudRequirementsLoader::default(),
/*host_name*/ None,
);
let result = service
+8 -4
View File
@@ -10,7 +10,7 @@ This module is the canonical place to **load and describe Codex configuration la
Exported from `codex_core::config_loader`:
- `load_config_layers_state(fs, codex_home, cwd_opt, cli_overrides, overrides, cloud_requirements) -> ConfigLayerStack`
- `load_config_layers_state(fs, codex_home, cwd_opt, cli_overrides, overrides, cloud_requirements, thread_config_loader, host_name) -> ConfigLayerStack`
- `ConfigLayerStack`
- `effective_config() -> toml::Value`
- `origins() -> HashMap<String, ConfigLayerMetadata>`
@@ -29,6 +29,9 @@ Precedence is **top overrides bottom**:
3. **Session flags** (CLI overrides, applied as dotted-path TOML writes)
4. **User** config (`config.toml`)
Thread config entries supplied by `thread_config_loader` are inserted according
to their translated `ConfigLayerSource` precedence.
Layers with a `disabled_reason` are still surfaced for UI, but are ignored when
computing the effective config and origins metadata. This is what
`ConfigLayerStack::effective_config()` implements.
@@ -38,9 +41,8 @@ computing the effective config and origins metadata. This is what
Most callers want the effective config plus metadata:
```rust
use codex_core::config_loader::{
CloudRequirementsLoader, LoaderOverrides, load_config_layers_state,
};
use codex_core::config_loader::{CloudRequirementsLoader, LoaderOverrides, load_config_layers_state};
use codex_config::NoopThreadConfigLoader;
use codex_exec_server::LOCAL_FS;
use codex_utils_absolute_path::AbsolutePathBuf;
use toml::Value as TomlValue;
@@ -54,6 +56,8 @@ let layers = load_config_layers_state(
&cli_overrides,
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&NoopThreadConfigLoader,
/*host_name*/ None,
).await?;
let effective = layers.effective_config();
+11 -2
View File
@@ -1,6 +1,7 @@
use super::ConfigRequirementsToml;
use super::ConfigRequirementsWithSources;
use super::RequirementSource;
use super::merge_requirements_with_remote_sandbox_config;
use base64::Engine;
use base64::prelude::BASE64_STANDARD;
use core_foundation::base::TCFType;
@@ -64,6 +65,7 @@ fn load_managed_admin_config() -> io::Result<Option<ManagedAdminConfigLayer>> {
pub(crate) async fn load_managed_admin_requirements_toml(
target: &mut ConfigRequirementsWithSources,
override_base64: Option<&str>,
host_name: Option<&str>,
) -> io::Result<()> {
if let Some(encoded) = override_base64 {
let trimmed = encoded.trim();
@@ -71,9 +73,11 @@ pub(crate) async fn load_managed_admin_requirements_toml(
return Ok(());
}
target.merge_unset_fields(
merge_requirements_with_remote_sandbox_config(
target,
managed_preferences_requirements_source(),
parse_managed_requirements_base64(trimmed)?,
host_name,
);
return Ok(());
}
@@ -81,7 +85,12 @@ pub(crate) async fn load_managed_admin_requirements_toml(
match task::spawn_blocking(load_managed_admin_requirements).await {
Ok(result) => {
if let Some(requirements) = result? {
target.merge_unset_fields(managed_preferences_requirements_source(), requirements);
merge_requirements_with_remote_sandbox_config(
target,
managed_preferences_requirements_source(),
requirements,
host_name,
);
}
Ok(())
}
+65 -6
View File
@@ -9,6 +9,8 @@ use crate::config_loader::layer_io::LoadedConfigLayers;
use codex_app_server_protocol::ConfigLayerSource;
use codex_config::CONFIG_TOML_FILE;
use codex_config::ConfigRequirementsWithSources;
use codex_config::ThreadConfigContext;
use codex_config::ThreadConfigLoader;
use codex_config::config_toml::ConfigToml;
use codex_config::config_toml::ProjectConfig;
use codex_exec_server::ExecutorFileSystem;
@@ -52,6 +54,7 @@ pub use codex_config::NetworkDomainPermissionsToml;
pub use codex_config::NetworkRequirementsToml;
pub use codex_config::NetworkUnixSocketPermissionToml;
pub use codex_config::NetworkUnixSocketPermissionsToml;
pub use codex_config::RemoteSandboxConfigToml;
pub use codex_config::RequirementSource;
pub use codex_config::ResidencyRequirement;
pub use codex_config::SandboxModeRequirement;
@@ -120,6 +123,7 @@ pub(crate) async fn first_layer_config_error_from_entries(
/// associated with it such that `cwd` should be `Some(...)`. Only for
/// thread-agnostic config loading (e.g., for the app server's `/config`
/// endpoint) should `cwd` be `None`.
#[allow(clippy::too_many_arguments)]
pub async fn load_config_layers_state(
fs: &dyn ExecutorFileSystem,
codex_home: &Path,
@@ -127,6 +131,8 @@ pub async fn load_config_layers_state(
cli_overrides: &[(String, TomlValue)],
overrides: LoaderOverrides,
cloud_requirements: CloudRequirementsLoader,
thread_config_loader: &dyn ThreadConfigLoader,
host_name: Option<&str>,
) -> io::Result<ConfigLayerStack> {
let ignore_user_config = overrides.ignore_user_config;
let ignore_user_and_project_exec_policy_rules =
@@ -134,8 +140,12 @@ pub async fn load_config_layers_state(
let mut config_requirements_toml = ConfigRequirementsWithSources::default();
if let Some(requirements) = cloud_requirements.get().await.map_err(io::Error::other)? {
config_requirements_toml
.merge_unset_fields(RequirementSource::CloudRequirements, requirements);
merge_requirements_with_remote_sandbox_config(
&mut config_requirements_toml,
RequirementSource::CloudRequirements,
requirements,
host_name,
);
}
#[cfg(target_os = "macos")]
@@ -144,12 +154,19 @@ pub async fn load_config_layers_state(
overrides
.macos_managed_config_requirements_base64
.as_deref(),
host_name,
)
.await?;
// Honor the system requirements.toml location.
let requirements_toml_file = system_requirements_toml_file()?;
load_requirements_toml(fs, &mut config_requirements_toml, &requirements_toml_file).await?;
load_requirements_toml(
fs,
&mut config_requirements_toml,
&requirements_toml_file,
host_name,
)
.await?;
// Make a best-effort to support the legacy `managed_config.toml` as a
// requirements specification.
@@ -158,9 +175,19 @@ pub async fn load_config_layers_state(
load_requirements_from_legacy_scheme(
&mut config_requirements_toml,
loaded_config_layers.clone(),
host_name,
)
.await?;
let thread_config_context = ThreadConfigContext {
thread_id: None,
cwd: cwd.clone(),
};
let thread_config_layers = thread_config_loader
.load_config_layers(thread_config_context)
.await
.map_err(io::Error::other)?;
let mut layers = Vec::<ConfigLayerEntry>::new();
let cli_overrides_layer = if cli_overrides.is_empty() {
@@ -283,6 +310,10 @@ pub async fn load_config_layers_state(
));
}
for thread_config_layer in thread_config_layers {
insert_layer_by_precedence(&mut layers, thread_config_layer);
}
// Make a best-effort to support the legacy `managed_config.toml` as a
// config layer on top of everything else. For fields in
// `managed_config.toml` that do not have an equivalent in
@@ -332,6 +363,16 @@ pub async fn load_config_layers_state(
.with_user_and_project_exec_policy_rules_ignored(ignore_user_and_project_exec_policy_rules))
}
fn insert_layer_by_precedence(layers: &mut Vec<ConfigLayerEntry>, layer: ConfigLayerEntry) {
match layers
.iter()
.position(|existing| existing.name.precedence() > layer.name.precedence())
{
Some(index) => layers.insert(index, layer),
None => layers.push(layer),
}
}
/// Attempts to load a config.toml file from `config_toml`.
/// - If the file exists and is valid TOML, passes the parsed `toml::Value` to
/// `create_entry` and returns the resulting layer entry.
@@ -387,6 +428,7 @@ async fn load_requirements_toml(
fs: &dyn ExecutorFileSystem,
config_requirements_toml: &mut ConfigRequirementsWithSources,
requirements_toml_file: &AbsolutePathBuf,
host_name: Option<&str>,
) -> io::Result<()> {
match fs
.read_file_text(requirements_toml_file, /*sandbox*/ None)
@@ -413,11 +455,13 @@ async fn load_requirements_toml(
),
)
})?;
config_requirements_toml.merge_unset_fields(
merge_requirements_with_remote_sandbox_config(
config_requirements_toml,
RequirementSource::SystemRequirementsToml {
file: requirements_toml_file.clone(),
},
requirements_config,
host_name,
);
}
Err(e) => {
@@ -532,6 +576,7 @@ fn windows_program_data_dir_from_known_folder() -> io::Result<PathBuf> {
async fn load_requirements_from_legacy_scheme(
config_requirements_toml: &mut ConfigRequirementsWithSources,
loaded_config_layers: LoadedConfigLayers,
host_name: Option<&str>,
) -> io::Result<()> {
// In this implementation, earlier layers cannot be overwritten by later
// layers, so list managed_config_from_mdm first because it has the highest
@@ -564,13 +609,27 @@ async fn load_requirements_from_legacy_scheme(
)
})?;
let new_requirements_toml = ConfigRequirementsToml::from(legacy_config);
config_requirements_toml.merge_unset_fields(source, new_requirements_toml);
merge_requirements_with_remote_sandbox_config(
config_requirements_toml,
source,
ConfigRequirementsToml::from(legacy_config),
host_name,
);
}
Ok(())
}
pub(super) fn merge_requirements_with_remote_sandbox_config(
target: &mut ConfigRequirementsWithSources,
source: RequirementSource,
mut requirements: ConfigRequirementsToml,
host_name: Option<&str>,
) {
requirements.apply_remote_sandbox_config(host_name);
target.merge_unset_fields(source, requirements);
}
struct ProjectTrustContext {
project_root: AbsolutePathBuf,
project_root_key: String,
+202 -1
View File
@@ -15,13 +15,15 @@ use crate::config_loader::RequirementSource;
use crate::config_loader::load_requirements_toml;
use crate::config_loader::version_for_toml;
use codex_config::CONFIG_TOML_FILE;
use codex_config::SessionThreadConfig;
use codex_config::StaticThreadConfigLoader;
use codex_config::ThreadConfigSource;
use codex_config::config_toml::ConfigToml;
use codex_config::config_toml::ProjectConfig;
use codex_exec_server::LOCAL_FS;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::config_types::WebSearchMode;
use codex_protocol::protocol::AskForApproval;
#[cfg(target_os = "macos")]
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
@@ -100,6 +102,8 @@ async fn returns_config_error_for_invalid_user_config_toml() {
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await
.expect_err("expected error");
@@ -131,6 +135,8 @@ async fn ignore_user_config_keeps_empty_user_layer() -> std::io::Result<()> {
..Default::default()
},
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -161,6 +167,8 @@ async fn ignore_rules_marks_config_stack_for_exec_policy_rule_skip() -> std::io:
..Default::default()
},
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -185,6 +193,8 @@ async fn returns_config_error_for_invalid_managed_config_toml() {
&[] as &[(String, TomlValue)],
overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await
.expect_err("expected error");
@@ -270,6 +280,8 @@ extra = true
&[] as &[(String, TomlValue)],
overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await
.expect("load config");
@@ -303,6 +315,8 @@ async fn returns_empty_when_all_layers_missing() {
&[] as &[(String, TomlValue)],
overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await
.expect("load layers");
@@ -354,6 +368,57 @@ async fn returns_empty_when_all_layers_missing() {
}
}
#[tokio::test]
async fn includes_thread_config_layers_in_stack() -> anyhow::Result<()> {
let tmp = tempdir()?;
let cwd_dir = tmp.path().join("project");
tokio::fs::create_dir_all(&cwd_dir).await?;
let cwd = AbsolutePathBuf::from_absolute_path(&cwd_dir)?;
let layers = load_config_layers_state(
LOCAL_FS.as_ref(),
tmp.path(),
Some(cwd),
&[("features.plugins".to_string(), TomlValue::Boolean(true))],
LoaderOverrides::without_managed_config_for_tests(),
CloudRequirementsLoader::default(),
&StaticThreadConfigLoader::new(vec![ThreadConfigSource::Session(SessionThreadConfig {
features: BTreeMap::from([("plugins".to_string(), false)]),
..Default::default()
})]),
/*host_name*/ None,
)
.await?;
let layer_sources = layers
.layers_high_to_low()
.into_iter()
.map(|layer| layer.name.clone())
.collect::<Vec<_>>();
assert_eq!(
layer_sources,
vec![
super::ConfigLayerSource::SessionFlags,
super::ConfigLayerSource::SessionFlags,
super::ConfigLayerSource::User {
file: AbsolutePathBuf::resolve_path_against_base(CONFIG_TOML_FILE, tmp.path()),
},
super::ConfigLayerSource::System {
file: super::system_config_toml_file()?,
},
]
);
assert_eq!(
layers
.effective_config()
.get("features")
.and_then(TomlValue::as_table)
.and_then(|features| features.get("plugins")),
Some(&TomlValue::Boolean(false))
);
Ok(())
}
#[cfg(target_os = "macos")]
#[tokio::test]
async fn managed_preferences_take_highest_precedence() {
@@ -396,6 +461,8 @@ flag = false
&[] as &[(String, TomlValue)],
overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await
.expect("load config");
@@ -498,6 +565,8 @@ allowed_sandbox_modes = ["read-only"]
&[] as &[(String, TomlValue)],
loader_overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -560,6 +629,8 @@ allowed_approval_policies = ["never"]
&[] as &[(String, TomlValue)],
loader_overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -601,6 +672,7 @@ personality = true
LOCAL_FS.as_ref(),
&mut config_requirements_toml,
&requirements_file,
/*host_name*/ None,
)
.await?;
@@ -702,6 +774,7 @@ allowed_approval_policies = ["on-request"]
allowed_approval_policies: Some(vec![AskForApproval::Never]),
allowed_approvals_reviewers: None,
allowed_sandbox_modes: None,
remote_sandbox_config: None,
allowed_web_search_modes: None,
feature_requirements: None,
mcp_servers: None,
@@ -713,6 +786,8 @@ allowed_approval_policies = ["on-request"]
guardian_policy_config: None,
}))
}),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -755,6 +830,7 @@ allowed_approval_policies = ["on-request"]
allowed_approval_policies: Some(vec![AskForApproval::Never]),
allowed_approvals_reviewers: None,
allowed_sandbox_modes: None,
remote_sandbox_config: None,
allowed_web_search_modes: None,
feature_requirements: None,
mcp_servers: None,
@@ -770,6 +846,7 @@ allowed_approval_policies = ["on-request"]
LOCAL_FS.as_ref(),
&mut config_requirements_toml,
&AbsolutePathBuf::try_from(requirements_file)?,
/*host_name*/ None,
)
.await?;
@@ -791,6 +868,54 @@ allowed_approval_policies = ["on-request"]
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn system_remote_sandbox_config_keeps_cloud_sandbox_modes() -> anyhow::Result<()> {
let tmp = tempdir()?;
let requirements_file = tmp.path().join("requirements.toml");
tokio::fs::write(
&requirements_file,
r#"
[[remote_sandbox_config]]
hostname_patterns = ["runner-*.ci.example.com"]
allowed_sandbox_modes = ["read-only", "workspace-write"]
"#,
)
.await?;
let cloud_source = RequirementSource::CloudRequirements;
let mut config_requirements_toml = ConfigRequirementsWithSources::default();
config_requirements_toml.merge_unset_fields(
cloud_source.clone(),
toml::from_str(
r#"
allowed_sandbox_modes = ["read-only"]
"#,
)?,
);
load_requirements_toml(
LOCAL_FS.as_ref(),
&mut config_requirements_toml,
&AbsolutePathBuf::try_from(requirements_file)?,
Some("runner-01.ci.example.com"),
)
.await?;
let config_requirements: ConfigRequirements = config_requirements_toml.try_into()?;
assert_eq!(
config_requirements
.sandbox_policy
.can_set(&SandboxPolicy::new_workspace_write_policy()),
Err(ConstraintError::InvalidValue {
field_name: "sandbox_mode",
candidate: "WorkspaceWrite".into(),
allowed: "[ReadOnly]".into(),
requirement_source: cloud_source,
})
);
Ok(())
}
#[tokio::test(flavor = "current_thread")]
async fn load_requirements_toml_resolves_deny_read_against_parent() -> anyhow::Result<()> {
let tmp = tempdir()?;
@@ -812,6 +937,7 @@ deny_read = ["./sensitive", "../shared/secret.txt"]
LOCAL_FS.as_ref(),
&mut config_requirements_toml,
&requirements_file,
/*host_name*/ None,
)
.await?;
@@ -866,6 +992,7 @@ deny_read = ["./sensitive/**/*.txt"]
LOCAL_FS.as_ref(),
&mut config_requirements_toml,
&requirements_file,
/*host_name*/ None,
)
.await?;
@@ -911,6 +1038,7 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()>
allowed_approval_policies: Some(vec![AskForApproval::Never]),
allowed_approvals_reviewers: None,
allowed_sandbox_modes: None,
remote_sandbox_config: None,
allowed_web_search_modes: None,
feature_requirements: None,
mcp_servers: None,
@@ -931,6 +1059,8 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()>
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
cloud_requirements,
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -954,6 +1084,53 @@ async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()>
Ok(())
}
#[tokio::test]
async fn load_config_layers_applies_matching_remote_sandbox_config() -> anyhow::Result<()> {
let tmp = tempdir()?;
let codex_home = tmp.path().join("home");
tokio::fs::create_dir_all(&codex_home).await?;
let cwd = AbsolutePathBuf::from_absolute_path(tmp.path())?;
let requirements: ConfigRequirementsToml = toml::from_str(
r#"
allowed_sandbox_modes = ["read-only"]
[[remote_sandbox_config]]
hostname_patterns = ["runner-*.ci.example.com"]
allowed_sandbox_modes = ["read-only", "workspace-write"]
"#,
)?;
let cloud_requirements = CloudRequirementsLoader::new(async move { Ok(Some(requirements)) });
let layers = load_config_layers_state(
LOCAL_FS.as_ref(),
&codex_home,
Some(cwd),
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
cloud_requirements,
&codex_config::NoopThreadConfigLoader,
Some("runner-01.ci.example.com"),
)
.await?;
assert_eq!(
layers.requirements_toml().allowed_sandbox_modes,
Some(vec![
crate::config_loader::SandboxModeRequirement::ReadOnly,
crate::config_loader::SandboxModeRequirement::WorkspaceWrite,
])
);
assert!(
layers
.requirements()
.sandbox_policy
.can_set(&SandboxPolicy::new_workspace_write_policy())
.is_ok()
);
Ok(())
}
#[tokio::test]
async fn load_config_layers_fails_when_cloud_requirements_loader_fails() -> anyhow::Result<()> {
let tmp = tempdir()?;
@@ -974,6 +1151,8 @@ async fn load_config_layers_fails_when_cloud_requirements_loader_fails() -> anyh
"cloud requirements failed",
))
}),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await
.expect_err("cloud requirements failure should fail closed");
@@ -1021,6 +1200,8 @@ async fn project_layers_prefer_closest_cwd() -> std::io::Result<()> {
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -1166,6 +1347,8 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -1206,6 +1389,8 @@ async fn codex_home_is_not_loaded_as_project_layer_from_home_dir() -> std::io::R
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -1263,6 +1448,8 @@ async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Resul
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -1334,6 +1521,8 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result<
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
let project_layers_untrusted: Vec<_> = layers_untrusted
@@ -1373,6 +1562,8 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result<
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
let project_layers_unknown: Vec<_> = layers_unknown
@@ -1439,6 +1630,8 @@ async fn project_trust_does_not_match_configured_alias_for_canonical_cwd() -> st
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -1592,6 +1785,8 @@ async fn invalid_project_config_ignored_when_untrusted_or_unknown() -> std::io::
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
let project_layers: Vec<_> = layers
@@ -1660,6 +1855,8 @@ async fn project_layer_without_config_toml_is_disabled_when_untrusted_or_unknown
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
let project_layers: Vec<_> = layers
@@ -1720,6 +1917,8 @@ async fn cli_overrides_with_relative_paths_do_not_break_trust_check() -> std::io
&cli_overrides,
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
@@ -1763,6 +1962,8 @@ async fn project_root_markers_supports_alternate_markers() -> std::io::Result<()
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await?;
+40 -13
View File
@@ -43,7 +43,7 @@ use codex_mcp::ToolInfo;
use codex_mcp::ToolPluginProvenance;
use codex_mcp::codex_apps_tools_cache_key;
use codex_mcp::compute_auth_statuses;
use codex_mcp::with_codex_apps_mcp;
use codex_mcp::with_codex_apps_mcp_with_authorization_header;
const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30);
const DIRECTORY_CONNECTORS_TIMEOUT: Duration = Duration::from_secs(60);
@@ -220,8 +220,20 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status(
});
}
let background_authorization_header_value = if let Some(auth) = auth.as_ref() {
auth_manager
.chatgpt_authorization_header_for_auth(auth)
.await
} else {
None
};
let mcp_config = config.to_mcp_config(plugins_manager.as_ref()).await;
let mcp_servers = with_codex_apps_mcp(HashMap::new(), auth.as_ref(), &mcp_config);
let mcp_servers = with_codex_apps_mcp_with_authorization_header(
HashMap::new(),
auth.as_ref(),
&mcp_config,
background_authorization_header_value.as_deref(),
);
if mcp_servers.is_empty() {
return Ok(AccessibleConnectorsStatus {
connectors: Vec::new(),
@@ -423,6 +435,18 @@ async fn list_directory_connectors_for_tool_suggest_with_auth(
};
let access_token = token_data.access_token.clone();
let account_id = account_id.to_string();
let is_fedramp_account = token_data.id_token.is_fedramp_account();
let authorization_header_value = {
let auth_manager =
AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false);
match auth {
Some(auth) if auth.is_chatgpt_auth() => auth_manager
.chatgpt_authorization_header_for_auth(auth)
.await
.unwrap_or_else(|| format!("Bearer {access_token}")),
_ => format!("Bearer {access_token}"),
}
};
let is_workspace_account = token_data.id_token.is_workspace_account();
let cache_key = AllConnectorsCacheKey::new(
config.chatgpt_base_url.clone(),
@@ -436,14 +460,15 @@ async fn list_directory_connectors_for_tool_suggest_with_auth(
is_workspace_account,
/*force_refetch*/ false,
|path| {
let access_token = access_token.clone();
let authorization_header_value = authorization_header_value.clone();
let account_id = account_id.clone();
async move {
chatgpt_get_request_with_token::<DirectoryListResponse>(
chatgpt_get_request_with_authorization_header::<DirectoryListResponse>(
config,
path,
access_token.as_str(),
authorization_header_value.as_str(),
account_id.as_str(),
is_fedramp_account,
)
.await
}
@@ -452,23 +477,25 @@ async fn list_directory_connectors_for_tool_suggest_with_auth(
.await
}
async fn chatgpt_get_request_with_token<T: DeserializeOwned>(
async fn chatgpt_get_request_with_authorization_header<T: DeserializeOwned>(
config: &Config,
path: String,
access_token: &str,
authorization_header_value: &str,
account_id: &str,
is_fedramp_account: bool,
) -> anyhow::Result<T> {
let client = create_client();
let url = format!("{}{}", config.chatgpt_base_url, path);
let response = client
let mut request = client
.get(&url)
.bearer_auth(access_token)
.header("authorization", authorization_header_value)
.header("chatgpt-account-id", account_id)
.header("Content-Type", "application/json")
.timeout(DIRECTORY_CONNECTORS_TIMEOUT)
.send()
.await
.context("failed to send request")?;
.timeout(DIRECTORY_CONNECTORS_TIMEOUT);
if is_fedramp_account {
request = request.header("X-OpenAI-Fedramp", "true");
}
let response = request.send().await.context("failed to send request")?;
if response.status().is_success() {
response
@@ -1,50 +1,47 @@
use codex_instructions::AGENTS_MD_FRAGMENT;
use codex_instructions::ContextualUserFragmentDefinition;
use codex_instructions::SKILL_FRAGMENT;
use codex_protocol::items::HookPromptItem;
use codex_protocol::items::parse_hook_prompt_fragment;
use codex_protocol::models::ContentItem;
use codex_protocol::protocol::ENVIRONMENT_CONTEXT_CLOSE_TAG;
use codex_protocol::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG;
pub(crate) const USER_SHELL_COMMAND_OPEN_TAG: &str = "<user_shell_command>";
pub(crate) const USER_SHELL_COMMAND_CLOSE_TAG: &str = "</user_shell_command>";
pub(crate) const TURN_ABORTED_OPEN_TAG: &str = "<turn_aborted>";
pub(crate) const TURN_ABORTED_CLOSE_TAG: &str = "</turn_aborted>";
pub(crate) const SUBAGENT_NOTIFICATION_OPEN_TAG: &str = "<subagent_notification>";
pub(crate) const SUBAGENT_NOTIFICATION_CLOSE_TAG: &str = "</subagent_notification>";
use super::EnvironmentContext;
use super::FragmentRegistration;
use super::FragmentRegistrationProxy;
use super::SkillInstructions;
use super::SubagentNotification;
use super::TurnAborted;
use super::UserInstructions;
use super::UserShellCommand;
pub(crate) const ENVIRONMENT_CONTEXT_FRAGMENT: ContextualUserFragmentDefinition =
ContextualUserFragmentDefinition::new(
ENVIRONMENT_CONTEXT_OPEN_TAG,
ENVIRONMENT_CONTEXT_CLOSE_TAG,
);
pub(crate) const USER_SHELL_COMMAND_FRAGMENT: ContextualUserFragmentDefinition =
ContextualUserFragmentDefinition::new(
USER_SHELL_COMMAND_OPEN_TAG,
USER_SHELL_COMMAND_CLOSE_TAG,
);
pub(crate) const TURN_ABORTED_FRAGMENT: ContextualUserFragmentDefinition =
ContextualUserFragmentDefinition::new(TURN_ABORTED_OPEN_TAG, TURN_ABORTED_CLOSE_TAG);
pub(crate) const SUBAGENT_NOTIFICATION_FRAGMENT: ContextualUserFragmentDefinition =
ContextualUserFragmentDefinition::new(
SUBAGENT_NOTIFICATION_OPEN_TAG,
SUBAGENT_NOTIFICATION_CLOSE_TAG,
);
static USER_INSTRUCTIONS_REGISTRATION: FragmentRegistrationProxy<UserInstructions> =
FragmentRegistrationProxy::new();
static ENVIRONMENT_CONTEXT_REGISTRATION: FragmentRegistrationProxy<EnvironmentContext> =
FragmentRegistrationProxy::new();
static SKILL_INSTRUCTIONS_REGISTRATION: FragmentRegistrationProxy<SkillInstructions> =
FragmentRegistrationProxy::new();
static USER_SHELL_COMMAND_REGISTRATION: FragmentRegistrationProxy<UserShellCommand> =
FragmentRegistrationProxy::new();
static TURN_ABORTED_REGISTRATION: FragmentRegistrationProxy<TurnAborted> =
FragmentRegistrationProxy::new();
static SUBAGENT_NOTIFICATION_REGISTRATION: FragmentRegistrationProxy<SubagentNotification> =
FragmentRegistrationProxy::new();
const CONTEXTUAL_USER_FRAGMENTS: &[ContextualUserFragmentDefinition] = &[
AGENTS_MD_FRAGMENT,
ENVIRONMENT_CONTEXT_FRAGMENT,
SKILL_FRAGMENT,
USER_SHELL_COMMAND_FRAGMENT,
TURN_ABORTED_FRAGMENT,
SUBAGENT_NOTIFICATION_FRAGMENT,
static CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[
&USER_INSTRUCTIONS_REGISTRATION,
&ENVIRONMENT_CONTEXT_REGISTRATION,
&SKILL_INSTRUCTIONS_REGISTRATION,
&USER_SHELL_COMMAND_REGISTRATION,
&TURN_ABORTED_REGISTRATION,
&SUBAGENT_NOTIFICATION_REGISTRATION,
];
static MEMORY_EXCLUDED_CONTEXTUAL_USER_FRAGMENTS: &[&dyn FragmentRegistration] = &[
&USER_INSTRUCTIONS_REGISTRATION,
&SKILL_INSTRUCTIONS_REGISTRATION,
];
fn is_standard_contextual_user_text(text: &str) -> bool {
CONTEXTUAL_USER_FRAGMENTS
.iter()
.any(|definition| definition.matches_text(text))
.any(|fragment| fragment.matches_text(text))
}
/// Returns whether a contextual user fragment should be omitted from memory
@@ -59,7 +56,9 @@ pub(crate) fn is_memory_excluded_contextual_user_fragment(content_item: &Content
let ContentItem::InputText { text } = content_item else {
return false;
};
AGENTS_MD_FRAGMENT.matches_text(text) || SKILL_FRAGMENT.matches_text(text)
MEMORY_EXCLUDED_CONTEXTUAL_USER_FRAGMENTS
.iter()
.any(|fragment| fragment.matches_text(text))
}
pub(crate) fn is_contextual_user_fragment(content_item: &ContentItem) -> bool {
@@ -1,4 +1,5 @@
use super::*;
use crate::context::ContextualUserFragment;
use codex_protocol::items::HookPromptFragment;
use codex_protocol::items::build_hook_prompt_message;
use codex_protocol::models::ResponseItem;
@@ -20,10 +21,9 @@ fn detects_agents_instructions_fragment() {
#[test]
fn detects_subagent_notification_fragment_case_insensitively() {
assert!(
SUBAGENT_NOTIFICATION_FRAGMENT
.matches_text("<SUBAGENT_NOTIFICATION>{}</subagent_notification>")
);
assert!(SubagentNotification::matches_text(
"<SUBAGENT_NOTIFICATION>{}</subagent_notification>"
));
}
#[test]
@@ -1,34 +1,40 @@
use crate::contextual_user_message::ENVIRONMENT_CONTEXT_FRAGMENT;
use crate::session::turn_context::TurnContext;
use crate::shell::Shell;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::TurnContextItem;
use codex_protocol::protocol::TurnContextNetworkItem;
use serde::Deserialize;
use serde::Serialize;
use std::path::PathBuf;
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
#[serde(rename = "environment_context", rename_all = "snake_case")]
use super::ContextualUserFragment;
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct EnvironmentContext {
pub cwd: Option<PathBuf>,
pub shell: Shell,
pub current_date: Option<String>,
pub timezone: Option<String>,
pub network: Option<NetworkContext>,
pub subagents: Option<String>,
pub(crate) cwd: Option<PathBuf>,
pub(crate) shell: String,
pub(crate) current_date: Option<String>,
pub(crate) timezone: Option<String>,
pub(crate) network: Option<NetworkContext>,
pub(crate) subagents: Option<String>,
}
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)]
#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub(crate) struct NetworkContext {
allowed_domains: Vec<String>,
denied_domains: Vec<String>,
}
impl NetworkContext {
pub(crate) fn new(allowed_domains: Vec<String>, denied_domains: Vec<String>) -> Self {
Self {
allowed_domains,
denied_domains,
}
}
}
impl EnvironmentContext {
pub fn new(
pub(crate) fn new(
cwd: Option<PathBuf>,
shell: Shell,
shell: String,
current_date: Option<String>,
timezone: Option<String>,
network: Option<NetworkContext>,
@@ -47,7 +53,7 @@ impl EnvironmentContext {
/// Compares two environment contexts, ignoring the shell. Useful when
/// comparing turn to turn, since the initial environment_context will
/// include the shell, and then it is not configurable from turn to turn.
pub fn equals_except_shell(&self, other: &EnvironmentContext) -> bool {
pub(crate) fn equals_except_shell(&self, other: &EnvironmentContext) -> bool {
let EnvironmentContext {
cwd,
current_date,
@@ -63,39 +69,34 @@ impl EnvironmentContext {
&& self.subagents == *subagents
}
pub fn diff_from_turn_context_item(
pub(crate) fn diff_from_turn_context_item(
before: &TurnContextItem,
after: &TurnContext,
shell: &Shell,
after: &EnvironmentContext,
) -> Self {
let before_network = Self::network_from_turn_context_item(before);
let after_network = Self::network_from_turn_context(after);
let cwd = if before.cwd.as_path() != after.cwd.as_path() {
Some(after.cwd.to_path_buf())
} else {
None
let cwd = match &after.cwd {
Some(cwd) if before.cwd.as_path() != cwd.as_path() => Some(cwd.clone()),
_ => None,
};
let current_date = after.current_date.clone();
let timezone = after.timezone.clone();
let network = if before_network != after_network {
after_network
let network = if before_network != after.network {
after.network.clone()
} else {
before_network
};
EnvironmentContext::new(
cwd,
shell.clone(),
current_date,
timezone,
after.shell.clone(),
after.current_date.clone(),
after.timezone.clone(),
network,
/*subagents*/ None,
)
}
pub fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self {
pub(crate) fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self {
Self::new(
Some(turn_context.cwd.to_path_buf()),
shell.clone(),
shell.name().to_string(),
turn_context.current_date.clone(),
turn_context.timezone.clone(),
Self::network_from_turn_context(turn_context),
@@ -103,10 +104,13 @@ impl EnvironmentContext {
)
}
pub fn from_turn_context_item(turn_context_item: &TurnContextItem, shell: &Shell) -> Self {
pub(crate) fn from_turn_context_item(
turn_context_item: &TurnContextItem,
shell: String,
) -> Self {
Self::new(
Some(turn_context_item.cwd.clone()),
shell.clone(),
shell,
turn_context_item.current_date.clone(),
turn_context_item.timezone.clone(),
Self::network_from_turn_context_item(turn_context_item),
@@ -114,7 +118,7 @@ impl EnvironmentContext {
)
}
pub fn with_subagents(mut self, subagents: String) -> Self {
pub(crate) fn with_subagents(mut self, subagents: String) -> Self {
if !subagents.is_empty() {
self.subagents = Some(subagents);
}
@@ -129,18 +133,18 @@ impl EnvironmentContext {
.network
.as_ref()?;
Some(NetworkContext {
allowed_domains: network
Some(NetworkContext::new(
network
.domains
.as_ref()
.and_then(codex_config::NetworkDomainPermissionsToml::allowed_domains)
.unwrap_or_default(),
denied_domains: network
network
.domains
.as_ref()
.and_then(codex_config::NetworkDomainPermissionsToml::denied_domains)
.unwrap_or_default(),
})
))
}
fn network_from_turn_context_item(
@@ -150,40 +154,33 @@ impl EnvironmentContext {
allowed_domains,
denied_domains,
} = turn_context_item.network.as_ref()?;
Some(NetworkContext {
allowed_domains: allowed_domains.clone(),
denied_domains: denied_domains.clone(),
})
Some(NetworkContext::new(
allowed_domains.clone(),
denied_domains.clone(),
))
}
}
impl EnvironmentContext {
/// Serializes the environment context to XML. Libraries like `quick-xml`
/// require custom macros to handle Enums with newtypes, so we just do it
/// manually, to keep things simple. Output looks like:
///
/// ```xml
/// <environment_context>
/// <cwd>...</cwd>
/// <shell>...</shell>
/// </environment_context>
/// ```
pub fn serialize_to_xml(self) -> String {
impl ContextualUserFragment for EnvironmentContext {
const ROLE: &'static str = "user";
const START_MARKER: &'static str = codex_protocol::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG;
const END_MARKER: &'static str = codex_protocol::protocol::ENVIRONMENT_CONTEXT_CLOSE_TAG;
fn body(&self) -> String {
let mut lines = Vec::new();
if let Some(cwd) = self.cwd {
if let Some(cwd) = &self.cwd {
lines.push(format!(" <cwd>{}</cwd>", cwd.to_string_lossy()));
}
let shell_name = self.shell.name();
lines.push(format!(" <shell>{shell_name}</shell>"));
if let Some(current_date) = self.current_date {
lines.push(format!(" <shell>{}</shell>", self.shell));
if let Some(current_date) = &self.current_date {
lines.push(format!(" <current_date>{current_date}</current_date>"));
}
if let Some(timezone) = self.timezone {
if let Some(timezone) = &self.timezone {
lines.push(format!(" <timezone>{timezone}</timezone>"));
}
match self.network {
Some(ref network) => {
match &self.network {
Some(network) => {
lines.push(" <network enabled=\"true\">".to_string());
for allowed in &network.allowed_domains {
lines.push(format!(" <allowed>{allowed}</allowed>"));
@@ -198,18 +195,12 @@ impl EnvironmentContext {
// lines.push(" <network enabled=\"false\" />".to_string());
}
}
if let Some(subagents) = self.subagents {
if let Some(subagents) = &self.subagents {
lines.push(" <subagents>".to_string());
lines.extend(subagents.lines().map(|line| format!(" {line}")));
lines.push(" </subagents>".to_string());
}
ENVIRONMENT_CONTEXT_FRAGMENT.wrap(lines.join("\n"))
}
}
impl From<EnvironmentContext> for ResponseItem {
fn from(ec: EnvironmentContext) -> Self {
ENVIRONMENT_CONTEXT_FRAGMENT.into_message(ec.serialize_to_xml())
format!("\n{}", lines.join("\n"))
}
}
@@ -3,13 +3,15 @@ use crate::shell::ShellType;
use super::*;
use core_test_support::test_path_buf;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
fn fake_shell() -> Shell {
Shell {
fn fake_shell_name() -> String {
let shell = crate::shell::Shell {
shell_type: ShellType::Bash,
shell_path: PathBuf::from("/bin/bash"),
shell_snapshot: crate::shell::empty_shell_snapshot_receiver(),
}
};
shell.name().to_string()
}
#[test]
@@ -17,7 +19,7 @@ fn serialize_workspace_write_environment_context() {
let cwd = test_path_buf("/repo");
let context = EnvironmentContext::new(
Some(cwd.clone()),
fake_shell(),
fake_shell_name(),
Some("2026-02-26".to_string()),
Some("America/Los_Angeles".to_string()),
/*network*/ None,
@@ -34,18 +36,18 @@ fn serialize_workspace_write_environment_context() {
cwd = cwd.display(),
);
assert_eq!(context.serialize_to_xml(), expected);
assert_eq!(context.render(), expected);
}
#[test]
fn serialize_environment_context_with_network() {
let network = NetworkContext {
allowed_domains: vec!["api.example.com".to_string(), "*.openai.com".to_string()],
denied_domains: vec!["blocked.example.com".to_string()],
};
let network = NetworkContext::new(
vec!["api.example.com".to_string(), "*.openai.com".to_string()],
vec!["blocked.example.com".to_string()],
);
let context = EnvironmentContext::new(
Some(test_path_buf("/repo")),
fake_shell(),
fake_shell_name(),
Some("2026-02-26".to_string()),
Some("America/Los_Angeles".to_string()),
Some(network),
@@ -67,14 +69,14 @@ fn serialize_environment_context_with_network() {
test_path_buf("/repo").display()
);
assert_eq!(context.serialize_to_xml(), expected);
assert_eq!(context.render(), expected);
}
#[test]
fn serialize_read_only_environment_context() {
let context = EnvironmentContext::new(
/*cwd*/ None,
fake_shell(),
fake_shell_name(),
Some("2026-02-26".to_string()),
Some("America/Los_Angeles".to_string()),
/*network*/ None,
@@ -87,74 +89,14 @@ fn serialize_read_only_environment_context() {
<timezone>America/Los_Angeles</timezone>
</environment_context>"#;
assert_eq!(context.serialize_to_xml(), expected);
}
#[test]
fn serialize_external_sandbox_environment_context() {
let context = EnvironmentContext::new(
/*cwd*/ None,
fake_shell(),
Some("2026-02-26".to_string()),
Some("America/Los_Angeles".to_string()),
/*network*/ None,
/*subagents*/ None,
);
let expected = r#"<environment_context>
<shell>bash</shell>
<current_date>2026-02-26</current_date>
<timezone>America/Los_Angeles</timezone>
</environment_context>"#;
assert_eq!(context.serialize_to_xml(), expected);
}
#[test]
fn serialize_external_sandbox_with_restricted_network_environment_context() {
let context = EnvironmentContext::new(
/*cwd*/ None,
fake_shell(),
Some("2026-02-26".to_string()),
Some("America/Los_Angeles".to_string()),
/*network*/ None,
/*subagents*/ None,
);
let expected = r#"<environment_context>
<shell>bash</shell>
<current_date>2026-02-26</current_date>
<timezone>America/Los_Angeles</timezone>
</environment_context>"#;
assert_eq!(context.serialize_to_xml(), expected);
}
#[test]
fn serialize_full_access_environment_context() {
let context = EnvironmentContext::new(
/*cwd*/ None,
fake_shell(),
Some("2026-02-26".to_string()),
Some("America/Los_Angeles".to_string()),
/*network*/ None,
/*subagents*/ None,
);
let expected = r#"<environment_context>
<shell>bash</shell>
<current_date>2026-02-26</current_date>
<timezone>America/Los_Angeles</timezone>
</environment_context>"#;
assert_eq!(context.serialize_to_xml(), expected);
assert_eq!(context.render(), expected);
}
#[test]
fn equals_except_shell_compares_cwd() {
let context1 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
fake_shell(),
fake_shell_name(),
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
@@ -162,7 +104,7 @@ fn equals_except_shell_compares_cwd() {
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
fake_shell(),
fake_shell_name(),
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
@@ -171,33 +113,11 @@ fn equals_except_shell_compares_cwd() {
assert!(context1.equals_except_shell(&context2));
}
#[test]
fn equals_except_shell_ignores_sandbox_policy() {
let context1 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
fake_shell(),
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
/*subagents*/ None,
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
fake_shell(),
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
/*subagents*/ None,
);
assert!(context1.equals_except_shell(&context2));
}
#[test]
fn equals_except_shell_compares_cwd_differences() {
let context1 = EnvironmentContext::new(
Some(PathBuf::from("/repo1")),
fake_shell(),
fake_shell_name(),
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
@@ -205,7 +125,7 @@ fn equals_except_shell_compares_cwd_differences() {
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo2")),
fake_shell(),
fake_shell_name(),
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
@@ -219,11 +139,7 @@ fn equals_except_shell_compares_cwd_differences() {
fn equals_except_shell_ignores_shell() {
let context1 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Shell {
shell_type: ShellType::Bash,
shell_path: "/bin/bash".into(),
shell_snapshot: crate::shell::empty_shell_snapshot_receiver(),
},
"bash".to_string(),
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
@@ -231,11 +147,7 @@ fn equals_except_shell_ignores_shell() {
);
let context2 = EnvironmentContext::new(
Some(PathBuf::from("/repo")),
Shell {
shell_type: ShellType::Zsh,
shell_path: "/bin/zsh".into(),
shell_snapshot: crate::shell::empty_shell_snapshot_receiver(),
},
"zsh".to_string(),
/*current_date*/ None,
/*timezone*/ None,
/*network*/ None,
@@ -249,7 +161,7 @@ fn equals_except_shell_ignores_shell() {
fn serialize_environment_context_with_subagents() {
let context = EnvironmentContext::new(
Some(test_path_buf("/repo")),
fake_shell(),
fake_shell_name(),
Some("2026-02-26".to_string()),
Some("America/Los_Angeles".to_string()),
/*network*/ None,
@@ -270,5 +182,5 @@ fn serialize_environment_context_with_subagents() {
test_path_buf("/repo").display()
);
assert_eq!(context.serialize_to_xml(), expected);
assert_eq!(context.render(), expected);
}
+82
View File
@@ -0,0 +1,82 @@
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use std::marker::PhantomData;
/// Type-erased registration for a contextual user fragment.
///
/// Implementations are used by context filtering code to recognize injected
/// fragments without constructing the concrete context payload.
pub(crate) trait FragmentRegistration: Sync {
fn matches_text(&self, text: &str) -> bool;
}
pub(crate) struct FragmentRegistrationProxy<T> {
_marker: PhantomData<fn() -> T>,
}
impl<T> FragmentRegistrationProxy<T> {
pub(crate) const fn new() -> Self {
Self {
_marker: PhantomData,
}
}
}
impl<T: ContextualUserFragment> FragmentRegistration for FragmentRegistrationProxy<T> {
fn matches_text(&self, text: &str) -> bool {
T::matches_text(text)
}
}
/// Context payload that is injected as a user-authored message fragment.
///
/// Implementations own the response role, start/end markers used to recognize
/// the fragment, and provide the fragment body appended directly after the
/// start marker. The default helpers wrap that body and convert it into the
/// response item shape expected by model input assembly.
pub(crate) trait ContextualUserFragment {
const ROLE: &'static str;
const START_MARKER: &'static str;
const END_MARKER: &'static str;
fn body(&self) -> String;
fn matches_text(text: &str) -> bool
where
Self: Sized,
{
let trimmed = text.trim_start();
let starts_with_marker = trimmed
.get(..Self::START_MARKER.len())
.is_some_and(|candidate| candidate.eq_ignore_ascii_case(Self::START_MARKER));
let trimmed = trimmed.trim_end();
let ends_with_marker = trimmed
.get(trimmed.len().saturating_sub(Self::END_MARKER.len())..)
.is_some_and(|candidate| candidate.eq_ignore_ascii_case(Self::END_MARKER));
starts_with_marker && ends_with_marker
}
fn render(&self) -> String {
format!(
"{}{}\n{}",
Self::START_MARKER,
self.body(),
Self::END_MARKER
)
}
fn into(self) -> ResponseItem
where
Self: Sized,
{
ResponseItem::Message {
id: None,
role: Self::ROLE.to_string(),
content: vec![ContentItem::InputText {
text: self.render(),
}],
end_turn: None,
phase: None,
}
}
}
+21
View File
@@ -0,0 +1,21 @@
mod contextual_user_message;
mod environment_context;
mod fragment;
mod skill_instructions;
mod subagent_notification;
mod turn_aborted;
mod user_instructions;
mod user_shell_command;
pub(crate) use contextual_user_message::is_contextual_user_fragment;
pub(crate) use contextual_user_message::is_memory_excluded_contextual_user_fragment;
pub(crate) use contextual_user_message::parse_visible_hook_prompt_message;
pub(crate) use environment_context::EnvironmentContext;
pub(crate) use fragment::ContextualUserFragment;
pub(crate) use fragment::FragmentRegistration;
pub(crate) use fragment::FragmentRegistrationProxy;
pub(crate) use skill_instructions::SkillInstructions;
pub(crate) use subagent_notification::SubagentNotification;
pub(crate) use turn_aborted::TurnAborted;
pub(crate) use user_instructions::UserInstructions;
pub(crate) use user_shell_command::UserShellCommand;
@@ -0,0 +1,33 @@
use codex_core_skills::injection::SkillInjection;
use super::ContextualUserFragment;
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct SkillInstructions {
pub(crate) name: String,
pub(crate) path: String,
pub(crate) contents: String,
}
impl From<&SkillInjection> for SkillInstructions {
fn from(skill: &SkillInjection) -> Self {
Self {
name: skill.name.clone(),
path: skill.path.clone(),
contents: skill.contents.clone(),
}
}
}
impl ContextualUserFragment for SkillInstructions {
const ROLE: &'static str = "user";
const START_MARKER: &'static str = "<skill>";
const END_MARKER: &'static str = "</skill>";
fn body(&self) -> String {
format!(
"\n<name>{}</name>\n<path>{}</path>\n{}",
self.name, self.path, self.contents
)
}
}
@@ -0,0 +1,34 @@
use codex_protocol::protocol::AgentStatus;
use super::ContextualUserFragment;
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct SubagentNotification {
pub(crate) agent_reference: String,
pub(crate) status: AgentStatus,
}
impl SubagentNotification {
pub(crate) fn new(agent_reference: impl Into<String>, status: AgentStatus) -> Self {
Self {
agent_reference: agent_reference.into(),
status,
}
}
}
impl ContextualUserFragment for SubagentNotification {
const ROLE: &'static str = "user";
const START_MARKER: &'static str = "<subagent_notification>";
const END_MARKER: &'static str = "</subagent_notification>";
fn body(&self) -> String {
format!(
"\n{}",
serde_json::json!({
"agent_path": &self.agent_reference,
"status": &self.status,
})
)
}
}
+26
View File
@@ -0,0 +1,26 @@
use super::ContextualUserFragment;
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct TurnAborted {
pub(crate) guidance: String,
}
impl TurnAborted {
pub(crate) const INTERRUPTED_GUIDANCE: &'static str = "The user interrupted the previous turn on purpose. Any running unified exec processes may still be running in the background. If any tools/commands were aborted, they may have partially executed.";
pub(crate) fn new(guidance: impl Into<String>) -> Self {
Self {
guidance: guidance.into(),
}
}
}
impl ContextualUserFragment for TurnAborted {
const ROLE: &'static str = "user";
const START_MARKER: &'static str = "<turn_aborted>";
const END_MARKER: &'static str = "</turn_aborted>";
fn body(&self) -> String {
format!("\n{}", self.guidance)
}
}
@@ -0,0 +1,17 @@
use super::ContextualUserFragment;
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct UserInstructions {
pub(crate) directory: String,
pub(crate) text: String,
}
impl ContextualUserFragment for UserInstructions {
const ROLE: &'static str = "user";
const START_MARKER: &'static str = "# AGENTS.md instructions for ";
const END_MARKER: &'static str = "</INSTRUCTIONS>";
fn body(&self) -> String {
format!("{}\n\n<INSTRUCTIONS>\n{}", self.directory, self.text)
}
}
@@ -0,0 +1,40 @@
use std::time::Duration;
use super::ContextualUserFragment;
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct UserShellCommand {
pub(crate) command: String,
pub(crate) exit_code: i32,
pub(crate) duration_seconds: f64,
pub(crate) output: String,
}
impl UserShellCommand {
pub(crate) fn new(
command: impl Into<String>,
exit_code: i32,
duration: Duration,
output: impl Into<String>,
) -> Self {
Self {
command: command.into(),
exit_code,
duration_seconds: duration.as_secs_f64(),
output: output.into(),
}
}
}
impl ContextualUserFragment for UserShellCommand {
const ROLE: &'static str = "user";
const START_MARKER: &'static str = "<user_shell_command>";
const END_MARKER: &'static str = "</user_shell_command>";
fn body(&self) -> String {
format!(
"\n<command>\n{}\n</command>\n<result>\nExit code: {}\nDuration: {:.4} seconds\nOutput:\n{}\n</result>",
self.command, self.exit_code, self.duration_seconds, self.output,
)
}
}
+5 -4
View File
@@ -1,4 +1,5 @@
use crate::environment_context::EnvironmentContext;
use crate::context::ContextualUserFragment;
use crate::context::EnvironmentContext;
use crate::session::PreviousTurnSettings;
use crate::session::turn_context::TurnContext;
use crate::shell::Shell;
@@ -21,14 +22,14 @@ fn build_environment_update_item(
}
let prev = previous?;
let prev_context = EnvironmentContext::from_turn_context_item(prev, shell);
let prev_context = EnvironmentContext::from_turn_context_item(prev, shell.name().to_string());
let next_context = EnvironmentContext::from_turn_context(next, shell);
if prev_context.equals_except_shell(&next_context) {
return None;
}
Some(ResponseItem::from(
EnvironmentContext::diff_from_turn_context_item(prev, next, shell),
Some(ContextualUserFragment::into(
EnvironmentContext::diff_from_turn_context_item(prev, &next_context),
))
}
+2 -2
View File
@@ -20,8 +20,8 @@ use codex_protocol::user_input::UserInput;
use tracing::warn;
use uuid::Uuid;
use crate::contextual_user_message::is_contextual_user_fragment;
use crate::contextual_user_message::parse_visible_hook_prompt_message;
use crate::context::is_contextual_user_fragment;
use crate::context::parse_visible_hook_prompt_message;
use crate::web_search::web_search_action_detail;
const CONTEXTUAL_DEVELOPER_PREFIXES: &[&str] = &[
+21 -4
View File
@@ -28,6 +28,7 @@ use codex_shell_command::is_dangerous_command::command_might_be_dangerous;
use codex_shell_command::is_safe_command::is_known_safe_command;
use thiserror::Error;
use tokio::fs;
use tokio::sync::Semaphore;
use tokio::task::spawn_blocking;
use tracing::instrument;
@@ -197,7 +198,7 @@ pub enum ExecPolicyUpdateError {
pub(crate) struct ExecPolicyManager {
policy: ArcSwap<Policy>,
update_lock: tokio::sync::Mutex<()>,
update_lock: Semaphore,
}
pub(crate) struct ExecApprovalRequest<'a> {
@@ -213,7 +214,7 @@ impl ExecPolicyManager {
pub(crate) fn new(policy: Arc<Policy>) -> Self {
Self {
policy: ArcSwap::from(policy),
update_lock: tokio::sync::Mutex::new(()),
update_lock: Semaphore::new(/*permits*/ 1),
}
}
@@ -331,7 +332,15 @@ impl ExecPolicyManager {
codex_home: &Path,
amendment: &ExecPolicyAmendment,
) -> Result<(), ExecPolicyUpdateError> {
let _update_guard = self.update_lock.lock().await;
let _update_guard =
self.update_lock
.acquire()
.await
.map_err(|_| ExecPolicyUpdateError::AddRule {
source: ExecPolicyRuleError::InvalidRule(
"exec policy update semaphore closed".to_string(),
),
})?;
let policy_path = default_policy_path(codex_home);
spawn_blocking({
let policy_path = policy_path.clone();
@@ -376,7 +385,15 @@ impl ExecPolicyManager {
decision: Decision,
justification: Option<String>,
) -> Result<(), ExecPolicyUpdateError> {
let _update_guard = self.update_lock.lock().await;
let _update_guard =
self.update_lock
.acquire()
.await
.map_err(|_| ExecPolicyUpdateError::AddRule {
source: ExecPolicyRuleError::InvalidRule(
"exec policy update semaphore closed".to_string(),
),
})?;
let policy_path = default_policy_path(codex_home);
let host = host.to_string();
spawn_blocking({
+39 -2
View File
@@ -4,6 +4,7 @@ use codex_protocol::approvals::GuardianAssessmentAction;
use codex_protocol::approvals::GuardianCommandSource;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::models::PermissionProfile;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Serialize;
use serde_json::Value;
@@ -65,6 +66,12 @@ pub(crate) enum GuardianApprovalRequest {
tool_description: Option<String>,
annotations: Option<GuardianMcpAnnotations>,
},
RequestPermissions {
id: String,
turn_id: String,
reason: Option<String>,
permissions: RequestPermissionProfile,
},
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
@@ -123,6 +130,15 @@ struct McpToolCallApprovalAction<'a> {
annotations: Option<&'a GuardianMcpAnnotations>,
}
#[derive(Serialize)]
struct RequestPermissionsApprovalAction<'a> {
tool: &'static str,
turn_id: &'a str,
#[serde(skip_serializing_if = "Option::is_none")]
reason: Option<&'a String>,
permissions: &'a RequestPermissionProfile,
}
fn serialize_guardian_action(value: impl Serialize) -> serde_json::Result<Value> {
serde_json::to_value(value)
}
@@ -293,6 +309,17 @@ pub(crate) fn guardian_approval_request_to_json(
tool_description: tool_description.as_ref(),
annotations: annotations.as_ref(),
}),
GuardianApprovalRequest::RequestPermissions {
id: _,
turn_id,
reason,
permissions,
} => serialize_guardian_action(RequestPermissionsApprovalAction {
tool: "request_permissions",
turn_id,
reason: reason.as_ref(),
permissions,
}),
}
}
@@ -352,6 +379,14 @@ pub(crate) fn guardian_assessment_action(
connector_name: connector_name.clone(),
tool_title: tool_title.clone(),
},
GuardianApprovalRequest::RequestPermissions {
reason,
permissions,
..
} => GuardianAssessmentAction::RequestPermissions {
reason: reason.clone(),
permissions: permissions.clone(),
},
}
}
@@ -360,7 +395,8 @@ pub(crate) fn guardian_request_target_item_id(request: &GuardianApprovalRequest)
GuardianApprovalRequest::Shell { id, .. }
| GuardianApprovalRequest::ExecCommand { id, .. }
| GuardianApprovalRequest::ApplyPatch { id, .. }
| GuardianApprovalRequest::McpToolCall { id, .. } => Some(id),
| GuardianApprovalRequest::McpToolCall { id, .. }
| GuardianApprovalRequest::RequestPermissions { id, .. } => Some(id),
GuardianApprovalRequest::NetworkAccess { .. } => None,
#[cfg(unix)]
GuardianApprovalRequest::Execve { id, .. } => Some(id),
@@ -372,7 +408,8 @@ pub(crate) fn guardian_request_turn_id<'a>(
default_turn_id: &'a str,
) -> &'a str {
match request {
GuardianApprovalRequest::NetworkAccess { turn_id, .. } => turn_id,
GuardianApprovalRequest::NetworkAccess { turn_id, .. }
| GuardianApprovalRequest::RequestPermissions { turn_id, .. } => turn_id,
GuardianApprovalRequest::Shell { .. }
| GuardianApprovalRequest::ExecCommand { .. }
| GuardianApprovalRequest::ApplyPatch { .. }
+3 -8
View File
@@ -19,6 +19,7 @@ mod review_session;
use std::time::Duration;
use codex_protocol::protocol::GuardianAssessmentDecisionSource;
use codex_protocol::protocol::GuardianAssessmentOutcome;
use serde::Deserialize;
use serde::Serialize;
@@ -30,8 +31,10 @@ pub(crate) use review::guardian_timeout_message;
pub(crate) use review::is_guardian_reviewer_source;
pub(crate) use review::new_guardian_review_id;
pub(crate) use review::review_approval_request;
#[cfg(test)]
pub(crate) use review::review_approval_request_with_cancel;
pub(crate) use review::routes_approval_to_guardian;
pub(crate) use review::spawn_approval_request_review;
pub(crate) use review_session::GuardianReviewSessionManager;
const GUARDIAN_PREFERRED_MODEL: &str = "codex-auto-review";
@@ -45,14 +48,6 @@ const GUARDIAN_MAX_ACTION_STRING_TOKENS: usize = 16_000;
const GUARDIAN_RECENT_ENTRY_LIMIT: usize = 40;
const TRUNCATION_TAG: &str = "truncated";
/// Final allow/deny outcome returned by the guardian reviewer.
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq)]
#[serde(rename_all = "lowercase")]
pub(crate) enum GuardianAssessmentOutcome {
Allow,
Deny,
}
/// Structured output contract that the guardian reviewer must satisfy.
#[derive(Debug, Clone, Deserialize, Serialize)]
pub(crate) struct GuardianAssessment {
+38 -5
View File
@@ -11,6 +11,7 @@ use codex_protocol::protocol::GuardianUserAuthorization;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::WarningEvent;
use tokio::sync::oneshot;
use tokio_util::sync::CancellationToken;
use crate::session::session::Session;
@@ -89,12 +90,14 @@ fn guardian_risk_level_str(level: GuardianRiskLevel) -> &'static str {
}
}
/// Whether this turn should route `on-request` approval prompts through the
/// guardian reviewer instead of surfacing them to the user. ARC may still
/// block actions earlier in the flow.
/// Whether this turn should route allowed approval prompts through the guardian
/// reviewer instead of surfacing them to the user. ARC may still block actions
/// earlier in the flow.
pub(crate) fn routes_approval_to_guardian(turn: &TurnContext) -> bool {
turn.approval_policy.value() == AskForApproval::OnRequest
&& turn.config.approvals_reviewer == ApprovalsReviewer::GuardianSubagent
matches!(
turn.approval_policy.value(),
AskForApproval::OnRequest | AskForApproval::Granular(_)
) && turn.config.approvals_reviewer == ApprovalsReviewer::GuardianSubagent
}
pub(crate) fn is_guardian_reviewer_source(
@@ -335,6 +338,36 @@ pub(crate) async fn review_approval_request_with_cancel(
.await
}
pub(crate) fn spawn_approval_request_review(
session: Arc<Session>,
turn: Arc<TurnContext>,
review_id: String,
request: GuardianApprovalRequest,
retry_reason: Option<String>,
cancel_token: CancellationToken,
) -> oneshot::Receiver<ReviewDecision> {
let (tx, rx) = oneshot::channel();
std::thread::spawn(move || {
let Ok(runtime) = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
else {
let _ = tx.send(ReviewDecision::Denied);
return;
};
let decision = runtime.block_on(review_approval_request_with_cancel(
&session,
&turn,
review_id,
request,
retry_reason,
cancel_token,
));
let _ = tx.send(decision);
});
rx
}
/// Runs the guardian in a locked-down reusable review session.
///
/// The guardian itself should not mutate state or trigger further approvals, so
+22 -8
View File
@@ -19,6 +19,7 @@ use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SubAgentSource;
use serde_json::Value;
use tokio::sync::Mutex;
use tokio::sync::Semaphore;
use tokio_util::sync::CancellationToken;
use tracing::warn;
@@ -91,7 +92,7 @@ struct GuardianReviewSession {
codex: Codex,
cancel_token: CancellationToken,
reuse_key: GuardianReviewSessionReuseKey,
review_lock: Mutex<()>,
review_lock: Semaphore,
state: Mutex<GuardianReviewState>,
}
@@ -264,6 +265,10 @@ impl GuardianReviewSessionManager {
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "review session selection and trunk spawning must stay serialized"
)]
pub(crate) async fn run_review(
&self,
params: GuardianReviewSessionParams,
@@ -281,7 +286,7 @@ impl GuardianReviewSessionManager {
Ok(mut state) => {
if let Some(trunk) = state.trunk.as_ref()
&& trunk.reuse_key != next_reuse_key
&& trunk.review_lock.try_lock().is_ok()
&& trunk.review_lock.try_acquire().is_ok()
{
stale_trunk_to_shutdown = state.trunk.take();
}
@@ -336,7 +341,7 @@ impl GuardianReviewSessionManager {
.await;
}
let trunk_guard = match trunk.review_lock.try_lock() {
let trunk_guard = match trunk.review_lock.try_acquire() {
Ok(trunk_guard) => trunk_guard,
Err(_) => {
return Box::pin(self.run_ephemeral_review(
@@ -375,7 +380,7 @@ impl GuardianReviewSessionManager {
reuse_key,
codex,
cancel_token: CancellationToken::new(),
review_lock: Mutex::new(()),
review_lock: Semaphore::new(/*permits*/ 1),
state: Mutex::new(GuardianReviewState {
prior_review_count: 0,
last_reviewed_transcript_cursor: None,
@@ -397,7 +402,7 @@ impl GuardianReviewSessionManager {
reuse_key,
codex,
cancel_token: CancellationToken::new(),
review_lock: Mutex::new(()),
review_lock: Semaphore::new(/*permits*/ 1),
state: Mutex::new(GuardianReviewState {
prior_review_count: 0,
last_reviewed_transcript_cursor: None,
@@ -531,7 +536,7 @@ async fn spawn_guardian_review_session(
codex,
cancel_token,
reuse_key,
review_lock: Mutex::new(()),
review_lock: Semaphore::new(/*permits*/ 1),
state: Mutex::new(GuardianReviewState {
prior_review_count,
last_reviewed_transcript_cursor: initial_transcript_cursor,
@@ -731,6 +736,13 @@ pub(crate) fn build_guardian_review_session_config(
guardian_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never);
guardian_config.permissions.sandbox_policy =
Constrained::allow_only(SandboxPolicy::new_read_only_policy());
guardian_config.include_apps_instructions = false;
guardian_config
.mcp_servers
.set(HashMap::new())
.map_err(|err| {
anyhow::anyhow!("guardian review session could not clear MCP servers: {err}")
})?;
if let Some(live_network_config) = live_network_config
&& guardian_config.permissions.network.is_some()
{
@@ -750,6 +762,8 @@ pub(crate) fn build_guardian_review_session_config(
Feature::SpawnCsv,
Feature::Collab,
Feature::CodexHooks,
Feature::Apps,
Feature::Plugins,
Feature::WebSearchRequest,
Feature::WebSearchCached,
] {
@@ -760,8 +774,8 @@ pub(crate) fn build_guardian_review_session_config(
)
})?;
if guardian_config.features.enabled(feature) {
anyhow::bail!(
"guardian review session requires `features.{}` to be disabled",
warn!(
"guardian review session could not disable `features.{}`; continuing with the feature enabled",
feature.key()
);
}
+63 -8
View File
@@ -16,7 +16,9 @@ use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::test_support;
use codex_config::config_toml::ConfigToml;
use codex_config::types::McpServerConfig;
use codex_exec_server::LOCAL_FS;
use codex_features::Feature;
use codex_model_provider::create_model_provider;
use codex_network_proxy::NetworkProxyConfig;
use codex_protocol::ThreadId;
@@ -26,6 +28,7 @@ use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::GranularApprovalConfig;
use codex_protocol::protocol::GuardianAssessmentStatus;
use codex_protocol::protocol::GuardianRiskLevel;
use codex_protocol::protocol::GuardianUserAuthorization;
@@ -52,6 +55,7 @@ use insta::Settings;
use insta::assert_snapshot;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;
use tempfile::TempDir;
@@ -738,7 +742,7 @@ fn guardian_timeout_message_distinguishes_timeout_from_policy_denial() {
}
#[tokio::test]
async fn routes_approval_to_guardian_requires_auto_only_review_policy() {
async fn routes_approval_to_guardian_requires_guardian_reviewer() {
let (_session, mut turn) = crate::session::tests::make_session_and_context().await;
let mut config = (*turn.config).clone();
config.approvals_reviewer = ApprovalsReviewer::User;
@@ -752,6 +756,25 @@ async fn routes_approval_to_guardian_requires_auto_only_review_policy() {
assert!(routes_approval_to_guardian(&turn));
}
#[tokio::test]
async fn routes_approval_to_guardian_allows_granular_review_policy() {
let (_session, mut turn) = crate::session::tests::make_session_and_context().await;
let mut config = (*turn.config).clone();
config.approvals_reviewer = ApprovalsReviewer::GuardianSubagent;
turn.config = Arc::new(config);
turn.approval_policy
.set(AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: true,
rules: true,
skill_approval: true,
request_permissions: true,
mcp_elicitations: true,
}))
.expect("test setup should allow updating approval policy");
assert!(routes_approval_to_guardian(&turn));
}
#[test]
fn build_guardian_transcript_reserves_separate_budget_for_tool_evidence() {
let repeated = "signal ".repeat(8_000);
@@ -1685,7 +1708,40 @@ async fn guardian_review_session_config_uses_live_network_proxy_state() {
}
#[tokio::test]
async fn guardian_review_session_config_rejects_pinned_collab_feature() {
async fn guardian_review_session_config_disables_mcp_apps_and_plugins() {
let mut parent_config = test_config().await;
let server: McpServerConfig =
toml::from_str("command = \"docs-server\"").expect("deserialize MCP server");
parent_config
.mcp_servers
.set(HashMap::from([("docs".to_string(), server)]))
.expect("parent MCP servers are configurable");
parent_config
.features
.enable(Feature::Apps)
.expect("apps feature is configurable");
parent_config
.features
.enable(Feature::Plugins)
.expect("plugins feature is configurable");
parent_config.include_apps_instructions = true;
let guardian_config = build_guardian_review_session_config_for_test(
&parent_config,
/*live_network_config*/ None,
"active-model",
/*reasoning_effort*/ None,
)
.expect("guardian config");
assert!(guardian_config.mcp_servers.get().is_empty());
assert!(!guardian_config.features.enabled(Feature::Apps));
assert!(!guardian_config.features.enabled(Feature::Plugins));
assert!(!guardian_config.include_apps_instructions);
}
#[tokio::test]
async fn guardian_review_session_config_allows_pinned_disabled_feature() {
let mut parent_config = test_config().await;
parent_config.features = ManagedFeatures::from_configured(
parent_config.features.get().clone(),
@@ -1698,18 +1754,17 @@ async fn guardian_review_session_config_rejects_pinned_collab_feature() {
)
.expect("managed features");
let err = build_guardian_review_session_config_for_test(
let guardian_config = build_guardian_review_session_config_for_test(
&parent_config,
/*live_network_config*/ None,
"active-model",
/*reasoning_effort*/ None,
)
.expect_err("guardian config should fail when collab is pinned on");
.expect("guardian config should continue when a disabled feature is pinned on");
assert!(
err.to_string()
.contains("guardian review session requires `features.multi_agent` to be disabled")
);
assert!(guardian_config.features.enabled(Feature::Collab));
assert!(guardian_config.mcp_servers.get().is_empty());
assert!(!guardian_config.include_apps_instructions);
}
#[tokio::test]
-1
View File
@@ -1 +0,0 @@
pub(crate) use codex_instructions::UserInstructions;
+1 -3
View File
@@ -27,9 +27,8 @@ mod commit_attribution;
pub mod config;
pub mod config_loader;
pub mod connectors;
mod context;
mod context_manager;
mod contextual_user_message;
mod environment_context;
pub mod exec;
pub mod exec_env;
mod exec_policy;
@@ -41,7 +40,6 @@ mod git_info_tests;
mod guardian;
mod hook_runtime;
mod installation_id;
pub(crate) mod instructions;
pub(crate) mod landlock;
pub use landlock::spawn_command_under_linux_sandbox;
pub(crate) mod mcp;
+18 -2
View File
@@ -7,7 +7,7 @@ use codex_config::McpServerConfig;
use codex_login::CodexAuth;
use codex_mcp::ToolPluginProvenance;
use codex_mcp::configured_mcp_servers;
use codex_mcp::effective_mcp_servers;
use codex_mcp::effective_mcp_servers_with_authorization_header;
use codex_mcp::tool_plugin_provenance as collect_tool_plugin_provenance;
#[derive(Clone)]
@@ -29,9 +29,25 @@ impl McpManager {
&self,
config: &Config,
auth: Option<&CodexAuth>,
) -> HashMap<String, McpServerConfig> {
self.effective_servers_with_authorization_header(
config, auth, /*authorization_header_value*/ None,
)
.await
}
pub async fn effective_servers_with_authorization_header(
&self,
config: &Config,
auth: Option<&CodexAuth>,
authorization_header_value: Option<&str>,
) -> HashMap<String, McpServerConfig> {
let mcp_config = config.to_mcp_config(self.plugins_manager.as_ref()).await;
effective_mcp_servers(&mcp_config, auth)
effective_mcp_servers_with_authorization_header(
&mcp_config,
auth,
authorization_header_value,
)
}
pub async fn tool_plugin_provenance(&self, config: &Config) -> ToolPluginProvenance {
+12
View File
@@ -494,6 +494,10 @@ async fn execute_mcp_tool_call(
)
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP sandbox metadata reads through the session-owned manager guard"
)]
async fn augment_mcp_tool_request_meta_with_sandbox_state(
sess: &Session,
turn_context: &TurnContext,
@@ -1067,6 +1071,10 @@ fn mcp_tool_approval_callsite_mode(
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP approval metadata reads through the session-owned manager guard"
)]
pub(crate) async fn lookup_mcp_tool_metadata(
sess: &Session,
turn_context: &TurnContext,
@@ -1149,6 +1157,10 @@ fn get_mcp_app_resource_uri(
})
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP app metadata reads through the session-owned manager guard"
)]
async fn lookup_mcp_app_usage_metadata(
sess: &Session,
server: &str,
+1 -1
View File
@@ -94,7 +94,7 @@ fn numbered_mcp_tools(count: usize) -> HashMap<String, ToolInfo> {
async fn tools_config_for_mcp_tool_exposure(search_tool: bool) -> ToolsConfig {
let config = test_config().await;
let model_info = ModelsManager::construct_model_info_offline_for_tests(
"gpt-5-codex",
"gpt-5.4",
&config.to_models_manager_config(),
);
let features = Features::with_defaults();
+6 -10
View File
@@ -42,16 +42,12 @@ pub fn parse_memory_citation(citations: Vec<String>) -> Option<MemoryCitation> {
}
}
pub fn get_thread_id_from_citations(citations: Vec<String>) -> Vec<ThreadId> {
let mut result = Vec::new();
if let Some(memory_citation) = parse_memory_citation(citations) {
for rollout_id in memory_citation.rollout_ids {
if let Ok(thread_id) = ThreadId::try_from(rollout_id.as_str()) {
result.push(thread_id);
}
}
}
result
pub fn thread_ids_from_memory_citation(memory_citation: &MemoryCitation) -> Vec<ThreadId> {
memory_citation
.rollout_ids
.iter()
.filter_map(|id| ThreadId::try_from(id.as_str()).ok())
.collect()
}
fn parse_memory_citation_entry(line: &str) -> Option<MemoryCitationEntry> {
+12 -5
View File
@@ -1,10 +1,10 @@
use super::get_thread_id_from_citations;
use super::parse_memory_citation;
use super::thread_ids_from_memory_citation;
use codex_protocol::ThreadId;
use pretty_assertions::assert_eq;
#[test]
fn get_thread_id_from_citations_extracts_thread_ids() {
fn parse_memory_citation_supports_legacy_thread_ids() {
let first = ThreadId::new();
let second = ThreadId::new();
@@ -12,18 +12,25 @@ fn get_thread_id_from_citations_extracts_thread_ids() {
"<memory_citation>\n<citation_entries>\nMEMORY.md:1-2|note=[x]\n</citation_entries>\n<thread_ids>\n{first}\nnot-a-uuid\n{second}\n</thread_ids>\n</memory_citation>"
)];
assert_eq!(get_thread_id_from_citations(citations), vec![first, second]);
let parsed = parse_memory_citation(citations).expect("memory citation should parse");
assert_eq!(
thread_ids_from_memory_citation(&parsed),
vec![first, second]
);
}
#[test]
fn get_thread_id_from_citations_supports_legacy_rollout_ids() {
fn parse_memory_citation_supports_rollout_ids() {
let thread_id = ThreadId::new();
let citations = vec![format!(
"<memory_citation>\n<rollout_ids>\n{thread_id}\n</rollout_ids>\n</memory_citation>"
)];
assert_eq!(get_thread_id_from_citations(citations), vec![thread_id]);
let parsed = parse_memory_citation(citations).expect("memory citation should parse");
assert_eq!(thread_ids_from_memory_citation(&parsed), vec![thread_id]);
}
#[test]
+1 -1
View File
@@ -1,7 +1,7 @@
use crate::Prompt;
use crate::RolloutRecorder;
use crate::config::Config;
use crate::contextual_user_message::is_memory_excluded_contextual_user_fragment;
use crate::context::is_memory_excluded_contextual_user_fragment;
use crate::memories::metrics;
use crate::memories::phase_one;
use crate::memories::phase_one::PRUNE_BATCH_SIZE;
+2 -2
View File
@@ -8,7 +8,7 @@ use tokio::fs as tokio_fs;
#[test]
fn build_stage_one_input_message_truncates_rollout_using_model_context_window() {
let input = format!("{}{}{}", "a".repeat(700_000), "middle", "z".repeat(700_000));
let mut model_info = model_info_from_slug("gpt-5.2-codex");
let mut model_info = model_info_from_slug("gpt-5.3-codex");
model_info.context_window = Some(123_000);
let expected_rollout_token_limit = usize::try_from(
((123_000_i64 * model_info.effective_context_window_percent) / 100)
@@ -37,7 +37,7 @@ fn build_stage_one_input_message_truncates_rollout_using_model_context_window()
#[test]
fn build_stage_one_input_message_uses_default_limit_when_model_context_window_missing() {
let input = format!("{}{}{}", "a".repeat(700_000), "middle", "z".repeat(700_000));
let mut model_info = model_info_from_slug("gpt-5.2-codex");
let mut model_info = model_info_from_slug("gpt-5.3-codex");
model_info.context_window = None;
model_info.max_context_window = None;
let expected_truncated = truncate_text(
@@ -53,6 +53,8 @@ async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec<LayerMtime
&cli_overrides,
overrides,
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await
.context("failed to load Codex config")?;
+6 -4
View File
@@ -70,7 +70,7 @@ use std::sync::RwLock;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::Ordering;
use std::time::Instant;
use tokio::sync::Mutex;
use tokio::sync::Semaphore;
use toml_edit::value;
use tracing::info;
use tracing::warn;
@@ -349,7 +349,7 @@ pub struct PluginsManager {
configured_marketplace_upgrade_state: RwLock<ConfiguredMarketplaceUpgradeState>,
non_curated_cache_refresh_state: RwLock<NonCuratedCacheRefreshState>,
cached_enabled_outcome: RwLock<Option<PluginLoadOutcome>>,
remote_sync_lock: Mutex<()>,
remote_sync_lock: Semaphore,
restriction_product: Option<Product>,
analytics_events_client: RwLock<Option<AnalyticsEventsClient>>,
}
@@ -379,7 +379,7 @@ impl PluginsManager {
),
non_curated_cache_refresh_state: RwLock::new(NonCuratedCacheRefreshState::default()),
cached_enabled_outcome: RwLock::new(None),
remote_sync_lock: Mutex::new(()),
remote_sync_lock: Semaphore::new(/*permits*/ 1),
restriction_product,
analytics_events_client: RwLock::new(None),
}
@@ -706,7 +706,9 @@ impl PluginsManager {
auth: Option<&CodexAuth>,
additive_only: bool,
) -> Result<RemotePluginSyncResult, PluginRemoteSyncError> {
let _remote_sync_guard = self.remote_sync_lock.lock().await;
let _remote_sync_guard = self.remote_sync_lock.acquire().await.map_err(|_| {
PluginRemoteSyncError::Config(anyhow::anyhow!("remote plugin sync semaphore closed"))
})?;
if !config.features.enabled(Feature::Plugins) {
return Ok(RemotePluginSyncResult::default());
+1 -1
View File
@@ -35,7 +35,7 @@ fn stored_thread(cwd: &str, title: &str, first_user_message: &str) -> StoredThre
preview: first_user_message.to_string(),
name: (!title.is_empty()).then(|| title.to_string()),
model_provider: "test-provider".to_string(),
model: Some("gpt-5".to_string()),
model: Some("gpt-5.2".to_string()),
reasoning_effort: None,
created_at: Utc
.timestamp_opt(1_709_251_100, 0)
+52 -16
View File
@@ -831,16 +831,14 @@ async fn handle_start_inner(
}
let maybe_routed_text = match &event {
RealtimeEvent::HandoffRequested(handoff) => {
realtime_text_from_handoff_request(handoff)
realtime_delegation_from_handoff(handoff)
}
_ => None,
};
if let Some(text) = maybe_routed_text {
debug!(text = %text, "[realtime-text] realtime conversation text output");
let sess_for_routed_text = Arc::clone(&sess_clone);
sess_for_routed_text
.route_realtime_text_input(wrap_realtime_delegation_input(&text))
.await;
sess_for_routed_text.route_realtime_text_input(text).await;
}
if !fanout_realtime_active.load(Ordering::Relaxed) {
break;
@@ -890,23 +888,40 @@ pub(crate) async fn handle_audio(
}
}
fn realtime_text_from_handoff_request(handoff: &RealtimeHandoffRequested) -> Option<String> {
fn realtime_transcript_delta_from_handoff(handoff: &RealtimeHandoffRequested) -> Option<String> {
let active_transcript = handoff
.active_transcript
.iter()
.map(|entry| format!("{role}: {text}", role = entry.role, text = entry.text))
.collect::<Vec<_>>()
.join("\n");
(!active_transcript.is_empty())
.then_some(active_transcript)
.or((!handoff.input_transcript.is_empty()).then_some(handoff.input_transcript.clone()))
(!active_transcript.is_empty()).then_some(active_transcript)
}
fn wrap_realtime_delegation_input(input: &str) -> String {
format!(
"<realtime_delegation>\n <input>{}</input>\n</realtime_delegation>",
escape_xml_text(input)
)
fn realtime_text_from_handoff_request(handoff: &RealtimeHandoffRequested) -> Option<String> {
(!handoff.input_transcript.is_empty())
.then_some(handoff.input_transcript.clone())
.or_else(|| realtime_transcript_delta_from_handoff(handoff))
}
fn realtime_delegation_from_handoff(handoff: &RealtimeHandoffRequested) -> Option<String> {
let input = realtime_text_from_handoff_request(handoff)?;
Some(wrap_realtime_delegation_input(
&input,
realtime_transcript_delta_from_handoff(handoff).as_deref(),
))
}
fn wrap_realtime_delegation_input(input: &str, transcript_delta: Option<&str>) -> String {
let input = escape_xml_text(input);
if let Some(transcript_delta) = transcript_delta.filter(|text| !text.is_empty()) {
let transcript_delta = escape_xml_text(transcript_delta);
return format!(
"<realtime_delegation>\n <input>{input}</input>\n <transcript_delta>{transcript_delta}</transcript_delta>\n</realtime_delegation>"
);
}
format!("<realtime_delegation>\n <input>{input}</input>\n</realtime_delegation>")
}
fn escape_xml_text(input: &str) -> String {
@@ -1090,7 +1105,7 @@ async fn handle_handoff_output(
output_text,
} => {
writer
.send_conversation_handoff_append(handoff_id, output_text)
.send_conversation_function_call_output(handoff_id, output_text)
.await
}
},
@@ -1114,7 +1129,7 @@ async fn handle_handoff_output(
output_text: _,
} => {
if let Err(err) = writer
.send_conversation_handoff_append(
.send_conversation_function_call_output(
handoff_id,
REALTIME_V2_HANDOFF_COMPLETE_ACKNOWLEDGEMENT.to_string(),
)
@@ -1248,7 +1263,7 @@ async fn handle_realtime_server_event(
match active_handoff {
Some(_) => {
if let Err(err) = writer
.send_conversation_handoff_append(
.send_conversation_function_call_output(
handoff.handoff_id.clone(),
REALTIME_V2_STEER_ACKNOWLEDGEMENT.to_string(),
)
@@ -1277,6 +1292,27 @@ async fn handle_realtime_server_event(
}
false
}
RealtimeEvent::NoopRequested(noop) => {
*output_audio_state = None;
match session_kind {
RealtimeSessionKind::V1 => {}
RealtimeSessionKind::V2 => {
if let Err(err) = writer
.send_conversation_function_call_output(noop.call_id.clone(), String::new())
.await
{
let mapped_error = map_api_error(err);
warn!("failed to send realtime noop function output: {mapped_error}");
let _ = events_tx
.send(RealtimeEvent::Error(mapped_error.to_string()))
.await;
return Err(mapped_error.into());
}
}
}
false
}
RealtimeEvent::Error(_) => true,
RealtimeEvent::SessionUpdated { session_id, .. } => {
info!(realtime_session_id = %session_id, "realtime session updated");
@@ -1,5 +1,6 @@
use super::RealtimeHandoffState;
use super::RealtimeSessionKind;
use super::realtime_delegation_from_handoff;
use super::realtime_text_from_handoff_request;
use super::wrap_realtime_delegation_input;
use async_channel::bounded;
@@ -8,7 +9,7 @@ use codex_protocol::protocol::RealtimeTranscriptEntry;
use pretty_assertions::assert_eq;
#[test]
fn extracts_text_from_handoff_request_active_transcript() {
fn prefers_handoff_input_transcript_over_active_transcript() {
let handoff = RealtimeHandoffRequested {
handoff_id: "handoff_1".to_string(),
item_id: "item_1".to_string(),
@@ -26,7 +27,50 @@ fn extracts_text_from_handoff_request_active_transcript() {
};
assert_eq!(
realtime_text_from_handoff_request(&handoff),
Some("user: hello\nassistant: hi there".to_string())
Some("ignored".to_string())
);
}
#[test]
fn extracts_text_from_handoff_request_active_transcript_if_input_missing() {
let handoff = RealtimeHandoffRequested {
handoff_id: "handoff_1".to_string(),
item_id: "item_1".to_string(),
input_transcript: String::new(),
active_transcript: vec![RealtimeTranscriptEntry {
role: "user".to_string(),
text: "hello".to_string(),
}],
};
assert_eq!(
realtime_text_from_handoff_request(&handoff),
Some("user: hello".to_string())
);
}
#[test]
fn wraps_handoff_with_transcript_delta() {
let handoff = RealtimeHandoffRequested {
handoff_id: "handoff_1".to_string(),
item_id: "item_1".to_string(),
input_transcript: "delegate this".to_string(),
active_transcript: vec![
RealtimeTranscriptEntry {
role: "user".to_string(),
text: "hello".to_string(),
},
RealtimeTranscriptEntry {
role: "assistant".to_string(),
text: "hi there".to_string(),
},
],
};
assert_eq!(
realtime_delegation_from_handoff(&handoff),
Some(
"<realtime_delegation>\n <input>delegate this</input>\n <transcript_delta>user: hello\nassistant: hi there</transcript_delta>\n</realtime_delegation>"
.to_string()
)
);
}
@@ -58,7 +102,7 @@ fn ignores_empty_handoff_request_input_transcript() {
#[test]
fn wraps_realtime_delegation_input() {
assert_eq!(
wrap_realtime_delegation_input("hello"),
wrap_realtime_delegation_input("hello", /*transcript_delta*/ None),
"<realtime_delegation>\n <input>hello</input>\n</realtime_delegation>"
);
}
@@ -66,7 +110,15 @@ fn wraps_realtime_delegation_input() {
#[test]
fn wraps_realtime_delegation_input_with_xml_escaping() {
assert_eq!(
wrap_realtime_delegation_input("use a < b && c > d"),
wrap_realtime_delegation_input("use a < b && c > d", Some("saw <that>")),
"<realtime_delegation>\n <input>use a &lt; b &amp;&amp; c &gt; d</input>\n <transcript_delta>saw &lt;that&gt;</transcript_delta>\n</realtime_delegation>"
);
}
#[test]
fn wraps_realtime_delegation_input_with_xml_escaping_without_transcript() {
assert_eq!(
wrap_realtime_delegation_input("use a < b && c > d", /*transcript_delta*/ None),
"<realtime_delegation>\n <input>use a &lt; b &amp;&amp; c &gt; d</input>\n</realtime_delegation>"
);
}
@@ -173,7 +173,11 @@ impl Session {
return Ok(Some(agent_task));
}
let _guard = self.agent_task_registration_lock.lock().await;
let _guard = self
.agent_task_registration_lock
.acquire()
.await
.map_err(|_| anyhow::anyhow!("agent task registration semaphore closed"))?;
if let Some(agent_task) = self.cached_agent_task_for_current_identity().await {
return Ok(Some(agent_task));
}
+19 -1
View File
@@ -468,13 +468,29 @@ pub async fn reload_user_config(sess: &Arc<Session>) {
sess.reload_user_config_layer().await;
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP tool listing reads through the session-owned manager guard"
)]
pub async fn list_mcp_tools(sess: &Session, config: &Arc<Config>, sub_id: String) {
let mcp_connection_manager = sess.services.mcp_connection_manager.read().await;
let auth = sess.services.auth_manager.auth().await;
let background_authorization_header_value = if let Some(auth) = auth.as_ref() {
sess.services
.auth_manager
.chatgpt_authorization_header_for_auth(auth)
.await
} else {
None
};
let mcp_servers = sess
.services
.mcp_manager
.effective_servers(config, auth.as_ref())
.effective_servers_with_authorization_header(
config,
auth.as_ref(),
background_authorization_header_value.as_deref(),
)
.await;
let snapshot = collect_mcp_snapshot_from_manager(
&mcp_connection_manager,
@@ -533,6 +549,8 @@ pub async fn list_skills(sess: &Session, sub_id: String, cwds: Vec<PathBuf>, for
empty_cli_overrides,
LoaderOverrides::default(),
CloudRequirementsLoader::default(),
&codex_config::NoopThreadConfigLoader,
/*host_name*/ None,
)
.await
{
+42 -1
View File
@@ -1,6 +1,10 @@
use super::*;
impl Session {
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn request_mcp_server_elicitation(
&self,
turn_context: &TurnContext,
@@ -80,6 +84,10 @@ impl Session {
rx_response.await.ok()
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and manager fallback must stay serialized"
)]
pub async fn resolve_elicitation(
&self,
server_name: String,
@@ -111,6 +119,10 @@ impl Session {
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP resource calls are serialized through the session-owned manager guard"
)]
pub async fn list_resources(
&self,
server: &str,
@@ -124,6 +136,10 @@ impl Session {
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP resource calls are serialized through the session-owned manager guard"
)]
pub async fn list_resource_templates(
&self,
server: &str,
@@ -137,6 +153,10 @@ impl Session {
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP resource calls are serialized through the session-owned manager guard"
)]
pub async fn read_resource(
&self,
server: &str,
@@ -150,6 +170,10 @@ impl Session {
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP tool calls are serialized through the session-owned manager guard"
)]
pub async fn call_tool(
&self,
server: &str,
@@ -165,6 +189,10 @@ impl Session {
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP tool metadata reads through the session-owned manager guard"
)]
pub(crate) async fn resolve_mcp_tool_info(&self, tool_name: &ToolName) -> Option<ToolInfo> {
self.services
.mcp_connection_manager
@@ -190,7 +218,20 @@ impl Session {
.mcp_manager
.tool_plugin_provenance(config.as_ref())
.await;
let mcp_servers = with_codex_apps_mcp(mcp_servers, auth.as_ref(), &mcp_config);
let background_authorization_header_value = if let Some(auth) = auth.as_ref() {
self.services
.auth_manager
.chatgpt_authorization_header_for_auth(auth)
.await
} else {
None
};
let mcp_servers = with_codex_apps_mcp_with_authorization_header(
mcp_servers,
auth.as_ref(),
&mcp_config,
background_authorization_header_value.as_deref(),
);
let auth_statuses = compute_auth_statuses(mcp_servers.iter(), store_mode).await;
{
let mut guard = self.services.mcp_startup_cancellation_token.lock().await;
+199 -34
View File
@@ -19,6 +19,7 @@ use crate::commit_attribution::commit_message_trailer_instruction;
use crate::compact;
use crate::config::ManagedFeatures;
use crate::connectors;
use crate::context::ContextualUserFragment;
use crate::default_skill_metadata_budget;
use crate::exec_policy::ExecPolicyManager;
use crate::installation_id::resolve_installation_id;
@@ -153,7 +154,6 @@ use crate::config::StartedNetworkProxy;
use crate::config::resolve_web_search_mode_for_turn;
use crate::context_manager::ContextManager;
use crate::context_manager::TotalTokenUsageBreakdown;
use crate::environment_context::EnvironmentContext;
use crate::thread_rollout_truncation::initial_history_has_prior_user_turns;
use codex_config::CONFIG_TOML_FILE;
use codex_config::types::McpServerConfig;
@@ -249,9 +249,9 @@ use crate::SkillLoadOutcome;
use crate::SkillMetadata;
use crate::SkillsManager;
use crate::agents_md::AgentsMdManager;
use crate::context::UserInstructions;
use crate::exec_policy::ExecPolicyUpdateError;
use crate::guardian::GuardianReviewSessionManager;
use crate::instructions::UserInstructions;
use crate::mcp::McpManager;
use crate::memories;
use crate::network_policy_decision::execpolicy_network_rule_amendment;
@@ -293,7 +293,7 @@ use crate::unified_exec::UnifiedExecProcessManager;
use crate::windows_sandbox::WindowsSandboxLevelExt;
use codex_git_utils::get_git_repo_root;
use codex_mcp::compute_auth_statuses;
use codex_mcp::with_codex_apps_mcp;
use codex_mcp::with_codex_apps_mcp_with_authorization_header;
use codex_otel::SessionTelemetry;
use codex_otel::THREAD_STARTED_METRIC;
use codex_otel::TelemetryAuthMode;
@@ -897,7 +897,10 @@ impl Session {
let Some(started_proxy) = self.services.network_proxy.as_ref() else {
return;
};
let _refresh_guard = self.managed_network_proxy_refresh_lock.lock().await;
let Ok(_refresh_guard) = self.managed_network_proxy_refresh_lock.acquire().await else {
error!("managed network proxy refresh semaphore closed");
return;
};
let session_configuration = {
let state = self.state.lock().await;
state.session_configuration.clone()
@@ -1675,7 +1678,11 @@ impl Session {
amendment: &NetworkPolicyAmendment,
network_approval_context: &NetworkApprovalContext,
) -> anyhow::Result<()> {
let _refresh_guard = self.managed_network_proxy_refresh_lock.lock().await;
let _refresh_guard = self
.managed_network_proxy_refresh_lock
.acquire()
.await
.map_err(|_| anyhow::anyhow!("managed network proxy refresh semaphore closed"))?;
let host =
Self::validated_network_policy_amendment_host(amendment, network_approval_context)?;
let codex_home = self
@@ -1779,6 +1786,10 @@ impl Session {
/// be used to derive the available decisions via
/// [ExecApprovalRequestEvent::default_available_decisions].
#[allow(clippy::too_many_arguments)]
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn request_command_approval(
&self,
turn_context: &TurnContext,
@@ -1850,6 +1861,10 @@ impl Session {
rx_approve.await.unwrap_or(ReviewDecision::Abort)
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn request_patch_approval(
&self,
turn_context: &TurnContext,
@@ -1886,13 +1901,18 @@ impl Session {
rx_approve
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn request_permissions(
&self,
turn_context: &TurnContext,
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
match turn_context.approval_policy.value() {
match turn_context.as_ref().approval_policy.value() {
AskForApproval::Never => {
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
@@ -1913,6 +1933,72 @@ impl Session {
| AskForApproval::Granular(_) => {}
}
if crate::guardian::routes_approval_to_guardian(turn_context.as_ref()) {
let requested_permissions = args.permissions;
let originating_turn_state = {
let active = self.active_turn.lock().await;
active.as_ref().map(|active| Arc::clone(&active.turn_state))
};
let review_id = crate::guardian::new_guardian_review_id();
let session = Arc::clone(self);
let turn = Arc::clone(turn_context);
let request = crate::guardian::GuardianApprovalRequest::RequestPermissions {
id: call_id,
turn_id: turn_context.sub_id.clone(),
reason: args.reason,
permissions: requested_permissions.clone(),
};
let review_rx = crate::guardian::spawn_approval_request_review(
session,
turn,
review_id,
request,
/*retry_reason*/ None,
cancellation_token.clone(),
);
let decision = tokio::select! {
biased;
_ = cancellation_token.cancelled() => return None,
decision = review_rx => decision.unwrap_or(ReviewDecision::Denied),
};
let response = match decision {
ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => {
RequestPermissionsResponse {
permissions: requested_permissions,
scope: PermissionGrantScope::Turn,
}
}
ReviewDecision::ApprovedForSession => RequestPermissionsResponse {
permissions: requested_permissions,
scope: PermissionGrantScope::Session,
},
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
} => match network_policy_amendment.action {
NetworkPolicyRuleAction::Allow => RequestPermissionsResponse {
permissions: requested_permissions,
scope: PermissionGrantScope::Turn,
},
NetworkPolicyRuleAction::Deny => RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
},
},
ReviewDecision::Abort | ReviewDecision::Denied | ReviewDecision::TimedOut => {
RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
}
}
};
self.record_granted_request_permissions_for_turn(
&response,
originating_turn_state.as_ref(),
)
.await;
return Some(response);
}
let (tx_response, rx_response) = oneshot::channel();
let prev_entry = {
let mut active = self.active_turn.lock().await;
@@ -1928,19 +2014,31 @@ impl Session {
warn!("Overwriting existing pending request_permissions for call_id: {call_id}");
}
// TODO(ccunningham): Support auto-review for request_permissions /
// with_additional_permissions. V0 still routes this surface through
// the existing manual RequestPermissions event flow.
let event = EventMsg::RequestPermissions(RequestPermissionsEvent {
call_id,
call_id: call_id.clone(),
turn_id: turn_context.sub_id.clone(),
reason: args.reason,
permissions: args.permissions,
});
self.send_event(turn_context, event).await;
rx_response.await.ok()
self.send_event(turn_context.as_ref(), event).await;
tokio::select! {
biased;
_ = cancellation_token.cancelled() => {
let mut active = self.active_turn.lock().await;
if let Some(at) = active.as_mut() {
let mut ts = at.turn_state.lock().await;
let _ = ts.remove_pending_request_permissions(&call_id);
}
None
}
response = rx_response => response.ok(),
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn request_user_input(
&self,
turn_context: &TurnContext,
@@ -1973,6 +2071,10 @@ impl Session {
rx_response.await.ok()
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn notify_user_input_response(
&self,
sub_id: &str,
@@ -1998,36 +2100,33 @@ impl Session {
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn notify_request_permissions_response(
&self,
call_id: &str,
response: RequestPermissionsResponse,
) {
let mut granted_for_session = None;
let entry = {
let (entry, originating_turn_state) = {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
Some(at) => {
let mut ts = at.turn_state.lock().await;
let entry = ts.remove_pending_request_permissions(call_id);
if entry.is_some() && !response.permissions.is_empty() {
match response.scope {
PermissionGrantScope::Turn => {
ts.record_granted_permissions(response.permissions.clone().into());
}
PermissionGrantScope::Session => {
granted_for_session = Some(response.permissions.clone());
}
}
}
entry
let originating_turn_state = entry.as_ref().map(|_| Arc::clone(&at.turn_state));
(entry, originating_turn_state)
}
None => None,
None => (None, None),
}
};
if let Some(permissions) = granted_for_session {
let mut state = self.state.lock().await;
state.record_granted_permissions(permissions.into());
if entry.is_some() {
self.record_granted_request_permissions_for_turn(
&response,
originating_turn_state.as_ref(),
)
.await;
}
match entry {
Some(tx_response) => {
@@ -2039,6 +2138,32 @@ impl Session {
}
}
async fn record_granted_request_permissions_for_turn(
&self,
response: &RequestPermissionsResponse,
originating_turn_state: Option<&Arc<Mutex<crate::state::TurnState>>>,
) {
if response.permissions.is_empty() {
return;
}
match response.scope {
PermissionGrantScope::Turn => {
if let Some(turn_state) = originating_turn_state {
let mut ts = turn_state.lock().await;
ts.record_granted_permissions(response.permissions.clone().into());
}
}
PermissionGrantScope::Session => {
let mut state = self.state.lock().await;
state.record_granted_permissions(response.permissions.clone().into());
}
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn reads must stay consistent with the matching turn state"
)]
pub(crate) async fn granted_turn_permissions(&self) -> Option<PermissionProfile> {
let active = self.active_turn.lock().await;
let active = active.as_ref()?;
@@ -2051,6 +2176,10 @@ impl Session {
state.granted_permissions()
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn notify_dynamic_tool_response(&self, call_id: &str, response: DynamicToolResponse) {
let entry = {
let mut active = self.active_turn.lock().await;
@@ -2072,6 +2201,10 @@ impl Session {
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn notify_approval(&self, approval_id: &str, decision: ReviewDecision) {
let entry = {
let mut active = self.active_turn.lock().await;
@@ -2232,6 +2365,10 @@ impl Session {
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP app context rendering reads through the session-owned manager guard"
)]
pub(crate) async fn build_initial_context(
&self,
turn_context: &TurnContext,
@@ -2390,7 +2527,7 @@ impl Session {
text: user_instructions.to_string(),
directory: turn_context.cwd.to_string_lossy().into_owned(),
}
.serialize_to_text(),
.render(),
);
}
if turn_context.config.include_environment_context {
@@ -2400,9 +2537,9 @@ impl Session {
.format_environment_context_subagents(self.conversation_id)
.await;
contextual_user_sections.push(
EnvironmentContext::from_turn_context(turn_context, shell.as_ref())
crate::context::EnvironmentContext::from_turn_context(turn_context, shell.as_ref())
.with_subagents(subagents)
.serialize_to_xml(),
.render(),
);
}
@@ -2694,6 +2831,10 @@ impl Session {
/// Inject additional user input into the currently active turn.
///
/// Returns the active turn id when accepted.
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn steer_input(
&self,
input: Vec<UserInput>,
@@ -2753,6 +2894,10 @@ impl Session {
}
/// Returns the input if there was no task running to inject into.
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn inject_response_items(
&self,
input: Vec<ResponseInputItem>,
@@ -2793,6 +2938,14 @@ impl Session {
.set_mailbox_delivery_phase(MailboxDeliveryPhase::CurrentTurn);
}
pub(crate) async fn record_memory_citation_for_turn(&self, sub_id: &str) {
let turn_state = self.turn_state_for_sub_id(sub_id).await;
let Some(turn_state) = turn_state else {
return;
};
turn_state.lock().await.has_memory_citation = true;
}
async fn turn_state_for_sub_id(
&self,
sub_id: &str,
@@ -2818,6 +2971,10 @@ impl Session {
self.mailbox_rx.lock().await.has_pending_trigger_turn()
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn prepend_pending_input(&self, input: Vec<ResponseInputItem>) -> Result<(), ()> {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
@@ -2830,6 +2987,10 @@ impl Session {
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn get_pending_input(&self) -> Vec<ResponseInputItem> {
let (pending_input, accepts_mailbox_delivery) = {
let mut active = self.active_turn.lock().await;
@@ -2885,6 +3046,10 @@ impl Session {
!self.idle_pending_input.lock().await.is_empty()
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state reads must remain atomic"
)]
pub async fn has_pending_input(&self) -> bool {
let (has_turn_pending_input, accepts_mailbox_delivery) = {
let active = self.active_turn.lock().await;
+22 -5
View File
@@ -1,4 +1,5 @@
use super::*;
use tokio::sync::Semaphore;
/// Context for an initialized model agent
///
@@ -11,7 +12,7 @@ pub(crate) struct Session {
pub(super) state: Mutex<SessionState>,
/// Serializes rebuild/apply cycles for the running proxy; each cycle
/// rebuilds from the current SessionState while holding this lock.
pub(super) managed_network_proxy_refresh_lock: Mutex<()>,
pub(super) managed_network_proxy_refresh_lock: Semaphore,
/// The set of enabled features should be invariant for the lifetime of the
/// session.
pub(super) features: ManagedFeatures,
@@ -25,7 +26,7 @@ pub(crate) struct Session {
pub(crate) services: SessionServices,
pub(super) js_repl: Arc<JsReplHandle>,
pub(super) next_internal_sub_id: AtomicU64,
pub(super) agent_task_registration_lock: Mutex<()>,
pub(super) agent_task_registration_lock: Semaphore,
}
#[derive(Clone)]
@@ -208,6 +209,10 @@ pub(crate) struct AppServerClientMetadata {
impl Session {
#[instrument(name = "session_init", level = "info", skip_all)]
#[allow(clippy::too_many_arguments)]
#[expect(
clippy::await_holding_invalid_type,
reason = "session initialization must serialize access through session-owned manager guards"
)]
pub(crate) async fn new(
mut session_configuration: SessionConfiguration,
config: Arc<Config>,
@@ -332,8 +337,20 @@ impl Session {
let mcp_manager_for_mcp = Arc::clone(&mcp_manager);
let auth_and_mcp_fut = async move {
let auth = auth_manager_clone.auth().await;
let authorization_header_value = match auth.as_ref() {
Some(auth) => {
auth_manager_clone
.chatgpt_authorization_header_for_auth(auth)
.await
}
None => None,
};
let mcp_servers = mcp_manager_for_mcp
.effective_servers(&config_for_mcp, auth.as_ref())
.effective_servers_with_authorization_header(
&config_for_mcp,
auth.as_ref(),
authorization_header_value.as_deref(),
)
.await;
let auth_statuses = compute_auth_statuses(
mcp_servers.iter(),
@@ -697,7 +714,7 @@ impl Session {
agent_status,
out_of_band_elicitation_paused,
state: Mutex::new(state),
managed_network_proxy_refresh_lock: Mutex::new(()),
managed_network_proxy_refresh_lock: Semaphore::new(/*permits*/ 1),
features: config.features.clone(),
pending_mcp_server_refresh_config: Mutex::new(None),
conversation: Arc::new(RealtimeConversationManager::new()),
@@ -709,7 +726,7 @@ impl Session {
services,
js_repl,
next_internal_sub_id: AtomicU64::new(0),
agent_task_registration_lock: Mutex::new(()),
agent_task_registration_lock: Semaphore::new(/*permits*/ 1),
});
if let Some(network_policy_decider_session) = network_policy_decider_session {
let mut guard = network_policy_decider_session.write().await;
+72 -24
View File
@@ -11,6 +11,8 @@ use crate::config_loader::NetworkDomainPermissionsToml;
use crate::config_loader::RequirementSource;
use crate::config_loader::Sourced;
use crate::config_loader::project_trust_key;
use crate::context::ContextualUserFragment;
use crate::context::TurnAborted;
use crate::exec::ExecCapturePolicy;
use crate::function_tool::FunctionCallError;
use crate::shell::default_user_shell;
@@ -143,6 +145,7 @@ use sha2::Digest as _;
use sha2::Sha512;
use std::path::Path;
use std::time::Duration;
use tokio::sync::Semaphore;
use tokio::time::sleep;
use tokio::time::timeout;
use tracing_opentelemetry::OpenTelemetrySpanExt;
@@ -201,8 +204,8 @@ fn test_session_telemetry_without_metadata() -> SessionTelemetry {
.expect("in-memory metrics client");
SessionTelemetry::new(
ThreadId::new(),
"gpt-5.1",
"gpt-5.1",
"gpt-5.4",
"gpt-5.4",
/*account_id*/ None,
/*account_email*/ None,
/*auth_mode*/ None,
@@ -454,7 +457,7 @@ async fn preview_session_start_hooks(
session_id: ThreadId::new(),
cwd: config.cwd.clone(),
transcript_path: None,
model: "gpt-5".to_string(),
model: "gpt-5.2".to_string(),
permission_mode: "default".to_string(),
source: codex_hooks::SessionStartSource::Startup,
}),
@@ -1041,19 +1044,19 @@ async fn get_base_instructions_no_user_content() {
};
let test_cases = vec![
InstructionsTestCase {
slug: "gpt-5",
slug: "gpt-5.4",
expects_apply_patch_description: false,
},
InstructionsTestCase {
slug: "gpt-5.1",
slug: "gpt-5.4-mini",
expects_apply_patch_description: false,
},
InstructionsTestCase {
slug: "gpt-5.1-codex",
slug: "gpt-5.3-codex",
expects_apply_patch_description: false,
},
InstructionsTestCase {
slug: "gpt-5.1-codex-max",
slug: "gpt-5.2",
expects_apply_patch_description: false,
},
];
@@ -2537,19 +2540,19 @@ async fn turn_context_with_model_updates_model_fields() {
let (session, mut turn_context) = make_session_and_context().await;
turn_context.reasoning_effort = Some(ReasoningEffortConfig::Minimal);
let updated = turn_context
.with_model("gpt-5.1".to_string(), &session.services.models_manager)
.with_model("gpt-5.4".to_string(), &session.services.models_manager)
.await;
let expected_model_info = session
.services
.models_manager
.get_model_info(
"gpt-5.1",
"gpt-5.4",
&updated.config.as_ref().to_models_manager_config(),
)
.await;
assert_eq!(updated.config.model.as_deref(), Some("gpt-5.1"));
assert_eq!(updated.collaboration_mode.model(), "gpt-5.1");
assert_eq!(updated.config.model.as_deref(), Some("gpt-5.4"));
assert_eq!(updated.collaboration_mode.model(), "gpt-5.4");
assert_eq!(updated.model_info, expected_model_info);
assert_eq!(
updated.reasoning_effort,
@@ -3293,7 +3296,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
agent_status: agent_status_tx,
out_of_band_elicitation_paused: watch::channel(false).0,
state: Mutex::new(state),
managed_network_proxy_refresh_lock: Mutex::new(()),
managed_network_proxy_refresh_lock: Semaphore::new(/*permits*/ 1),
features: config.features.clone(),
pending_mcp_server_refresh_config: Mutex::new(None),
conversation: Arc::new(RealtimeConversationManager::new()),
@@ -3305,7 +3308,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
services,
js_repl,
next_internal_sub_id: AtomicU64::new(0),
agent_task_registration_lock: Mutex::new(()),
agent_task_registration_lock: Semaphore::new(/*permits*/ 1),
};
(session, turn_context)
@@ -3437,6 +3440,41 @@ async fn notify_request_permissions_response_ignores_unmatched_call_id() {
assert_eq!(session.granted_turn_permissions().await, None);
}
#[tokio::test]
async fn record_granted_request_permissions_for_turn_uses_originating_turn() {
let (session, _turn_context) = make_session_and_context().await;
let originating_active_turn = ActiveTurn::default();
let originating_turn_state = Arc::clone(&originating_active_turn.turn_state);
*session.active_turn.lock().await = Some(originating_active_turn);
let current_active_turn = ActiveTurn::default();
let current_turn_state = Arc::clone(&current_active_turn.turn_state);
*session.active_turn.lock().await = Some(current_active_turn);
let requested_permissions = RequestPermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
enabled: Some(true),
}),
..RequestPermissionProfile::default()
};
session
.record_granted_request_permissions_for_turn(
&codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: requested_permissions.clone(),
scope: PermissionGrantScope::Turn,
},
Some(&originating_turn_state),
)
.await;
assert_eq!(
originating_turn_state.lock().await.granted_permissions(),
Some(requested_permissions.into())
);
assert_eq!(current_turn_state.lock().await.granted_permissions(), None);
assert_eq!(session.granted_turn_permissions().await, None);
}
#[tokio::test]
async fn request_permissions_emits_event_when_granular_policy_allows_requests() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
@@ -3473,7 +3511,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
async move {
session
.request_permissions(
turn_context.as_ref(),
&turn_context,
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
reason: Some("need network".to_string()),
@@ -3484,6 +3522,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
..RequestPermissionProfile::default()
},
},
CancellationToken::new(),
)
.await
}
@@ -3531,7 +3570,7 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req
let call_id = "call-1".to_string();
let response = session
.request_permissions(
turn_context.as_ref(),
&turn_context,
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
reason: Some("need network".to_string()),
@@ -3542,6 +3581,7 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req
..RequestPermissionProfile::default()
},
},
CancellationToken::new(),
)
.await;
@@ -4263,7 +4303,7 @@ where
agent_status: agent_status_tx,
out_of_band_elicitation_paused: watch::channel(false).0,
state: Mutex::new(state),
managed_network_proxy_refresh_lock: Mutex::new(()),
managed_network_proxy_refresh_lock: Semaphore::new(/*permits*/ 1),
features: config.features.clone(),
pending_mcp_server_refresh_config: Mutex::new(None),
conversation: Arc::new(RealtimeConversationManager::new()),
@@ -4275,7 +4315,7 @@ where
services,
js_repl,
next_internal_sub_id: AtomicU64::new(0),
agent_task_registration_lock: Mutex::new(()),
agent_task_registration_lock: Semaphore::new(/*permits*/ 1),
});
(session, turn_context, rx_event)
@@ -4467,6 +4507,7 @@ fn seed_stored_identity(
agent_runtime_id: stored_identity.agent_runtime_id.clone(),
agent_private_key: stored_identity.private_key_pkcs8_base64.clone(),
registered_at: stored_identity.registered_at.clone(),
background_task_id: None,
})
.expect("store identity");
@@ -5301,10 +5342,10 @@ async fn record_context_updates_and_set_reference_context_item_reinjects_full_co
async fn record_context_updates_and_set_reference_context_item_persists_baseline_without_emitting_diffs()
{
let (session, previous_context) = make_session_and_context().await;
let next_model = if previous_context.model_info.slug == "gpt-5.1" {
"gpt-5"
let next_model = if previous_context.model_info.slug == "gpt-5.4" {
"gpt-5.2"
} else {
"gpt-5.1"
"gpt-5.4"
};
let turn_context = previous_context
.with_model(next_model.to_string(), &session.services.models_manager)
@@ -5453,10 +5494,10 @@ async fn build_initial_context_prepends_model_switch_message() {
async fn record_context_updates_and_set_reference_context_item_persists_full_reinjection_to_rollout()
{
let (session, previous_context) = make_session_and_context().await;
let next_model = if previous_context.model_info.slug == "gpt-5.1" {
"gpt-5"
let next_model = if previous_context.model_info.slug == "gpt-5.4" {
"gpt-5.2"
} else {
"gpt-5.1"
"gpt-5.4"
};
let turn_context = previous_context
.with_model(next_model.to_string(), &session.services.models_manager)
@@ -6310,7 +6351,7 @@ async fn abort_review_task_emits_exited_then_aborted_and_records_history() {
let ContentItem::InputText { text } = content_item else {
return false;
};
text.contains(crate::contextual_user_message::TURN_ABORTED_OPEN_TAG)
TurnAborted::matches_text(text)
})
}),
"expected a model-visible turn aborted marker in history after interrupt"
@@ -6318,6 +6359,10 @@ async fn abort_review_task_emits_exited_then_aborted_and_records_history() {
}
#[tokio::test]
#[expect(
clippy::await_holding_invalid_type,
reason = "test builds a router from session-owned MCP manager state"
)]
async fn fatal_tool_error_stops_turn_and_reports_error() {
let (session, turn_context, _rx) = make_session_and_context_with_rx().await;
let tools = {
@@ -6358,6 +6403,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() {
.dispatch_tool_call_with_code_mode_result(
Arc::clone(&session),
Arc::clone(&turn_context),
CancellationToken::new(),
tracker,
call,
ToolCallSource::Direct,
@@ -6600,6 +6646,7 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
.handle(ToolInvocation {
session: Arc::clone(&session),
turn: Arc::clone(&turn_context),
cancellation_token: CancellationToken::new(),
tracker: Arc::clone(&turn_diff_tracker),
call_id,
tool_name: codex_tools::ToolName::plain(tool_name),
@@ -6678,6 +6725,7 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request()
.handle(ToolInvocation {
session: Arc::clone(&session),
turn: Arc::clone(&turn_context),
cancellation_token: CancellationToken::new(),
tracker: Arc::clone(&tracker),
call_id: "exec-call".to_string(),
tool_name: codex_tools::ToolName::plain("exec_command"),
@@ -17,6 +17,7 @@ use codex_execpolicy::Evaluation;
use codex_execpolicy::RuleMatch;
use codex_features::Feature;
use codex_model_provider::create_model_provider;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::models::ContentItem;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::models::PermissionProfile;
@@ -25,26 +26,214 @@ use codex_protocol::models::function_call_output_content_items_to_text;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_protocol::request_permissions::RequestPermissionsArgs;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use core_test_support::PathExt;
use core_test_support::TempDirExt;
use core_test_support::codex_linux_sandbox_exe_or_skip;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_response_once;
use core_test_support::responses::mount_sse_once;
use core_test_support::responses::sse;
use core_test_support::responses::sse_response;
use core_test_support::responses::start_mock_server;
use pretty_assertions::assert_eq;
use serde::Deserialize;
use std::collections::HashMap;
use std::fs;
use std::sync::Arc;
use std::time::Duration;
use tempfile::tempdir;
use tokio_util::sync::CancellationToken;
fn expect_text_output(output: &FunctionToolOutput) -> String {
function_call_output_content_items_to_text(&output.body).unwrap_or_default()
}
#[tokio::test]
async fn request_permissions_routes_to_guardian_when_reviewer_is_enabled() {
let server = start_mock_server().await;
let guardian_request_log = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-guardian"),
ev_assistant_message(
"msg-guardian",
&serde_json::json!({
"risk_level": "low",
"user_authorization": "high",
"outcome": "allow",
"rationale": "The request grants narrowly scoped network access for this turn.",
})
.to_string(),
),
ev_completed("resp-guardian"),
]),
)
.await;
let (mut session, mut turn_context_raw) = make_session_and_context().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
turn_context_raw
.approval_policy
.set(AskForApproval::OnRequest)
.expect("test setup should allow updating approval policy");
turn_context_raw
.features
.enable(Feature::GuardianApproval)
.expect("test setup should allow enabling guardian approvals");
let mut config = (*turn_context_raw.config).clone();
config.approvals_reviewer = ApprovalsReviewer::GuardianSubagent;
config.model_provider.base_url = Some(format!("{}/v1", server.uri()));
let config = Arc::new(config);
let models_manager = Arc::new(crate::test_support::models_manager_with_provider(
config.codex_home.to_path_buf(),
Arc::clone(&session.services.auth_manager),
config.model_provider.clone(),
));
session.services.models_manager = models_manager;
turn_context_raw.config = Arc::clone(&config);
turn_context_raw.provider = create_model_provider(
config.model_provider.clone(),
turn_context_raw.auth_manager.clone(),
);
let session = Arc::new(session);
let turn_context = Arc::new(turn_context_raw);
let requested_permissions = RequestPermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..RequestPermissionProfile::default()
};
let response = tokio::time::timeout(
Duration::from_secs(45),
session.request_permissions(
&turn_context,
"perm-call-1".to_string(),
RequestPermissionsArgs {
reason: Some("need network".to_string()),
permissions: requested_permissions.clone(),
},
CancellationToken::new(),
),
)
.await
.expect("request_permissions should not wait for a client approval");
assert_eq!(
response,
Some(RequestPermissionsResponse {
permissions: requested_permissions.clone(),
scope: PermissionGrantScope::Turn,
})
);
assert_eq!(
session.granted_turn_permissions().await,
Some(requested_permissions.into())
);
let guardian_request = guardian_request_log.single_request();
assert_eq!(guardian_request.path(), "/v1/responses");
assert!(guardian_request.body_contains_text("request_permissions"));
assert!(guardian_request.body_contains_text("need network"));
}
#[tokio::test]
async fn request_permissions_guardian_review_stops_when_cancelled() {
let server = start_mock_server().await;
let _guardian_request_log = mount_response_once(
&server,
sse_response(sse(vec![ev_response_created("resp-guardian-delayed")]))
.set_delay(Duration::from_secs(60)),
)
.await;
let (mut session, mut turn_context, rx_event) = make_session_and_context_with_rx().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
let turn_context_raw = Arc::get_mut(&mut turn_context).expect("single turn context ref");
turn_context_raw
.approval_policy
.set(AskForApproval::OnRequest)
.expect("test setup should allow updating approval policy");
turn_context_raw
.features
.enable(Feature::GuardianApproval)
.expect("test setup should allow enabling guardian approvals");
let mut config = (*turn_context_raw.config).clone();
config.approvals_reviewer = ApprovalsReviewer::GuardianSubagent;
config.model_provider.base_url = Some(format!("{}/v1", server.uri()));
let config = Arc::new(config);
let models_manager = Arc::new(crate::test_support::models_manager_with_provider(
config.codex_home.to_path_buf(),
Arc::clone(&session.services.auth_manager),
config.model_provider.clone(),
));
Arc::get_mut(&mut session)
.expect("single session ref")
.services
.models_manager = models_manager;
turn_context_raw.config = Arc::clone(&config);
turn_context_raw.provider = create_model_provider(
config.model_provider.clone(),
turn_context_raw.auth_manager.clone(),
);
let requested_permissions = RequestPermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..RequestPermissionProfile::default()
};
let cancellation_token = CancellationToken::new();
let request_handle = tokio::spawn({
let session = Arc::clone(&session);
let turn_context = Arc::clone(&turn_context);
let requested_permissions = requested_permissions.clone();
let cancellation_token = cancellation_token.clone();
async move {
session
.request_permissions(
&turn_context,
"perm-call-cancelled".to_string(),
RequestPermissionsArgs {
reason: Some("need network".to_string()),
permissions: requested_permissions,
},
cancellation_token,
)
.await
}
});
timeout(Duration::from_secs(5), async {
loop {
let event = rx_event.recv().await.expect("event channel should be open");
if matches!(
event.msg,
codex_protocol::protocol::EventMsg::GuardianAssessment(_)
) {
break;
}
}
})
.await
.expect("guardian review should start before cancellation");
cancellation_token.cancel();
let response = timeout(Duration::from_secs(5), request_handle)
.await
.expect("request_permissions should stop when cancelled")
.expect("request_permissions task should not panic");
assert_eq!(response, None);
assert_eq!(session.granted_turn_permissions().await, None);
}
#[tokio::test]
async fn guardian_allows_shell_additional_permissions_requests_past_policy_validation() {
let server = start_mock_server().await;
@@ -146,6 +335,7 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid
.handle(ToolInvocation {
session: Arc::clone(&session),
turn: Arc::clone(&turn_context),
cancellation_token: CancellationToken::new(),
tracker: Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())),
call_id: "test-call".to_string(),
tool_name: codex_tools::ToolName::plain("shell"),
@@ -212,6 +402,7 @@ async fn guardian_allows_unified_exec_additional_permissions_requests_past_polic
.handle(ToolInvocation {
session: Arc::clone(&session),
turn: Arc::clone(&turn_context),
cancellation_token: CancellationToken::new(),
tracker: Arc::clone(&tracker),
call_id: "exec-call".to_string(),
tool_name: codex_tools::ToolName::plain("exec_command"),
@@ -298,6 +489,10 @@ async fn process_compacted_history_preserves_separate_guardian_developer_message
#[tokio::test]
#[cfg(unix)]
#[expect(
clippy::await_holding_invalid_type,
reason = "test mutates active turn state directly to seed granted permissions"
)]
async fn shell_handler_allows_sticky_turn_permissions_without_inline_request_permissions_feature() {
let (mut session, turn_context_raw) = make_session_and_context().await;
session
@@ -325,6 +520,7 @@ async fn shell_handler_allows_sticky_turn_permissions_without_inline_request_per
.handle(ToolInvocation {
session: Arc::clone(&session),
turn: Arc::clone(&turn_context),
cancellation_token: CancellationToken::new(),
tracker: Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())),
call_id: "sticky-turn-grant".to_string(),
tool_name: codex_tools::ToolName::plain("shell"),
+53 -4
View File
@@ -16,6 +16,7 @@ use crate::compact::run_inline_auto_compact_task;
use crate::compact::should_use_remote_compact_task;
use crate::compact_remote::run_inline_remote_auto_compact_task;
use crate::connectors;
use crate::context::ContextualUserFragment;
use crate::feedback_tags;
use crate::hook_runtime::PendingInputHookDisposition;
use crate::hook_runtime::emit_hook_completed_events;
@@ -93,7 +94,9 @@ use codex_protocol::protocol::ReasoningRawContentDeltaEvent;
use codex_protocol::protocol::TurnDiffEvent;
use codex_protocol::protocol::WarningEvent;
use codex_protocol::user_input::UserInput;
use codex_tools::ResponsesApiNamespaceTool;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use codex_tools::filter_tool_suggest_discoverable_tools_for_client;
use codex_utils_stream_parser::AssistantTextChunk;
use codex_utils_stream_parser::AssistantTextStreamParser;
@@ -127,6 +130,10 @@ use tracing::warn;
/// - If the model sends only an assistant message, we record it in the
/// conversation history and consider the turn complete.
///
#[expect(
clippy::await_holding_invalid_type,
reason = "turn execution must keep active-turn state transitions atomic"
)]
pub(crate) async fn run_turn(
sess: Arc<Session>,
turn_context: Arc<TurnContext>,
@@ -241,7 +248,7 @@ pub(crate) async fn run_turn(
turn_context.sub_id.clone(),
);
let SkillInjections {
items: skill_items,
items: skill_injections,
warnings: skill_warnings,
} = build_skill_injections(
&mentioned_skills,
@@ -257,6 +264,11 @@ pub(crate) async fn run_turn(
.await;
}
let skill_items: Vec<ResponseItem> = skill_injections
.iter()
.map(|skill| ContextualUserFragment::into(crate::context::SkillInstructions::from(skill)))
.collect();
let plugin_items =
build_plugin_injections(&mentioned_plugins, &mcp_tools, &available_connectors);
let mentioned_plugin_metadata = mentioned_plugins
@@ -969,7 +981,7 @@ pub(crate) fn build_prompt(
.dynamic_tools
.iter()
.filter(|tool| tool.defer_loading)
.map(|tool| tool.name.as_str())
.map(|tool| ToolName::new(tool.namespace.clone(), tool.name.clone()))
.collect::<HashSet<_>>();
let tools = if deferred_dynamic_tools.is_empty() {
router.model_visible_specs()
@@ -977,7 +989,7 @@ pub(crate) fn build_prompt(
router
.model_visible_specs()
.into_iter()
.filter(|spec| !deferred_dynamic_tools.contains(spec.name()))
.filter_map(|spec| filter_deferred_dynamic_tool_spec(spec, &deferred_dynamic_tools))
.collect()
};
@@ -991,6 +1003,35 @@ pub(crate) fn build_prompt(
}
}
fn filter_deferred_dynamic_tool_spec(
spec: ToolSpec,
deferred_dynamic_tools: &HashSet<ToolName>,
) -> Option<ToolSpec> {
match spec {
ToolSpec::Function(tool) => {
if deferred_dynamic_tools.contains(&ToolName::plain(tool.name.as_str())) {
None
} else {
Some(ToolSpec::Function(tool))
}
}
ToolSpec::Namespace(mut namespace) => {
let namespace_name = namespace.name.clone();
namespace.tools.retain(|tool| match tool {
ResponsesApiNamespaceTool::Function(tool) => !deferred_dynamic_tools.contains(
&ToolName::namespaced(namespace_name.as_str(), tool.name.as_str()),
),
});
if namespace.tools.is_empty() {
None
} else {
Some(ToolSpec::Namespace(namespace))
}
}
spec => Some(spec),
}
}
#[allow(clippy::too_many_arguments)]
#[instrument(level = "trace",
skip_all,
@@ -1143,6 +1184,10 @@ async fn run_sampling_request(
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "tool router construction reads through the session-owned manager guard"
)]
pub(crate) async fn built_tools(
sess: &Session,
turn_context: &TurnContext,
@@ -1925,7 +1970,11 @@ async fn try_run_sampling_request(
match event {
ResponseEvent::Created => {}
ResponseEvent::OutputItemDone(item) => {
active_tool_argument_diff_consumer = None;
if let Some((_, mut consumer)) = active_tool_argument_diff_consumer.take()
&& let Some(event) = consumer.flush_on_complete()
{
sess.send_event(&turn_context, event).await;
}
let previously_active_item = active_item.take();
if let Some(previous) = previously_active_item.as_ref()
&& matches!(previous, TurnItem::AgentMessage(_))
+6 -9
View File
@@ -1,20 +1,17 @@
use codex_protocol::protocol::AgentStatus;
/// Helpers for model-visible session state markers that are stored in user-role
/// messages but are not user intent.
use crate::contextual_user_message::SUBAGENT_NOTIFICATION_FRAGMENT;
use crate::context::ContextualUserFragment;
use crate::context::SubagentNotification;
// Helpers for model-visible session state markers that are stored in user-role
// messages but are not user intent.
// TODO(jif) unify with structured schema
pub(crate) fn format_subagent_notification_message(
agent_reference: &str,
status: &AgentStatus,
) -> String {
let payload_json = serde_json::json!({
"agent_path": agent_reference,
"status": status,
})
.to_string();
SUBAGENT_NOTIFICATION_FRAGMENT.wrap(payload_json)
SubagentNotification::new(agent_reference, status.clone()).render()
}
pub(crate) fn format_subagent_context_line(
+4 -9
View File
@@ -151,20 +151,15 @@ impl ShellSnapshot {
temp_path.display()
);
let temp_snapshot = Self {
path: temp_path.clone(),
cwd: session_cwd.clone(),
};
if let Err(err) = validate_snapshot(shell, &temp_snapshot.path, session_cwd).await {
if let Err(err) = validate_snapshot(shell, &temp_path, session_cwd).await {
tracing::error!("Shell snapshot validation failed: {err:?}");
remove_snapshot_file(&temp_snapshot.path).await;
remove_snapshot_file(&temp_path).await;
return Err("validation_failed");
}
if let Err(err) = fs::rename(&temp_snapshot.path, &path).await {
if let Err(err) = fs::rename(&temp_path, &path).await {
tracing::warn!("Failed to finalize shell snapshot: {err:?}");
remove_snapshot_file(&temp_snapshot.path).await;
remove_snapshot_file(&temp_path).await;
return Err("write_failed");
}
+1
View File
@@ -105,6 +105,7 @@ pub(crate) struct TurnState {
mailbox_delivery_phase: MailboxDeliveryPhase,
granted_permissions: Option<PermissionProfile>,
pub(crate) tool_calls: u64,
pub(crate) has_memory_citation: bool,
pub(crate) token_usage_at_turn_start: TokenUsage,
}
+16 -7
View File
@@ -9,8 +9,8 @@ use codex_utils_stream_parser::strip_citations;
use tokio_util::sync::CancellationToken;
use crate::function_tool::FunctionCallError;
use crate::memories::citations::get_thread_id_from_citations;
use crate::memories::citations::parse_memory_citation;
use crate::memories::citations::thread_ids_from_memory_citation;
use crate::parse_turn_item;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
@@ -137,7 +137,12 @@ pub(crate) async fn record_completed_response_item(
.await;
}
mark_thread_memory_mode_polluted_if_external_context(sess, turn_context, item).await;
record_stage1_output_usage_for_completed_item(turn_context, item).await;
let has_memory_citation =
record_stage1_output_usage_and_detect_memory_citation(turn_context, item).await;
if has_memory_citation {
sess.record_memory_citation_for_turn(&turn_context.sub_id)
.await;
}
}
fn response_item_may_include_external_context(item: &ResponseItem) -> bool {
@@ -167,23 +172,27 @@ pub(crate) async fn mark_thread_memory_mode_polluted_if_external_context(
.await;
}
async fn record_stage1_output_usage_for_completed_item(
async fn record_stage1_output_usage_and_detect_memory_citation(
turn_context: &TurnContext,
item: &ResponseItem,
) {
) -> bool {
let Some(raw_text) = raw_assistant_output_text_from_item(item) else {
return;
return false;
};
let (_, citations) = strip_citations(&raw_text);
let thread_ids = get_thread_id_from_citations(citations);
let Some(memory_citation) = parse_memory_citation(citations) else {
return false;
};
let thread_ids = thread_ids_from_memory_citation(&memory_citation);
if thread_ids.is_empty() {
return;
return true;
}
if let Some(db) = state_db::get_state_db(turn_context.config.as_ref()).await {
let _ = db.record_stage1_output_usage(&thread_ids).await;
}
true
}
/// Handle a completed output item from the model stream, recording it and
+36 -15
View File
@@ -19,8 +19,7 @@ use tracing::info_span;
use tracing::trace;
use tracing::warn;
use crate::contextual_user_message::TURN_ABORTED_CLOSE_TAG;
use crate::contextual_user_message::TURN_ABORTED_OPEN_TAG;
use crate::context::ContextualUserFragment;
use crate::hook_runtime::PendingInputHookDisposition;
use crate::hook_runtime::inspect_pending_input;
use crate::hook_runtime::record_additional_contexts;
@@ -35,10 +34,10 @@ use codex_login::AuthManager;
use codex_models_manager::manager::ModelsManager;
use codex_otel::SessionTelemetry;
use codex_otel::TURN_E2E_DURATION_METRIC;
use codex_otel::TURN_MEMORY_METRIC;
use codex_otel::TURN_NETWORK_PROXY_METRIC;
use codex_otel::TURN_TOKEN_USAGE_METRIC;
use codex_otel::TURN_TOOL_CALL_METRIC;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::EventMsg;
@@ -61,22 +60,13 @@ pub(crate) use user_shell::UserShellCommandTask;
pub(crate) use user_shell::execute_user_shell_command;
const GRACEFULL_INTERRUPTION_TIMEOUT_MS: u64 = 100;
const TURN_ABORTED_INTERRUPTED_GUIDANCE: &str = "The user interrupted the previous turn on purpose. Any running unified exec processes may still be running in the background. If any tools/commands were aborted, they may have partially executed.";
/// Shared model-visible marker used by both the real interrupt path and
/// interrupted fork snapshots.
pub(crate) fn interrupted_turn_history_marker() -> ResponseItem {
ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText {
text: format!(
"{TURN_ABORTED_OPEN_TAG}\n{TURN_ABORTED_INTERRUPTED_GUIDANCE}\n{TURN_ABORTED_CLOSE_TAG}"
),
}],
end_turn: None,
phase: None,
}
ContextualUserFragment::into(crate::context::TurnAborted::new(
crate::context::TurnAborted::INTERRUPTED_GUIDANCE,
))
}
fn emit_turn_network_proxy_metric(
@@ -96,6 +86,29 @@ fn emit_turn_network_proxy_metric(
);
}
fn emit_turn_memory_metric(
session_telemetry: &SessionTelemetry,
feature_enabled: bool,
config_enabled: bool,
has_citations: bool,
) {
let read_allowed = feature_enabled && config_enabled;
session_telemetry.counter(
TURN_MEMORY_METRIC,
/*inc*/ 1,
&[
("read_allowed", bool_tag(read_allowed)),
("feature_enabled", bool_tag(feature_enabled)),
("config_use_memories", bool_tag(config_enabled)),
("has_citations", bool_tag(has_citations)),
],
);
}
fn bool_tag(value: bool) -> &'static str {
if value { "true" } else { "false" }
}
/// Thin wrapper that exposes the parts of [`Session`] task runners need.
#[derive(Clone)]
pub(crate) struct SessionTaskContext {
@@ -409,6 +422,7 @@ impl Session {
let mut pending_input = Vec::<ResponseInputItem>::new();
let mut should_clear_active_turn = false;
let mut token_usage_at_turn_start = None;
let mut turn_had_memory_citation = false;
let mut turn_tool_calls = 0_u64;
let turn_state = {
let mut active = self.active_turn.lock().await;
@@ -428,6 +442,7 @@ impl Session {
if let Some(turn_state) = turn_state {
let mut ts = turn_state.lock().await;
pending_input = ts.take_pending_input();
turn_had_memory_citation = ts.has_memory_citation;
turn_tool_calls = ts.tool_calls;
token_usage_at_turn_start = Some(ts.token_usage_at_turn_start.clone());
}
@@ -531,6 +546,12 @@ impl Session {
&[("token_type", "reasoning_output"), tmp_mem],
);
}
emit_turn_memory_metric(
&self.services.session_telemetry,
turn_context.features.enabled(Feature::MemoryTool),
turn_context.config.memories.use_memories,
turn_had_memory_citation,
);
let (completed_at, duration_ms) = turn_context
.turn_timing_state
.completed_at_and_duration_ms()
+64 -6
View File
@@ -1,7 +1,9 @@
use super::emit_turn_memory_metric;
use super::emit_turn_network_proxy_metric;
use codex_otel::MetricsClient;
use codex_otel::MetricsConfig;
use codex_otel::SessionTelemetry;
use codex_otel::TURN_MEMORY_METRIC;
use codex_otel::TURN_NETWORK_PROXY_METRIC;
use codex_protocol::ThreadId;
use codex_protocol::protocol::SessionSource;
@@ -23,8 +25,8 @@ fn test_session_telemetry() -> SessionTelemetry {
.expect("in-memory metrics client");
SessionTelemetry::new(
ThreadId::new(),
"gpt-5.1",
"gpt-5.1",
"gpt-5.4",
"gpt-5.4",
/*account_id*/ None,
/*account_email*/ None,
/*auth_mode*/ None,
@@ -55,8 +57,8 @@ fn attributes_to_map<'a>(
.collect()
}
fn metric_point(resource_metrics: &ResourceMetrics) -> (BTreeMap<String, String>, u64) {
let metric = find_metric(resource_metrics, TURN_NETWORK_PROXY_METRIC);
fn metric_point(resource_metrics: &ResourceMetrics, name: &str) -> (BTreeMap<String, String>, u64) {
let metric = find_metric(resource_metrics, name);
match metric.data() {
AggregatedMetrics::U64(data) => match data {
MetricData::Sum(sum) => {
@@ -84,7 +86,7 @@ fn emit_turn_network_proxy_metric_records_active_turn() {
let snapshot = session_telemetry
.snapshot_metrics()
.expect("runtime metrics snapshot");
let (attrs, value) = metric_point(&snapshot);
let (attrs, value) = metric_point(&snapshot, TURN_NETWORK_PROXY_METRIC);
assert_eq!(value, 1);
assert_eq!(
@@ -109,7 +111,7 @@ fn emit_turn_network_proxy_metric_records_inactive_turn() {
let snapshot = session_telemetry
.snapshot_metrics()
.expect("runtime metrics snapshot");
let (attrs, value) = metric_point(&snapshot);
let (attrs, value) = metric_point(&snapshot, TURN_NETWORK_PROXY_METRIC);
assert_eq!(value, 1);
assert_eq!(
@@ -120,3 +122,59 @@ fn emit_turn_network_proxy_metric_records_inactive_turn() {
])
);
}
#[test]
fn emit_turn_memory_metric_records_read_allowed_with_citations() {
let session_telemetry = test_session_telemetry();
emit_turn_memory_metric(
&session_telemetry,
/*feature_enabled*/ true,
/*config_enabled*/ true,
/*has_citations*/ true,
);
let snapshot = session_telemetry
.snapshot_metrics()
.expect("runtime metrics snapshot");
let (attrs, value) = metric_point(&snapshot, TURN_MEMORY_METRIC);
assert_eq!(value, 1);
assert_eq!(
attrs,
BTreeMap::from([
("config_use_memories".to_string(), "true".to_string()),
("feature_enabled".to_string(), "true".to_string()),
("has_citations".to_string(), "true".to_string()),
("read_allowed".to_string(), "true".to_string()),
])
);
}
#[test]
fn emit_turn_memory_metric_records_config_disabled_without_citations() {
let session_telemetry = test_session_telemetry();
emit_turn_memory_metric(
&session_telemetry,
/*feature_enabled*/ true,
/*config_enabled*/ false,
/*has_citations*/ false,
);
let snapshot = session_telemetry
.snapshot_metrics()
.expect("runtime metrics snapshot");
let (attrs, value) = metric_point(&snapshot, TURN_MEMORY_METRIC);
assert_eq!(value, 1);
assert_eq!(
attrs,
BTreeMap::from([
("config_use_memories".to_string(), "false".to_string()),
("feature_enabled".to_string(), "true".to_string()),
("has_citations".to_string(), "false".to_string()),
("read_allowed".to_string(), "false".to_string()),
])
);
}
+4
View File
@@ -259,6 +259,10 @@ pub(super) async fn build_enabled_tools(
collect_code_mode_tool_definitions(&specs)
}
#[expect(
clippy::await_holding_invalid_type,
reason = "nested tool router construction reads through the session-owned manager guard"
)]
async fn build_nested_router(exec: &ExecContext) -> ToolRouter {
let nested_tools_config = exec.turn.tools_config.for_code_mode_nested_tools();
let listed_mcp_tools = exec
+4 -2
View File
@@ -15,8 +15,8 @@ use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::SearchToolCallParams;
use codex_protocol::models::ShellToolCallParams;
use codex_protocol::models::function_call_output_content_items_to_text;
use codex_tools::LoadableToolSpec;
use codex_tools::ToolName;
use codex_tools::ToolSearchOutputTool;
use codex_utils_output_truncation::TruncationPolicy;
use codex_utils_output_truncation::formatted_truncate_text;
use codex_utils_string::take_bytes_at_char_boundary;
@@ -26,6 +26,7 @@ use std::borrow::Cow;
use std::sync::Arc;
use std::time::Duration;
use tokio::sync::Mutex;
use tokio_util::sync::CancellationToken;
pub type SharedTurnDiffTracker = Arc<Mutex<TurnDiffTracker>>;
@@ -40,6 +41,7 @@ pub enum ToolCallSource {
pub struct ToolInvocation {
pub session: Arc<Session>,
pub turn: Arc<TurnContext>,
pub cancellation_token: CancellationToken,
pub tracker: SharedTurnDiffTracker,
pub call_id: String,
pub tool_name: ToolName,
@@ -184,7 +186,7 @@ impl McpToolOutput {
#[derive(Clone)]
pub struct ToolSearchOutput {
pub tools: Vec<ToolSearchOutputTool>,
pub tools: Vec<LoadableToolSpec>,
}
impl ToolOutput for ToolSearchOutput {
+12 -14
View File
@@ -284,20 +284,18 @@ fn tool_search_payloads_roundtrip_as_tool_search_outputs() {
},
};
let response = ToolSearchOutput {
tools: vec![ToolSearchOutputTool::Function(
codex_tools::ResponsesApiTool {
name: "create_event".to_string(),
description: String::new(),
strict: false,
defer_loading: Some(true),
parameters: codex_tools::JsonSchema::object(
/*properties*/ Default::default(),
/*required*/ None,
/*additional_properties*/ None,
),
output_schema: None,
},
)],
tools: vec![LoadableToolSpec::Function(codex_tools::ResponsesApiTool {
name: "create_event".to_string(),
description: String::new(),
strict: false,
defer_loading: Some(true),
parameters: codex_tools::JsonSchema::object(
/*properties*/ Default::default(),
/*required*/ None,
/*additional_properties*/ None,
),
output_schema: None,
})],
}
.to_response_item("search-1", &payload);
@@ -3,6 +3,8 @@ use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
use std::time::Instant;
use crate::apply_patch;
use crate::apply_patch::InternalApplyPatchInvocation;
@@ -44,12 +46,16 @@ use codex_sandboxing::policy_transforms::normalize_additional_permissions;
use codex_tools::ApplyPatchToolArgs;
use codex_utils_absolute_path::AbsolutePathBuf;
const APPLY_PATCH_ARGUMENT_DIFF_BUFFER_INTERVAL: Duration = Duration::from_millis(500);
pub struct ApplyPatchHandler;
#[derive(Default)]
struct ApplyPatchArgumentDiffConsumer {
input: String,
last_progress: Option<Vec<Hunk>>,
last_sent_at: Option<Instant>,
pending: Option<PatchApplyUpdatedEvent>,
}
impl ToolArgumentDiffConsumer for ApplyPatchArgumentDiffConsumer {
@@ -66,6 +72,11 @@ impl ToolArgumentDiffConsumer for ApplyPatchArgumentDiffConsumer {
self.push_delta(call_id, diff)
.map(EventMsg::PatchApplyUpdated)
}
fn flush_on_complete(&mut self) -> Option<EventMsg> {
self.flush_update_on_complete()
.map(EventMsg::PatchApplyUpdated)
}
}
impl ApplyPatchArgumentDiffConsumer {
@@ -82,7 +93,29 @@ impl ApplyPatchArgumentDiffConsumer {
let changes = convert_apply_patch_hunks_to_protocol(&hunks);
self.last_progress = Some(hunks);
Some(PatchApplyUpdatedEvent { call_id, changes })
let event = PatchApplyUpdatedEvent { call_id, changes };
let now = Instant::now();
match self.last_sent_at {
Some(last_sent_at)
if now.duration_since(last_sent_at) < APPLY_PATCH_ARGUMENT_DIFF_BUFFER_INTERVAL =>
{
self.pending = Some(event);
None
}
Some(_) | None => {
self.pending = None;
self.last_sent_at = Some(now);
Some(event)
}
}
}
fn flush_update_on_complete(&mut self) -> Option<PatchApplyUpdatedEvent> {
let event = self.pending.take();
if event.is_some() {
self.last_sent_at = Some(Instant::now());
}
event
}
}
@@ -196,10 +229,10 @@ fn write_permissions_for_paths(
.ok()?;
let permissions = (!write_paths.is_empty()).then_some(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: Some(vec![]),
write: Some(write_paths),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![]),
Some(write_paths),
)),
..Default::default()
})?;
@@ -54,9 +54,13 @@ fn diff_consumer_streams_apply_patch_changes() {
)
);
let event = consumer
.push_delta("call-1".to_string(), "\n+world")
.expect("progress event");
assert!(
consumer
.push_delta("call-1".to_string(), "\n+world")
.is_none()
);
let event = consumer.flush_update_on_complete().expect("progress event");
assert_eq!(
(event.call_id, event.changes),
(
@@ -71,6 +75,39 @@ fn diff_consumer_streams_apply_patch_changes() {
);
}
#[test]
fn diff_consumer_sends_next_update_after_buffer_interval() {
let mut consumer = ApplyPatchArgumentDiffConsumer::default();
consumer.push_delta("call-1".to_string(), "*** Begin Patch\n");
let first = consumer
.push_delta("call-1".to_string(), "*** Add File: hello.txt\n+hello")
.expect("first progress event");
assert_eq!(
first.changes,
HashMap::from([(
PathBuf::from("hello.txt"),
FileChange::Add {
content: "hello\n".to_string(),
},
)])
);
consumer.last_sent_at =
Some(std::time::Instant::now() - APPLY_PATCH_ARGUMENT_DIFF_BUFFER_INTERVAL);
let second = consumer
.push_delta("call-1".to_string(), "\n+world")
.expect("second progress event");
assert_eq!(
second.changes,
HashMap::from([(
PathBuf::from("hello.txt"),
FileChange::Add {
content: "hello\nworld\n".to_string(),
},
)])
);
}
#[tokio::test]
async fn approval_keys_include_move_destination() {
let tmp = TempDir::new().expect("tmp");
@@ -148,7 +185,10 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() {
dunce::simplified(&outside.canonicalize().expect("canonicalize outside dir")).abs();
assert_eq!(
permissions.and_then(|profile| profile.file_system.and_then(|fs| fs.write)),
permissions
.and_then(|profile| profile.file_system)
.and_then(|fs| fs.legacy_read_write_roots())
.and_then(|(_read, write)| write),
Some(vec![expected_outside])
);
}
+18 -9
View File
@@ -12,6 +12,7 @@ use codex_protocol::dynamic_tools::DynamicToolResponse;
use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::protocol::DynamicToolCallResponseEvent;
use codex_protocol::protocol::EventMsg;
use codex_tools::ToolName;
use serde_json::Value;
use std::time::Instant;
use tokio::sync::oneshot;
@@ -50,14 +51,13 @@ impl ToolHandler for DynamicToolHandler {
};
let args: Value = parse_arguments(&arguments)?;
let response =
request_dynamic_tool(&session, turn.as_ref(), call_id, tool_name.display(), args)
.await
.ok_or_else(|| {
FunctionCallError::RespondToModel(
"dynamic tool call was cancelled before receiving a response".to_string(),
)
})?;
let response = request_dynamic_tool(&session, turn.as_ref(), call_id, tool_name, args)
.await
.ok_or_else(|| {
FunctionCallError::RespondToModel(
"dynamic tool call was cancelled before receiving a response".to_string(),
)
})?;
let DynamicToolResponse {
content_items,
@@ -71,13 +71,19 @@ impl ToolHandler for DynamicToolHandler {
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and dynamic tool response registration must remain atomic"
)]
async fn request_dynamic_tool(
session: &Session,
turn_context: &TurnContext,
call_id: String,
tool: String,
tool_name: ToolName,
arguments: Value,
) -> Option<DynamicToolResponse> {
let namespace = tool_name.namespace;
let tool = tool_name.name;
let turn_id = turn_context.sub_id.clone();
let (tx_response, rx_response) = oneshot::channel();
let event_id = call_id.clone();
@@ -99,6 +105,7 @@ async fn request_dynamic_tool(
let event = EventMsg::DynamicToolCallRequest(DynamicToolCallRequest {
call_id: call_id.clone(),
turn_id: turn_id.clone(),
namespace: namespace.clone(),
tool: tool.clone(),
arguments: arguments.clone(),
});
@@ -109,6 +116,7 @@ async fn request_dynamic_tool(
Some(response) => EventMsg::DynamicToolCallResponse(DynamicToolCallResponseEvent {
call_id,
turn_id,
namespace,
tool,
arguments,
content_items: response.content_items.clone(),
@@ -119,6 +127,7 @@ async fn request_dynamic_tool(
None => EventMsg::DynamicToolCallResponse(DynamicToolCallResponseEvent {
call_id,
turn_id,
namespace,
tool,
arguments,
content_items: Vec::new(),
+8 -1
View File
@@ -109,6 +109,7 @@ impl ToolHandler for JsReplHandler {
let ToolInvocation {
session,
turn,
cancellation_token,
tracker,
payload,
call_id,
@@ -134,7 +135,13 @@ impl ToolHandler for JsReplHandler {
let started_at = Instant::now();
emit_js_repl_exec_begin(session.as_ref(), turn.as_ref(), &call_id).await;
let result = manager
.execute(Arc::clone(&session), Arc::clone(&turn), tracker, args)
.execute_with_cancellation(
Arc::clone(&session),
Arc::clone(&turn),
cancellation_token,
tracker,
args,
)
.await;
let result = match result {
Ok(result) => result,
@@ -240,6 +240,10 @@ impl ToolHandler for McpResourceHandler {
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP resource listing reads through the session-owned manager guard"
)]
async fn handle_list_resources(
session: Arc<Session>,
turn: Arc<TurnContext>,
@@ -344,6 +348,10 @@ async fn handle_list_resources(
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP resource template listing reads through the session-owned manager guard"
)]
async fn handle_list_resource_templates(
session: Arc<Session>,
turn: Arc<TurnContext>,
+4 -4
View File
@@ -238,12 +238,12 @@ mod tests {
fn file_system_permissions(path: &std::path::Path) -> PermissionProfile {
PermissionProfile {
file_system: Some(FileSystemPermissions {
read: None,
write: Some(vec![
file_system: Some(FileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![
AbsolutePathBuf::from_absolute_path(path).expect("absolute path"),
]),
}),
)),
..Default::default()
}
}
@@ -68,6 +68,7 @@ fn invocation(
ToolInvocation {
session,
turn,
cancellation_token: CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::default())),
call_id: "call-1".to_string(),
tool_name: codex_tools::ToolName::plain(tool_name),
@@ -22,6 +22,7 @@ impl ToolHandler for RequestPermissionsHandler {
let ToolInvocation {
session,
turn,
cancellation_token,
call_id,
payload,
..
@@ -48,7 +49,7 @@ impl ToolHandler for RequestPermissionsHandler {
}
let response = session
.request_permissions(turn.as_ref(), call_id, args)
.request_permissions(&turn, call_id, args, cancellation_token)
.await
.ok_or_else(|| {
FunctionCallError::RespondToModel(
@@ -27,6 +27,7 @@ async fn multi_agent_v2_request_user_input_rejects_subagent_threads() {
.handle(ToolInvocation {
session: Arc::new(session),
turn: Arc::new(turn),
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::default())),
call_id: "call-1".to_string(),
tool_name: codex_tools::ToolName::plain(REQUEST_USER_INPUT_TOOL_NAME),
@@ -225,6 +225,7 @@ async fn shell_pre_tool_use_payload_uses_joined_command() {
handler.pre_tool_use_payload(&ToolInvocation {
session: session.into(),
turn: turn.into(),
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-41".to_string(),
tool_name: codex_tools::ToolName::plain("shell"),
@@ -250,6 +251,7 @@ async fn shell_command_pre_tool_use_payload_uses_raw_command() {
handler.pre_tool_use_payload(&ToolInvocation {
session: session.into(),
turn: turn.into(),
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-42".to_string(),
tool_name: codex_tools::ToolName::plain("shell_command"),
+28 -46
View File
@@ -9,9 +9,10 @@ use bm25::Document;
use bm25::Language;
use bm25::SearchEngine;
use bm25::SearchEngineBuilder;
use codex_tools::LoadableToolSpec;
use codex_tools::TOOL_SEARCH_DEFAULT_LIMIT;
use codex_tools::TOOL_SEARCH_TOOL_NAME;
use codex_tools::ToolSearchOutputTool;
use codex_tools::coalesce_loadable_tool_specs;
use std::collections::HashMap;
const COMPUTER_USE_MCP_SERVER_NAME: &str = "computer-use";
@@ -93,7 +94,7 @@ impl ToolSearchHandler {
query: &str,
limit: usize,
use_default_limit: bool,
) -> Result<Vec<ToolSearchOutputTool>, FunctionCallError> {
) -> Result<Vec<LoadableToolSpec>, FunctionCallError> {
let results = self.search_result_entries(query, limit, use_default_limit);
self.search_output_tools(results)
}
@@ -135,34 +136,10 @@ impl ToolSearchHandler {
fn search_output_tools<'a>(
&self,
results: impl IntoIterator<Item = &'a ToolSearchEntry>,
) -> Result<Vec<ToolSearchOutputTool>, FunctionCallError> {
let mut tools = Vec::new();
// Preserve search order: group namespace children, emit standalone tools directly.
for entry in results {
match &entry.output {
ToolSearchOutputTool::Function(tool) => {
tools.push(ToolSearchOutputTool::Function(tool.clone()));
}
ToolSearchOutputTool::Namespace(namespace) => {
if let Some(output) = tools.iter_mut().find_map(|tool| match tool {
ToolSearchOutputTool::Namespace(output)
if output.name == namespace.name =>
{
Some(output)
}
ToolSearchOutputTool::Namespace(_) | ToolSearchOutputTool::Function(_) => {
None
}
}) {
output.tools.extend(namespace.tools.clone());
} else {
tools.push(ToolSearchOutputTool::Namespace(namespace.clone()));
}
}
}
}
Ok(tools)
) -> Result<Vec<LoadableToolSpec>, FunctionCallError> {
Ok(coalesce_loadable_tool_specs(
results.into_iter().map(|entry| entry.output.clone()),
))
}
}
@@ -209,6 +186,7 @@ mod tests {
#[test]
fn mixed_search_results_coalesce_mcp_namespaces() {
let dynamic_tools = vec![DynamicToolSpec {
namespace: Some("codex_app".to_string()),
name: "automation_update".to_string(),
description: "Create, update, view, or delete recurring automations.".to_string(),
input_schema: serde_json::json!({
@@ -247,7 +225,7 @@ mod tests {
assert_eq!(
tools,
vec![
ToolSearchOutputTool::Namespace(ResponsesApiNamespace {
LoadableToolSpec::Namespace(ResponsesApiNamespace {
name: "mcp__calendar__".to_string(),
description: "Tools in the mcp__calendar__ namespace.".to_string(),
tools: vec![
@@ -277,21 +255,25 @@ mod tests {
}),
],
}),
ToolSearchOutputTool::Function(ResponsesApiTool {
name: "automation_update".to_string(),
description: "Create, update, view, or delete recurring automations."
.to_string(),
strict: false,
defer_loading: Some(true),
parameters: codex_tools::JsonSchema::object(
std::collections::BTreeMap::from([(
"mode".to_string(),
codex_tools::JsonSchema::string(/*description*/ None),
)]),
Some(vec!["mode".to_string()]),
Some(false.into()),
),
output_schema: None,
LoadableToolSpec::Namespace(ResponsesApiNamespace {
name: "codex_app".to_string(),
description: "Tools in the codex_app namespace.".to_string(),
tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool {
name: "automation_update".to_string(),
description: "Create, update, view, or delete recurring automations."
.to_string(),
strict: false,
defer_loading: Some(true),
parameters: codex_tools::JsonSchema::object(
std::collections::BTreeMap::from([(
"mode".to_string(),
codex_tools::JsonSchema::string(/*description*/ None),
)]),
Some(vec!["mode".to_string()]),
Some(false.into()),
),
output_schema: None,
})],
}),
],
);
@@ -34,6 +34,10 @@ impl ToolHandler for ToolSuggestHandler {
ToolKind::Function
}
#[expect(
clippy::await_holding_invalid_type,
reason = "tool suggestion discovery reads through the session-owned manager guard"
)]
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
payload,
@@ -193,6 +197,10 @@ async fn verify_tool_suggestion_completed(
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "connector cache refresh reads through the session-owned manager guard"
)]
async fn refresh_missing_suggested_connectors(
session: &crate::session::session::Session,
turn: &crate::session::turn_context::TurnContext,
@@ -188,10 +188,10 @@ fn exec_command_args_resolve_relative_additional_permissions_against_workdir() -
assert_eq!(
args.additional_permissions,
Some(PermissionProfile {
file_system: Some(FileSystemPermissions {
read: None,
write: Some(vec![expected_write.abs()]),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![expected_write.abs()]),
)),
..Default::default()
})
);
@@ -210,6 +210,7 @@ async fn exec_command_pre_tool_use_payload_uses_raw_command() {
handler.pre_tool_use_payload(&ToolInvocation {
session: session.into(),
turn: turn.into(),
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-43".to_string(),
tool_name: codex_tools::ToolName::plain("exec_command"),
@@ -233,6 +234,7 @@ async fn exec_command_pre_tool_use_payload_skips_write_stdin() {
handler.pre_tool_use_payload(&ToolInvocation {
session: session.into(),
turn: turn.into(),
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-44".to_string(),
tool_name: codex_tools::ToolName::plain("write_stdin"),
+38 -1
View File
@@ -132,6 +132,7 @@ struct KernelState {
struct ExecContext {
session: Arc<Session>,
turn: Arc<TurnContext>,
cancellation_token: CancellationToken,
tracker: SharedTurnDiffTracker,
}
@@ -836,12 +837,29 @@ impl JsReplManager {
}
}
#[cfg(test)]
pub async fn execute(
&self,
session: Arc<Session>,
turn: Arc<TurnContext>,
tracker: SharedTurnDiffTracker,
args: JsReplArgs,
) -> Result<JsExecResult, FunctionCallError> {
self.execute_with_cancellation(session, turn, CancellationToken::new(), tracker, args)
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "js_repl kernel initialization must be serialized with kernel state"
)]
pub async fn execute_with_cancellation(
&self,
session: Arc<Session>,
turn: Arc<TurnContext>,
cancellation_token: CancellationToken,
tracker: SharedTurnDiffTracker,
args: JsReplArgs,
) -> Result<JsExecResult, FunctionCallError> {
let _permit = self.exec_lock.clone().acquire_owned().await.map_err(|_| {
FunctionCallError::RespondToModel("js_repl execution unavailable".to_string())
@@ -892,6 +910,7 @@ impl JsReplManager {
ExecContext {
session: Arc::clone(&session),
turn: Arc::clone(&turn),
cancellation_token,
tracker,
},
);
@@ -1162,6 +1181,10 @@ impl JsReplManager {
Ok(kernel_path)
}
#[expect(
clippy::await_holding_invalid_type,
reason = "js_repl stdin writes must be serialized per kernel"
)]
async fn write_message(
stdin: &Arc<Mutex<ChildStdin>>,
msg: &HostToKernel,
@@ -1209,6 +1232,10 @@ impl JsReplManager {
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "js_repl child shutdown must serialize process inspection and termination"
)]
async fn kill_kernel_child(child: &Arc<Mutex<Child>>, reason: &'static str) {
let mut guard = child.lock().await;
let pid = guard.id();
@@ -1532,6 +1559,10 @@ impl JsReplManager {
}
}
#[expect(
clippy::await_holding_invalid_type,
reason = "nested js_repl tool routing reads through the session-owned manager guard"
)]
async fn run_tool_request(exec: ExecContext, req: RunToolRequest) -> RunToolResult {
if is_js_repl_internal_tool(&req.tool_name) {
let error = "js_repl cannot invoke itself".to_string();
@@ -1588,7 +1619,11 @@ impl JsReplManager {
ResponsesApiNamespaceTool::Function(tool) => {
let tool_name =
ToolName::namespaced(namespace.name.clone(), tool.name.clone());
(tool_name.display() == req.tool_name).then_some(tool_name)
let code_mode_name =
codex_tools::code_mode_name_for_tool_name(&tool_name);
(code_mode_name == req.tool_name
|| tool_name.display() == req.tool_name)
.then_some(tool_name)
}
})
}
@@ -1640,12 +1675,14 @@ impl JsReplManager {
let session = Arc::clone(&exec.session);
let turn = Arc::clone(&exec.turn);
let cancellation_token = exec.cancellation_token.clone();
let tracker = Arc::clone(&exec.tracker);
match router
.dispatch_tool_call_with_code_mode_result(
session,
turn,
cancellation_token,
tracker,
call,
crate::tools::router::ToolCallSource::JsRepl,
+4 -1
View File
@@ -1819,6 +1819,7 @@ async fn js_repl_emit_image_rejects_mixed_content() -> anyhow::Result<()> {
let (session, turn, rx_event) =
make_session_and_context_with_dynamic_tools_and_rx(vec![DynamicToolSpec {
namespace: None,
name: "inline_image".to_string(),
description: "Returns inline text and image content.".to_string(),
input_schema: serde_json::json!({
@@ -1918,6 +1919,7 @@ async fn js_repl_dynamic_tool_response_preserves_js_line_separator_text() -> any
] {
let (session, turn, rx_event) =
make_session_and_context_with_dynamic_tools_and_rx(vec![DynamicToolSpec {
namespace: None,
name: tool_name.to_string(),
description: description.to_string(),
input_schema: serde_json::json!({
@@ -1993,6 +1995,7 @@ async fn js_repl_can_call_hidden_dynamic_tools() -> anyhow::Result<()> {
let (session, turn, rx_event) =
make_session_and_context_with_dynamic_tools_and_rx(vec![DynamicToolSpec {
namespace: Some("codex_app".to_string()),
name: "hidden_dynamic_tool".to_string(),
description: "A hidden dynamic tool.".to_string(),
input_schema: serde_json::json!({
@@ -2012,7 +2015,7 @@ async fn js_repl_can_call_hidden_dynamic_tools() -> anyhow::Result<()> {
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default()));
let manager = turn.js_repl.manager().await?;
let code = r#"
const out = await codex.tool("hidden_dynamic_tool", { city: "Paris" });
const out = await codex.tool("codex_app_hidden_dynamic_tool", { city: "Paris" });
console.log(JSON.stringify(out));
"#;
+2
View File
@@ -92,6 +92,7 @@ impl ToolCallRuntime {
let turn = Arc::clone(&self.turn_context);
let tracker = Arc::clone(&self.tracker);
let lock = Arc::clone(&self.parallel_execution);
let invocation_cancellation_token = cancellation_token.clone();
let started = Instant::now();
let display_name = call.tool_name.display();
@@ -122,6 +123,7 @@ impl ToolCallRuntime {
.dispatch_tool_call_with_code_mode_result(
session,
turn,
invocation_cancellation_token,
tracker,
call.clone(),
source,
+9
View File
@@ -95,6 +95,11 @@ pub(crate) trait ToolArgumentDiffConsumer: Send {
/// Consume the next argument diff for a tool call.
fn consume_diff(&mut self, turn: &TurnContext, call_id: String, diff: &str)
-> Option<EventMsg>;
/// Flush any buffered event before the tool call completes.
fn flush_on_complete(&mut self) -> Option<EventMsg> {
None
}
}
pub(crate) struct AnyToolResult {
@@ -234,6 +239,10 @@ impl ToolRegistry {
// }
// }
#[expect(
clippy::await_holding_invalid_type,
reason = "tool dispatch must keep active-turn accounting atomic"
)]
pub(crate) async fn dispatch_any(
&self,
invocation: ToolInvocation,
+3
View File
@@ -24,6 +24,7 @@ use codex_tools::ToolsConfig;
use std::collections::HashMap;
use std::collections::HashSet;
use std::sync::Arc;
use tokio_util::sync::CancellationToken;
use tracing::instrument;
pub use crate::tools::context::ToolCallSource;
@@ -267,6 +268,7 @@ impl ToolRouter {
&self,
session: Arc<Session>,
turn: Arc<TurnContext>,
cancellation_token: CancellationToken,
tracker: SharedTurnDiffTracker,
call: ToolCall,
source: ToolCallSource,
@@ -292,6 +294,7 @@ impl ToolRouter {
let invocation = ToolInvocation {
session,
turn,
cancellation_token,
tracker,
call_id,
tool_name,
+16
View File
@@ -7,6 +7,7 @@ use crate::tools::context::ToolPayload;
use crate::turn_diff_tracker::TurnDiffTracker;
use codex_protocol::models::ResponseItem;
use codex_tools::ToolName;
use tokio_util::sync::CancellationToken;
use super::ToolCall;
use super::ToolCallSource;
@@ -14,6 +15,10 @@ use super::ToolRouter;
use super::ToolRouterParams;
#[tokio::test]
#[expect(
clippy::await_holding_invalid_type,
reason = "test builds a router from session-owned MCP manager state"
)]
async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> {
let (session, mut turn) = make_session_and_context().await;
turn.tools_config.js_repl_tools_only = true;
@@ -52,6 +57,7 @@ async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> {
.dispatch_tool_call_with_code_mode_result(
session,
turn,
CancellationToken::new(),
tracker,
call,
ToolCallSource::Direct,
@@ -68,6 +74,10 @@ async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> {
}
#[tokio::test]
#[expect(
clippy::await_holding_invalid_type,
reason = "test builds a router from session-owned MCP manager state"
)]
async fn js_repl_tools_only_allows_js_repl_source_calls() -> anyhow::Result<()> {
let (session, mut turn) = make_session_and_context().await;
turn.tools_config.js_repl_tools_only = true;
@@ -106,6 +116,7 @@ async fn js_repl_tools_only_allows_js_repl_source_calls() -> anyhow::Result<()>
.dispatch_tool_call_with_code_mode_result(
session,
turn,
CancellationToken::new(),
tracker,
call,
ToolCallSource::JsRepl,
@@ -155,6 +166,7 @@ async fn js_repl_tools_only_blocks_namespaced_js_repl_tool() -> anyhow::Result<(
.dispatch_tool_call_with_code_mode_result(
session,
turn,
CancellationToken::new(),
tracker,
call,
ToolCallSource::Direct,
@@ -171,6 +183,10 @@ async fn js_repl_tools_only_blocks_namespaced_js_repl_tool() -> anyhow::Result<(
}
#[tokio::test]
#[expect(
clippy::await_holding_invalid_type,
reason = "test builds a router from session-owned MCP manager state"
)]
async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow::Result<()> {
let (session, turn) = make_session_and_context().await;
let mcp_tools = session
@@ -85,10 +85,10 @@ fn file_system_sandbox_context_uses_active_attempt() {
.abs();
let additional_permissions = PermissionProfile {
network: None,
file_system: Some(FileSystemPermissions {
read: Some(vec![path.clone()]),
write: Some(Vec::new()),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![path.clone()]),
Some(Vec::new()),
)),
};
let req = ApplyPatchRequest {
action: ApplyPatchAction::new_add_for_test(&path, "hello".to_string()),
+72 -4
View File
@@ -8,6 +8,12 @@ use crate::exec_env::CODEX_THREAD_ID_ENV_VAR;
use crate::path_utils;
use crate::shell::Shell;
use crate::tools::sandboxing::ToolError;
#[cfg(target_os = "macos")]
use codex_network_proxy::CODEX_PROXY_GIT_SSH_COMMAND_MARKER;
use codex_network_proxy::PROXY_ACTIVE_ENV_KEY;
use codex_network_proxy::PROXY_ENV_KEYS;
#[cfg(target_os = "macos")]
use codex_network_proxy::PROXY_GIT_SSH_COMMAND_ENV_KEY;
use codex_protocol::models::PermissionProfile;
use codex_sandboxing::SandboxCommand;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -102,6 +108,9 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot(
override_env.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.clone());
}
let (override_captures, override_exports) = build_override_exports(&override_env);
let (proxy_captures, proxy_exports) = build_proxy_env_exports();
let override_captures = join_shell_blocks([override_captures, proxy_captures]);
let override_exports = join_shell_blocks([override_exports, proxy_exports]);
let rewritten_script = if override_exports.is_empty() {
format!(
"if . '{snapshot_path}' >/dev/null 2>&1; then :; fi\n\nexec '{original_shell}' -c '{original_script}'{trailing_args}"
@@ -118,10 +127,59 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot(
fn build_override_exports(explicit_env_overrides: &HashMap<String, String>) -> (String, String) {
let mut keys = explicit_env_overrides
.keys()
.map(String::as_str)
.filter(|key| is_valid_shell_variable_name(key))
.collect::<Vec<_>>();
keys.sort_unstable();
build_override_exports_for_keys("__CODEX_SNAPSHOT_OVERRIDE", &keys)
}
fn build_proxy_env_exports() -> (String, String) {
let mut keys = PROXY_ENV_KEYS
.iter()
.copied()
.filter(|key| is_valid_shell_variable_name(key))
.collect::<Vec<_>>();
keys.sort_unstable();
keys.dedup();
let (captures, restores) =
build_override_exports_for_keys("__CODEX_SNAPSHOT_PROXY_OVERRIDE", &keys);
let key = PROXY_ACTIVE_ENV_KEY;
let proxy_blocks = (
format!("{captures}\n__CODEX_SNAPSHOT_PROXY_ENV_SET=\"${{{key}+x}}\""),
format!(
"if [ -n \"$__CODEX_SNAPSHOT_PROXY_ENV_SET\" ] || [ -n \"${{{key}+x}}\" ]; then\n{restores}\nfi"
),
);
let git_blocks = build_codex_proxy_git_ssh_command_exports();
(
join_shell_blocks([proxy_blocks.0, git_blocks.0]),
join_shell_blocks([proxy_blocks.1, git_blocks.1]),
)
}
#[cfg(target_os = "macos")]
fn build_codex_proxy_git_ssh_command_exports() -> (String, String) {
let key = PROXY_GIT_SSH_COMMAND_ENV_KEY;
let marker_pattern = format!("{}\\ *", CODEX_PROXY_GIT_SSH_COMMAND_MARKER.trim_end());
(
format!(
"__CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND_SET=\"${{{key}+x}}\"\n__CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND=\"${{{key}-}}\"\ncase \"$__CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND\" in\n {marker_pattern}) __CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND_LIVE_MARKED=1 ;;\n *) __CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND_LIVE_MARKED= ;;\nesac"
),
format!(
"case \"${{{key}-}}\" in\n {marker_pattern}) __CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND_AFTER_MARKED=1 ;;\n *) __CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND_AFTER_MARKED= ;;\nesac\nif [ -n \"$__CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND_LIVE_MARKED\" ]; then\n if [ -z \"${{{key}+x}}\" ] || [ -n \"$__CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND_AFTER_MARKED\" ]; then\n export {key}=\"$__CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND\"\n fi\nelif [ -n \"$__CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND_AFTER_MARKED\" ]; then\n if [ -n \"$__CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND_SET\" ]; then\n export {key}=\"$__CODEX_SNAPSHOT_PROXY_GIT_SSH_COMMAND\"\n else\n unset {key}\n fi\nfi"
),
)
}
#[cfg(not(target_os = "macos"))]
fn build_codex_proxy_git_ssh_command_exports() -> (String, String) {
(String::new(), String::new())
}
fn build_override_exports_for_keys(variable_prefix: &str, keys: &[&str]) -> (String, String) {
if keys.is_empty() {
return (String::new(), String::new());
}
@@ -130,9 +188,9 @@ fn build_override_exports(explicit_env_overrides: &HashMap<String, String>) -> (
.iter()
.enumerate()
.map(|(idx, key)| {
format!(
"__CODEX_SNAPSHOT_OVERRIDE_SET_{idx}=\"${{{key}+x}}\"\n__CODEX_SNAPSHOT_OVERRIDE_{idx}=\"${{{key}-}}\""
)
let set_var = format!("{variable_prefix}_SET_{idx}");
let value_var = format!("{variable_prefix}_{idx}");
format!("{set_var}=\"${{{key}+x}}\"\n{value_var}=\"${{{key}-}}\"")
})
.collect::<Vec<_>>()
.join("\n");
@@ -140,8 +198,10 @@ fn build_override_exports(explicit_env_overrides: &HashMap<String, String>) -> (
.iter()
.enumerate()
.map(|(idx, key)| {
let set_var = format!("{variable_prefix}_SET_{idx}");
let value_var = format!("{variable_prefix}_{idx}");
format!(
"if [ -n \"${{__CODEX_SNAPSHOT_OVERRIDE_SET_{idx}}}\" ]; then export {key}=\"${{__CODEX_SNAPSHOT_OVERRIDE_{idx}}}\"; else unset {key}; fi"
"if [ -n \"${{{set_var}}}\" ]; then export {key}=\"${{{value_var}}}\"; else unset {key}; fi"
)
})
.collect::<Vec<_>>()
@@ -150,6 +210,14 @@ fn build_override_exports(explicit_env_overrides: &HashMap<String, String>) -> (
(captures, restores)
}
fn join_shell_blocks(blocks: impl IntoIterator<Item = String>) -> String {
blocks
.into_iter()
.filter(|block| !block.is_empty())
.collect::<Vec<_>>()
.join("\n")
}
fn is_valid_shell_variable_name(name: &str) -> bool {
let mut chars = name.chars();
let Some(first) = chars.next() else {
@@ -1,6 +1,11 @@
use super::*;
use crate::shell::ShellType;
use crate::shell_snapshot::ShellSnapshot;
#[cfg(target_os = "macos")]
use codex_network_proxy::CODEX_PROXY_GIT_SSH_COMMAND_MARKER;
use codex_network_proxy::PROXY_ACTIVE_ENV_KEY;
#[cfg(target_os = "macos")]
use codex_network_proxy::PROXY_GIT_SSH_COMMAND_ENV_KEY;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::PathBufExt;
use core_test_support::PathExt;
@@ -325,6 +330,289 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_codex_thread_id_from_env() {
assert_eq!(String::from_utf8_lossy(&output.stdout), "nested-thread");
}
#[test]
fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
std::fs::write(
&snapshot_path,
"# Snapshot file\n\
export PIP_PROXY='http://127.0.0.1:8080'\n\
export HTTP_PROXY='http://127.0.0.1:8080'\n\
export http_proxy='http://127.0.0.1:8080'\n\
export GIT_SSH_COMMAND='ssh -o ProxyCommand=stale'\n",
)
.expect("write snapshot");
let session_shell = shell_with_snapshot(
ShellType::Bash,
"/bin/bash",
snapshot_path.abs(),
dir.path().abs(),
);
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"printf '%s\\n%s\\n%s\\n%s' \"$PIP_PROXY\" \"$HTTP_PROXY\" \"$http_proxy\" \"$GIT_SSH_COMMAND\""
.to_string(),
];
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
&dir.path().abs(),
&HashMap::new(),
&HashMap::new(),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env(PROXY_ACTIVE_ENV_KEY, "1")
.env("PIP_PROXY", "http://127.0.0.1:4321")
.env("HTTP_PROXY", "http://127.0.0.1:4321")
.env("http_proxy", "http://127.0.0.1:4321")
.env("GIT_SSH_COMMAND", "ssh -o ProxyCommand=fresh")
.output()
.expect("run rewritten command");
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(
String::from_utf8_lossy(&output.stdout),
"http://127.0.0.1:4321\n\
http://127.0.0.1:4321\n\
http://127.0.0.1:4321\n\
ssh -o ProxyCommand=stale"
);
}
#[cfg(target_os = "macos")]
#[test]
fn maybe_wrap_shell_lc_with_snapshot_refreshes_codex_proxy_git_ssh_command() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
let stale_command = format!(
"{CODEX_PROXY_GIT_SSH_COMMAND_MARKER}ssh -o ProxyCommand='nc -X 5 -x 127.0.0.1:8081 %h %p'"
);
let fresh_command = format!(
"{CODEX_PROXY_GIT_SSH_COMMAND_MARKER}ssh -o ProxyCommand='nc -X 5 -x 127.0.0.1:48081 %h %p'"
);
std::fs::write(
&snapshot_path,
format!(
"# Snapshot file\nexport {PROXY_GIT_SSH_COMMAND_ENV_KEY}='{}'\n",
shell_single_quote(&stale_command)
),
)
.expect("write snapshot");
let session_shell = shell_with_snapshot(
ShellType::Bash,
"/bin/bash",
snapshot_path.abs(),
dir.path().abs(),
);
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
format!("printf '%s' \"${PROXY_GIT_SSH_COMMAND_ENV_KEY}\""),
];
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
&dir.path().abs(),
&HashMap::new(),
&HashMap::new(),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env(PROXY_GIT_SSH_COMMAND_ENV_KEY, &fresh_command)
.output()
.expect("run rewritten command");
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(String::from_utf8_lossy(&output.stdout), fresh_command);
}
#[cfg(target_os = "macos")]
#[test]
fn maybe_wrap_shell_lc_with_snapshot_restores_custom_git_ssh_command() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
let stale_command = format!(
"{CODEX_PROXY_GIT_SSH_COMMAND_MARKER}ssh -o ProxyCommand='nc -X 5 -x 127.0.0.1:8081 %h %p'"
);
let custom_command = "ssh -o ProxyCommand='tsh proxy ssh --cluster=dev %r@%h:%p'";
std::fs::write(
&snapshot_path,
format!(
"# Snapshot file\nexport {PROXY_GIT_SSH_COMMAND_ENV_KEY}='{}'\n",
shell_single_quote(&stale_command)
),
)
.expect("write snapshot");
let session_shell = shell_with_snapshot(
ShellType::Bash,
"/bin/bash",
snapshot_path.abs(),
dir.path().abs(),
);
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
format!("printf '%s' \"${PROXY_GIT_SSH_COMMAND_ENV_KEY}\""),
];
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
&dir.path().abs(),
&HashMap::new(),
&HashMap::new(),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env(PROXY_GIT_SSH_COMMAND_ENV_KEY, custom_command)
.output()
.expect("run rewritten command");
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(String::from_utf8_lossy(&output.stdout), custom_command);
}
#[cfg(target_os = "macos")]
#[test]
fn maybe_wrap_shell_lc_with_snapshot_clears_stale_codex_git_ssh_command_without_live_command() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
let stale_command = format!(
"{CODEX_PROXY_GIT_SSH_COMMAND_MARKER}ssh -o ProxyCommand='nc -X 5 -x 127.0.0.1:8081 %h %p'"
);
std::fs::write(
&snapshot_path,
format!(
"# Snapshot file\nexport {PROXY_GIT_SSH_COMMAND_ENV_KEY}='{}'\n",
shell_single_quote(&stale_command)
),
)
.expect("write snapshot");
let session_shell = shell_with_snapshot(
ShellType::Bash,
"/bin/bash",
snapshot_path.abs(),
dir.path().abs(),
);
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
format!(
"if [ \"${{{PROXY_GIT_SSH_COMMAND_ENV_KEY}+x}}\" = x ]; then printf 'set'; else printf 'unset'; fi"
),
];
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
&dir.path().abs(),
&HashMap::new(),
&HashMap::new(),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env_remove(PROXY_GIT_SSH_COMMAND_ENV_KEY)
.output()
.expect("run rewritten command");
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(String::from_utf8_lossy(&output.stdout), "unset");
}
#[test]
fn maybe_wrap_shell_lc_with_snapshot_keeps_user_proxy_env_when_proxy_inactive() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
std::fs::write(
&snapshot_path,
"# Snapshot file\nexport HTTP_PROXY='http://user.proxy:8080'\n",
)
.expect("write snapshot");
let session_shell = shell_with_snapshot(
ShellType::Bash,
"/bin/bash",
snapshot_path.abs(),
dir.path().abs(),
);
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"printf '%s' \"$HTTP_PROXY\"".to_string(),
];
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
&dir.path().abs(),
&HashMap::new(),
&HashMap::new(),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.output()
.expect("run rewritten command");
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(
String::from_utf8_lossy(&output.stdout),
"http://user.proxy:8080"
);
}
#[test]
fn maybe_wrap_shell_lc_with_snapshot_restores_live_env_when_snapshot_proxy_active() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
std::fs::write(
&snapshot_path,
format!(
"# Snapshot file\n\
export {PROXY_ACTIVE_ENV_KEY}='1'\n\
export PIP_PROXY='http://127.0.0.1:8080'\n\
export HTTP_PROXY='http://127.0.0.1:8080'\n"
),
)
.expect("write snapshot");
let session_shell = shell_with_snapshot(
ShellType::Bash,
"/bin/bash",
snapshot_path.abs(),
dir.path().abs(),
);
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
format!(
"if [ \"${{PIP_PROXY+x}}\" = x ]; then printf 'pip:%s\\n' \"$PIP_PROXY\"; else printf 'pip:unset\\n'; fi; \
printf 'http:%s\\n' \"$HTTP_PROXY\"; \
if [ \"${{{PROXY_ACTIVE_ENV_KEY}+x}}\" = x ]; then printf 'active:%s' \"${PROXY_ACTIVE_ENV_KEY}\"; else printf 'active:unset'; fi"
),
];
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
&dir.path().abs(),
&HashMap::new(),
&HashMap::from([(
"HTTP_PROXY".to_string(),
"http://user.proxy:8080".to_string(),
)]),
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.env("HTTP_PROXY", "http://user.proxy:8080")
.env_remove("PIP_PROXY")
.env_remove(PROXY_ACTIVE_ENV_KEY)
.output()
.expect("run rewritten command");
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(
String::from_utf8_lossy(&output.stdout),
"pip:unset\nhttp:http://user.proxy:8080\nactive:unset"
);
}
#[test]
fn maybe_wrap_shell_lc_with_snapshot_keeps_snapshot_path_without_override() {
let dir = tempdir().expect("create temp dir");
@@ -258,12 +258,12 @@ fn map_exec_result_preserves_stdout_and_stderr() {
#[test]
fn shell_request_escalation_execution_is_explicit() {
let requested_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
read: None,
write: Some(vec![
file_system: Some(FileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![
AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(),
]),
}),
)),
..Default::default()
};
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
+27 -24
View File
@@ -109,7 +109,7 @@ fn discoverable_connector(id: &str, name: &str, description: &str) -> Discoverab
async fn search_capable_model_info() -> ModelInfo {
let config = test_config().await;
let mut model_info = construct_model_info_offline("gpt-5-codex", &config);
let mut model_info = construct_model_info_offline("gpt-5.4", &config);
model_info.supports_search_tool = true;
model_info
}
@@ -218,7 +218,7 @@ fn find_namespace_function_tool<'a>(
async fn multi_agent_v2_tools_config() -> ToolsConfig {
let config = test_config().await;
let model_info = construct_model_info_offline("gpt-5-codex", &config);
let model_info = construct_model_info_offline("gpt-5.4", &config);
let mut features = Features::with_defaults();
features.enable(Feature::Collab);
features.enable(Feature::MultiAgentV2);
@@ -297,7 +297,7 @@ fn build_specs_with_unavailable_tools(
#[tokio::test]
async fn model_provided_unified_exec_is_blocked_for_windows_sandboxed_policies() {
let mut model_info = model_info_from_models_json("gpt-5-codex").await;
let mut model_info = model_info_from_models_json("gpt-5.4").await;
model_info.shell_type = ConfigShellToolType::UnifiedExec;
let features = Features::with_defaults();
let available_models = Vec::new();
@@ -323,7 +323,7 @@ async fn model_provided_unified_exec_is_blocked_for_windows_sandboxed_policies()
#[tokio::test]
async fn get_memory_requires_feature_flag() {
let config = test_config().await;
let model_info = construct_model_info_offline("gpt-5-codex", &config);
let model_info = construct_model_info_offline("gpt-5.4", &config);
let mut features = Features::with_defaults();
features.disable(Feature::MemoryTool);
let available_models = Vec::new();
@@ -408,7 +408,7 @@ async fn assert_default_model_tools(
async fn test_build_specs_gpt5_codex_default() {
let features = Features::with_defaults();
assert_default_model_tools(
"gpt-5-codex",
"gpt-5.4",
&features,
Some(WebSearchMode::Cached),
"shell_command",
@@ -433,7 +433,7 @@ async fn test_build_specs_gpt5_codex_default() {
async fn test_build_specs_gpt51_codex_default() {
let features = Features::with_defaults();
assert_default_model_tools(
"gpt-5.1-codex",
"gpt-5.4",
&features,
Some(WebSearchMode::Cached),
"shell_command",
@@ -459,7 +459,7 @@ async fn test_build_specs_gpt5_codex_unified_exec_web_search() {
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
assert_model_tools(
"gpt-5-codex",
"gpt-5.4",
&features,
Some(WebSearchMode::Live),
&[
@@ -486,7 +486,7 @@ async fn test_build_specs_gpt51_codex_unified_exec_web_search() {
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
assert_model_tools(
"gpt-5.1-codex",
"gpt-5.4",
&features,
Some(WebSearchMode::Live),
&[
@@ -512,7 +512,7 @@ async fn test_build_specs_gpt51_codex_unified_exec_web_search() {
async fn test_gpt_5_1_codex_max_defaults() {
let features = Features::with_defaults();
assert_default_model_tools(
"gpt-5.1-codex-max",
"gpt-5.4",
&features,
Some(WebSearchMode::Cached),
"shell_command",
@@ -537,7 +537,7 @@ async fn test_gpt_5_1_codex_max_defaults() {
async fn test_codex_5_1_mini_defaults() {
let features = Features::with_defaults();
assert_default_model_tools(
"gpt-5.1-codex-mini",
"gpt-5.4-mini",
&features,
Some(WebSearchMode::Cached),
"shell_command",
@@ -562,13 +562,14 @@ async fn test_codex_5_1_mini_defaults() {
async fn test_gpt_5_defaults() {
let features = Features::with_defaults();
assert_default_model_tools(
"gpt-5",
"gpt-5.2",
&features,
Some(WebSearchMode::Cached),
"shell",
"shell_command",
&[
"update_plan",
"request_user_input",
"apply_patch",
"web_search",
"image_generation",
"view_image",
@@ -586,7 +587,7 @@ async fn test_gpt_5_defaults() {
async fn test_gpt_5_1_defaults() {
let features = Features::with_defaults();
assert_default_model_tools(
"gpt-5.1",
"gpt-5.4",
&features,
Some(WebSearchMode::Cached),
"shell_command",
@@ -612,7 +613,7 @@ async fn test_gpt_5_1_codex_max_unified_exec_web_search() {
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
assert_model_tools(
"gpt-5.1-codex-max",
"gpt-5.4",
&features,
Some(WebSearchMode::Live),
&[
@@ -741,10 +742,11 @@ async fn spawn_agent_description_omits_usage_hint_when_disabled() {
assert_regex_match(
r#"(?sx)
^\s*
No\ picker-visible\ models\ are\ currently\ loaded\.
No\ picker-visible\ model\ overrides\ are\ currently\ loaded\.
\s+Spawns\ an\ agent\ to\ work\ on\ the\ specified\ task\.\ If\ your\ current\ task\ is\ `/root/task1`\ and\ you\ spawn_agent\ with\ task_name\ "task_3"\ the\ agent\ will\ have\ canonical\ task\ name\ `/root/task1/task_3`\.
\s+You\ are\ then\ able\ to\ refer\ to\ this\ agent\ as\ `task_3`\ or\ `/root/task1/task_3`\ interchangeably\.\ However\ an\ agent\ `/root/task2/task_3`\ would\ only\ be\ able\ to\ communicate\ with\ this\ agent\ via\ its\ canonical\ name\ `/root/task1/task_3`\.
\s+The\ spawned\ agent\ will\ have\ the\ same\ tools\ as\ you\ and\ the\ ability\ to\ spawn\ its\ own\ subagents\.
\s+Spawned\ agents\ inherit\ your\ current\ model\ by\ default\.\ Omit\ `model`\ to\ use\ that\ preferred\ default;\ set\ `model`\ only\ when\ an\ explicit\ override\ is\ needed\.
\s+It\ will\ be\ able\ to\ send\ you\ and\ other\ running\ agents\ messages,\ and\ its\ final\ answer\ will\ be\ provided\ to\ you\ when\ it\ finishes\.
\s+The\ new\ agent's\ canonical\ task\ name\ will\ be\ provided\ to\ it\ along\ with\ the\ message\.
\s*$
@@ -765,10 +767,11 @@ async fn spawn_agent_description_uses_configured_usage_hint_text() {
assert_regex_match(
r#"(?sx)
^\s*
No\ picker-visible\ models\ are\ currently\ loaded\.
No\ picker-visible\ model\ overrides\ are\ currently\ loaded\.
\s+Spawns\ an\ agent\ to\ work\ on\ the\ specified\ task\.\ If\ your\ current\ task\ is\ `/root/task1`\ and\ you\ spawn_agent\ with\ task_name\ "task_3"\ the\ agent\ will\ have\ canonical\ task\ name\ `/root/task1/task_3`\.
\s+You\ are\ then\ able\ to\ refer\ to\ this\ agent\ as\ `task_3`\ or\ `/root/task1/task_3`\ interchangeably\.\ However\ an\ agent\ `/root/task2/task_3`\ would\ only\ be\ able\ to\ communicate\ with\ this\ agent\ via\ its\ canonical\ name\ `/root/task1/task_3`\.
\s+The\ spawned\ agent\ will\ have\ the\ same\ tools\ as\ you\ and\ the\ ability\ to\ spawn\ its\ own\ subagents\.
\s+Spawned\ agents\ inherit\ your\ current\ model\ by\ default\.\ Omit\ `model`\ to\ use\ that\ preferred\ default;\ set\ `model`\ only\ when\ an\ explicit\ override\ is\ needed\.
\s+It\ will\ be\ able\ to\ send\ you\ and\ other\ running\ agents\ messages,\ and\ its\ final\ answer\ will\ be\ provided\ to\ you\ when\ it\ finishes\.
\s+The\ new\ agent's\ canonical\ task\ name\ will\ be\ provided\ to\ it\ along\ with\ the\ message\.
\s+Custom\ delegation\ guidance\ only\.
@@ -998,7 +1001,7 @@ async fn search_tool_registers_namespaced_mcp_tool_aliases() {
#[tokio::test]
async fn direct_mcp_tools_register_namespaced_handlers() {
let config = test_config().await;
let model_info = construct_model_info_offline("gpt-5-codex", &config);
let model_info = construct_model_info_offline("gpt-5.4", &config);
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
let available_models = Vec::new();
@@ -1035,7 +1038,7 @@ async fn direct_mcp_tools_register_namespaced_handlers() {
#[tokio::test]
async fn unavailable_mcp_tools_are_exposed_as_dummy_function_tools() {
let config = test_config().await;
let model_info = construct_model_info_offline("gpt-5-codex", &config);
let model_info = construct_model_info_offline("gpt-5.4", &config);
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
let available_models = Vec::new();
@@ -1084,7 +1087,7 @@ async fn unavailable_mcp_tools_are_exposed_as_dummy_function_tools() {
#[tokio::test]
async fn test_mcp_tool_property_missing_type_defaults_to_string() {
let config = test_config().await;
let model_info = construct_model_info_offline("gpt-5-codex", &config);
let model_info = construct_model_info_offline("gpt-5.4", &config);
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
let available_models = Vec::new();
@@ -1147,7 +1150,7 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() {
#[tokio::test]
async fn test_mcp_tool_preserves_integer_schema() {
let config = test_config().await;
let model_info = construct_model_info_offline("gpt-5-codex", &config);
let model_info = construct_model_info_offline("gpt-5.4", &config);
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
let available_models = Vec::new();
@@ -1208,7 +1211,7 @@ async fn test_mcp_tool_preserves_integer_schema() {
#[tokio::test]
async fn test_mcp_tool_array_without_items_gets_default_string_items() {
let config = test_config().await;
let model_info = construct_model_info_offline("gpt-5-codex", &config);
let model_info = construct_model_info_offline("gpt-5.4", &config);
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
features.enable(Feature::ApplyPatchFreeform);
@@ -1273,7 +1276,7 @@ async fn test_mcp_tool_array_without_items_gets_default_string_items() {
#[tokio::test]
async fn test_mcp_tool_anyof_defaults_to_string() {
let config = test_config().await;
let model_info = construct_model_info_offline("gpt-5-codex", &config);
let model_info = construct_model_info_offline("gpt-5.4", &config);
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
let available_models = Vec::new();
@@ -1342,7 +1345,7 @@ async fn test_mcp_tool_anyof_defaults_to_string() {
#[tokio::test]
async fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() {
let config = test_config().await;
let model_info = construct_model_info_offline("gpt-5-codex", &config);
let model_info = construct_model_info_offline("gpt-5.4", &config);
let mut features = Features::with_defaults();
features.enable(Feature::UnifiedExec);
let available_models = Vec::new();
@@ -1461,7 +1464,7 @@ async fn code_mode_only_restricts_model_tools_to_exec_tools() {
features.enable(Feature::CodeModeOnly);
assert_model_tools(
"gpt-5.1-codex",
"gpt-5.4",
&features,
Some(WebSearchMode::Live),
&["exec", "wait"],
+11 -7
View File
@@ -1,15 +1,15 @@
use codex_mcp::ToolInfo;
use codex_protocol::dynamic_tools::DynamicToolSpec;
use codex_tools::ToolSearchOutputTool;
use codex_tools::LoadableToolSpec;
use codex_tools::ToolSearchResultSource;
use codex_tools::dynamic_tool_to_responses_api_tool;
use codex_tools::tool_search_result_source_to_output_tool;
use codex_tools::dynamic_tool_to_loadable_tool_spec;
use codex_tools::tool_search_result_source_to_loadable_tool_spec;
use std::collections::HashMap;
#[derive(Clone)]
pub(crate) struct ToolSearchEntry {
pub(crate) search_text: String,
pub(crate) output: ToolSearchOutputTool,
pub(crate) output: LoadableToolSpec,
pub(crate) limit_bucket: Option<String>,
}
@@ -36,7 +36,7 @@ pub(crate) fn build_tool_search_entries(
}
let mut dynamic_tools = dynamic_tools.iter().collect::<Vec<_>>();
dynamic_tools.sort_by(|a, b| a.name.cmp(&b.name));
dynamic_tools.sort_by(|a, b| a.namespace.cmp(&b.namespace).then(a.name.cmp(&b.name)));
for tool in dynamic_tools {
match dynamic_tool_search_entry(tool) {
Ok(entry) => entries.push(entry),
@@ -55,7 +55,7 @@ pub(crate) fn build_tool_search_entries(
fn mcp_tool_search_entry(info: &ToolInfo) -> Result<ToolSearchEntry, serde_json::Error> {
Ok(ToolSearchEntry {
search_text: build_mcp_search_text(info),
output: tool_search_result_source_to_output_tool(ToolSearchResultSource {
output: tool_search_result_source_to_loadable_tool_spec(ToolSearchResultSource {
server_name: info.server_name.as_str(),
tool_namespace: info.callable_namespace.as_str(),
tool_name: info.callable_name.as_str(),
@@ -70,7 +70,7 @@ fn mcp_tool_search_entry(info: &ToolInfo) -> Result<ToolSearchEntry, serde_json:
fn dynamic_tool_search_entry(tool: &DynamicToolSpec) -> Result<ToolSearchEntry, serde_json::Error> {
Ok(ToolSearchEntry {
search_text: build_dynamic_search_text(tool),
output: ToolSearchOutputTool::Function(dynamic_tool_to_responses_api_tool(tool)?),
output: dynamic_tool_to_loadable_tool_spec(tool)?,
limit_bucket: None,
})
}
@@ -135,6 +135,10 @@ fn build_dynamic_search_text(tool: &DynamicToolSpec) -> String {
tool.description.clone(),
];
if let Some(namespace) = &tool.namespace {
parts.push(namespace.clone());
}
parts.extend(
tool.input_schema
.get("properties")
+9 -27
View File
@@ -1,45 +1,27 @@
use std::time::Duration;
use codex_protocol::exec_output::ExecToolCallOutput;
use codex_protocol::models::ResponseItem;
use crate::contextual_user_message::USER_SHELL_COMMAND_FRAGMENT;
use crate::context::ContextualUserFragment;
use crate::context::UserShellCommand;
use crate::session::turn_context::TurnContext;
use crate::tools::format_exec_output_str;
fn format_duration_line(duration: Duration) -> String {
let duration_seconds = duration.as_secs_f64();
format!("Duration: {duration_seconds:.4} seconds")
}
fn format_user_shell_command_body(
fn user_shell_command_fragment(
command: &str,
exec_output: &ExecToolCallOutput,
turn_context: &TurnContext,
) -> String {
let mut sections = Vec::new();
sections.push("<command>".to_string());
sections.push(command.to_string());
sections.push("</command>".to_string());
sections.push("<result>".to_string());
sections.push(format!("Exit code: {}", exec_output.exit_code));
sections.push(format_duration_line(exec_output.duration));
sections.push("Output:".to_string());
sections.push(format_exec_output_str(
exec_output,
turn_context.truncation_policy,
));
sections.push("</result>".to_string());
sections.join("\n")
) -> UserShellCommand {
let output = format_exec_output_str(exec_output, turn_context.truncation_policy);
UserShellCommand::new(command, exec_output.exit_code, exec_output.duration, output)
}
#[cfg(test)]
pub fn format_user_shell_command_record(
command: &str,
exec_output: &ExecToolCallOutput,
turn_context: &TurnContext,
) -> String {
let body = format_user_shell_command_body(command, exec_output, turn_context);
USER_SHELL_COMMAND_FRAGMENT.wrap(body)
user_shell_command_fragment(command, exec_output, turn_context).render()
}
pub fn user_shell_command_record_item(
@@ -47,7 +29,7 @@ pub fn user_shell_command_record_item(
exec_output: &ExecToolCallOutput,
turn_context: &TurnContext,
) -> ResponseItem {
USER_SHELL_COMMAND_FRAGMENT.into_message(format_user_shell_command_record(
ContextualUserFragment::into(user_shell_command_fragment(
command,
exec_output,
turn_context,
@@ -1,16 +1,18 @@
use super::*;
use crate::context::ContextualUserFragment;
use crate::context::UserShellCommand;
use crate::session::tests::make_session_and_context;
use codex_protocol::exec_output::StreamOutput;
use codex_protocol::models::ContentItem;
use pretty_assertions::assert_eq;
use std::time::Duration;
#[test]
fn detects_user_shell_command_text_variants() {
assert!(
USER_SHELL_COMMAND_FRAGMENT
.matches_text("<user_shell_command>\necho hi\n</user_shell_command>")
);
assert!(!USER_SHELL_COMMAND_FRAGMENT.matches_text("echo hi"));
assert!(UserShellCommand::matches_text(
"<user_shell_command>\necho hi\n</user_shell_command>"
));
assert!(!UserShellCommand::matches_text("echo hi"));
}
#[tokio::test]
+1 -1
View File
@@ -20,7 +20,7 @@ fn feedback_tags_macro_compiles() {
#[derive(Debug)]
struct OnlyDebug;
feedback_tags!(model = "gpt-5", cached = true, debug_only = OnlyDebug);
feedback_tags!(model = "gpt-5.2", cached = true, debug_only = OnlyDebug);
}
#[derive(Default)]