mirror of
https://github.com/openai/codex.git
synced 2026-11-13 21:30:47 +00:00
Merge remote-tracking branch 'origin/main' into etraut/live_skill_update
# Conflicts: # codex-rs/core/Cargo.toml # codex-rs/core/src/codex.rs
This commit is contained in:
@@ -45,6 +45,7 @@ codex-utils-pty = { workspace = true }
|
||||
codex-utils-readiness = { workspace = true }
|
||||
codex-utils-string = { workspace = true }
|
||||
codex-windows-sandbox = { package = "codex-windows-sandbox", path = "../windows-sandbox-rs" }
|
||||
dirs = { workspace = true }
|
||||
dunce = { workspace = true }
|
||||
encoding_rs = { workspace = true }
|
||||
env-flags = { workspace = true }
|
||||
@@ -56,7 +57,6 @@ indexmap = { workspace = true }
|
||||
indoc = { workspace = true }
|
||||
keyring = { workspace = true, features = ["crypto-rust"] }
|
||||
libc = { workspace = true }
|
||||
mcp-types = { workspace = true }
|
||||
notify = { workspace = true }
|
||||
multimap = { workspace = true }
|
||||
once_cell = { workspace = true }
|
||||
@@ -65,6 +65,12 @@ rand = { workspace = true }
|
||||
regex = { workspace = true }
|
||||
regex-lite = { workspace = true }
|
||||
reqwest = { workspace = true, features = ["json", "stream"] }
|
||||
rmcp = { workspace = true, default-features = false, features = [
|
||||
"base64",
|
||||
"macros",
|
||||
"schemars",
|
||||
"server",
|
||||
] }
|
||||
schemars = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
|
||||
@@ -1,10 +1,13 @@
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::OnceLock;
|
||||
use std::sync::RwLock;
|
||||
|
||||
use crate::api_bridge::CoreAuthProvider;
|
||||
use crate::api_bridge::auth_provider_from_auth;
|
||||
use crate::api_bridge::map_api_error;
|
||||
use crate::auth::UnauthorizedRecovery;
|
||||
use crate::turn_metadata::build_turn_metadata_header;
|
||||
use codex_api::AggregateStreamExt;
|
||||
use codex_api::ChatClient as ApiChatClient;
|
||||
use codex_api::CompactClient as ApiCompactClient;
|
||||
@@ -72,6 +75,13 @@ use crate::transport_manager::TransportManager;
|
||||
|
||||
pub const WEB_SEARCH_ELIGIBLE_HEADER: &str = "x-oai-web-search-eligible";
|
||||
pub const X_CODEX_TURN_STATE_HEADER: &str = "x-codex-turn-state";
|
||||
pub const X_CODEX_TURN_METADATA_HEADER: &str = "x-codex-turn-metadata";
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
struct TurnMetadataCache {
|
||||
cwd: Option<PathBuf>,
|
||||
header: Option<HeaderValue>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct ModelClientState {
|
||||
@@ -85,6 +95,7 @@ struct ModelClientState {
|
||||
summary: ReasoningSummaryConfig,
|
||||
session_source: SessionSource,
|
||||
transport_manager: TransportManager,
|
||||
turn_metadata_cache: Arc<RwLock<TurnMetadataCache>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -136,11 +147,13 @@ impl ModelClient {
|
||||
summary,
|
||||
session_source,
|
||||
transport_manager,
|
||||
turn_metadata_cache: Arc::new(RwLock::new(TurnMetadataCache::default())),
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn new_session(&self) -> ModelClientSession {
|
||||
pub fn new_session(&self, turn_metadata_cwd: Option<PathBuf>) -> ModelClientSession {
|
||||
self.prewarm_turn_metadata_header(turn_metadata_cwd);
|
||||
ModelClientSession {
|
||||
state: Arc::clone(&self.state),
|
||||
connection: None,
|
||||
@@ -149,6 +162,38 @@ impl ModelClient {
|
||||
turn_state: Arc::new(OnceLock::new()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Refresh turn metadata in the background and update a cached header that request
|
||||
/// builders can read without blocking.
|
||||
fn prewarm_turn_metadata_header(&self, turn_metadata_cwd: Option<PathBuf>) {
|
||||
let turn_metadata_cwd =
|
||||
turn_metadata_cwd.map(|cwd| std::fs::canonicalize(&cwd).unwrap_or(cwd));
|
||||
|
||||
if let Ok(mut cache) = self.state.turn_metadata_cache.write()
|
||||
&& cache.cwd != turn_metadata_cwd
|
||||
{
|
||||
cache.cwd = turn_metadata_cwd.clone();
|
||||
cache.header = None;
|
||||
}
|
||||
|
||||
let Some(cwd) = turn_metadata_cwd else {
|
||||
return;
|
||||
};
|
||||
let turn_metadata_cache = Arc::clone(&self.state.turn_metadata_cache);
|
||||
if let Ok(handle) = tokio::runtime::Handle::try_current() {
|
||||
let _task = handle.spawn(async move {
|
||||
let header = build_turn_metadata_header(cwd.as_path())
|
||||
.await
|
||||
.and_then(|value| HeaderValue::from_str(value.as_str()).ok());
|
||||
|
||||
if let Ok(mut cache) = turn_metadata_cache.write()
|
||||
&& cache.cwd.as_ref() == Some(&cwd)
|
||||
{
|
||||
cache.header = header;
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ModelClient {
|
||||
@@ -257,6 +302,14 @@ impl ModelClient {
|
||||
}
|
||||
|
||||
impl ModelClientSession {
|
||||
fn turn_metadata_header(&self) -> Option<HeaderValue> {
|
||||
self.state
|
||||
.turn_metadata_cache
|
||||
.try_read()
|
||||
.ok()
|
||||
.and_then(|cache| cache.header.clone())
|
||||
}
|
||||
|
||||
/// Streams a single model turn using either the Responses or Chat
|
||||
/// Completions wire API, depending on the configured provider.
|
||||
///
|
||||
@@ -332,6 +385,7 @@ impl ModelClientSession {
|
||||
prompt: &Prompt,
|
||||
compression: Compression,
|
||||
) -> ApiResponsesOptions {
|
||||
let turn_metadata_header = self.turn_metadata_header();
|
||||
let model_info = &self.state.model_info;
|
||||
|
||||
let default_reasoning_effort = model_info.default_reasoning_level;
|
||||
@@ -380,7 +434,11 @@ impl ModelClientSession {
|
||||
store_override: None,
|
||||
conversation_id: Some(conversation_id),
|
||||
session_source: Some(self.state.session_source.clone()),
|
||||
extra_headers: build_responses_headers(&self.state.config, Some(&self.turn_state)),
|
||||
extra_headers: build_responses_headers(
|
||||
&self.state.config,
|
||||
Some(&self.turn_state),
|
||||
turn_metadata_header.as_ref(),
|
||||
),
|
||||
compression,
|
||||
turn_state: Some(Arc::clone(&self.turn_state)),
|
||||
}
|
||||
@@ -713,6 +771,7 @@ fn experimental_feature_headers(config: &Config) -> ApiHeaderMap {
|
||||
fn build_responses_headers(
|
||||
config: &Config,
|
||||
turn_state: Option<&Arc<OnceLock<String>>>,
|
||||
turn_metadata_header: Option<&HeaderValue>,
|
||||
) -> ApiHeaderMap {
|
||||
let mut headers = experimental_feature_headers(config);
|
||||
headers.insert(
|
||||
@@ -731,6 +790,9 @@ fn build_responses_headers(
|
||||
{
|
||||
headers.insert(X_CODEX_TURN_STATE_HEADER, header_value);
|
||||
}
|
||||
if let Some(header_value) = turn_metadata_header {
|
||||
headers.insert(X_CODEX_TURN_METADATA_HEADER, header_value.clone());
|
||||
}
|
||||
headers
|
||||
}
|
||||
|
||||
|
||||
+96
-30
@@ -50,6 +50,7 @@ use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_protocol::items::PlanItem;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::items::UserMessageItem;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::format_allow_prefixes;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
@@ -72,14 +73,12 @@ use codex_rmcp_client::OAuthCredentialsStoreMode;
|
||||
use futures::future::BoxFuture;
|
||||
use futures::prelude::*;
|
||||
use futures::stream::FuturesOrdered;
|
||||
use mcp_types::CallToolResult;
|
||||
use mcp_types::ListResourceTemplatesRequestParams;
|
||||
use mcp_types::ListResourceTemplatesResult;
|
||||
use mcp_types::ListResourcesRequestParams;
|
||||
use mcp_types::ListResourcesResult;
|
||||
use mcp_types::ReadResourceRequestParams;
|
||||
use mcp_types::ReadResourceResult;
|
||||
use mcp_types::RequestId;
|
||||
use rmcp::model::ListResourceTemplatesResult;
|
||||
use rmcp::model::ListResourcesResult;
|
||||
use rmcp::model::PaginatedRequestParam;
|
||||
use rmcp::model::ReadResourceRequestParam;
|
||||
use rmcp::model::ReadResourceResult;
|
||||
use rmcp::model::RequestId;
|
||||
use serde_json;
|
||||
use serde_json::Value;
|
||||
use tokio::sync::Mutex;
|
||||
@@ -121,6 +120,7 @@ use crate::exec_policy::ExecPolicyUpdateError;
|
||||
use crate::feedback_tags;
|
||||
use crate::file_watcher::FileWatcher;
|
||||
use crate::file_watcher::FileWatcherEvent;
|
||||
use crate::git_info::get_git_repo_root;
|
||||
use crate::instructions::UserInstructions;
|
||||
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use crate::mcp::auth::compute_auth_statuses;
|
||||
@@ -336,9 +336,36 @@ impl Codex {
|
||||
.clone()
|
||||
.or_else(|| conversation_history.get_base_instructions().map(|s| s.text))
|
||||
.unwrap_or_else(|| model_info.get_model_instructions(config.personality));
|
||||
// Respect explicit thread-start tools; fall back to persisted tools when resuming a thread.
|
||||
|
||||
// Respect thread-start tools. When missing (resumed/forked threads), read from the db
|
||||
// first, then fall back to rollout-file tools.
|
||||
let persisted_tools = if dynamic_tools.is_empty()
|
||||
&& config.features.enabled(Feature::Sqlite)
|
||||
{
|
||||
let thread_id = match &conversation_history {
|
||||
InitialHistory::Resumed(resumed) => Some(resumed.conversation_id),
|
||||
InitialHistory::Forked(_) => conversation_history.forked_from_id(),
|
||||
InitialHistory::New => None,
|
||||
};
|
||||
match thread_id {
|
||||
Some(thread_id) => {
|
||||
let state_db_ctx = state_db::open_if_present(
|
||||
config.codex_home.as_path(),
|
||||
config.model_provider_id.as_str(),
|
||||
)
|
||||
.await;
|
||||
state_db::get_dynamic_tools(state_db_ctx.as_deref(), thread_id, "codex_spawn")
|
||||
.await
|
||||
}
|
||||
None => None,
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let dynamic_tools = if dynamic_tools.is_empty() {
|
||||
conversation_history.get_dynamic_tools().unwrap_or_default()
|
||||
persisted_tools
|
||||
.or_else(|| conversation_history.get_dynamic_tools())
|
||||
.unwrap_or_default()
|
||||
} else {
|
||||
dynamic_tools
|
||||
};
|
||||
@@ -512,7 +539,6 @@ pub(crate) struct TurnContext {
|
||||
pub(crate) truncation_policy: TruncationPolicy,
|
||||
pub(crate) dynamic_tools: Vec<DynamicToolSpec>,
|
||||
}
|
||||
|
||||
impl TurnContext {
|
||||
pub(crate) fn resolve_path(&self, path: Option<String>) -> PathBuf {
|
||||
path.as_ref()
|
||||
@@ -1113,6 +1139,11 @@ impl Session {
|
||||
state.get_total_token_usage(state.server_reasoning_included())
|
||||
}
|
||||
|
||||
async fn get_estimated_token_count(&self, turn_context: &TurnContext) -> Option<i64> {
|
||||
let state = self.state.lock().await;
|
||||
state.history.estimate_token_count(turn_context)
|
||||
}
|
||||
|
||||
pub(crate) async fn get_base_instructions(&self) -> BaseInstructions {
|
||||
let state = self.state.lock().await;
|
||||
BaseInstructions {
|
||||
@@ -1896,6 +1927,7 @@ impl Session {
|
||||
text: format!("Warning: {}", message.into()),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
|
||||
self.record_conversation_items(ctx, &[item]).await;
|
||||
@@ -2314,7 +2346,7 @@ impl Session {
|
||||
pub async fn list_resources(
|
||||
&self,
|
||||
server: &str,
|
||||
params: Option<ListResourcesRequestParams>,
|
||||
params: Option<PaginatedRequestParam>,
|
||||
) -> anyhow::Result<ListResourcesResult> {
|
||||
self.services
|
||||
.mcp_connection_manager
|
||||
@@ -2327,7 +2359,7 @@ impl Session {
|
||||
pub async fn list_resource_templates(
|
||||
&self,
|
||||
server: &str,
|
||||
params: Option<ListResourceTemplatesRequestParams>,
|
||||
params: Option<PaginatedRequestParam>,
|
||||
) -> anyhow::Result<ListResourceTemplatesResult> {
|
||||
self.services
|
||||
.mcp_connection_manager
|
||||
@@ -2340,7 +2372,7 @@ impl Session {
|
||||
pub async fn read_resource(
|
||||
&self,
|
||||
server: &str,
|
||||
params: ReadResourceRequestParams,
|
||||
params: ReadResourceRequestParam,
|
||||
) -> anyhow::Result<ReadResourceResult> {
|
||||
self.services
|
||||
.mcp_connection_manager
|
||||
@@ -2668,10 +2700,10 @@ mod handlers {
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::dynamic_tools::DynamicToolResponse;
|
||||
use codex_protocol::mcp::RequestId as ProtocolRequestId;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use codex_rmcp_client::ElicitationAction;
|
||||
use codex_rmcp_client::ElicitationResponse;
|
||||
use mcp_types::RequestId;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use tracing::info;
|
||||
@@ -2810,7 +2842,7 @@ mod handlers {
|
||||
pub async fn resolve_elicitation(
|
||||
sess: &Arc<Session>,
|
||||
server_name: String,
|
||||
request_id: RequestId,
|
||||
request_id: ProtocolRequestId,
|
||||
decision: codex_protocol::approvals::ElicitationAction,
|
||||
) {
|
||||
let action = match decision {
|
||||
@@ -2825,6 +2857,12 @@ mod handlers {
|
||||
ElicitationAction::Decline | ElicitationAction::Cancel => None,
|
||||
};
|
||||
let response = ElicitationResponse { action, content };
|
||||
let request_id = match request_id {
|
||||
ProtocolRequestId::String(value) => {
|
||||
rmcp::model::NumberOrString::String(std::sync::Arc::from(value))
|
||||
}
|
||||
ProtocolRequestId::Integer(value) => rmcp::model::NumberOrString::Number(value),
|
||||
};
|
||||
if let Err(err) = sess
|
||||
.resolve_elicitation(server_name, request_id, response)
|
||||
.await
|
||||
@@ -3405,6 +3443,7 @@ pub(crate) async fn run_turn(
|
||||
let model_info = turn_context.client.get_model_info();
|
||||
let auto_compact_limit = model_info.auto_compact_token_limit().unwrap_or(i64::MAX);
|
||||
let total_usage_tokens = sess.get_total_token_usage().await;
|
||||
|
||||
let event = EventMsg::TurnStarted(TurnStartedEvent {
|
||||
model_context_window: turn_context.client.get_model_context_window(),
|
||||
collaboration_mode_kind: turn_context.collaboration_mode.mode,
|
||||
@@ -3507,7 +3546,9 @@ pub(crate) async fn run_turn(
|
||||
// many turns, from the perspective of the user, it is a single turn.
|
||||
let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
|
||||
let mut client_session = turn_context.client.new_session();
|
||||
let mut client_session = turn_context
|
||||
.client
|
||||
.new_session(Some(turn_context.cwd.clone()));
|
||||
|
||||
loop {
|
||||
// Note that pending_input would be something like a message the user
|
||||
@@ -3558,6 +3599,19 @@ pub(crate) async fn run_turn(
|
||||
let total_usage_tokens = sess.get_total_token_usage().await;
|
||||
let token_limit_reached = total_usage_tokens >= auto_compact_limit;
|
||||
|
||||
let estimated_token_count =
|
||||
sess.get_estimated_token_count(turn_context.as_ref()).await;
|
||||
|
||||
info!(
|
||||
turn_id = %turn_context.sub_id,
|
||||
total_usage_tokens,
|
||||
estimated_token_count = ?estimated_token_count,
|
||||
auto_compact_limit,
|
||||
token_limit_reached,
|
||||
needs_follow_up,
|
||||
"post sampling token usage"
|
||||
);
|
||||
|
||||
// as long as compaction works well in getting us way below the token limit, we shouldn't worry about being in an infinite loop.
|
||||
if token_limit_reached && needs_follow_up {
|
||||
run_auto_compact(&sess, &turn_context).await;
|
||||
@@ -4570,8 +4624,6 @@ pub(super) fn get_last_assistant_message_from_turn(responses: &[ResponseItem]) -
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) use tests::make_session_and_context;
|
||||
|
||||
use crate::git_info::get_git_repo_root;
|
||||
#[cfg(test)]
|
||||
pub(crate) use tests::make_session_and_context_with_rx;
|
||||
|
||||
@@ -4621,8 +4673,7 @@ mod tests {
|
||||
use std::time::Duration;
|
||||
use tokio::time::sleep;
|
||||
|
||||
use mcp_types::ContentBlock;
|
||||
use mcp_types::TextContent;
|
||||
use codex_protocol::mcp::CallToolResult as McpCallToolResult;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde::Deserialize;
|
||||
use serde_json::json;
|
||||
@@ -4645,6 +4696,7 @@ mod tests {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4926,6 +4978,7 @@ mod tests {
|
||||
text: "turn 1 user".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -4934,6 +4987,7 @@ mod tests {
|
||||
text: "turn 1 assistant".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
];
|
||||
sess.record_into_history(&turn_1, tc.as_ref()).await;
|
||||
@@ -4946,6 +5000,7 @@ mod tests {
|
||||
text: "turn 2 user".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -4954,6 +5009,7 @@ mod tests {
|
||||
text: "turn 2 assistant".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
];
|
||||
sess.record_into_history(&turn_2, tc.as_ref()).await;
|
||||
@@ -4986,6 +5042,7 @@ mod tests {
|
||||
text: "turn 1 user".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}];
|
||||
sess.record_into_history(&turn_1, tc.as_ref()).await;
|
||||
|
||||
@@ -5208,7 +5265,7 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn prefers_structured_content_when_present() {
|
||||
let ctr = CallToolResult {
|
||||
let ctr = McpCallToolResult {
|
||||
// Content present but should be ignored because structured_content is set.
|
||||
content: vec![text_block("ignored")],
|
||||
is_error: None,
|
||||
@@ -5216,6 +5273,7 @@ mod tests {
|
||||
"ok": true,
|
||||
"value": 42
|
||||
})),
|
||||
meta: None,
|
||||
};
|
||||
|
||||
let got = FunctionCallOutputPayload::from(&ctr);
|
||||
@@ -5254,10 +5312,11 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn falls_back_to_content_when_structured_is_null() {
|
||||
let ctr = CallToolResult {
|
||||
let ctr = McpCallToolResult {
|
||||
content: vec![text_block("hello"), text_block("world")],
|
||||
is_error: None,
|
||||
structured_content: Some(serde_json::Value::Null),
|
||||
meta: None,
|
||||
};
|
||||
|
||||
let got = FunctionCallOutputPayload::from(&ctr);
|
||||
@@ -5273,10 +5332,11 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn success_flag_reflects_is_error_true() {
|
||||
let ctr = CallToolResult {
|
||||
let ctr = McpCallToolResult {
|
||||
content: vec![text_block("unused")],
|
||||
is_error: Some(true),
|
||||
structured_content: Some(json!({ "message": "bad" })),
|
||||
meta: None,
|
||||
};
|
||||
|
||||
let got = FunctionCallOutputPayload::from(&ctr);
|
||||
@@ -5291,10 +5351,11 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn success_flag_true_with_no_error_and_content_used() {
|
||||
let ctr = CallToolResult {
|
||||
let ctr = McpCallToolResult {
|
||||
content: vec![text_block("alpha")],
|
||||
is_error: Some(false),
|
||||
structured_content: None,
|
||||
meta: None,
|
||||
};
|
||||
|
||||
let got = FunctionCallOutputPayload::from(&ctr);
|
||||
@@ -5345,11 +5406,10 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn text_block(s: &str) -> ContentBlock {
|
||||
ContentBlock::TextContent(TextContent {
|
||||
annotations: None,
|
||||
text: s.to_string(),
|
||||
r#type: "text".to_string(),
|
||||
fn text_block(s: &str) -> serde_json::Value {
|
||||
json!({
|
||||
"type": "text",
|
||||
"text": s,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -5995,6 +6055,7 @@ mod tests {
|
||||
text: "first user".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
live_history.record_items(std::iter::once(&user1), turn_context.truncation_policy);
|
||||
rollout_items.push(RolloutItem::ResponseItem(user1.clone()));
|
||||
@@ -6006,6 +6067,7 @@ mod tests {
|
||||
text: "assistant reply one".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
live_history.record_items(std::iter::once(&assistant1), turn_context.truncation_policy);
|
||||
rollout_items.push(RolloutItem::ResponseItem(assistant1.clone()));
|
||||
@@ -6031,6 +6093,7 @@ mod tests {
|
||||
text: "second user".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
live_history.record_items(std::iter::once(&user2), turn_context.truncation_policy);
|
||||
rollout_items.push(RolloutItem::ResponseItem(user2.clone()));
|
||||
@@ -6042,6 +6105,7 @@ mod tests {
|
||||
text: "assistant reply two".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
live_history.record_items(std::iter::once(&assistant2), turn_context.truncation_policy);
|
||||
rollout_items.push(RolloutItem::ResponseItem(assistant2.clone()));
|
||||
@@ -6067,6 +6131,7 @@ mod tests {
|
||||
text: "third user".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
live_history.record_items(std::iter::once(&user3), turn_context.truncation_policy);
|
||||
rollout_items.push(RolloutItem::ResponseItem(user3));
|
||||
@@ -6078,6 +6143,7 @@ mod tests {
|
||||
text: "assistant reply three".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
live_history.record_items(std::iter::once(&assistant3), turn_context.truncation_policy);
|
||||
rollout_items.push(RolloutItem::ResponseItem(assistant3));
|
||||
|
||||
@@ -27,12 +27,100 @@ pub fn command_might_be_dangerous(command: &[String]) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
fn is_git_global_option_with_value(arg: &str) -> bool {
|
||||
matches!(
|
||||
arg,
|
||||
"-C" | "-c"
|
||||
| "--config-env"
|
||||
| "--exec-path"
|
||||
| "--git-dir"
|
||||
| "--namespace"
|
||||
| "--super-prefix"
|
||||
| "--work-tree"
|
||||
)
|
||||
}
|
||||
|
||||
fn is_git_global_option_with_inline_value(arg: &str) -> bool {
|
||||
matches!(
|
||||
arg,
|
||||
s if s.starts_with("--config-env=")
|
||||
|| s.starts_with("--exec-path=")
|
||||
|| s.starts_with("--git-dir=")
|
||||
|| s.starts_with("--namespace=")
|
||||
|| s.starts_with("--super-prefix=")
|
||||
|| s.starts_with("--work-tree=")
|
||||
) || ((arg.starts_with("-C") || arg.starts_with("-c")) && arg.len() > 2)
|
||||
}
|
||||
|
||||
/// Find the first matching git subcommand, skipping known global options that
|
||||
/// may appear before it (e.g., `-C`, `-c`, `--git-dir`).
|
||||
///
|
||||
/// Shared with `is_safe_command` to avoid git-global-option bypasses.
|
||||
pub(crate) fn find_git_subcommand<'a>(
|
||||
command: &'a [String],
|
||||
subcommands: &[&str],
|
||||
) -> Option<(usize, &'a str)> {
|
||||
let cmd0 = command.first().map(String::as_str)?;
|
||||
if !cmd0.ends_with("git") {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut skip_next = false;
|
||||
for (idx, arg) in command.iter().enumerate().skip(1) {
|
||||
if skip_next {
|
||||
skip_next = false;
|
||||
continue;
|
||||
}
|
||||
|
||||
let arg = arg.as_str();
|
||||
|
||||
if is_git_global_option_with_inline_value(arg) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if is_git_global_option_with_value(arg) {
|
||||
skip_next = true;
|
||||
continue;
|
||||
}
|
||||
|
||||
if arg == "--" || arg.starts_with('-') {
|
||||
continue;
|
||||
}
|
||||
|
||||
if subcommands.contains(&arg) {
|
||||
return Some((idx, arg));
|
||||
}
|
||||
|
||||
// In git, the first non-option token is the subcommand. If it isn't
|
||||
// one of the subcommands we're looking for, we must stop scanning to
|
||||
// avoid misclassifying later positional args (e.g., branch names).
|
||||
return None;
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
fn is_dangerous_to_call_with_exec(command: &[String]) -> bool {
|
||||
let cmd0 = command.first().map(String::as_str);
|
||||
|
||||
match cmd0 {
|
||||
Some(cmd) if cmd.ends_with("git") || cmd.ends_with("/git") => {
|
||||
matches!(command.get(1).map(String::as_str), Some("reset" | "rm"))
|
||||
Some(cmd) if cmd.ends_with("git") => {
|
||||
let Some((subcommand_idx, subcommand)) =
|
||||
find_git_subcommand(command, &["reset", "rm", "branch", "push", "clean"])
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
|
||||
match subcommand {
|
||||
"reset" | "rm" => true,
|
||||
"branch" => git_branch_is_delete(&command[subcommand_idx + 1..]),
|
||||
"push" => git_push_is_dangerous(&command[subcommand_idx + 1..]),
|
||||
"clean" => git_clean_is_force(&command[subcommand_idx + 1..]),
|
||||
other => {
|
||||
debug_assert!(false, "unexpected git subcommand from matcher: {other}");
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Some("rm") => matches!(command.get(1).map(String::as_str), Some("-f" | "-rf")),
|
||||
@@ -45,6 +133,48 @@ fn is_dangerous_to_call_with_exec(command: &[String]) -> bool {
|
||||
}
|
||||
}
|
||||
|
||||
fn git_branch_is_delete(branch_args: &[String]) -> bool {
|
||||
// Git allows stacking short flags (for example, `-dv` or `-vd`). Treat any
|
||||
// short-flag group containing `d`/`D` as a delete flag.
|
||||
branch_args.iter().map(String::as_str).any(|arg| {
|
||||
matches!(arg, "-d" | "-D" | "--delete")
|
||||
|| arg.starts_with("--delete=")
|
||||
|| short_flag_group_contains(arg, 'd')
|
||||
|| short_flag_group_contains(arg, 'D')
|
||||
})
|
||||
}
|
||||
|
||||
fn short_flag_group_contains(arg: &str, target: char) -> bool {
|
||||
arg.starts_with('-') && !arg.starts_with("--") && arg.chars().skip(1).any(|c| c == target)
|
||||
}
|
||||
|
||||
fn git_push_is_dangerous(push_args: &[String]) -> bool {
|
||||
push_args.iter().map(String::as_str).any(|arg| {
|
||||
matches!(
|
||||
arg,
|
||||
"--force" | "--force-with-lease" | "--force-if-includes" | "--delete" | "-f" | "-d"
|
||||
) || arg.starts_with("--force-with-lease=")
|
||||
|| arg.starts_with("--force-if-includes=")
|
||||
|| arg.starts_with("--delete=")
|
||||
|| short_flag_group_contains(arg, 'f')
|
||||
|| short_flag_group_contains(arg, 'd')
|
||||
|| git_push_refspec_is_dangerous(arg)
|
||||
})
|
||||
}
|
||||
|
||||
fn git_push_refspec_is_dangerous(arg: &str) -> bool {
|
||||
// `+<refspec>` forces updates and `:<dst>` deletes remote refs.
|
||||
(arg.starts_with('+') || arg.starts_with(':')) && arg.len() > 1
|
||||
}
|
||||
|
||||
fn git_clean_is_force(clean_args: &[String]) -> bool {
|
||||
clean_args.iter().map(String::as_str).any(|arg| {
|
||||
matches!(arg, "--force" | "-f")
|
||||
|| arg.starts_with("--force=")
|
||||
|| short_flag_group_contains(arg, 'f')
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -63,7 +193,7 @@ mod tests {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"bash",
|
||||
"-lc",
|
||||
"git reset --hard"
|
||||
"git reset --hard",
|
||||
])));
|
||||
}
|
||||
|
||||
@@ -72,7 +202,7 @@ mod tests {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"zsh",
|
||||
"-lc",
|
||||
"git reset --hard"
|
||||
"git reset --hard",
|
||||
])));
|
||||
}
|
||||
|
||||
@@ -86,14 +216,14 @@ mod tests {
|
||||
assert!(!command_might_be_dangerous(&vec_str(&[
|
||||
"bash",
|
||||
"-lc",
|
||||
"git status"
|
||||
"git status",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sudo_git_reset_is_dangerous() {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"sudo", "git", "reset", "--hard"
|
||||
"sudo", "git", "reset", "--hard",
|
||||
])));
|
||||
}
|
||||
|
||||
@@ -102,7 +232,141 @@ mod tests {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"/usr/bin/git",
|
||||
"reset",
|
||||
"--hard"
|
||||
"--hard",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_branch_delete_is_dangerous() {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "branch", "-d", "feature",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "branch", "-D", "feature",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"bash",
|
||||
"-lc",
|
||||
"git branch --delete feature",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_branch_delete_with_stacked_short_flags_is_dangerous() {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "branch", "-dv", "feature",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "branch", "-vd", "feature",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "branch", "-vD", "feature",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "branch", "-Dvv", "feature",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_branch_delete_with_global_options_is_dangerous() {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "-C", ".", "branch", "-d", "feature",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git",
|
||||
"-c",
|
||||
"color.ui=false",
|
||||
"branch",
|
||||
"-D",
|
||||
"feature",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"bash",
|
||||
"-lc",
|
||||
"git -C . branch -d feature",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_checkout_reset_is_not_dangerous() {
|
||||
// The first non-option token is "checkout", so later positional args
|
||||
// like branch names must not be treated as subcommands.
|
||||
assert!(!command_might_be_dangerous(&vec_str(&[
|
||||
"git", "checkout", "reset",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_push_force_is_dangerous() {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "push", "--force", "origin", "main",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "push", "-f", "origin", "main",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git",
|
||||
"-C",
|
||||
".",
|
||||
"push",
|
||||
"--force-with-lease",
|
||||
"origin",
|
||||
"main",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_push_plus_refspec_is_dangerous() {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "push", "origin", "+main",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git",
|
||||
"push",
|
||||
"origin",
|
||||
"+refs/heads/main:refs/heads/main",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_push_delete_flag_is_dangerous() {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "push", "--delete", "origin", "feature",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "push", "-d", "origin", "feature",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_push_delete_refspec_is_dangerous() {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "push", "origin", ":feature",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"bash",
|
||||
"-lc",
|
||||
"git push origin :feature",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_push_without_force_is_not_dangerous() {
|
||||
assert!(!command_might_be_dangerous(&vec_str(&[
|
||||
"git", "push", "origin", "main",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_clean_force_is_dangerous_even_when_f_is_not_first_flag() {
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "clean", "-fdx",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "clean", "-xdf",
|
||||
])));
|
||||
assert!(command_might_be_dangerous(&vec_str(&[
|
||||
"git", "clean", "--force",
|
||||
])));
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
use crate::bash::parse_shell_lc_plain_commands;
|
||||
// Find the first matching git subcommand, skipping known global options that
|
||||
// may appear before it (e.g., `-C`, `-c`, `--git-dir`).
|
||||
// Implemented in `is_dangerous_command` and shared here.
|
||||
use crate::command_safety::is_dangerous_command::find_git_subcommand;
|
||||
use crate::command_safety::windows_safe_commands::is_safe_command_windows;
|
||||
|
||||
pub fn is_known_safe_command(command: &[String]) -> bool {
|
||||
@@ -131,13 +135,36 @@ fn is_safe_to_call_with_exec(command: &[String]) -> bool {
|
||||
}
|
||||
|
||||
// Git
|
||||
Some("git") => matches!(
|
||||
command.get(1).map(String::as_str),
|
||||
Some("branch" | "status" | "log" | "diff" | "show")
|
||||
),
|
||||
Some("git") => {
|
||||
// Global config overrides like `-c core.pager=...` can force git
|
||||
// to execute arbitrary external commands. With no sandboxing, we
|
||||
// should always prompt in those cases.
|
||||
if git_has_config_override_global_option(command) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Rust
|
||||
Some("cargo") if command.get(1).map(String::as_str) == Some("check") => true,
|
||||
let Some((subcommand_idx, subcommand)) =
|
||||
find_git_subcommand(command, &["status", "log", "diff", "show", "branch"])
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
|
||||
let subcommand_args = &command[subcommand_idx + 1..];
|
||||
|
||||
match subcommand {
|
||||
"status" | "log" | "diff" | "show" => {
|
||||
git_subcommand_args_are_read_only(subcommand_args)
|
||||
}
|
||||
"branch" => {
|
||||
git_subcommand_args_are_read_only(subcommand_args)
|
||||
&& git_branch_is_read_only(subcommand_args)
|
||||
}
|
||||
other => {
|
||||
debug_assert!(false, "unexpected git subcommand from matcher: {other}");
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Special-case `sed -n {N|M,N}p`
|
||||
Some("sed")
|
||||
@@ -155,6 +182,60 @@ fn is_safe_to_call_with_exec(command: &[String]) -> bool {
|
||||
}
|
||||
}
|
||||
|
||||
// Treat `git branch` as safe only when the arguments clearly indicate
|
||||
// a read-only query, not a branch mutation (create/rename/delete).
|
||||
fn git_branch_is_read_only(branch_args: &[String]) -> bool {
|
||||
if branch_args.is_empty() {
|
||||
// `git branch` with no additional args lists branches.
|
||||
return true;
|
||||
}
|
||||
|
||||
let mut saw_read_only_flag = false;
|
||||
for arg in branch_args.iter().map(String::as_str) {
|
||||
match arg {
|
||||
"--list" | "-l" | "--show-current" | "-a" | "--all" | "-r" | "--remotes" | "-v"
|
||||
| "-vv" | "--verbose" => {
|
||||
saw_read_only_flag = true;
|
||||
}
|
||||
_ if arg.starts_with("--format=") => {
|
||||
saw_read_only_flag = true;
|
||||
}
|
||||
_ => {
|
||||
// Any other flag or positional argument may create, rename, or delete branches.
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
saw_read_only_flag
|
||||
}
|
||||
|
||||
fn git_has_config_override_global_option(command: &[String]) -> bool {
|
||||
command.iter().map(String::as_str).any(|arg| {
|
||||
matches!(arg, "-c" | "--config-env")
|
||||
|| (arg.starts_with("-c") && arg.len() > 2)
|
||||
|| arg.starts_with("--config-env=")
|
||||
})
|
||||
}
|
||||
|
||||
fn git_subcommand_args_are_read_only(args: &[String]) -> bool {
|
||||
// Flags that can write to disk or execute external tools should never be
|
||||
// auto-approved on an unsandboxed machine.
|
||||
const UNSAFE_GIT_FLAGS: &[&str] = &[
|
||||
"--output",
|
||||
"--ext-diff",
|
||||
"--textconv",
|
||||
"--exec",
|
||||
"--paginate",
|
||||
];
|
||||
|
||||
!args.iter().map(String::as_str).any(|arg| {
|
||||
UNSAFE_GIT_FLAGS.contains(&arg)
|
||||
|| arg.starts_with("--output=")
|
||||
|| arg.starts_with("--exec=")
|
||||
})
|
||||
}
|
||||
|
||||
// (bash parsing helpers implemented in crate::bash)
|
||||
|
||||
/* ----------------------------------------------------------
|
||||
@@ -207,6 +288,12 @@ mod tests {
|
||||
fn known_safe_examples() {
|
||||
assert!(is_safe_to_call_with_exec(&vec_str(&["ls"])));
|
||||
assert!(is_safe_to_call_with_exec(&vec_str(&["git", "status"])));
|
||||
assert!(is_safe_to_call_with_exec(&vec_str(&["git", "branch"])));
|
||||
assert!(is_safe_to_call_with_exec(&vec_str(&[
|
||||
"git",
|
||||
"branch",
|
||||
"--show-current"
|
||||
])));
|
||||
assert!(is_safe_to_call_with_exec(&vec_str(&["base64"])));
|
||||
assert!(is_safe_to_call_with_exec(&vec_str(&[
|
||||
"sed", "-n", "1,5p", "file.txt"
|
||||
@@ -231,6 +318,86 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_branch_mutating_flags_are_not_safe() {
|
||||
assert!(!is_known_safe_command(&vec_str(&[
|
||||
"git", "branch", "-d", "feature"
|
||||
])));
|
||||
assert!(!is_known_safe_command(&vec_str(&[
|
||||
"git",
|
||||
"branch",
|
||||
"new-branch"
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_branch_global_options_respect_safety_rules() {
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
assert_eq!(
|
||||
is_known_safe_command(&vec_str(&["git", "-C", ".", "branch", "--show-current"])),
|
||||
true
|
||||
);
|
||||
assert_eq!(
|
||||
is_known_safe_command(&vec_str(&["git", "-C", ".", "branch", "-d", "feature"])),
|
||||
false
|
||||
);
|
||||
assert_eq!(
|
||||
is_known_safe_command(&vec_str(&["bash", "-lc", "git -C . branch -d feature",])),
|
||||
false
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_first_positional_is_the_subcommand() {
|
||||
// In git, the first non-option token is the subcommand. Later positional
|
||||
// args (like branch names) must not be treated as subcommands.
|
||||
assert!(!is_known_safe_command(&vec_str(&[
|
||||
"git", "checkout", "status",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_output_and_config_override_flags_are_not_safe() {
|
||||
assert!(!is_known_safe_command(&vec_str(&[
|
||||
"git",
|
||||
"log",
|
||||
"--output=/tmp/git-log-out-test",
|
||||
"-n",
|
||||
"1",
|
||||
])));
|
||||
assert!(!is_known_safe_command(&vec_str(&[
|
||||
"git",
|
||||
"diff",
|
||||
"--output",
|
||||
"/tmp/git-diff-out-test",
|
||||
])));
|
||||
assert!(!is_known_safe_command(&vec_str(&[
|
||||
"git",
|
||||
"show",
|
||||
"--output=/tmp/git-show-out-test",
|
||||
"HEAD",
|
||||
])));
|
||||
assert!(!is_known_safe_command(&vec_str(&[
|
||||
"git",
|
||||
"-c",
|
||||
"core.pager=cat",
|
||||
"log",
|
||||
"-n",
|
||||
"1",
|
||||
])));
|
||||
assert!(!is_known_safe_command(&vec_str(&[
|
||||
"git",
|
||||
"-ccore.pager=cat",
|
||||
"status",
|
||||
])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cargo_check_is_not_safe() {
|
||||
assert!(!is_known_safe_command(&vec_str(&["cargo", "check"])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn zsh_lc_safe_command_sequence() {
|
||||
assert!(is_known_safe_command(&vec_str(&["zsh", "-lc", "ls"])));
|
||||
|
||||
@@ -311,6 +311,7 @@ fn build_compacted_history_with_limit(
|
||||
text: message.clone(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -325,6 +326,7 @@ fn build_compacted_history_with_limit(
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText { text: summary_text }],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
});
|
||||
|
||||
history
|
||||
@@ -335,7 +337,9 @@ async fn drain_to_completed(
|
||||
turn_context: &TurnContext,
|
||||
prompt: &Prompt,
|
||||
) -> CodexResult<()> {
|
||||
let mut client_session = turn_context.client.new_session();
|
||||
let mut client_session = turn_context
|
||||
.client
|
||||
.new_session(Some(turn_context.cwd.clone()));
|
||||
let mut stream = client_session.stream(prompt).await?;
|
||||
loop {
|
||||
let maybe_event = stream.next().await;
|
||||
@@ -414,6 +418,7 @@ mod tests {
|
||||
text: "ignored".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: Some("user".to_string()),
|
||||
@@ -422,6 +427,7 @@ mod tests {
|
||||
text: "first".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Other,
|
||||
];
|
||||
@@ -442,6 +448,7 @@ mod tests {
|
||||
.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -450,6 +457,7 @@ mod tests {
|
||||
text: "<ENVIRONMENT_CONTEXT>cwd=/tmp</ENVIRONMENT_CONTEXT>".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -458,6 +466,7 @@ mod tests {
|
||||
text: "real user message".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
];
|
||||
|
||||
@@ -543,6 +552,7 @@ mod tests {
|
||||
text: marker.clone(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -551,6 +561,7 @@ mod tests {
|
||||
text: "real user message".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
];
|
||||
|
||||
|
||||
@@ -3,6 +3,8 @@ use std::sync::Arc;
|
||||
use crate::Prompt;
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::context_manager::ContextManager;
|
||||
use crate::context_manager::is_codex_generated_item;
|
||||
use crate::error::Result as CodexResult;
|
||||
use crate::protocol::CompactedItem;
|
||||
use crate::protocol::EventMsg;
|
||||
@@ -11,6 +13,7 @@ use crate::protocol::TurnStartedEvent;
|
||||
use codex_protocol::items::ContextCompactionItem;
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use tracing::info;
|
||||
|
||||
pub(crate) async fn run_inline_remote_auto_compact_task(
|
||||
sess: Arc<Session>,
|
||||
@@ -45,7 +48,16 @@ async fn run_remote_compact_task_inner_impl(
|
||||
let compaction_item = TurnItem::ContextCompaction(ContextCompactionItem::new());
|
||||
sess.emit_turn_item_started(turn_context, &compaction_item)
|
||||
.await;
|
||||
let history = sess.clone_history().await;
|
||||
let mut history = sess.clone_history().await;
|
||||
let deleted_items =
|
||||
trim_function_call_history_to_fit_context_window(&mut history, turn_context.as_ref());
|
||||
if deleted_items > 0 {
|
||||
info!(
|
||||
turn_id = %turn_context.sub_id,
|
||||
deleted_items,
|
||||
"trimmed history items before remote compaction"
|
||||
);
|
||||
}
|
||||
|
||||
// Required to keep `/undo` available after compaction
|
||||
let ghost_snapshots: Vec<ResponseItem> = history
|
||||
@@ -86,3 +98,31 @@ async fn run_remote_compact_task_inner_impl(
|
||||
.await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn trim_function_call_history_to_fit_context_window(
|
||||
history: &mut ContextManager,
|
||||
turn_context: &TurnContext,
|
||||
) -> usize {
|
||||
let mut deleted_items = 0usize;
|
||||
let Some(context_window) = turn_context.client.get_model_context_window() else {
|
||||
return deleted_items;
|
||||
};
|
||||
|
||||
while history
|
||||
.estimate_token_count(turn_context)
|
||||
.is_some_and(|estimated_tokens| estimated_tokens > context_window)
|
||||
{
|
||||
let Some(last_item) = history.raw_items().last() else {
|
||||
break;
|
||||
};
|
||||
if !is_codex_generated_item(last_item) {
|
||||
break;
|
||||
}
|
||||
if !history.remove_last_item() {
|
||||
break;
|
||||
}
|
||||
deleted_items += 1;
|
||||
}
|
||||
|
||||
deleted_items
|
||||
}
|
||||
|
||||
@@ -93,24 +93,7 @@ impl ContextManager {
|
||||
let base_tokens = i64::try_from(approx_token_count(&base_instructions)).unwrap_or(i64::MAX);
|
||||
|
||||
let items_tokens = self.items.iter().fold(0i64, |acc, item| {
|
||||
acc + match item {
|
||||
ResponseItem::GhostSnapshot { .. } => 0,
|
||||
ResponseItem::Reasoning {
|
||||
encrypted_content: Some(content),
|
||||
..
|
||||
}
|
||||
| ResponseItem::Compaction {
|
||||
encrypted_content: content,
|
||||
} => {
|
||||
let reasoning_bytes = estimate_reasoning_length(content.len());
|
||||
i64::try_from(approx_tokens_from_byte_count(reasoning_bytes))
|
||||
.unwrap_or(i64::MAX)
|
||||
}
|
||||
item => {
|
||||
let serialized = serde_json::to_string(item).unwrap_or_default();
|
||||
i64::try_from(approx_token_count(&serialized)).unwrap_or(i64::MAX)
|
||||
}
|
||||
}
|
||||
acc.saturating_add(estimate_item_token_count(item))
|
||||
});
|
||||
|
||||
Some(base_tokens.saturating_add(items_tokens))
|
||||
@@ -128,6 +111,15 @@ impl ContextManager {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn remove_last_item(&mut self) -> bool {
|
||||
if let Some(removed) = self.items.pop() {
|
||||
normalize::remove_corresponding_for(&mut self.items, &removed);
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn replace(&mut self, items: Vec<ResponseItem>) {
|
||||
self.items = items;
|
||||
}
|
||||
@@ -207,36 +199,42 @@ impl ContextManager {
|
||||
);
|
||||
}
|
||||
|
||||
fn get_non_last_reasoning_items_tokens(&self) -> usize {
|
||||
// get reasoning items excluding all the ones after the last user message
|
||||
fn get_non_last_reasoning_items_tokens(&self) -> i64 {
|
||||
// Get reasoning items excluding all the ones after the last user message.
|
||||
let Some(last_user_index) = self
|
||||
.items
|
||||
.iter()
|
||||
.rposition(|item| matches!(item, ResponseItem::Message { role, .. } if role == "user"))
|
||||
else {
|
||||
return 0usize;
|
||||
return 0;
|
||||
};
|
||||
|
||||
let total_reasoning_bytes = self
|
||||
.items
|
||||
self.items
|
||||
.iter()
|
||||
.take(last_user_index)
|
||||
.filter_map(|item| {
|
||||
if let ResponseItem::Reasoning {
|
||||
encrypted_content: Some(content),
|
||||
..
|
||||
} = item
|
||||
{
|
||||
Some(content.len())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
.filter(|item| {
|
||||
matches!(
|
||||
item,
|
||||
ResponseItem::Reasoning {
|
||||
encrypted_content: Some(_),
|
||||
..
|
||||
}
|
||||
)
|
||||
})
|
||||
.map(estimate_reasoning_length)
|
||||
.fold(0usize, usize::saturating_add);
|
||||
.fold(0i64, |acc, item| {
|
||||
acc.saturating_add(estimate_item_token_count(item))
|
||||
})
|
||||
}
|
||||
|
||||
let token_estimate = approx_tokens_from_byte_count(total_reasoning_bytes);
|
||||
token_estimate as usize
|
||||
fn get_trailing_codex_generated_items_tokens(&self) -> i64 {
|
||||
let mut total = 0i64;
|
||||
for item in self.items.iter().rev() {
|
||||
if !is_codex_generated_item(item) {
|
||||
break;
|
||||
}
|
||||
total = total.saturating_add(estimate_item_token_count(item));
|
||||
}
|
||||
total
|
||||
}
|
||||
|
||||
/// When true, the server already accounted for past reasoning tokens and
|
||||
@@ -247,10 +245,13 @@ impl ContextManager {
|
||||
.as_ref()
|
||||
.map(|info| info.last_token_usage.total_tokens)
|
||||
.unwrap_or(0);
|
||||
let trailing_codex_generated_tokens = self.get_trailing_codex_generated_items_tokens();
|
||||
if server_reasoning_included {
|
||||
last_tokens
|
||||
last_tokens.saturating_add(trailing_codex_generated_tokens)
|
||||
} else {
|
||||
last_tokens.saturating_add(self.get_non_last_reasoning_items_tokens() as i64)
|
||||
last_tokens
|
||||
.saturating_add(self.get_non_last_reasoning_items_tokens())
|
||||
.saturating_add(trailing_codex_generated_tokens)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -332,6 +333,33 @@ fn estimate_reasoning_length(encoded_len: usize) -> usize {
|
||||
.saturating_sub(650)
|
||||
}
|
||||
|
||||
fn estimate_item_token_count(item: &ResponseItem) -> i64 {
|
||||
match item {
|
||||
ResponseItem::GhostSnapshot { .. } => 0,
|
||||
ResponseItem::Reasoning {
|
||||
encrypted_content: Some(content),
|
||||
..
|
||||
}
|
||||
| ResponseItem::Compaction {
|
||||
encrypted_content: content,
|
||||
} => {
|
||||
let reasoning_bytes = estimate_reasoning_length(content.len());
|
||||
i64::try_from(approx_tokens_from_byte_count(reasoning_bytes)).unwrap_or(i64::MAX)
|
||||
}
|
||||
item => {
|
||||
let serialized = serde_json::to_string(item).unwrap_or_default();
|
||||
i64::try_from(approx_token_count(&serialized)).unwrap_or(i64::MAX)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn is_codex_generated_item(item: &ResponseItem) -> bool {
|
||||
matches!(
|
||||
item,
|
||||
ResponseItem::FunctionCallOutput { .. } | ResponseItem::CustomToolCallOutput { .. }
|
||||
) || matches!(item, ResponseItem::Message { role, .. } if role == "developer")
|
||||
}
|
||||
|
||||
pub(crate) fn is_user_turn_boundary(item: &ResponseItem) -> bool {
|
||||
let ResponseItem::Message { role, content, .. } = item else {
|
||||
return false;
|
||||
|
||||
@@ -24,6 +24,7 @@ fn assistant_msg(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -43,6 +44,7 @@ fn user_msg(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -54,6 +56,24 @@ fn user_input_text_msg(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
fn function_call_output(call_id: &str, content: &str) -> ResponseItem {
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: call_id.to_string(),
|
||||
output: FunctionCallOutputPayload {
|
||||
content: content.to_string(),
|
||||
..Default::default()
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
fn custom_tool_call_output(call_id: &str, output: &str) -> ResponseItem {
|
||||
ResponseItem::CustomToolCallOutput {
|
||||
call_id: call_id.to_string(),
|
||||
output: output.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -97,6 +117,7 @@ fn filters_non_api_messages() {
|
||||
text: "ignored".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
let reasoning = reasoning_msg("thinking...");
|
||||
h.record_items([&system, &reasoning, &ResponseItem::Other], policy);
|
||||
@@ -127,6 +148,7 @@ fn filters_non_api_messages() {
|
||||
text: "hi".to_string()
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -135,6 +157,7 @@ fn filters_non_api_messages() {
|
||||
text: "hello".to_string()
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
]
|
||||
);
|
||||
@@ -162,6 +185,63 @@ fn non_last_reasoning_tokens_ignore_entries_after_last_user() {
|
||||
assert_eq!(history.get_non_last_reasoning_items_tokens(), 32);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trailing_codex_generated_tokens_stop_at_first_non_generated_item() {
|
||||
let earlier_output = function_call_output("call-earlier", "earlier output");
|
||||
let trailing_function_output = function_call_output("call-tail-1", "tail function output");
|
||||
let trailing_custom_output = custom_tool_call_output("call-tail-2", "tail custom output");
|
||||
let history = create_history_with_items(vec![
|
||||
earlier_output,
|
||||
user_msg("boundary item"),
|
||||
trailing_function_output.clone(),
|
||||
trailing_custom_output.clone(),
|
||||
]);
|
||||
let expected_tokens = estimate_item_token_count(&trailing_function_output)
|
||||
.saturating_add(estimate_item_token_count(&trailing_custom_output));
|
||||
|
||||
assert_eq!(
|
||||
history.get_trailing_codex_generated_items_tokens(),
|
||||
expected_tokens
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trailing_codex_generated_tokens_exclude_function_call_tail() {
|
||||
let history = create_history_with_items(vec![ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "not-generated".to_string(),
|
||||
arguments: "{}".to_string(),
|
||||
call_id: "call-tail".to_string(),
|
||||
}]);
|
||||
|
||||
assert_eq!(history.get_trailing_codex_generated_items_tokens(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn total_token_usage_includes_only_trailing_codex_generated_items() {
|
||||
let non_trailing_output = function_call_output("call-before-message", "not trailing");
|
||||
let trailing_assistant = assistant_msg("assistant boundary");
|
||||
let trailing_output = custom_tool_call_output("tool-tail", "trailing output");
|
||||
let mut history = create_history_with_items(vec![
|
||||
non_trailing_output,
|
||||
user_msg("boundary"),
|
||||
trailing_assistant,
|
||||
trailing_output.clone(),
|
||||
]);
|
||||
history.update_token_info(
|
||||
&TokenUsage {
|
||||
total_tokens: 100,
|
||||
..Default::default()
|
||||
},
|
||||
None,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
history.get_total_token_usage(true),
|
||||
100 + estimate_item_token_count(&trailing_output)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn get_history_for_prompt_drops_ghost_commits() {
|
||||
let items = vec![ResponseItem::GhostSnapshot {
|
||||
@@ -216,6 +296,30 @@ fn remove_first_item_removes_matching_call_for_output() {
|
||||
assert_eq!(h.raw_items(), vec![]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_last_item_removes_matching_call_for_output() {
|
||||
let items = vec![
|
||||
user_msg("before tool call"),
|
||||
ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "do_it".to_string(),
|
||||
arguments: "{}".to_string(),
|
||||
call_id: "call-delete-last".to_string(),
|
||||
},
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: "call-delete-last".to_string(),
|
||||
output: FunctionCallOutputPayload {
|
||||
content: "ok".to_string(),
|
||||
..Default::default()
|
||||
},
|
||||
},
|
||||
];
|
||||
let mut h = create_history_with_items(items);
|
||||
|
||||
assert!(h.remove_last_item());
|
||||
assert_eq!(h.raw_items(), vec![user_msg("before tool call")]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replace_last_turn_images_replaces_tool_output_images() {
|
||||
let items = vec![
|
||||
@@ -262,6 +366,7 @@ fn replace_last_turn_images_does_not_touch_user_images() {
|
||||
image_url: "data:image/png;base64,AAA".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}];
|
||||
let mut history = create_history_with_items(items.clone());
|
||||
|
||||
|
||||
@@ -2,4 +2,5 @@ mod history;
|
||||
mod normalize;
|
||||
|
||||
pub(crate) use history::ContextManager;
|
||||
pub(crate) use history::is_codex_generated_item;
|
||||
pub(crate) use history::is_user_turn_boundary;
|
||||
|
||||
@@ -28,6 +28,7 @@ impl EnvironmentContext {
|
||||
cwd,
|
||||
// should compare all fields except shell
|
||||
shell: _,
|
||||
..
|
||||
} = other;
|
||||
|
||||
self.cwd == *cwd
|
||||
@@ -80,6 +81,7 @@ impl From<EnvironmentContext> for ResponseItem {
|
||||
text: ec.serialize_to_xml(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -177,6 +177,7 @@ mod tests {
|
||||
},
|
||||
],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
|
||||
let turn_item = parse_turn_item(&item).expect("expected user message turn item");
|
||||
@@ -219,6 +220,7 @@ mod tests {
|
||||
},
|
||||
],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
|
||||
let turn_item = parse_turn_item(&item).expect("expected user message turn item");
|
||||
@@ -260,6 +262,7 @@ mod tests {
|
||||
},
|
||||
],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
|
||||
let turn_item = parse_turn_item(&item).expect("expected user message turn item");
|
||||
@@ -289,6 +292,7 @@ mod tests {
|
||||
text: "<user_instructions>test_text</user_instructions>".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -297,6 +301,7 @@ mod tests {
|
||||
text: "<environment_context>test_text</environment_context>".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -305,6 +310,7 @@ mod tests {
|
||||
text: "# AGENTS.md instructions for test_directory\n\n<INSTRUCTIONS>\ntest_text\n</INSTRUCTIONS>".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -314,6 +320,7 @@ mod tests {
|
||||
.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
@@ -322,6 +329,7 @@ mod tests {
|
||||
text: "<user_shell_command>echo 42</user_shell_command>".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
];
|
||||
|
||||
@@ -340,6 +348,7 @@ mod tests {
|
||||
text: "Hello from Codex".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
|
||||
let turn_item = parse_turn_item(&item).expect("expected agent message turn item");
|
||||
|
||||
@@ -1280,6 +1280,30 @@ prefix_rule(
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn dangerous_git_push_requires_approval_in_danger_full_access() {
|
||||
let command = vec_str(&["git", "push", "origin", "+main"]);
|
||||
let manager = ExecPolicyManager::default();
|
||||
let requirement = manager
|
||||
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
|
||||
features: &Features::with_defaults(),
|
||||
command: &command,
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
sandbox_policy: &SandboxPolicy::DangerFullAccess,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
prefix_rule: None,
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
fn vec_str(items: &[&str]) -> Vec<String> {
|
||||
items.iter().map(std::string::ToString::to_string).collect()
|
||||
}
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
@@ -109,6 +110,73 @@ pub async fn collect_git_info(cwd: &Path) -> Option<GitInfo> {
|
||||
Some(git_info)
|
||||
}
|
||||
|
||||
/// Collect fetch remotes in a multi-root-friendly format: {"origin": "https://..."}.
|
||||
pub async fn get_git_remote_urls(cwd: &Path) -> Option<BTreeMap<String, String>> {
|
||||
let is_git_repo = run_git_command_with_timeout(&["rev-parse", "--git-dir"], cwd)
|
||||
.await?
|
||||
.status
|
||||
.success();
|
||||
if !is_git_repo {
|
||||
return None;
|
||||
}
|
||||
|
||||
get_git_remote_urls_assume_git_repo(cwd).await
|
||||
}
|
||||
|
||||
/// Collect fetch remotes without checking whether `cwd` is in a git repo.
|
||||
pub async fn get_git_remote_urls_assume_git_repo(cwd: &Path) -> Option<BTreeMap<String, String>> {
|
||||
let output = run_git_command_with_timeout(&["remote", "-v"], cwd).await?;
|
||||
if !output.status.success() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let stdout = String::from_utf8(output.stdout).ok()?;
|
||||
parse_git_remote_urls(stdout.as_str())
|
||||
}
|
||||
|
||||
/// Return the current HEAD commit hash without checking whether `cwd` is in a git repo.
|
||||
pub async fn get_head_commit_hash(cwd: &Path) -> Option<String> {
|
||||
let output = run_git_command_with_timeout(&["rev-parse", "HEAD"], cwd).await?;
|
||||
if !output.status.success() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let stdout = String::from_utf8(output.stdout).ok()?;
|
||||
let hash = stdout.trim();
|
||||
if hash.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(hash.to_string())
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_git_remote_urls(stdout: &str) -> Option<BTreeMap<String, String>> {
|
||||
let mut remotes = BTreeMap::new();
|
||||
for line in stdout.lines() {
|
||||
let Some(fetch_line) = line.strip_suffix(" (fetch)") else {
|
||||
continue;
|
||||
};
|
||||
|
||||
let Some((name, url_part)) = fetch_line
|
||||
.split_once('\t')
|
||||
.or_else(|| fetch_line.split_once(' '))
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
|
||||
let url = url_part.trim_start();
|
||||
if !url.is_empty() {
|
||||
remotes.insert(name.to_string(), url.to_string());
|
||||
}
|
||||
}
|
||||
|
||||
if remotes.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(remotes)
|
||||
}
|
||||
}
|
||||
|
||||
/// A minimal commit summary entry used for pickers (subject + timestamp + sha).
|
||||
#[derive(Clone, Debug, Serialize, Deserialize)]
|
||||
pub struct CommitLogEntry {
|
||||
@@ -185,11 +253,9 @@ pub async fn git_diff_to_remote(cwd: &Path) -> Option<GitDiffToRemote> {
|
||||
|
||||
/// Run a git command with a timeout to prevent blocking on large repositories
|
||||
async fn run_git_command_with_timeout(args: &[&str], cwd: &Path) -> Option<std::process::Output> {
|
||||
let result = timeout(
|
||||
GIT_COMMAND_TIMEOUT,
|
||||
Command::new("git").args(args).current_dir(cwd).output(),
|
||||
)
|
||||
.await;
|
||||
let mut command = Command::new("git");
|
||||
command.args(args).current_dir(cwd).kill_on_drop(true);
|
||||
let result = timeout(GIT_COMMAND_TIMEOUT, command.output()).await;
|
||||
|
||||
match result {
|
||||
Ok(Ok(output)) => Some(output),
|
||||
|
||||
@@ -39,6 +39,7 @@ impl From<UserInstructions> for ResponseItem {
|
||||
),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -73,6 +74,7 @@ impl From<SkillInstructions> for ResponseItem {
|
||||
),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -102,6 +102,7 @@ pub mod state_db;
|
||||
pub mod terminal;
|
||||
mod tools;
|
||||
pub mod turn_diff_tracker;
|
||||
mod turn_metadata;
|
||||
pub use rollout::ARCHIVED_SESSIONS_SUBDIR;
|
||||
pub use rollout::INTERACTIVE_SESSION_SOURCES;
|
||||
pub use rollout::RolloutRecorder;
|
||||
@@ -132,6 +133,7 @@ pub mod util;
|
||||
|
||||
pub use apply_patch::CODEX_APPLY_PATCH_ARG1;
|
||||
pub use client::WEB_SEARCH_ELIGIBLE_HEADER;
|
||||
pub use client::X_CODEX_TURN_METADATA_HEADER;
|
||||
pub use command_safety::is_dangerous_command;
|
||||
pub use command_safety::is_safe_command;
|
||||
pub use exec_policy::ExecPolicyError;
|
||||
|
||||
+103
-19
@@ -1,6 +1,5 @@
|
||||
pub mod auth;
|
||||
mod skill_dependencies;
|
||||
|
||||
pub(crate) use skill_dependencies::maybe_prompt_and_install_mcp_dependencies;
|
||||
|
||||
use std::collections::HashMap;
|
||||
@@ -9,9 +8,12 @@ use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
use async_channel::unbounded;
|
||||
use codex_protocol::mcp::Resource;
|
||||
use codex_protocol::mcp::ResourceTemplate;
|
||||
use codex_protocol::mcp::Tool;
|
||||
use codex_protocol::protocol::McpListToolsResponseEvent;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use mcp_types::Tool as McpTool;
|
||||
use serde_json::Value;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
use crate::AuthManager;
|
||||
@@ -201,8 +203,8 @@ pub fn split_qualified_tool_name(qualified_name: &str) -> Option<(String, String
|
||||
}
|
||||
|
||||
pub fn group_tools_by_server(
|
||||
tools: &HashMap<String, McpTool>,
|
||||
) -> HashMap<String, HashMap<String, McpTool>> {
|
||||
tools: &HashMap<String, Tool>,
|
||||
) -> HashMap<String, HashMap<String, Tool>> {
|
||||
let mut grouped = HashMap::new();
|
||||
for (qualified_name, tool) in tools {
|
||||
if let Some((server_name, tool_name)) = split_qualified_tool_name(qualified_name) {
|
||||
@@ -230,11 +232,96 @@ pub(crate) async fn collect_mcp_snapshot_from_manager(
|
||||
.map(|(name, entry)| (name.clone(), entry.auth_status))
|
||||
.collect();
|
||||
|
||||
let tools = tools
|
||||
.into_iter()
|
||||
.filter_map(|(name, tool)| match serde_json::to_value(tool.tool) {
|
||||
Ok(value) => match Tool::from_mcp_value(value) {
|
||||
Ok(tool) => Some((name, tool)),
|
||||
Err(err) => {
|
||||
tracing::warn!("Failed to convert MCP tool '{name}': {err}");
|
||||
None
|
||||
}
|
||||
},
|
||||
Err(err) => {
|
||||
tracing::warn!("Failed to serialize MCP tool '{name}': {err}");
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
|
||||
let resources = resources
|
||||
.into_iter()
|
||||
.map(|(name, resources)| {
|
||||
let resources = resources
|
||||
.into_iter()
|
||||
.filter_map(|resource| match serde_json::to_value(resource) {
|
||||
Ok(value) => match Resource::from_mcp_value(value.clone()) {
|
||||
Ok(resource) => Some(resource),
|
||||
Err(err) => {
|
||||
let (uri, resource_name) = match value {
|
||||
Value::Object(obj) => (
|
||||
obj.get("uri")
|
||||
.and_then(|v| v.as_str().map(ToString::to_string)),
|
||||
obj.get("name")
|
||||
.and_then(|v| v.as_str().map(ToString::to_string)),
|
||||
),
|
||||
_ => (None, None),
|
||||
};
|
||||
|
||||
tracing::warn!(
|
||||
"Failed to convert MCP resource (uri={uri:?}, name={resource_name:?}): {err}"
|
||||
);
|
||||
None
|
||||
}
|
||||
},
|
||||
Err(err) => {
|
||||
tracing::warn!("Failed to serialize MCP resource: {err}");
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
(name, resources)
|
||||
})
|
||||
.collect();
|
||||
|
||||
let resource_templates = resource_templates
|
||||
.into_iter()
|
||||
.map(|(name, templates)| {
|
||||
let templates = templates
|
||||
.into_iter()
|
||||
.filter_map(|template| match serde_json::to_value(template) {
|
||||
Ok(value) => match ResourceTemplate::from_mcp_value(value.clone()) {
|
||||
Ok(template) => Some(template),
|
||||
Err(err) => {
|
||||
let (uri_template, template_name) = match value {
|
||||
Value::Object(obj) => (
|
||||
obj.get("uriTemplate")
|
||||
.or_else(|| obj.get("uri_template"))
|
||||
.and_then(|v| v.as_str().map(ToString::to_string)),
|
||||
obj.get("name")
|
||||
.and_then(|v| v.as_str().map(ToString::to_string)),
|
||||
),
|
||||
_ => (None, None),
|
||||
};
|
||||
|
||||
tracing::warn!(
|
||||
"Failed to convert MCP resource template (uri_template={uri_template:?}, name={template_name:?}): {err}"
|
||||
);
|
||||
None
|
||||
}
|
||||
},
|
||||
Err(err) => {
|
||||
tracing::warn!("Failed to serialize MCP resource template: {err}");
|
||||
None
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
(name, templates)
|
||||
})
|
||||
.collect();
|
||||
|
||||
McpListToolsResponseEvent {
|
||||
tools: tools
|
||||
.into_iter()
|
||||
.map(|(name, tool)| (name, tool.tool))
|
||||
.collect(),
|
||||
tools,
|
||||
resources,
|
||||
resource_templates,
|
||||
auth_statuses,
|
||||
@@ -244,21 +331,18 @@ pub(crate) async fn collect_mcp_snapshot_from_manager(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use mcp_types::ToolInputSchema;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn make_tool(name: &str) -> McpTool {
|
||||
McpTool {
|
||||
annotations: None,
|
||||
description: None,
|
||||
input_schema: ToolInputSchema {
|
||||
properties: None,
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
fn make_tool(name: &str) -> Tool {
|
||||
Tool {
|
||||
name: name.to_string(),
|
||||
output_schema: None,
|
||||
title: None,
|
||||
description: None,
|
||||
input_schema: serde_json::json!({"type": "object", "properties": {}}),
|
||||
output_schema: None,
|
||||
annotations: None,
|
||||
icons: None,
|
||||
meta: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -23,6 +23,8 @@ use async_channel::Sender;
|
||||
use codex_async_utils::CancelErr;
|
||||
use codex_async_utils::OrCancelExt;
|
||||
use codex_protocol::approvals::ElicitationRequestEvent;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
use codex_protocol::mcp::RequestId as ProtocolRequestId;
|
||||
use codex_protocol::protocol::Event;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::McpStartupCompleteEvent;
|
||||
@@ -37,22 +39,23 @@ use codex_rmcp_client::SendElicitation;
|
||||
use futures::future::BoxFuture;
|
||||
use futures::future::FutureExt;
|
||||
use futures::future::Shared;
|
||||
use mcp_types::ClientCapabilities;
|
||||
use mcp_types::Implementation;
|
||||
use mcp_types::ListResourceTemplatesRequestParams;
|
||||
use mcp_types::ListResourceTemplatesResult;
|
||||
use mcp_types::ListResourcesRequestParams;
|
||||
use mcp_types::ListResourcesResult;
|
||||
use mcp_types::ReadResourceRequestParams;
|
||||
use mcp_types::ReadResourceResult;
|
||||
use mcp_types::RequestId;
|
||||
use mcp_types::Resource;
|
||||
use mcp_types::ResourceTemplate;
|
||||
use mcp_types::Tool;
|
||||
use rmcp::model::ClientCapabilities;
|
||||
use rmcp::model::ElicitationCapability;
|
||||
use rmcp::model::Implementation;
|
||||
use rmcp::model::InitializeRequestParam;
|
||||
use rmcp::model::ListResourceTemplatesResult;
|
||||
use rmcp::model::ListResourcesResult;
|
||||
use rmcp::model::PaginatedRequestParam;
|
||||
use rmcp::model::ProtocolVersion;
|
||||
use rmcp::model::ReadResourceRequestParam;
|
||||
use rmcp::model::ReadResourceResult;
|
||||
use rmcp::model::RequestId;
|
||||
use rmcp::model::Resource;
|
||||
use rmcp::model::ResourceTemplate;
|
||||
use rmcp::model::Tool;
|
||||
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json::json;
|
||||
use sha1::Digest;
|
||||
use sha1::Sha1;
|
||||
use tokio::sync::Mutex;
|
||||
@@ -198,7 +201,14 @@ impl ElicitationRequestManager {
|
||||
id: "mcp_elicitation_request".to_string(),
|
||||
msg: EventMsg::ElicitationRequest(ElicitationRequestEvent {
|
||||
server_name,
|
||||
id,
|
||||
id: match id.clone() {
|
||||
rmcp::model::NumberOrString::String(value) => {
|
||||
ProtocolRequestId::String(value.to_string())
|
||||
}
|
||||
rmcp::model::NumberOrString::Number(value) => {
|
||||
ProtocolRequestId::Integer(value)
|
||||
}
|
||||
},
|
||||
message: elicitation.message,
|
||||
}),
|
||||
})
|
||||
@@ -493,7 +503,7 @@ impl McpConnectionManager {
|
||||
let mut cursor: Option<String> = None;
|
||||
|
||||
loop {
|
||||
let params = cursor.as_ref().map(|next| ListResourcesRequestParams {
|
||||
let params = cursor.as_ref().map(|next| PaginatedRequestParam {
|
||||
cursor: Some(next.clone()),
|
||||
});
|
||||
let response = match client.list_resources(params, timeout).await {
|
||||
@@ -558,11 +568,9 @@ impl McpConnectionManager {
|
||||
let mut cursor: Option<String> = None;
|
||||
|
||||
loop {
|
||||
let params = cursor
|
||||
.as_ref()
|
||||
.map(|next| ListResourceTemplatesRequestParams {
|
||||
cursor: Some(next.clone()),
|
||||
});
|
||||
let params = cursor.as_ref().map(|next| PaginatedRequestParam {
|
||||
cursor: Some(next.clone()),
|
||||
});
|
||||
let response = match client.list_resource_templates(params, timeout).await {
|
||||
Ok(result) => result,
|
||||
Err(err) => return (server_name_cloned, Err(err)),
|
||||
@@ -615,7 +623,7 @@ impl McpConnectionManager {
|
||||
server: &str,
|
||||
tool: &str,
|
||||
arguments: Option<serde_json::Value>,
|
||||
) -> Result<mcp_types::CallToolResult> {
|
||||
) -> Result<CallToolResult> {
|
||||
let client = self.client_by_name(server).await?;
|
||||
if !client.tool_filter.allows(tool) {
|
||||
return Err(anyhow!(
|
||||
@@ -623,18 +631,34 @@ impl McpConnectionManager {
|
||||
));
|
||||
}
|
||||
|
||||
client
|
||||
let result: rmcp::model::CallToolResult = client
|
||||
.client
|
||||
.call_tool(tool.to_string(), arguments, client.tool_timeout)
|
||||
.await
|
||||
.with_context(|| format!("tool call failed for `{server}/{tool}`"))
|
||||
.with_context(|| format!("tool call failed for `{server}/{tool}`"))?;
|
||||
|
||||
let content = result
|
||||
.content
|
||||
.into_iter()
|
||||
.map(|content| {
|
||||
serde_json::to_value(content)
|
||||
.unwrap_or_else(|_| serde_json::Value::String("<content>".to_string()))
|
||||
})
|
||||
.collect();
|
||||
|
||||
Ok(CallToolResult {
|
||||
content,
|
||||
structured_content: result.structured_content,
|
||||
is_error: result.is_error,
|
||||
meta: result.meta.and_then(|meta| serde_json::to_value(meta).ok()),
|
||||
})
|
||||
}
|
||||
|
||||
/// List resources from the specified server.
|
||||
pub async fn list_resources(
|
||||
&self,
|
||||
server: &str,
|
||||
params: Option<ListResourcesRequestParams>,
|
||||
params: Option<PaginatedRequestParam>,
|
||||
) -> Result<ListResourcesResult> {
|
||||
let managed = self.client_by_name(server).await?;
|
||||
let timeout = managed.tool_timeout;
|
||||
@@ -650,7 +674,7 @@ impl McpConnectionManager {
|
||||
pub async fn list_resource_templates(
|
||||
&self,
|
||||
server: &str,
|
||||
params: Option<ListResourceTemplatesRequestParams>,
|
||||
params: Option<PaginatedRequestParam>,
|
||||
) -> Result<ListResourceTemplatesResult> {
|
||||
let managed = self.client_by_name(server).await?;
|
||||
let client = managed.client.clone();
|
||||
@@ -666,7 +690,7 @@ impl McpConnectionManager {
|
||||
pub async fn read_resource(
|
||||
&self,
|
||||
server: &str,
|
||||
params: ReadResourceRequestParams,
|
||||
params: ReadResourceRequestParam,
|
||||
) -> Result<ReadResourceResult> {
|
||||
let managed = self.client_by_name(server).await?;
|
||||
let client = managed.client.clone();
|
||||
@@ -849,25 +873,25 @@ async fn start_server_task(
|
||||
tx_event: Sender<Event>,
|
||||
elicitation_requests: ElicitationRequestManager,
|
||||
) -> Result<ManagedClient, StartupOutcomeError> {
|
||||
let params = mcp_types::InitializeRequestParams {
|
||||
let params = InitializeRequestParam {
|
||||
capabilities: ClientCapabilities {
|
||||
experimental: None,
|
||||
roots: None,
|
||||
sampling: None,
|
||||
// https://modelcontextprotocol.io/specification/2025-06-18/client/elicitation#capabilities
|
||||
// indicates this should be an empty object.
|
||||
elicitation: Some(json!({})),
|
||||
elicitation: Some(ElicitationCapability {
|
||||
schema_validation: None,
|
||||
}),
|
||||
},
|
||||
client_info: Implementation {
|
||||
name: "codex-mcp-client".to_owned(),
|
||||
version: env!("CARGO_PKG_VERSION").to_owned(),
|
||||
title: Some("Codex".into()),
|
||||
// This field is used by Codex when it is an MCP
|
||||
// server: it should not be used when Codex is
|
||||
// an MCP client.
|
||||
user_agent: None,
|
||||
icons: None,
|
||||
website_url: None,
|
||||
},
|
||||
protocol_version: mcp_types::MCP_SCHEMA_VERSION.to_owned(),
|
||||
protocol_version: ProtocolVersion::V_2025_06_18,
|
||||
};
|
||||
|
||||
let send_elicitation = elicitation_requests.make_sender(server_name.clone(), tx_event);
|
||||
@@ -964,7 +988,7 @@ async fn list_tools_for_client(
|
||||
}
|
||||
ToolInfo {
|
||||
server_name: server_name.to_owned(),
|
||||
tool_name: tool_def.name.clone(),
|
||||
tool_name: tool_def.name.to_string(),
|
||||
tool: tool_def,
|
||||
connector_id: tool.connector_id,
|
||||
connector_name,
|
||||
@@ -1047,24 +1071,23 @@ mod mcp_init_error_display_tests {}
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::protocol::McpAuthStatus;
|
||||
use mcp_types::ToolInputSchema;
|
||||
use rmcp::model::JsonObject;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
fn create_test_tool(server_name: &str, tool_name: &str) -> ToolInfo {
|
||||
ToolInfo {
|
||||
server_name: server_name.to_string(),
|
||||
tool_name: tool_name.to_string(),
|
||||
tool: Tool {
|
||||
annotations: None,
|
||||
description: Some(format!("Test tool: {tool_name}")),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: None,
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
name: tool_name.to_string(),
|
||||
output_schema: None,
|
||||
name: tool_name.to_string().into(),
|
||||
title: None,
|
||||
description: Some(format!("Test tool: {tool_name}").into()),
|
||||
input_schema: Arc::new(JsonObject::default()),
|
||||
output_schema: None,
|
||||
annotations: None,
|
||||
icons: None,
|
||||
meta: None,
|
||||
},
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
|
||||
@@ -10,6 +10,7 @@ use crate::protocol::EventMsg;
|
||||
use crate::protocol::McpInvocation;
|
||||
use crate::protocol::McpToolCallBeginEvent;
|
||||
use crate::protocol::McpToolCallEndEvent;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
@@ -18,7 +19,7 @@ use codex_protocol::request_user_input::RequestUserInputArgs;
|
||||
use codex_protocol::request_user_input::RequestUserInputQuestion;
|
||||
use codex_protocol::request_user_input::RequestUserInputQuestionOption;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
use mcp_types::ToolAnnotations;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use std::sync::Arc;
|
||||
|
||||
/// Handles the specified tool call dispatches the appropriate
|
||||
@@ -72,7 +73,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
.await;
|
||||
|
||||
let start = Instant::now();
|
||||
let result = sess
|
||||
let result: Result<CallToolResult, String> = sess
|
||||
.call_tool(&server, &tool_name, arguments_value.clone())
|
||||
.await
|
||||
.map_err(|e| format!("tool call error: {e:?}"));
|
||||
@@ -134,7 +135,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
|
||||
let start = Instant::now();
|
||||
// Perform the tool call.
|
||||
let result = sess
|
||||
let result: Result<CallToolResult, String> = sess
|
||||
.call_tool(&server, &tool_name, arguments_value.clone())
|
||||
.await
|
||||
.map_err(|e| format!("tool call error: {e:?}"));
|
||||
@@ -341,7 +342,7 @@ async fn notify_mcp_tool_call_skip(
|
||||
call_id: &str,
|
||||
invocation: McpInvocation,
|
||||
message: String,
|
||||
) -> Result<mcp_types::CallToolResult, String> {
|
||||
) -> Result<CallToolResult, String> {
|
||||
let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent {
|
||||
call_id: call_id.to_string(),
|
||||
invocation: invocation.clone(),
|
||||
|
||||
@@ -27,7 +27,7 @@ impl ModelsCacheManager {
|
||||
}
|
||||
|
||||
/// Attempt to load a fresh cache entry. Returns `None` if the cache doesn't exist or is stale.
|
||||
pub(crate) async fn load_fresh(&self) -> Option<ModelsCache> {
|
||||
pub(crate) async fn load_fresh(&self, expected_version: &str) -> Option<ModelsCache> {
|
||||
let cache = match self.load().await {
|
||||
Ok(cache) => cache?,
|
||||
Err(err) => {
|
||||
@@ -35,6 +35,9 @@ impl ModelsCacheManager {
|
||||
return None;
|
||||
}
|
||||
};
|
||||
if cache.client_version.as_deref() != Some(expected_version) {
|
||||
return None;
|
||||
}
|
||||
if !cache.is_fresh(self.cache_ttl) {
|
||||
return None;
|
||||
}
|
||||
@@ -42,10 +45,16 @@ impl ModelsCacheManager {
|
||||
}
|
||||
|
||||
/// Persist the cache to disk, creating parent directories as needed.
|
||||
pub(crate) async fn persist_cache(&self, models: &[ModelInfo], etag: Option<String>) {
|
||||
pub(crate) async fn persist_cache(
|
||||
&self,
|
||||
models: &[ModelInfo],
|
||||
etag: Option<String>,
|
||||
client_version: String,
|
||||
) {
|
||||
let cache = ModelsCache {
|
||||
fetched_at: Utc::now(),
|
||||
etag,
|
||||
client_version: Some(client_version),
|
||||
models: models.to_vec(),
|
||||
};
|
||||
if let Err(err) = self.save_internal(&cache).await {
|
||||
@@ -103,6 +112,20 @@ impl ModelsCacheManager {
|
||||
f(&mut cache.fetched_at);
|
||||
self.save_internal(&cache).await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
/// Mutate the full cache contents for testing.
|
||||
pub(crate) async fn mutate_cache_for_test<F>(&self, f: F) -> io::Result<()>
|
||||
where
|
||||
F: FnOnce(&mut ModelsCache),
|
||||
{
|
||||
let mut cache = match self.load().await? {
|
||||
Some(cache) => cache,
|
||||
None => return Err(io::Error::new(ErrorKind::NotFound, "cache not found")),
|
||||
};
|
||||
f(&mut cache);
|
||||
self.save_internal(&cache).await
|
||||
}
|
||||
}
|
||||
|
||||
/// Serialized snapshot of models and metadata cached on disk.
|
||||
@@ -111,6 +134,8 @@ pub(crate) struct ModelsCache {
|
||||
pub(crate) fetched_at: DateTime<Utc>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) etag: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) client_version: Option<String>,
|
||||
pub(crate) models: Vec<ModelInfo>,
|
||||
}
|
||||
|
||||
|
||||
@@ -210,7 +210,7 @@ impl ModelsManager {
|
||||
let transport = ReqwestTransport::new(build_reqwest_client());
|
||||
let client = ModelsClient::new(transport, api_provider, api_auth);
|
||||
|
||||
let client_version = format_client_version_to_whole();
|
||||
let client_version = crate::models_manager::client_version_to_whole();
|
||||
let (models, etag) = timeout(
|
||||
MODELS_REFRESH_TIMEOUT,
|
||||
client.list_models(&client_version, HeaderMap::new()),
|
||||
@@ -221,7 +221,9 @@ impl ModelsManager {
|
||||
|
||||
self.apply_remote_models(models.clone()).await;
|
||||
*self.etag.write().await = etag.clone();
|
||||
self.cache_manager.persist_cache(&models, etag).await;
|
||||
self.cache_manager
|
||||
.persist_cache(&models, etag, client_version)
|
||||
.await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -255,7 +257,8 @@ impl ModelsManager {
|
||||
async fn try_load_cache(&self) -> bool {
|
||||
let _timer =
|
||||
codex_otel::start_global_timer("codex.remote_models.load_cache.duration_ms", &[]);
|
||||
let cache = match self.cache_manager.load_fresh().await {
|
||||
let client_version = crate::models_manager::client_version_to_whole();
|
||||
let cache = match self.cache_manager.load_fresh(&client_version).await {
|
||||
Some(cache) => cache,
|
||||
None => return false,
|
||||
};
|
||||
@@ -350,16 +353,6 @@ impl ModelsManager {
|
||||
}
|
||||
}
|
||||
|
||||
/// Convert a client version string to a whole version string (e.g. "1.2.3-alpha.4" -> "1.2.3")
|
||||
fn format_client_version_to_whole() -> String {
|
||||
format!(
|
||||
"{}.{}.{}",
|
||||
env!("CARGO_PKG_VERSION_MAJOR"),
|
||||
env!("CARGO_PKG_VERSION_MINOR"),
|
||||
env!("CARGO_PKG_VERSION_PATCH")
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -613,6 +606,75 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn refresh_available_models_refetches_when_version_mismatch() {
|
||||
let server = MockServer::start().await;
|
||||
let initial_models = vec![remote_model("old", "Old", 1)];
|
||||
let initial_mock = mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: initial_models.clone(),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let codex_home = tempdir().expect("temp dir");
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("load default test config");
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
let auth_manager = Arc::new(AuthManager::new(
|
||||
codex_home.path().to_path_buf(),
|
||||
false,
|
||||
AuthCredentialsStoreMode::File,
|
||||
));
|
||||
let provider = provider_for(server.uri());
|
||||
let manager =
|
||||
ModelsManager::with_provider(codex_home.path().to_path_buf(), auth_manager, provider);
|
||||
|
||||
manager
|
||||
.refresh_available_models(&config, RefreshStrategy::OnlineIfUncached)
|
||||
.await
|
||||
.expect("initial refresh succeeds");
|
||||
|
||||
manager
|
||||
.cache_manager
|
||||
.mutate_cache_for_test(|cache| {
|
||||
let client_version = crate::models_manager::client_version_to_whole();
|
||||
cache.client_version = Some(format!("{client_version}-mismatch"));
|
||||
})
|
||||
.await
|
||||
.expect("cache mutation succeeds");
|
||||
|
||||
let updated_models = vec![remote_model("new", "New", 2)];
|
||||
server.reset().await;
|
||||
let refreshed_mock = mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: updated_models.clone(),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
manager
|
||||
.refresh_available_models(&config, RefreshStrategy::OnlineIfUncached)
|
||||
.await
|
||||
.expect("second refresh succeeds");
|
||||
assert_models_contain(&manager.get_remote_models(&config).await, &updated_models);
|
||||
assert_eq!(
|
||||
initial_mock.requests().len(),
|
||||
1,
|
||||
"initial refresh should only hit /models once"
|
||||
);
|
||||
assert_eq!(
|
||||
refreshed_mock.requests().len(),
|
||||
1,
|
||||
"version mismatch should fetch /models once"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn refresh_available_models_drops_removed_remote_models() {
|
||||
let server = MockServer::start().await;
|
||||
|
||||
@@ -6,3 +6,13 @@ pub mod model_presets;
|
||||
|
||||
#[cfg(any(test, feature = "test-support"))]
|
||||
pub use collaboration_mode_presets::test_builtin_collaboration_mode_presets;
|
||||
|
||||
/// Convert the client version string to a whole version string (e.g. "1.2.3-alpha.4" -> "1.2.3").
|
||||
pub fn client_version_to_whole() -> String {
|
||||
format!(
|
||||
"{}.{}.{}",
|
||||
env!("CARGO_PKG_VERSION_MAJOR"),
|
||||
env!("CARGO_PKG_VERSION_MINOR"),
|
||||
env!("CARGO_PKG_VERSION_PATCH")
|
||||
)
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::TruncationMode;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
use codex_protocol::openai_models::default_input_modalities;
|
||||
|
||||
use crate::config::Config;
|
||||
use crate::features::Feature;
|
||||
@@ -66,6 +67,7 @@ macro_rules! model_info {
|
||||
auto_compact_token_limit: None,
|
||||
effective_context_window_percent: 95,
|
||||
experimental_supported_tools: Vec::new(),
|
||||
input_modalities: default_input_modalities(),
|
||||
};
|
||||
|
||||
$(
|
||||
|
||||
@@ -3,6 +3,7 @@ use codex_protocol::openai_models::ModelPreset;
|
||||
use codex_protocol::openai_models::ModelUpgrade;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::default_input_modalities;
|
||||
use indoc::indoc;
|
||||
use once_cell::sync::Lazy;
|
||||
|
||||
@@ -41,6 +42,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: None,
|
||||
show_in_picker: true,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
ModelPreset {
|
||||
id: "gpt-5.1-codex-max".to_string(),
|
||||
@@ -71,6 +73,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: Some(gpt_52_codex_upgrade()),
|
||||
show_in_picker: true,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
ModelPreset {
|
||||
id: "gpt-5.1-codex-mini".to_string(),
|
||||
@@ -94,6 +97,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: Some(gpt_52_codex_upgrade()),
|
||||
show_in_picker: true,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
ModelPreset {
|
||||
id: "gpt-5.2".to_string(),
|
||||
@@ -124,6 +128,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: Some(gpt_52_codex_upgrade()),
|
||||
show_in_picker: true,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
ModelPreset {
|
||||
id: "bengalfox".to_string(),
|
||||
@@ -154,6 +159,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: None,
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
ModelPreset {
|
||||
id: "boomslang".to_string(),
|
||||
@@ -184,6 +190,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: None,
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
// Deprecated models.
|
||||
ModelPreset {
|
||||
@@ -211,6 +218,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: Some(gpt_52_codex_upgrade()),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
ModelPreset {
|
||||
id: "gpt-5-codex-mini".to_string(),
|
||||
@@ -233,6 +241,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: Some(gpt_52_codex_upgrade()),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
ModelPreset {
|
||||
id: "gpt-5.1-codex".to_string(),
|
||||
@@ -260,6 +269,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: Some(gpt_52_codex_upgrade()),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
ModelPreset {
|
||||
id: "gpt-5".to_string(),
|
||||
@@ -290,6 +300,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: Some(gpt_52_codex_upgrade()),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
ModelPreset {
|
||||
id: "gpt-5.1".to_string(),
|
||||
@@ -316,6 +327,7 @@ static PRESETS: Lazy<Vec<ModelPreset>> = Lazy::new(|| {
|
||||
upgrade: Some(gpt_52_codex_upgrade()),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
},
|
||||
]
|
||||
});
|
||||
|
||||
@@ -15,7 +15,9 @@ use uuid::Uuid;
|
||||
|
||||
use super::ARCHIVED_SESSIONS_SUBDIR;
|
||||
use super::SESSIONS_SUBDIR;
|
||||
use crate::instructions::UserInstructions;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::session_prefix::is_session_prefix_content;
|
||||
use crate::state_db;
|
||||
use codex_file_search as file_search;
|
||||
use codex_protocol::ThreadId;
|
||||
@@ -982,9 +984,12 @@ async fn read_head_summary(path: &Path, head_limit: usize) -> io::Result<HeadTai
|
||||
.created_at
|
||||
.clone()
|
||||
.or_else(|| Some(rollout_line.timestamp.clone()));
|
||||
if let codex_protocol::models::ResponseItem::Message { role, .. } = &item
|
||||
if let codex_protocol::models::ResponseItem::Message { role, content, .. } = &item
|
||||
&& role == "user"
|
||||
&& !UserInstructions::is_user_instructions(content.as_slice())
|
||||
&& !is_session_prefix_content(content.as_slice())
|
||||
{
|
||||
tracing::warn!("Item: {item:#?}");
|
||||
summary.saw_user_event = true;
|
||||
}
|
||||
if summary.head.len() < head_limit
|
||||
|
||||
@@ -187,6 +187,29 @@ pub(crate) async fn backfill_sessions(
|
||||
warn!("failed to upsert rollout {}: {err}", path.display());
|
||||
} else {
|
||||
stats.upserted = stats.upserted.saturating_add(1);
|
||||
if let Ok(meta_line) = rollout::list::read_session_meta_line(&path).await {
|
||||
if let Err(err) = runtime
|
||||
.persist_dynamic_tools(
|
||||
meta_line.meta.id,
|
||||
meta_line.meta.dynamic_tools.as_deref(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
if let Some(otel) = otel {
|
||||
otel.counter(
|
||||
DB_ERROR_METRIC,
|
||||
1,
|
||||
&[("stage", "backfill_dynamic_tools")],
|
||||
);
|
||||
}
|
||||
warn!("failed to backfill dynamic tools {}: {err}", path.display());
|
||||
}
|
||||
} else {
|
||||
warn!(
|
||||
"failed to read session meta for dynamic tools {}",
|
||||
path.display()
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
|
||||
@@ -902,6 +902,7 @@ async fn test_updated_at_uses_file_mtime() -> Result<()> {
|
||||
text: format!("reply-{idx}"),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}),
|
||||
};
|
||||
writeln!(file, "{}", serde_json::to_string(&response_line)?)?;
|
||||
|
||||
@@ -86,6 +86,7 @@ mod tests {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -97,6 +98,7 @@ mod tests {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
use codex_protocol::models::ContentItem;
|
||||
|
||||
/// Helpers for identifying model-visible "session prefix" messages.
|
||||
///
|
||||
/// A session prefix is a user-role message that carries configuration or state needed by
|
||||
@@ -13,3 +15,12 @@ pub(crate) fn is_session_prefix(text: &str) -> bool {
|
||||
let lowered = trimmed.to_ascii_lowercase();
|
||||
lowered.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG) || lowered.starts_with(TURN_ABORTED_OPEN_TAG)
|
||||
}
|
||||
|
||||
/// Returns true if `text` starts with a session prefix marker (case-insensitive).
|
||||
pub(crate) fn is_session_prefix_content(content: &[ContentItem]) -> bool {
|
||||
if let [ContentItem::InputText { text }] = content {
|
||||
is_session_prefix(text)
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ use crate::skills::model::SkillToolDependency;
|
||||
use crate::skills::system::system_cache_root_dir;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use dirs::home_dir;
|
||||
use dunce::canonicalize as canonicalize_path;
|
||||
use serde::Deserialize;
|
||||
use std::collections::HashSet;
|
||||
@@ -164,7 +165,10 @@ where
|
||||
outcome
|
||||
}
|
||||
|
||||
fn skill_roots_from_layer_stack_inner(config_layer_stack: &ConfigLayerStack) -> Vec<SkillRoot> {
|
||||
fn skill_roots_from_layer_stack_inner(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
home_dir: Option<&Path>,
|
||||
) -> Vec<SkillRoot> {
|
||||
let mut roots = Vec::new();
|
||||
|
||||
for layer in
|
||||
@@ -182,12 +186,21 @@ fn skill_roots_from_layer_stack_inner(config_layer_stack: &ConfigLayerStack) ->
|
||||
});
|
||||
}
|
||||
ConfigLayerSource::User { .. } => {
|
||||
// `$CODEX_HOME/skills` (user-installed skills).
|
||||
// Deprecated user skills location (`$CODEX_HOME/skills`), kept for backward
|
||||
// compatibility.
|
||||
roots.push(SkillRoot {
|
||||
path: config_folder.as_path().join(SKILLS_DIR_NAME),
|
||||
scope: SkillScope::User,
|
||||
});
|
||||
|
||||
// `$HOME/.agents/skills` (user-installed skills).
|
||||
if let Some(home_dir) = home_dir {
|
||||
roots.push(SkillRoot {
|
||||
path: home_dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
|
||||
scope: SkillScope::User,
|
||||
});
|
||||
}
|
||||
|
||||
// Embedded system skills are cached under `$CODEX_HOME/skills/.system` and are a
|
||||
// special case (not a config layer).
|
||||
roots.push(SkillRoot {
|
||||
@@ -219,15 +232,16 @@ fn skill_roots(config: &Config) -> Vec<SkillRoot> {
|
||||
|
||||
pub(crate) fn skill_roots_from_layer_stack(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
home_dir: Option<&Path>,
|
||||
) -> Vec<SkillRoot> {
|
||||
skill_roots_from_layer_stack_inner(config_layer_stack)
|
||||
skill_roots_from_layer_stack_inner(config_layer_stack, home_dir)
|
||||
}
|
||||
|
||||
pub(crate) fn skill_roots_from_layer_stack_with_agents(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
cwd: &Path,
|
||||
) -> Vec<SkillRoot> {
|
||||
let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack);
|
||||
let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack, home_dir().as_deref());
|
||||
roots.extend(repo_agents_skill_roots(config_layer_stack, cwd));
|
||||
dedupe_skill_roots_by_path(&mut roots);
|
||||
roots
|
||||
@@ -828,7 +842,8 @@ mod tests {
|
||||
let tmp = tempfile::tempdir()?;
|
||||
|
||||
let system_folder = tmp.path().join("etc/codex");
|
||||
let user_folder = tmp.path().join("home/codex");
|
||||
let home_folder = tmp.path().join("home");
|
||||
let user_folder = home_folder.join("codex");
|
||||
fs::create_dir_all(&system_folder)?;
|
||||
fs::create_dir_all(&user_folder)?;
|
||||
|
||||
@@ -852,7 +867,7 @@ mod tests {
|
||||
ConfigRequirementsToml::default(),
|
||||
)?;
|
||||
|
||||
let got = skill_roots_from_layer_stack(&stack)
|
||||
let got = skill_roots_from_layer_stack(&stack, Some(&home_folder))
|
||||
.into_iter()
|
||||
.map(|root| (root.scope, root.path))
|
||||
.collect::<Vec<_>>();
|
||||
@@ -861,6 +876,10 @@ mod tests {
|
||||
got,
|
||||
vec![
|
||||
(SkillScope::User, user_folder.join("skills")),
|
||||
(
|
||||
SkillScope::User,
|
||||
home_folder.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME)
|
||||
),
|
||||
(
|
||||
SkillScope::System,
|
||||
user_folder.join("skills").join(".system")
|
||||
@@ -876,7 +895,8 @@ mod tests {
|
||||
fn skill_roots_from_layer_stack_includes_disabled_project_layers() -> anyhow::Result<()> {
|
||||
let tmp = tempfile::tempdir()?;
|
||||
|
||||
let user_folder = tmp.path().join("home/codex");
|
||||
let home_folder = tmp.path().join("home");
|
||||
let user_folder = home_folder.join("codex");
|
||||
fs::create_dir_all(&user_folder)?;
|
||||
|
||||
let project_root = tmp.path().join("repo");
|
||||
@@ -905,7 +925,7 @@ mod tests {
|
||||
ConfigRequirementsToml::default(),
|
||||
)?;
|
||||
|
||||
let got = skill_roots_from_layer_stack(&stack)
|
||||
let got = skill_roots_from_layer_stack(&stack, Some(&home_folder))
|
||||
.into_iter()
|
||||
.map(|root| (root.scope, root.path))
|
||||
.collect::<Vec<_>>();
|
||||
@@ -915,6 +935,10 @@ mod tests {
|
||||
vec![
|
||||
(SkillScope::Repo, dot_codex.join("skills")),
|
||||
(SkillScope::User, user_folder.join("skills")),
|
||||
(
|
||||
SkillScope::User,
|
||||
home_folder.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME)
|
||||
),
|
||||
(
|
||||
SkillScope::System,
|
||||
user_folder.join("skills").join(".system")
|
||||
@@ -925,6 +949,55 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loads_skills_from_home_agents_dir_for_user_scope() -> anyhow::Result<()> {
|
||||
let tmp = tempfile::tempdir()?;
|
||||
|
||||
let home_folder = tmp.path().join("home");
|
||||
let user_folder = home_folder.join("codex");
|
||||
fs::create_dir_all(&user_folder)?;
|
||||
|
||||
let user_file = AbsolutePathBuf::from_absolute_path(user_folder.join("config.toml"))?;
|
||||
let layers = vec![ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User { file: user_file },
|
||||
TomlValue::Table(toml::map::Map::new()),
|
||||
)];
|
||||
let stack = ConfigLayerStack::new(
|
||||
layers,
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)?;
|
||||
|
||||
let skill_path = write_skill_at(
|
||||
&home_folder.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
|
||||
"agents-home",
|
||||
"agents-home-skill",
|
||||
"from home agents",
|
||||
);
|
||||
|
||||
let outcome =
|
||||
load_skills_from_roots(skill_roots_from_layer_stack(&stack, Some(&home_folder)));
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "agents-home-skill".to_string(),
|
||||
description: "from home agents".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_skill(codex_home: &TempDir, dir: &str, name: &str, description: &str) -> PathBuf {
|
||||
write_skill_at(&codex_home.path().join("skills"), dir, name, description)
|
||||
}
|
||||
@@ -2135,6 +2208,9 @@ interface:
|
||||
.map(|root| root.scope)
|
||||
.collect();
|
||||
let mut expected = vec![SkillScope::User, SkillScope::System];
|
||||
if home_dir().is_some() {
|
||||
expected.insert(1, SkillScope::User);
|
||||
}
|
||||
if cfg!(unix) {
|
||||
expected.push(SkillScope::Admin);
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use chrono::Timelike;
|
||||
use chrono::Utc;
|
||||
use codex_otel::OtelManager;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_state::DB_METRIC_COMPARE_ERROR;
|
||||
@@ -196,6 +197,37 @@ pub async fn find_rollout_path_by_id(
|
||||
})
|
||||
}
|
||||
|
||||
/// Get dynamic tools for a thread id using SQLite.
|
||||
pub async fn get_dynamic_tools(
|
||||
context: Option<&codex_state::StateRuntime>,
|
||||
thread_id: ThreadId,
|
||||
stage: &str,
|
||||
) -> Option<Vec<DynamicToolSpec>> {
|
||||
let ctx = context?;
|
||||
match ctx.get_dynamic_tools(thread_id).await {
|
||||
Ok(tools) => tools,
|
||||
Err(err) => {
|
||||
warn!("state db get_dynamic_tools failed during {stage}: {err}");
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Persist dynamic tools for a thread id using SQLite, if none exist yet.
|
||||
pub async fn persist_dynamic_tools(
|
||||
context: Option<&codex_state::StateRuntime>,
|
||||
thread_id: ThreadId,
|
||||
tools: Option<&[DynamicToolSpec]>,
|
||||
stage: &str,
|
||||
) {
|
||||
let Some(ctx) = context else {
|
||||
return;
|
||||
};
|
||||
if let Err(err) = ctx.persist_dynamic_tools(thread_id, tools).await {
|
||||
warn!("state db persist_dynamic_tools failed during {stage}: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Reconcile rollout items into SQLite, falling back to scanning the rollout file.
|
||||
pub async fn reconcile_rollout(
|
||||
context: Option<&codex_state::StateRuntime>,
|
||||
@@ -235,6 +267,21 @@ pub async fn reconcile_rollout(
|
||||
"state db reconcile_rollout upsert failed {}: {err}",
|
||||
rollout_path.display()
|
||||
);
|
||||
return;
|
||||
}
|
||||
if let Ok(meta_line) = crate::rollout::list::read_session_meta_line(rollout_path).await {
|
||||
persist_dynamic_tools(
|
||||
Some(ctx),
|
||||
meta_line.meta.id,
|
||||
meta_line.meta.dynamic_tools.as_deref(),
|
||||
"reconcile_rollout",
|
||||
)
|
||||
.await;
|
||||
} else {
|
||||
warn!(
|
||||
"state db reconcile_rollout missing session meta {}",
|
||||
rollout_path.display()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -265,6 +265,7 @@ impl Session {
|
||||
),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
self.record_into_history(std::slice::from_ref(&marker), task.turn_context.as_ref())
|
||||
.await;
|
||||
|
||||
@@ -222,6 +222,7 @@ pub(crate) async fn exit_review_mode(
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText { text: user_message }],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}],
|
||||
)
|
||||
.await;
|
||||
@@ -241,6 +242,7 @@ pub(crate) async fn exit_review_mode(
|
||||
text: assistant_message,
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -519,6 +519,7 @@ mod tests {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
fn assistant_msg(text: &str) -> ResponseItem {
|
||||
@@ -529,6 +530,7 @@ mod tests {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,12 +4,12 @@ use crate::tools::TELEMETRY_PREVIEW_MAX_BYTES;
|
||||
use crate::tools::TELEMETRY_PREVIEW_MAX_LINES;
|
||||
use crate::tools::TELEMETRY_PREVIEW_TRUNCATION_NOTICE;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
use codex_protocol::models::FunctionCallOutputContentItem;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::models::ShellToolCallParams;
|
||||
use codex_utils_string::take_bytes_at_char_boundary;
|
||||
use mcp_types::CallToolResult;
|
||||
use std::borrow::Cow;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::Mutex;
|
||||
|
||||
@@ -4,17 +4,14 @@ use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use mcp_types::CallToolResult;
|
||||
use mcp_types::ContentBlock;
|
||||
use mcp_types::ListResourceTemplatesRequestParams;
|
||||
use mcp_types::ListResourceTemplatesResult;
|
||||
use mcp_types::ListResourcesRequestParams;
|
||||
use mcp_types::ListResourcesResult;
|
||||
use mcp_types::ReadResourceRequestParams;
|
||||
use mcp_types::ReadResourceResult;
|
||||
use mcp_types::Resource;
|
||||
use mcp_types::ResourceTemplate;
|
||||
use mcp_types::TextContent;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
use rmcp::model::ListResourceTemplatesResult;
|
||||
use rmcp::model::ListResourcesResult;
|
||||
use rmcp::model::PaginatedRequestParam;
|
||||
use rmcp::model::ReadResourceRequestParam;
|
||||
use rmcp::model::ReadResourceResult;
|
||||
use rmcp::model::Resource;
|
||||
use rmcp::model::ResourceTemplate;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde::de::DeserializeOwned;
|
||||
@@ -264,7 +261,7 @@ async fn handle_list_resources(
|
||||
|
||||
let payload_result: Result<ListResourcesPayload, FunctionCallError> = async {
|
||||
if let Some(server_name) = server.clone() {
|
||||
let params = cursor.clone().map(|value| ListResourcesRequestParams {
|
||||
let params = cursor.clone().map(|value| PaginatedRequestParam {
|
||||
cursor: Some(value),
|
||||
});
|
||||
let result = session
|
||||
@@ -371,11 +368,9 @@ async fn handle_list_resource_templates(
|
||||
|
||||
let payload_result: Result<ListResourceTemplatesPayload, FunctionCallError> = async {
|
||||
if let Some(server_name) = server.clone() {
|
||||
let params = cursor
|
||||
.clone()
|
||||
.map(|value| ListResourceTemplatesRequestParams {
|
||||
cursor: Some(value),
|
||||
});
|
||||
let params = cursor.clone().map(|value| PaginatedRequestParam {
|
||||
cursor: Some(value),
|
||||
});
|
||||
let result = session
|
||||
.list_resource_templates(&server_name, params)
|
||||
.await
|
||||
@@ -482,7 +477,7 @@ async fn handle_read_resource(
|
||||
|
||||
let payload_result: Result<ReadResourcePayload, FunctionCallError> = async {
|
||||
let result = session
|
||||
.read_resource(&server, ReadResourceRequestParams { uri: uri.clone() })
|
||||
.read_resource(&server, ReadResourceRequestParam { uri: uri.clone() })
|
||||
.await
|
||||
.map_err(|err| {
|
||||
FunctionCallError::RespondToModel(format!("resources/read failed: {err:#}"))
|
||||
@@ -551,13 +546,10 @@ async fn handle_read_resource(
|
||||
|
||||
fn call_tool_result_from_content(content: &str, success: Option<bool>) -> CallToolResult {
|
||||
CallToolResult {
|
||||
content: vec![ContentBlock::TextContent(TextContent {
|
||||
annotations: None,
|
||||
text: content.to_string(),
|
||||
r#type: "text".to_string(),
|
||||
})],
|
||||
is_error: success.map(|value| !value),
|
||||
content: vec![serde_json::json!({"type": "text", "text": content})],
|
||||
structured_content: None,
|
||||
is_error: success.map(|value| !value),
|
||||
meta: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -678,32 +670,33 @@ where
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use mcp_types::ListResourcesResult;
|
||||
use mcp_types::ResourceTemplate;
|
||||
use pretty_assertions::assert_eq;
|
||||
use rmcp::model::AnnotateAble;
|
||||
use serde_json::json;
|
||||
|
||||
fn resource(uri: &str, name: &str) -> Resource {
|
||||
Resource {
|
||||
annotations: None,
|
||||
rmcp::model::RawResource {
|
||||
uri: uri.to_string(),
|
||||
name: name.to_string(),
|
||||
title: None,
|
||||
description: None,
|
||||
mime_type: None,
|
||||
name: name.to_string(),
|
||||
size: None,
|
||||
title: None,
|
||||
uri: uri.to_string(),
|
||||
icons: None,
|
||||
meta: None,
|
||||
}
|
||||
.no_annotation()
|
||||
}
|
||||
|
||||
fn template(uri_template: &str, name: &str) -> ResourceTemplate {
|
||||
ResourceTemplate {
|
||||
annotations: None,
|
||||
description: None,
|
||||
mime_type: None,
|
||||
rmcp::model::RawResourceTemplate {
|
||||
uri_template: uri_template.to_string(),
|
||||
name: name.to_string(),
|
||||
title: None,
|
||||
uri_template: uri_template.to_string(),
|
||||
description: None,
|
||||
mime_type: None,
|
||||
}
|
||||
.no_annotation()
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -719,6 +712,7 @@ mod tests {
|
||||
#[test]
|
||||
fn list_resources_payload_from_single_server_copies_next_cursor() {
|
||||
let result = ListResourcesResult {
|
||||
meta: None,
|
||||
next_cursor: Some("cursor-1".to_string()),
|
||||
resources: vec![resource("memo://id", "memo")],
|
||||
};
|
||||
|
||||
@@ -15,6 +15,7 @@ use codex_protocol::models::LocalShellAction;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::models::ShellToolCallParams;
|
||||
use rmcp::model::Tool;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use tracing::instrument;
|
||||
@@ -34,7 +35,7 @@ pub struct ToolRouter {
|
||||
impl ToolRouter {
|
||||
pub fn from_config(
|
||||
config: &ToolsConfig,
|
||||
mcp_tools: Option<HashMap<String, mcp_types::Tool>>,
|
||||
mcp_tools: Option<HashMap<String, Tool>>,
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> Self {
|
||||
let builder = build_specs(config, mcp_tools, dynamic_tools);
|
||||
|
||||
+130
-165
@@ -1087,20 +1087,26 @@ pub(crate) fn create_tools_json_for_chat_completions_api(
|
||||
|
||||
pub(crate) fn mcp_tool_to_openai_tool(
|
||||
fully_qualified_name: String,
|
||||
tool: mcp_types::Tool,
|
||||
tool: rmcp::model::Tool,
|
||||
) -> Result<ResponsesApiTool, serde_json::Error> {
|
||||
let mcp_types::Tool {
|
||||
let rmcp::model::Tool {
|
||||
description,
|
||||
mut input_schema,
|
||||
input_schema,
|
||||
..
|
||||
} = tool;
|
||||
|
||||
// OpenAI models mandate the "properties" field in the schema. The Agents
|
||||
// SDK fixed this by inserting an empty object for "properties" if it is not
|
||||
// already present https://github.com/openai/openai-agents-python/issues/449
|
||||
// so here we do the same.
|
||||
if input_schema.properties.is_none() {
|
||||
input_schema.properties = Some(serde_json::Value::Object(serde_json::Map::new()));
|
||||
let mut serialized_input_schema = serde_json::Value::Object(input_schema.as_ref().clone());
|
||||
|
||||
// OpenAI models mandate the "properties" field in the schema. Some MCP
|
||||
// servers omit it (or set it to null), so we insert an empty object to
|
||||
// match the behavior of the Agents SDK.
|
||||
if let serde_json::Value::Object(obj) = &mut serialized_input_schema
|
||||
&& obj.get("properties").is_none_or(serde_json::Value::is_null)
|
||||
{
|
||||
obj.insert(
|
||||
"properties".to_string(),
|
||||
serde_json::Value::Object(serde_json::Map::new()),
|
||||
);
|
||||
}
|
||||
|
||||
// Serialize to a raw JSON value so we can sanitize schemas coming from MCP
|
||||
@@ -1108,13 +1114,12 @@ pub(crate) fn mcp_tool_to_openai_tool(
|
||||
// Schemas (e.g. using enum/anyOf), or use unsupported variants like
|
||||
// `integer`. Our internal JsonSchema is a small subset and requires
|
||||
// `type`, so we coerce/sanitize here for compatibility.
|
||||
let mut serialized_input_schema = serde_json::to_value(input_schema)?;
|
||||
sanitize_json_schema(&mut serialized_input_schema);
|
||||
let input_schema = serde_json::from_value::<JsonSchema>(serialized_input_schema)?;
|
||||
|
||||
Ok(ResponsesApiTool {
|
||||
name: fully_qualified_name,
|
||||
description: description.unwrap_or_default(),
|
||||
description: description.map(Into::into).unwrap_or_default(),
|
||||
strict: false,
|
||||
parameters: input_schema,
|
||||
})
|
||||
@@ -1254,7 +1259,7 @@ fn sanitize_json_schema(value: &mut JsonValue) {
|
||||
/// Builds the tool registry builder while collecting tool specs for later serialization.
|
||||
pub(crate) fn build_specs(
|
||||
config: &ToolsConfig,
|
||||
mcp_tools: Option<HashMap<String, mcp_types::Tool>>,
|
||||
mcp_tools: Option<HashMap<String, rmcp::model::Tool>>,
|
||||
dynamic_tools: &[DynamicToolSpec],
|
||||
) -> ToolRegistryBuilder {
|
||||
use crate::tools::handlers::ApplyPatchHandler;
|
||||
@@ -1410,7 +1415,7 @@ pub(crate) fn build_specs(
|
||||
}
|
||||
|
||||
if let Some(mcp_tools) = mcp_tools {
|
||||
let mut entries: Vec<(String, mcp_types::Tool)> = mcp_tools.into_iter().collect();
|
||||
let mut entries: Vec<(String, rmcp::model::Tool)> = mcp_tools.into_iter().collect();
|
||||
entries.sort_by(|a, b| a.0.cmp(&b.0));
|
||||
|
||||
for (name, tool) in entries.into_iter() {
|
||||
@@ -1452,11 +1457,50 @@ mod tests {
|
||||
use crate::config::test_config;
|
||||
use crate::models_manager::manager::ModelsManager;
|
||||
use crate::tools::registry::ConfiguredToolSpec;
|
||||
use mcp_types::ToolInputSchema;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::*;
|
||||
|
||||
fn mcp_tool(
|
||||
name: &str,
|
||||
description: &str,
|
||||
input_schema: serde_json::Value,
|
||||
) -> rmcp::model::Tool {
|
||||
rmcp::model::Tool {
|
||||
name: name.to_string().into(),
|
||||
title: None,
|
||||
description: Some(description.to_string().into()),
|
||||
input_schema: std::sync::Arc::new(rmcp::model::object(input_schema)),
|
||||
output_schema: None,
|
||||
annotations: None,
|
||||
icons: None,
|
||||
meta: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_tool_to_openai_tool_inserts_empty_properties() {
|
||||
let mut schema = rmcp::model::JsonObject::new();
|
||||
schema.insert("type".to_string(), serde_json::json!("object"));
|
||||
|
||||
let tool = rmcp::model::Tool {
|
||||
name: "no_props".to_string().into(),
|
||||
title: None,
|
||||
description: Some("No properties".to_string().into()),
|
||||
input_schema: std::sync::Arc::new(schema),
|
||||
output_schema: None,
|
||||
annotations: None,
|
||||
icons: None,
|
||||
meta: None,
|
||||
};
|
||||
|
||||
let openai_tool =
|
||||
mcp_tool_to_openai_tool("server/no_props".to_string(), tool).expect("convert tool");
|
||||
let parameters = serde_json::to_value(openai_tool.parameters).expect("serialize schema");
|
||||
|
||||
assert_eq!(parameters.get("properties"), Some(&serde_json::json!({})));
|
||||
}
|
||||
|
||||
fn tool_name(tool: &ToolSpec) -> &str {
|
||||
match tool {
|
||||
ToolSpec::Function(ResponsesApiTool { name, .. }) => name,
|
||||
@@ -2026,37 +2070,26 @@ mod tests {
|
||||
&tools_config,
|
||||
Some(HashMap::from([(
|
||||
"test_server/do_something_cool".to_string(),
|
||||
mcp_types::Tool {
|
||||
name: "do_something_cool".to_string(),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: Some(serde_json::json!({
|
||||
"string_argument": {
|
||||
"type": "string",
|
||||
},
|
||||
"number_argument": {
|
||||
"type": "number",
|
||||
},
|
||||
mcp_tool(
|
||||
"do_something_cool",
|
||||
"Do something cool",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"string_argument": { "type": "string" },
|
||||
"number_argument": { "type": "number" },
|
||||
"object_argument": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"string_property": { "type": "string" },
|
||||
"number_property": { "type": "number" },
|
||||
},
|
||||
"required": [
|
||||
"string_property",
|
||||
"number_property",
|
||||
],
|
||||
"additionalProperties": Some(false),
|
||||
"required": ["string_property", "number_property"],
|
||||
"additionalProperties": false,
|
||||
},
|
||||
})),
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
output_schema: None,
|
||||
title: None,
|
||||
annotations: None,
|
||||
description: Some("Do something cool".to_string()),
|
||||
},
|
||||
},
|
||||
}),
|
||||
),
|
||||
)])),
|
||||
&[],
|
||||
)
|
||||
@@ -2120,51 +2153,18 @@ mod tests {
|
||||
});
|
||||
|
||||
// Intentionally construct a map with keys that would sort alphabetically.
|
||||
let tools_map: HashMap<String, mcp_types::Tool> = HashMap::from([
|
||||
let tools_map: HashMap<String, rmcp::model::Tool> = HashMap::from([
|
||||
(
|
||||
"test_server/do".to_string(),
|
||||
mcp_types::Tool {
|
||||
name: "a".to_string(),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: Some(serde_json::json!({})),
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
output_schema: None,
|
||||
title: None,
|
||||
annotations: None,
|
||||
description: Some("a".to_string()),
|
||||
},
|
||||
mcp_tool("a", "a", serde_json::json!({"type": "object"})),
|
||||
),
|
||||
(
|
||||
"test_server/something".to_string(),
|
||||
mcp_types::Tool {
|
||||
name: "b".to_string(),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: Some(serde_json::json!({})),
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
output_schema: None,
|
||||
title: None,
|
||||
annotations: None,
|
||||
description: Some("b".to_string()),
|
||||
},
|
||||
mcp_tool("b", "b", serde_json::json!({"type": "object"})),
|
||||
),
|
||||
(
|
||||
"test_server/cool".to_string(),
|
||||
mcp_types::Tool {
|
||||
name: "c".to_string(),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: Some(serde_json::json!({})),
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
output_schema: None,
|
||||
title: None,
|
||||
annotations: None,
|
||||
description: Some("c".to_string()),
|
||||
},
|
||||
mcp_tool("c", "c", serde_json::json!({"type": "object"})),
|
||||
),
|
||||
]);
|
||||
|
||||
@@ -2200,22 +2200,16 @@ mod tests {
|
||||
&tools_config,
|
||||
Some(HashMap::from([(
|
||||
"dash/search".to_string(),
|
||||
mcp_types::Tool {
|
||||
name: "search".to_string(),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: Some(serde_json::json!({
|
||||
"query": {
|
||||
"description": "search query"
|
||||
}
|
||||
})),
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
output_schema: None,
|
||||
title: None,
|
||||
annotations: None,
|
||||
description: Some("Search docs".to_string()),
|
||||
},
|
||||
mcp_tool(
|
||||
"search",
|
||||
"Search docs",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": {"description": "search query"}
|
||||
}
|
||||
}),
|
||||
),
|
||||
)])),
|
||||
&[],
|
||||
)
|
||||
@@ -2258,20 +2252,14 @@ mod tests {
|
||||
&tools_config,
|
||||
Some(HashMap::from([(
|
||||
"dash/paginate".to_string(),
|
||||
mcp_types::Tool {
|
||||
name: "paginate".to_string(),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: Some(serde_json::json!({
|
||||
"page": { "type": "integer" }
|
||||
})),
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
output_schema: None,
|
||||
title: None,
|
||||
annotations: None,
|
||||
description: Some("Pagination".to_string()),
|
||||
},
|
||||
mcp_tool(
|
||||
"paginate",
|
||||
"Pagination",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {"page": {"type": "integer"}}
|
||||
}),
|
||||
),
|
||||
)])),
|
||||
&[],
|
||||
)
|
||||
@@ -2313,20 +2301,14 @@ mod tests {
|
||||
&tools_config,
|
||||
Some(HashMap::from([(
|
||||
"dash/tags".to_string(),
|
||||
mcp_types::Tool {
|
||||
name: "tags".to_string(),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: Some(serde_json::json!({
|
||||
"tags": { "type": "array" }
|
||||
})),
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
output_schema: None,
|
||||
title: None,
|
||||
annotations: None,
|
||||
description: Some("Tags".to_string()),
|
||||
},
|
||||
mcp_tool(
|
||||
"tags",
|
||||
"Tags",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {"tags": {"type": "array"}}
|
||||
}),
|
||||
),
|
||||
)])),
|
||||
&[],
|
||||
)
|
||||
@@ -2370,20 +2352,16 @@ mod tests {
|
||||
&tools_config,
|
||||
Some(HashMap::from([(
|
||||
"dash/value".to_string(),
|
||||
mcp_types::Tool {
|
||||
name: "value".to_string(),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: Some(serde_json::json!({
|
||||
"value": { "anyOf": [ { "type": "string" }, { "type": "number" } ] }
|
||||
})),
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
output_schema: None,
|
||||
title: None,
|
||||
annotations: None,
|
||||
description: Some("AnyOf Value".to_string()),
|
||||
},
|
||||
mcp_tool(
|
||||
"value",
|
||||
"AnyOf Value",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"value": {"anyOf": [{"type": "string"}, {"type": "number"}]}
|
||||
}
|
||||
}),
|
||||
),
|
||||
)])),
|
||||
&[],
|
||||
)
|
||||
@@ -2482,46 +2460,33 @@ Examples of valid command strings:
|
||||
&tools_config,
|
||||
Some(HashMap::from([(
|
||||
"test_server/do_something_cool".to_string(),
|
||||
mcp_types::Tool {
|
||||
name: "do_something_cool".to_string(),
|
||||
input_schema: ToolInputSchema {
|
||||
properties: Some(serde_json::json!({
|
||||
"string_argument": {
|
||||
"type": "string",
|
||||
},
|
||||
"number_argument": {
|
||||
"type": "number",
|
||||
},
|
||||
mcp_tool(
|
||||
"do_something_cool",
|
||||
"Do something cool",
|
||||
serde_json::json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"string_argument": {"type": "string"},
|
||||
"number_argument": {"type": "number"},
|
||||
"object_argument": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"string_property": { "type": "string" },
|
||||
"number_property": { "type": "number" },
|
||||
"string_property": {"type": "string"},
|
||||
"number_property": {"type": "number"}
|
||||
},
|
||||
"required": [
|
||||
"string_property",
|
||||
"number_property",
|
||||
],
|
||||
"required": ["string_property", "number_property"],
|
||||
"additionalProperties": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"addtl_prop": { "type": "string" },
|
||||
"addtl_prop": {"type": "string"}
|
||||
},
|
||||
"required": [
|
||||
"addtl_prop",
|
||||
],
|
||||
"additionalProperties": false,
|
||||
},
|
||||
},
|
||||
})),
|
||||
required: None,
|
||||
r#type: "object".to_string(),
|
||||
},
|
||||
output_schema: None,
|
||||
title: None,
|
||||
annotations: None,
|
||||
description: Some("Do something cool".to_string()),
|
||||
},
|
||||
"required": ["addtl_prop"],
|
||||
"additionalProperties": false
|
||||
}
|
||||
}
|
||||
}
|
||||
}),
|
||||
),
|
||||
)])),
|
||||
&[],
|
||||
)
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::Path;
|
||||
|
||||
use serde::Serialize;
|
||||
|
||||
use crate::git_info::get_git_remote_urls_assume_git_repo;
|
||||
use crate::git_info::get_git_repo_root;
|
||||
use crate::git_info::get_head_commit_hash;
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct TurnMetadataWorkspace {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
associated_remote_urls: Option<BTreeMap<String, String>>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
latest_git_commit_hash: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct TurnMetadata {
|
||||
workspaces: BTreeMap<String, TurnMetadataWorkspace>,
|
||||
}
|
||||
|
||||
pub(crate) async fn build_turn_metadata_header(cwd: &Path) -> Option<String> {
|
||||
let repo_root = get_git_repo_root(cwd)?;
|
||||
|
||||
let (latest_git_commit_hash, associated_remote_urls) = tokio::join!(
|
||||
get_head_commit_hash(cwd),
|
||||
get_git_remote_urls_assume_git_repo(cwd)
|
||||
);
|
||||
if latest_git_commit_hash.is_none() && associated_remote_urls.is_none() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut workspaces = BTreeMap::new();
|
||||
workspaces.insert(
|
||||
repo_root.to_string_lossy().into_owned(),
|
||||
TurnMetadataWorkspace {
|
||||
associated_remote_urls,
|
||||
latest_git_commit_hash,
|
||||
},
|
||||
);
|
||||
serde_json::to_string(&TurnMetadata { workspaces }).ok()
|
||||
}
|
||||
@@ -63,6 +63,7 @@ pub fn user_shell_command_record_item(
|
||||
text: format_user_shell_command_record(command, exec_output, turn_context),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -65,6 +65,25 @@ pub fn elevated_setup_failure_details(_err: &anyhow::Error) -> Option<(String, S
|
||||
None
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
pub fn elevated_setup_failure_metric_name(err: &anyhow::Error) -> &'static str {
|
||||
if codex_windows_sandbox::extract_setup_failure(err).is_some_and(|failure| {
|
||||
matches!(
|
||||
failure.code,
|
||||
codex_windows_sandbox::SetupErrorCode::OrchestratorHelperLaunchCanceled
|
||||
)
|
||||
}) {
|
||||
"codex.windows_sandbox.elevated_setup_canceled"
|
||||
} else {
|
||||
"codex.windows_sandbox.elevated_setup_failure"
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
pub fn elevated_setup_failure_metric_name(_err: &anyhow::Error) -> &'static str {
|
||||
panic!("elevated_setup_failure_metric_name is only supported on Windows")
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
pub fn run_elevated_setup(
|
||||
policy: &SandboxPolicy,
|
||||
|
||||
@@ -102,7 +102,7 @@ async fn run_request(input: Vec<ResponseItem>) -> Value {
|
||||
SessionSource::Exec,
|
||||
TransportManager::new(),
|
||||
)
|
||||
.new_session();
|
||||
.new_session(None);
|
||||
|
||||
let mut prompt = Prompt::default();
|
||||
prompt.input = input;
|
||||
@@ -139,6 +139,7 @@ fn user_message(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -150,6 +151,7 @@ fn assistant_message(text: &str) -> ResponseItem {
|
||||
text: text.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -103,7 +103,7 @@ async fn run_stream_with_bytes(sse_body: &[u8]) -> Vec<ResponseEvent> {
|
||||
SessionSource::Exec,
|
||||
TransportManager::new(),
|
||||
)
|
||||
.new_session();
|
||||
.new_session(None);
|
||||
|
||||
let mut prompt = Prompt::default();
|
||||
prompt.input = vec![ResponseItem::Message {
|
||||
@@ -113,6 +113,7 @@ async fn run_stream_with_bytes(sse_body: &[u8]) -> Vec<ResponseEvent> {
|
||||
text: "hello".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}];
|
||||
|
||||
let mut stream = match client.stream(&prompt).await {
|
||||
|
||||
@@ -209,7 +209,7 @@ where
|
||||
use tokio::time::timeout;
|
||||
loop {
|
||||
// Allow a bit more time to accommodate async startup work (e.g. config IO, tool discovery)
|
||||
let ev = timeout(wait_time.max(Duration::from_secs(5)), codex.next_event())
|
||||
let ev = timeout(wait_time.max(Duration::from_secs(10)), codex.next_event())
|
||||
.await
|
||||
.expect("timeout waiting for event")
|
||||
.expect("stream ended unexpectedly");
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::process::Command;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
@@ -23,6 +24,7 @@ use core_test_support::load_default_config_for_test;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use futures::StreamExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use wiremock::matchers::header;
|
||||
|
||||
@@ -98,7 +100,7 @@ async fn responses_stream_includes_subagent_header_on_review() {
|
||||
session_source,
|
||||
TransportManager::new(),
|
||||
)
|
||||
.new_session();
|
||||
.new_session(None);
|
||||
|
||||
let mut prompt = Prompt::default();
|
||||
prompt.input = vec![ResponseItem::Message {
|
||||
@@ -108,6 +110,7 @@ async fn responses_stream_includes_subagent_header_on_review() {
|
||||
text: "hello".into(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}];
|
||||
|
||||
let mut stream = client_session.stream(&prompt).await.expect("stream failed");
|
||||
@@ -197,7 +200,7 @@ async fn responses_stream_includes_subagent_header_on_other() {
|
||||
session_source,
|
||||
TransportManager::new(),
|
||||
)
|
||||
.new_session();
|
||||
.new_session(None);
|
||||
|
||||
let mut prompt = Prompt::default();
|
||||
prompt.input = vec![ResponseItem::Message {
|
||||
@@ -207,6 +210,7 @@ async fn responses_stream_includes_subagent_header_on_other() {
|
||||
text: "hello".into(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}];
|
||||
|
||||
let mut stream = client_session.stream(&prompt).await.expect("stream failed");
|
||||
@@ -354,7 +358,7 @@ async fn responses_respects_model_info_overrides_from_config() {
|
||||
session_source,
|
||||
TransportManager::new(),
|
||||
)
|
||||
.new_session();
|
||||
.new_session(None);
|
||||
|
||||
let mut prompt = Prompt::default();
|
||||
prompt.input = vec![ResponseItem::Message {
|
||||
@@ -364,6 +368,7 @@ async fn responses_respects_model_info_overrides_from_config() {
|
||||
text: "hello".into(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}];
|
||||
|
||||
let mut stream = client.stream(&prompt).await.expect("stream failed");
|
||||
@@ -393,3 +398,197 @@ async fn responses_respects_model_info_overrides_from_config() {
|
||||
Some("detailed")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn responses_stream_includes_turn_metadata_header_for_git_workspace_e2e() {
|
||||
core_test_support::skip_if_no_network!();
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let response_body = responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_completed("resp-1"),
|
||||
]);
|
||||
let provider = ModelProviderInfo {
|
||||
name: "mock".into(),
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
env_key: None,
|
||||
env_key_instructions: None,
|
||||
experimental_bearer_token: None,
|
||||
wire_api: WireApi::Responses,
|
||||
query_params: None,
|
||||
http_headers: None,
|
||||
env_http_headers: None,
|
||||
request_max_retries: Some(0),
|
||||
stream_max_retries: Some(0),
|
||||
stream_idle_timeout_ms: Some(5_000),
|
||||
requires_openai_auth: false,
|
||||
supports_websockets: false,
|
||||
};
|
||||
|
||||
let codex_home = TempDir::new().expect("failed to create TempDir");
|
||||
let mut config = load_default_config_for_test(&codex_home).await;
|
||||
config.model_provider_id = provider.name.clone();
|
||||
config.model_provider = provider.clone();
|
||||
let effort = config.model_reasoning_effort;
|
||||
let summary = config.model_reasoning_summary;
|
||||
let model = ModelsManager::get_model_offline(config.model.as_deref());
|
||||
config.model = Some(model.clone());
|
||||
let config = Arc::new(config);
|
||||
|
||||
let conversation_id = ThreadId::new();
|
||||
let auth_mode = AuthMode::Chatgpt;
|
||||
let session_source =
|
||||
SessionSource::SubAgent(SubAgentSource::Other("turn-metadata-e2e".to_string()));
|
||||
let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config);
|
||||
let otel_manager = OtelManager::new(
|
||||
conversation_id,
|
||||
model.as_str(),
|
||||
model_info.slug.as_str(),
|
||||
None,
|
||||
Some("test@test.com".to_string()),
|
||||
Some(auth_mode),
|
||||
false,
|
||||
"test".to_string(),
|
||||
session_source.clone(),
|
||||
);
|
||||
|
||||
let client = ModelClient::new(
|
||||
Arc::clone(&config),
|
||||
None,
|
||||
model_info,
|
||||
otel_manager,
|
||||
provider,
|
||||
effort,
|
||||
summary,
|
||||
conversation_id,
|
||||
session_source,
|
||||
TransportManager::new(),
|
||||
);
|
||||
|
||||
let workspace = TempDir::new().expect("workspace tempdir");
|
||||
let cwd = workspace.path();
|
||||
|
||||
let mut prompt = Prompt::default();
|
||||
prompt.input = vec![ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".into(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "hello".into(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}];
|
||||
|
||||
let first_request = responses::mount_sse_once(&server, response_body.clone()).await;
|
||||
let mut first_session = client.new_session(Some(cwd.to_path_buf()));
|
||||
let mut first_stream = first_session
|
||||
.stream(&prompt)
|
||||
.await
|
||||
.expect("stream first turn");
|
||||
while let Some(event) = first_stream.next().await {
|
||||
if matches!(event, Ok(ResponseEvent::Completed { .. })) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert_eq!(
|
||||
first_request
|
||||
.single_request()
|
||||
.header("x-codex-turn-metadata"),
|
||||
None
|
||||
);
|
||||
|
||||
let git_config_global = cwd.join("empty-git-config");
|
||||
std::fs::write(&git_config_global, "").expect("write empty git config");
|
||||
let run_git = |args: &[&str]| {
|
||||
let output = Command::new("git")
|
||||
.env("GIT_CONFIG_GLOBAL", &git_config_global)
|
||||
.env("GIT_CONFIG_NOSYSTEM", "1")
|
||||
.args(args)
|
||||
.current_dir(cwd)
|
||||
.output()
|
||||
.expect("git command should run");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"git {:?} failed: stdout={} stderr={}",
|
||||
args,
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
output
|
||||
};
|
||||
|
||||
run_git(&["init"]);
|
||||
run_git(&["config", "user.name", "Test User"]);
|
||||
run_git(&["config", "user.email", "test@example.com"]);
|
||||
std::fs::write(cwd.join("README.md"), "hello").expect("write README");
|
||||
run_git(&["add", "."]);
|
||||
run_git(&["commit", "-m", "initial commit"]);
|
||||
run_git(&[
|
||||
"remote",
|
||||
"add",
|
||||
"origin",
|
||||
"https://github.com/openai/codex.git",
|
||||
]);
|
||||
|
||||
let expected_head = String::from_utf8(run_git(&["rev-parse", "HEAD"]).stdout)
|
||||
.expect("git rev-parse output should be valid UTF-8")
|
||||
.trim()
|
||||
.to_string();
|
||||
let expected_origin = String::from_utf8(run_git(&["remote", "get-url", "origin"]).stdout)
|
||||
.expect("git remote get-url output should be valid UTF-8")
|
||||
.trim()
|
||||
.to_string();
|
||||
|
||||
let repo_root = std::fs::canonicalize(cwd)
|
||||
.unwrap_or_else(|_| cwd.to_path_buf())
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5);
|
||||
loop {
|
||||
let request_recorder = responses::mount_sse_once(&server, response_body.clone()).await;
|
||||
let mut session = client.new_session(Some(cwd.to_path_buf()));
|
||||
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
|
||||
let mut stream = session.stream(&prompt).await.expect("stream post-git turn");
|
||||
while let Some(event) = stream.next().await {
|
||||
if matches!(event, Ok(ResponseEvent::Completed { .. })) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
let maybe_header = request_recorder
|
||||
.single_request()
|
||||
.header("x-codex-turn-metadata");
|
||||
if let Some(header_value) = maybe_header {
|
||||
let parsed: serde_json::Value = serde_json::from_str(&header_value)
|
||||
.expect("x-codex-turn-metadata should be valid JSON");
|
||||
let workspace = parsed
|
||||
.get("workspaces")
|
||||
.and_then(serde_json::Value::as_object)
|
||||
.and_then(|workspaces| workspaces.get(&repo_root))
|
||||
.expect("metadata should include cwd repo root workspace entry");
|
||||
|
||||
assert_eq!(
|
||||
workspace
|
||||
.get("latest_git_commit_hash")
|
||||
.and_then(serde_json::Value::as_str),
|
||||
Some(expected_head.as_str())
|
||||
);
|
||||
assert_eq!(
|
||||
workspace
|
||||
.get("associated_remote_urls")
|
||||
.and_then(serde_json::Value::as_object)
|
||||
.and_then(|remotes| remotes.get("origin"))
|
||||
.and_then(serde_json::Value::as_str),
|
||||
Some(expected_origin.as_str())
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
if tokio::time::Instant::now() >= deadline {
|
||||
break;
|
||||
}
|
||||
tokio::time::sleep(std::time::Duration::from_millis(25)).await;
|
||||
}
|
||||
|
||||
panic!("x-codex-turn-metadata was never observed within 5s after git setup");
|
||||
}
|
||||
|
||||
@@ -29,6 +29,7 @@ use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::Settings;
|
||||
use codex_protocol::config_types::Verbosity;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::MessagePhase;
|
||||
use codex_protocol::models::ReasoningItemContent;
|
||||
use codex_protocol::models::ReasoningItemReasoningSummary;
|
||||
use codex_protocol::models::WebSearchAction;
|
||||
@@ -197,6 +198,7 @@ async fn resume_includes_initial_messages_and_sends_prior_items() {
|
||||
text: "resumed user message".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
let prior_user_json = serde_json::to_value(&prior_user).unwrap();
|
||||
writeln!(
|
||||
@@ -218,6 +220,7 @@ async fn resume_includes_initial_messages_and_sends_prior_items() {
|
||||
text: "resumed system instruction".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
let prior_system_json = serde_json::to_value(&prior_system).unwrap();
|
||||
writeln!(
|
||||
@@ -239,6 +242,7 @@ async fn resume_includes_initial_messages_and_sends_prior_items() {
|
||||
text: "resumed assistant message".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: Some(MessagePhase::Commentary),
|
||||
};
|
||||
let prior_item_json = serde_json::to_value(&prior_item).unwrap();
|
||||
writeln!(
|
||||
@@ -318,6 +322,25 @@ async fn resume_includes_initial_messages_and_sends_prior_items() {
|
||||
.iter()
|
||||
.position(|(role, text)| role == "assistant" && text == "resumed assistant message")
|
||||
.expect("prior assistant message");
|
||||
let prior_assistant = input
|
||||
.iter()
|
||||
.find(|item| {
|
||||
item.get("role").and_then(|role| role.as_str()) == Some("assistant")
|
||||
&& item
|
||||
.get("content")
|
||||
.and_then(|content| content.as_array())
|
||||
.and_then(|content| content.first())
|
||||
.and_then(|entry| entry.get("text"))
|
||||
.and_then(|text| text.as_str())
|
||||
== Some("resumed assistant message")
|
||||
})
|
||||
.expect("resumed assistant message request item");
|
||||
assert_eq!(
|
||||
prior_assistant
|
||||
.get("phase")
|
||||
.and_then(|phase| phase.as_str()),
|
||||
Some("commentary")
|
||||
);
|
||||
let pos_permissions = messages
|
||||
.iter()
|
||||
.position(|(role, text)| role == "developer" && text.contains("<permissions instructions>"))
|
||||
@@ -1190,7 +1213,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() {
|
||||
SessionSource::Exec,
|
||||
TransportManager::new(),
|
||||
)
|
||||
.new_session();
|
||||
.new_session(None);
|
||||
|
||||
let mut prompt = Prompt::default();
|
||||
prompt.input.push(ResponseItem::Reasoning {
|
||||
@@ -1210,6 +1233,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() {
|
||||
text: "message".into(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
});
|
||||
prompt.input.push(ResponseItem::WebSearchCall {
|
||||
id: Some("web-search-id".into()),
|
||||
|
||||
@@ -53,7 +53,7 @@ async fn responses_websocket_streams_request() {
|
||||
.await;
|
||||
|
||||
let harness = websocket_harness(&server).await;
|
||||
let mut session = harness.client.new_session();
|
||||
let mut session = harness.client.new_session(None);
|
||||
let prompt = prompt_with_input(vec![message_item("hello")]);
|
||||
|
||||
stream_until_complete(&mut session, &prompt).await;
|
||||
@@ -83,7 +83,7 @@ async fn responses_websocket_emits_websocket_telemetry_events() {
|
||||
|
||||
let harness = websocket_harness(&server).await;
|
||||
harness.otel_manager.reset_runtime_metrics();
|
||||
let mut session = harness.client.new_session();
|
||||
let mut session = harness.client.new_session(None);
|
||||
let prompt = prompt_with_input(vec![message_item("hello")]);
|
||||
|
||||
stream_until_complete(&mut session, &prompt).await;
|
||||
@@ -113,7 +113,7 @@ async fn responses_websocket_emits_reasoning_included_event() {
|
||||
.await;
|
||||
|
||||
let harness = websocket_harness(&server).await;
|
||||
let mut session = harness.client.new_session();
|
||||
let mut session = harness.client.new_session(None);
|
||||
let prompt = prompt_with_input(vec![message_item("hello")]);
|
||||
|
||||
let mut stream = session
|
||||
@@ -147,7 +147,7 @@ async fn responses_websocket_appends_on_prefix() {
|
||||
.await;
|
||||
|
||||
let harness = websocket_harness(&server).await;
|
||||
let mut session = harness.client.new_session();
|
||||
let mut session = harness.client.new_session(None);
|
||||
let prompt_one = prompt_with_input(vec![message_item("hello")]);
|
||||
let prompt_two = prompt_with_input(vec![message_item("hello"), message_item("second")]);
|
||||
|
||||
@@ -183,7 +183,7 @@ async fn responses_websocket_creates_on_non_prefix() {
|
||||
.await;
|
||||
|
||||
let harness = websocket_harness(&server).await;
|
||||
let mut session = harness.client.new_session();
|
||||
let mut session = harness.client.new_session(None);
|
||||
let prompt_one = prompt_with_input(vec![message_item("hello")]);
|
||||
let prompt_two = prompt_with_input(vec![message_item("different")]);
|
||||
|
||||
@@ -211,6 +211,7 @@ fn message_item(text: &str) -> ResponseItem {
|
||||
role: "user".into(),
|
||||
content: vec![ContentItem::InputText { text: text.into() }],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1452,6 +1452,7 @@ async fn auto_compact_runs_after_resume_when_token_usage_is_over_limit() {
|
||||
text: remote_summary.to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
codex_protocol::models::ResponseItem::Compaction {
|
||||
encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(),
|
||||
@@ -2275,6 +2276,7 @@ async fn auto_compact_counts_encrypted_reasoning_before_last_user() {
|
||||
text: "REMOTE_COMPACT_SUMMARY".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
codex_protocol::models::ResponseItem::Compaction {
|
||||
encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(),
|
||||
@@ -2395,6 +2397,7 @@ async fn auto_compact_runs_when_reasoning_header_clears_between_turns() {
|
||||
text: "REMOTE_COMPACT_SUMMARY".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
codex_protocol::models::ResponseItem::Compaction {
|
||||
encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(),
|
||||
|
||||
@@ -62,6 +62,7 @@ async fn remote_compact_replaces_history_for_followups() -> Result<()> {
|
||||
text: "REMOTE_COMPACTED_SUMMARY".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Compaction {
|
||||
encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(),
|
||||
@@ -184,6 +185,7 @@ async fn remote_compact_runs_automatically() -> Result<()> {
|
||||
text: "REMOTE_COMPACTED_SUMMARY".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Compaction {
|
||||
encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(),
|
||||
@@ -220,6 +222,134 @@ async fn remote_compact_runs_automatically() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_compact_trims_function_call_history_to_fit_context_window() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let first_user_message = "turn with retained shell call";
|
||||
let second_user_message = "turn with trimmed shell call";
|
||||
let retained_call_id = "retained-call";
|
||||
let trimmed_call_id = "trimmed-call";
|
||||
let retained_command = "echo retained-shell-output";
|
||||
let trimmed_command = "yes x | head -n 3000";
|
||||
|
||||
let harness = TestCodexHarness::with_builder(
|
||||
test_codex()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::RemoteCompaction);
|
||||
config.model_context_window = Some(2_000);
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
let codex = harness.test().codex.clone();
|
||||
|
||||
let response_log = responses::mount_sse_sequence(
|
||||
harness.server(),
|
||||
vec![
|
||||
sse(vec![
|
||||
responses::ev_shell_command_call(retained_call_id, retained_command),
|
||||
responses::ev_completed("retained-call-response"),
|
||||
]),
|
||||
sse(vec![
|
||||
responses::ev_assistant_message("retained-assistant", "retained complete"),
|
||||
responses::ev_completed("retained-final-response"),
|
||||
]),
|
||||
sse(vec![
|
||||
responses::ev_shell_command_call(trimmed_call_id, trimmed_command),
|
||||
responses::ev_completed("trimmed-call-response"),
|
||||
]),
|
||||
sse(vec![responses::ev_completed("trimmed-final-response")]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: first_user_message.into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: second_user_message.into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let compact_mock =
|
||||
responses::mount_compact_json_once(harness.server(), serde_json::json!({ "output": [] }))
|
||||
.await;
|
||||
|
||||
codex.submit(Op::Compact).await?;
|
||||
wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
assert!(
|
||||
response_log
|
||||
.function_call_output_text(retained_call_id)
|
||||
.is_some(),
|
||||
"expected retained shell call to produce function_call_output before compaction"
|
||||
);
|
||||
assert!(
|
||||
response_log
|
||||
.function_call_output_text(trimmed_call_id)
|
||||
.is_some(),
|
||||
"expected trimmed shell call to produce function_call_output before compaction"
|
||||
);
|
||||
|
||||
let compact_request = compact_mock.single_request();
|
||||
let user_messages = compact_request.message_input_texts("user");
|
||||
assert!(
|
||||
user_messages
|
||||
.iter()
|
||||
.any(|message| message == first_user_message),
|
||||
"expected compact request to retain earlier user history"
|
||||
);
|
||||
assert!(
|
||||
user_messages
|
||||
.iter()
|
||||
.any(|message| message == second_user_message),
|
||||
"expected compact request to retain the user boundary message"
|
||||
);
|
||||
|
||||
assert!(
|
||||
compact_request.has_function_call(retained_call_id)
|
||||
&& compact_request
|
||||
.function_call_output_text(retained_call_id)
|
||||
.is_some(),
|
||||
"expected compact request to keep the older function call/result pair"
|
||||
);
|
||||
assert!(
|
||||
!compact_request.has_function_call(trimmed_call_id)
|
||||
&& compact_request
|
||||
.function_call_output_text(trimmed_call_id)
|
||||
.is_none(),
|
||||
"expected compact request to drop the trailing function call/result pair past the boundary"
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
compact_request.inputs_of_type("function_call").len(),
|
||||
1,
|
||||
"expected exactly one function call after trimming"
|
||||
);
|
||||
assert_eq!(
|
||||
compact_request.inputs_of_type("function_call_output").len(),
|
||||
1,
|
||||
"expected exactly one function call output after trimming"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_manual_compact_emits_context_compaction_items() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -251,6 +381,7 @@ async fn remote_manual_compact_emits_context_compaction_items() -> Result<()> {
|
||||
text: "REMOTE_COMPACTED_SUMMARY".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Compaction {
|
||||
encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(),
|
||||
@@ -352,6 +483,7 @@ async fn remote_compact_persists_replacement_history_in_rollout() -> Result<()>
|
||||
text: "COMPACTED_USER_SUMMARY".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Compaction {
|
||||
encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(),
|
||||
@@ -363,6 +495,7 @@ async fn remote_compact_persists_replacement_history_in_rollout() -> Result<()>
|
||||
text: "COMPACTED_ASSISTANT_NOTE".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
];
|
||||
let compact_mock = responses::mount_compact_json_once(
|
||||
|
||||
@@ -158,6 +158,7 @@ async fn copy_paste_local_image_persists_rollout_request_shape() -> anyhow::Resu
|
||||
},
|
||||
],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
|
||||
assert_eq!(actual, expected);
|
||||
@@ -239,6 +240,7 @@ async fn drag_drop_image_persists_rollout_request_shape() -> anyhow::Result<()>
|
||||
},
|
||||
],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
|
||||
assert_eq!(actual, expected);
|
||||
|
||||
@@ -7,6 +7,7 @@ use codex_protocol::openai_models::ModelPreset;
|
||||
use codex_protocol::openai_models::ModelUpgrade;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::default_input_modalities;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use indoc::indoc;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -99,6 +100,7 @@ fn gpt_52_codex() -> ModelPreset {
|
||||
upgrade: None,
|
||||
show_in_picker: true,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -142,6 +144,7 @@ fn gpt_5_1_codex_max() -> ModelPreset {
|
||||
)),
|
||||
show_in_picker: true,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -177,6 +180,7 @@ fn gpt_5_1_codex_mini() -> ModelPreset {
|
||||
)),
|
||||
show_in_picker: true,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -222,6 +226,7 @@ fn gpt_5_2() -> ModelPreset {
|
||||
)),
|
||||
show_in_picker: true,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -255,6 +260,7 @@ fn bengalfox() -> ModelPreset {
|
||||
upgrade: None,
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -288,6 +294,7 @@ fn boomslang() -> ModelPreset {
|
||||
upgrade: None,
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -327,6 +334,7 @@ fn gpt_5_codex() -> ModelPreset {
|
||||
)),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -362,6 +370,7 @@ fn gpt_5_codex_mini() -> ModelPreset {
|
||||
)),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -401,6 +410,7 @@ fn gpt_5_1_codex() -> ModelPreset {
|
||||
)),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -444,6 +454,7 @@ fn gpt_5() -> ModelPreset {
|
||||
)),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -483,6 +494,7 @@ fn gpt_5_1() -> ModelPreset {
|
||||
)),
|
||||
show_in_picker: false,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
use codex_protocol::openai_models::default_input_modalities;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
@@ -36,6 +37,9 @@ use wiremock::MockServer;
|
||||
const ETAG: &str = "\"models-etag-ttl\"";
|
||||
const CACHE_FILE: &str = "models_cache.json";
|
||||
const REMOTE_MODEL: &str = "codex-test-ttl";
|
||||
const VERSIONED_MODEL: &str = "codex-test-versioned";
|
||||
const MISSING_VERSION_MODEL: &str = "codex-test-missing-version";
|
||||
const DIFFERENT_VERSION_MODEL: &str = "codex-test-different-version";
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn renews_cache_ttl_on_matching_models_etag() -> Result<()> {
|
||||
@@ -131,11 +135,157 @@ async fn renews_cache_ttl_on_matching_models_etag() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn uses_cache_when_version_matches() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
let cached_model = test_remote_model(VERSIONED_MODEL, 1);
|
||||
let models_mock = responses::mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![test_remote_model("remote", 2)],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing());
|
||||
builder = builder
|
||||
.with_pre_build_hook(move |home| {
|
||||
let cache = ModelsCache {
|
||||
fetched_at: Utc::now(),
|
||||
etag: None,
|
||||
client_version: Some(codex_core::models_manager::client_version_to_whole()),
|
||||
models: vec![cached_model],
|
||||
};
|
||||
let cache_path = home.join(CACHE_FILE);
|
||||
write_cache_sync(&cache_path, &cache).expect("write cache");
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
config.model_provider.request_max_retries = Some(0);
|
||||
});
|
||||
|
||||
let test = builder.build(&server).await?;
|
||||
let models_manager = test.thread_manager.get_models_manager();
|
||||
let models = models_manager
|
||||
.list_models(&test.config, RefreshStrategy::OnlineIfUncached)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
models.iter().any(|preset| preset.model == VERSIONED_MODEL),
|
||||
"expected cached model"
|
||||
);
|
||||
assert_eq!(
|
||||
models_mock.requests().len(),
|
||||
0,
|
||||
"/models should not be called when cache version matches"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn refreshes_when_cache_version_missing() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
let cached_model = test_remote_model(MISSING_VERSION_MODEL, 1);
|
||||
let models_mock = responses::mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![test_remote_model("remote-missing", 2)],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing());
|
||||
builder = builder
|
||||
.with_pre_build_hook(move |home| {
|
||||
let cache = ModelsCache {
|
||||
fetched_at: Utc::now(),
|
||||
etag: None,
|
||||
client_version: None,
|
||||
models: vec![cached_model],
|
||||
};
|
||||
let cache_path = home.join(CACHE_FILE);
|
||||
write_cache_sync(&cache_path, &cache).expect("write cache");
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
config.model_provider.request_max_retries = Some(0);
|
||||
});
|
||||
|
||||
let test = builder.build(&server).await?;
|
||||
let models_manager = test.thread_manager.get_models_manager();
|
||||
let models = models_manager
|
||||
.list_models(&test.config, RefreshStrategy::OnlineIfUncached)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
models.iter().any(|preset| preset.model == "remote-missing"),
|
||||
"expected refreshed models"
|
||||
);
|
||||
assert_eq!(
|
||||
models_mock.requests().len(),
|
||||
1,
|
||||
"/models should be called when cache version is missing"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn refreshes_when_cache_version_differs() -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
let cached_model = test_remote_model(DIFFERENT_VERSION_MODEL, 1);
|
||||
let models_mock = responses::mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![test_remote_model("remote-different", 2)],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut builder = test_codex().with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing());
|
||||
builder = builder
|
||||
.with_pre_build_hook(move |home| {
|
||||
let client_version = codex_core::models_manager::client_version_to_whole();
|
||||
let cache = ModelsCache {
|
||||
fetched_at: Utc::now(),
|
||||
etag: None,
|
||||
client_version: Some(format!("{client_version}-diff")),
|
||||
models: vec![cached_model],
|
||||
};
|
||||
let cache_path = home.join(CACHE_FILE);
|
||||
write_cache_sync(&cache_path, &cache).expect("write cache");
|
||||
})
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
config.model_provider.request_max_retries = Some(0);
|
||||
});
|
||||
|
||||
let test = builder.build(&server).await?;
|
||||
let models_manager = test.thread_manager.get_models_manager();
|
||||
let models = models_manager
|
||||
.list_models(&test.config, RefreshStrategy::OnlineIfUncached)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
models
|
||||
.iter()
|
||||
.any(|preset| preset.model == "remote-different"),
|
||||
"expected refreshed models"
|
||||
);
|
||||
assert_eq!(
|
||||
models_mock.requests().len(),
|
||||
1,
|
||||
"/models should be called when cache version differs"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn rewrite_cache_timestamp(path: &Path, fetched_at: DateTime<Utc>) -> Result<()> {
|
||||
let mut cache = read_cache(path).await?;
|
||||
cache.fetched_at = fetched_at;
|
||||
let contents = serde_json::to_vec_pretty(&cache)?;
|
||||
tokio::fs::write(path, contents).await?;
|
||||
write_cache(path, &cache).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -145,11 +295,25 @@ async fn read_cache(path: &Path) -> Result<ModelsCache> {
|
||||
Ok(cache)
|
||||
}
|
||||
|
||||
async fn write_cache(path: &Path, cache: &ModelsCache) -> Result<()> {
|
||||
let contents = serde_json::to_vec_pretty(cache)?;
|
||||
tokio::fs::write(path, contents).await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_cache_sync(path: &Path, cache: &ModelsCache) -> Result<()> {
|
||||
let contents = serde_json::to_vec_pretty(cache)?;
|
||||
std::fs::write(path, contents)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
struct ModelsCache {
|
||||
fetched_at: DateTime<Utc>,
|
||||
#[serde(default)]
|
||||
etag: Option<String>,
|
||||
#[serde(default)]
|
||||
client_version: Option<String>,
|
||||
models: Vec<ModelInfo>,
|
||||
}
|
||||
|
||||
@@ -186,5 +350,6 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo {
|
||||
auto_compact_token_limit: None,
|
||||
effective_context_window_percent: 95,
|
||||
experimental_supported_tools: Vec::new(),
|
||||
input_modalities: default_input_modalities(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
use codex_protocol::openai_models::default_input_modalities;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use core_test_support::responses::ev_completed;
|
||||
@@ -512,6 +513,7 @@ async fn ignores_remote_personality_if_remote_models_disabled() -> anyhow::Resul
|
||||
auto_compact_token_limit: None,
|
||||
effective_context_window_percent: 95,
|
||||
experimental_supported_tools: Vec::new(),
|
||||
input_modalities: default_input_modalities(),
|
||||
};
|
||||
|
||||
let _models_mock = mount_models_once(
|
||||
@@ -627,6 +629,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
||||
auto_compact_token_limit: None,
|
||||
effective_context_window_percent: 95,
|
||||
experimental_supported_tools: Vec::new(),
|
||||
input_modalities: default_input_modalities(),
|
||||
};
|
||||
|
||||
let _models_mock = mount_models_once(
|
||||
@@ -737,6 +740,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
auto_compact_token_limit: None,
|
||||
effective_context_window_percent: 95,
|
||||
experimental_supported_tools: Vec::new(),
|
||||
input_modalities: default_input_modalities(),
|
||||
};
|
||||
|
||||
let _models_mock = mount_models_once(
|
||||
|
||||
@@ -25,6 +25,7 @@ use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
use codex_protocol::openai_models::default_input_modalities;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
@@ -76,6 +77,7 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> {
|
||||
shell_type: ConfigShellToolType::UnifiedExec,
|
||||
visibility: ModelVisibility::List,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
priority: 1,
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
@@ -313,6 +315,7 @@ async fn remote_models_apply_remote_base_instructions() -> Result<()> {
|
||||
shell_type: ConfigShellToolType::ShellCommand,
|
||||
visibility: ModelVisibility::List,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
priority: 1,
|
||||
upgrade: None,
|
||||
base_instructions: remote_base.to_string(),
|
||||
@@ -787,6 +790,7 @@ fn test_remote_model_with_policy(
|
||||
shell_type: ConfigShellToolType::ShellCommand,
|
||||
visibility,
|
||||
supported_in_api: true,
|
||||
input_modalities: default_input_modalities(),
|
||||
priority,
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
|
||||
@@ -530,6 +530,7 @@ async fn review_input_isolated_from_parent_history() {
|
||||
text: "parent: earlier user message".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
let user_json = serde_json::to_value(&user).unwrap();
|
||||
let user_line = serde_json::json!({
|
||||
@@ -549,6 +550,7 @@ async fn review_input_isolated_from_parent_history() {
|
||||
text: "parent: assistant reply".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
};
|
||||
let assistant_json = serde_json::to_value(&assistant).unwrap();
|
||||
let assistant_line = serde_json::json!({
|
||||
|
||||
@@ -26,7 +26,6 @@ use core_test_support::skip_if_no_network;
|
||||
use core_test_support::stdio_server_bin;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use mcp_types::ContentBlock;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use serial_test::serial;
|
||||
@@ -307,14 +306,10 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> {
|
||||
let base64_only = OPENAI_PNG
|
||||
.strip_prefix("data:image/png;base64,")
|
||||
.expect("data url prefix");
|
||||
match &result.content[0] {
|
||||
ContentBlock::ImageContent(img) => {
|
||||
assert_eq!(img.mime_type, "image/png");
|
||||
assert_eq!(img.r#type, "image");
|
||||
assert_eq!(img.data, base64_only);
|
||||
}
|
||||
other => panic!("expected image content, got {other:?}"),
|
||||
}
|
||||
let entry = result.content[0].as_object().expect("content object");
|
||||
assert_eq!(entry.get("type"), Some(&json!("image")));
|
||||
assert_eq!(entry.get("mimeType"), Some(&json!("image/png")));
|
||||
assert_eq!(entry.get("data"), Some(&json!(base64_only)));
|
||||
|
||||
wait_for_event(&fixture.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use anyhow::Result;
|
||||
use codex_core::features::Feature;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
@@ -74,6 +75,28 @@ async fn backfill_scans_existing_rollouts() -> Result<()> {
|
||||
let rollout_rel_path = format!("sessions/2026/01/27/rollout-2026-01-27T12-00-00-{uuid}.jsonl");
|
||||
let rollout_rel_path_for_hook = rollout_rel_path.clone();
|
||||
|
||||
let dynamic_tools = vec![
|
||||
DynamicToolSpec {
|
||||
name: "geo_lookup".to_string(),
|
||||
description: "lookup a city".to_string(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"required": ["city"],
|
||||
"properties": { "city": { "type": "string" } }
|
||||
}),
|
||||
},
|
||||
DynamicToolSpec {
|
||||
name: "weather_lookup".to_string(),
|
||||
description: "lookup weather".to_string(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"required": ["zip"],
|
||||
"properties": { "zip": { "type": "string" } }
|
||||
}),
|
||||
},
|
||||
];
|
||||
let dynamic_tools_for_hook = dynamic_tools.clone();
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_pre_build_hook(move |codex_home| {
|
||||
let rollout_path = codex_home.join(&rollout_rel_path_for_hook);
|
||||
@@ -81,7 +104,6 @@ async fn backfill_scans_existing_rollouts() -> Result<()> {
|
||||
.parent()
|
||||
.expect("rollout path should have parent");
|
||||
fs::create_dir_all(parent).expect("should create rollout directory");
|
||||
|
||||
let session_meta_line = SessionMetaLine {
|
||||
meta: SessionMeta {
|
||||
id: thread_id,
|
||||
@@ -93,7 +115,7 @@ async fn backfill_scans_existing_rollouts() -> Result<()> {
|
||||
source: SessionSource::default(),
|
||||
model_provider: None,
|
||||
base_instructions: None,
|
||||
dynamic_tools: None,
|
||||
dynamic_tools: Some(dynamic_tools_for_hook),
|
||||
},
|
||||
git: None,
|
||||
};
|
||||
@@ -155,6 +177,17 @@ async fn backfill_scans_existing_rollouts() -> Result<()> {
|
||||
assert_eq!(metadata.model_provider, default_provider);
|
||||
assert!(metadata.has_user_event);
|
||||
|
||||
let mut stored_tools = None;
|
||||
for _ in 0..40 {
|
||||
stored_tools = db.get_dynamic_tools(thread_id).await?;
|
||||
if stored_tools.is_some() {
|
||||
break;
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(25)).await;
|
||||
}
|
||||
let stored_tools = stored_tools.expect("dynamic tools should be stored");
|
||||
assert_eq!(stored_tools, dynamic_tools);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user