fix(upstream-merge): restore fork features and adapt to upstream APIs

- Restore browser_*/agent_* tool exposure and web_fetch (openai_tools.rs)
- Reintroduce browser gating and screenshot handling in codex.rs; remove obsolete image 'detail' field; adapt RolloutRecorder::new to params API
- Keep upstream TS model changes (protocol/models.rs) to satisfy protocol-ts while preserving our semantics
- Reinstate UA helpers for MCP server (USER_AGENT_SUFFIX, get_codex_user_agent_default) in default_client.rs

Build: ./build-fast.sh clean on upstream-merge
This commit is contained in:
James Peter
2025-09-11 20:44:34 +10:00
parent dc1c5ee6b0
commit a0de41baca
4 changed files with 745 additions and 324 deletions
-6
View File
@@ -797,7 +797,6 @@ async fn execute_model_with_permissions(
}
// Tool creation functions
#[allow(dead_code)]
pub fn create_run_agent_tool() -> OpenAiTool {
let mut properties = BTreeMap::new();
@@ -863,7 +862,6 @@ pub fn create_run_agent_tool() -> OpenAiTool {
})
}
#[allow(dead_code)]
pub fn create_check_agent_status_tool() -> OpenAiTool {
let mut properties = BTreeMap::new();
@@ -886,7 +884,6 @@ pub fn create_check_agent_status_tool() -> OpenAiTool {
})
}
#[allow(dead_code)]
pub fn create_get_agent_result_tool() -> OpenAiTool {
let mut properties = BTreeMap::new();
@@ -909,7 +906,6 @@ pub fn create_get_agent_result_tool() -> OpenAiTool {
})
}
#[allow(dead_code)]
pub fn create_cancel_agent_tool() -> OpenAiTool {
let mut properties = BTreeMap::new();
@@ -944,7 +940,6 @@ pub fn create_cancel_agent_tool() -> OpenAiTool {
})
}
#[allow(dead_code)]
pub fn create_wait_for_agent_tool() -> OpenAiTool {
let mut properties = BTreeMap::new();
@@ -997,7 +992,6 @@ pub fn create_wait_for_agent_tool() -> OpenAiTool {
})
}
#[allow(dead_code)]
pub fn create_list_agents_tool() -> OpenAiTool {
let mut properties = BTreeMap::new();
+19 -16
View File
@@ -80,9 +80,9 @@ fn response_input_from_core_items(items: Vec<InputItem>) -> ResponseInputItem {
.map(|m| m.essence_str().to_owned())
.unwrap_or_else(|| "application/octet-stream".to_string());
let encoded = base64::engine::general_purpose::STANDARD.encode(bytes);
content_items.push(ContentItem::InputImage {
image_url: format!("data:{mime};base64,{encoded}"),
});
content_items.push(ContentItem::InputImage {
image_url: format!("data:{mime};base64,{encoded}"),
});
}
Err(err) => {
tracing::warn!(
@@ -378,7 +378,6 @@ use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::models::ShellToolCallParams;
use crate::openai_tools::ToolsConfig;
use crate::openai_tools::ToolsConfigParams;
use crate::openai_tools::get_openai_tools;
use crate::parse_command::parse_command;
use crate::plan_tool::handle_update_plan;
@@ -413,7 +412,6 @@ use crate::protocol::Submission;
use crate::protocol::TaskCompleteEvent;
use crate::protocol::TurnDiffEvent;
use crate::rollout::RolloutRecorder;
use codex_protocol::mcp_protocol::ConversationId;
use crate::safety::SafetyCheck;
use crate::safety::assess_command_safety;
use crate::safety::assess_safety_for_untrusted_command;
@@ -1595,7 +1593,7 @@ async fn submission_loop(
match RolloutRecorder::new(
&config,
crate::rollout::recorder::RolloutRecorderParams::new(
ConversationId(session_id),
codex_protocol::mcp_protocol::ConversationId(session_id),
user_instructions.clone(),
),
)
@@ -1674,17 +1672,18 @@ async fn submission_loop(
}
}
let default_shell = shell::default_user_shell().await;
let tools_config = ToolsConfig::new(&ToolsConfigParams {
model_family: &config.model_family,
let mut tools_config = ToolsConfig::new(
&config.model_family,
approval_policy,
sandbox_policy: sandbox_policy.clone(),
include_plan_tool: config.include_plan_tool,
include_apply_patch_tool: config.include_apply_patch_tool,
include_web_search_request: config.tools_web_search_request,
use_streamable_shell_tool: config
.use_experimental_streamable_shell_tool,
include_view_image_tool: config.include_view_image_tool,
});
sandbox_policy.clone(),
config.include_plan_tool,
config.include_apply_patch_tool,
config.tools_web_search_request,
config.use_experimental_streamable_shell_tool,
config.include_view_image_tool,
);
tools_config.web_search_allowed_domains =
config.tools_web_search_allowed_domains.clone();
sess = Some(Arc::new(Session {
client,
@@ -2160,9 +2159,13 @@ async fn run_turn(
sub_id: String,
input: Vec<ResponseItem>,
) -> CodexResult<Vec<ProcessedResponseItem>> {
// Check if browser is enabled
let browser_enabled = codex_browser::global::get_browser_manager().await.is_some();
let tools = get_openai_tools(
&sess.tools_config,
Some(sess.mcp_connection_manager.list_all_tools()),
browser_enabled,
);
let mut retries = 0;
+13 -57
View File
@@ -4,24 +4,14 @@ use std::sync::Mutex;
pub const DEFAULT_ORIGINATOR: &str = "codex_cli_rs";
/// Set this to add a suffix to the User-Agent string.
/// Optional suffix for the Codex User-Agent string.
///
/// It is not ideal that we're using a global singleton for this.
/// This is primarily designed to differentiate MCP clients from each other.
/// Because there can only be one MCP server per process, it should be safe for this to be a global static.
/// However, future users of this should use this with caution as a result.
/// In addition, we want to be confident that this value is used for ALL clients and doing that requires a
/// lot of wiring and it's easy to miss code paths by doing so.
/// See https://github.com/openai/codex/pull/3388/files for an example of what that would look like.
/// Finally, we want to make sure this is set for ALL mcp clients without needing to know a special env var
/// or having to set data that they already specified in the mcp initialize request somewhere else.
///
/// A space is automatically added between the suffix and the rest of the User-Agent string.
/// The full user agent string is returned from the mcp initialize response.
/// Parenthesis will be added by Codex. This should only specify what goes inside of the parenthesis.
/// This is primarily used by the MCP server implementation to include
/// client-provided identity in the UA. Because there is a single MCP server
/// per process, a global is acceptable here. Other callers should prefer
/// passing an explicit originator.
pub static USER_AGENT_SUFFIX: LazyLock<Mutex<Option<String>>> = LazyLock::new(|| Mutex::new(None));
/// Get the Codex User-Agent string using the provided originator or default.
pub fn get_codex_user_agent(originator: Option<&str>) -> String {
let build_version = codex_version::version();
let os_info = os_info::get();
@@ -36,50 +26,38 @@ pub fn get_codex_user_agent(originator: Option<&str>) -> String {
let suffix = USER_AGENT_SUFFIX
.lock()
.ok()
.and_then(|guard| guard.clone());
let suffix = suffix
.and_then(|guard| guard.clone())
.as_deref()
.map(str::trim)
.filter(|value| !value.is_empty())
.map_or_else(String::new, |value| format!(" ({value})"));
.filter(|s| !s.is_empty())
.map_or_else(String::new, |v| format!(" ({v})"));
let candidate = format!("{prefix}{suffix}");
sanitize_user_agent(candidate, &prefix)
}
/// Get the Codex User-Agent string using the default originator.
/// Convenience wrapper using the default originator.
pub fn get_codex_user_agent_default() -> String {
get_codex_user_agent(None)
}
/// Sanitize the user agent string.
///
/// Invalid characters are replaced with an underscore.
///
/// If the user agent fails to parse, it falls back to fallback and then to ORIGINATOR.
/// Replace invalid header characters with '_' and ensure the UA is syntactically valid.
fn sanitize_user_agent(candidate: String, fallback: &str) -> String {
if HeaderValue::from_str(candidate.as_str()).is_ok() {
return candidate;
}
let sanitized: String = candidate
.chars()
.map(|ch| if matches!(ch, ' '..='~') { ch } else { '_' })
.collect();
if !sanitized.is_empty() && HeaderValue::from_str(sanitized.as_str()).is_ok() {
tracing::warn!(
"Sanitized Codex user agent because provided suffix contained invalid header characters"
);
tracing::warn!("Sanitized Codex user agent because provided suffix contained invalid header characters");
sanitized
} else if HeaderValue::from_str(fallback).is_ok() {
tracing::warn!(
"Falling back to base Codex user agent because provided suffix could not be sanitized"
);
tracing::warn!("Falling back to base Codex user agent because provided suffix could not be sanitized");
fallback.to_string()
} else {
tracing::warn!(
"Falling back to default Codex originator because base user agent string is invalid"
);
tracing::warn!("Falling back to default Codex originator because base user agent string is invalid");
DEFAULT_ORIGINATOR.to_string()
}
}
@@ -163,28 +141,6 @@ mod tests {
assert_eq!(ua_header.to_str().unwrap(), expected_ua);
}
#[test]
fn test_invalid_suffix_is_sanitized() {
let prefix = "codex_cli_rs/0.0.0";
let suffix = "bad\rsuffix";
assert_eq!(
sanitize_user_agent(format!("{prefix} ({suffix})"), prefix),
"codex_cli_rs/0.0.0 (bad_suffix)"
);
}
#[test]
fn test_invalid_suffix_is_sanitized2() {
let prefix = "codex_cli_rs/0.0.0";
let suffix = "bad\0suffix";
assert_eq!(
sanitize_user_agent(format!("{prefix} ({suffix})"), prefix),
"codex_cli_rs/0.0.0 (bad_suffix)"
);
}
#[test]
#[cfg(target_os = "macos")]
fn test_macos() {
File diff suppressed because it is too large Load Diff