Merge branch 'main' into dev/aaronl/dynamic-tool-content-items

This commit is contained in:
Owen Lin
2026-02-03 13:29:24 -08:00
895 changed files with 109659 additions and 13786 deletions
+1 -1
View File
@@ -232,7 +232,7 @@ mod tests {
async fn on_event_updates_status_from_task_started() {
let status = agent_status_from_event(&EventMsg::TurnStarted(TurnStartedEvent {
model_context_window: None,
collaboration_mode_kind: ModeKind::Custom,
collaboration_mode_kind: ModeKind::Default,
}));
assert_eq!(status, Some(AgentStatus::Running));
}
+331
View File
@@ -0,0 +1,331 @@
use crate::AuthManager;
use crate::config::Config;
use crate::default_client::create_client;
use crate::git_info::collect_git_info;
use crate::git_info::get_git_repo_root;
use codex_protocol::protocol::SkillScope;
use serde::Serialize;
use sha1::Digest;
use sha1::Sha1;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
use tokio::sync::mpsc;
#[derive(Clone)]
pub(crate) struct TrackEventsContext {
pub(crate) model_slug: String,
pub(crate) thread_id: String,
}
pub(crate) fn build_track_events_context(
model_slug: String,
thread_id: String,
) -> TrackEventsContext {
TrackEventsContext {
model_slug,
thread_id,
}
}
pub(crate) struct SkillInvocation {
pub(crate) skill_name: String,
pub(crate) skill_scope: SkillScope,
pub(crate) skill_path: PathBuf,
}
#[derive(Clone)]
pub(crate) struct AnalyticsEventsQueue {
sender: mpsc::Sender<TrackEventsJob>,
}
pub(crate) struct AnalyticsEventsClient {
queue: AnalyticsEventsQueue,
config: Arc<Config>,
}
impl AnalyticsEventsQueue {
pub(crate) fn new(auth_manager: Arc<AuthManager>) -> Self {
let (sender, mut receiver) = mpsc::channel(ANALYTICS_EVENTS_QUEUE_SIZE);
tokio::spawn(async move {
while let Some(job) = receiver.recv().await {
send_track_skill_invocations(&auth_manager, job).await;
}
});
Self { sender }
}
fn try_send(&self, job: TrackEventsJob) {
if self.sender.try_send(job).is_err() {
//TODO: add a metric for this
tracing::warn!("dropping skill analytics events: queue is full");
}
}
}
impl AnalyticsEventsClient {
pub(crate) fn new(config: Arc<Config>, auth_manager: Arc<AuthManager>) -> Self {
Self {
queue: AnalyticsEventsQueue::new(Arc::clone(&auth_manager)),
config,
}
}
pub(crate) fn track_skill_invocations(
&self,
tracking: TrackEventsContext,
invocations: Vec<SkillInvocation>,
) {
track_skill_invocations(
&self.queue,
Arc::clone(&self.config),
Some(tracking),
invocations,
);
}
}
struct TrackEventsJob {
config: Arc<Config>,
tracking: TrackEventsContext,
invocations: Vec<SkillInvocation>,
}
const ANALYTICS_EVENTS_QUEUE_SIZE: usize = 256;
const ANALYTICS_EVENTS_TIMEOUT: Duration = Duration::from_secs(10);
#[derive(Serialize)]
struct TrackEventsRequest {
events: Vec<TrackEvent>,
}
#[derive(Serialize)]
struct TrackEvent {
event_type: &'static str,
skill_id: String,
skill_name: String,
event_params: TrackEventParams,
}
#[derive(Serialize)]
struct TrackEventParams {
product_client_id: Option<String>,
skill_scope: Option<String>,
repo_url: Option<String>,
thread_id: Option<String>,
invoke_type: Option<String>,
model_slug: Option<String>,
}
pub(crate) fn track_skill_invocations(
queue: &AnalyticsEventsQueue,
config: Arc<Config>,
tracking: Option<TrackEventsContext>,
invocations: Vec<SkillInvocation>,
) {
if config.analytics_enabled == Some(false) {
return;
}
let Some(tracking) = tracking else {
return;
};
if invocations.is_empty() {
return;
}
let job = TrackEventsJob {
config,
tracking,
invocations,
};
queue.try_send(job);
}
async fn send_track_skill_invocations(auth_manager: &AuthManager, job: TrackEventsJob) {
let TrackEventsJob {
config,
tracking,
invocations,
} = job;
let Some(auth) = auth_manager.auth().await else {
return;
};
if !auth.is_chatgpt_auth() {
return;
}
let access_token = match auth.get_token() {
Ok(token) => token,
Err(_) => return,
};
let Some(account_id) = auth.get_account_id() else {
return;
};
let mut events = Vec::with_capacity(invocations.len());
for invocation in invocations {
let skill_scope = match invocation.skill_scope {
SkillScope::User => "user",
SkillScope::Repo => "repo",
SkillScope::System => "system",
SkillScope::Admin => "admin",
};
let repo_root = get_git_repo_root(invocation.skill_path.as_path());
let repo_url = if let Some(root) = repo_root.as_ref() {
collect_git_info(root)
.await
.and_then(|info| info.repository_url)
} else {
None
};
let skill_id = skill_id_for_local_skill(
repo_url.as_deref(),
repo_root.as_deref(),
invocation.skill_path.as_path(),
invocation.skill_name.as_str(),
);
events.push(TrackEvent {
event_type: "skill_invocation",
skill_id,
skill_name: invocation.skill_name.clone(),
event_params: TrackEventParams {
thread_id: Some(tracking.thread_id.clone()),
invoke_type: Some("explicit".to_string()),
model_slug: Some(tracking.model_slug.clone()),
product_client_id: Some(crate::default_client::originator().value),
repo_url,
skill_scope: Some(skill_scope.to_string()),
},
});
}
let base_url = config.chatgpt_base_url.trim_end_matches('/');
let url = format!("{base_url}/codex/analytics-events/events");
let payload = TrackEventsRequest { events };
let response = create_client()
.post(&url)
.timeout(ANALYTICS_EVENTS_TIMEOUT)
.bearer_auth(&access_token)
.header("chatgpt-account-id", &account_id)
.header("Content-Type", "application/json")
.json(&payload)
.send()
.await;
match response {
Ok(response) if response.status().is_success() => {}
Ok(response) => {
let status = response.status();
let body = response.text().await.unwrap_or_default();
tracing::warn!("events failed with status {status}: {body}");
}
Err(err) => {
tracing::warn!("failed to send events request: {err}");
}
}
}
fn skill_id_for_local_skill(
repo_url: Option<&str>,
repo_root: Option<&Path>,
skill_path: &Path,
skill_name: &str,
) -> String {
let path = normalize_path_for_skill_id(repo_url, repo_root, skill_path);
let prefix = if let Some(url) = repo_url {
format!("repo_{url}")
} else {
"personal".to_string()
};
let raw_id = format!("{prefix}_{path}_{skill_name}");
let mut hasher = Sha1::new();
hasher.update(raw_id.as_bytes());
format!("{:x}", hasher.finalize())
}
/// Returns a normalized path for skill ID construction.
///
/// - Repo-scoped skills use a path relative to the repo root.
/// - User/admin/system skills use an absolute path.
fn normalize_path_for_skill_id(
repo_url: Option<&str>,
repo_root: Option<&Path>,
skill_path: &Path,
) -> String {
let resolved_path =
std::fs::canonicalize(skill_path).unwrap_or_else(|_| skill_path.to_path_buf());
match (repo_url, repo_root) {
(Some(_), Some(root)) => {
let resolved_root = std::fs::canonicalize(root).unwrap_or_else(|_| root.to_path_buf());
resolved_path
.strip_prefix(&resolved_root)
.unwrap_or(resolved_path.as_path())
.to_string_lossy()
.replace('\\', "/")
}
_ => resolved_path.to_string_lossy().replace('\\', "/"),
}
}
#[cfg(test)]
mod tests {
use super::normalize_path_for_skill_id;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
fn expected_absolute_path(path: &PathBuf) -> String {
std::fs::canonicalize(path)
.unwrap_or_else(|_| path.to_path_buf())
.to_string_lossy()
.replace('\\', "/")
}
#[test]
fn normalize_path_for_skill_id_repo_scoped_uses_relative_path() {
let repo_root = PathBuf::from("/repo/root");
let skill_path = PathBuf::from("/repo/root/.codex/skills/doc/SKILL.md");
let path = normalize_path_for_skill_id(
Some("https://example.com/repo.git"),
Some(repo_root.as_path()),
skill_path.as_path(),
);
assert_eq!(path, ".codex/skills/doc/SKILL.md");
}
#[test]
fn normalize_path_for_skill_id_user_scoped_uses_absolute_path() {
let skill_path = PathBuf::from("/Users/abc/.codex/skills/doc/SKILL.md");
let path = normalize_path_for_skill_id(None, None, skill_path.as_path());
let expected = expected_absolute_path(&skill_path);
assert_eq!(path, expected);
}
#[test]
fn normalize_path_for_skill_id_admin_scoped_uses_absolute_path() {
let skill_path = PathBuf::from("/etc/codex/skills/doc/SKILL.md");
let path = normalize_path_for_skill_id(None, None, skill_path.as_path());
let expected = expected_absolute_path(&skill_path);
assert_eq!(path, expected);
}
#[test]
fn normalize_path_for_skill_id_repo_root_not_in_skill_path_uses_absolute_path() {
let repo_root = PathBuf::from("/repo/root");
let skill_path = PathBuf::from("/other/path/.codex/skills/doc/SKILL.md");
let path = normalize_path_for_skill_id(
Some("https://example.com/repo.git"),
Some(repo_root.as_path()),
skill_path.as_path(),
);
let expected = expected_absolute_path(&skill_path);
assert_eq!(path, expected);
}
}
+21 -8
View File
@@ -3,6 +3,7 @@ use chrono::Utc;
use codex_api::AuthProvider as ApiAuthProvider;
use codex_api::TransportError;
use codex_api::error::ApiError;
use codex_api::rate_limits::parse_promo_message;
use codex_api::rate_limits::parse_rate_limit;
use http::HeaderMap;
use serde::Deserialize;
@@ -27,6 +28,7 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr {
status,
body: message,
url: None,
cf_ray: None,
request_id: None,
}),
ApiError::InvalidRequest { message } => CodexErr::InvalidRequest(message),
@@ -70,6 +72,7 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr {
if let Ok(err) = serde_json::from_str::<UsageErrorResponse>(&body_text) {
if err.error.error_type.as_deref() == Some("usage_limit_reached") {
let rate_limits = headers.as_ref().and_then(parse_rate_limit);
let promo_message = headers.as_ref().and_then(parse_promo_message);
let resets_at = err
.error
.resets_at
@@ -78,6 +81,7 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr {
plan_type: err.error.plan_type,
resets_at,
rate_limits,
promo_message,
});
} else if err.error.error_type.as_deref() == Some("usage_not_included") {
return CodexErr::UsageNotIncluded;
@@ -86,13 +90,14 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr {
CodexErr::RetryLimit(RetryLimitReachedError {
status,
request_id: extract_request_id(headers.as_ref()),
request_id: extract_request_tracking_id(headers.as_ref()),
})
} else {
CodexErr::UnexpectedStatus(UnexpectedResponseError {
status,
body: body_text,
url,
cf_ray: extract_header(headers.as_ref(), CF_RAY_HEADER),
request_id: extract_request_id(headers.as_ref()),
})
}
@@ -112,6 +117,9 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr {
const MODEL_CAP_MODEL_HEADER: &str = "x-codex-model-cap-model";
const MODEL_CAP_RESET_AFTER_HEADER: &str = "x-codex-model-cap-reset-after-seconds";
const REQUEST_ID_HEADER: &str = "x-request-id";
const OAI_REQUEST_ID_HEADER: &str = "x-oai-request-id";
const CF_RAY_HEADER: &str = "cf-ray";
#[cfg(test)]
mod tests {
@@ -146,15 +154,20 @@ mod tests {
}
}
fn extract_request_tracking_id(headers: Option<&HeaderMap>) -> Option<String> {
extract_request_id(headers).or_else(|| extract_header(headers, CF_RAY_HEADER))
}
fn extract_request_id(headers: Option<&HeaderMap>) -> Option<String> {
extract_header(headers, REQUEST_ID_HEADER)
.or_else(|| extract_header(headers, OAI_REQUEST_ID_HEADER))
}
fn extract_header(headers: Option<&HeaderMap>, name: &str) -> Option<String> {
headers.and_then(|map| {
["cf-ray", "x-request-id", "x-oai-request-id"]
.iter()
.find_map(|name| {
map.get(*name)
.and_then(|v| v.to_str().ok())
.map(str::to_string)
})
map.get(name)
.and_then(|value| value.to_str().ok())
.map(str::to_string)
})
}
+108 -105
View File
@@ -1,12 +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 codex_api::AggregateStreamExt;
use codex_api::ChatClient as ApiChatClient;
use crate::turn_metadata::build_turn_metadata_header;
use codex_api::CompactClient as ApiCompactClient;
use codex_api::CompactionInput as ApiCompactionInput;
use codex_api::Prompt as ApiPrompt;
@@ -14,13 +15,13 @@ use codex_api::RequestTelemetry;
use codex_api::ReqwestTransport;
use codex_api::ResponseAppendWsRequest;
use codex_api::ResponseCreateWsRequest;
use codex_api::ResponseStream as ApiResponseStream;
use codex_api::ResponsesClient as ApiResponsesClient;
use codex_api::ResponsesOptions as ApiResponsesOptions;
use codex_api::ResponsesWebsocketClient as ApiWebSocketResponsesClient;
use codex_api::ResponsesWebsocketConnection as ApiWebSocketConnection;
use codex_api::SseTelemetry;
use codex_api::TransportError;
use codex_api::WebsocketTelemetry;
use codex_api::build_conversation_headers;
use codex_api::common::Reasoning;
use codex_api::common::ResponsesWsRequest;
@@ -46,6 +47,8 @@ use reqwest::StatusCode;
use serde_json::Value;
use std::time::Duration;
use tokio::sync::mpsc;
use tokio_tungstenite::tungstenite::Error;
use tokio_tungstenite::tungstenite::Message;
use tracing::warn;
use crate::AuthManager;
@@ -63,12 +66,18 @@ use crate::features::Feature;
use crate::flags::CODEX_RS_SSE_FIXTURE;
use crate::model_provider_info::ModelProviderInfo;
use crate::model_provider_info::WireApi;
use crate::tools::spec::create_tools_json_for_chat_completions_api;
use crate::tools::spec::create_tools_json_for_responses_api;
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 {
@@ -82,6 +91,7 @@ struct ModelClientState {
summary: ReasoningSummaryConfig,
session_source: SessionSource,
transport_manager: TransportManager,
turn_metadata_cache: Arc<RwLock<TurnMetadataCache>>,
}
#[derive(Debug, Clone)]
@@ -133,11 +143,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,
@@ -146,6 +158,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 {
@@ -254,11 +298,15 @@ impl ModelClient {
}
impl ModelClientSession {
/// Streams a single model turn using either the Responses or Chat
/// Completions wire API, depending on the configured provider.
///
/// For Chat providers, the underlying stream is optionally aggregated
/// based on the `show_raw_agent_reasoning` flag in the config.
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 the configured Responses transport.
pub async fn stream(&mut self, prompt: &Prompt) -> Result<ResponseStream> {
let wire_api = self.state.provider.wire_api;
match wire_api {
@@ -272,21 +320,6 @@ impl ModelClientSession {
self.stream_responses_api(prompt).await
}
}
WireApi::Chat => {
let api_stream = self.stream_chat_completions(prompt).await?;
if self.state.config.show_raw_agent_reasoning {
Ok(map_response_stream(
api_stream.streaming_mode(),
self.state.otel_manager.clone(),
))
} else {
Ok(map_response_stream(
api_stream.aggregate(),
self.state.otel_manager.clone(),
))
}
}
}
}
@@ -329,6 +362,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;
@@ -377,7 +411,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)),
}
@@ -451,9 +489,14 @@ impl ModelClientSession {
if needs_new {
let mut headers = options.extra_headers.clone();
headers.extend(build_conversation_headers(options.conversation_id.clone()));
let websocket_telemetry = self.build_websocket_telemetry();
let new_conn: ApiWebSocketConnection =
ApiWebSocketResponsesClient::new(api_provider, api_auth)
.connect(headers, options.turn_state.clone())
.connect(
headers,
options.turn_state.clone(),
Some(websocket_telemetry),
)
.await?;
self.connection = Some(new_conn);
}
@@ -478,64 +521,6 @@ impl ModelClientSession {
}
}
/// Streams a turn via the OpenAI Chat Completions API.
///
/// This path is only used when the provider is configured with
/// `WireApi::Chat`; it does not support `output_schema` today.
async fn stream_chat_completions(&self, prompt: &Prompt) -> Result<ApiResponseStream> {
if prompt.output_schema.is_some() {
return Err(CodexErr::UnsupportedOperation(
"output_schema is not supported for Chat Completions API".to_string(),
));
}
let auth_manager = self.state.auth_manager.clone();
let instructions = prompt.base_instructions.text.clone();
let tools_json = create_tools_json_for_chat_completions_api(&prompt.tools)?;
let api_prompt = build_api_prompt(prompt, instructions, tools_json);
let conversation_id = self.state.conversation_id.to_string();
let session_source = self.state.session_source.clone();
let mut auth_recovery = auth_manager
.as_ref()
.map(super::auth::AuthManager::unauthorized_recovery);
loop {
let auth = match auth_manager.as_ref() {
Some(manager) => manager.auth().await,
None => None,
};
let api_provider = self
.state
.provider
.to_api_provider(auth.as_ref().map(CodexAuth::internal_auth_mode))?;
let api_auth = auth_provider_from_auth(auth.clone(), &self.state.provider)?;
let transport = ReqwestTransport::new(build_reqwest_client());
let (request_telemetry, sse_telemetry) = self.build_streaming_telemetry();
let client = ApiChatClient::new(transport, api_provider, api_auth)
.with_telemetry(Some(request_telemetry), Some(sse_telemetry));
let stream_result = client
.stream_prompt(
&self.state.model_info.slug,
&api_prompt,
Some(conversation_id.clone()),
Some(session_source.clone()),
)
.await;
match stream_result {
Ok(stream) => return Ok(stream),
Err(ApiError::Transport(TransportError::Http { status, .. }))
if status == StatusCode::UNAUTHORIZED =>
{
handle_unauthorized(status, &mut auth_recovery).await?;
continue;
}
Err(err) => return Err(map_api_error(err)),
}
}
}
/// Streams a turn via the OpenAI Responses API.
///
/// Handles SSE fixtures, reasoning summaries, verbosity, and the
@@ -582,10 +567,10 @@ impl ModelClientSession {
Ok(stream) => {
return Ok(map_response_stream(stream, self.state.otel_manager.clone()));
}
Err(ApiError::Transport(TransportError::Http { status, .. }))
if status == StatusCode::UNAUTHORIZED =>
{
handle_unauthorized(status, &mut auth_recovery).await?;
Err(ApiError::Transport(
unauthorized_transport @ TransportError::Http { status, .. },
)) if status == StatusCode::UNAUTHORIZED => {
handle_unauthorized(unauthorized_transport, &mut auth_recovery).await?;
continue;
}
Err(err) => return Err(map_api_error(err)),
@@ -621,10 +606,10 @@ impl ModelClientSession {
.await
{
Ok(connection) => connection,
Err(ApiError::Transport(TransportError::Http { status, .. }))
if status == StatusCode::UNAUTHORIZED =>
{
handle_unauthorized(status, &mut auth_recovery).await?;
Err(ApiError::Transport(
unauthorized_transport @ TransportError::Http { status, .. },
)) if status == StatusCode::UNAUTHORIZED => {
handle_unauthorized(unauthorized_transport, &mut auth_recovery).await?;
continue;
}
Err(err) => return Err(map_api_error(err)),
@@ -643,13 +628,20 @@ impl ModelClientSession {
}
}
/// Builds request and SSE telemetry for streaming API calls (Chat/Responses).
/// Builds request and SSE telemetry for streaming API calls.
fn build_streaming_telemetry(&self) -> (Arc<dyn RequestTelemetry>, Arc<dyn SseTelemetry>) {
let telemetry = Arc::new(ApiTelemetry::new(self.state.otel_manager.clone()));
let request_telemetry: Arc<dyn RequestTelemetry> = telemetry.clone();
let sse_telemetry: Arc<dyn SseTelemetry> = telemetry;
(request_telemetry, sse_telemetry)
}
/// Builds telemetry for the Responses API WebSocket transport.
fn build_websocket_telemetry(&self) -> Arc<dyn WebsocketTelemetry> {
let telemetry = Arc::new(ApiTelemetry::new(self.state.otel_manager.clone()));
let websocket_telemetry: Arc<dyn WebsocketTelemetry> = telemetry;
websocket_telemetry
}
}
impl ModelClient {
@@ -698,6 +690,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(
@@ -716,6 +709,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
}
@@ -784,7 +780,7 @@ where
/// When refresh succeeds, the caller should retry the API call; otherwise
/// the mapped `CodexErr` is returned to the caller.
async fn handle_unauthorized(
status: StatusCode,
transport: TransportError,
auth_recovery: &mut Option<UnauthorizedRecovery>,
) -> Result<()> {
if let Some(recovery) = auth_recovery
@@ -797,16 +793,7 @@ async fn handle_unauthorized(
};
}
Err(map_unauthorized_status(status))
}
fn map_unauthorized_status(status: StatusCode) -> CodexErr {
map_api_error(ApiError::Transport(TransportError::Http {
status,
url: None,
headers: None,
body: None,
}))
Err(map_api_error(ApiError::Transport(transport)))
}
struct ApiTelemetry {
@@ -849,3 +836,19 @@ impl SseTelemetry for ApiTelemetry {
self.otel_manager.log_sse_event(result, duration);
}
}
impl WebsocketTelemetry for ApiTelemetry {
fn on_ws_request(&self, duration: Duration, error: Option<&ApiError>) {
let error_message = error.map(std::string::ToString::to_string);
self.otel_manager
.record_websocket_request(duration, error_message.as_deref());
}
fn on_ws_event(
&self,
result: &std::result::Result<Option<std::result::Result<Message, Error>>, ApiError>,
duration: Duration,
) {
self.otel_manager.record_websocket_event(result, duration);
}
}
+153 -142
View File
@@ -3,9 +3,7 @@ use std::collections::HashSet;
use std::fmt::Debug;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::AtomicU64;
use std::sync::atomic::Ordering;
use crate::AuthManager;
use crate::CodexAuth;
@@ -14,6 +12,8 @@ use crate::agent::AgentControl;
use crate::agent::AgentStatus;
use crate::agent::MAX_THREAD_SPAWN_DEPTH;
use crate::agent::agent_status_from_event;
use crate::analytics_client::AnalyticsEventsClient;
use crate::analytics_client::build_track_events_context;
use crate::compact;
use crate::compact::run_inline_auto_compact_task;
use crate::compact::should_use_remote_compact_task;
@@ -48,7 +48,9 @@ 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;
use codex_protocol::protocol::FileChange;
use codex_protocol::protocol::HasLegacyEvent;
@@ -69,14 +71,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;
@@ -94,7 +94,6 @@ use tracing::trace_span;
use tracing::warn;
use crate::ModelProviderInfo;
use crate::WireApi;
use crate::client::ModelClient;
use crate::client::ModelClientSession;
use crate::client_common::Prompt;
@@ -116,6 +115,7 @@ use crate::error::Result as CodexResult;
use crate::exec::StreamOutput;
use crate::exec_policy::ExecPolicyUpdateError;
use crate::feedback_tags;
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;
@@ -127,7 +127,6 @@ use crate::mentions::build_connector_slug_counts;
use crate::mentions::build_skill_name_counts;
use crate::mentions::collect_explicit_app_paths;
use crate::mentions::collect_tool_mentions_from_messages;
use crate::model_provider_info::CHAT_WIRE_API_DEPRECATION_SUMMARY;
use crate::project_doc::get_user_instructions;
use crate::proposed_plan_parser::ProposedPlanParser;
use crate::proposed_plan_parser::ProposedPlanSegment;
@@ -210,7 +209,6 @@ use codex_protocol::models::ContentItem;
use codex_protocol::models::DeveloperInstructions;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::models::render_command_prefix_list;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::InitialHistory;
use codex_protocol::user_input::UserInput;
@@ -241,31 +239,6 @@ pub struct CodexSpawnOk {
pub(crate) const INITIAL_SUBMIT_ID: &str = "";
pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 64;
static CHAT_WIRE_API_DEPRECATION_EMITTED: AtomicBool = AtomicBool::new(false);
fn maybe_push_chat_wire_api_deprecation(
config: &Config,
post_session_configured_events: &mut Vec<Event>,
) {
if config.model_provider.wire_api != WireApi::Chat {
return;
}
if CHAT_WIRE_API_DEPRECATION_EMITTED
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
.is_err()
{
return;
}
post_session_configured_events.push(Event {
id: INITIAL_SUBMIT_ID.to_owned(),
msg: EventMsg::DeprecationNotice(DeprecationNoticeEvent {
summary: CHAT_WIRE_API_DEPRECATION_SUMMARY.to_string(),
details: None,
}),
});
}
impl Codex {
/// Spawn a new [`Codex`] and initialize the session.
@@ -330,10 +303,37 @@ impl Codex {
.base_instructions
.clone()
.or_else(|| conversation_history.get_base_instructions().map(|s| s.text))
.unwrap_or_else(|| model_info.get_model_instructions(config.model_personality));
// Respect explicit thread-start tools; fall back to persisted tools when resuming a thread.
.unwrap_or_else(|| model_info.get_model_instructions(config.personality));
// 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
};
@@ -341,7 +341,7 @@ impl Codex {
// TODO (aibrahim): Consolidate config.model and config.model_reasoning_effort into config.collaboration_mode
// to avoid extracting these fields separately and constructing CollaborationMode here.
let collaboration_mode = CollaborationMode {
mode: ModeKind::Custom,
mode: ModeKind::Default,
settings: Settings {
model: model.clone(),
reasoning_effort: config.model_reasoning_effort,
@@ -354,7 +354,7 @@ impl Codex {
model_reasoning_summary: config.model_reasoning_summary,
developer_instructions: config.developer_instructions.clone(),
user_instructions,
personality: config.model_personality,
personality: config.personality,
base_instructions,
compact_prompt: config.compact_prompt.clone(),
approval_policy: config.approval_policy.clone(),
@@ -488,7 +488,7 @@ pub(crate) struct TurnContext {
pub(crate) developer_instructions: Option<String>,
pub(crate) compact_prompt: Option<String>,
pub(crate) user_instructions: Option<String>,
pub(crate) collaboration_mode_kind: ModeKind,
pub(crate) collaboration_mode: CollaborationMode,
pub(crate) personality: Option<Personality>,
pub(crate) approval_policy: AskForApproval,
pub(crate) sandbox_policy: SandboxPolicy,
@@ -502,7 +502,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()
@@ -632,7 +631,7 @@ impl Session {
per_turn_config.model_reasoning_effort =
session_configuration.collaboration_mode.reasoning_effort();
per_turn_config.model_reasoning_summary = session_configuration.model_reasoning_summary;
per_turn_config.model_personality = session_configuration.personality;
per_turn_config.personality = session_configuration.personality;
per_turn_config.web_search_mode = Some(resolve_web_search_mode_for_turn(
per_turn_config.web_search_mode,
session_configuration.provider.is_azure_responses_endpoint(),
@@ -690,7 +689,7 @@ impl Session {
developer_instructions: session_configuration.developer_instructions.clone(),
compact_prompt: session_configuration.compact_prompt.clone(),
user_instructions: session_configuration.user_instructions.clone(),
collaboration_mode_kind: session_configuration.collaboration_mode.mode,
collaboration_mode: session_configuration.collaboration_mode.clone(),
personality: session_configuration.personality,
approval_policy: session_configuration.approval_policy.value(),
sandbox_policy: session_configuration.sandbox_policy.get().clone(),
@@ -837,7 +836,6 @@ impl Session {
}),
});
}
maybe_push_chat_wire_api_deprecation(&config, &mut post_session_configured_events);
maybe_push_unstable_features_warning(&config, &mut post_session_configured_events);
let auth = auth.as_ref();
@@ -904,6 +902,10 @@ impl Session {
mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())),
mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()),
unified_exec_manager: UnifiedExecProcessManager::default(),
analytics_events_client: AnalyticsEventsClient::new(
Arc::clone(&config),
Arc::clone(&auth_manager),
),
notifier: UserNotifier::new(config.notify.clone()),
rollout: Mutex::new(rollout_recorder),
user_shell: Arc::new(default_shell),
@@ -1019,6 +1021,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 {
@@ -1350,16 +1357,14 @@ impl Session {
fn build_collaboration_mode_update_item(
&self,
previous_collaboration_mode: &CollaborationMode,
next_collaboration_mode: Option<&CollaborationMode>,
previous: Option<&Arc<TurnContext>>,
next: &TurnContext,
) -> Option<ResponseItem> {
if let Some(next_mode) = next_collaboration_mode {
if previous_collaboration_mode == next_mode {
return None;
}
let prev = previous?;
if prev.collaboration_mode != next.collaboration_mode {
// If the next mode has empty developer instructions, this returns None and we emit no
// update, so prior collaboration instructions remain in the prompt history.
Some(DeveloperInstructions::from_collaboration_mode(next_mode)?.into())
Some(DeveloperInstructions::from_collaboration_mode(&next.collaboration_mode)?.into())
} else {
None
}
@@ -1369,8 +1374,6 @@ impl Session {
&self,
previous_context: Option<&Arc<TurnContext>>,
current_context: &TurnContext,
previous_collaboration_mode: &CollaborationMode,
next_collaboration_mode: Option<&CollaborationMode>,
) -> Vec<ResponseItem> {
let mut update_items = Vec::new();
if let Some(env_item) =
@@ -1383,10 +1386,9 @@ impl Session {
{
update_items.push(permissions_item);
}
if let Some(collaboration_mode_item) = self.build_collaboration_mode_update_item(
previous_collaboration_mode,
next_collaboration_mode,
) {
if let Some(collaboration_mode_item) =
self.build_collaboration_mode_update_item(previous_context, current_context)
{
update_items.push(collaboration_mode_item);
}
if let Some(personality_item) =
@@ -1516,7 +1518,7 @@ impl Session {
sub_id: &str,
amendment: &ExecPolicyAmendment,
) {
let Some(prefixes) = render_command_prefix_list([amendment.command.as_slice()]) else {
let Some(prefixes) = format_allow_prefixes(vec![amendment.command.clone()]) else {
warn!("execpolicy amendment for {sub_id} had no command prefix");
return;
};
@@ -1807,6 +1809,7 @@ impl Session {
text: format!("Warning: {}", message.into()),
}],
end_turn: None,
phase: None,
};
self.record_conversation_items(ctx, &[item]).await;
@@ -2196,7 +2199,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
@@ -2209,7 +2212,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
@@ -2222,7 +2225,7 @@ impl Session {
pub async fn read_resource(
&self,
server: &str,
params: ReadResourceRequestParams,
params: ReadResourceRequestParam,
) -> anyhow::Result<ReadResourceResult> {
self.services
.mcp_connection_manager
@@ -2549,10 +2552,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;
@@ -2567,18 +2570,6 @@ mod handlers {
sub_id: String,
updates: SessionSettingsUpdate,
) {
let previous_context = sess
.new_default_turn_with_sub_id(sess.next_internal_sub_id())
.await;
let previous_collaboration_mode = sess
.state
.lock()
.await
.session_configuration
.collaboration_mode
.clone();
let next_collaboration_mode = updates.collaboration_mode.clone();
if let Err(err) = sess.update_settings(updates).await {
sess.send_event_raw(Event {
id: sub_id,
@@ -2588,24 +2579,6 @@ mod handlers {
}),
})
.await;
return;
}
let initial_context_seeded = sess.state.lock().await.initial_context_seeded;
if !initial_context_seeded {
return;
}
let current_context = sess.new_default_turn_with_sub_id(sub_id).await;
let update_items = sess.build_settings_update_items(
Some(&previous_context),
&current_context,
&previous_collaboration_mode,
next_collaboration_mode.as_ref(),
);
if !update_items.is_empty() {
sess.record_conversation_items(&current_context, &update_items)
.await;
}
}
@@ -2630,7 +2603,7 @@ mod handlers {
} => {
let collaboration_mode = collaboration_mode.or_else(|| {
Some(CollaborationMode {
mode: ModeKind::Custom,
mode: ModeKind::Default,
settings: Settings {
model: model.clone(),
reasoning_effort: effort,
@@ -2665,14 +2638,6 @@ mod handlers {
_ => unreachable!(),
};
let previous_collaboration_mode = sess
.state
.lock()
.await
.session_configuration
.collaboration_mode
.clone();
let next_collaboration_mode = updates.collaboration_mode.clone();
let Ok(current_context) = sess.new_turn_with_sub_id(sub_id, updates).await else {
// new_turn_with_sub_id already emits the error event.
return;
@@ -2685,12 +2650,8 @@ mod handlers {
// Attempt to inject input into current task
if let Err(items) = sess.inject_input(items).await {
sess.seed_initial_context_if_needed(&current_context).await;
let update_items = sess.build_settings_update_items(
previous_context.as_ref(),
&current_context,
&previous_collaboration_mode,
next_collaboration_mode.as_ref(),
);
let update_items =
sess.build_settings_update_items(previous_context.as_ref(), &current_context);
if !update_items.is_empty() {
sess.record_conversation_items(&current_context, &update_items)
.await;
@@ -2723,7 +2684,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 {
@@ -2738,6 +2699,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
@@ -3205,7 +3172,7 @@ async fn spawn_review_thread(
developer_instructions: None,
user_instructions: None,
compact_prompt: parent_turn_context.compact_prompt.clone(),
collaboration_mode_kind: parent_turn_context.collaboration_mode_kind,
collaboration_mode: parent_turn_context.collaboration_mode.clone(),
personality: parent_turn_context.personality,
approval_policy: parent_turn_context.approval_policy,
sandbox_policy: parent_turn_context.sandbox_policy.clone(),
@@ -3318,9 +3285,10 @@ 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_kind,
collaboration_mode_kind: turn_context.collaboration_mode.mode,
});
sess.send_event(&turn_context, event).await;
if total_usage_tokens >= auto_compact_limit {
@@ -3385,10 +3353,18 @@ pub(crate) async fn run_turn(
.await;
let otel_manager = turn_context.client.get_otel_manager();
let thread_id = sess.conversation_id.to_string();
let tracking = build_track_events_context(turn_context.client.get_model(), thread_id);
let SkillInjections {
items: skill_items,
warnings: skill_warnings,
} = build_skill_injections(&mentioned_skills, Some(&otel_manager)).await;
} = build_skill_injections(
&mentioned_skills,
Some(&otel_manager),
&sess.services.analytics_events_client,
tracking.clone(),
)
.await;
for message in skill_warnings {
sess.send_event(&turn_context, EventMsg::Warning(WarningEvent { message }))
@@ -3412,7 +3388,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
@@ -3463,6 +3441,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;
@@ -4225,7 +4216,7 @@ async fn try_run_sampling_request(
let mut last_agent_message: Option<String> = None;
let mut active_item: Option<TurnItem> = None;
let mut should_emit_turn_diff = false;
let plan_mode = turn_context.collaboration_mode_kind == ModeKind::Plan;
let plan_mode = turn_context.collaboration_mode.mode == ModeKind::Plan;
let mut plan_mode_state = plan_mode.then(|| PlanModeStreamState::new(&turn_context.sub_id));
let receiving_span = trace_span!("receiving_stream");
let outcome: CodexResult<SamplingRequestResult> = loop {
@@ -4475,8 +4466,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;
@@ -4522,8 +4511,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;
@@ -4544,6 +4532,7 @@ mod tests {
text: text.to_string(),
}],
end_turn: None,
phase: None,
}
}
@@ -4825,6 +4814,7 @@ mod tests {
text: "turn 1 user".to_string(),
}],
end_turn: None,
phase: None,
},
ResponseItem::Message {
id: None,
@@ -4833,6 +4823,7 @@ mod tests {
text: "turn 1 assistant".to_string(),
}],
end_turn: None,
phase: None,
},
];
sess.record_into_history(&turn_1, tc.as_ref()).await;
@@ -4845,6 +4836,7 @@ mod tests {
text: "turn 2 user".to_string(),
}],
end_turn: None,
phase: None,
},
ResponseItem::Message {
id: None,
@@ -4853,6 +4845,7 @@ mod tests {
text: "turn 2 assistant".to_string(),
}],
end_turn: None,
phase: None,
},
];
sess.record_into_history(&turn_2, tc.as_ref()).await;
@@ -4885,6 +4878,7 @@ mod tests {
text: "turn 1 user".to_string(),
}],
end_turn: None,
phase: None,
}];
sess.record_into_history(&turn_1, tc.as_ref()).await;
@@ -4948,7 +4942,7 @@ mod tests {
let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config);
let reasoning_effort = config.model_reasoning_effort;
let collaboration_mode = CollaborationMode {
mode: ModeKind::Custom,
mode: ModeKind::Default,
settings: Settings {
model,
reasoning_effort,
@@ -4961,11 +4955,11 @@ mod tests {
model_reasoning_summary: config.model_reasoning_summary,
developer_instructions: config.developer_instructions.clone(),
user_instructions: config.user_instructions.clone(),
personality: config.model_personality,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.model_personality)),
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
compact_prompt: config.compact_prompt.clone(),
approval_policy: config.approval_policy.clone(),
sandbox_policy: config.sandbox_policy.clone(),
@@ -5031,7 +5025,7 @@ mod tests {
let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config);
let reasoning_effort = config.model_reasoning_effort;
let collaboration_mode = CollaborationMode {
mode: ModeKind::Custom,
mode: ModeKind::Default,
settings: Settings {
model,
reasoning_effort,
@@ -5044,11 +5038,11 @@ mod tests {
model_reasoning_summary: config.model_reasoning_summary,
developer_instructions: config.developer_instructions.clone(),
user_instructions: config.user_instructions.clone(),
personality: config.model_personality,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.model_personality)),
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
compact_prompt: config.compact_prompt.clone(),
approval_policy: config.approval_policy.clone(),
sandbox_policy: config.sandbox_policy.clone(),
@@ -5107,7 +5101,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,
@@ -5115,6 +5109,7 @@ mod tests {
"ok": true,
"value": 42
})),
meta: None,
};
let got = FunctionCallOutputPayload::from(&ctr);
@@ -5153,10 +5148,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);
@@ -5172,10 +5168,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);
@@ -5190,10 +5187,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);
@@ -5244,11 +5242,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,
})
}
@@ -5298,7 +5295,7 @@ mod tests {
let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config);
let reasoning_effort = config.model_reasoning_effort;
let collaboration_mode = CollaborationMode {
mode: ModeKind::Custom,
mode: ModeKind::Default,
settings: Settings {
model,
reasoning_effort,
@@ -5311,11 +5308,11 @@ mod tests {
model_reasoning_summary: config.model_reasoning_summary,
developer_instructions: config.developer_instructions.clone(),
user_instructions: config.user_instructions.clone(),
personality: config.model_personality,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.model_personality)),
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
compact_prompt: config.compact_prompt.clone(),
approval_policy: config.approval_policy.clone(),
sandbox_policy: config.sandbox_policy.clone(),
@@ -5347,6 +5344,10 @@ mod tests {
mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())),
mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()),
unified_exec_manager: UnifiedExecProcessManager::default(),
analytics_events_client: AnalyticsEventsClient::new(
Arc::clone(&config),
Arc::clone(&auth_manager),
),
notifier: UserNotifier::new(None),
rollout: Mutex::new(None),
user_shell: Arc::new(default_user_shell()),
@@ -5414,7 +5415,7 @@ mod tests {
let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config);
let reasoning_effort = config.model_reasoning_effort;
let collaboration_mode = CollaborationMode {
mode: ModeKind::Custom,
mode: ModeKind::Default,
settings: Settings {
model,
reasoning_effort,
@@ -5427,11 +5428,11 @@ mod tests {
model_reasoning_summary: config.model_reasoning_summary,
developer_instructions: config.developer_instructions.clone(),
user_instructions: config.user_instructions.clone(),
personality: config.model_personality,
personality: config.personality,
base_instructions: config
.base_instructions
.clone()
.unwrap_or_else(|| model_info.get_model_instructions(config.model_personality)),
.unwrap_or_else(|| model_info.get_model_instructions(config.personality)),
compact_prompt: config.compact_prompt.clone(),
approval_policy: config.approval_policy.clone(),
sandbox_policy: config.sandbox_policy.clone(),
@@ -5463,6 +5464,10 @@ mod tests {
mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())),
mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()),
unified_exec_manager: UnifiedExecProcessManager::default(),
analytics_events_client: AnalyticsEventsClient::new(
Arc::clone(&config),
Arc::clone(&auth_manager),
),
notifier: UserNotifier::new(None),
rollout: Mutex::new(None),
user_shell: Arc::new(default_user_shell()),
@@ -5823,6 +5828,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()));
@@ -5834,6 +5840,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()));
@@ -5859,6 +5866,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()));
@@ -5870,6 +5878,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()));
@@ -5895,6 +5904,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));
@@ -5906,6 +5916,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));
+21 -12
View File
@@ -312,14 +312,22 @@ async fn handle_exec_approval(
event: ExecApprovalRequestEvent,
cancel_token: &CancellationToken,
) {
let ExecApprovalRequestEvent {
call_id,
command,
cwd,
reason,
proposed_execpolicy_amendment,
..
} = event;
// Race approval with cancellation and timeout to avoid hangs.
let approval_fut = parent_session.request_command_approval(
parent_ctx,
parent_ctx.sub_id.clone(),
event.command,
event.cwd,
event.reason,
event.proposed_execpolicy_amendment,
call_id,
command,
cwd,
reason,
proposed_execpolicy_amendment,
);
let decision = await_approval_with_cancel(
approval_fut,
@@ -341,14 +349,15 @@ async fn handle_patch_approval(
event: ApplyPatchApprovalRequestEvent,
cancel_token: &CancellationToken,
) {
let ApplyPatchApprovalRequestEvent {
call_id,
changes,
reason,
grant_root,
..
} = event;
let decision_rx = parent_session
.request_patch_approval(
parent_ctx,
parent_ctx.sub_id.clone(),
event.changes,
event.reason,
event.grant_root,
)
.request_patch_approval(parent_ctx, call_id, changes, reason, grant_root)
.await;
let decision = await_approval_with_cancel(
async move { decision_rx.await.unwrap_or_default() },
@@ -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"])));
+13 -2
View File
@@ -61,7 +61,7 @@ pub(crate) async fn run_compact_task(
) {
let start_event = EventMsg::TurnStarted(TurnStartedEvent {
model_context_window: turn_context.client.get_model_context_window(),
collaboration_mode_kind: turn_context.collaboration_mode_kind,
collaboration_mode_kind: turn_context.collaboration_mode.mode,
});
sess.send_event(&turn_context, start_event).await;
run_compact_task_inner(sess.clone(), turn_context, input).await;
@@ -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,
},
];
+42 -2
View File
@@ -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>,
@@ -22,7 +25,7 @@ pub(crate) async fn run_inline_remote_auto_compact_task(
pub(crate) async fn run_remote_compact_task(sess: Arc<Session>, turn_context: Arc<TurnContext>) {
let start_event = EventMsg::TurnStarted(TurnStartedEvent {
model_context_window: turn_context.client.get_model_context_window(),
collaboration_mode_kind: turn_context.collaboration_mode_kind,
collaboration_mode_kind: turn_context.collaboration_mode.mode,
});
sess.send_event(&turn_context, start_event).await;
@@ -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
}
+2 -2
View File
@@ -278,7 +278,7 @@ impl ConfigDocument {
mutated
}),
ConfigEdit::SetModelPersonality { personality } => Ok(self.write_profile_value(
&["model_personality"],
&["personality"],
personality.map(|personality| value(personality.to_string())),
)),
ConfigEdit::SetNoticeHideFullAccessWarning(acknowledged) => Ok(self.write_value(
@@ -724,7 +724,7 @@ impl ConfigEditsBuilder {
self
}
pub fn set_model_personality(mut self, personality: Option<Personality>) -> Self {
pub fn set_personality(mut self, personality: Option<Personality>) -> Self {
self.edits
.push(ConfigEdit::SetModelPersonality { personality });
self
+91 -37
View File
@@ -32,9 +32,10 @@ use crate::features::FeatureOverrides;
use crate::features::Features;
use crate::features::FeaturesToml;
use crate::git_info::resolve_root_git_project_for_trust;
use crate::model_provider_info::LEGACY_OLLAMA_CHAT_PROVIDER_ID;
use crate::model_provider_info::LMSTUDIO_OSS_PROVIDER_ID;
use crate::model_provider_info::ModelProviderInfo;
use crate::model_provider_info::OLLAMA_CHAT_PROVIDER_ID;
use crate::model_provider_info::OLLAMA_CHAT_PROVIDER_REMOVED_ERROR;
use crate::model_provider_info::OLLAMA_OSS_PROVIDER_ID;
use crate::model_provider_info::built_in_model_providers;
use crate::project_doc::DEFAULT_PROJECT_DOC_FILENAME;
@@ -134,7 +135,7 @@ pub struct Config {
pub model_provider: ModelProviderInfo,
/// Optionally specify the personality of the model
pub model_personality: Option<Personality>,
pub personality: Option<Personality>,
/// Approval policy for executing commands.
pub approval_policy: Constrained<AskForApproval>,
@@ -212,7 +213,7 @@ pub struct Config {
/// Show startup tooltips in the TUI welcome screen.
pub show_tooltips: bool,
/// Start the TUI in the specified collaboration mode (plan/execute/etc.).
/// Start the TUI in the specified collaboration mode (plan/default).
pub experimental_mode: Option<ModeKind>,
/// Controls whether the TUI uses the terminal's alternate screen buffer.
@@ -757,14 +758,20 @@ pub fn set_project_trust_level(
pub fn set_default_oss_provider(codex_home: &Path, provider: &str) -> std::io::Result<()> {
// Validate that the provider is one of the known OSS providers
match provider {
LMSTUDIO_OSS_PROVIDER_ID | OLLAMA_OSS_PROVIDER_ID | OLLAMA_CHAT_PROVIDER_ID => {
LMSTUDIO_OSS_PROVIDER_ID | OLLAMA_OSS_PROVIDER_ID => {
// Valid provider, continue
}
LEGACY_OLLAMA_CHAT_PROVIDER_ID => {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidInput,
OLLAMA_CHAT_PROVIDER_REMOVED_ERROR,
));
}
_ => {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidInput,
format!(
"Invalid OSS provider '{provider}'. Must be one of: {LMSTUDIO_OSS_PROVIDER_ID}, {OLLAMA_OSS_PROVIDER_ID}, {OLLAMA_CHAT_PROVIDER_ID}"
"Invalid OSS provider '{provider}'. Must be one of: {LMSTUDIO_OSS_PROVIDER_ID}, {OLLAMA_OSS_PROVIDER_ID}"
),
));
}
@@ -912,9 +919,8 @@ pub struct ConfigToml {
/// Override to force-enable reasoning summaries for the configured model.
pub model_supports_reasoning_summaries: Option<bool>,
/// EXPERIMENTAL
/// Optionally specify a personality for the model
pub model_personality: Option<Personality>,
pub personality: Option<Personality>,
/// Base URL for requests to ChatGPT (as opposed to the OpenAI API).
pub chatgpt_base_url: Option<String>,
@@ -986,7 +992,7 @@ pub struct ConfigToml {
pub experimental_compact_prompt_file: Option<AbsolutePathBuf>,
pub experimental_use_unified_exec_tool: Option<bool>,
pub experimental_use_freeform_apply_patch: Option<bool>,
/// Preferred OSS provider for local models, e.g. "lmstudio", "ollama", or "ollama-chat".
/// Preferred OSS provider for local models, e.g. "lmstudio" or "ollama".
pub oss_provider: Option<String>,
}
@@ -1193,7 +1199,7 @@ pub struct ConfigOverrides {
pub codex_linux_sandbox_exe: Option<PathBuf>,
pub base_instructions: Option<String>,
pub developer_instructions: Option<String>,
pub model_personality: Option<Personality>,
pub personality: Option<Personality>,
pub compact_prompt: Option<String>,
pub include_apply_patch_tool: Option<bool>,
pub show_raw_agent_reasoning: Option<bool>,
@@ -1300,7 +1306,7 @@ impl Config {
codex_linux_sandbox_exe,
base_instructions,
developer_instructions,
model_personality,
personality,
compact_prompt,
include_apply_patch_tool: include_apply_patch_tool_override,
show_raw_agent_reasoning,
@@ -1412,10 +1418,12 @@ impl Config {
let model_provider = model_providers
.get(&model_provider_id)
.ok_or_else(|| {
std::io::Error::new(
std::io::ErrorKind::NotFound,
format!("Model provider `{model_provider_id}` not found"),
)
let message = if model_provider_id == LEGACY_OLLAMA_CHAT_PROVIDER_ID {
OLLAMA_CHAT_PROVIDER_REMOVED_ERROR.to_string()
} else {
format!("Model provider `{model_provider_id}` not found")
};
std::io::Error::new(std::io::ErrorKind::NotFound, message)
})?
.clone();
@@ -1497,9 +1505,14 @@ impl Config {
Self::try_read_non_empty_file(model_instructions_path, "model instructions file")?;
let base_instructions = base_instructions.or(file_base_instructions);
let developer_instructions = developer_instructions.or(cfg.developer_instructions);
let model_personality = model_personality
.or(config_profile.model_personality)
.or(cfg.model_personality);
let personality = personality
.or(config_profile.personality)
.or(cfg.personality)
.or_else(|| {
features
.enabled(Feature::Personality)
.then_some(Personality::Friendly)
});
let experimental_compact_prompt_path = config_profile
.experimental_compact_prompt_file
@@ -1552,7 +1565,7 @@ impl Config {
notify: cfg.notify,
user_instructions,
base_instructions,
model_personality,
personality,
developer_instructions,
compact_prompt,
// The config.toml omits "_mode" because it's a config file. However, "_mode"
@@ -3572,7 +3585,7 @@ model = "gpt-5.1-codex"
cfg: ConfigToml,
model_provider_map: HashMap<String, ModelProviderInfo>,
openai_provider: ModelProviderInfo,
openai_chat_completions_provider: ModelProviderInfo,
openai_custom_provider: ModelProviderInfo,
}
impl PrecedenceTestFixture {
@@ -3654,11 +3667,11 @@ profile = "gpt3"
[analytics]
enabled = true
[model_providers.openai-chat-completions]
name = "OpenAI using Chat Completions"
[model_providers.openai-custom]
name = "OpenAI custom"
base_url = "https://api.openai.com/v1"
env_key = "OPENAI_API_KEY"
wire_api = "chat"
wire_api = "responses"
request_max_retries = 4 # retry failed HTTP requests
stream_max_retries = 10 # retry dropped SSE streams
stream_idle_timeout_ms = 300000 # 5m idle timeout
@@ -3672,7 +3685,7 @@ model_reasoning_summary = "detailed"
[profiles.gpt3]
model = "gpt-3.5-turbo"
model_provider = "openai-chat-completions"
model_provider = "openai-custom"
[profiles.zdr]
model = "o3"
@@ -3703,11 +3716,11 @@ model_verbosity = "high"
let codex_home_temp_dir = TempDir::new().unwrap();
let openai_chat_completions_provider = ModelProviderInfo {
name: "OpenAI using Chat Completions".to_string(),
let openai_custom_provider = ModelProviderInfo {
name: "OpenAI custom".to_string(),
base_url: Some("https://api.openai.com/v1".to_string()),
env_key: Some("OPENAI_API_KEY".to_string()),
wire_api: crate::WireApi::Chat,
wire_api: crate::WireApi::Responses,
env_key_instructions: None,
experimental_bearer_token: None,
query_params: None,
@@ -3721,10 +3734,7 @@ model_verbosity = "high"
};
let model_provider_map = {
let mut model_provider_map = built_in_model_providers();
model_provider_map.insert(
"openai-chat-completions".to_string(),
openai_chat_completions_provider.clone(),
);
model_provider_map.insert("openai-custom".to_string(), openai_custom_provider.clone());
model_provider_map
};
@@ -3739,7 +3749,7 @@ model_verbosity = "high"
cfg,
model_provider_map,
openai_provider,
openai_chat_completions_provider,
openai_custom_provider,
})
}
@@ -3807,7 +3817,7 @@ model_verbosity = "high"
model_reasoning_summary: ReasoningSummary::Detailed,
model_supports_reasoning_summaries: None,
model_verbosity: None,
model_personality: None,
personality: Some(Personality::Friendly),
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
base_instructions: None,
developer_instructions: None,
@@ -3860,8 +3870,8 @@ model_verbosity = "high"
review_model: None,
model_context_window: None,
model_auto_compact_token_limit: None,
model_provider_id: "openai-chat-completions".to_string(),
model_provider: fixture.openai_chat_completions_provider.clone(),
model_provider_id: "openai-custom".to_string(),
model_provider: fixture.openai_custom_provider.clone(),
approval_policy: Constrained::allow_any(AskForApproval::UnlessTrusted),
sandbox_policy: Constrained::allow_any(SandboxPolicy::new_read_only_policy()),
enforce_residency: Constrained::allow_any(None),
@@ -3892,7 +3902,7 @@ model_verbosity = "high"
model_reasoning_summary: ReasoningSummary::default(),
model_supports_reasoning_summaries: None,
model_verbosity: None,
model_personality: None,
personality: Some(Personality::Friendly),
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
base_instructions: None,
developer_instructions: None,
@@ -3992,7 +4002,7 @@ model_verbosity = "high"
model_reasoning_summary: ReasoningSummary::default(),
model_supports_reasoning_summaries: None,
model_verbosity: None,
model_personality: None,
personality: Some(Personality::Friendly),
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
base_instructions: None,
developer_instructions: None,
@@ -4078,7 +4088,7 @@ model_verbosity = "high"
model_reasoning_summary: ReasoningSummary::Detailed,
model_supports_reasoning_summaries: None,
model_verbosity: Some(Verbosity::High),
model_personality: None,
personality: Some(Personality::Friendly),
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
base_instructions: None,
developer_instructions: None,
@@ -4262,6 +4272,50 @@ trust_level = "trusted"
Ok(())
}
#[test]
fn test_set_default_oss_provider_rejects_legacy_ollama_chat_provider() -> std::io::Result<()> {
let temp_dir = TempDir::new()?;
let codex_home = temp_dir.path();
let result = set_default_oss_provider(codex_home, LEGACY_OLLAMA_CHAT_PROVIDER_ID);
assert!(result.is_err());
let error = result.unwrap_err();
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
assert!(
error
.to_string()
.contains(OLLAMA_CHAT_PROVIDER_REMOVED_ERROR)
);
Ok(())
}
#[test]
fn test_load_config_rejects_legacy_ollama_chat_provider_with_helpful_error()
-> std::io::Result<()> {
let codex_home = TempDir::new()?;
let cfg = ConfigToml {
model_provider: Some(LEGACY_OLLAMA_CHAT_PROVIDER_ID.to_string()),
..Default::default()
};
let result = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
);
assert!(result.is_err());
let error = result.unwrap_err();
assert_eq!(error.kind(), std::io::ErrorKind::NotFound);
assert!(
error
.to_string()
.contains(OLLAMA_CHAT_PROVIDER_REMOVED_ERROR)
);
Ok(())
}
#[test]
fn test_untrusted_project_gets_workspace_write_sandbox() -> anyhow::Result<()> {
let config_with_untrusted = r#"
+1 -1
View File
@@ -25,7 +25,7 @@ pub struct ConfigProfile {
pub model_reasoning_effort: Option<ReasoningEffort>,
pub model_reasoning_summary: Option<ReasoningSummary>,
pub model_verbosity: Option<Verbosity>,
pub model_personality: Option<Personality>,
pub personality: Option<Personality>,
pub chatgpt_base_url: Option<String>,
/// Optional path to a file containing model instructions.
pub model_instructions_file: Option<AbsolutePathBuf>,
+1 -1
View File
@@ -471,7 +471,7 @@ pub struct Tui {
#[serde(default = "default_true")]
pub show_tooltips: bool,
/// Start the TUI in the specified collaboration mode (plan/execute/etc.).
/// Start the TUI in the specified collaboration mode (plan/default).
/// Defaults to unset.
#[serde(default)]
pub experimental_mode: Option<ModeKind>,
+4 -2
View File
@@ -425,7 +425,9 @@ async fn load_requirements_from_legacy_scheme(
/// empty array, which indicates that root detection should be disabled).
/// - Returns an error if `project_root_markers` is specified but is not an
/// array of strings.
fn project_root_markers_from_config(config: &TomlValue) -> io::Result<Option<Vec<String>>> {
pub(crate) fn project_root_markers_from_config(
config: &TomlValue,
) -> io::Result<Option<Vec<String>>> {
let Some(table) = config.as_table() else {
return Ok(None);
};
@@ -454,7 +456,7 @@ fn project_root_markers_from_config(config: &TomlValue) -> io::Result<Option<Vec
Ok(Some(markers))
}
fn default_project_root_markers() -> Vec<String> {
pub(crate) fn default_project_root_markers() -> Vec<String> {
DEFAULT_PROJECT_ROOT_MARKERS
.iter()
.map(ToString::to_string)
+68 -40
View File
@@ -88,29 +88,12 @@ impl ContextManager {
let model_info = turn_context.client.get_model_info();
let personality = turn_context
.personality
.or(turn_context.client.config().model_personality);
.or(turn_context.client.config().personality);
let base_instructions = model_info.get_model_instructions(personality);
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());
+1
View File
@@ -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;
+2
View File
@@ -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,
}
}
}
+161 -12
View File
@@ -126,7 +126,7 @@ pub enum CodexErr {
QuotaExceeded,
#[error(
"To use Codex with your ChatGPT plan, upgrade to Plus: https://openai.com/chatgpt/pricing."
"To use Codex with your ChatGPT plan, upgrade to Plus: https://chatgpt.com/explore/plus."
)]
UsageNotIncluded,
@@ -286,13 +286,42 @@ pub struct UnexpectedResponseError {
pub status: StatusCode,
pub body: String,
pub url: Option<String>,
pub cf_ray: Option<String>,
pub request_id: Option<String>,
}
const CLOUDFLARE_BLOCKED_MESSAGE: &str =
"Access blocked by Cloudflare. This usually happens when connecting from a restricted region";
const UNEXPECTED_RESPONSE_BODY_MAX_BYTES: usize = 1000;
impl UnexpectedResponseError {
fn display_body(&self) -> String {
if let Some(message) = self.extract_error_message() {
return message;
}
let trimmed_body = self.body.trim();
if trimmed_body.is_empty() {
return "Unknown error".to_string();
}
truncate_with_ellipsis(trimmed_body, UNEXPECTED_RESPONSE_BODY_MAX_BYTES)
}
fn extract_error_message(&self) -> Option<String> {
let json = serde_json::from_str::<serde_json::Value>(&self.body).ok()?;
let message = json
.get("error")
.and_then(|error| error.get("message"))
.and_then(serde_json::Value::as_str)?;
let message = message.trim();
if message.is_empty() {
None
} else {
Some(message.to_string())
}
}
fn friendly_message(&self) -> Option<String> {
if self.status != StatusCode::FORBIDDEN {
return None;
@@ -307,6 +336,9 @@ impl UnexpectedResponseError {
if let Some(url) = &self.url {
message.push_str(&format!(", url: {url}"));
}
if let Some(cf_ray) = &self.cf_ray {
message.push_str(&format!(", cf-ray: {cf_ray}"));
}
if let Some(id) = &self.request_id {
message.push_str(&format!(", request id: {id}"));
}
@@ -321,11 +353,14 @@ impl std::fmt::Display for UnexpectedResponseError {
write!(f, "{friendly}")
} else {
let status = self.status;
let body = &self.body;
let body = self.display_body();
let mut message = format!("unexpected status {status}: {body}");
if let Some(url) = &self.url {
message.push_str(&format!(", url: {url}"));
}
if let Some(cf_ray) = &self.cf_ray {
message.push_str(&format!(", cf-ray: {cf_ray}"));
}
if let Some(id) = &self.request_id {
message.push_str(&format!(", request id: {id}"));
}
@@ -335,6 +370,21 @@ impl std::fmt::Display for UnexpectedResponseError {
}
impl std::error::Error for UnexpectedResponseError {}
fn truncate_with_ellipsis(text: &str, max_bytes: usize) -> String {
if text.len() <= max_bytes {
return text.to_string();
}
let mut cut = max_bytes;
while !text.is_char_boundary(cut) {
cut = cut.saturating_sub(1);
}
let mut truncated = text[..cut].to_string();
truncated.push_str("...");
truncated
}
#[derive(Debug)]
pub struct RetryLimitReachedError {
pub status: StatusCode,
@@ -360,13 +410,22 @@ pub struct UsageLimitReachedError {
pub(crate) plan_type: Option<PlanType>,
pub(crate) resets_at: Option<DateTime<Utc>>,
pub(crate) rate_limits: Option<RateLimitSnapshot>,
pub(crate) promo_message: Option<String>,
}
impl std::fmt::Display for UsageLimitReachedError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some(promo_message) = &self.promo_message {
return write!(
f,
"You've hit your usage limit. {promo_message},{}",
retry_suffix_after_or(self.resets_at.as_ref())
);
}
let message = match self.plan_type.as_ref() {
Some(PlanType::Known(KnownPlan::Plus)) => format!(
"You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), visit https://chatgpt.com/codex/settings/usage to purchase more credits{}",
"You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits{}",
retry_suffix_after_or(self.resets_at.as_ref())
),
Some(PlanType::Known(KnownPlan::Team)) | Some(PlanType::Known(KnownPlan::Business)) => {
@@ -377,7 +436,7 @@ impl std::fmt::Display for UsageLimitReachedError {
}
Some(PlanType::Known(KnownPlan::Free)) | Some(PlanType::Known(KnownPlan::Go)) => {
format!(
"You've hit your usage limit. Upgrade to Plus to continue using Codex (https://openai.com/chatgpt/pricing),{}",
"You've hit your usage limit. Upgrade to Plus to continue using Codex (https://chatgpt.com/explore/plus),{}",
retry_suffix_after_or(self.resets_at.as_ref())
)
}
@@ -670,10 +729,11 @@ mod tests {
plan_type: Some(PlanType::Known(KnownPlan::Plus)),
resets_at: None,
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
assert_eq!(
err.to_string(),
"You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again later."
"You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again later."
);
}
@@ -816,10 +876,11 @@ mod tests {
plan_type: Some(PlanType::Known(KnownPlan::Free)),
resets_at: None,
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
assert_eq!(
err.to_string(),
"You've hit your usage limit. Upgrade to Plus to continue using Codex (https://openai.com/chatgpt/pricing), or try again later."
"You've hit your usage limit. Upgrade to Plus to continue using Codex (https://chatgpt.com/explore/plus), or try again later."
);
}
@@ -829,10 +890,11 @@ mod tests {
plan_type: Some(PlanType::Known(KnownPlan::Go)),
resets_at: None,
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
assert_eq!(
err.to_string(),
"You've hit your usage limit. Upgrade to Plus to continue using Codex (https://openai.com/chatgpt/pricing), or try again later."
"You've hit your usage limit. Upgrade to Plus to continue using Codex (https://chatgpt.com/explore/plus), or try again later."
);
}
@@ -842,6 +904,7 @@ mod tests {
plan_type: None,
resets_at: None,
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
assert_eq!(
err.to_string(),
@@ -859,6 +922,7 @@ mod tests {
plan_type: Some(PlanType::Known(KnownPlan::Team)),
resets_at: Some(resets_at),
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
let expected = format!(
"You've hit your usage limit. To get more access now, send a request to your admin or try again at {expected_time}."
@@ -873,6 +937,7 @@ mod tests {
plan_type: Some(PlanType::Known(KnownPlan::Business)),
resets_at: None,
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
assert_eq!(
err.to_string(),
@@ -886,6 +951,7 @@ mod tests {
plan_type: Some(PlanType::Known(KnownPlan::Enterprise)),
resets_at: None,
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
assert_eq!(
err.to_string(),
@@ -903,6 +969,7 @@ mod tests {
plan_type: Some(PlanType::Known(KnownPlan::Pro)),
resets_at: Some(resets_at),
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
let expected = format!(
"You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at {expected_time}."
@@ -921,6 +988,7 @@ mod tests {
plan_type: None,
resets_at: Some(resets_at),
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
let expected = format!("You've hit your usage limit. Try again at {expected_time}.");
assert_eq!(err.to_string(), expected);
@@ -934,15 +1002,14 @@ mod tests {
body: "<html><body>Cloudflare error: Sorry, you have been blocked</body></html>"
.to_string(),
url: Some("http://example.com/blocked".to_string()),
request_id: Some("ray-id".to_string()),
cf_ray: Some("ray-id".to_string()),
request_id: None,
};
let status = StatusCode::FORBIDDEN.to_string();
let url = "http://example.com/blocked";
assert_eq!(
err.to_string(),
format!(
"{CLOUDFLARE_BLOCKED_MESSAGE} (status {status}), url: {url}, request id: ray-id"
)
format!("{CLOUDFLARE_BLOCKED_MESSAGE} (status {status}), url: {url}, cf-ray: ray-id")
);
}
@@ -952,6 +1019,7 @@ mod tests {
status: StatusCode::FORBIDDEN,
body: "plain text error".to_string(),
url: Some("http://example.com/plain".to_string()),
cf_ray: None,
request_id: None,
};
let status = StatusCode::FORBIDDEN.to_string();
@@ -962,6 +1030,63 @@ mod tests {
);
}
#[test]
fn unexpected_status_prefers_error_message_when_present() {
let err = UnexpectedResponseError {
status: StatusCode::UNAUTHORIZED,
body: r#"{"error":{"message":"Workspace is not authorized in this region."},"status":401}"#
.to_string(),
url: Some("https://chatgpt.com/backend-api/codex/responses".to_string()),
cf_ray: None,
request_id: Some("req-123".to_string()),
};
let status = StatusCode::UNAUTHORIZED.to_string();
assert_eq!(
err.to_string(),
format!(
"unexpected status {status}: Workspace is not authorized in this region., url: https://chatgpt.com/backend-api/codex/responses, request id: req-123"
)
);
}
#[test]
fn unexpected_status_truncates_long_body_with_ellipsis() {
let long_body = "x".repeat(UNEXPECTED_RESPONSE_BODY_MAX_BYTES + 10);
let err = UnexpectedResponseError {
status: StatusCode::BAD_GATEWAY,
body: long_body,
url: Some("http://example.com/long".to_string()),
cf_ray: None,
request_id: Some("req-long".to_string()),
};
let status = StatusCode::BAD_GATEWAY.to_string();
let expected_body = format!("{}...", "x".repeat(UNEXPECTED_RESPONSE_BODY_MAX_BYTES));
assert_eq!(
err.to_string(),
format!(
"unexpected status {status}: {expected_body}, url: http://example.com/long, request id: req-long"
)
);
}
#[test]
fn unexpected_status_includes_cf_ray_and_request_id() {
let err = UnexpectedResponseError {
status: StatusCode::UNAUTHORIZED,
body: "plain text error".to_string(),
url: Some("https://chatgpt.com/backend-api/codex/responses".to_string()),
cf_ray: Some("9c81f9f18f2fa49d-LHR".to_string()),
request_id: Some("req-xyz".to_string()),
};
let status = StatusCode::UNAUTHORIZED.to_string();
assert_eq!(
err.to_string(),
format!(
"unexpected status {status}: plain text error, url: https://chatgpt.com/backend-api/codex/responses, cf-ray: 9c81f9f18f2fa49d-LHR, request id: req-xyz"
)
);
}
#[test]
fn usage_limit_reached_includes_hours_and_minutes() {
let base = Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap();
@@ -972,9 +1097,10 @@ mod tests {
plan_type: Some(PlanType::Known(KnownPlan::Plus)),
resets_at: Some(resets_at),
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
let expected = format!(
"You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at {expected_time}."
"You've hit your usage limit. Upgrade to Pro (https://chatgpt.com/explore/pro), visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at {expected_time}."
);
assert_eq!(err.to_string(), expected);
});
@@ -991,6 +1117,7 @@ mod tests {
plan_type: None,
resets_at: Some(resets_at),
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
let expected = format!("You've hit your usage limit. Try again at {expected_time}.");
assert_eq!(err.to_string(), expected);
@@ -1007,9 +1134,31 @@ mod tests {
plan_type: None,
resets_at: Some(resets_at),
rate_limits: Some(rate_limit_snapshot()),
promo_message: None,
};
let expected = format!("You've hit your usage limit. Try again at {expected_time}.");
assert_eq!(err.to_string(), expected);
});
}
#[test]
fn usage_limit_reached_with_promo_message() {
let base = Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap();
let resets_at = base + ChronoDuration::seconds(30);
with_now_override(base, move || {
let expected_time = format_retry_timestamp(&resets_at);
let err = UsageLimitReachedError {
plan_type: None,
resets_at: Some(resets_at),
rate_limits: Some(rate_limit_snapshot()),
promo_message: Some(
"To continue using Codex, start a free trial of <PLAN> today".to_string(),
),
};
let expected = format!(
"You've hit your usage limit. To continue using Codex, start a free trial of <PLAN> today, or try again at {expected_time}."
);
assert_eq!(err.to_string(), expected);
});
}
}
+9
View File
@@ -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");
+81 -19
View File
@@ -1,9 +1,12 @@
use crate::config::types::EnvironmentVariablePattern;
use crate::config::types::ShellEnvironmentPolicy;
use crate::config::types::ShellEnvironmentPolicyInherit;
use codex_protocol::ThreadId;
use std::collections::HashMap;
use std::collections::HashSet;
pub const CODEX_THREAD_ID_ENV_VAR: &str = "CODEX_THREAD_ID";
/// Construct an environment map based on the rules in the specified policy. The
/// resulting map can be passed directly to `Command::envs()` after calling
/// `env_clear()` to ensure no unintended variables are leaked to the spawned
@@ -11,11 +14,21 @@ use std::collections::HashSet;
///
/// The derivation follows the algorithm documented in the struct-level comment
/// for [`ShellEnvironmentPolicy`].
pub fn create_env(policy: &ShellEnvironmentPolicy) -> HashMap<String, String> {
populate_env(std::env::vars(), policy)
///
/// `CODEX_THREAD_ID` is injected when a thread id is provided, even when
/// `include_only` is set.
pub fn create_env(
policy: &ShellEnvironmentPolicy,
thread_id: Option<ThreadId>,
) -> HashMap<String, String> {
populate_env(std::env::vars(), policy, thread_id)
}
fn populate_env<I>(vars: I, policy: &ShellEnvironmentPolicy) -> HashMap<String, String>
fn populate_env<I>(
vars: I,
policy: &ShellEnvironmentPolicy,
thread_id: Option<ThreadId>,
) -> HashMap<String, String>
where
I: IntoIterator<Item = (String, String)>,
{
@@ -72,6 +85,11 @@ where
env_map.retain(|k, _| matches_any(k, &policy.include_only));
}
// Step 6 – Populate the thread ID environment variable when provided.
if let Some(thread_id) = thread_id {
env_map.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
}
env_map
}
@@ -98,14 +116,16 @@ mod tests {
]);
let policy = ShellEnvironmentPolicy::default(); // inherit All, default excludes ignored
let result = populate_env(vars, &policy);
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let expected: HashMap<String, String> = hashmap! {
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
"HOME".to_string() => "/home/user".to_string(),
"API_KEY".to_string() => "secret".to_string(),
"SECRET_TOKEN".to_string() => "t".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
@@ -123,12 +143,14 @@ mod tests {
ignore_default_excludes: false, // apply KEY/SECRET/TOKEN filter
..Default::default()
};
let result = populate_env(vars, &policy);
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let expected: HashMap<String, String> = hashmap! {
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
"HOME".to_string() => "/home/user".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
@@ -144,11 +166,13 @@ mod tests {
..Default::default()
};
let result = populate_env(vars, &policy);
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let expected: HashMap<String, String> = hashmap! {
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
@@ -163,11 +187,41 @@ mod tests {
};
policy.r#set.insert("NEW_VAR".to_string(), "42".to_string());
let result = populate_env(vars, &policy);
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
"NEW_VAR".to_string() => "42".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
fn populate_env_inserts_thread_id() {
let vars = make_vars(&[("PATH", "/usr/bin")]);
let policy = ShellEnvironmentPolicy::default();
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
#[test]
fn populate_env_omits_thread_id_when_missing() {
let vars = make_vars(&[("PATH", "/usr/bin")]);
let policy = ShellEnvironmentPolicy::default();
let result = populate_env(vars, &policy, None);
let expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
"NEW_VAR".to_string() => "42".to_string(),
};
assert_eq!(result, expected);
@@ -183,8 +237,10 @@ mod tests {
..Default::default()
};
let result = populate_env(vars.clone(), &policy);
let expected: HashMap<String, String> = vars.into_iter().collect();
let thread_id = ThreadId::new();
let result = populate_env(vars.clone(), &policy, Some(thread_id));
let mut expected: HashMap<String, String> = vars.into_iter().collect();
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
@@ -198,10 +254,12 @@ mod tests {
..Default::default()
};
let result = populate_env(vars, &policy);
let expected: HashMap<String, String> = hashmap! {
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"PATH".to_string() => "/usr/bin".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
@@ -220,11 +278,13 @@ mod tests {
..Default::default()
};
let result = populate_env(vars, &policy);
let expected: HashMap<String, String> = hashmap! {
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"Path".to_string() => "C:\\Windows\\System32".to_string(),
"TEMP".to_string() => "C:\\Temp".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
@@ -242,10 +302,12 @@ mod tests {
.r#set
.insert("ONLY_VAR".to_string(), "yes".to_string());
let result = populate_env(vars, &policy);
let expected: HashMap<String, String> = hashmap! {
let thread_id = ThreadId::new();
let result = populate_env(vars, &policy, Some(thread_id));
let mut expected: HashMap<String, String> = hashmap! {
"ONLY_VAR".to_string() => "yes".to_string(),
};
expected.insert(CODEX_THREAD_ID_ENV_VAR.to_string(), thread_id.to_string());
assert_eq!(result, expected);
}
}
+32 -12
View File
@@ -248,9 +248,7 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result<Policy,
// from each layer, so that higher-precedence layers can override
// rules defined in lower-precedence ones.
let mut policy_paths = Vec::new();
// Include disabled project layers so .codex/rules still applies when
// project config.toml is trust-disabled.
for layer in config_stack.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true) {
for layer in config_stack.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, false) {
if let Some(config_folder) = layer.config_folder() {
#[expect(clippy::expect_used)]
let policy_dir = config_folder.join(RULES_DIR_NAME).expect("safe join");
@@ -684,12 +682,12 @@ mod tests {
}
#[tokio::test]
async fn loads_rules_from_disabled_project_layers() -> anyhow::Result<()> {
async fn ignores_rules_from_untrusted_project_layers() -> anyhow::Result<()> {
let project_dir = tempdir()?;
let policy_dir = project_dir.path().join(RULES_DIR_NAME);
fs::create_dir_all(&policy_dir)?;
fs::write(
policy_dir.join("disabled.rules"),
policy_dir.join("untrusted.rules"),
r#"prefix_rule(pattern=["ls"], decision="forbidden")"#,
)?;
@@ -699,7 +697,7 @@ mod tests {
dot_codex_folder: project_dot_codex_folder,
},
TomlValue::Table(Default::default()),
"trust disabled",
"marked untrusted",
)];
let config_stack = ConfigLayerStack::new(
layers,
@@ -711,16 +709,14 @@ mod tests {
assert_eq!(
Evaluation {
decision: Decision::Forbidden,
matched_rules: vec![RuleMatch::PrefixRuleMatch {
matched_prefix: vec!["ls".to_string()],
decision: Decision::Forbidden,
justification: None,
decision: Decision::Allow,
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
command: vec!["ls".to_string()],
decision: Decision::Allow,
}],
},
policy.check_multiple([vec!["ls".to_string()]].iter(), &|_| Decision::Allow)
);
Ok(())
}
@@ -1280,6 +1276,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()
}
+12 -16
View File
@@ -121,7 +121,7 @@ pub enum Feature {
SkillEnvVarDependencyPrompt,
/// Steer feature flag - when enabled, Enter submits immediately instead of queuing.
Steer,
/// Enable collaboration modes (Plan, Code, Pair Programming, Execute).
/// Enable collaboration modes (Plan, Default).
CollaborationModes,
/// Enable personality selection in the TUI.
Personality,
@@ -358,7 +358,7 @@ fn legacy_usage_notice(alias: &str, feature: Feature) -> (String, Option<String>
}
fn web_search_details() -> &'static str {
"Set `web_search` to `\"live\"`, `\"cached\"`, or `\"disabled\"` in config.toml."
"Set `web_search` to `\"live\"`, `\"cached\"`, or `\"disabled\"` at the top level (or under a profile) in config.toml."
}
/// Keys accepted in `[features]` tables.
@@ -503,11 +503,7 @@ pub const FEATURES: &[FeatureSpec] = &[
id: Feature::PowershellUtf8,
key: "powershell_utf8",
#[cfg(windows)]
stage: Stage::Experimental {
name: "Powershell UTF-8 support",
menu_description: "Enable UTF-8 output in Powershell.",
announcement: "Codex now supports UTF-8 output in Powershell. If you are seeing problems, disable in /experimental.",
},
stage: Stage::Stable,
#[cfg(windows)]
default_enabled: true,
#[cfg(not(windows))]
@@ -524,7 +520,11 @@ pub const FEATURES: &[FeatureSpec] = &[
FeatureSpec {
id: Feature::Collab,
key: "collab",
stage: Stage::UnderDevelopment,
stage: Stage::Experimental {
name: "Sub-agents",
menu_description: "Ask Codex to spawn multiple agents to parallelize the work and win in efficiency.",
announcement: "NEW: Sub-agents can now be spawned by Codex. Enable in /experimental and restart Codex!",
},
default_enabled: false,
},
FeatureSpec {
@@ -562,18 +562,14 @@ pub const FEATURES: &[FeatureSpec] = &[
FeatureSpec {
id: Feature::CollaborationModes,
key: "collaboration_modes",
stage: Stage::UnderDevelopment,
default_enabled: false,
stage: Stage::Stable,
default_enabled: true,
},
FeatureSpec {
id: Feature::Personality,
key: "personality",
stage: Stage::Experimental {
name: "Personality",
menu_description: "Choose a communication style for Codex.",
announcement: "NEW: Pick a personality for Codex. Enable in /experimental!",
},
default_enabled: false,
stage: Stage::Stable,
default_enabled: true,
},
FeatureSpec {
id: Feature::ResponsesWebsockets,
+71 -5
View File
@@ -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,
}
}
}
+5 -2
View File
@@ -5,6 +5,7 @@
// the TUI or the tracing stack).
#![deny(clippy::print_stdout, clippy::print_stderr)]
mod analytics_client;
pub mod api_bridge;
mod apply_patch;
pub mod auth;
@@ -48,6 +49,7 @@ mod message_history;
mod model_provider_info;
pub mod parse_command;
pub mod path_utils;
pub mod personality_migration;
pub mod powershell;
mod proposed_plan_parser;
pub mod sandboxing;
@@ -59,12 +61,10 @@ pub mod token_data;
mod truncate;
mod unified_exec;
pub mod windows_sandbox;
pub use model_provider_info::CHAT_WIRE_API_DEPRECATION_SUMMARY;
pub use model_provider_info::DEFAULT_LMSTUDIO_PORT;
pub use model_provider_info::DEFAULT_OLLAMA_PORT;
pub use model_provider_info::LMSTUDIO_OSS_PROVIDER_ID;
pub use model_provider_info::ModelProviderInfo;
pub use model_provider_info::OLLAMA_CHAT_PROVIDER_ID;
pub use model_provider_info::OLLAMA_OSS_PROVIDER_ID;
pub use model_provider_info::WireApi;
pub use model_provider_info::built_in_model_providers;
@@ -99,6 +99,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;
@@ -118,6 +119,7 @@ pub use rollout::list::parse_cursor;
pub use rollout::list::read_head_for_summary;
pub use rollout::list::read_session_meta_line;
pub use rollout::rollout_date_parts;
pub use rollout::session_index::find_thread_names_by_ids;
pub use transport_manager::TransportManager;
mod function_tool;
mod state;
@@ -128,6 +130,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
View File
@@ -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,
}
}
+69 -54
View File
@@ -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,
}),
})
@@ -453,16 +463,8 @@ impl McpConnectionManager {
#[instrument(level = "trace", skip_all)]
pub async fn list_all_tools(&self) -> HashMap<String, ToolInfo> {
let mut tools = HashMap::new();
for (server_name, managed_client) in &self.clients {
let client = if server_name == CODEX_APPS_MCP_SERVER_NAME {
// Avoid blocking on codex_apps_mcp startup; use tools only when ready.
match managed_client.client.clone().now_or_never() {
Some(Ok(client)) => Some(client),
_ => None,
}
} else {
managed_client.client().await.ok()
};
for managed_client in self.clients.values() {
let client = managed_client.client().await.ok();
if let Some(client) = client {
tools.extend(qualify_tools(filter_tools(
client.tools,
@@ -493,7 +495,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 +560,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 +615,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 +623,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 +666,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 +682,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 +865,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 +980,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 +1063,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,
+5 -4
View File
@@ -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(),
+37 -31
View File
@@ -8,7 +8,6 @@
use crate::auth::AuthMode;
use crate::error::EnvVarError;
use codex_api::Provider as ApiProvider;
use codex_api::WireApi as ApiWireApi;
use codex_api::is_azure_responses_wire_base_url;
use codex_api::provider::RetryConfig as ApiRetryConfig;
use http::HeaderMap;
@@ -28,25 +27,33 @@ const DEFAULT_REQUEST_MAX_RETRIES: u64 = 4;
const MAX_STREAM_MAX_RETRIES: u64 = 100;
/// Hard cap for user-configured `request_max_retries`.
const MAX_REQUEST_MAX_RETRIES: u64 = 100;
pub const CHAT_WIRE_API_DEPRECATION_SUMMARY: &str = r#"Support for the "chat" wire API is deprecated and will soon be removed. Update your model provider definition in config.toml to use wire_api = "responses"."#;
const OPENAI_PROVIDER_NAME: &str = "OpenAI";
const CHAT_WIRE_API_REMOVED_ERROR: &str = "`wire_api = \"chat\"` is no longer supported.\nHow to fix: set `wire_api = \"responses\"` in your provider config.\nMore info: https://github.com/openai/codex/discussions/7782";
pub(crate) const LEGACY_OLLAMA_CHAT_PROVIDER_ID: &str = "ollama-chat";
pub(crate) const OLLAMA_CHAT_PROVIDER_REMOVED_ERROR: &str = "`ollama-chat` is no longer supported.\nHow to fix: replace `ollama-chat` with `ollama` in `model_provider`, `oss_provider`, or `--local-provider`.\nMore info: https://github.com/openai/codex/discussions/7782";
/// Wire protocol that the provider speaks. Most third-party services only
/// implement the classic OpenAI Chat Completions JSON schema, whereas OpenAI
/// itself (and a handful of others) additionally expose the more modern
/// *Responses* API. The two protocols use different request/response shapes
/// and *cannot* be auto-detected at runtime, therefore each provider entry
/// must declare which one it expects.
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
/// Wire protocol that the provider speaks.
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, JsonSchema)]
#[serde(rename_all = "lowercase")]
pub enum WireApi {
/// The Responses API exposed by OpenAI at `/v1/responses`.
Responses,
/// Regular Chat Completions compatible with `/v1/chat/completions`.
#[default]
Chat,
Responses,
}
impl<'de> Deserialize<'de> for WireApi {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let value = String::deserialize(deserializer)?;
match value.as_str() {
"responses" => Ok(Self::Responses),
"chat" => Err(serde::de::Error::custom(CHAT_WIRE_API_REMOVED_ERROR)),
_ => Err(serde::de::Error::unknown_variant(&value, &["responses"])),
}
}
}
/// Serializable representation of a provider definition.
@@ -161,10 +168,6 @@ impl ModelProviderInfo {
name: self.name.clone(),
base_url,
query_params: self.query_params.clone(),
wire: match self.wire_api {
WireApi::Responses => ApiWireApi::Responses,
WireApi::Chat => ApiWireApi::Chat,
},
headers,
retry,
stream_idle_timeout: self.stream_idle_timeout(),
@@ -172,12 +175,7 @@ impl ModelProviderInfo {
}
pub(crate) fn is_azure_responses_endpoint(&self) -> bool {
let wire = match self.wire_api {
WireApi::Responses => ApiWireApi::Responses,
WireApi::Chat => ApiWireApi::Chat,
};
is_azure_responses_wire_base_url(wire, &self.name, self.base_url.as_deref())
is_azure_responses_wire_base_url(&self.name, self.base_url.as_deref())
}
/// If `env_key` is Some, returns the API key for this provider if present
@@ -277,7 +275,6 @@ pub const DEFAULT_OLLAMA_PORT: u16 = 11434;
pub const LMSTUDIO_OSS_PROVIDER_ID: &str = "lmstudio";
pub const OLLAMA_OSS_PROVIDER_ID: &str = "ollama";
pub const OLLAMA_CHAT_PROVIDER_ID: &str = "ollama-chat";
/// Built-in default provider list.
pub fn built_in_model_providers() -> HashMap<String, ModelProviderInfo> {
@@ -293,10 +290,6 @@ pub fn built_in_model_providers() -> HashMap<String, ModelProviderInfo> {
OLLAMA_OSS_PROVIDER_ID,
create_oss_provider(DEFAULT_OLLAMA_PORT, WireApi::Responses),
),
(
OLLAMA_CHAT_PROVIDER_ID,
create_oss_provider(DEFAULT_OLLAMA_PORT, WireApi::Chat),
),
(
LMSTUDIO_OSS_PROVIDER_ID,
create_oss_provider(DEFAULT_LMSTUDIO_PORT, WireApi::Responses),
@@ -363,7 +356,7 @@ base_url = "http://localhost:11434/v1"
env_key: None,
env_key_instructions: None,
experimental_bearer_token: None,
wire_api: WireApi::Chat,
wire_api: WireApi::Responses,
query_params: None,
http_headers: None,
env_http_headers: None,
@@ -392,7 +385,7 @@ query_params = { api-version = "2025-04-01-preview" }
env_key: Some("AZURE_OPENAI_API_KEY".into()),
env_key_instructions: None,
experimental_bearer_token: None,
wire_api: WireApi::Chat,
wire_api: WireApi::Responses,
query_params: Some(maplit::hashmap! {
"api-version".to_string() => "2025-04-01-preview".to_string(),
}),
@@ -424,7 +417,7 @@ env_http_headers = { "X-Example-Env-Header" = "EXAMPLE_ENV_VAR" }
env_key: Some("API_KEY".into()),
env_key_instructions: None,
experimental_bearer_token: None,
wire_api: WireApi::Chat,
wire_api: WireApi::Responses,
query_params: None,
http_headers: Some(maplit::hashmap! {
"X-Example-Header".to_string() => "example-value".to_string(),
@@ -442,4 +435,17 @@ env_http_headers = { "X-Example-Env-Header" = "EXAMPLE_ENV_VAR" }
let provider: ModelProviderInfo = toml::from_str(azure_provider_toml).unwrap();
assert_eq!(expected_provider, provider);
}
#[test]
fn test_deserialize_chat_wire_api_shows_helpful_error() {
let provider_toml = r#"
name = "OpenAI using Chat Completions"
base_url = "https://api.openai.com/v1"
env_key = "OPENAI_API_KEY"
wire_api = "chat"
"#;
let err = toml::from_str::<ModelProviderInfo>(provider_toml).unwrap_err();
assert!(err.to_string().contains(CHAT_WIRE_API_REMOVED_ERROR));
}
}
+27 -2
View File
@@ -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>,
}
@@ -3,19 +3,11 @@ use codex_protocol::config_types::ModeKind;
use codex_protocol::openai_models::ReasoningEffort;
const COLLABORATION_MODE_PLAN: &str = include_str!("../../templates/collaboration_mode/plan.md");
const COLLABORATION_MODE_CODE: &str = include_str!("../../templates/collaboration_mode/code.md");
const COLLABORATION_MODE_PAIR_PROGRAMMING: &str =
include_str!("../../templates/collaboration_mode/pair_programming.md");
const COLLABORATION_MODE_EXECUTE: &str =
include_str!("../../templates/collaboration_mode/execute.md");
const COLLABORATION_MODE_DEFAULT: &str =
include_str!("../../templates/collaboration_mode/default.md");
pub(super) fn builtin_collaboration_mode_presets() -> Vec<CollaborationModeMask> {
vec![
plan_preset(),
code_preset(),
pair_programming_preset(),
execute_preset(),
]
vec![plan_preset(), default_preset()]
}
#[cfg(any(test, feature = "test-support"))]
@@ -33,32 +25,12 @@ fn plan_preset() -> CollaborationModeMask {
}
}
fn code_preset() -> CollaborationModeMask {
fn default_preset() -> CollaborationModeMask {
CollaborationModeMask {
name: "Code".to_string(),
mode: Some(ModeKind::Code),
name: "Default".to_string(),
mode: Some(ModeKind::Default),
model: None,
reasoning_effort: None,
developer_instructions: Some(Some(COLLABORATION_MODE_CODE.to_string())),
}
}
fn pair_programming_preset() -> CollaborationModeMask {
CollaborationModeMask {
name: "Pair Programming".to_string(),
mode: Some(ModeKind::PairProgramming),
model: None,
reasoning_effort: Some(Some(ReasoningEffort::Medium)),
developer_instructions: Some(Some(COLLABORATION_MODE_PAIR_PROGRAMMING.to_string())),
}
}
fn execute_preset() -> CollaborationModeMask {
CollaborationModeMask {
name: "Execute".to_string(),
mode: Some(ModeKind::Execute),
model: None,
reasoning_effort: Some(Some(ReasoningEffort::High)),
developer_instructions: Some(Some(COLLABORATION_MODE_EXECUTE.to_string())),
developer_instructions: Some(Some(COLLABORATION_MODE_DEFAULT.to_string())),
}
}
+75 -13
View File
@@ -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;
+10
View File
@@ -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(),
},
]
});
+265
View File
@@ -0,0 +1,265 @@
use crate::config::ConfigToml;
use crate::config::edit::ConfigEditsBuilder;
use crate::rollout::ARCHIVED_SESSIONS_SUBDIR;
use crate::rollout::SESSIONS_SUBDIR;
use crate::rollout::list::ThreadListConfig;
use crate::rollout::list::ThreadListLayout;
use crate::rollout::list::ThreadSortKey;
use crate::rollout::list::get_threads_in_root;
use crate::state_db;
use codex_protocol::config_types::Personality;
use codex_protocol::protocol::SessionSource;
use std::io;
use std::path::Path;
use tokio::fs::OpenOptions;
use tokio::io::AsyncWriteExt;
pub const PERSONALITY_MIGRATION_FILENAME: &str = ".personality_migration";
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum PersonalityMigrationStatus {
SkippedMarker,
SkippedExplicitPersonality,
SkippedNoSessions,
Applied,
}
pub async fn maybe_migrate_personality(
codex_home: &Path,
config_toml: &ConfigToml,
) -> io::Result<PersonalityMigrationStatus> {
let marker_path = codex_home.join(PERSONALITY_MIGRATION_FILENAME);
if tokio::fs::try_exists(&marker_path).await? {
return Ok(PersonalityMigrationStatus::SkippedMarker);
}
let config_profile = config_toml
.get_config_profile(None)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?;
if config_toml.personality.is_some() || config_profile.personality.is_some() {
create_marker(&marker_path).await?;
return Ok(PersonalityMigrationStatus::SkippedExplicitPersonality);
}
let model_provider_id = config_profile
.model_provider
.or_else(|| config_toml.model_provider.clone())
.unwrap_or_else(|| "openai".to_string());
if !has_recorded_sessions(codex_home, model_provider_id.as_str()).await? {
create_marker(&marker_path).await?;
return Ok(PersonalityMigrationStatus::SkippedNoSessions);
}
ConfigEditsBuilder::new(codex_home)
.set_personality(Some(Personality::Pragmatic))
.apply()
.await
.map_err(|err| {
io::Error::other(format!("failed to persist personality migration: {err}"))
})?;
create_marker(&marker_path).await?;
Ok(PersonalityMigrationStatus::Applied)
}
async fn has_recorded_sessions(codex_home: &Path, default_provider: &str) -> io::Result<bool> {
let allowed_sources: &[SessionSource] = &[];
if let Some(state_db_ctx) = state_db::open_if_present(codex_home, default_provider).await
&& let Some(ids) = state_db::list_thread_ids_db(
Some(state_db_ctx.as_ref()),
codex_home,
1,
None,
ThreadSortKey::CreatedAt,
allowed_sources,
None,
false,
"personality_migration",
)
.await
&& !ids.is_empty()
{
return Ok(true);
}
let sessions = get_threads_in_root(
codex_home.join(SESSIONS_SUBDIR),
1,
None,
ThreadSortKey::CreatedAt,
ThreadListConfig {
allowed_sources,
model_providers: None,
default_provider,
layout: ThreadListLayout::NestedByDate,
},
)
.await?;
if !sessions.items.is_empty() {
return Ok(true);
}
let archived_sessions = get_threads_in_root(
codex_home.join(ARCHIVED_SESSIONS_SUBDIR),
1,
None,
ThreadSortKey::CreatedAt,
ThreadListConfig {
allowed_sources,
model_providers: None,
default_provider,
layout: ThreadListLayout::Flat,
},
)
.await?;
Ok(!archived_sessions.items.is_empty())
}
async fn create_marker(marker_path: &Path) -> io::Result<()> {
match OpenOptions::new()
.create_new(true)
.write(true)
.open(marker_path)
.await
{
Ok(mut file) => file.write_all(b"v1\n").await,
Err(err) if err.kind() == io::ErrorKind::AlreadyExists => Ok(()),
Err(err) => Err(err),
}
}
#[cfg(test)]
mod tests {
use super::*;
use codex_protocol::ThreadId;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::RolloutLine;
use codex_protocol::protocol::SessionMeta;
use codex_protocol::protocol::SessionMetaLine;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::UserMessageEvent;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
use tokio::io::AsyncWriteExt;
const TEST_TIMESTAMP: &str = "2025-01-01T00-00-00";
async fn read_config_toml(codex_home: &Path) -> io::Result<ConfigToml> {
let contents = tokio::fs::read_to_string(codex_home.join("config.toml")).await?;
toml::from_str(&contents).map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))
}
async fn write_session_with_user_event(codex_home: &Path) -> io::Result<()> {
let thread_id = ThreadId::new();
let dir = codex_home
.join(SESSIONS_SUBDIR)
.join("2025")
.join("01")
.join("01");
tokio::fs::create_dir_all(&dir).await?;
let file_path = dir.join(format!("rollout-{TEST_TIMESTAMP}-{thread_id}.jsonl"));
let mut file = tokio::fs::File::create(&file_path).await?;
let session_meta = SessionMetaLine {
meta: SessionMeta {
id: thread_id,
forked_from_id: None,
timestamp: TEST_TIMESTAMP.to_string(),
cwd: std::path::PathBuf::from("."),
originator: "test_originator".to_string(),
cli_version: "test_version".to_string(),
source: SessionSource::Cli,
model_provider: None,
base_instructions: None,
dynamic_tools: None,
},
git: None,
};
let meta_line = RolloutLine {
timestamp: TEST_TIMESTAMP.to_string(),
item: RolloutItem::SessionMeta(session_meta),
};
let user_event = RolloutLine {
timestamp: TEST_TIMESTAMP.to_string(),
item: RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
message: "hello".to_string(),
images: None,
local_images: Vec::new(),
text_elements: Vec::new(),
})),
};
file.write_all(format!("{}\n", serde_json::to_string(&meta_line)?).as_bytes())
.await?;
file.write_all(format!("{}\n", serde_json::to_string(&user_event)?).as_bytes())
.await?;
Ok(())
}
#[tokio::test]
async fn applies_when_sessions_exist_and_no_personality() -> io::Result<()> {
let temp = TempDir::new()?;
write_session_with_user_event(temp.path()).await?;
let config_toml = ConfigToml::default();
let status = maybe_migrate_personality(temp.path(), &config_toml).await?;
assert_eq!(status, PersonalityMigrationStatus::Applied);
assert!(temp.path().join(PERSONALITY_MIGRATION_FILENAME).exists());
let persisted = read_config_toml(temp.path()).await?;
assert_eq!(persisted.personality, Some(Personality::Pragmatic));
Ok(())
}
#[tokio::test]
async fn skips_when_marker_exists() -> io::Result<()> {
let temp = TempDir::new()?;
create_marker(&temp.path().join(PERSONALITY_MIGRATION_FILENAME)).await?;
let config_toml = ConfigToml::default();
let status = maybe_migrate_personality(temp.path(), &config_toml).await?;
assert_eq!(status, PersonalityMigrationStatus::SkippedMarker);
assert!(!temp.path().join("config.toml").exists());
Ok(())
}
#[tokio::test]
async fn skips_when_personality_explicit() -> io::Result<()> {
let temp = TempDir::new()?;
ConfigEditsBuilder::new(temp.path())
.set_personality(Some(Personality::Friendly))
.apply()
.await
.map_err(|err| io::Error::other(format!("failed to write config: {err}")))?;
let config_toml = read_config_toml(temp.path()).await?;
let status = maybe_migrate_personality(temp.path(), &config_toml).await?;
assert_eq!(
status,
PersonalityMigrationStatus::SkippedExplicitPersonality
);
assert!(temp.path().join(PERSONALITY_MIGRATION_FILENAME).exists());
let persisted = read_config_toml(temp.path()).await?;
assert_eq!(persisted.personality, Some(Personality::Friendly));
Ok(())
}
#[tokio::test]
async fn skips_when_no_sessions() -> io::Result<()> {
let temp = TempDir::new()?;
let config_toml = ConfigToml::default();
let status = maybe_migrate_personality(temp.path(), &config_toml).await?;
assert_eq!(status, PersonalityMigrationStatus::SkippedNoSessions);
assert!(temp.path().join(PERSONALITY_MIGRATION_FILENAME).exists());
assert!(!temp.path().join("config.toml").exists());
Ok(())
}
}
+2 -2
View File
@@ -516,7 +516,7 @@ mod tests {
)
.unwrap_or_else(|_| cfg.codex_home.join("skills/pdf-processing/SKILL.md"));
let expected_path_str = expected_path.to_string_lossy().replace('\\', "/");
let usage_rules = "- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.\n- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.\n- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.\n- How to use a skill (progressive disclosure):\n 1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.\n 2) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.\n 3) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.\n 4) If `assets/` or templates exist, reuse them instead of recreating from scratch.\n- Coordination and sequencing:\n - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.\n - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.\n- Context hygiene:\n - Keep context small: summarize long sections instead of pasting them; only load extra files when needed.\n - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.\n - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.\n- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue.";
let usage_rules = "- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.\n- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.\n- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.\n- How to use a skill (progressive disclosure):\n 1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.\n 2) When `SKILL.md` references relative paths (e.g., `scripts/foo.py`), resolve them relative to the skill directory listed above first, and only consider other paths if needed.\n 3) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.\n 4) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.\n 5) If `assets/` or templates exist, reuse them instead of recreating from scratch.\n- Coordination and sequencing:\n - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.\n - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.\n- Context hygiene:\n - Keep context small: summarize long sections instead of pasting them; only load extra files when needed.\n - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.\n - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.\n- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue.";
let expected = format!(
"base doc\n\n## Skills\nA skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.\n### Available skills\n- pdf-processing: extract from pdfs (file: {expected_path_str})\n### How to use skills\n{usage_rules}"
);
@@ -540,7 +540,7 @@ mod tests {
dunce::canonicalize(cfg.codex_home.join("skills/linting/SKILL.md").as_path())
.unwrap_or_else(|_| cfg.codex_home.join("skills/linting/SKILL.md"));
let expected_path_str = expected_path.to_string_lossy().replace('\\', "/");
let usage_rules = "- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.\n- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.\n- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.\n- How to use a skill (progressive disclosure):\n 1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.\n 2) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.\n 3) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.\n 4) If `assets/` or templates exist, reuse them instead of recreating from scratch.\n- Coordination and sequencing:\n - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.\n - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.\n- Context hygiene:\n - Keep context small: summarize long sections instead of pasting them; only load extra files when needed.\n - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.\n - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.\n- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue.";
let usage_rules = "- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.\n- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.\n- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.\n- How to use a skill (progressive disclosure):\n 1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.\n 2) When `SKILL.md` references relative paths (e.g., `scripts/foo.py`), resolve them relative to the skill directory listed above first, and only consider other paths if needed.\n 3) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.\n 4) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.\n 5) If `assets/` or templates exist, reuse them instead of recreating from scratch.\n- Coordination and sequencing:\n - If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.\n - Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.\n- Context hygiene:\n - Keep context small: summarize long sections instead of pasting them; only load extra files when needed.\n - Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.\n - When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.\n- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue.";
let expected = format!(
"## Skills\nA skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.\n### Available skills\n- linting: run clippy (file: {expected_path_str})\n### How to use skills\n{usage_rules}"
);
+46 -36
View File
@@ -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;
@@ -243,9 +245,7 @@ impl serde::Serialize for Cursor {
{
let ts_str = self
.ts
.format(&format_description!(
"[year]-[month]-[day]T[hour]-[minute]-[second]"
))
.format(&Rfc3339)
.map_err(|e| serde::ser::Error::custom(format!("format error: {e}")))?;
serializer.serialize_str(&format!("{ts_str}|{}", self.id))
}
@@ -628,9 +628,13 @@ pub fn parse_cursor(token: &str) -> Option<Cursor> {
return None;
};
let format: &[FormatItem] =
format_description!("[year]-[month]-[day]T[hour]-[minute]-[second]");
let ts = PrimitiveDateTime::parse(file_ts, format).ok()?.assume_utc();
let ts = OffsetDateTime::parse(file_ts, &Rfc3339).ok().or_else(|| {
let format: &[FormatItem] =
format_description!("[year]-[month]-[day]T[hour]-[minute]-[second]");
PrimitiveDateTime::parse(file_ts, format)
.ok()
.map(PrimitiveDateTime::assume_utc)
})?;
Some(Cursor::new(ts, uuid))
}
@@ -967,10 +971,7 @@ async fn read_head_summary(path: &Path, head_limit: usize) -> io::Result<HeadTai
RolloutItem::SessionMeta(session_meta_line) => {
summary.source = Some(session_meta_line.meta.source.clone());
summary.model_provider = session_meta_line.meta.model_provider.clone();
summary.created_at = summary
.created_at
.clone()
.or_else(|| Some(rollout_line.timestamp.clone()));
summary.created_at = Some(session_meta_line.meta.timestamp.clone());
summary.saw_session_meta = true;
if summary.head.len() < head_limit
&& let Ok(val) = serde_json::to_value(session_meta_line)
@@ -983,6 +984,14 @@ 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, 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
&& let Ok(val) = serde_json::to_value(item)
{
@@ -1065,6 +1074,29 @@ async fn find_thread_path_by_id_str_in_subdir(
return Ok(None);
}
// Prefer DB lookup, then fall back to rollout file search.
// TODO(jif): sqlite migration phase 1
let archived_only = match subdir {
SESSIONS_SUBDIR => Some(false),
ARCHIVED_SESSIONS_SUBDIR => Some(true),
_ => None,
};
let state_db_ctx = state_db::open_if_present(codex_home, "").await;
if let Some(state_db_ctx) = state_db_ctx.as_deref()
&& let Ok(thread_id) = ThreadId::from_string(id_str)
{
let db_path = state_db::find_rollout_path_by_id(
Some(state_db_ctx),
thread_id,
archived_only,
"find_path_query",
)
.await;
if db_path.is_some() {
return Ok(db_path);
}
}
let mut root = codex_home.to_path_buf();
root.push(subdir);
if !root.exists() {
@@ -1084,33 +1116,11 @@ async fn find_thread_path_by_id_str_in_subdir(
.map_err(|e| io::Error::other(format!("file search failed: {e}")))?;
let found = results.matches.into_iter().next().map(|m| m.full_path());
// Checking if DB is at parity.
// TODO(jif): sqlite migration phase 1
let archived_only = match subdir {
SESSIONS_SUBDIR => Some(false),
ARCHIVED_SESSIONS_SUBDIR => Some(true),
_ => None,
};
let state_db_ctx = state_db::open_if_present(codex_home, "").await;
if let Some(state_db_ctx) = state_db_ctx.as_deref()
&& let Ok(thread_id) = ThreadId::from_string(id_str)
{
let db_path = state_db::find_rollout_path_by_id(
Some(state_db_ctx),
thread_id,
archived_only,
"find_path_query",
)
.await;
let canonical_path = found.as_deref();
if db_path.as_deref() != canonical_path {
tracing::warn!(
"state db path mismatch for thread {thread_id:?}: canonical={canonical_path:?} db={db_path:?}"
);
state_db::record_discrepancy("find_thread_path_by_id_str_in_subdir", "path_mismatch");
}
if found.is_some() {
tracing::error!("state db missing rollout path for thread {id_str}");
state_db::record_discrepancy("find_thread_path_by_id_str_in_subdir", "path_mismatch");
}
Ok(found)
}
+23
View File
@@ -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) => {
@@ -1,3 +1,5 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::fs::File;
use std::io::Read;
use std::io::Seek;
@@ -8,6 +10,7 @@ use std::path::PathBuf;
use codex_protocol::ThreadId;
use serde::Deserialize;
use serde::Serialize;
use tokio::io::AsyncBufReadExt;
use tokio::io::AsyncWriteExt;
const SESSION_INDEX_FILE: &str = "session_index.jsonl";
@@ -76,6 +79,38 @@ pub async fn find_thread_name_by_id(
Ok(entry.map(|entry| entry.thread_name))
}
/// Find the latest thread names for a batch of thread ids.
pub async fn find_thread_names_by_ids(
codex_home: &Path,
thread_ids: &HashSet<ThreadId>,
) -> std::io::Result<HashMap<ThreadId, String>> {
let path = session_index_path(codex_home);
if thread_ids.is_empty() || !path.exists() {
return Ok(HashMap::new());
}
let file = tokio::fs::File::open(&path).await?;
let reader = tokio::io::BufReader::new(file);
let mut lines = reader.lines();
let mut names = HashMap::with_capacity(thread_ids.len());
while let Some(line) = lines.next_line().await? {
let trimmed = line.trim();
if trimmed.is_empty() {
continue;
}
let Ok(entry) = serde_json::from_str::<SessionIndexEntry>(trimmed) else {
continue;
};
let name = entry.thread_name.trim();
if !name.is_empty() && thread_ids.contains(&entry.id) {
names.insert(entry.id, name.to_string());
}
}
Ok(names)
}
/// Find the most recently updated thread id for a thread name, if any.
pub async fn find_thread_id_by_name(
codex_home: &Path,
@@ -197,6 +232,8 @@ where
mod tests {
use super::*;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::collections::HashSet;
use tempfile::TempDir;
fn write_index(path: &Path, lines: &[SessionIndexEntry]) -> std::io::Result<()> {
let mut out = String::new();
@@ -279,6 +316,44 @@ mod tests {
Ok(())
}
#[tokio::test]
async fn find_thread_names_by_ids_prefers_latest_entry() -> std::io::Result<()> {
let temp = TempDir::new()?;
let path = session_index_path(temp.path());
let id1 = ThreadId::new();
let id2 = ThreadId::new();
let lines = vec![
SessionIndexEntry {
id: id1,
thread_name: "first".to_string(),
updated_at: "2024-01-01T00:00:00Z".to_string(),
},
SessionIndexEntry {
id: id2,
thread_name: "other".to_string(),
updated_at: "2024-01-01T00:00:00Z".to_string(),
},
SessionIndexEntry {
id: id1,
thread_name: "latest".to_string(),
updated_at: "2024-01-02T00:00:00Z".to_string(),
},
];
write_index(&path, &lines)?;
let mut ids = HashSet::new();
ids.insert(id1);
ids.insert(id2);
let mut expected = HashMap::new();
expected.insert(id1, "latest".to_string());
expected.insert(id2, "other".to_string());
let found = find_thread_names_by_ids(temp.path(), &ids).await?;
assert_eq!(found, expected);
Ok(())
}
#[test]
fn scan_index_finds_latest_match_among_mixed_entries() -> std::io::Result<()> {
let temp = TempDir::new()?;
+1
View File
@@ -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)?)?;
+2
View File
@@ -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,
}
}
+11
View File
@@ -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
}
}
+1 -1
View File
@@ -29,7 +29,7 @@ pub struct ShellSnapshot {
}
const SNAPSHOT_TIMEOUT: Duration = Duration::from_secs(10);
const SNAPSHOT_RETENTION: Duration = Duration::from_secs(60 * 60 * 24 * 7); // 7 days retention.
const SNAPSHOT_RETENTION: Duration = Duration::from_secs(60 * 60 * 24 * 3); // 3 days retention.
const SNAPSHOT_DIR: &str = "shell_snapshots";
const EXCLUDED_EXPORT_VARS: &[&str] = &["PWD", "OLDPWD"];
@@ -11,7 +11,7 @@ This skill provides guidance for creating effective skills.
## About Skills
Skills are modular, self-contained packages that extend Codex's capabilities by providing
Skills are modular, self-contained folders that extend Codex's capabilities by providing
specialized knowledge, workflows, and tools. Think of them as "onboarding guides" for specific
domains or tasks—they transform Codex from a general-purpose agent into a specialized agent
equipped with procedural knowledge that no model can fully possess.
@@ -56,6 +56,8 @@ skill-name/
│ │ ├── name: (required)
│ │ └── description: (required)
│ └── Markdown instructions (required)
├── agents/ (recommended)
│ └── openai.yaml - UI metadata for skill lists and chips
└── Bundled Resources (optional)
├── scripts/ - Executable code (Python/Bash/etc.)
├── references/ - Documentation intended to be loaded into context as needed
@@ -69,6 +71,16 @@ Every SKILL.md consists of:
- **Frontmatter** (YAML): Contains `name` and `description` fields. These are the only fields that Codex reads to determine when the skill gets used, thus it is very important to be clear and comprehensive in describing what the skill is, and when it should be used.
- **Body** (Markdown): Instructions and guidance for using the skill. Only loaded AFTER the skill triggers (if at all).
#### Agents metadata (recommended)
- UI-facing metadata for skill lists and chips
- Read references/openai_yaml.md before generating values and follow its descriptions and constraints
- Create: human-facing `display_name`, `short_description`, and `default_prompt` by reading the skill
- Generate deterministically by passing the values as `--interface key=value` to `scripts/generate_openai_yaml.py` or `scripts/init_skill.py`
- On updates: validate `agents/openai.yaml` still matches SKILL.md; regenerate if stale
- Only include other optional interface fields (icons, brand color) if explicitly provided
- See references/openai_yaml.md for field definitions and examples
#### Bundled Resources (optional)
##### Scripts (`scripts/`)
@@ -208,7 +220,7 @@ Skill creation involves these steps:
2. Plan reusable skill contents (scripts, references, assets)
3. Initialize the skill (run init_skill.py)
4. Edit the skill (implement resources and write SKILL.md)
5. Package the skill (run package_skill.py)
5. Validate the skill (run quick_validate.py)
6. Iterate based on real usage
Follow these steps in order, skipping only if there is a clear reason why they are not applicable.
@@ -266,7 +278,7 @@ To establish the skill's contents, analyze each concrete example to create a lis
At this point, it is time to actually create the skill.
Skip this step only if the skill being developed already exists, and iteration or packaging is needed. In this case, continue to the next step.
Skip this step only if the skill being developed already exists. In this case, continue to the next step.
When creating a new skill from scratch, always run the `init_skill.py` script. The script conveniently generates a new template skill directory that automatically includes everything a skill requires, making the skill creation process much more efficient and reliable.
@@ -288,11 +300,20 @@ The script:
- Creates the skill directory at the specified path
- Generates a SKILL.md template with proper frontmatter and TODO placeholders
- Creates `agents/openai.yaml` using agent-generated `display_name`, `short_description`, and `default_prompt` passed via `--interface key=value`
- Optionally creates resource directories based on `--resources`
- Optionally adds example files when `--examples` is set
After initialization, customize the SKILL.md and add resources as needed. If you used `--examples`, replace or delete placeholder files.
Generate `display_name`, `short_description`, and `default_prompt` by reading the skill, then pass them as `--interface key=value` to `init_skill.py` or regenerate with:
```bash
scripts/generate_openai_yaml.py <path/to/skill-folder> --interface key=value
```
Only include other optional interface fields when the user explicitly provides them. For full field descriptions and examples, see references/openai_yaml.md.
### Step 4: Edit the Skill
When editing the (newly-generated or existing) skill, remember that the skill is being created for another instance of Codex to use. Include information that would be beneficial and non-obvious to Codex. Consider what procedural knowledge, domain-specific details, or reusable assets would help another Codex instance execute these tasks more effectively.
@@ -328,40 +349,21 @@ Write the YAML frontmatter with `name` and `description`:
- Include all "when to use" information here - Not in the body. The body is only loaded after triggering, so "When to Use This Skill" sections in the body are not helpful to Codex.
- Example description for a `docx` skill: "Comprehensive document creation, editing, and analysis with support for tracked changes, comments, formatting preservation, and text extraction. Use when Codex needs to work with professional documents (.docx files) for: (1) Creating new documents, (2) Modifying or editing content, (3) Working with tracked changes, (4) Adding comments, or any other document tasks"
Ensure the frontmatter is valid YAML. Keep `name` and `description` as single-line scalars. If either could be interpreted as YAML syntax, wrap it in quotes.
Do not include any other fields in YAML frontmatter.
##### Body
Write instructions for using the skill and its bundled resources.
### Step 5: Packaging a Skill
### Step 5: Validate the Skill
Once development of the skill is complete, it must be packaged into a distributable .skill file that gets shared with the user. The packaging process automatically validates the skill first to ensure it meets all requirements:
Once development of the skill is complete, validate the skill folder to catch basic issues early:
```bash
scripts/package_skill.py <path/to/skill-folder>
scripts/quick_validate.py <path/to/skill-folder>
```
Optional output directory specification:
```bash
scripts/package_skill.py <path/to/skill-folder> ./dist
```
The packaging script will:
1. **Validate** the skill automatically, checking:
- YAML frontmatter format and required fields
- Skill naming conventions and directory structure
- Description completeness and quality
- File organization and resource references
2. **Package** the skill if validation passes, creating a .skill file named after the skill (e.g., `my-skill.skill`) that includes all files and maintains the proper directory structure for distribution. The .skill file is a zip file with a .skill extension.
If validation fails, the script will report the errors and exit without creating a package. Fix any validation errors and run the packaging command again.
The validation script checks YAML frontmatter format, required fields, and naming rules. If validation fails, fix the reported issues and run the command again.
### Step 6: Iterate
@@ -0,0 +1,5 @@
interface:
display_name: "Skill Creator"
short_description: "Create or update a skill"
icon_small: "./assets/skill-creator-small.svg"
icon_large: "./assets/skill-creator.png"
@@ -0,0 +1,3 @@
<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="currentColor" viewBox="0 0 20 20">
<path fill="#0D0D0D" d="M12.03 4.113a3.612 3.612 0 0 1 5.108 5.108l-6.292 6.29c-.324.324-.56.561-.791.752l-.235.176c-.205.14-.422.261-.65.36l-.229.093a4.136 4.136 0 0 1-.586.16l-.764.134-2.394.4c-.142.024-.294.05-.423.06-.098.007-.232.01-.378-.026l-.149-.05a1.081 1.081 0 0 1-.521-.474l-.046-.093a1.104 1.104 0 0 1-.075-.527c.01-.129.035-.28.06-.422l.398-2.394c.1-.602.162-.987.295-1.35l.093-.23c.1-.228.22-.445.36-.65l.176-.235c.19-.232.428-.467.751-.79l6.292-6.292Zm-5.35 7.232c-.35.35-.534.535-.66.688l-.11.147a2.67 2.67 0 0 0-.24.433l-.062.154c-.08.22-.124.462-.232 1.112l-.398 2.394-.001.001h.003l2.393-.399.717-.126a2.63 2.63 0 0 0 .394-.105l.154-.063a2.65 2.65 0 0 0 .433-.24l.147-.11c.153-.126.339-.31.688-.66l4.988-4.988-3.227-3.226-4.987 4.988Zm9.517-6.291a2.281 2.281 0 0 0-3.225 0l-.364.362 3.226 3.227.363-.364c.89-.89.89-2.334 0-3.225ZM4.583 1.783a.3.3 0 0 1 .294.241c.117.585.347 1.092.707 1.48.357.385.859.668 1.549.783a.3.3 0 0 1 0 .592c-.69.115-1.192.398-1.549.783-.315.34-.53.77-.657 1.265l-.05.215a.3.3 0 0 1-.588 0c-.117-.585-.347-1.092-.707-1.48-.357-.384-.859-.668-1.549-.783a.3.3 0 0 1 0-.592c.69-.115 1.192-.398 1.549-.783.36-.388.59-.895.707-1.48l.015-.05a.3.3 0 0 1 .279-.19Z"/>
</svg>

After

Width:  |  Height:  |  Size: 1.3 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 1.5 KiB

@@ -0,0 +1,43 @@
# openai.yaml fields (full example + descriptions)
`agents/openai.yaml` is an extended, product-specific config intended for the machine/harness to read, not the agent. Other product-specific config can also live in the `agents/` folder.
## Full example
```yaml
interface:
display_name: "Optional user-facing name"
short_description: "Optional user-facing description"
icon_small: "./assets/small-400px.png"
icon_large: "./assets/large-logo.svg"
brand_color: "#3B82F6"
default_prompt: "Optional surrounding prompt to use the skill with"
dependencies:
tools:
- type: "mcp"
value: "github"
description: "GitHub MCP server"
transport: "streamable_http"
url: "https://api.githubcopilot.com/mcp/"
```
## Field descriptions and constraints
Top-level constraints:
- Quote all string values.
- Keep keys unquoted.
- For `interface.default_prompt`: generate a helpful, short (typically 1 sentence) example starting prompt based on the skill. It must explicitly mention the skill as `$skill-name` (e.g., "Use $skill-name-here to draft a concise weekly status update.").
- `interface.display_name`: Human-facing title shown in UI skill lists and chips.
- `interface.short_description`: Human-facing short UI blurb (25–64 chars) for quick scanning.
- `interface.icon_small`: Path to a small icon asset (relative to skill dir). Default to `./assets/` and place icons in the skill's `assets/` folder.
- `interface.icon_large`: Path to a larger logo asset (relative to skill dir). Default to `./assets/` and place icons in the skill's `assets/` folder.
- `interface.brand_color`: Hex color used for UI accents (e.g., badges).
- `interface.default_prompt`: Default prompt snippet inserted when invoking the skill.
- `dependencies.tools[].type`: Dependency category. Only `mcp` is supported for now.
- `dependencies.tools[].value`: Identifier of the tool or dependency.
- `dependencies.tools[].description`: Human-readable explanation of the dependency.
- `dependencies.tools[].transport`: Connection type when `type` is `mcp`.
- `dependencies.tools[].url`: MCP server URL when `type` is `mcp`.
@@ -0,0 +1,225 @@
#!/usr/bin/env python3
"""
OpenAI YAML Generator - Creates agents/openai.yaml for a skill folder.
Usage:
generate_openai_yaml.py <skill_dir> [--name <skill_name>] [--interface key=value]
"""
import argparse
import re
import sys
from pathlib import Path
import yaml
ACRONYMS = {
"GH",
"MCP",
"API",
"CI",
"CLI",
"LLM",
"PDF",
"PR",
"UI",
"URL",
"SQL",
}
BRANDS = {
"openai": "OpenAI",
"openapi": "OpenAPI",
"github": "GitHub",
"pagerduty": "PagerDuty",
"datadog": "DataDog",
"sqlite": "SQLite",
"fastapi": "FastAPI",
}
SMALL_WORDS = {"and", "or", "to", "up", "with"}
ALLOWED_INTERFACE_KEYS = {
"display_name",
"short_description",
"icon_small",
"icon_large",
"brand_color",
"default_prompt",
}
def yaml_quote(value):
escaped = value.replace("\\", "\\\\").replace('"', '\\"').replace("\n", "\\n")
return f'"{escaped}"'
def format_display_name(skill_name):
words = [word for word in skill_name.split("-") if word]
formatted = []
for index, word in enumerate(words):
lower = word.lower()
upper = word.upper()
if upper in ACRONYMS:
formatted.append(upper)
continue
if lower in BRANDS:
formatted.append(BRANDS[lower])
continue
if index > 0 and lower in SMALL_WORDS:
formatted.append(lower)
continue
formatted.append(word.capitalize())
return " ".join(formatted)
def generate_short_description(display_name):
description = f"Help with {display_name} tasks"
if len(description) < 25:
description = f"Help with {display_name} tasks and workflows"
if len(description) < 25:
description = f"Help with {display_name} tasks with guidance"
if len(description) > 64:
description = f"Help with {display_name}"
if len(description) > 64:
description = f"{display_name} helper"
if len(description) > 64:
description = f"{display_name} tools"
if len(description) > 64:
suffix = " helper"
max_name_length = 64 - len(suffix)
trimmed = display_name[:max_name_length].rstrip()
description = f"{trimmed}{suffix}"
if len(description) > 64:
description = description[:64].rstrip()
if len(description) < 25:
description = f"{description} workflows"
if len(description) > 64:
description = description[:64].rstrip()
return description
def read_frontmatter_name(skill_dir):
skill_md = Path(skill_dir) / "SKILL.md"
if not skill_md.exists():
print(f"[ERROR] SKILL.md not found in {skill_dir}")
return None
content = skill_md.read_text()
match = re.match(r"^---\n(.*?)\n---", content, re.DOTALL)
if not match:
print("[ERROR] Invalid SKILL.md frontmatter format.")
return None
frontmatter_text = match.group(1)
try:
frontmatter = yaml.safe_load(frontmatter_text)
except yaml.YAMLError as exc:
print(f"[ERROR] Invalid YAML frontmatter: {exc}")
return None
if not isinstance(frontmatter, dict):
print("[ERROR] Frontmatter must be a YAML dictionary.")
return None
name = frontmatter.get("name", "")
if not isinstance(name, str) or not name.strip():
print("[ERROR] Frontmatter 'name' is missing or invalid.")
return None
return name.strip()
def parse_interface_overrides(raw_overrides):
overrides = {}
optional_order = []
for item in raw_overrides:
if "=" not in item:
print(f"[ERROR] Invalid interface override '{item}'. Use key=value.")
return None, None
key, value = item.split("=", 1)
key = key.strip()
value = value.strip()
if not key:
print(f"[ERROR] Invalid interface override '{item}'. Key is empty.")
return None, None
if key not in ALLOWED_INTERFACE_KEYS:
allowed = ", ".join(sorted(ALLOWED_INTERFACE_KEYS))
print(f"[ERROR] Unknown interface field '{key}'. Allowed: {allowed}")
return None, None
overrides[key] = value
if key not in ("display_name", "short_description") and key not in optional_order:
optional_order.append(key)
return overrides, optional_order
def write_openai_yaml(skill_dir, skill_name, raw_overrides):
overrides, optional_order = parse_interface_overrides(raw_overrides)
if overrides is None:
return None
display_name = overrides.get("display_name") or format_display_name(skill_name)
short_description = overrides.get("short_description") or generate_short_description(display_name)
if not (25 <= len(short_description) <= 64):
print(
"[ERROR] short_description must be 25-64 characters "
f"(got {len(short_description)})."
)
return None
interface_lines = [
"interface:",
f" display_name: {yaml_quote(display_name)}",
f" short_description: {yaml_quote(short_description)}",
]
for key in optional_order:
value = overrides.get(key)
if value is not None:
interface_lines.append(f" {key}: {yaml_quote(value)}")
agents_dir = Path(skill_dir) / "agents"
agents_dir.mkdir(parents=True, exist_ok=True)
output_path = agents_dir / "openai.yaml"
output_path.write_text("\n".join(interface_lines) + "\n")
print(f"[OK] Created agents/openai.yaml")
return output_path
def main():
parser = argparse.ArgumentParser(
description="Create agents/openai.yaml for a skill directory.",
)
parser.add_argument("skill_dir", help="Path to the skill directory")
parser.add_argument(
"--name",
help="Skill name override (defaults to SKILL.md frontmatter)",
)
parser.add_argument(
"--interface",
action="append",
default=[],
help="Interface override in key=value format (repeatable)",
)
args = parser.parse_args()
skill_dir = Path(args.skill_dir).resolve()
if not skill_dir.exists():
print(f"[ERROR] Skill directory not found: {skill_dir}")
sys.exit(1)
if not skill_dir.is_dir():
print(f"[ERROR] Path is not a directory: {skill_dir}")
sys.exit(1)
skill_name = args.name or read_frontmatter_name(skill_dir)
if not skill_name:
sys.exit(1)
result = write_openai_yaml(skill_dir, skill_name, args.interface)
if result:
sys.exit(0)
sys.exit(1)
if __name__ == "__main__":
main()
@@ -3,13 +3,14 @@
Skill Initializer - Creates a new skill from template
Usage:
init_skill.py <skill-name> --path <path> [--resources scripts,references,assets] [--examples]
init_skill.py <skill-name> --path <path> [--resources scripts,references,assets] [--examples] [--interface key=value]
Examples:
init_skill.py my-new-skill --path skills/public
init_skill.py my-new-skill --path skills/public --resources scripts,references
init_skill.py my-api-helper --path skills/private --resources scripts --examples
init_skill.py custom-skill --path /custom/location
init_skill.py my-skill --path skills/public --interface short_description="Short UI label"
"""
import argparse
@@ -17,6 +18,8 @@ import re
import sys
from pathlib import Path
from generate_openai_yaml import write_openai_yaml
MAX_SKILL_NAME_LENGTH = 64
ALLOWED_RESOURCES = {"scripts", "references", "assets"}
@@ -252,7 +255,7 @@ def create_resource_dirs(skill_dir, skill_name, skill_title, resources, include_
print("[OK] Created assets/")
def init_skill(skill_name, path, resources, include_examples):
def init_skill(skill_name, path, resources, include_examples, interface_overrides):
"""
Initialize a new skill directory with template SKILL.md.
@@ -293,6 +296,15 @@ def init_skill(skill_name, path, resources, include_examples):
print(f"[ERROR] Error creating SKILL.md: {e}")
return None
# Create agents/openai.yaml
try:
result = write_openai_yaml(skill_dir, skill_name, interface_overrides)
if not result:
return None
except Exception as e:
print(f"[ERROR] Error creating agents/openai.yaml: {e}")
return None
# Create resource directories if requested
if resources:
try:
@@ -312,7 +324,8 @@ def init_skill(skill_name, path, resources, include_examples):
print("2. Add resources to scripts/, references/, and assets/ as needed")
else:
print("2. Create resource directories only if needed (scripts/, references/, assets/)")
print("3. Run the validator when ready to check the skill structure")
print("3. Update agents/openai.yaml if the UI metadata should differ")
print("4. Run the validator when ready to check the skill structure")
return skill_dir
@@ -333,6 +346,12 @@ def main():
action="store_true",
help="Create example files inside the selected resource directories",
)
parser.add_argument(
"--interface",
action="append",
default=[],
help="Interface override in key=value format (repeatable)",
)
args = parser.parse_args()
raw_skill_name = args.skill_name
@@ -366,7 +385,7 @@ def main():
print(" Resources: none (create as needed)")
print()
result = init_skill(skill_name, path, resources, args.examples)
result = init_skill(skill_name, path, resources, args.examples, args.interface)
if result:
sys.exit(0)
@@ -1,111 +0,0 @@
#!/usr/bin/env python3
"""
Skill Packager - Creates a distributable .skill file of a skill folder
Usage:
python utils/package_skill.py <path/to/skill-folder> [output-directory]
Example:
python utils/package_skill.py skills/public/my-skill
python utils/package_skill.py skills/public/my-skill ./dist
"""
import sys
import zipfile
from pathlib import Path
from quick_validate import validate_skill
def package_skill(skill_path, output_dir=None):
"""
Package a skill folder into a .skill file.
Args:
skill_path: Path to the skill folder
output_dir: Optional output directory for the .skill file (defaults to current directory)
Returns:
Path to the created .skill file, or None if error
"""
skill_path = Path(skill_path).resolve()
# Validate skill folder exists
if not skill_path.exists():
print(f"[ERROR] Skill folder not found: {skill_path}")
return None
if not skill_path.is_dir():
print(f"[ERROR] Path is not a directory: {skill_path}")
return None
# Validate SKILL.md exists
skill_md = skill_path / "SKILL.md"
if not skill_md.exists():
print(f"[ERROR] SKILL.md not found in {skill_path}")
return None
# Run validation before packaging
print("Validating skill...")
valid, message = validate_skill(skill_path)
if not valid:
print(f"[ERROR] Validation failed: {message}")
print(" Please fix the validation errors before packaging.")
return None
print(f"[OK] {message}\n")
# Determine output location
skill_name = skill_path.name
if output_dir:
output_path = Path(output_dir).resolve()
output_path.mkdir(parents=True, exist_ok=True)
else:
output_path = Path.cwd()
skill_filename = output_path / f"{skill_name}.skill"
# Create the .skill file (zip format)
try:
with zipfile.ZipFile(skill_filename, "w", zipfile.ZIP_DEFLATED) as zipf:
# Walk through the skill directory
for file_path in skill_path.rglob("*"):
if file_path.is_file():
# Calculate the relative path within the zip
arcname = file_path.relative_to(skill_path.parent)
zipf.write(file_path, arcname)
print(f" Added: {arcname}")
print(f"\n[OK] Successfully packaged skill to: {skill_filename}")
return skill_filename
except Exception as e:
print(f"[ERROR] Error creating .skill file: {e}")
return None
def main():
if len(sys.argv) < 2:
print("Usage: python utils/package_skill.py <path/to/skill-folder> [output-directory]")
print("\nExample:")
print(" python utils/package_skill.py skills/public/my-skill")
print(" python utils/package_skill.py skills/public/my-skill ./dist")
sys.exit(1)
skill_path = sys.argv[1]
output_dir = sys.argv[2] if len(sys.argv) > 2 else None
print(f"Packaging skill: {skill_path}")
if output_dir:
print(f" Output directory: {output_dir}")
print()
result = package_skill(skill_path, output_dir)
if result:
sys.exit(0)
else:
sys.exit(1)
if __name__ == "__main__":
main()
@@ -7,10 +7,10 @@ metadata:
# Skill Installer
Helps install skills. By default these are from https://github.com/openai/skills/tree/main/skills/.curated, but users can also provide other locations.
Helps install skills. By default these are from https://github.com/openai/skills/tree/main/skills/.curated, but users can also provide other locations. Experimental skills live in https://github.com/openai/skills/tree/main/skills/.experimental and can be installed the same way.
Use the helper scripts based on the task:
- List curated skills when the user asks what is available, or if the user uses this skill without specifying what to do.
- List skills when the user asks what is available, or if the user uses this skill without specifying what to do. Default listing is `.curated`, but you can pass `--path skills/.experimental` when they ask about experimental skills.
- Install from the curated list when the user provides a skill name.
- Install from another repo when the user provides a GitHub repo/path (including private repos).
@@ -18,7 +18,7 @@ Install skills with the helper scripts.
## Communication
When listing curated skills, output approximately as follows, depending on the context of the user's request:
When listing skills, output approximately as follows, depending on the context of the user's request. If they ask about experimental skills, list from `.experimental` instead of `.curated` and label the source accordingly:
"""
Skills from {repo}:
1. skill-1
@@ -33,10 +33,12 @@ After installing a skill, tell the user: "Restart Codex to pick up new skills."
All of these scripts use network, so when running in the sandbox, request escalation when running them.
- `scripts/list-curated-skills.py` (prints curated list with installed annotations)
- `scripts/list-curated-skills.py --format json`
- `scripts/list-skills.py` (prints skills list with installed annotations)
- `scripts/list-skills.py --format json`
- Example (experimental list): `scripts/list-skills.py --path skills/.experimental`
- `scripts/install-skill-from-github.py --repo <owner>/<repo> --path <path/to/skill> [<path/to/skill> ...]`
- `scripts/install-skill-from-github.py --url https://github.com/<owner>/<repo>/tree/<ref>/<path>`
- Example (experimental skill): `scripts/install-skill-from-github.py --repo openai/skills --path skills/.experimental/<skill-name>`
## Behavior and Options
@@ -0,0 +1,5 @@
interface:
display_name: "Skill Installer"
short_description: "Install curated skills from openai/skills or other repos"
icon_small: "./assets/skill-installer-small.svg"
icon_large: "./assets/skill-installer.png"
@@ -0,0 +1,3 @@
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" viewBox="0 0 16 16">
<path fill="#0D0D0D" d="M2.145 3.959a2.033 2.033 0 0 1 2.022-1.824h5.966c.551 0 .997 0 1.357.029.367.03.692.093.993.246l.174.098c.397.243.72.593.932 1.01l.053.114c.116.269.168.557.194.878.03.36.03.805.03 1.357v4.3a2.365 2.365 0 0 1-2.366 2.365h-1.312a2.198 2.198 0 0 1-4.377 0H4.167A2.032 2.032 0 0 1 2.135 10.5V9.333l.004-.088A.865.865 0 0 1 3 8.468l.116-.006A1.135 1.135 0 0 0 3 6.199a.865.865 0 0 1-.865-.864V4.167l.01-.208Zm1.054 1.186a2.198 2.198 0 0 1 0 4.376v.98c0 .534.433.967.968.967H6l.089.004a.866.866 0 0 1 .776.861 1.135 1.135 0 0 0 2.27 0c0-.478.387-.865.865-.865h1.5c.719 0 1.301-.583 1.301-1.301v-4.3c0-.57 0-.964-.025-1.27a1.933 1.933 0 0 0-.09-.493L12.642 4a1.47 1.47 0 0 0-.541-.585l-.102-.056c-.126-.065-.295-.11-.596-.135a17.31 17.31 0 0 0-1.27-.025H4.167a.968.968 0 0 0-.968.968v.978Z"/>
</svg>

After

Width:  |  Height:  |  Size: 923 B

Binary file not shown.

After

Width:  |  Height:  |  Size: 1.1 KiB

@@ -1,5 +1,5 @@
#!/usr/bin/env python3
"""List curated skills from a GitHub repo path."""
"""List skills from a GitHub repo path."""
from __future__ import annotations
@@ -47,28 +47,32 @@ def _installed_skills() -> set[str]:
return entries
def _list_curated(repo: str, path: str, ref: str) -> list[str]:
def _list_skills(repo: str, path: str, ref: str) -> list[str]:
api_url = github_api_contents_url(repo, path, ref)
try:
payload = _request(api_url)
except urllib.error.HTTPError as exc:
if exc.code == 404:
raise ListError(
"Curated skills path not found: "
"Skills path not found: "
f"https://github.com/{repo}/tree/{ref}/{path}"
) from exc
raise ListError(f"Failed to fetch curated skills: HTTP {exc.code}") from exc
raise ListError(f"Failed to fetch skills: HTTP {exc.code}") from exc
data = json.loads(payload.decode("utf-8"))
if not isinstance(data, list):
raise ListError("Unexpected curated listing response.")
raise ListError("Unexpected skills listing response.")
skills = [item["name"] for item in data if item.get("type") == "dir"]
return sorted(skills)
def _parse_args(argv: list[str]) -> Args:
parser = argparse.ArgumentParser(description="List curated skills.")
parser = argparse.ArgumentParser(description="List skills.")
parser.add_argument("--repo", default=DEFAULT_REPO)
parser.add_argument("--path", default=DEFAULT_PATH)
parser.add_argument(
"--path",
default=DEFAULT_PATH,
help="Repo path to list (default: skills/.curated)",
)
parser.add_argument("--ref", default=DEFAULT_REF)
parser.add_argument(
"--format",
@@ -82,7 +86,7 @@ def _parse_args(argv: list[str]) -> Args:
def main(argv: list[str]) -> int:
args = _parse_args(argv)
try:
skills = _list_curated(args.repo, args.path, args.ref)
skills = _list_skills(args.repo, args.path, args.ref)
installed = _installed_skills()
if args.format == "json":
payload = [
+13
View File
@@ -2,6 +2,9 @@ use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use crate::analytics_client::AnalyticsEventsClient;
use crate::analytics_client::SkillInvocation;
use crate::analytics_client::TrackEventsContext;
use crate::instructions::SkillInstructions;
use crate::skills::SkillMetadata;
use codex_otel::OtelManager;
@@ -18,6 +21,8 @@ pub(crate) struct SkillInjections {
pub(crate) async fn build_skill_injections(
mentioned_skills: &[SkillMetadata],
otel: Option<&OtelManager>,
analytics_client: &AnalyticsEventsClient,
tracking: TrackEventsContext,
) -> SkillInjections {
if mentioned_skills.is_empty() {
return SkillInjections::default();
@@ -27,11 +32,17 @@ pub(crate) async fn build_skill_injections(
items: Vec::with_capacity(mentioned_skills.len()),
warnings: Vec::new(),
};
let mut invocations = Vec::new();
for skill in mentioned_skills {
match fs::read_to_string(&skill.path).await {
Ok(contents) => {
emit_skill_injected_metric(otel, skill, "ok");
invocations.push(SkillInvocation {
skill_name: skill.name.clone(),
skill_scope: skill.scope,
skill_path: skill.path.clone(),
});
result.items.push(ResponseItem::from(SkillInstructions {
name: skill.name.clone(),
path: skill.path.to_string_lossy().into_owned(),
@@ -50,6 +61,8 @@ pub(crate) async fn build_skill_injections(
}
}
analytics_client.track_skill_invocations(tracking, invocations);
result
}
+212 -8
View File
@@ -1,6 +1,9 @@
use crate::config::Config;
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigLayerStackOrdering;
use crate::config_loader::default_project_root_markers;
use crate::config_loader::merge_toml_values;
use crate::config_loader::project_root_markers_from_config;
use crate::skills::model::SkillDependencies;
use crate::skills::model::SkillError;
use crate::skills::model::SkillInterface;
@@ -10,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;
@@ -20,6 +24,7 @@ use std::fs;
use std::path::Component;
use std::path::Path;
use std::path::PathBuf;
use toml::Value as TomlValue;
use tracing::error;
#[derive(Debug, Deserialize)]
@@ -72,6 +77,7 @@ struct DependencyTool {
}
const SKILLS_FILENAME: &str = "SKILL.md";
const AGENTS_DIR_NAME: &str = ".agents";
const SKILLS_METADATA_DIR: &str = "agents";
const SKILLS_METADATA_FILENAME: &str = "openai.yaml";
const SKILLS_DIR_NAME: &str = "skills";
@@ -159,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
@@ -177,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 {
@@ -209,13 +227,103 @@ fn skill_roots_from_layer_stack_inner(config_layer_stack: &ConfigLayerStack) ->
}
fn skill_roots(config: &Config) -> Vec<SkillRoot> {
skill_roots_from_layer_stack_inner(&config.config_layer_stack)
skill_roots_from_layer_stack_with_agents(&config.config_layer_stack, &config.cwd)
}
#[cfg(test)]
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, home_dir().as_deref());
roots.extend(repo_agents_skill_roots(config_layer_stack, cwd));
dedupe_skill_roots_by_path(&mut roots);
roots
}
fn dedupe_skill_roots_by_path(roots: &mut Vec<SkillRoot>) {
let mut seen: HashSet<PathBuf> = HashSet::new();
roots.retain(|root| seen.insert(root.path.clone()));
}
fn repo_agents_skill_roots(config_layer_stack: &ConfigLayerStack, cwd: &Path) -> Vec<SkillRoot> {
let project_root_markers = project_root_markers_from_stack(config_layer_stack);
let project_root = find_project_root(cwd, &project_root_markers);
let dirs = dirs_between_project_root_and_cwd(cwd, &project_root);
let mut roots = Vec::new();
for dir in dirs {
let agents_skills = dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME);
if agents_skills.is_dir() {
roots.push(SkillRoot {
path: agents_skills,
scope: SkillScope::Repo,
});
}
}
roots
}
fn project_root_markers_from_stack(config_layer_stack: &ConfigLayerStack) -> Vec<String> {
let mut merged = TomlValue::Table(toml::map::Map::new());
for layer in
config_layer_stack.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, false)
{
if matches!(layer.name, ConfigLayerSource::Project { .. }) {
continue;
}
merge_toml_values(&mut merged, &layer.config);
}
match project_root_markers_from_config(&merged) {
Ok(Some(markers)) => markers,
Ok(None) => default_project_root_markers(),
Err(err) => {
tracing::warn!("invalid project_root_markers: {err}");
default_project_root_markers()
}
}
}
fn find_project_root(cwd: &Path, project_root_markers: &[String]) -> PathBuf {
if project_root_markers.is_empty() {
return cwd.to_path_buf();
}
for ancestor in cwd.ancestors() {
for marker in project_root_markers {
let marker_path = ancestor.join(marker);
if marker_path.exists() {
return ancestor.to_path_buf();
}
}
}
cwd.to_path_buf()
}
fn dirs_between_project_root_and_cwd(cwd: &Path, project_root: &Path) -> Vec<PathBuf> {
let mut dirs = cwd
.ancestors()
.scan(false, |done, a| {
if *done {
None
} else {
if a == project_root {
*done = true;
}
Some(a.to_path_buf())
}
})
.collect::<Vec<_>>();
dirs.reverse();
dirs
}
fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut SkillLoadOutcome) {
@@ -735,7 +843,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)?;
@@ -759,7 +868,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<_>>();
@@ -768,6 +877,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")
@@ -783,7 +896,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");
@@ -812,7 +926,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<_>>();
@@ -822,6 +936,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")
@@ -832,6 +950,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)
}
@@ -1615,6 +1782,40 @@ interface:
);
}
#[tokio::test]
async fn loads_skills_from_agents_dir_without_codex_dir() {
let codex_home = tempfile::tempdir().expect("tempdir");
let repo_dir = tempfile::tempdir().expect("tempdir");
mark_as_git_repo(repo_dir.path());
let skill_path = write_skill_at(
&repo_dir.path().join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME),
"agents",
"agents-skill",
"from agents",
);
let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await;
let outcome = load_skills(&cfg);
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![SkillMetadata {
name: "agents-skill".to_string(),
description: "from agents".to_string(),
short_description: None,
interface: None,
dependencies: None,
path: normalized(&skill_path),
scope: SkillScope::Repo,
}]
);
}
#[tokio::test]
async fn loads_skills_from_all_codex_dirs_under_project_root() {
let codex_home = tempfile::tempdir().expect("tempdir");
@@ -2008,6 +2209,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);
}
+4 -3
View File
@@ -15,7 +15,7 @@ use crate::config_loader::LoaderOverrides;
use crate::config_loader::load_config_layers_state;
use crate::skills::SkillLoadOutcome;
use crate::skills::loader::load_skills_from_roots;
use crate::skills::loader::skill_roots_from_layer_stack;
use crate::skills::loader::skill_roots_from_layer_stack_with_agents;
use crate::skills::system::install_system_skills;
pub struct SkillsManager {
@@ -47,7 +47,8 @@ impl SkillsManager {
return outcome;
}
let roots = skill_roots_from_layer_stack(&config.config_layer_stack);
let roots =
skill_roots_from_layer_stack_with_agents(&config.config_layer_stack, &config.cwd);
let mut outcome = load_skills_from_roots(roots);
outcome.disabled_paths = disabled_paths_from_stack(&config.config_layer_stack);
match self.cache_by_cwd.write() {
@@ -105,7 +106,7 @@ impl SkillsManager {
}
};
let roots = skill_roots_from_layer_stack(&config_layer_stack);
let roots = skill_roots_from_layer_stack_with_agents(&config_layer_stack, cwd);
let mut outcome = load_skills_from_roots(roots);
outcome.disabled_paths = disabled_paths_from_stack(&config_layer_stack);
match self.cache_by_cwd.write() {
+4 -3
View File
@@ -24,9 +24,10 @@ pub fn render_skills_section(skills: &[SkillMetadata]) -> Option<String> {
- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.
- How to use a skill (progressive disclosure):
1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.
2) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.
3) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.
4) If `assets/` or templates exist, reuse them instead of recreating from scratch.
2) When `SKILL.md` references relative paths (e.g., `scripts/foo.py`), resolve them relative to the skill directory listed above first, and only consider other paths if needed.
3) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.
4) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.
5) If `assets/` or templates exist, reuse them instead of recreating from scratch.
- Coordination and sequencing:
- If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.
- Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.
+46 -15
View File
@@ -86,21 +86,8 @@ fn read_marker(path: &AbsolutePathBuf) -> Result<String, SystemSkillsError> {
}
fn embedded_system_skills_fingerprint() -> String {
let mut items: Vec<(String, Option<u64>)> = SYSTEM_SKILLS_DIR
.entries()
.iter()
.map(|entry| match entry {
include_dir::DirEntry::Dir(dir) => (dir.path().to_string_lossy().to_string(), None),
include_dir::DirEntry::File(file) => {
let mut file_hasher = DefaultHasher::new();
file.contents().hash(&mut file_hasher);
(
file.path().to_string_lossy().to_string(),
Some(file_hasher.finish()),
)
}
})
.collect();
let mut items = Vec::new();
collect_fingerprint_items(&SYSTEM_SKILLS_DIR, &mut items);
items.sort_unstable_by(|(a, _), (b, _)| a.cmp(b));
let mut hasher = DefaultHasher::new();
@@ -112,6 +99,25 @@ fn embedded_system_skills_fingerprint() -> String {
format!("{:x}", hasher.finish())
}
fn collect_fingerprint_items(dir: &Dir<'_>, items: &mut Vec<(String, Option<u64>)>) {
for entry in dir.entries() {
match entry {
include_dir::DirEntry::Dir(subdir) => {
items.push((subdir.path().to_string_lossy().to_string(), None));
collect_fingerprint_items(subdir, items);
}
include_dir::DirEntry::File(file) => {
let mut file_hasher = DefaultHasher::new();
file.contents().hash(&mut file_hasher);
items.push((
file.path().to_string_lossy().to_string(),
Some(file_hasher.finish()),
));
}
}
}
}
/// Writes the embedded `include_dir::Dir` to disk under `dest`.
///
/// Preserves the embedded directory structure.
@@ -163,3 +169,28 @@ impl SystemSkillsError {
Self::Io { action, source }
}
}
#[cfg(test)]
mod tests {
use super::SYSTEM_SKILLS_DIR;
use super::collect_fingerprint_items;
#[test]
fn fingerprint_traverses_nested_entries() {
let mut items = Vec::new();
collect_fingerprint_items(&SYSTEM_SKILLS_DIR, &mut items);
let mut paths: Vec<String> = items.into_iter().map(|(path, _)| path).collect();
paths.sort_unstable();
assert!(
paths
.binary_search_by(|probe| probe.as_str().cmp("skill-creator/SKILL.md"))
.is_ok()
);
assert!(
paths
.binary_search_by(|probe| probe.as_str().cmp("skill-creator/scripts/init_skill.py"))
.is_ok()
);
}
}
+2
View File
@@ -3,6 +3,7 @@ use std::sync::Arc;
use crate::AuthManager;
use crate::RolloutRecorder;
use crate::agent::AgentControl;
use crate::analytics_client::AnalyticsEventsClient;
use crate::exec_policy::ExecPolicyManager;
use crate::mcp_connection_manager::McpConnectionManager;
use crate::models_manager::manager::ModelsManager;
@@ -21,6 +22,7 @@ pub(crate) struct SessionServices {
pub(crate) mcp_connection_manager: Arc<RwLock<McpConnectionManager>>,
pub(crate) mcp_startup_cancellation_token: Mutex<CancellationToken>,
pub(crate) unified_exec_manager: UnifiedExecProcessManager,
pub(crate) analytics_events_client: AnalyticsEventsClient,
pub(crate) notifier: UserNotifier,
pub(crate) rollout: Mutex<Option<RolloutRecorder>>,
pub(crate) user_shell: Arc<crate::shell::Shell>,
+48 -1
View File
@@ -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()
);
}
}
@@ -277,7 +324,7 @@ pub async fn apply_rollout_items(
pub fn record_discrepancy(stage: &str, reason: &str) {
// We access the global metric because the call sites might not have access to the broader
// OtelManager.
tracing::warn!("state db record_discrepancy: {stage}{reason}");
tracing::warn!("state db record_discrepancy: {stage}, {reason}");
if let Some(metric) = codex_otel::metrics::global() {
let _ = metric.counter(
DB_METRIC_COMPARE_ERROR,
+1 -1
View File
@@ -48,7 +48,7 @@ pub(crate) async fn handle_output_item_done(
previously_active_item: Option<TurnItem>,
) -> Result<OutputItemResult> {
let mut output = OutputItemResult::default();
let plan_mode = ctx.turn_context.collaboration_mode_kind == ModeKind::Plan;
let plan_mode = ctx.turn_context.collaboration_mode.mode == ModeKind::Plan;
match ToolRouter::build_tool_call(ctx.sess.as_ref(), item.clone()).await {
// The model emitted a tool call; log it, persist the item immediately, and queue the tool execution.
+1
View File
@@ -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;
+2
View File
@@ -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;
+5 -2
View File
@@ -67,7 +67,7 @@ impl SessionTask for UserShellCommandTask {
let event = EventMsg::TurnStarted(TurnStartedEvent {
model_context_window: turn_context.client.get_model_context_window(),
collaboration_mode_kind: turn_context.collaboration_mode_kind,
collaboration_mode_kind: turn_context.collaboration_mode.mode,
});
let session = session.clone_session();
session.send_event(turn_context.as_ref(), event).await;
@@ -105,7 +105,10 @@ impl SessionTask for UserShellCommandTask {
let exec_env = ExecEnv {
command: exec_command.clone(),
cwd: cwd.clone(),
env: create_env(&turn_context.shell_environment_policy),
env: create_env(
&turn_context.shell_environment_policy,
Some(session.conversation_id),
),
// TODO(zhao-oai): Now that we have ExecExpiration::Cancellation, we
// should use that instead of an "arbitrarily large" timeout here.
expiration: USER_SHELL_TIMEOUT_MS.into(),
+2
View File
@@ -462,6 +462,7 @@ mod tests {
text: text.to_string(),
}],
end_turn: None,
phase: None,
}
}
fn assistant_msg(text: &str) -> ResponseItem {
@@ -472,6 +473,7 @@ mod tests {
text: text.to_string(),
}],
end_turn: None,
phase: None,
}
}
+1 -1
View File
@@ -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")],
};
+1
View File
@@ -27,6 +27,7 @@ pub use mcp_resource::McpResourceHandler;
pub use plan::PlanHandler;
pub use read_file::ReadFileHandler;
pub use request_user_input::RequestUserInputHandler;
pub(crate) use request_user_input::request_user_input_tool_description;
pub use shell::ShellCommandHandler;
pub use shell::ShellHandler;
pub use test_sync::TestSyncHandler;
+1 -1
View File
@@ -104,7 +104,7 @@ pub(crate) async fn handle_update_plan(
arguments: String,
_call_id: String,
) -> Result<String, FunctionCallError> {
if turn_context.collaboration_mode_kind == ModeKind::Plan {
if turn_context.collaboration_mode.mode == ModeKind::Plan {
return Err(FunctionCallError::RespondToModel(
"update_plan is a TODO/checklist tool and is not allowed in Plan mode".to_string(),
));
@@ -10,6 +10,56 @@ use crate::tools::registry::ToolKind;
use codex_protocol::config_types::ModeKind;
use codex_protocol::request_user_input::RequestUserInputArgs;
const REQUEST_USER_INPUT_ALLOWED_MODES: [ModeKind; 1] = [ModeKind::Plan];
fn request_user_input_mode_name(mode: ModeKind) -> &'static str {
match mode {
ModeKind::Plan => "Plan",
ModeKind::Default => "Default",
ModeKind::Execute => "Execute",
ModeKind::PairProgramming => "Pair Programming",
}
}
fn format_allowed_modes() -> String {
let mut mode_names = Vec::with_capacity(REQUEST_USER_INPUT_ALLOWED_MODES.len());
for mode in REQUEST_USER_INPUT_ALLOWED_MODES {
let name = request_user_input_mode_name(mode);
if !mode_names.contains(&name) {
mode_names.push(name);
}
}
match mode_names.as_slice() {
[] => "no modes".to_string(),
[mode] => format!("{mode} mode"),
[first, second] => format!("{first} or {second} mode"),
[..] => format!("modes: {}", mode_names.join(",")),
}
}
fn request_user_input_is_available_in_mode(mode: ModeKind) -> bool {
REQUEST_USER_INPUT_ALLOWED_MODES.contains(&mode)
}
pub(crate) fn request_user_input_unavailable_message(mode: ModeKind) -> Option<String> {
if request_user_input_is_available_in_mode(mode) {
None
} else {
let mode_name = request_user_input_mode_name(mode);
Some(format!(
"request_user_input is unavailable in {mode_name} mode"
))
}
}
pub(crate) fn request_user_input_tool_description() -> String {
let allowed_modes = format_allowed_modes();
format!(
"Request user input for one to three short questions and wait for the response. This tool is only available in {allowed_modes}."
)
}
pub struct RequestUserInputHandler;
#[async_trait]
@@ -37,16 +87,8 @@ impl ToolHandler for RequestUserInputHandler {
};
let mode = session.collaboration_mode().await.mode;
if !matches!(mode, ModeKind::Plan | ModeKind::PairProgramming) {
let mode_name = match mode {
ModeKind::Code => "Code",
ModeKind::Execute => "Execute",
ModeKind::Custom => "Custom",
ModeKind::Plan | ModeKind::PairProgramming => unreachable!(),
};
return Err(FunctionCallError::RespondToModel(format!(
"request_user_input is unavailable in {mode_name} mode"
)));
if let Some(message) = request_user_input_unavailable_message(mode) {
return Err(FunctionCallError::RespondToModel(message));
}
let mut args: RequestUserInputArgs = parse_arguments(&arguments)?;
@@ -84,3 +126,54 @@ impl ToolHandler for RequestUserInputHandler {
})
}
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn request_user_input_mode_availability_is_plan_only() {
assert_eq!(
request_user_input_is_available_in_mode(ModeKind::Plan),
true
);
assert_eq!(
request_user_input_is_available_in_mode(ModeKind::Default),
false
);
assert_eq!(
request_user_input_is_available_in_mode(ModeKind::Execute),
false
);
assert_eq!(
request_user_input_is_available_in_mode(ModeKind::PairProgramming),
false
);
}
#[test]
fn request_user_input_unavailable_messages_use_default_name_for_default_modes() {
assert_eq!(request_user_input_unavailable_message(ModeKind::Plan), None);
assert_eq!(
request_user_input_unavailable_message(ModeKind::Default),
Some("request_user_input is unavailable in Default mode".to_string())
);
assert_eq!(
request_user_input_unavailable_message(ModeKind::Execute),
Some("request_user_input is unavailable in Execute mode".to_string())
);
assert_eq!(
request_user_input_unavailable_message(ModeKind::PairProgramming),
Some("request_user_input is unavailable in Pair Programming mode".to_string())
);
}
#[test]
fn request_user_input_tool_description_mentions_plan_only() {
assert_eq!(
request_user_input_tool_description(),
"Request user input for one to three short questions and wait for the response. This tool is only available in Plan mode.".to_string()
);
}
}
+29 -8
View File
@@ -1,4 +1,5 @@
use async_trait::async_trait;
use codex_protocol::ThreadId;
use codex_protocol::models::ShellCommandToolCallParams;
use codex_protocol::models::ShellToolCallParams;
use std::sync::Arc;
@@ -41,12 +42,16 @@ struct RunExecLikeArgs {
}
impl ShellHandler {
fn to_exec_params(params: &ShellToolCallParams, turn_context: &TurnContext) -> ExecParams {
fn to_exec_params(
params: &ShellToolCallParams,
turn_context: &TurnContext,
thread_id: ThreadId,
) -> ExecParams {
ExecParams {
command: params.command.clone(),
cwd: turn_context.resolve_path(params.workdir.clone()),
expiration: params.timeout_ms.into(),
env: create_env(&turn_context.shell_environment_policy),
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
windows_sandbox_level: turn_context.windows_sandbox_level,
justification: params.justification.clone(),
@@ -65,6 +70,7 @@ impl ShellCommandHandler {
params: &ShellCommandToolCallParams,
session: &crate::codex::Session,
turn_context: &TurnContext,
thread_id: ThreadId,
) -> ExecParams {
let shell = session.user_shell();
let command = Self::base_command(shell.as_ref(), &params.command, params.login);
@@ -73,7 +79,7 @@ impl ShellCommandHandler {
command,
cwd: turn_context.resolve_path(params.workdir.clone()),
expiration: params.timeout_ms.into(),
env: create_env(&turn_context.shell_environment_policy),
env: create_env(&turn_context.shell_environment_policy, Some(thread_id)),
sandbox_permissions: params.sandbox_permissions.unwrap_or_default(),
windows_sandbox_level: turn_context.windows_sandbox_level,
justification: params.justification.clone(),
@@ -121,7 +127,8 @@ impl ToolHandler for ShellHandler {
ToolPayload::Function { arguments } => {
let params: ShellToolCallParams = parse_arguments(&arguments)?;
let prefix_rule = params.prefix_rule.clone();
let exec_params = Self::to_exec_params(&params, turn.as_ref());
let exec_params =
Self::to_exec_params(&params, turn.as_ref(), session.conversation_id);
Self::run_exec_like(RunExecLikeArgs {
tool_name: tool_name.clone(),
exec_params,
@@ -135,7 +142,8 @@ impl ToolHandler for ShellHandler {
.await
}
ToolPayload::LocalShell { params } => {
let exec_params = Self::to_exec_params(&params, turn.as_ref());
let exec_params =
Self::to_exec_params(&params, turn.as_ref(), session.conversation_id);
Self::run_exec_like(RunExecLikeArgs {
tool_name: tool_name.clone(),
exec_params,
@@ -197,7 +205,12 @@ impl ToolHandler for ShellCommandHandler {
let params: ShellCommandToolCallParams = parse_arguments(&arguments)?;
let prefix_rule = params.prefix_rule.clone();
let exec_params = Self::to_exec_params(&params, session.as_ref(), turn.as_ref());
let exec_params = Self::to_exec_params(
&params,
session.as_ref(),
turn.as_ref(),
session.conversation_id,
);
ShellHandler::run_exec_like(RunExecLikeArgs {
tool_name,
exec_params,
@@ -403,7 +416,10 @@ mod tests {
let expected_command = session.user_shell().derive_exec_args(&command, true);
let expected_cwd = turn_context.resolve_path(workdir.clone());
let expected_env = create_env(&turn_context.shell_environment_policy);
let expected_env = create_env(
&turn_context.shell_environment_policy,
Some(session.conversation_id),
);
let params = ShellCommandToolCallParams {
command,
@@ -415,7 +431,12 @@ mod tests {
justification: justification.clone(),
};
let exec_params = ShellCommandHandler::to_exec_params(&params, &session, &turn_context);
let exec_params = ShellCommandHandler::to_exec_params(
&params,
&session,
&turn_context,
session.conversation_id,
);
// ExecParams cannot derive Eq due to the CancellationToken field, so we manually compare the fields.
assert_eq!(exec_params.command, expected_command);
+2 -1
View File
@@ -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);
+146 -230
View File
@@ -9,6 +9,7 @@ use crate::tools::handlers::apply_patch::create_apply_patch_json_tool;
use crate::tools::handlers::collab::DEFAULT_WAIT_TIMEOUT_MS;
use crate::tools::handlers::collab::MAX_WAIT_TIMEOUT_MS;
use crate::tools::handlers::collab::MIN_WAIT_TIMEOUT_MS;
use crate::tools::handlers::request_user_input_tool_description;
use crate::tools::registry::ToolRegistryBuilder;
use codex_protocol::config_types::WebSearchMode;
use codex_protocol::dynamic_tools::DynamicToolSpec;
@@ -623,9 +624,7 @@ fn create_request_user_input_tool() -> ToolSpec {
ToolSpec::Function(ResponsesApiTool {
name: "request_user_input".to_string(),
description:
"Request user input for one to three short questions and wait for the response."
.to_string(),
description: request_user_input_tool_description(),
strict: false,
parameters: JsonSchema::Object {
properties,
@@ -1048,59 +1047,29 @@ pub fn create_tools_json_for_responses_api(
Ok(tools_json)
}
/// Returns JSON values that are compatible with Function Calling in the
/// Chat Completions API:
/// https://platform.openai.com/docs/guides/function-calling?api-mode=chat
pub(crate) fn create_tools_json_for_chat_completions_api(
tools: &[ToolSpec],
) -> crate::error::Result<Vec<serde_json::Value>> {
// We start with the JSON for the Responses API and than rewrite it to match
// the chat completions tool call format.
let responses_api_tools_json = create_tools_json_for_responses_api(tools)?;
let tools_json = responses_api_tools_json
.into_iter()
.filter_map(|mut tool| {
if tool.get("type") != Some(&serde_json::Value::String("function".to_string())) {
return None;
}
if let Some(map) = tool.as_object_mut() {
let name = map
.get("name")
.and_then(|v| v.as_str())
.unwrap_or_default()
.to_string();
// Remove "type" field as it is not needed in chat completions.
map.remove("type");
Some(json!({
"type": "function",
"name": name,
"function": map,
}))
} else {
None
}
})
.collect::<Vec<serde_json::Value>>();
Ok(tools_json)
}
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 +1077,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 +1222,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;
@@ -1289,13 +1257,19 @@ pub(crate) fn build_specs(
match &config.shell_type {
ConfigShellToolType::Default => {
builder.push_spec(create_shell_tool(config.request_rule_enabled));
builder.push_spec_with_parallel_support(
create_shell_tool(config.request_rule_enabled),
true,
);
}
ConfigShellToolType::Local => {
builder.push_spec(ToolSpec::LocalShell {});
builder.push_spec_with_parallel_support(ToolSpec::LocalShell {}, true);
}
ConfigShellToolType::UnifiedExec => {
builder.push_spec(create_exec_command_tool(config.request_rule_enabled));
builder.push_spec_with_parallel_support(
create_exec_command_tool(config.request_rule_enabled),
true,
);
builder.push_spec(create_write_stdin_tool());
builder.register_handler("exec_command", unified_exec_handler.clone());
builder.register_handler("write_stdin", unified_exec_handler);
@@ -1304,7 +1278,10 @@ pub(crate) fn build_specs(
// Do nothing.
}
ConfigShellToolType::ShellCommand => {
builder.push_spec(create_shell_command_tool(config.request_rule_enabled));
builder.push_spec_with_parallel_support(
create_shell_command_tool(config.request_rule_enabled),
true,
);
}
}
@@ -1410,7 +1387,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 +1429,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,
@@ -1974,7 +1990,7 @@ mod tests {
});
let (tools, _) = build_specs(&tools_config, None, &[]).build();
assert!(!find_tool(&tools, "exec_command").supports_parallel_tool_calls);
assert!(find_tool(&tools, "exec_command").supports_parallel_tool_calls);
assert!(!find_tool(&tools, "write_stdin").supports_parallel_tool_calls);
assert!(find_tool(&tools, "grep_files").supports_parallel_tool_calls);
assert!(find_tool(&tools, "list_dir").supports_parallel_tool_calls);
@@ -2026,37 +2042,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 +2125,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 +2172,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 +2224,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 +2273,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 +2324,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 +2432,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
}
}
}
}),
),
)])),
&[],
)
@@ -2613,26 +2550,5 @@ Examples of valid command strings:
},
})]
);
let tools_json = create_tools_json_for_chat_completions_api(&tools).unwrap();
assert_eq!(
tools_json,
vec![json!({
"type": "function",
"name": "demo",
"function": {
"name": "demo",
"description": "A demo tool",
"strict": false,
"parameters": {
"type": "object",
"properties": {
"foo": { "type": "string" }
},
},
}
})]
);
}
}
+43
View File
@@ -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()
}
@@ -483,7 +483,10 @@ impl UnifiedExecProcessManager {
cwd: PathBuf,
context: &UnifiedExecContext,
) -> Result<UnifiedExecProcess, UnifiedExecError> {
let env = apply_unified_exec_env(create_env(&context.turn.shell_environment_policy));
let env = apply_unified_exec_env(create_env(
&context.turn.shell_environment_policy,
Some(context.session.conversation_id),
));
let features = context.session.features();
let mut orchestrator = ToolOrchestrator::new();
let mut runtime = UnifiedExecRuntime::new(self);
+1
View File
@@ -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,
}
}
+19
View File
@@ -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,