Merge remote-tracking branch 'origin/main' into daniels-oai/approval-requirements-fallback-tests

This commit is contained in:
daniel-oai
2026-02-17 09:13:07 -08:00
48 changed files with 1605 additions and 336 deletions
+2 -4
View File
@@ -408,9 +408,8 @@ jobs:
shell: bash
run: |
set -euo pipefail
git clone --depth 1 https://git.code.sf.net/p/zsh/code /tmp/zsh
git clone https://git.code.sf.net/p/zsh/code /tmp/zsh
cd /tmp/zsh
git fetch --depth 1 origin 77045ef899e53b9598bebc5a41db93a548a40ca6
git checkout 77045ef899e53b9598bebc5a41db93a548a40ca6
git apply "${GITHUB_WORKSPACE}/shell-tool-mcp/patches/zsh-exec-wrapper.patch"
./Util/preconfig
@@ -487,9 +486,8 @@ jobs:
shell: bash
run: |
set -euo pipefail
git clone --depth 1 https://git.code.sf.net/p/zsh/code /tmp/zsh
git clone https://git.code.sf.net/p/zsh/code /tmp/zsh
cd /tmp/zsh
git fetch --depth 1 origin 77045ef899e53b9598bebc5a41db93a548a40ca6
git checkout 77045ef899e53b9598bebc5a41db93a548a40ca6
git apply "${GITHUB_WORKSPACE}/shell-tool-mcp/patches/zsh-exec-wrapper.patch"
./Util/preconfig
+1 -1
View File
@@ -557,7 +557,7 @@ Today both notifications carry an empty `items` array even when item events were
`ThreadItem` is the tagged union carried in turn responses and `item/*` notifications. Currently we support events for the following items:
- `userMessage` — `{id, content}` where `content` is a list of user inputs (`text`, `image`, or `localImage`).
- `userMessage` — `{id, content}` where `content` is a list of user inputs (`text`, `image`, or `localImage`). Cyber model-routing warnings are surfaced as synthetic `userMessage` items with `text` prefixed by `Warning:`.
- `agentMessage` — `{id, text}` containing the accumulated agent reply.
- `plan` — `{id, text}` emitted for plan-mode turns; plan text can stream via `item/plan/delta` (experimental).
- `reasoning` — `{id, summary, content}` where `summary` holds streamed reasoning summaries (applicable for most OpenAI models) and `content` holds raw reasoning blocks (applicable for e.g. open source models).
@@ -68,6 +68,7 @@ use codex_app_server_protocol::TurnInterruptResponse;
use codex_app_server_protocol::TurnPlanStep;
use codex_app_server_protocol::TurnPlanUpdatedNotification;
use codex_app_server_protocol::TurnStatus;
use codex_app_server_protocol::UserInput as V2UserInput;
use codex_app_server_protocol::build_turns_from_rollout_items;
use codex_core::CodexThread;
use codex_core::parse_command::shlex_join;
@@ -95,6 +96,8 @@ use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUse
use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse;
use std::collections::HashMap;
use std::convert::TryFrom;
use std::hash::Hash;
use std::hash::Hasher;
use std::path::PathBuf;
use std::sync::Arc;
use tokio::sync::Mutex;
@@ -122,6 +125,35 @@ pub(crate) async fn apply_bespoke_event_handling(
EventMsg::TurnComplete(_ev) => {
handle_turn_complete(conversation_id, event_turn_id, &outgoing, &thread_state).await;
}
EventMsg::Warning(warning_event) => {
if matches!(api_version, ApiVersion::V2)
&& is_safety_check_downgrade_warning(&warning_event.message)
{
let item = ThreadItem::UserMessage {
id: warning_item_id(&event_turn_id, &warning_event.message),
content: vec![V2UserInput::Text {
text: format!("Warning: {}", warning_event.message),
text_elements: Vec::new(),
}],
};
let started = ItemStartedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item: item.clone(),
};
outgoing
.send_server_notification(ServerNotification::ItemStarted(started))
.await;
let completed = ItemCompletedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item,
};
outgoing
.send_server_notification(ServerNotification::ItemCompleted(completed))
.await;
}
}
EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
call_id,
turn_id,
@@ -1286,6 +1318,18 @@ async fn complete_command_execution_item(
.await;
}
fn is_safety_check_downgrade_warning(message: &str) -> bool {
message.contains("Your account was flagged for potentially high-risk cyber activity")
&& message.contains("apply for trusted access: https://chatgpt.com/cyber")
}
fn warning_item_id(turn_id: &str, message: &str) -> String {
let mut hasher = std::collections::hash_map::DefaultHasher::new();
message.hash(&mut hasher);
let digest = hasher.finish();
format!("{turn_id}-warning-{digest:x}")
}
async fn maybe_emit_raw_response_item_completed(
api_version: ApiVersion,
conversation_id: ThreadId,
@@ -2016,6 +2060,18 @@ mod tests {
assert_eq!(item, expected);
}
#[test]
fn safety_check_downgrade_warning_detection_matches_expected_message() {
let warning = "Your account was flagged for potentially high-risk cyber activity and this request was routed to gpt-5.2 as a fallback. To regain access to gpt-5.3-codex, apply for trusted access: https://chatgpt.com/cyber\nLearn more: https://developers.openai.com/codex/concepts/cyber-safety";
assert!(is_safety_check_downgrade_warning(warning));
}
#[test]
fn safety_check_downgrade_warning_detection_ignores_other_warnings() {
let warning = "Model metadata for `mock-model` not found. Defaulting to fallback metadata; this can degrade performance and cause issues.";
assert!(!is_safety_check_downgrade_warning(warning));
}
#[tokio::test]
async fn test_handle_error_records_message() -> Result<()> {
let conversation_id = ThreadId::new();
@@ -477,7 +477,6 @@ fn assert_permissions_message(item: &ResponseItem) {
&SandboxPolicy::DangerFullAccess,
AskForApproval::Never,
&Policy::empty(),
false,
&PathBuf::from("/tmp"),
)
.into_text();
@@ -15,6 +15,7 @@ mod plan_item;
mod rate_limits;
mod request_user_input;
mod review;
mod safety_check_downgrade;
mod skills_list;
mod thread_archive;
mod thread_fork;
@@ -0,0 +1,266 @@
use anyhow::Result;
use app_test_support::McpProcess;
use app_test_support::to_response;
use codex_app_server_protocol::ItemCompletedNotification;
use codex_app_server_protocol::ItemStartedNotification;
use codex_app_server_protocol::JSONRPCNotification;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::ThreadItem;
use codex_app_server_protocol::ThreadStartParams;
use codex_app_server_protocol::ThreadStartResponse;
use codex_app_server_protocol::TurnStartParams;
use codex_app_server_protocol::TurnStartResponse;
use codex_app_server_protocol::UserInput;
use core_test_support::responses;
use core_test_support::skip_if_no_network;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
use tokio::time::timeout;
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
const REQUESTED_MODEL: &str = "gpt-5.1-codex-max";
const SERVER_MODEL: &str = "gpt-5.2-codex";
#[tokio::test]
async fn openai_model_header_mismatch_emits_warning_item_v2() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
let body = responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_assistant_message("msg-1", "Done"),
responses::ev_completed("resp-1"),
]);
let response = responses::sse_response(body).insert_header("OpenAI-Model", SERVER_MODEL);
let _response_mock = responses::mount_response_once(&server, response).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri())?;
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let thread_req = mcp
.send_thread_start_request(ThreadStartParams {
model: Some(REQUESTED_MODEL.to_string()),
..Default::default()
})
.await?;
let thread_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(thread_req)),
)
.await??;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
let turn_req = mcp
.send_turn_start_request(TurnStartParams {
thread_id: thread.id.clone(),
input: vec![UserInput::Text {
text: "trigger safeguard".to_string(),
text_elements: Vec::new(),
}],
..Default::default()
})
.await?;
let _turn_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(turn_req)),
)
.await??;
let _turn_start: TurnStartResponse = to_response(_turn_resp)?;
let warning_started = timeout(DEFAULT_READ_TIMEOUT, async {
loop {
let notification: JSONRPCNotification = mcp
.read_stream_until_notification_message("item/started")
.await?;
let params = notification.params.expect("item/started params");
let started: ItemStartedNotification =
serde_json::from_value(params).expect("deserialize item/started");
if warning_text_from_item(&started.item).is_some_and(is_cyber_model_warning_text) {
return Ok::<ItemStartedNotification, anyhow::Error>(started);
}
}
})
.await??;
let warning_text =
warning_text_from_item(&warning_started.item).expect("expected warning user message item");
assert!(warning_text.contains("Warning:"));
assert!(warning_text.contains("gpt-5.2 as a fallback"));
assert!(warning_text.contains("regain access to gpt-5.3-codex"));
let warning_completed = timeout(DEFAULT_READ_TIMEOUT, async {
loop {
let notification: JSONRPCNotification = mcp
.read_stream_until_notification_message("item/completed")
.await?;
let params = notification.params.expect("item/completed params");
let completed: ItemCompletedNotification =
serde_json::from_value(params).expect("deserialize item/completed");
if warning_text_from_item(&completed.item).is_some_and(is_cyber_model_warning_text) {
return Ok::<ItemCompletedNotification, anyhow::Error>(completed);
}
}
})
.await??;
assert_eq!(
warning_text_from_item(&warning_completed.item),
warning_text_from_item(&warning_started.item)
);
timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("turn/completed"),
)
.await??;
Ok(())
}
#[tokio::test]
async fn response_model_field_mismatch_emits_warning_item_v2_when_header_matches_requested()
-> Result<()> {
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
let body = responses::sse(vec![
serde_json::json!({
"type": "response.created",
"response": {
"id": "resp-1",
"model": SERVER_MODEL,
}
}),
responses::ev_assistant_message("msg-1", "Done"),
responses::ev_completed("resp-1"),
]);
let response = responses::sse_response(body).insert_header("OpenAI-Model", REQUESTED_MODEL);
let _response_mock = responses::mount_response_once(&server, response).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri())?;
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let thread_req = mcp
.send_thread_start_request(ThreadStartParams {
model: Some(REQUESTED_MODEL.to_string()),
..Default::default()
})
.await?;
let thread_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(thread_req)),
)
.await??;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
let turn_req = mcp
.send_turn_start_request(TurnStartParams {
thread_id: thread.id.clone(),
input: vec![UserInput::Text {
text: "trigger response model check".to_string(),
text_elements: Vec::new(),
}],
..Default::default()
})
.await?;
let turn_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(turn_req)),
)
.await??;
let _turn_start: TurnStartResponse = to_response(turn_resp)?;
let warning_started = timeout(DEFAULT_READ_TIMEOUT, async {
loop {
let notification: JSONRPCNotification = mcp
.read_stream_until_notification_message("item/started")
.await?;
let params = notification.params.expect("item/started params");
let started: ItemStartedNotification =
serde_json::from_value(params).expect("deserialize item/started");
if warning_text_from_item(&started.item).is_some_and(is_cyber_model_warning_text) {
return Ok::<ItemStartedNotification, anyhow::Error>(started);
}
}
})
.await??;
let warning_text =
warning_text_from_item(&warning_started.item).expect("expected warning user message item");
assert!(warning_text.contains("gpt-5.2 as a fallback"));
let warning_completed = timeout(DEFAULT_READ_TIMEOUT, async {
loop {
let notification: JSONRPCNotification = mcp
.read_stream_until_notification_message("item/completed")
.await?;
let params = notification.params.expect("item/completed params");
let completed: ItemCompletedNotification =
serde_json::from_value(params).expect("deserialize item/completed");
if warning_text_from_item(&completed.item).is_some_and(is_cyber_model_warning_text) {
return Ok::<ItemCompletedNotification, anyhow::Error>(completed);
}
}
})
.await??;
assert_eq!(
warning_text_from_item(&warning_completed.item),
warning_text_from_item(&warning_started.item)
);
timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("turn/completed"),
)
.await??;
Ok(())
}
fn warning_text_from_item(item: &ThreadItem) -> Option<&str> {
let ThreadItem::UserMessage { content, .. } = item else {
return None;
};
content.iter().find_map(|input| match input {
UserInput::Text { text, .. } if text.starts_with("Warning: ") => Some(text.as_str()),
_ => None,
})
}
fn is_cyber_model_warning_text(text: &str) -> bool {
text.contains("flagged for potentially high-risk cyber activity")
&& text.contains("apply for trusted access: https://chatgpt.com/cyber")
}
fn create_config_toml(codex_home: &std::path::Path, server_uri: &str) -> std::io::Result<()> {
let config_toml = codex_home.join("config.toml");
std::fs::write(
config_toml,
format!(
r#"
model = "{REQUESTED_MODEL}"
approval_policy = "never"
sandbox_mode = "read-only"
model_provider = "mock_provider"
[features]
remote_models = false
personality = true
[model_providers.mock_provider]
name = "Mock provider for test"
base_url = "{server_uri}/v1"
wire_api = "responses"
request_max_retries = 0
stream_max_retries = 0
"#
),
)
}
+3
View File
@@ -56,6 +56,9 @@ pub enum ResponseEvent {
Created,
OutputItemDone(ResponseItem),
OutputItemAdded(ResponseItem),
/// Emitted when the server includes `OpenAI-Model` on the stream response.
/// This can differ from the requested model when backend safety routing applies.
ServerModel(String),
/// Emitted when `X-Reasoning-Included: true` is present on the response,
/// meaning the server already accounted for past reasoning tokens and the
/// client should not re-estimate them.
@@ -63,6 +63,9 @@ impl Stream for AggregatedStream {
Poll::Ready(Some(Ok(ResponseEvent::ModelsEtag(etag)))) => {
return Poll::Ready(Some(Ok(ResponseEvent::ModelsEtag(etag))));
}
Poll::Ready(Some(Ok(ResponseEvent::ServerModel(model)))) => {
return Poll::Ready(Some(Ok(ResponseEvent::ServerModel(model))));
}
Poll::Ready(Some(Ok(ResponseEvent::Completed {
response_id,
token_usage,
@@ -163,6 +163,7 @@ impl Drop for WsStream {
const X_CODEX_TURN_STATE_HEADER: &str = "x-codex-turn-state";
const X_MODELS_ETAG_HEADER: &str = "x-models-etag";
const X_REASONING_INCLUDED_HEADER: &str = "x-reasoning-included";
const OPENAI_MODEL_HEADER: &str = "openai-model";
pub struct ResponsesWebsocketConnection {
stream: Arc<Mutex<Option<WsStream>>>,
@@ -170,6 +171,7 @@ pub struct ResponsesWebsocketConnection {
idle_timeout: Duration,
server_reasoning_included: bool,
models_etag: Option<String>,
server_model: Option<String>,
telemetry: Option<Arc<dyn WebsocketTelemetry>>,
}
@@ -179,6 +181,7 @@ impl ResponsesWebsocketConnection {
idle_timeout: Duration,
server_reasoning_included: bool,
models_etag: Option<String>,
server_model: Option<String>,
telemetry: Option<Arc<dyn WebsocketTelemetry>>,
) -> Self {
Self {
@@ -186,6 +189,7 @@ impl ResponsesWebsocketConnection {
idle_timeout,
server_reasoning_included,
models_etag,
server_model,
telemetry,
}
}
@@ -204,12 +208,16 @@ impl ResponsesWebsocketConnection {
let idle_timeout = self.idle_timeout;
let server_reasoning_included = self.server_reasoning_included;
let models_etag = self.models_etag.clone();
let server_model = self.server_model.clone();
let telemetry = self.telemetry.clone();
let request_body = serde_json::to_value(&request).map_err(|err| {
ApiError::Stream(format!("failed to encode websocket request: {err}"))
})?;
tokio::spawn(async move {
if let Some(model) = server_model {
let _ = tx_event.send(Ok(ResponseEvent::ServerModel(model))).await;
}
if let Some(etag) = models_etag {
let _ = tx_event.send(Ok(ResponseEvent::ModelsEtag(etag))).await;
}
@@ -273,13 +281,14 @@ impl<A: AuthProvider> ResponsesWebsocketClient<A> {
merge_request_headers(&self.provider.headers, extra_headers, default_headers);
add_auth_headers_to_header_map(&self.auth, &mut headers);
let (stream, server_reasoning_included, models_etag) =
let (stream, server_reasoning_included, models_etag, server_model) =
connect_websocket(ws_url, headers, turn_state.clone()).await?;
Ok(ResponsesWebsocketConnection::new(
stream,
self.provider.stream_idle_timeout,
server_reasoning_included,
models_etag,
server_model,
telemetry,
))
}
@@ -304,7 +313,7 @@ async fn connect_websocket(
url: Url,
headers: HeaderMap,
turn_state: Option<Arc<OnceLock<String>>>,
) -> Result<(WsStream, bool, Option<String>), ApiError> {
) -> Result<(WsStream, bool, Option<String>, Option<String>), ApiError> {
ensure_rustls_crypto_provider();
info!("connecting to websocket: {url}");
@@ -341,6 +350,11 @@ async fn connect_websocket(
.get(X_MODELS_ETAG_HEADER)
.and_then(|value| value.to_str().ok())
.map(ToString::to_string);
let server_model = response
.headers()
.get(OPENAI_MODEL_HEADER)
.and_then(|value| value.to_str().ok())
.map(ToString::to_string);
if let Some(turn_state) = turn_state
&& let Some(header_value) = response
.headers()
@@ -349,7 +363,12 @@ async fn connect_websocket(
{
let _ = turn_state.set(header_value.to_string());
}
Ok((WsStream::new(stream), reasoning_included, models_etag))
Ok((
WsStream::new(stream),
reasoning_included,
models_etag,
server_model,
))
}
fn websocket_config() -> WebSocketConfig {
@@ -469,6 +488,7 @@ async fn run_websocket_response_stream(
idle_timeout: Duration,
telemetry: Option<Arc<dyn WebsocketTelemetry>>,
) -> Result<(), ApiError> {
let mut last_server_model: Option<String> = None;
let request_text = match serde_json::to_string(&request_body) {
Ok(text) => text,
Err(err) => {
@@ -536,6 +556,14 @@ async fn run_websocket_response_stream(
}
continue;
}
if let Some(model) = event.response_model()
&& last_server_model.as_deref() != Some(model.as_str())
{
let _ = tx_event
.send(Ok(ResponseEvent::ServerModel(model.clone())))
.await;
last_server_model = Some(model);
}
match process_responses_event(event) {
Ok(Some(event)) => {
let is_completed = matches!(event, ResponseEvent::Completed { .. });
+207
View File
@@ -26,6 +26,7 @@ use tracing::debug;
use tracing::trace;
const X_REASONING_INCLUDED_HEADER: &str = "x-reasoning-included";
const OPENAI_MODEL_HEADER: &str = "openai-model";
/// Streams SSE events from an on-disk fixture for tests.
pub fn stream_from_fixture(
@@ -60,6 +61,11 @@ pub fn spawn_response_stream(
.get("X-Models-Etag")
.and_then(|v| v.to_str().ok())
.map(ToString::to_string);
let server_model = stream_response
.headers
.get(OPENAI_MODEL_HEADER)
.and_then(|v| v.to_str().ok())
.map(ToString::to_string);
let reasoning_included = stream_response
.headers
.get(X_REASONING_INCLUDED_HEADER)
@@ -74,6 +80,9 @@ pub fn spawn_response_stream(
}
let (tx_event, rx_event) = mpsc::channel::<Result<ResponseEvent, ApiError>>(1600);
tokio::spawn(async move {
if let Some(model) = server_model {
let _ = tx_event.send(Ok(ResponseEvent::ServerModel(model))).await;
}
for snapshot in rate_limit_snapshots {
let _ = tx_event.send(Ok(ResponseEvent::RateLimits(snapshot))).await;
}
@@ -169,6 +178,41 @@ impl ResponsesStreamEvent {
pub fn kind(&self) -> &str {
&self.kind
}
pub fn response_model(&self) -> Option<String> {
self.response.as_ref().and_then(extract_server_model)
}
}
fn extract_server_model(value: &Value) -> Option<String> {
value
.get("model")
.and_then(json_value_as_string)
.or_else(|| {
value
.get("headers")
.and_then(header_openai_model_value_from_json)
})
}
fn header_openai_model_value_from_json(value: &Value) -> Option<String> {
let headers = value.as_object()?;
headers.iter().find_map(|(name, value)| {
if name.eq_ignore_ascii_case("openai-model") || name.eq_ignore_ascii_case("x-openai-model")
{
json_value_as_string(value)
} else {
None
}
})
}
fn json_value_as_string(value: &Value) -> Option<String> {
match value {
Value::String(value) => Some(value.clone()),
Value::Array(items) => items.first().and_then(json_value_as_string),
_ => None,
}
}
#[derive(Debug)]
@@ -339,6 +383,7 @@ pub async fn process_sse(
) {
let mut stream = stream.eventsource();
let mut response_error: Option<ApiError> = None;
let mut last_server_model: Option<String> = None;
loop {
let start = Instant::now();
@@ -378,6 +423,19 @@ pub async fn process_sse(
}
};
if let Some(model) = event.response_model()
&& last_server_model.as_deref() != Some(model.as_str())
{
if tx_event
.send(Ok(ResponseEvent::ServerModel(model.clone())))
.await
.is_err()
{
return;
}
last_server_model = Some(model);
}
match process_responses_event(event) {
Ok(Some(event)) => {
let is_completed = matches!(event, ResponseEvent::Completed { .. });
@@ -456,9 +514,13 @@ mod tests {
use super::*;
use assert_matches::assert_matches;
use bytes::Bytes;
use codex_client::StreamResponse;
use codex_protocol::models::MessagePhase;
use codex_protocol::models::ResponseItem;
use futures::stream;
use http::HeaderMap;
use http::HeaderValue;
use http::StatusCode;
use pretty_assertions::assert_eq;
use serde_json::json;
use tokio::sync::mpsc;
@@ -870,6 +932,149 @@ mod tests {
}
}
#[tokio::test]
async fn spawn_response_stream_emits_server_model_header() {
let mut headers = HeaderMap::new();
headers.insert(
OPENAI_MODEL_HEADER,
HeaderValue::from_static(CYBER_RESTRICTED_MODEL_FOR_TESTS),
);
let bytes = stream::iter(Vec::<Result<Bytes, TransportError>>::new());
let stream_response = StreamResponse {
status: StatusCode::OK,
headers,
bytes: Box::pin(bytes),
};
let mut stream = spawn_response_stream(stream_response, idle_timeout(), None, None);
let event = stream
.rx_event
.recv()
.await
.expect("expected server model event")
.expect("expected ok event");
match event {
ResponseEvent::ServerModel(model) => {
assert_eq!(model, CYBER_RESTRICTED_MODEL_FOR_TESTS);
}
other => panic!("expected server model event, got {other:?}"),
}
}
#[tokio::test]
async fn process_sse_emits_server_model_from_response_payload() {
let events = run_sse(vec![
json!({
"type": "response.created",
"response": {
"id": "resp-1",
"model": CYBER_RESTRICTED_MODEL_FOR_TESTS
}
}),
json!({
"type": "response.completed",
"response": {
"id": "resp-1",
"model": CYBER_RESTRICTED_MODEL_FOR_TESTS
}
}),
])
.await;
assert_eq!(events.len(), 3);
assert_matches!(
&events[0],
ResponseEvent::ServerModel(model) if model == CYBER_RESTRICTED_MODEL_FOR_TESTS
);
assert_matches!(&events[1], ResponseEvent::Created);
assert_matches!(
&events[2],
ResponseEvent::Completed {
response_id,
token_usage: None,
can_append: false
} if response_id == "resp-1"
);
}
#[tokio::test]
async fn process_sse_emits_server_model_from_response_headers_payload() {
let events = run_sse(vec![
json!({
"type": "response.created",
"response": {
"id": "resp-1",
"headers": {
"OpenAI-Model": CYBER_RESTRICTED_MODEL_FOR_TESTS
}
}
}),
json!({
"type": "response.completed",
"response": {
"id": "resp-1"
}
}),
])
.await;
assert_eq!(events.len(), 3);
assert_matches!(
&events[0],
ResponseEvent::ServerModel(model) if model == CYBER_RESTRICTED_MODEL_FOR_TESTS
);
assert_matches!(&events[1], ResponseEvent::Created);
assert_matches!(
&events[2],
ResponseEvent::Completed {
response_id,
token_usage: None,
can_append: false
} if response_id == "resp-1"
);
}
#[tokio::test]
async fn process_sse_emits_server_model_again_when_response_model_changes() {
let events = run_sse(vec![
json!({
"type": "response.created",
"response": {
"id": "resp-1",
"model": "gpt-5.2-codex"
}
}),
json!({
"type": "response.completed",
"response": {
"id": "resp-1",
"model": "gpt-5.3-codex"
}
}),
])
.await;
assert_eq!(events.len(), 4);
assert_matches!(
&events[0],
ResponseEvent::ServerModel(model) if model == "gpt-5.2-codex"
);
assert_matches!(&events[1], ResponseEvent::Created);
assert_matches!(
&events[2],
ResponseEvent::ServerModel(model) if model == "gpt-5.3-codex"
);
assert_matches!(
&events[3],
ResponseEvent::Completed {
response_id,
token_usage: None,
can_append: false
} if response_id == "resp-1"
);
}
#[test]
fn test_try_parse_retry_after() {
let err = Error {
@@ -909,4 +1114,6 @@ mod tests {
let delay = try_parse_retry_after(&err);
assert_eq!(delay, Some(Duration::from_secs(35)));
}
const CYBER_RESTRICTED_MODEL_FOR_TESTS: &str = "gpt-5.3-codex";
}
+1 -1
View File
@@ -2,6 +2,6 @@ use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
pub(crate) fn render_apps_section() -> String {
format!(
"## Apps\nApps are mentioned in the prompt in the format `[$app-name](apps://{{connector_id}})`.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nWhen you see an app mention, the app's MCP tools are either already provided in `{CODEX_APPS_MCP_SERVER_NAME}`, or do not exist because the user did not install it.\nDo not additionally call list_mcp_resources for apps that are already mentioned."
"## Apps\nApps are mentioned in the prompt in the format `[$app-name](app://{{connector_id}})`.\nAn app is equivalent to a set of MCP tools within the `{CODEX_APPS_MCP_SERVER_NAME}` MCP.\nWhen you see an app mention, the app's MCP tools are either already provided in `{CODEX_APPS_MCP_SERVER_NAME}`, or do not exist because the user did not install it.\nDo not additionally call list_mcp_resources for apps that are already mentioned."
)
}
+52 -10
View File
@@ -279,6 +279,8 @@ pub struct CodexSpawnOk {
pub(crate) const INITIAL_SUBMIT_ID: &str = "";
pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 64;
const CYBER_VERIFY_URL: &str = "https://chatgpt.com/cyber";
const CYBER_SAFETY_URL: &str = "https://developers.openai.com/codex/concepts/cyber-safety";
impl Codex {
/// Spawn a new [`Codex`] and initialize the session.
@@ -2004,7 +2006,6 @@ impl Session {
&next.sandbox_policy,
next.approval_policy,
self.services.exec_policy.current().as_ref(),
self.features.enabled(Feature::RequestRule),
&next.cwd,
)
.into(),
@@ -2561,6 +2562,35 @@ impl Session {
self.record_conversation_items(ctx, &[item]).await;
}
async fn maybe_warn_on_server_model_mismatch(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
server_model: String,
) -> bool {
let requested_model = turn_context.model_info.slug.as_str();
if server_model == requested_model {
info!("server reported model {server_model} (matches requested model)");
return false;
}
warn!("server reported model {server_model} while requested model was {requested_model}");
let warning_message = format!(
"Your account was flagged for potentially high-risk cyber activity and this request was routed to gpt-5.2 as a fallback. To regain access to gpt-5.3-codex, apply for trusted access: {CYBER_VERIFY_URL} or learn more: {CYBER_SAFETY_URL}"
);
self.send_event(
turn_context,
EventMsg::Warning(WarningEvent {
message: warning_message.clone(),
}),
)
.await;
self.record_model_warning(warning_message, turn_context)
.await;
true
}
pub(crate) async fn replace_history(&self, items: Vec<ResponseItem>) {
let mut state = self.state.lock().await;
state.replace_history(items);
@@ -2624,7 +2654,6 @@ impl Session {
&turn_context.sandbox_policy,
turn_context.approval_policy,
self.services.exec_policy.current().as_ref(),
self.features.enabled(Feature::RequestRule),
&turn_context.cwd,
)
.into(),
@@ -4437,6 +4466,7 @@ pub(crate) async fn run_turn(
// Although from the perspective of codex.rs, TurnDiffTracker has the lifecycle of a Task which contains
// 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 server_model_warning_emitted_for_turn = false;
// `ModelClientSession` is turn-scoped and caches WebSocket + sticky routing state, so we reuse
// one instance across retries within this turn.
@@ -4499,6 +4529,7 @@ pub(crate) async fn run_turn(
sampling_request_input,
&explicitly_enabled_connectors,
skills_outcome.as_ref(),
&mut server_model_warning_emitted_for_turn,
cancellation_token.child_token(),
)
.await
@@ -4872,6 +4903,7 @@ async fn run_sampling_request(
input: Vec<ResponseItem>,
explicitly_enabled_connectors: &HashSet<String>,
skills_outcome: Option<&SkillLoadOutcome>,
server_model_warning_emitted_for_turn: &mut bool,
cancellation_token: CancellationToken,
) -> CodexResult<SamplingRequestResult> {
let router = built_tools(
@@ -4908,6 +4940,7 @@ async fn run_sampling_request(
client_session,
turn_metadata_header,
Arc::clone(&turn_diff_tracker),
server_model_warning_emitted_for_turn,
&prompt,
cancellation_token.child_token(),
)
@@ -4997,14 +5030,13 @@ async fn built_tools(
skills_outcome: Option<&SkillLoadOutcome>,
cancellation_token: &CancellationToken,
) -> CodexResult<Arc<ToolRouter>> {
let mut mcp_tools = sess
.services
.mcp_connection_manager
.read()
.await
let mcp_connection_manager = sess.services.mcp_connection_manager.read().await;
let has_mcp_servers = mcp_connection_manager.has_servers();
let mut mcp_tools = mcp_connection_manager
.list_all_tools()
.or_cancel(cancellation_token)
.await?;
drop(mcp_connection_manager);
let mut effective_explicitly_enabled_connectors = explicitly_enabled_connectors.clone();
effective_explicitly_enabled_connectors.extend(sess.get_connector_selection().await);
@@ -5050,12 +5082,12 @@ async fn built_tools(
Ok(Arc::new(ToolRouter::from_config(
&turn_context.tools_config,
Some(
has_mcp_servers.then(|| {
mcp_tools
.into_iter()
.map(|(name, tool)| (name, tool.tool))
.collect(),
),
.collect()
}),
app_tools,
turn_context.dynamic_tools.as_slice(),
)))
@@ -5477,6 +5509,7 @@ async fn try_run_sampling_request(
client_session: &mut ModelClientSession,
turn_metadata_header: Option<&str>,
turn_diff_tracker: SharedTurnDiffTracker,
server_model_warning_emitted_for_turn: &mut bool,
prompt: &Prompt,
cancellation_token: CancellationToken,
) -> CodexResult<SamplingRequestResult> {
@@ -5619,6 +5652,15 @@ async fn try_run_sampling_request(
active_item = Some(turn_item);
}
}
ResponseEvent::ServerModel(server_model) => {
if !*server_model_warning_emitted_for_turn
&& sess
.maybe_warn_on_server_model_mismatch(&turn_context, server_model)
.await
{
*server_model_warning_emitted_for_turn = true;
}
}
ResponseEvent::ServerReasoningIncluded(included) => {
sess.set_server_reasoning_included(included).await;
}
+55 -24
View File
@@ -170,23 +170,26 @@ impl ExecPolicyManager {
prefix_rule,
} = req;
let exec_policy = self.current();
let (commands, used_heredoc_fallback) = commands_for_exec_policy(command);
let (commands, used_complex_parsing) = commands_for_exec_policy(command);
// Keep heredoc prefix parsing for rule evaluation so existing
// allow/prompt/forbidden rules still apply, but avoid auto-derived
// amendments when only the heredoc fallback parser matched.
let auto_amendment_allowed = !used_heredoc_fallback;
let auto_amendment_allowed = !used_complex_parsing;
let exec_policy_fallback = |cmd: &[String]| {
render_decision_for_unmatched_command(
approval_policy,
sandbox_policy,
cmd,
sandbox_permissions,
used_complex_parsing,
)
};
let evaluation = exec_policy.check_multiple(commands.iter(), &exec_policy_fallback);
let requested_amendment =
derive_requested_execpolicy_amendment(prefix_rule.as_ref(), &evaluation.matched_rules);
let requested_amendment = derive_requested_execpolicy_amendment_from_prefix_rule(
prefix_rule.as_ref(),
&evaluation.matched_rules,
);
match evaluation.decision {
Decision::Forbidden => ExecApprovalRequirement::Forbidden {
@@ -406,8 +409,9 @@ pub fn render_decision_for_unmatched_command(
sandbox_policy: &SandboxPolicy,
command: &[String],
sandbox_permissions: SandboxPermissions,
used_complex_parsing: bool,
) -> Decision {
if is_known_safe_command(command) {
if is_known_safe_command(command) && !used_complex_parsing {
return Decision::Allow;
}
@@ -534,7 +538,7 @@ fn try_derive_execpolicy_amendment_for_allow_rules(
})
}
fn derive_requested_execpolicy_amendment(
fn derive_requested_execpolicy_amendment_from_prefix_rule(
prefix_rule: Option<&Vec<String>>,
matched_rules: &[RuleMatch],
) -> Option<ExecPolicyAmendment> {
@@ -552,10 +556,8 @@ fn derive_requested_execpolicy_amendment(
return None;
}
if matched_rules
.iter()
.any(|rule_match| is_policy_match(rule_match) && rule_match.decision() == Decision::Prompt)
{
// if any policy rule already matches, don't suggest an additional rule that might conflict or not apply
if matched_rules.iter().any(is_policy_match) {
return None;
}
@@ -686,8 +688,6 @@ mod tests {
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigRequirements;
use crate::config_loader::ConfigRequirementsToml;
use crate::features::Feature;
use crate::features::Features;
use codex_app_server_protocol::ConfigLayerSource;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
@@ -1281,8 +1281,6 @@ prefix_rule(
"cargo-insta".to_string(),
];
let manager = ExecPolicyManager::default();
let mut features = Features::with_defaults();
features.enable(Feature::RequestRule);
let requirement = manager
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
@@ -1564,14 +1562,17 @@ prefix_rule(
#[test]
fn derive_requested_execpolicy_amendment_returns_none_for_missing_prefix_rule() {
assert_eq!(None, derive_requested_execpolicy_amendment(None, &[]));
assert_eq!(
None,
derive_requested_execpolicy_amendment_from_prefix_rule(None, &[])
);
}
#[test]
fn derive_requested_execpolicy_amendment_returns_none_for_empty_prefix_rule() {
assert_eq!(
None,
derive_requested_execpolicy_amendment(Some(&Vec::new()), &[])
derive_requested_execpolicy_amendment_from_prefix_rule(Some(&Vec::new()), &[])
);
}
@@ -1579,7 +1580,7 @@ prefix_rule(
fn derive_requested_execpolicy_amendment_returns_none_for_exact_banned_prefix_rule() {
assert_eq!(
None,
derive_requested_execpolicy_amendment(
derive_requested_execpolicy_amendment_from_prefix_rule(
Some(&vec!["python".to_string(), "-c".to_string()]),
&[],
)
@@ -1598,7 +1599,7 @@ prefix_rule(
] {
assert_eq!(
None,
derive_requested_execpolicy_amendment(Some(&prefix_rule), &[])
derive_requested_execpolicy_amendment_from_prefix_rule(Some(&prefix_rule), &[])
);
}
}
@@ -1624,7 +1625,7 @@ prefix_rule(
] {
assert_eq!(
None,
derive_requested_execpolicy_amendment(Some(&prefix_rule), &[])
derive_requested_execpolicy_amendment_from_prefix_rule(Some(&prefix_rule), &[])
);
}
}
@@ -1639,22 +1640,52 @@ prefix_rule(
assert_eq!(
Some(ExecPolicyAmendment::new(prefix_rule.clone())),
derive_requested_execpolicy_amendment(Some(&prefix_rule), &[])
derive_requested_execpolicy_amendment_from_prefix_rule(Some(&prefix_rule), &[])
);
}
#[test]
fn derive_requested_execpolicy_amendment_returns_none_when_policy_prompt_matches() {
fn derive_requested_execpolicy_amendment_returns_none_when_policy_matches() {
let prefix_rule = vec!["cargo".to_string(), "build".to_string()];
let matched_rules = vec![RuleMatch::PrefixRuleMatch {
let matched_rules_prompt = vec![RuleMatch::PrefixRuleMatch {
matched_prefix: vec!["cargo".to_string()],
decision: Decision::Prompt,
justification: None,
}];
assert_eq!(
None,
derive_requested_execpolicy_amendment(Some(&prefix_rule), &matched_rules)
derive_requested_execpolicy_amendment_from_prefix_rule(
Some(&prefix_rule),
&matched_rules_prompt
),
"should return none when prompt policy matches"
);
let matched_rules_allow = vec![RuleMatch::PrefixRuleMatch {
matched_prefix: vec!["cargo".to_string()],
decision: Decision::Allow,
justification: None,
}];
assert_eq!(
None,
derive_requested_execpolicy_amendment_from_prefix_rule(
Some(&prefix_rule),
&matched_rules_allow
),
"should return none when prompt policy matches"
);
let matched_rules_forbidden = vec![RuleMatch::PrefixRuleMatch {
matched_prefix: vec!["cargo".to_string()],
decision: Decision::Forbidden,
justification: None,
}];
assert_eq!(
None,
derive_requested_execpolicy_amendment_from_prefix_rule(
Some(&prefix_rule),
&matched_rules_forbidden
),
"should return none when prompt policy matches"
);
}
+4 -4
View File
@@ -511,8 +511,8 @@ pub const FEATURES: &[FeatureSpec] = &[
FeatureSpec {
id: Feature::RequestRule,
key: "request_rule",
stage: Stage::Stable,
default_enabled: true,
stage: Stage::Removed,
default_enabled: false,
},
FeatureSpec {
id: Feature::WindowsSandbox,
@@ -554,9 +554,9 @@ pub const FEATURES: &[FeatureSpec] = &[
id: Feature::Collab,
key: "multi_agent",
stage: Stage::Experimental {
name: "Sub-agents",
name: "Multi-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!",
announcement: "NEW: Multi-agents can now be spawned by Codex. Enable in /experimental and restart Codex!",
},
default_enabled: false,
},
@@ -353,6 +353,10 @@ pub(crate) struct McpConnectionManager {
}
impl McpConnectionManager {
pub(crate) fn has_servers(&self) -> bool {
!self.clients.is_empty()
}
pub async fn initialize(
&mut self,
mcp_servers: &HashMap<String, McpServerConfig>,
+47 -2
View File
@@ -20,8 +20,10 @@ const LOCAL_PRAGMATIC_TEMPLATE: &str = "You are a deeply pragmatic, effective so
const PERSONALITY_PLACEHOLDER: &str = "{{ personality }}";
pub(crate) fn with_config_overrides(mut model: ModelInfo, config: &Config) -> ModelInfo {
if let Some(supports_reasoning_summaries) = config.model_supports_reasoning_summaries {
model.supports_reasoning_summaries = supports_reasoning_summaries;
if let Some(supports_reasoning_summaries) = config.model_supports_reasoning_summaries
&& supports_reasoning_summaries
{
model.supports_reasoning_summaries = true;
}
if let Some(context_window) = config.model_context_window {
model.context_window = Some(context_window);
@@ -100,3 +102,46 @@ fn local_personality_messages_for_slug(slug: &str) -> Option<ModelMessages> {
_ => None,
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::config::test_config;
use pretty_assertions::assert_eq;
#[test]
fn reasoning_summaries_override_true_enables_support() {
let model = model_info_from_slug("unknown-model");
let mut config = test_config();
config.model_supports_reasoning_summaries = Some(true);
let updated = with_config_overrides(model.clone(), &config);
let mut expected = model;
expected.supports_reasoning_summaries = true;
assert_eq!(updated, expected);
}
#[test]
fn reasoning_summaries_override_false_does_not_disable_support() {
let mut model = model_info_from_slug("unknown-model");
model.supports_reasoning_summaries = true;
let mut config = test_config();
config.model_supports_reasoning_summaries = Some(false);
let updated = with_config_overrides(model.clone(), &config);
assert_eq!(updated, model);
}
#[test]
fn reasoning_summaries_override_false_is_noop_when_model_is_false() {
let model = model_info_from_slug("unknown-model");
let mut config = test_config();
config.model_supports_reasoning_summaries = Some(false);
let updated = with_config_overrides(model.clone(), &config);
assert_eq!(updated, model);
}
}
+2 -2
View File
@@ -1,11 +1,11 @@
pub mod apply_patch;
pub(crate) mod collab;
mod dynamic;
mod grep_files;
mod js_repl;
mod list_dir;
mod mcp;
mod mcp_resource;
pub(crate) mod multi_agents;
mod plan;
mod read_file;
mod request_user_input;
@@ -20,7 +20,6 @@ use serde::Deserialize;
use crate::function_tool::FunctionCallError;
pub use apply_patch::ApplyPatchHandler;
pub use collab::CollabHandler;
pub use dynamic::DynamicToolHandler;
pub use grep_files::GrepFilesHandler;
pub use js_repl::JsReplHandler;
@@ -28,6 +27,7 @@ pub use js_repl::JsReplResetHandler;
pub use list_dir::ListDirHandler;
pub use mcp::McpHandler;
pub use mcp_resource::McpResourceHandler;
pub use multi_agents::MultiAgentHandler;
pub use plan::PlanHandler;
pub use read_file::ReadFileHandler;
pub use request_user_input::RequestUserInputHandler;
@@ -34,7 +34,7 @@ use codex_protocol::user_input::UserInput;
use serde::Deserialize;
use serde::Serialize;
pub struct CollabHandler;
pub struct MultiAgentHandler;
/// Minimum wait timeout to prevent tight polling loops from burning CPU.
pub(crate) const MIN_WAIT_TIMEOUT_MS: i64 = 10_000;
@@ -47,7 +47,7 @@ struct CloseAgentArgs {
}
#[async_trait]
impl ToolHandler for CollabHandler {
impl ToolHandler for MultiAgentHandler {
fn kind(&self) -> ToolKind {
ToolKind::Function
}
@@ -917,7 +917,7 @@ mod tests {
input: "hello".to_string(),
},
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("payload should be rejected");
};
assert_eq!(
@@ -937,7 +937,7 @@ mod tests {
"unknown_tool",
function_payload(json!({})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("tool should be rejected");
};
assert_eq!(
@@ -955,7 +955,7 @@ mod tests {
"spawn_agent",
function_payload(json!({"message": " "})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("empty message should be rejected");
};
assert_eq!(
@@ -978,7 +978,7 @@ mod tests {
"items": [{"type": "mention", "name": "drive", "path": "app://drive"}]
})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("message+items should be rejected");
};
assert_eq!(
@@ -1016,7 +1016,7 @@ mod tests {
"agent_type": "explorer"
})),
);
let output = CollabHandler
let output = MultiAgentHandler
.handle(invocation)
.await
.expect("spawn_agent should succeed");
@@ -1048,7 +1048,7 @@ mod tests {
"spawn_agent",
function_payload(json!({"message": "hello"})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("spawn should fail without a manager");
};
assert_eq!(
@@ -1074,7 +1074,7 @@ mod tests {
"spawn_agent",
function_payload(json!({"message": "hello"})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("spawn should fail when depth limit exceeded");
};
assert_eq!(
@@ -1094,7 +1094,7 @@ mod tests {
"send_input",
function_payload(json!({"id": ThreadId::new().to_string(), "message": ""})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("empty message should be rejected");
};
assert_eq!(
@@ -1118,7 +1118,7 @@ mod tests {
"items": [{"type": "mention", "name": "drive", "path": "app://drive"}]
})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("message+items should be rejected");
};
assert_eq!(
@@ -1138,7 +1138,7 @@ mod tests {
"send_input",
function_payload(json!({"id": "not-a-uuid", "message": "hi"})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("invalid id should be rejected");
};
let FunctionCallError::RespondToModel(msg) = err else {
@@ -1159,7 +1159,7 @@ mod tests {
"send_input",
function_payload(json!({"id": agent_id.to_string(), "message": "hi"})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("missing agent should be reported");
};
assert_eq!(
@@ -1186,7 +1186,7 @@ mod tests {
"interrupt": true
})),
);
CollabHandler
MultiAgentHandler
.handle(invocation)
.await
.expect("send_input should succeed");
@@ -1227,7 +1227,7 @@ mod tests {
]
})),
);
CollabHandler
MultiAgentHandler
.handle(invocation)
.await
.expect("send_input should succeed");
@@ -1267,7 +1267,7 @@ mod tests {
"resume_agent",
function_payload(json!({"id": "not-a-uuid"})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("invalid id should be rejected");
};
let FunctionCallError::RespondToModel(msg) = err else {
@@ -1288,7 +1288,7 @@ mod tests {
"resume_agent",
function_payload(json!({"id": agent_id.to_string()})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("missing agent should be reported");
};
assert_eq!(
@@ -1313,7 +1313,7 @@ mod tests {
function_payload(json!({"id": agent_id.to_string()})),
);
let output = CollabHandler
let output = MultiAgentHandler
.handle(invocation)
.await
.expect("resume_agent should succeed");
@@ -1382,7 +1382,7 @@ mod tests {
"resume_agent",
function_payload(json!({"id": agent_id.to_string()})),
);
let output = CollabHandler
let output = MultiAgentHandler
.handle(resume_invocation)
.await
.expect("resume_agent should succeed");
@@ -1405,7 +1405,7 @@ mod tests {
"send_input",
function_payload(json!({"id": agent_id.to_string(), "message": "hello"})),
);
let output = CollabHandler
let output = MultiAgentHandler
.handle(send_invocation)
.await
.expect("send_input should succeed after resume");
@@ -1450,7 +1450,7 @@ mod tests {
"resume_agent",
function_payload(json!({"id": ThreadId::new().to_string()})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("resume should fail when depth limit exceeded");
};
assert_eq!(
@@ -1479,7 +1479,7 @@ mod tests {
"timeout_ms": 0
})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("non-positive timeout should be rejected");
};
assert_eq!(
@@ -1497,7 +1497,7 @@ mod tests {
"wait",
function_payload(json!({"ids": ["invalid"]})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("invalid id should be rejected");
};
let FunctionCallError::RespondToModel(msg) = err else {
@@ -1515,7 +1515,7 @@ mod tests {
"wait",
function_payload(json!({"ids": []})),
);
let Err(err) = CollabHandler.handle(invocation).await else {
let Err(err) = MultiAgentHandler.handle(invocation).await else {
panic!("empty ids should be rejected");
};
assert_eq!(
@@ -1540,7 +1540,7 @@ mod tests {
"timeout_ms": 1000
})),
);
let output = CollabHandler
let output = MultiAgentHandler
.handle(invocation)
.await
.expect("wait should succeed");
@@ -1584,7 +1584,7 @@ mod tests {
"timeout_ms": MIN_WAIT_TIMEOUT_MS
})),
);
let output = CollabHandler
let output = MultiAgentHandler
.handle(invocation)
.await
.expect("wait should succeed");
@@ -1632,7 +1632,11 @@ mod tests {
})),
);
let early = timeout(Duration::from_millis(50), CollabHandler.handle(invocation)).await;
let early = timeout(
Duration::from_millis(50),
MultiAgentHandler.handle(invocation),
)
.await;
assert!(
early.is_err(),
"wait should not return before the minimum timeout clamp"
@@ -1677,7 +1681,7 @@ mod tests {
"timeout_ms": 1000
})),
);
let output = CollabHandler
let output = MultiAgentHandler
.handle(invocation)
.await
.expect("wait should succeed");
@@ -1717,7 +1721,7 @@ mod tests {
"close_agent",
function_payload(json!({"id": agent_id.to_string()})),
);
let output = CollabHandler
let output = MultiAgentHandler
.handle(invocation)
.await
.expect("close_agent should succeed");
@@ -243,14 +243,6 @@ impl ShellHandler {
freeform,
} = args;
let features = session.features();
let request_rule_enabled = features.enabled(crate::features::Feature::RequestRule);
let prefix_rule = if request_rule_enabled {
prefix_rule
} else {
None
};
let mut exec_params = exec_params;
let dependency_env = session.dependency_env().await;
if !dependency_env.is_empty() {
@@ -142,14 +142,6 @@ impl ToolHandler for UnifiedExecHandler {
..
} = args;
let features = session.features();
let request_rule_enabled = features.enabled(crate::features::Feature::RequestRule);
let prefix_rule = if request_rule_enabled {
prefix_rule
} else {
None
};
if sandbox_permissions.requires_escalated_permissions()
&& !matches!(
context.turn.approval_policy,
+154 -31
View File
@@ -114,6 +114,7 @@ struct ExecContext {
struct ExecToolCalls {
in_flight: usize,
notify: Arc<Notify>,
cancel: CancellationToken,
}
enum KernelStreamEnd {
@@ -300,6 +301,7 @@ impl JsReplManager {
async fn clear_exec_tool_calls(&self, exec_id: &str) {
if let Some(state) = self.exec_tool_calls.lock().await.remove(exec_id) {
state.cancel.cancel();
state.notify.notify_waiters();
}
}
@@ -320,32 +322,14 @@ impl JsReplManager {
}
}
async fn wait_for_all_exec_tool_calls(&self) {
loop {
let notified = {
let calls = self.exec_tool_calls.lock().await;
calls
.values()
.find(|state| state.in_flight > 0)
.map(|state| Arc::clone(&state.notify).notified_owned())
};
match notified {
Some(notified) => notified.await,
None => return,
}
}
}
async fn begin_exec_tool_call(
exec_tool_calls: &Arc<Mutex<HashMap<String, ExecToolCalls>>>,
exec_id: &str,
) -> bool {
) -> Option<CancellationToken> {
let mut calls = exec_tool_calls.lock().await;
let Some(state) = calls.get_mut(exec_id) else {
return false;
};
let state = calls.get_mut(exec_id)?;
state.in_flight += 1;
true
Some(state.cancel.clone())
}
async fn finish_exec_tool_call(
@@ -396,14 +380,30 @@ impl JsReplManager {
exec_id: &str,
) {
if let Some(state) = exec_tool_calls.lock().await.remove(exec_id) {
state.cancel.cancel();
state.notify.notify_waiters();
}
}
async fn clear_all_exec_tool_calls_map(
exec_tool_calls: &Arc<Mutex<HashMap<String, ExecToolCalls>>>,
) {
let states = {
let mut calls = exec_tool_calls.lock().await;
calls.drain().map(|(_, state)| state).collect::<Vec<_>>()
};
for state in states {
state.cancel.cancel();
state.notify.notify_waiters();
}
}
pub async fn reset(&self) -> Result<(), FunctionCallError> {
let _permit = self.exec_lock.clone().acquire_owned().await.map_err(|_| {
FunctionCallError::RespondToModel("js_repl execution unavailable".to_string())
})?;
self.reset_kernel().await;
self.wait_for_all_exec_tool_calls().await;
self.exec_tool_calls.lock().await.clear();
Self::clear_all_exec_tool_calls_map(&self.exec_tool_calls).await;
Ok(())
}
@@ -530,7 +530,9 @@ impl JsReplManager {
return Err(FunctionCallError::RespondToModel(message));
}
Err(_) => {
self.reset().await?;
self.reset_kernel().await;
self.wait_for_exec_tool_calls(&req_id).await;
self.exec_tool_calls.lock().await.clear();
return Err(FunctionCallError::RespondToModel(
"js_repl execution timed out; kernel reset, rerun your request".to_string(),
));
@@ -854,7 +856,9 @@ impl JsReplManager {
JsReplManager::clear_exec_tool_calls_map(&exec_tool_calls, &id).await;
}
KernelToHost::RunTool(req) => {
if !JsReplManager::begin_exec_tool_call(&exec_tool_calls, &req.exec_id).await {
let Some(reset_cancel) =
JsReplManager::begin_exec_tool_call(&exec_tool_calls, &req.exec_id).await
else {
let exec_id = req.exec_id.clone();
let tool_call_id = req.id.clone();
let payload = HostToKernel::RunToolResult(RunToolResult {
@@ -877,10 +881,10 @@ impl JsReplManager {
);
}
continue;
}
};
let stdin_clone = Arc::clone(&stdin);
let exec_contexts = Arc::clone(&exec_contexts);
let exec_tool_calls = Arc::clone(&exec_tool_calls);
let exec_tool_calls_for_task = Arc::clone(&exec_tool_calls);
let recent_stderr = Arc::clone(&recent_stderr);
tokio::spawn(async move {
let exec_id = req.exec_id.clone();
@@ -888,15 +892,26 @@ impl JsReplManager {
let tool_name = req.tool_name.clone();
let context = { exec_contexts.lock().await.get(&exec_id).cloned() };
let result = match context {
Some(ctx) => JsReplManager::run_tool_request(ctx, req).await,
Some(ctx) => {
tokio::select! {
_ = reset_cancel.cancelled() => RunToolResult {
id: tool_call_id.clone(),
ok: false,
response: None,
error: Some("js_repl execution reset".to_string()),
},
result = JsReplManager::run_tool_request(ctx, req) => result,
}
}
None => RunToolResult {
id: req.id.clone(),
id: tool_call_id.clone(),
ok: false,
response: None,
error: Some("js_repl exec context not found".to_string()),
},
};
JsReplManager::finish_exec_tool_call(&exec_tool_calls, &exec_id).await;
JsReplManager::finish_exec_tool_call(&exec_tool_calls_for_task, &exec_id)
.await;
let payload = HostToKernel::RunToolResult(result);
if let Err(err) = JsReplManager::write_message(&stdin_clone, &payload).await
{
@@ -1417,7 +1432,11 @@ mod tests {
.lock()
.await
.insert(exec_id.clone(), ExecToolCalls::default());
assert!(JsReplManager::begin_exec_tool_call(&exec_tool_calls, &exec_id).await);
assert!(
JsReplManager::begin_exec_tool_call(&exec_tool_calls, &exec_id)
.await
.is_some()
);
let wait_map = Arc::clone(&exec_tool_calls);
let wait_exec_id = exec_id.clone();
@@ -1441,6 +1460,110 @@ mod tests {
JsReplManager::clear_exec_tool_calls_map(&exec_tool_calls, &exec_id).await;
}
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn reset_waits_for_exec_lock_before_clearing_exec_tool_calls() {
let manager = JsReplManager::new(None, PathBuf::from("."))
.await
.expect("manager should initialize");
let permit = manager
.exec_lock
.clone()
.acquire_owned()
.await
.expect("lock should be acquirable");
let exec_id = Uuid::new_v4().to_string();
manager.register_exec_tool_calls(&exec_id).await;
let reset_manager = Arc::clone(&manager);
let mut reset_task = tokio::spawn(async move { reset_manager.reset().await });
tokio::time::sleep(Duration::from_millis(50)).await;
assert!(
!reset_task.is_finished(),
"reset should wait until execute lock is released"
);
assert!(
manager.exec_tool_calls.lock().await.contains_key(&exec_id),
"reset must not clear tool-call contexts while execute lock is held"
);
drop(permit);
tokio::time::timeout(Duration::from_secs(1), &mut reset_task)
.await
.expect("reset should complete after execute lock release")
.expect("reset task should not panic")
.expect("reset should succeed");
assert!(
!manager.exec_tool_calls.lock().await.contains_key(&exec_id),
"reset should clear tool-call contexts after lock acquisition"
);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn reset_clears_inflight_exec_tool_calls_without_waiting() {
let manager = JsReplManager::new(None, std::env::temp_dir())
.await
.expect("manager should initialize");
let exec_id = Uuid::new_v4().to_string();
manager.register_exec_tool_calls(&exec_id).await;
assert!(
JsReplManager::begin_exec_tool_call(&manager.exec_tool_calls, &exec_id)
.await
.is_some()
);
let wait_manager = Arc::clone(&manager);
let wait_exec_id = exec_id.clone();
let waiter = tokio::spawn(async move {
wait_manager.wait_for_exec_tool_calls(&wait_exec_id).await;
});
tokio::task::yield_now().await;
tokio::time::timeout(Duration::from_secs(1), manager.reset())
.await
.expect("reset should not hang")
.expect("reset should succeed");
tokio::time::timeout(Duration::from_secs(1), waiter)
.await
.expect("waiter should be released")
.expect("wait task should not panic");
assert!(manager.exec_tool_calls.lock().await.is_empty());
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn reset_aborts_inflight_exec_tool_tasks() {
let manager = JsReplManager::new(None, std::env::temp_dir())
.await
.expect("manager should initialize");
let exec_id = Uuid::new_v4().to_string();
manager.register_exec_tool_calls(&exec_id).await;
let reset_cancel = JsReplManager::begin_exec_tool_call(&manager.exec_tool_calls, &exec_id)
.await
.expect("exec should be registered");
let task = tokio::spawn(async move {
tokio::select! {
_ = reset_cancel.cancelled() => "cancelled",
_ = tokio::time::sleep(Duration::from_secs(60)) => "timed_out",
}
});
tokio::time::timeout(Duration::from_secs(1), manager.reset())
.await
.expect("reset should not hang")
.expect("reset should succeed");
let outcome = tokio::time::timeout(Duration::from_secs(1), task)
.await
.expect("cancelled task should resolve promptly")
.expect("task should not panic");
assert_eq!(outcome, "cancelled");
}
async fn can_run_js_repl_runtime_tests() -> bool {
if std::env::var_os("CODEX_SANDBOX").is_some() {
return false;
+88 -82
View File
@@ -10,9 +10,9 @@ use crate::tools::handlers::SEARCH_TOOL_BM25_DEFAULT_LIMIT;
use crate::tools::handlers::SEARCH_TOOL_BM25_TOOL_NAME;
use crate::tools::handlers::apply_patch::create_apply_patch_freeform_tool;
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::multi_agents::DEFAULT_WAIT_TIMEOUT_MS;
use crate::tools::handlers::multi_agents::MAX_WAIT_TIMEOUT_MS;
use crate::tools::handlers::multi_agents::MIN_WAIT_TIMEOUT_MS;
use crate::tools::handlers::request_user_input_tool_description;
use crate::tools::registry::ToolRegistryBuilder;
use codex_protocol::config_types::WebSearchMode;
@@ -41,7 +41,6 @@ pub(crate) struct ToolsConfig {
pub js_repl_tools_only: bool,
pub collab_tools: bool,
pub collaboration_modes_tools: bool,
pub request_rule_enabled: bool,
pub experimental_supported_tools: Vec<String>,
}
@@ -64,7 +63,6 @@ impl ToolsConfig {
include_js_repl && features.enabled(Feature::JsReplToolsOnly);
let include_collab_tools = features.enabled(Feature::Collab);
let include_collaboration_modes_tools = features.enabled(Feature::CollaborationModes);
let request_rule_enabled = features.enabled(Feature::RequestRule);
let include_search_tool = features.enabled(Feature::Apps);
let shell_type = if !features.enabled(Feature::ShellTool) {
@@ -101,7 +99,6 @@ impl ToolsConfig {
js_repl_tools_only: include_js_repl_tools_only,
collab_tools: include_collab_tools,
collaboration_modes_tools: include_collaboration_modes_tools,
request_rule_enabled,
experimental_supported_tools: model_info.experimental_supported_tools.clone(),
}
}
@@ -174,7 +171,7 @@ impl From<JsonSchema> for AdditionalProperties {
}
}
fn create_approval_parameters(include_prefix_rule: bool) -> BTreeMap<String, JsonSchema> {
fn create_approval_parameters() -> BTreeMap<String, JsonSchema> {
let mut properties = BTreeMap::from([
(
"sandbox_permissions".to_string(),
@@ -200,23 +197,22 @@ fn create_approval_parameters(include_prefix_rule: bool) -> BTreeMap<String, Jso
),
]);
if include_prefix_rule {
properties.insert(
"prefix_rule".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some(
r#"Only specify when sandbox_permissions is `require_escalated`.
properties.insert(
"prefix_rule".to_string(),
JsonSchema::Array {
items: Box::new(JsonSchema::String { description: None }),
description: Some(
r#"Only specify when sandbox_permissions is `require_escalated`.
Suggest a prefix command pattern that will allow you to fulfill similar requests from the user in the future.
Should be a short but reasonable prefix, e.g. [\"git\", \"pull\"] or [\"uv\", \"run\"] or [\"pytest\"]."#.to_string(),
),
});
}
),
},
);
properties
}
fn create_exec_command_tool(include_prefix_rule: bool) -> ToolSpec {
fn create_exec_command_tool() -> ToolSpec {
let mut properties = BTreeMap::from([
(
"cmd".to_string(),
@@ -274,7 +270,7 @@ fn create_exec_command_tool(include_prefix_rule: bool) -> ToolSpec {
},
),
]);
properties.extend(create_approval_parameters(include_prefix_rule));
properties.extend(create_approval_parameters());
ToolSpec::Function(ResponsesApiTool {
name: "exec_command".to_string(),
@@ -337,7 +333,7 @@ fn create_write_stdin_tool() -> ToolSpec {
})
}
fn create_shell_tool(include_prefix_rule: bool) -> ToolSpec {
fn create_shell_tool() -> ToolSpec {
let mut properties = BTreeMap::from([
(
"command".to_string(),
@@ -359,7 +355,7 @@ fn create_shell_tool(include_prefix_rule: bool) -> ToolSpec {
},
),
]);
properties.extend(create_approval_parameters(include_prefix_rule));
properties.extend(create_approval_parameters());
let description = if cfg!(windows) {
r#"Runs a Powershell command (Windows) and returns its output. Arguments to `shell` will be passed to CreateProcessW(). Most commands should be prefixed with ["powershell.exe", "-Command"].
@@ -390,7 +386,7 @@ Examples of valid command strings:
})
}
fn create_shell_command_tool(include_prefix_rule: bool) -> ToolSpec {
fn create_shell_command_tool() -> ToolSpec {
let mut properties = BTreeMap::from([
(
"command".to_string(),
@@ -422,7 +418,7 @@ fn create_shell_command_tool(include_prefix_rule: bool) -> ToolSpec {
},
),
]);
properties.extend(create_approval_parameters(include_prefix_rule));
properties.extend(create_approval_parameters());
let description = if cfg!(windows) {
r#"Runs a Powershell command (Windows) and returns its output.
@@ -1405,7 +1401,6 @@ pub(crate) fn build_specs(
dynamic_tools: &[DynamicToolSpec],
) -> ToolRegistryBuilder {
use crate::tools::handlers::ApplyPatchHandler;
use crate::tools::handlers::CollabHandler;
use crate::tools::handlers::DynamicToolHandler;
use crate::tools::handlers::GrepFilesHandler;
use crate::tools::handlers::JsReplHandler;
@@ -1413,6 +1408,7 @@ pub(crate) fn build_specs(
use crate::tools::handlers::ListDirHandler;
use crate::tools::handlers::McpHandler;
use crate::tools::handlers::McpResourceHandler;
use crate::tools::handlers::MultiAgentHandler;
use crate::tools::handlers::PlanHandler;
use crate::tools::handlers::ReadFileHandler;
use crate::tools::handlers::RequestUserInputHandler;
@@ -1442,19 +1438,13 @@ pub(crate) fn build_specs(
match &config.shell_type {
ConfigShellToolType::Default => {
builder.push_spec_with_parallel_support(
create_shell_tool(config.request_rule_enabled),
true,
);
builder.push_spec_with_parallel_support(create_shell_tool(), true);
}
ConfigShellToolType::Local => {
builder.push_spec_with_parallel_support(ToolSpec::LocalShell {}, true);
}
ConfigShellToolType::UnifiedExec => {
builder.push_spec_with_parallel_support(
create_exec_command_tool(config.request_rule_enabled),
true,
);
builder.push_spec_with_parallel_support(create_exec_command_tool(), 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);
@@ -1463,10 +1453,7 @@ pub(crate) fn build_specs(
// Do nothing.
}
ConfigShellToolType::ShellCommand => {
builder.push_spec_with_parallel_support(
create_shell_command_tool(config.request_rule_enabled),
true,
);
builder.push_spec_with_parallel_support(create_shell_command_tool(), true);
}
}
@@ -1478,12 +1465,14 @@ pub(crate) fn build_specs(
builder.register_handler("shell_command", shell_command_handler);
}
builder.push_spec_with_parallel_support(create_list_mcp_resources_tool(), true);
builder.push_spec_with_parallel_support(create_list_mcp_resource_templates_tool(), true);
builder.push_spec_with_parallel_support(create_read_mcp_resource_tool(), true);
builder.register_handler("list_mcp_resources", mcp_resource_handler.clone());
builder.register_handler("list_mcp_resource_templates", mcp_resource_handler.clone());
builder.register_handler("read_mcp_resource", mcp_resource_handler);
if mcp_tools.is_some() {
builder.push_spec_with_parallel_support(create_list_mcp_resources_tool(), true);
builder.push_spec_with_parallel_support(create_list_mcp_resource_templates_tool(), true);
builder.push_spec_with_parallel_support(create_read_mcp_resource_tool(), true);
builder.register_handler("list_mcp_resources", mcp_resource_handler.clone());
builder.register_handler("list_mcp_resource_templates", mcp_resource_handler.clone());
builder.register_handler("read_mcp_resource", mcp_resource_handler);
}
builder.push_spec(PLAN_TOOL.clone());
builder.register_handler("update_plan", plan_handler);
@@ -1574,17 +1563,17 @@ pub(crate) fn build_specs(
builder.register_handler("view_image", view_image_handler);
if config.collab_tools {
let collab_handler = Arc::new(CollabHandler);
let multi_agent_handler = Arc::new(MultiAgentHandler);
builder.push_spec(create_spawn_agent_tool());
builder.push_spec(create_send_input_tool());
builder.push_spec(create_resume_agent_tool());
builder.push_spec(create_wait_tool());
builder.push_spec(create_close_agent_tool());
builder.register_handler("spawn_agent", collab_handler.clone());
builder.register_handler("send_input", collab_handler.clone());
builder.register_handler("resume_agent", collab_handler.clone());
builder.register_handler("wait", collab_handler.clone());
builder.register_handler("close_agent", collab_handler);
builder.register_handler("spawn_agent", multi_agent_handler.clone());
builder.register_handler("send_input", multi_agent_handler.clone());
builder.register_handler("resume_agent", multi_agent_handler.clone());
builder.register_handler("wait", multi_agent_handler.clone());
builder.register_handler("close_agent", multi_agent_handler);
}
if let Some(mcp_tools) = mcp_tools {
@@ -1830,11 +1819,8 @@ mod tests {
// Build expected from the same helpers used by the builder.
let mut expected: BTreeMap<String, ToolSpec> = BTreeMap::from([]);
for spec in [
create_exec_command_tool(true),
create_exec_command_tool(),
create_write_stdin_tool(),
create_list_mcp_resources_tool(),
create_list_mcp_resource_templates_tool(),
create_read_mcp_resource_tool(),
PLAN_TOOL.clone(),
create_request_user_input_tool(),
create_apply_patch_freeform_tool(),
@@ -2039,7 +2025,7 @@ mod tests {
features,
web_search_mode,
});
let (tools, _) = build_specs(&tools_config, Some(HashMap::new()), None, &[]).build();
let (tools, _) = build_specs(&tools_config, None, None, &[]).build();
let tool_names = tools.iter().map(|t| t.spec.name()).collect::<Vec<_>>();
assert_eq!(&tool_names, &expected_tools,);
}
@@ -2106,6 +2092,53 @@ mod tests {
);
}
#[test]
fn mcp_resource_tools_are_hidden_without_mcp_servers() {
let config = test_config();
let model_info =
ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config);
let mut features = Features::with_defaults();
features.enable(Feature::CollaborationModes);
let tools_config = ToolsConfig::new(&ToolsConfigParams {
model_info: &model_info,
features: &features,
web_search_mode: Some(WebSearchMode::Cached),
});
let (tools, _) = build_specs(&tools_config, None, None, &[]).build();
assert!(
!tools.iter().any(|tool| matches!(
tool.spec.name(),
"list_mcp_resources" | "list_mcp_resource_templates" | "read_mcp_resource"
)),
"MCP resource tools should be omitted when no MCP servers are configured"
);
}
#[test]
fn mcp_resource_tools_are_included_when_mcp_servers_are_present() {
let config = test_config();
let model_info =
ModelsManager::construct_model_info_offline_for_tests("gpt-5-codex", &config);
let mut features = Features::with_defaults();
features.enable(Feature::CollaborationModes);
let tools_config = ToolsConfig::new(&ToolsConfigParams {
model_info: &model_info,
features: &features,
web_search_mode: Some(WebSearchMode::Cached),
});
let (tools, _) = build_specs(&tools_config, Some(HashMap::new()), None, &[]).build();
assert_contains_tool_names(
&tools,
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
],
);
}
#[test]
fn test_build_specs_gpt5_codex_default() {
let mut features = Features::with_defaults();
@@ -2116,9 +2149,6 @@ mod tests {
Some(WebSearchMode::Cached),
"shell_command",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -2138,9 +2168,6 @@ mod tests {
Some(WebSearchMode::Cached),
"shell_command",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -2162,9 +2189,6 @@ mod tests {
&[
"exec_command",
"write_stdin",
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -2186,9 +2210,6 @@ mod tests {
&[
"exec_command",
"write_stdin",
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -2208,9 +2229,6 @@ mod tests {
Some(WebSearchMode::Cached),
"shell_command",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -2230,9 +2248,6 @@ mod tests {
Some(WebSearchMode::Cached),
"shell_command",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -2252,9 +2267,6 @@ mod tests {
Some(WebSearchMode::Cached),
"shell",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"web_search",
@@ -2273,9 +2285,6 @@ mod tests {
Some(WebSearchMode::Cached),
"shell_command",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -2297,9 +2306,6 @@ mod tests {
&[
"exec_command",
"write_stdin",
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -2796,7 +2802,7 @@ mod tests {
#[test]
fn test_shell_tool() {
let tool = super::create_shell_tool(true);
let tool = super::create_shell_tool();
let ToolSpec::Function(ResponsesApiTool {
description, name, ..
}) = &tool
@@ -2826,7 +2832,7 @@ Examples of valid command strings:
#[test]
fn test_shell_command_tool() {
let tool = super::create_shell_command_tool(true);
let tool = super::create_shell_command_tool();
let ToolSpec::Function(ResponsesApiTool {
description, name, ..
}) = &tool
@@ -24,5 +24,5 @@ Notes:
- `description`
- `connector_name`
- input schema property keys (`input_keys`)
- If the needed app is already explicit in the prompt (for example an `apps://...` mention) or already present in the current `tools` list, you can call that tool directly.
- If the needed app is already explicit in the prompt (for example `[$app-name](app://{connector_id})`) or already present in the current `tools` list, you can call that tool directly.
- Do not use `search_tool_bm25` for non-apps/local tasks (filesystem, repo search, or shell-only workflows) or anything not related to {{app_names}}.
+44 -5
View File
@@ -1479,6 +1479,44 @@ fn scenarios() -> Vec<ScenarioSpec> {
output_contains: "rejected by user",
},
},
ScenarioSpec {
name: "safe command with heredoc and redirect still requires approval",
approval_policy: AskForApproval::OnRequest,
sandbox_policy: workspace_write(false),
action: ActionKind::RunUnifiedExecCommand {
command: "cat <<'EOF' > /tmp/out.txt \nhello\nEOF",
justification: None,
},
sandbox_permissions: SandboxPermissions::RequireEscalated,
features: vec![Feature::UnifiedExec],
model_override: None,
outcome: Outcome::ExecApproval {
decision: ReviewDecision::Denied,
expected_reason: None,
},
expectation: Expectation::CommandFailure {
output_contains: "rejected by user",
},
},
ScenarioSpec {
name: "compound command with one safe command still requires approval",
approval_policy: AskForApproval::OnRequest,
sandbox_policy: workspace_write(false),
action: ActionKind::RunUnifiedExecCommand {
command: "cat ./one.txt && touch ./two.txt",
justification: None,
},
sandbox_permissions: SandboxPermissions::RequireEscalated,
features: vec![Feature::UnifiedExec],
model_override: None,
outcome: Outcome::ExecApproval {
decision: ReviewDecision::Denied,
expected_reason: None,
},
expectation: Expectation::CommandFailure {
output_contains: "rejected by user",
},
},
]
}
@@ -1890,14 +1928,15 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts
Ok(())
}
// todo(dylan) add ScenarioSpec support for rules
#[tokio::test(flavor = "current_thread")]
#[cfg(unix)]
async fn heredoc_with_chained_allowed_prefix_still_requires_approval() -> Result<()> {
async fn compound_command_with_one_safe_command_still_requires_approval() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let approval_policy = AskForApproval::UnlessTrusted;
let sandbox_policy = SandboxPolicy::new_read_only_policy();
let sandbox_policy = SandboxPolicy::new_workspace_write_policy();
let sandbox_policy_for_config = sandbox_policy.clone();
let mut builder = test_codex().with_config(move |config| {
config.permissions.approval_policy = Constrained::allow_any(approval_policy);
@@ -1913,12 +1952,12 @@ async fn heredoc_with_chained_allowed_prefix_still_requires_approval() -> Result
)?;
let call_id = "heredoc-with-chained-prefix";
let command = "cat <<'EOF' > /tmp/test.txt && touch allow-prefix.txt\nhello\nEOF";
let command = "touch ./test.txt && rm ./test.txt";
let (event, expected_command) = ActionKind::RunCommand { command }
.prepare(&test, &server, call_id, SandboxPermissions::UseDefault)
.await?;
let expected_command =
expected_command.expect("heredoc chained command scenario should produce a shell command");
expected_command.expect("compound command should produce a shell command");
let _ = mount_sse_once(
&server,
@@ -1940,7 +1979,7 @@ async fn heredoc_with_chained_allowed_prefix_still_requires_approval() -> Result
submit_turn(
&test,
"heredoc chained prefix",
"compound command",
approval_policy,
sandbox_policy.clone(),
)
+1
View File
@@ -103,6 +103,7 @@ mod resume_warning;
mod review;
mod rmcp_client;
mod rollout_list_find;
mod safety_check_downgrade;
mod search_tool;
mod seatbelt;
mod shell_command;
-15
View File
@@ -75,9 +75,6 @@ async fn model_selects_expected_tools() {
expected_default_tools(
"shell_command",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -94,9 +91,6 @@ async fn model_selects_expected_tools() {
expected_default_tools(
"shell_command",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -113,9 +107,6 @@ async fn model_selects_expected_tools() {
expected_default_tools(
"shell_command",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -132,9 +123,6 @@ async fn model_selects_expected_tools() {
expected_default_tools(
"shell",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"web_search",
@@ -150,9 +138,6 @@ async fn model_selects_expected_tools() {
expected_default_tools(
"shell_command",
&[
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -488,7 +488,6 @@ async fn permissions_message_includes_writable_roots() -> Result<()> {
&sandbox_policy,
AskForApproval::OnRequest,
&Policy::empty(),
true,
test.config.cwd.as_path(),
)
.into_text();
@@ -144,9 +144,6 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> {
vec!["exec_command", "write_stdin"]
};
expected_tools_names.extend([
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
"update_plan",
"request_user_input",
"apply_patch",
@@ -0,0 +1,228 @@
use anyhow::Result;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::EventMsg;
use codex_core::protocol::Op;
use codex_core::protocol::SandboxPolicy;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::user_input::UserInput;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_function_call;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_response_once;
use core_test_support::responses::mount_response_sequence;
use core_test_support::responses::sse;
use core_test_support::responses::sse_completed;
use core_test_support::responses::sse_response;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
const SERVER_MODEL: &str = "gpt-5.2";
const REQUESTED_MODEL: &str = "gpt-5.3-codex";
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn openai_model_header_mismatch_emits_warning_event_and_warning_item() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let response =
sse_response(sse_completed("resp-1")).insert_header("OpenAI-Model", SERVER_MODEL);
let _mock = mount_response_once(&server, response).await;
let mut builder = test_codex().with_model(REQUESTED_MODEL);
let test = builder.build(&server).await?;
test.codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: "trigger safety check".to_string(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: test.cwd_path().to_path_buf(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::DangerFullAccess,
model: REQUESTED_MODEL.to_string(),
effort: test.config.model_reasoning_effort,
summary: ReasoningSummary::Auto,
collaboration_mode: None,
personality: None,
})
.await?;
let warning = wait_for_event(&test.codex, |event| matches!(event, EventMsg::Warning(_))).await;
let EventMsg::Warning(warning) = warning else {
panic!("expected warning event");
};
assert!(warning.message.contains(REQUESTED_MODEL));
assert!(warning.message.contains(SERVER_MODEL));
let warning_item = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::RawResponseItem(raw)
if matches!(
&raw.item,
ResponseItem::Message { content, .. }
if content.iter().any(|item| matches!(
item,
ContentItem::InputText { text } if text.starts_with("Warning: ")
))
)
)
})
.await;
let EventMsg::RawResponseItem(raw) = warning_item else {
panic!("expected raw response item event");
};
let ResponseItem::Message { role, content, .. } = raw.item else {
panic!("expected warning to be recorded as a message item");
};
assert_eq!(role, "user");
let warning_text = content.iter().find_map(|item| match item {
ContentItem::InputText { text } => Some(text.as_str()),
_ => None,
});
let warning_text = warning_text.expect("warning message should include input_text content");
assert!(warning_text.contains(REQUESTED_MODEL));
assert!(warning_text.contains(SERVER_MODEL));
let _ = wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn response_model_field_mismatch_emits_warning_when_header_matches_requested() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let response = sse_response(sse(vec![
serde_json::json!({
"type": "response.created",
"response": {
"id": "resp-1",
"model": SERVER_MODEL,
}
}),
core_test_support::responses::ev_completed("resp-1"),
]))
.insert_header("OpenAI-Model", REQUESTED_MODEL);
let _mock = mount_response_once(&server, response).await;
let mut builder = test_codex().with_model(REQUESTED_MODEL);
let test = builder.build(&server).await?;
test.codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: "trigger response model check".to_string(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: test.cwd_path().to_path_buf(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::DangerFullAccess,
model: REQUESTED_MODEL.to_string(),
effort: test.config.model_reasoning_effort,
summary: ReasoningSummary::Auto,
collaboration_mode: None,
personality: None,
})
.await?;
let warning = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::Warning(warning)
if warning
.message
.contains("flagged for potentially high-risk cyber activity")
)
})
.await;
let EventMsg::Warning(warning) = warning else {
panic!("expected warning event");
};
assert!(warning.message.contains("gpt-5.2 as a fallback"));
let _ = wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn openai_model_header_mismatch_only_emits_one_warning_per_turn() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let tool_args = serde_json::json!({
"command": "echo hello",
"timeout_ms": 1_000
});
let first_response = sse_response(sse(vec![
ev_response_created("resp-1"),
ev_function_call(
"call-1",
"shell_command",
&serde_json::to_string(&tool_args)?,
),
core_test_support::responses::ev_completed("resp-1"),
]))
.insert_header("OpenAI-Model", SERVER_MODEL);
let second_response = sse_response(sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "done"),
core_test_support::responses::ev_completed("resp-2"),
]))
.insert_header("OpenAI-Model", SERVER_MODEL);
let _mock = mount_response_sequence(&server, vec![first_response, second_response]).await;
let mut builder = test_codex().with_model(REQUESTED_MODEL);
let test = builder.build(&server).await?;
test.codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: "trigger follow-up turn".to_string(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: test.cwd_path().to_path_buf(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::DangerFullAccess,
model: REQUESTED_MODEL.to_string(),
effort: test.config.model_reasoning_effort,
summary: ReasoningSummary::Auto,
collaboration_mode: None,
personality: None,
})
.await?;
let mut warning_count = 0;
loop {
let event = wait_for_event(&test.codex, |_| true).await;
match event {
EventMsg::Warning(warning) if warning.message.contains(REQUESTED_MODEL) => {
warning_count += 1;
}
EventMsg::TurnComplete(_) => break,
_ => {}
}
}
assert_eq!(warning_count, 1);
Ok(())
}
+2 -1
View File
@@ -279,7 +279,7 @@ impl CodexLogSnapshot {
}
let level = match classification {
"bug" | "bad_result" => Level::Error,
"bug" | "bad_result" | "safety_check" => Level::Error,
_ => Level::Info,
};
@@ -342,6 +342,7 @@ fn display_classification(classification: &str) -> String {
"bug" => "Bug".to_string(),
"bad_result" => "Bad result".to_string(),
"good_result" => "Good result".to_string(),
"safety_check" => "Safety check".to_string(),
_ => "Other".to_string(),
}
}
+1
View File
@@ -748,6 +748,7 @@ impl OtelManager {
ResponseEvent::ReasoningSummaryPartAdded { .. } => {
"reasoning_summary_part_added".into()
}
ResponseEvent::ServerModel(_) => "server_model".into(),
ResponseEvent::ServerReasoningIncluded(_) => "server_reasoning_included".into(),
ResponseEvent::RateLimits(_) => "rate_limits".into(),
ResponseEvent::ModelsEtag(_) => "models_etag".into(),
+11 -32
View File
@@ -225,8 +225,6 @@ const APPROVAL_POLICY_UNLESS_TRUSTED: &str =
include_str!("prompts/permissions/approval_policy/unless_trusted.md");
const APPROVAL_POLICY_ON_FAILURE: &str =
include_str!("prompts/permissions/approval_policy/on_failure.md");
const APPROVAL_POLICY_ON_REQUEST: &str =
include_str!("prompts/permissions/approval_policy/on_request.md");
const APPROVAL_POLICY_ON_REQUEST_RULE: &str =
include_str!("prompts/permissions/approval_policy/on_request_rule.md");
@@ -241,29 +239,20 @@ impl DeveloperInstructions {
Self { text: text.into() }
}
pub fn from(
approval_policy: AskForApproval,
exec_policy: &Policy,
request_rule_enabled: bool,
) -> DeveloperInstructions {
pub fn from(approval_policy: AskForApproval, exec_policy: &Policy) -> DeveloperInstructions {
let text = match approval_policy {
AskForApproval::Never => APPROVAL_POLICY_NEVER.to_string(),
AskForApproval::UnlessTrusted => APPROVAL_POLICY_UNLESS_TRUSTED.to_string(),
AskForApproval::OnFailure => APPROVAL_POLICY_ON_FAILURE.to_string(),
AskForApproval::OnRequest => {
if !request_rule_enabled {
APPROVAL_POLICY_ON_REQUEST.to_string()
} else {
let command_prefixes =
format_allow_prefixes(exec_policy.get_allowed_prefixes());
match command_prefixes {
Some(prefixes) => {
format!(
"{APPROVAL_POLICY_ON_REQUEST_RULE}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}"
)
}
None => APPROVAL_POLICY_ON_REQUEST_RULE.to_string(),
let command_prefixes = format_allow_prefixes(exec_policy.get_allowed_prefixes());
match command_prefixes {
Some(prefixes) => {
format!(
"{APPROVAL_POLICY_ON_REQUEST_RULE}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}"
)
}
None => APPROVAL_POLICY_ON_REQUEST_RULE.to_string(),
}
}
};
@@ -301,7 +290,6 @@ impl DeveloperInstructions {
sandbox_policy: &SandboxPolicy,
approval_policy: AskForApproval,
exec_policy: &Policy,
request_rule_enabled: bool,
cwd: &Path,
) -> Self {
let network_access = if sandbox_policy.has_full_network_access() {
@@ -325,7 +313,6 @@ impl DeveloperInstructions {
network_access,
approval_policy,
exec_policy,
request_rule_enabled,
writable_roots,
)
}
@@ -349,7 +336,6 @@ impl DeveloperInstructions {
network_access: NetworkAccess,
approval_policy: AskForApproval,
exec_policy: &Policy,
request_rule_enabled: bool,
writable_roots: Option<Vec<WritableRoot>>,
) -> Self {
let start_tag = DeveloperInstructions::new("<permissions instructions>");
@@ -359,11 +345,7 @@ impl DeveloperInstructions {
sandbox_mode,
network_access,
))
.concat(DeveloperInstructions::from(
approval_policy,
exec_policy,
request_rule_enabled,
))
.concat(DeveloperInstructions::from(approval_policy, exec_policy))
.concat(DeveloperInstructions::from_writable_roots(writable_roots))
.concat(end_tag)
}
@@ -1207,7 +1189,6 @@ mod tests {
NetworkAccess::Enabled,
AskForApproval::OnRequest,
&Policy::empty(),
false,
None,
);
@@ -1217,7 +1198,7 @@ mod tests {
"expected network access to be enabled in message"
);
assert!(
text.contains("`approval_policy` is `on-request`"),
text.contains("How to request escalation"),
"expected approval guidance to be included"
);
}
@@ -1236,7 +1217,6 @@ mod tests {
&policy,
AskForApproval::UnlessTrusted,
&Policy::empty(),
false,
&PathBuf::from("/tmp"),
);
let text = instructions.into_text();
@@ -1245,7 +1225,7 @@ mod tests {
}
#[test]
fn includes_request_rule_instructions_when_enabled() {
fn includes_request_rule_instructions_for_on_request() {
let mut exec_policy = Policy::empty();
exec_policy
.add_prefix_rule(
@@ -1258,7 +1238,6 @@ mod tests {
NetworkAccess::Enabled,
AskForApproval::OnRequest,
&exec_policy,
true,
None,
);
@@ -1,12 +0,0 @@
Approvals are your mechanism to get user consent to run shell commands without the sandbox. `approval_policy` is `on-request`: Commands will be run in the sandbox by default, and you can specify in your tool call if you want to escalate a command to run without sandboxing. If the completing the task requires escalated permissions, Do not let these settings or the sandbox deter you from attempting to accomplish the user's task.
Here are scenarios where you'll need to request approval:
- You need to run a command that writes to a directory that requires it (e.g. running tests that write to /var)
- You need to run a GUI app (e.g., open/xdg-open/osascript) to open browsers or files.
- You are running sandboxed and need to run a command that requires network access (e.g. installing packages)
- If you run a command that is important to solving the user's query, but it fails because of sandboxing, rerun the command with approval. ALWAYS proceed to use the `sandbox_permissions` and `justification` parameters - do not message the user before requesting approval for the command.
- You are about to take a potentially destructive action such as an `rm` or `git reset` that the user did not explicitly ask for.
When requesting approval to execute a command that will require escalated privileges:
- Provide the `sandbox_permissions` parameter with the value `"require_escalated"`
- Include a short, 1 sentence explanation for why you need escalated permissions in the justification parameter
+14 -18
View File
@@ -128,9 +128,6 @@ pub fn parse_shell_lc_single_command_prefix(command: &[String]) -> Option<Vec<St
if root.has_error() {
return None;
}
if !has_named_descendant_kind(root, "heredoc_redirect") {
return None;
}
let command_node = find_single_command_node(root)?;
parse_heredoc_command_words(command_node, script)
@@ -237,20 +234,6 @@ fn is_literal_word_or_number(node: Node<'_>) -> bool {
node.named_children(&mut cursor).next().is_none()
}
fn has_named_descendant_kind(node: Node<'_>, kind: &str) -> bool {
let mut stack = vec![node];
while let Some(current) = stack.pop() {
if current.kind() == kind {
return true;
}
let mut cursor = current.walk();
for child in current.named_children(&mut cursor) {
stack.push(child);
}
}
false
}
fn is_allowed_heredoc_attachment_kind(kind: &str) -> bool {
matches!(
kind,
@@ -532,7 +515,10 @@ mod tests {
"-lc".to_string(),
"echo hello > /tmp/out.txt".to_string(),
];
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
assert_eq!(
parse_shell_lc_single_command_prefix(&command),
Some(vec!["echo".to_string(), "hello".to_string()])
);
}
#[test]
@@ -548,6 +534,16 @@ mod tests {
);
}
#[test]
fn parse_shell_lc_single_command_prefix_rejects_herestring_with_chaining() {
let command = vec![
"bash".to_string(),
"-lc".to_string(),
r#"echo hello > /tmp/out.txt && cat /tmp/out.txt"#.to_string(),
];
assert_eq!(parse_shell_lc_single_command_prefix(&command), None);
}
#[test]
fn parse_shell_lc_single_command_prefix_rejects_herestring_with_substitution() {
let command = vec![
+27 -9
View File
@@ -46,6 +46,10 @@ struct Args {
#[arg(long = "thread-id")]
thread_id: Vec<String>,
/// Substring match against the log message.
#[arg(long)]
search: Option<String>,
/// Include logs that do not have a thread id.
#[arg(long)]
threadless: bool,
@@ -57,6 +61,10 @@ struct Args {
/// Poll interval in milliseconds.
#[arg(long, default_value_t = 500)]
poll_ms: u64,
/// Show compact output with only time, level, and message.
#[arg(long)]
compact: bool,
}
#[derive(Debug, Clone)]
@@ -67,6 +75,7 @@ struct LogFilter {
module_like: Vec<String>,
file_like: Vec<String>,
thread_ids: Vec<String>,
search: Option<String>,
include_threadless: bool,
}
@@ -81,7 +90,8 @@ async fn main() -> anyhow::Result<()> {
.unwrap_or_else(|| PathBuf::from("."));
let runtime = StateRuntime::init(codex_home, "logs-client".to_string(), None).await?;
let mut last_id = print_backfill(runtime.as_ref(), &filter, args.backfill).await?;
let mut last_id =
print_backfill(runtime.as_ref(), &filter, args.backfill, args.compact).await?;
if last_id == 0 {
last_id = fetch_max_id(runtime.as_ref(), &filter).await?;
}
@@ -91,7 +101,7 @@ async fn main() -> anyhow::Result<()> {
let rows = fetch_new_rows(runtime.as_ref(), &filter, last_id).await?;
for row in rows {
last_id = last_id.max(row.id);
println!("{}", format_row(&row));
println!("{}", format_row(&row, args.compact));
}
tokio::time::sleep(poll_interval).await;
}
@@ -154,6 +164,7 @@ fn build_filter(args: &Args) -> anyhow::Result<LogFilter> {
module_like,
file_like,
thread_ids,
search: args.search.clone(),
include_threadless: args.threadless,
})
}
@@ -172,6 +183,7 @@ async fn print_backfill(
runtime: &StateRuntime,
filter: &LogFilter,
backfill: usize,
compact: bool,
) -> anyhow::Result<i64> {
if backfill == 0 {
return Ok(0);
@@ -183,7 +195,7 @@ async fn print_backfill(
let mut last_id = 0;
for row in rows {
last_id = last_id.max(row.id);
println!("{}", format_row(&row));
println!("{}", format_row(&row, compact));
}
Ok(last_id)
}
@@ -233,6 +245,7 @@ fn to_log_query(
module_like: filter.module_like.clone(),
file_like: filter.file_like.clone(),
thread_ids: filter.thread_ids.clone(),
search: filter.search.clone(),
include_threadless: filter.include_threadless,
after_id,
limit,
@@ -240,8 +253,8 @@ fn to_log_query(
}
}
fn format_row(row: &LogRow) -> String {
let timestamp = formatter::ts(row.ts, row.ts_nanos);
fn format_row(row: &LogRow, compact: bool) -> String {
let timestamp = formatter::ts(row.ts, row.ts_nanos, compact);
let level = row.level.as_str();
let target = row.target.as_str();
let message = row.message.as_deref().unwrap_or("");
@@ -251,9 +264,13 @@ fn format_row(row: &LogRow) -> String {
let thread_id_colored = thread_id.blue().dimmed().to_string();
let target_colored = target.dimmed().to_string();
let message_colored = heuristic_formatting(message);
format!(
"{timestamp_colored} {level_colored} [{thread_id_colored}] {target_colored} - {message_colored}"
)
if compact {
format!("{timestamp_colored} {level_colored} {message_colored}")
} else {
format!(
"{timestamp_colored} {level_colored} [{thread_id_colored}] {target_colored} - {message_colored}"
)
}
}
fn heuristic_formatting(message: &str) -> String {
@@ -292,9 +309,10 @@ mod formatter {
.join("\n")
}
pub(super) fn ts(ts: i64, ts_nanos: i64) -> String {
pub(super) fn ts(ts: i64, ts_nanos: i64, compact: bool) -> String {
let nanos = u32::try_from(ts_nanos).unwrap_or(0);
match DateTime::<Utc>::from_timestamp(ts, nanos) {
Some(dt) if compact => dt.format("%H:%M:%S").to_string(),
Some(dt) => dt.to_rfc3339_opts(SecondsFormat::Millis, true),
None => format!("{ts}.{ts_nanos:09}Z"),
}
+1
View File
@@ -37,6 +37,7 @@ pub struct LogQuery {
pub module_like: Vec<String>,
pub file_like: Vec<String>,
pub thread_ids: Vec<String>,
pub search: Option<String>,
pub include_threadless: bool,
pub after_id: Option<i64>,
pub limit: Option<usize>,
+58
View File
@@ -711,6 +711,11 @@ fn push_log_filters<'a>(builder: &mut QueryBuilder<'a, Sqlite>, query: &'a LogQu
if let Some(after_id) = query.after_id {
builder.push(" AND id > ").push_bind(after_id);
}
if let Some(search) = query.search.as_ref() {
builder.push(" AND INSTR(message, ");
builder.push_bind(search.as_str());
builder.push(") > 0");
}
}
fn push_like_filters<'a>(
@@ -906,6 +911,8 @@ mod tests {
use super::StateRuntime;
use super::ThreadMetadata;
use super::state_db_filename;
use crate::LogEntry;
use crate::LogQuery;
use crate::STATE_DB_FILENAME;
use crate::STATE_DB_VERSION;
use crate::model::Phase2JobClaimOutcome;
@@ -2495,6 +2502,57 @@ VALUES (?, ?, ?, ?, ?)
let _ = tokio::fs::remove_dir_all(codex_home).await;
}
#[tokio::test]
async fn query_logs_with_search_matches_substring() {
let codex_home = unique_temp_dir();
let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None)
.await
.expect("initialize runtime");
runtime
.insert_logs(&[
LogEntry {
ts: 1_700_000_001,
ts_nanos: 0,
level: "INFO".to_string(),
target: "cli".to_string(),
message: Some("alpha".to_string()),
thread_id: Some("thread-1".to_string()),
process_uuid: None,
file: Some("main.rs".to_string()),
line: Some(42),
module_path: None,
},
LogEntry {
ts: 1_700_000_002,
ts_nanos: 0,
level: "INFO".to_string(),
target: "cli".to_string(),
message: Some("alphabet".to_string()),
thread_id: Some("thread-1".to_string()),
process_uuid: None,
file: Some("main.rs".to_string()),
line: Some(43),
module_path: None,
},
])
.await
.expect("insert test logs");
let rows = runtime
.query_logs(&LogQuery {
search: Some("alphab".to_string()),
..Default::default()
})
.await
.expect("query matching logs");
assert_eq!(rows.len(), 1);
assert_eq!(rows[0].message.as_deref(), Some("alphabet"));
let _ = tokio::fs::remove_dir_all(codex_home).await;
}
fn test_thread_metadata(
codex_home: &Path,
thread_id: ThreadId,
+104 -1
View File
@@ -978,6 +978,27 @@ impl App {
self.refresh_status_line();
}
fn should_wait_for_initial_session(session_selection: &SessionSelection) -> bool {
matches!(
session_selection,
SessionSelection::StartFresh | SessionSelection::Exit
)
}
fn should_handle_active_thread_events(
waiting_for_initial_session_configured: bool,
has_active_thread_receiver: bool,
) -> bool {
has_active_thread_receiver && !waiting_for_initial_session_configured
}
fn should_stop_waiting_for_initial_session(
waiting_for_initial_session_configured: bool,
primary_thread_id: Option<ThreadId>,
) -> bool {
waiting_for_initial_session_configured && primary_thread_id.is_some()
}
#[allow(clippy::too_many_arguments)]
pub async fn run(
tui: &mut tui::Tui,
@@ -1068,6 +1089,8 @@ impl App {
let status_line_invalid_items_warned = Arc::new(AtomicBool::new(false));
let enhanced_keys_supported = tui.enhanced_keys_supported();
let wait_for_initial_session_configured =
Self::should_wait_for_initial_session(&session_selection);
let mut chat_widget = match session_selection {
SessionSelection::StartFresh | SessionSelection::Exit => {
let init = crate::chatwidget::ChatWidgetInit {
@@ -1252,6 +1275,7 @@ impl App {
let mut thread_created_rx = thread_manager.subscribe_thread_created();
let mut listen_for_threads = true;
let mut waiting_for_initial_session_configured = wait_for_initial_session_configured;
let exit_reason = loop {
let control = select! {
@@ -1264,7 +1288,10 @@ impl App {
} else {
None
}
}, if app.active_thread_rx.is_some() => {
}, if App::should_handle_active_thread_events(
waiting_for_initial_session_configured,
app.active_thread_rx.is_some()
) => {
if let Some(event) = active {
app.handle_active_thread_event(tui, event).await?;
} else {
@@ -1291,6 +1318,12 @@ impl App {
AppRunControl::Continue
}
};
if App::should_stop_waiting_for_initial_session(
waiting_for_initial_session_configured,
app.primary_thread_id,
) {
waiting_for_initial_session_configured = false;
}
match control {
AppRunControl::Continue => {}
AppRunControl::Exit(reason) => break reason,
@@ -2915,6 +2948,76 @@ mod tests {
Ok(())
}
#[test]
fn startup_waiting_gate_is_only_for_fresh_or_exit_session_selection() {
assert_eq!(
App::should_wait_for_initial_session(&SessionSelection::StartFresh),
true
);
assert_eq!(
App::should_wait_for_initial_session(&SessionSelection::Exit),
true
);
assert_eq!(
App::should_wait_for_initial_session(&SessionSelection::Resume(PathBuf::from(
"/tmp/restore"
))),
false
);
assert_eq!(
App::should_wait_for_initial_session(&SessionSelection::Fork(PathBuf::from(
"/tmp/fork"
))),
false
);
}
#[test]
fn startup_waiting_gate_holds_active_thread_events_until_primary_thread_configured() {
let mut wait_for_initial_session =
App::should_wait_for_initial_session(&SessionSelection::StartFresh);
assert_eq!(wait_for_initial_session, true);
assert_eq!(
App::should_handle_active_thread_events(wait_for_initial_session, true),
false
);
assert_eq!(
App::should_stop_waiting_for_initial_session(wait_for_initial_session, None),
false
);
if App::should_stop_waiting_for_initial_session(
wait_for_initial_session,
Some(ThreadId::new()),
) {
wait_for_initial_session = false;
}
assert_eq!(wait_for_initial_session, false);
assert_eq!(
App::should_handle_active_thread_events(wait_for_initial_session, true),
true
);
}
#[test]
fn startup_waiting_gate_not_applied_for_resume_or_fork_session_selection() {
let wait_for_resume = App::should_wait_for_initial_session(&SessionSelection::Resume(
PathBuf::from("/tmp/restore"),
));
assert_eq!(
App::should_handle_active_thread_events(wait_for_resume, true),
true
);
let wait_for_fork = App::should_wait_for_initial_session(&SessionSelection::Fork(
PathBuf::from("/tmp/fork"),
));
assert_eq!(
App::should_handle_active_thread_events(wait_for_fork, true),
true
);
}
#[tokio::test]
async fn enqueue_thread_event_does_not_block_when_channel_full() -> Result<()> {
let mut app = make_test_app().await;
+1
View File
@@ -371,5 +371,6 @@ pub(crate) enum FeedbackCategory {
BadResult,
GoodResult,
Bug,
SafetyCheck,
Other,
}
+23 -2
View File
@@ -45,12 +45,15 @@ pub(crate) struct CommandPopupFlags {
impl CommandPopup {
pub(crate) fn new(mut prompts: Vec<CustomPrompt>, flags: CommandPopupFlags) -> Self {
// Keep built-in availability in sync with the composer.
let builtins = slash_commands::builtins_for_input(
let builtins: Vec<(&'static str, SlashCommand)> = slash_commands::builtins_for_input(
flags.collaboration_modes_enabled,
flags.connectors_enabled,
flags.personality_command_enabled,
flags.windows_degraded_sandbox_active,
);
)
.into_iter()
.filter(|(name, _)| !name.starts_with("debug"))
.collect();
// Exclude prompts that collide with builtin command names and sort by name.
let exclude: HashSet<String> = builtins.iter().map(|(n, _)| (*n).to_string()).collect();
prompts.retain(|p| !exclude.contains(&p.name));
@@ -566,4 +569,22 @@ mod tests {
other => panic!("expected personality to be selected for exact match, got {other:?}"),
}
}
#[test]
fn debug_commands_are_hidden_from_popup() {
let popup = CommandPopup::new(Vec::new(), CommandPopupFlags::default());
let cmds: Vec<&str> = popup
.filtered_items()
.into_iter()
.filter_map(|item| match item {
CommandItem::Builtin(cmd) => Some(cmd.command()),
CommandItem::UserPrompt(_) => None,
})
.collect();
assert!(
!cmds.iter().any(|name| name.starts_with("debug")),
"expected no /debug* command in popup menu, got {cmds:?}"
);
}
}
+35 -9
View File
@@ -353,6 +353,10 @@ fn feedback_title_and_placeholder(category: FeedbackCategory) -> (String, String
"Tell us more (bug)".to_string(),
"(optional) Write a short description to help us further".to_string(),
),
FeedbackCategory::SafetyCheck => (
"Tell us more (safety check)".to_string(),
"(optional) Share what was refused and why it should have been allowed".to_string(),
),
FeedbackCategory::Other => (
"Tell us more (other)".to_string(),
"(optional) Write a short description to help us further".to_string(),
@@ -365,6 +369,7 @@ fn feedback_classification(category: FeedbackCategory) -> &'static str {
FeedbackCategory::BadResult => "bad_result",
FeedbackCategory::GoodResult => "good_result",
FeedbackCategory::Bug => "bug",
FeedbackCategory::SafetyCheck => "safety_check",
FeedbackCategory::Other => "other",
}
}
@@ -378,14 +383,15 @@ fn issue_url_for_category(
// the external GitHub behavior identical while routing internal users to
// the internal go link.
match category {
FeedbackCategory::Bug | FeedbackCategory::BadResult | FeedbackCategory::Other => {
Some(match feedback_audience {
FeedbackAudience::OpenAiEmployee => slack_feedback_url(thread_id),
FeedbackAudience::External => {
format!("{BASE_BUG_ISSUE_URL}&steps=Uploaded%20thread:%20{thread_id}")
}
})
}
FeedbackCategory::Bug
| FeedbackCategory::BadResult
| FeedbackCategory::SafetyCheck
| FeedbackCategory::Other => Some(match feedback_audience {
FeedbackAudience::OpenAiEmployee => slack_feedback_url(thread_id),
FeedbackAudience::External => {
format!("{BASE_BUG_ISSUE_URL}&steps=Uploaded%20thread:%20{thread_id}")
}
}),
FeedbackCategory::GoodResult => None,
}
}
@@ -423,6 +429,12 @@ pub(crate) fn feedback_selection_params(
"Helpful, correct, high‑quality, or delightful result worth celebrating.",
FeedbackCategory::GoodResult,
),
make_feedback_item(
app_event_tx.clone(),
"safety check",
"Benign usage blocked due to safety checks or refusals.",
FeedbackCategory::SafetyCheck,
),
make_feedback_item(
app_event_tx,
"other",
@@ -616,7 +628,14 @@ mod tests {
}
#[test]
fn issue_url_available_for_bug_bad_result_and_other() {
fn feedback_view_safety_check() {
let view = make_view(FeedbackCategory::SafetyCheck);
let rendered = render(&view, 60);
insta::assert_snapshot!("feedback_view_safety_check", rendered);
}
#[test]
fn issue_url_available_for_bug_bad_result_safety_check_and_other() {
let bug_url = issue_url_for_category(
FeedbackCategory::Bug,
"thread-1",
@@ -639,6 +658,13 @@ mod tests {
);
assert!(other_url.is_some());
let safety_check_url = issue_url_for_category(
FeedbackCategory::SafetyCheck,
"thread-4",
FeedbackAudience::OpenAiEmployee,
);
assert!(safety_check_url.is_some());
assert!(
issue_url_for_category(
FeedbackCategory::GoodResult,
@@ -63,3 +63,15 @@ pub(crate) fn has_builtin_prefix(
.into_iter()
.any(|(command_name, _)| fuzzy_match(command_name, name).is_some())
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn debug_command_still_resolves_for_dispatch() {
let cmd = find_builtin_command("debug-config", true, true, true, false);
assert_eq!(cmd, Some(SlashCommand::DebugConfig));
}
}
@@ -0,0 +1,9 @@
---
source: tui/src/bottom_pane/feedback_view.rs
expression: rendered
---
▌ Tell us more (safety check)
▌
▌ (optional) Share what was refused and why it should have b
Press enter to confirm or esc to go back
+10 -8
View File
@@ -184,7 +184,6 @@ use crate::bottom_pane::SelectionViewParams;
use crate::bottom_pane::custom_prompt_view::CustomPromptView;
use crate::bottom_pane::popup_consts::standard_popup_hint_line;
use crate::clipboard_paste::paste_image_to_temp_png;
use crate::collab;
use crate::collaboration_modes;
use crate::diff_render::display_path_for;
use crate::exec_cell::CommandOutput;
@@ -201,6 +200,7 @@ use crate::history_cell::WebSearchCell;
use crate::key_hint;
use crate::key_hint::KeyBinding;
use crate::markdown::append_markdown;
use crate::multi_agents;
use crate::render::Insets;
use crate::render::renderable::ColumnRenderable;
use crate::render::renderable::FlexRenderable;
@@ -4113,17 +4113,19 @@ impl ChatWidget {
EventMsg::ExitedReviewMode(review) => self.on_exited_review_mode(review),
EventMsg::ContextCompacted(_) => self.on_agent_message("Context compacted".to_owned()),
EventMsg::CollabAgentSpawnBegin(_) => {}
EventMsg::CollabAgentSpawnEnd(ev) => self.on_collab_event(collab::spawn_end(ev)),
EventMsg::CollabAgentSpawnEnd(ev) => self.on_collab_event(multi_agents::spawn_end(ev)),
EventMsg::CollabAgentInteractionBegin(_) => {}
EventMsg::CollabAgentInteractionEnd(ev) => {
self.on_collab_event(collab::interaction_end(ev))
self.on_collab_event(multi_agents::interaction_end(ev))
}
EventMsg::CollabWaitingBegin(ev) => self.on_collab_event(collab::waiting_begin(ev)),
EventMsg::CollabWaitingEnd(ev) => self.on_collab_event(collab::waiting_end(ev)),
EventMsg::CollabWaitingBegin(ev) => {
self.on_collab_event(multi_agents::waiting_begin(ev))
}
EventMsg::CollabWaitingEnd(ev) => self.on_collab_event(multi_agents::waiting_end(ev)),
EventMsg::CollabCloseBegin(_) => {}
EventMsg::CollabCloseEnd(ev) => self.on_collab_event(collab::close_end(ev)),
EventMsg::CollabResumeBegin(ev) => self.on_collab_event(collab::resume_begin(ev)),
EventMsg::CollabResumeEnd(ev) => self.on_collab_event(collab::resume_end(ev)),
EventMsg::CollabCloseEnd(ev) => self.on_collab_event(multi_agents::close_end(ev)),
EventMsg::CollabResumeBegin(ev) => self.on_collab_event(multi_agents::resume_begin(ev)),
EventMsg::CollabResumeEnd(ev) => self.on_collab_event(multi_agents::resume_end(ev)),
EventMsg::ThreadRolledBack(rollback) => {
if from_replay {
self.app_event_tx.send(AppEvent::ApplyThreadRollback {
@@ -4,8 +4,10 @@ expression: popup
---
How was this?
› 1. bug Crash, error message, hang, or broken UI/behavior.
2. bad result Output was off-target, incorrect, incomplete, or unhelpful.
3. good result Helpful, correct, high‑quality, or delightful result worth
celebrating.
4. other Slowness, feature suggestion, UX feedback, or anything else.
› 1. bug Crash, error message, hang, or broken UI/behavior.
2. bad result Output was off-target, incorrect, incomplete, or unhelpful.
3. good result Helpful, correct, high‑quality, or delightful result worth
celebrating.
4. safety check Benign usage blocked due to safety checks or refusals.
5. other Slowness, feature suggestion, UX feedback, or anything
else.
+1 -1
View File
@@ -64,7 +64,6 @@ mod bottom_pane;
mod chatwidget;
mod cli;
mod clipboard_paste;
mod collab;
mod collaboration_modes;
mod color;
pub mod custom_terminal;
@@ -86,6 +85,7 @@ mod markdown_render;
mod markdown_stream;
mod mention_codec;
mod model_migration;
mod multi_agents;
mod notifications;
pub mod onboarding;
mod oss_selection;
+5 -3
View File
@@ -218,9 +218,11 @@ impl Renderable for StatusIndicatorWidget {
return;
}
// Schedule next animation frame.
self.frame_requester
.schedule_frame_in(Duration::from_millis(32));
if self.animations_enabled {
// Schedule next animation frame.
self.frame_requester
.schedule_frame_in(Duration::from_millis(32));
}
let now = Instant::now();
let elapsed_duration = self.elapsed_duration_at(now);
let pretty_elapsed = fmt_elapsed_compact(elapsed_duration.as_secs());