mirror of
https://github.com/hanzoai/dev.git
synced 2026-10-25 03:32:32 +00:00
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:
@@ -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
@@ -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;
|
||||
|
||||
@@ -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() {
|
||||
|
||||
+713
-245
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user