Compare commits

..

1 Commits

Author SHA1 Message Date
Michael Bolin
4a62376e6b fix: attempting to resume an existing conversation in ConversationManager should reuse it 2025-10-24 16:12:43 -07:00
44 changed files with 361 additions and 1464 deletions

1
codex-rs/Cargo.lock generated
View File

@@ -853,7 +853,6 @@ dependencies = [
"pretty_assertions",
"serde",
"serde_json",
"serial_test",
"tempfile",
"tokio",
"toml",

View File

@@ -717,8 +717,6 @@ pub struct SendUserMessageResponse {}
#[serde(rename_all = "camelCase")]
pub struct AddConversationListenerParams {
pub conversation_id: ConversationId,
#[serde(default)]
pub experimental_raw_events: bool,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]

View File

@@ -47,7 +47,6 @@ base64 = { workspace = true }
core_test_support = { workspace = true }
os_info = { workspace = true }
pretty_assertions = { workspace = true }
serial_test = { workspace = true }
tempfile = { workspace = true }
toml = { workspace = true }
wiremock = { workspace = true }

View File

@@ -1256,10 +1256,7 @@ impl CodexMessageProcessor {
request_id: RequestId,
params: AddConversationListenerParams,
) {
let AddConversationListenerParams {
conversation_id,
experimental_raw_events,
} = params;
let AddConversationListenerParams { conversation_id } = params;
let Ok(conversation) = self
.conversation_manager
.get_conversation(conversation_id)
@@ -1296,11 +1293,6 @@ impl CodexMessageProcessor {
}
};
if let EventMsg::RawResponseItem(_) = &event.msg
&& !experimental_raw_events {
continue;
}
// For now, we send a notification for every event,
// JSON-serializing the `Event` as-is, but these should
// be migrated to be variants of `ServerNotification`

View File

@@ -103,10 +103,7 @@ async fn test_codex_jsonrpc_conversation_flow() {
// 2) addConversationListener
let add_listener_id = mcp
.send_add_conversation_listener_request(AddConversationListenerParams {
conversation_id,
experimental_raw_events: false,
})
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
.await
.expect("send addConversationListener");
let add_listener_resp: JSONRPCResponse = timeout(
@@ -255,10 +252,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() {
// 2) addConversationListener
let add_listener_id = mcp
.send_add_conversation_listener_request(AddConversationListenerParams {
conversation_id,
experimental_raw_events: false,
})
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
.await
.expect("send addConversationListener");
let _: AddConversationSubscriptionResponse =
@@ -465,10 +459,7 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() {
.expect("deserialize newConversation response");
let add_listener_id = mcp
.send_add_conversation_listener_request(AddConversationListenerParams {
conversation_id,
experimental_raw_events: false,
})
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
.await
.expect("send addConversationListener");
timeout(

View File

@@ -67,10 +67,7 @@ async fn test_conversation_create_and_send_message_ok() {
// Add a listener so we receive notifications for this conversation (not strictly required for this test).
let add_listener_id = mcp
.send_add_conversation_listener_request(AddConversationListenerParams {
conversation_id,
experimental_raw_events: false,
})
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
.await
.expect("send addConversationListener");
let _sub: AddConversationSubscriptionResponse =

View File

@@ -88,10 +88,7 @@ async fn shell_command_interruption() -> anyhow::Result<()> {
// 2) addConversationListener
let add_listener_id = mcp
.send_add_conversation_listener_request(AddConversationListenerParams {
conversation_id,
experimental_raw_events: false,
})
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
.await?;
let _add_listener_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,

View File

@@ -13,7 +13,6 @@ use codex_app_server_protocol::LoginChatGptResponse;
use codex_app_server_protocol::LogoutChatGptResponse;
use codex_app_server_protocol::RequestId;
use codex_login::login_with_api_key;
use serial_test::serial;
use tempfile::TempDir;
use tokio::time::timeout;
@@ -95,8 +94,6 @@ async fn logout_chatgpt_removes_auth() {
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
// Serialize tests that launch the login server since it binds to a fixed port.
#[serial(login_port)]
async fn login_and_cancel_chatgpt() {
let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}"));
create_config_toml(codex_home.path()).unwrap_or_else(|err| panic!("write config.toml: {err}"));
@@ -211,8 +208,6 @@ async fn login_chatgpt_rejected_when_forced_api() {
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
// Serialize tests that launch the login server since it binds to a fixed port.
#[serial(login_port)]
async fn login_chatgpt_includes_forced_workspace_query_param() {
let codex_home = TempDir::new().unwrap_or_else(|e| panic!("create tempdir: {e}"));
create_config_toml_forced_workspace(codex_home.path(), "ws-forced")

View File

@@ -15,8 +15,6 @@ use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SendUserMessageParams;
use codex_app_server_protocol::SendUserMessageResponse;
use codex_protocol::ConversationId;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
use tokio::time::timeout;
@@ -64,10 +62,7 @@ async fn test_send_message_success() {
// 2) addConversationListener
let add_listener_id = mcp
.send_add_conversation_listener_request(AddConversationListenerParams {
conversation_id,
experimental_raw_events: false,
})
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
.await
.expect("send addConversationListener");
let add_listener_resp: JSONRPCResponse = timeout(
@@ -129,105 +124,6 @@ async fn send_message(message: &str, conversation_id: ConversationId, mcp: &mut
.expect("should have conversationId"),
&serde_json::Value::String(conversation_id.to_string())
);
let raw_attempt = tokio::time::timeout(
std::time::Duration::from_millis(200),
mcp.read_stream_until_notification_message("codex/event/raw_response_item"),
)
.await;
assert!(
raw_attempt.is_err(),
"unexpected raw item notification when not opted in"
);
}
#[tokio::test]
async fn test_send_message_raw_notifications_opt_in() {
let responses = vec![
create_final_assistant_message_sse_response("Done").expect("build mock assistant message"),
];
let server = create_mock_chat_completions_server(responses).await;
let codex_home = TempDir::new().expect("create temp dir");
create_config_toml(codex_home.path(), &server.uri()).expect("write config.toml");
let mut mcp = McpProcess::new(codex_home.path())
.await
.expect("spawn mcp process");
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize())
.await
.expect("init timed out")
.expect("init failed");
let new_conv_id = mcp
.send_new_conversation_request(NewConversationParams::default())
.await
.expect("send newConversation");
let new_conv_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(new_conv_id)),
)
.await
.expect("newConversation timeout")
.expect("newConversation resp");
let NewConversationResponse {
conversation_id, ..
} = to_response::<_>(new_conv_resp).expect("deserialize newConversation response");
let add_listener_id = mcp
.send_add_conversation_listener_request(AddConversationListenerParams {
conversation_id,
experimental_raw_events: true,
})
.await
.expect("send addConversationListener");
let add_listener_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(add_listener_id)),
)
.await
.expect("addConversationListener timeout")
.expect("addConversationListener resp");
let AddConversationSubscriptionResponse { subscription_id: _ } =
to_response::<_>(add_listener_resp).expect("deserialize addConversationListener response");
let send_id = mcp
.send_send_user_message_request(SendUserMessageParams {
conversation_id,
items: vec![InputItem::Text {
text: "Hello".to_string(),
}],
})
.await
.expect("send sendUserMessage");
let instructions = read_raw_response_item(&mut mcp, conversation_id).await;
assert_instructions_message(&instructions);
let environment = read_raw_response_item(&mut mcp, conversation_id).await;
assert_environment_message(&environment);
let response: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(send_id)),
)
.await
.expect("sendUserMessage response timeout")
.expect("sendUserMessage response error");
let _ok: SendUserMessageResponse = to_response::<SendUserMessageResponse>(response)
.expect("deserialize sendUserMessage response");
let user_message = read_raw_response_item(&mut mcp, conversation_id).await;
assert_user_message(&user_message, "Hello");
let assistant_message = read_raw_response_item(&mut mcp, conversation_id).await;
assert_assistant_message(&assistant_message, "Done");
let _ = tokio::time::timeout(
std::time::Duration::from_millis(250),
mcp.read_stream_until_notification_message("codex/event/task_complete"),
)
.await;
}
#[tokio::test]
@@ -288,108 +184,3 @@ stream_max_retries = 0
),
)
}
#[expect(clippy::expect_used)]
async fn read_raw_response_item(
mcp: &mut McpProcess,
conversation_id: ConversationId,
) -> ResponseItem {
let raw_notification: JSONRPCNotification = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("codex/event/raw_response_item"),
)
.await
.expect("codex/event/raw_response_item notification timeout")
.expect("codex/event/raw_response_item notification resp");
let serde_json::Value::Object(params) = raw_notification
.params
.expect("codex/event/raw_response_item should have params")
else {
panic!("codex/event/raw_response_item should have params");
};
let conversation_id_value = params
.get("conversationId")
.and_then(|value| value.as_str())
.expect("raw response item should include conversationId");
assert_eq!(
conversation_id_value,
conversation_id.to_string(),
"raw response item conversation mismatch"
);
let msg_value = params
.get("msg")
.cloned()
.expect("raw response item should include msg payload");
serde_json::from_value(msg_value).expect("deserialize raw response item")
}
fn assert_instructions_message(item: &ResponseItem) {
match item {
ResponseItem::Message { role, content, .. } => {
assert_eq!(role, "user");
let texts = content_texts(content);
assert!(
texts
.iter()
.any(|text| text.contains("<user_instructions>")),
"expected instructions message, got {texts:?}"
);
}
other => panic!("expected instructions message, got {other:?}"),
}
}
fn assert_environment_message(item: &ResponseItem) {
match item {
ResponseItem::Message { role, content, .. } => {
assert_eq!(role, "user");
let texts = content_texts(content);
assert!(
texts
.iter()
.any(|text| text.contains("<environment_context>")),
"expected environment context message, got {texts:?}"
);
}
other => panic!("expected environment message, got {other:?}"),
}
}
fn assert_user_message(item: &ResponseItem, expected_text: &str) {
match item {
ResponseItem::Message { role, content, .. } => {
assert_eq!(role, "user");
let texts = content_texts(content);
assert_eq!(texts, vec![expected_text]);
}
other => panic!("expected user message, got {other:?}"),
}
}
fn assert_assistant_message(item: &ResponseItem, expected_text: &str) {
match item {
ResponseItem::Message { role, content, .. } => {
assert_eq!(role, "assistant");
let texts = content_texts(content);
assert_eq!(texts, vec![expected_text]);
}
other => panic!("expected assistant message, got {other:?}"),
}
}
fn content_texts(content: &[ContentItem]) -> Vec<&str> {
content
.iter()
.filter_map(|item| match item {
ContentItem::InputText { text } | ContentItem::OutputText { text } => {
Some(text.as_str())
}
_ => None,
})
.collect()
}

View File

@@ -274,33 +274,19 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
http_headers,
env_http_headers,
} = transport
&& matches!(supports_oauth_login(&url).await, Ok(true))
{
match supports_oauth_login(&url).await {
Ok(true) => {
if !config.features.enabled(Feature::RmcpClient) {
println!(
"MCP server supports login. Add `experimental_use_rmcp_client = true` \
to your config.toml and run `codex mcp login {name}` to login."
);
} else {
println!("Detected OAuth support. Starting OAuth flow…");
perform_oauth_login(
&name,
&url,
config.mcp_oauth_credentials_store_mode,
http_headers.clone(),
env_http_headers.clone(),
&Vec::new(),
)
.await?;
println!("Successfully logged in.");
}
}
Ok(false) => {}
Err(_) => println!(
"MCP server may or may not require login. Run `codex mcp login {name}` to login."
),
}
println!("Detected OAuth support. Starting OAuth flow…");
perform_oauth_login(
&name,
&url,
config.mcp_oauth_credentials_store_mode,
http_headers.clone(),
env_http_headers.clone(),
&Vec::new(),
)
.await?;
println!("Successfully logged in.");
}
Ok(())
@@ -537,12 +523,10 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) ->
.map(|entry| entry.auth_status)
.unwrap_or(McpAuthStatus::Unsupported)
.to_string();
let bearer_token_display =
bearer_token_env_var.as_deref().unwrap_or("-").to_string();
http_rows.push([
name.clone(),
url.clone(),
bearer_token_display,
bearer_token_env_var.clone().unwrap_or("-".to_string()),
status,
auth_status,
]);
@@ -768,15 +752,15 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
} => {
println!(" transport: streamable_http");
println!(" url: {url}");
let bearer_token_display = bearer_token_env_var.as_deref().unwrap_or("-");
println!(" bearer_token_env_var: {bearer_token_display}");
let env_var = bearer_token_env_var.as_deref().unwrap_or("-");
println!(" bearer_token_env_var: {env_var}");
let headers_display = match http_headers {
Some(map) if !map.is_empty() => {
let mut pairs: Vec<_> = map.iter().collect();
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
pairs
.into_iter()
.map(|(k, _)| format!("{k}=*****"))
.map(|(k, v)| format!("{k}={v}"))
.collect::<Vec<_>>()
.join(", ")
}
@@ -789,7 +773,7 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
pairs
.into_iter()
.map(|(k, var)| format!("{k}={var}"))
.map(|(k, v)| format!("{k}={v}"))
.collect::<Vec<_>>()
.join(", ")
}

View File

@@ -68,9 +68,9 @@ async fn list_and_get_render_expected_output() -> Result<()> {
assert!(stdout.contains("Name"));
assert!(stdout.contains("docs"));
assert!(stdout.contains("docs-server"));
assert!(stdout.contains("TOKEN=*****"));
assert!(stdout.contains("APP_TOKEN=*****"));
assert!(stdout.contains("WORKSPACE_ID=*****"));
assert!(stdout.contains("TOKEN=secret"));
assert!(stdout.contains("APP_TOKEN=$APP_TOKEN"));
assert!(stdout.contains("WORKSPACE_ID=$WORKSPACE_ID"));
assert!(stdout.contains("Status"));
assert!(stdout.contains("Auth"));
assert!(stdout.contains("enabled"));
@@ -119,9 +119,9 @@ async fn list_and_get_render_expected_output() -> Result<()> {
assert!(stdout.contains("transport: stdio"));
assert!(stdout.contains("command: docs-server"));
assert!(stdout.contains("args: --port 4000"));
assert!(stdout.contains("env: TOKEN=*****"));
assert!(stdout.contains("APP_TOKEN=*****"));
assert!(stdout.contains("WORKSPACE_ID=*****"));
assert!(stdout.contains("env: TOKEN=secret"));
assert!(stdout.contains("APP_TOKEN=$APP_TOKEN"));
assert!(stdout.contains("WORKSPACE_ID=$WORKSPACE_ID"));
assert!(stdout.contains("enabled: true"));
assert!(stdout.contains("remove: codex mcp remove docs"));

View File

@@ -6,11 +6,15 @@ pub fn format_env_display(env: Option<&HashMap<String, String>>, env_vars: &[Str
if let Some(map) = env {
let mut pairs: Vec<_> = map.iter().collect();
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
parts.extend(pairs.into_iter().map(|(key, _)| format!("{key}=*****")));
parts.extend(
pairs
.into_iter()
.map(|(key, value)| format!("{key}={value}")),
);
}
if !env_vars.is_empty() {
parts.extend(env_vars.iter().map(|var| format!("{var}=*****")));
parts.extend(env_vars.iter().map(|var| format!("{var}=${var}")));
}
if parts.is_empty() {
@@ -38,14 +42,14 @@ mod tests {
env.insert("B".to_string(), "two".to_string());
env.insert("A".to_string(), "one".to_string());
assert_eq!(format_env_display(Some(&env), &[]), "A=*****, B=*****");
assert_eq!(format_env_display(Some(&env), &[]), "A=one, B=two");
}
#[test]
fn formats_env_vars_with_dollar_prefix() {
let vars = vec!["TOKEN".to_string(), "PATH".to_string()];
assert_eq!(format_env_display(None, &vars), "TOKEN=*****, PATH=*****");
assert_eq!(format_env_display(None, &vars), "TOKEN=$TOKEN, PATH=$PATH");
}
#[test]
@@ -56,7 +60,7 @@ mod tests {
assert_eq!(
format_env_display(Some(&env), &vars),
"HOME=*****, TOKEN=*****"
"HOME=/tmp, TOKEN=$TOKEN"
);
}
}

View File

@@ -570,6 +570,7 @@ impl Session {
// Dispatch the SessionConfiguredEvent first and then report any errors.
// If resuming, include converted initial messages in the payload so UIs can render them immediately.
let initial_messages = initial_history.get_event_msgs();
sess.record_initial_history(initial_history).await;
let events = std::iter::once(Event {
id: INITIAL_SUBMIT_ID.to_owned(),
@@ -588,9 +589,6 @@ impl Session {
sess.send_event_raw(event).await;
}
// record_initial_history can emit events. We record only after the SessionConfiguredEvent is emitted.
sess.record_initial_history(initial_history).await;
Ok(sess)
}
@@ -611,7 +609,7 @@ impl Session {
InitialHistory::New => {
// Build and record initial items (user instructions + environment context)
let items = self.build_initial_context(&turn_context);
self.record_conversation_items(&turn_context, &items).await;
self.record_conversation_items(&items).await;
}
InitialHistory::Resumed(_) | InitialHistory::Forked(_) => {
let rollout_items = conversation_history.get_rollout_items();
@@ -888,14 +886,9 @@ impl Session {
/// Records input items: always append to conversation history and
/// persist these response items to rollout.
pub(crate) async fn record_conversation_items(
&self,
turn_context: &TurnContext,
items: &[ResponseItem],
) {
pub(crate) async fn record_conversation_items(&self, items: &[ResponseItem]) {
self.record_into_history(items).await;
self.persist_rollout_response_items(items).await;
self.send_raw_response_items(turn_context, items).await;
}
fn reconstruct_history_from_rollout(
@@ -945,13 +938,6 @@ impl Session {
self.persist_rollout_items(&rollout_items).await;
}
async fn send_raw_response_items(&self, turn_context: &TurnContext, items: &[ResponseItem]) {
for item in items {
self.send_event(turn_context, EventMsg::RawResponseItem(item.clone()))
.await;
}
}
pub(crate) fn build_initial_context(&self, turn_context: &TurnContext) -> Vec<ResponseItem> {
let mut items = Vec::<ResponseItem>::with_capacity(2);
if let Some(user_instructions) = turn_context.user_instructions.as_deref() {
@@ -1047,7 +1033,7 @@ impl Session {
) {
let response_item: ResponseItem = response_input.clone().into();
// Add to conversation history and persist response item to rollout
self.record_conversation_items(turn_context, std::slice::from_ref(&response_item))
self.record_conversation_items(std::slice::from_ref(&response_item))
.await;
// Derive user message events and persist only UserMessage to rollout
@@ -1238,11 +1224,8 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
if let Some(env_item) = sess
.build_environment_update_item(previous_context.as_ref(), &current_context)
{
sess.record_conversation_items(
&current_context,
std::slice::from_ref(&env_item),
)
.await;
sess.record_conversation_items(std::slice::from_ref(&env_item))
.await;
}
sess.spawn_task(Arc::clone(&current_context), items, RegularTask)
@@ -1614,8 +1597,7 @@ pub(crate) async fn run_task(
}
review_thread_history.get_history()
} else {
sess.record_conversation_items(&turn_context, &pending_input)
.await;
sess.record_conversation_items(&pending_input).await;
sess.history_snapshot().await
};
@@ -1662,7 +1644,6 @@ pub(crate) async fn run_task(
is_review_mode,
&mut review_thread_history,
&sess,
&turn_context,
)
.await;
@@ -1711,7 +1692,6 @@ pub(crate) async fn run_task(
is_review_mode,
&mut review_thread_history,
&sess,
&turn_context,
)
.await;
// Aborted turn is reported via a different event.
@@ -2222,14 +2202,11 @@ pub(crate) async fn exit_review_mode(
}
session
.record_conversation_items(
&turn_context,
&[ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText { text: user_message }],
}],
)
.record_conversation_items(&[ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText { text: user_message }],
}])
.await;
}
@@ -2824,19 +2801,13 @@ mod tests {
EventMsg::ExitedReviewMode(ev) => assert!(ev.review_output.is_none()),
other => panic!("unexpected first event: {other:?}"),
}
loop {
let evt = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for next event")
.expect("event");
match evt.msg {
EventMsg::RawResponseItem(_) => continue,
EventMsg::TurnAborted(e) => {
assert_eq!(TurnAbortReason::Interrupted, e.reason);
break;
}
other => panic!("unexpected second event: {other:?}"),
}
let second = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for second event")
.expect("second event");
match second.msg {
EventMsg::TurnAborted(e) => assert_eq!(TurnAbortReason::Interrupted, e.reason),
other => panic!("unexpected second event: {other:?}"),
}
let history = sess.history_snapshot().await;

View File

@@ -200,20 +200,7 @@ pub(crate) fn build_compacted_history(
user_messages: &[String],
summary_text: &str,
) -> Vec<ResponseItem> {
build_compacted_history_with_limit(
initial_context,
user_messages,
summary_text,
COMPACT_USER_MESSAGE_MAX_TOKENS * 4,
)
}
fn build_compacted_history_with_limit(
mut history: Vec<ResponseItem>,
user_messages: &[String],
summary_text: &str,
max_bytes: usize,
) -> Vec<ResponseItem> {
let mut history = initial_context;
let mut user_messages_text = if user_messages.is_empty() {
"(none)".to_string()
} else {
@@ -221,6 +208,7 @@ fn build_compacted_history_with_limit(
};
// Truncate the concatenated prior user messages so the bridge message
// stays well under the context window (approx. 4 bytes/token).
let max_bytes = COMPACT_USER_MESSAGE_MAX_TOKENS * 4;
if user_messages_text.len() > max_bytes {
user_messages_text = truncate_middle(&user_messages_text, max_bytes).0;
}
@@ -373,16 +361,11 @@ mod tests {
#[test]
fn build_compacted_history_truncates_overlong_user_messages() {
// Use a small truncation limit so the test remains fast while still validating
// that oversized user content is truncated.
let max_bytes = 128;
let big = "X".repeat(max_bytes + 50);
let history = super::build_compacted_history_with_limit(
Vec::new(),
std::slice::from_ref(&big),
"SUMMARY",
max_bytes,
);
// Prepare a very large prior user message so the aggregated
// `user_messages_text` exceeds the truncation threshold used by
// `build_compacted_history` (80k bytes).
let big = "X".repeat(200_000);
let history = build_compacted_history(Vec::new(), std::slice::from_ref(&big), "SUMMARY");
// Expect exactly one bridge message added to history (plus any initial context we provided, which is none).
assert_eq!(history.len(), 1);

View File

@@ -1027,11 +1027,9 @@ impl ConfigToml {
fn derive_sandbox_policy(
&self,
sandbox_mode_override: Option<SandboxMode>,
profile_sandbox_mode: Option<SandboxMode>,
resolved_cwd: &Path,
) -> SandboxPolicy {
let resolved_sandbox_mode = sandbox_mode_override
.or(profile_sandbox_mode)
.or(self.sandbox_mode)
.or_else(|| {
// if no sandbox_mode is set, but user has marked directory as trusted, use WorkspaceWrite
@@ -1221,8 +1219,7 @@ impl Config {
.get_active_project(&resolved_cwd)
.unwrap_or(ProjectConfig { trust_level: None });
let mut sandbox_policy =
cfg.derive_sandbox_policy(sandbox_mode, config_profile.sandbox_mode, &resolved_cwd);
let mut sandbox_policy = cfg.derive_sandbox_policy(sandbox_mode, &resolved_cwd);
if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy {
for path in additional_writable_roots {
if !writable_roots.iter().any(|existing| existing == &path) {
@@ -1245,8 +1242,8 @@ impl Config {
.is_some()
|| config_profile.approval_policy.is_some()
|| cfg.approval_policy.is_some()
// TODO(#3034): profile.sandbox_mode is not implemented
|| sandbox_mode.is_some()
|| config_profile.sandbox_mode.is_some()
|| cfg.sandbox_mode.is_some();
let mut model_providers = built_in_model_providers();
@@ -1606,11 +1603,8 @@ network_access = false # This should be ignored.
let sandbox_mode_override = None;
assert_eq!(
SandboxPolicy::DangerFullAccess,
sandbox_full_access_cfg.derive_sandbox_policy(
sandbox_mode_override,
None,
&PathBuf::from("/tmp/test")
)
sandbox_full_access_cfg
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
);
let sandbox_read_only = r#"
@@ -1625,11 +1619,8 @@ network_access = true # This should be ignored.
let sandbox_mode_override = None;
assert_eq!(
SandboxPolicy::ReadOnly,
sandbox_read_only_cfg.derive_sandbox_policy(
sandbox_mode_override,
None,
&PathBuf::from("/tmp/test")
)
sandbox_read_only_cfg
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
);
let sandbox_workspace_write = r#"
@@ -1653,11 +1644,8 @@ exclude_slash_tmp = true
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
sandbox_workspace_write_cfg.derive_sandbox_policy(
sandbox_mode_override,
None,
&PathBuf::from("/tmp/test")
)
sandbox_workspace_write_cfg
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
);
let sandbox_workspace_write = r#"
@@ -1684,11 +1672,8 @@ trust_level = "trusted"
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
sandbox_workspace_write_cfg.derive_sandbox_policy(
sandbox_mode_override,
None,
&PathBuf::from("/tmp/test")
)
sandbox_workspace_write_cfg
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
);
}
@@ -1780,75 +1765,6 @@ trust_level = "trusted"
Ok(())
}
#[test]
fn profile_sandbox_mode_overrides_base() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let mut profiles = HashMap::new();
profiles.insert(
"work".to_string(),
ConfigProfile {
sandbox_mode: Some(SandboxMode::DangerFullAccess),
..Default::default()
},
);
let cfg = ConfigToml {
profiles,
profile: Some("work".to_string()),
sandbox_mode: Some(SandboxMode::ReadOnly),
..Default::default()
};
let config = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
)?;
assert!(matches!(
config.sandbox_policy,
SandboxPolicy::DangerFullAccess
));
assert!(config.did_user_set_custom_approval_policy_or_sandbox_mode);
Ok(())
}
#[test]
fn cli_override_takes_precedence_over_profile_sandbox_mode() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let mut profiles = HashMap::new();
profiles.insert(
"work".to_string(),
ConfigProfile {
sandbox_mode: Some(SandboxMode::DangerFullAccess),
..Default::default()
},
);
let cfg = ConfigToml {
profiles,
profile: Some("work".to_string()),
..Default::default()
};
let overrides = ConfigOverrides {
sandbox_mode: Some(SandboxMode::WorkspaceWrite),
..Default::default()
};
let config = Config::load_from_base_config_with_overrides(
cfg,
overrides,
codex_home.path().to_path_buf(),
)?;
assert!(matches!(
config.sandbox_policy,
SandboxPolicy::WorkspaceWrite { .. }
));
Ok(())
}
#[test]
fn feature_table_overrides_legacy_flags() -> std::io::Result<()> {
let codex_home = TempDir::new()?;

View File

@@ -4,7 +4,6 @@ use std::path::PathBuf;
use crate::protocol::AskForApproval;
use codex_protocol::config_types::ReasoningEffort;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::config_types::Verbosity;
/// Collection of common configuration options that a user can define as a unit
@@ -16,7 +15,6 @@ pub struct ConfigProfile {
/// [`ModelProviderInfo`] to use.
pub model_provider: Option<String>,
pub approval_policy: Option<AskForApproval>,
pub sandbox_mode: Option<SandboxMode>,
pub model_reasoning_effort: Option<ReasoningEffort>,
pub model_reasoning_summary: Option<ReasoningSummary>,
pub model_verbosity: Option<Verbosity>,

View File

@@ -32,8 +32,34 @@ pub struct NewConversation {
/// [`ConversationManager`] is responsible for creating conversations and
/// maintaining them in memory.
#[derive(Clone)]
struct ConversationEntry {
conversation: Arc<CodexConversation>,
session_configured: SessionConfiguredEvent,
}
impl ConversationEntry {
fn new(
conversation: Arc<CodexConversation>,
session_configured: SessionConfiguredEvent,
) -> Self {
Self {
conversation,
session_configured,
}
}
fn to_new_conversation(&self, conversation_id: ConversationId) -> NewConversation {
NewConversation {
conversation_id,
conversation: self.conversation.clone(),
session_configured: self.session_configured.clone(),
}
}
}
pub struct ConversationManager {
conversations: Arc<RwLock<HashMap<ConversationId, Arc<CodexConversation>>>>,
conversations: Arc<RwLock<HashMap<ConversationId, ConversationEntry>>>,
auth_manager: Arc<AuthManager>,
session_source: SessionSource,
}
@@ -99,10 +125,11 @@ impl ConversationManager {
};
let conversation = Arc::new(CodexConversation::new(codex));
let entry = ConversationEntry::new(conversation.clone(), session_configured.clone());
self.conversations
.write()
.await
.insert(conversation_id, conversation.clone());
.insert(conversation_id, entry);
Ok(NewConversation {
conversation_id,
@@ -118,7 +145,7 @@ impl ConversationManager {
let conversations = self.conversations.read().await;
conversations
.get(&conversation_id)
.cloned()
.map(|entry| entry.conversation.clone())
.ok_or_else(|| CodexErr::ConversationNotFound(conversation_id))
}
@@ -129,11 +156,22 @@ impl ConversationManager {
auth_manager: Arc<AuthManager>,
) -> CodexResult<NewConversation> {
let initial_history = RolloutRecorder::get_rollout_history(&rollout_path).await?;
let CodexSpawnOk {
codex,
conversation_id,
} = Codex::spawn(config, auth_manager, initial_history, self.session_source).await?;
self.finalize_spawn(codex, conversation_id).await
if let InitialHistory::Resumed(resumed) = &initial_history
&& let Some(existing) = self
.conversations
.read()
.await
.get(&resumed.conversation_id)
.cloned()
{
Ok(existing.to_new_conversation(resumed.conversation_id))
} else {
let CodexSpawnOk {
codex,
conversation_id,
} = Codex::spawn(config, auth_manager, initial_history, self.session_source).await?;
self.finalize_spawn(codex, conversation_id).await
}
}
/// Removes the conversation from the manager's internal map, though the
@@ -144,7 +182,11 @@ impl ConversationManager {
&self,
conversation_id: &ConversationId,
) -> Option<Arc<CodexConversation>> {
self.conversations.write().await.remove(conversation_id)
self.conversations
.write()
.await
.remove(conversation_id)
.map(|entry| entry.conversation)
}
/// Fork an existing conversation by taking messages up to the given position

View File

@@ -55,7 +55,7 @@ pub enum SandboxErr {
#[derive(Error, Debug)]
pub enum CodexErr {
// todo(aibrahim): git rid of this error carrying the dangling artifacts
#[error("turn aborted. Something went wrong? Hit `/feedback` to report the issue.")]
#[error("turn aborted")]
TurnAborted {
dangling_artifacts: Vec<ProcessedResponseItem>,
},
@@ -91,7 +91,7 @@ pub enum CodexErr {
/// Returned by run_command_stream when the user pressed CtrlC (SIGINT). Session uses this to
/// surface a polite FunctionCallOutput back to the model instead of crashing the CLI.
#[error("interrupted (Ctrl-C). Something went wrong? Hit `/feedback` to report the issue.")]
#[error("interrupted (Ctrl-C)")]
Interrupted,
/// Unexpected HTTP status code.

View File

@@ -1,5 +1,4 @@
use crate::codex::Session;
use crate::codex::TurnContext;
use crate::conversation_history::ConversationHistory;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ResponseInputItem;
@@ -14,7 +13,6 @@ pub(crate) async fn process_items(
is_review_mode: bool,
review_thread_history: &mut ConversationHistory,
sess: &Session,
turn_context: &TurnContext,
) -> (Vec<ResponseInputItem>, Vec<ResponseItem>) {
let mut items_to_record_in_conversation_history = Vec::<ResponseItem>::new();
let mut responses = Vec::<ResponseInputItem>::new();
@@ -106,7 +104,7 @@ pub(crate) async fn process_items(
if is_review_mode {
review_thread_history.record_items(items_to_record_in_conversation_history.iter());
} else {
sess.record_conversation_items(turn_context, &items_to_record_in_conversation_history)
sess.record_conversation_items(&items_to_record_in_conversation_history)
.await;
}
}

View File

@@ -50,7 +50,6 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool {
| EventMsg::AgentReasoningDelta(_)
| EventMsg::AgentReasoningRawContentDelta(_)
| EventMsg::AgentReasoningSectionBreak(_)
| EventMsg::RawResponseItem(_)
| EventMsg::SessionConfigured(_)
| EventMsg::McpToolCallBegin(_)
| EventMsg::McpToolCallEnd(_)

View File

@@ -247,7 +247,11 @@ async fn resume_includes_initial_messages_and_sends_prior_items() {
session_configured,
..
} = conversation_manager
.resume_conversation_from_rollout(config, session_path.clone(), auth_manager)
.resume_conversation_from_rollout(
config.clone(),
session_path.clone(),
auth_manager.clone(),
)
.await
.expect("resume conversation");
@@ -260,6 +264,23 @@ async fn resume_includes_initial_messages_and_sends_prior_items() {
let expected_initial_json = json!([]);
assert_eq!(initial_json, expected_initial_json);
let NewConversation {
conversation: codex_again,
session_configured: session_configured_again,
..
} = conversation_manager
.resume_conversation_from_rollout(
config.clone(),
session_path.clone(),
auth_manager.clone(),
)
.await
.expect("resume existing conversation");
assert!(Arc::ptr_eq(&codex, &codex_again));
let session_configured_json = serde_json::to_value(&session_configured).unwrap();
let session_configured_again_json = serde_json::to_value(&session_configured_again).unwrap();
assert_eq!(session_configured_json, session_configured_again_json);
// 2) Submit new input; the request body must include the prior item followed by the new user input.
codex
.submit(Op::UserInput {

View File

@@ -519,7 +519,6 @@ impl EventProcessor for EventProcessorWithHumanOutput {
EventMsg::AgentReasoningRawContentDelta(_) => {}
EventMsg::ItemStarted(_) => {}
EventMsg::ItemCompleted(_) => {}
EventMsg::RawResponseItem(_) => {}
}
CodexStatus::Running
}

View File

@@ -167,17 +167,8 @@ impl CodexLogSnapshot {
Ok(path)
}
/// Upload feedback to Sentry with optional attachments.
pub fn upload_feedback(
&self,
classification: &str,
reason: Option<&str>,
cli_version: &str,
include_logs: bool,
rollout_path: Option<&std::path::Path>,
) -> Result<()> {
pub fn upload_to_sentry(&self) -> Result<()> {
use std::collections::BTreeMap;
use std::fs;
use std::str::FromStr;
use std::sync::Arc;
@@ -191,87 +182,33 @@ impl CodexLogSnapshot {
use sentry::transports::DefaultTransportFactory;
use sentry::types::Dsn;
// Build Sentry client
let client = Client::from_config(ClientOptions {
dsn: Some(Dsn::from_str(SENTRY_DSN).map_err(|e| anyhow!("invalid DSN: {e}"))?),
dsn: Some(Dsn::from_str(SENTRY_DSN).map_err(|e| anyhow!("invalid DSN: {}", e))?),
transport: Some(Arc::new(DefaultTransportFactory {})),
..Default::default()
});
let mut tags = BTreeMap::from([
(String::from("thread_id"), self.thread_id.to_string()),
(String::from("classification"), classification.to_string()),
(String::from("cli_version"), cli_version.to_string()),
]);
if let Some(r) = reason {
tags.insert(String::from("reason"), r.to_string());
}
let tags = BTreeMap::from([(String::from("thread_id"), self.thread_id.to_string())]);
let level = match classification {
"bug" | "bad_result" => Level::Error,
_ => Level::Info,
};
let mut envelope = Envelope::new();
let title = format!(
"[{}]: Codex session {}",
display_classification(classification),
self.thread_id
);
let mut event = Event {
level,
message: Some(title.clone()),
let event = Event {
level: Level::Error,
message: Some("Codex Log Upload ".to_string() + &self.thread_id),
tags,
..Default::default()
};
if let Some(r) = reason {
use sentry::protocol::Exception;
use sentry::protocol::Values;
event.exception = Values::from(vec![Exception {
ty: title.clone(),
value: Some(r.to_string()),
..Default::default()
}]);
}
let mut envelope = Envelope::new();
envelope.add_item(EnvelopeItem::Event(event));
if include_logs {
envelope.add_item(EnvelopeItem::Attachment(Attachment {
buffer: self.bytes.clone(),
filename: String::from("codex-logs.log"),
content_type: Some("text/plain".to_string()),
ty: None,
}));
}
if let Some((path, data)) = rollout_path.and_then(|p| fs::read(p).ok().map(|d| (p, d))) {
let fname = path
.file_name()
.map(|s| s.to_string_lossy().to_string())
.unwrap_or_else(|| "rollout.jsonl".to_string());
let content_type = "text/plain".to_string();
envelope.add_item(EnvelopeItem::Attachment(Attachment {
buffer: data,
filename: fname,
content_type: Some(content_type),
ty: None,
}));
}
envelope.add_item(EnvelopeItem::Attachment(Attachment {
buffer: self.bytes.clone(),
filename: String::from("codex-logs.log"),
content_type: Some("text/plain".to_string()),
ty: None,
}));
client.send_envelope(envelope);
client.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS)));
Ok(())
}
}
fn display_classification(classification: &str) -> String {
match classification {
"bug" => "Bug".to_string(),
"bad_result" => "Bad result".to_string(),
"good_result" => "Good result".to_string(),
_ => "Other".to_string(),
Ok(())
}
}

View File

@@ -285,7 +285,6 @@ async fn run_codex_tool_session_inner(
| EventMsg::UserMessage(_)
| EventMsg::ShutdownComplete
| EventMsg::ViewImageToolCall(_)
| EventMsg::RawResponseItem(_)
| EventMsg::EnteredReviewMode(_)
| EventMsg::ItemStarted(_)
| EventMsg::ItemCompleted(_)

View File

@@ -527,8 +527,6 @@ pub enum EventMsg {
/// Exited review mode with an optional final result to apply.
ExitedReviewMode(ExitedReviewModeEvent),
RawResponseItem(ResponseItem),
ItemStarted(ItemStartedEvent),
ItemCompleted(ItemCompletedEvent),
}

View File

@@ -360,15 +360,6 @@ impl App {
AppEvent::OpenFullAccessConfirmation { preset } => {
self.chat_widget.open_full_access_confirmation(preset);
}
AppEvent::OpenFeedbackNote {
category,
include_logs,
} => {
self.chat_widget.open_feedback_note(category, include_logs);
}
AppEvent::OpenFeedbackConsent { category } => {
self.chat_widget.open_feedback_consent(category);
}
AppEvent::PersistModelSelection { model, effort } => {
let profile = self.active_profile.as_deref();
match persist_model_selection(&self.config.codex_home, profile, &model, effort)

View File

@@ -101,23 +101,4 @@ pub(crate) enum AppEvent {
/// Open the approval popup.
FullScreenApprovalRequest(ApprovalRequest),
/// Open the feedback note entry overlay after the user selects a category.
OpenFeedbackNote {
category: FeedbackCategory,
include_logs: bool,
},
/// Open the upload consent popup for feedback after selecting a category.
OpenFeedbackConsent {
category: FeedbackCategory,
},
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum FeedbackCategory {
BadResult,
GoodResult,
Bug,
Other,
}

View File

@@ -52,7 +52,6 @@ use crate::ui_consts::LIVE_PREFIX_COLS;
use codex_file_search::FileMatch;
use std::cell::RefCell;
use std::collections::HashMap;
use std::io::ErrorKind;
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
@@ -62,34 +61,6 @@ use std::time::Instant;
/// placeholder in the UI.
const LARGE_PASTE_CHAR_THRESHOLD: usize = 1000;
fn maybe_prefix_root_like(path: &Path) -> Option<PathBuf> {
#[cfg(windows)]
{
let _ = path;
None
}
#[cfg(not(windows))]
{
if path.has_root() {
return None;
}
let path_str = path.to_string_lossy();
const ROOT_PREFIXES: [&str; 5] =
["Applications/", "Library/", "System/", "Users/", "Volumes/"];
if ROOT_PREFIXES
.iter()
.any(|prefix| path_str.starts_with(prefix))
{
return Some(PathBuf::from(format!("/{path_str}")));
}
None
}
}
/// Result returned when the user interacts with the text area.
#[derive(Debug, PartialEq)]
pub enum InputResult {
@@ -304,11 +275,11 @@ impl ChatComposer {
return false;
};
match Self::resolve_image_path_with_fallback(path_buf) {
Ok((resolved_path, w, h)) => {
match image::image_dimensions(&path_buf) {
Ok((w, h)) => {
tracing::info!("OK: {pasted}");
let format_label = pasted_image_format(&resolved_path).label();
self.attach_image(resolved_path, w, h, format_label);
let format_label = pasted_image_format(&path_buf).label();
self.attach_image(path_buf, w, h, format_label);
true
}
Err(err) => {
@@ -318,34 +289,6 @@ impl ChatComposer {
}
}
fn resolve_image_path_with_fallback(
path: PathBuf,
) -> Result<(PathBuf, u32, u32), image::ImageError> {
match image::image_dimensions(&path) {
Ok((w, h)) => Ok((path, w, h)),
Err(err) => {
if let image::ImageError::IoError(io_err) = &err
&& io_err.kind() == ErrorKind::NotFound
{
if let Some(fallback) = maybe_prefix_root_like(&path) {
match image::image_dimensions(&fallback) {
Ok((w, h)) => return Ok((fallback, w, h)),
Err(fallback_err) => {
tracing::debug!(
?fallback_err,
original = %path.display(),
fallback = %fallback.display(),
"fallback_dimensions_failed",
);
}
}
}
}
Err(err)
}
}
}
pub(crate) fn set_disable_paste_burst(&mut self, disabled: bool) {
let was_disabled = self.disable_paste_burst;
self.disable_paste_burst = disabled;
@@ -3505,20 +3448,4 @@ mod tests {
assert_eq!(composer.textarea.text(), "z".repeat(count));
assert!(composer.pending_pastes.is_empty());
}
#[cfg(not(windows))]
#[test]
fn maybe_prefix_root_like_adds_leading_slash() {
let input = PathBuf::from("Users/example/image.png");
let result = maybe_prefix_root_like(&input);
assert_eq!(result, Some(PathBuf::from("/Users/example/image.png")));
}
#[cfg(not(windows))]
#[test]
fn maybe_prefix_root_like_ignores_relative_dirs() {
let input = PathBuf::from("project/assets/image.png");
let result = maybe_prefix_root_like(&input);
assert!(result.is_none());
}
}

View File

@@ -1,448 +1,165 @@
use std::cell::RefCell;
use std::path::PathBuf;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use crate::history_cell;
use crate::history_cell::PlainHistoryCell;
use crate::render::renderable::Renderable;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::style::Stylize;
use ratatui::text::Line;
use ratatui::text::Span;
use ratatui::widgets::Clear;
use ratatui::widgets::Paragraph;
use ratatui::widgets::StatefulWidgetRef;
use ratatui::widgets::Widget;
use std::path::PathBuf;
use crate::app_event::AppEvent;
use crate::app_event::FeedbackCategory;
use crate::app_event_sender::AppEventSender;
use crate::history_cell;
use crate::render::renderable::Renderable;
use super::CancellationEvent;
use super::bottom_pane_view::BottomPaneView;
use super::popup_consts::standard_popup_hint_line;
use super::textarea::TextArea;
use super::textarea::TextAreaState;
use super::BottomPane;
use super::SelectionAction;
use super::SelectionItem;
use super::SelectionViewParams;
const BASE_ISSUE_URL: &str = "https://github.com/openai/codex/issues/new?template=2-bug-report.yml";
/// Minimal input overlay to collect an optional feedback note, then upload
/// both logs and rollout with classification + metadata.
pub(crate) struct FeedbackNoteView {
category: FeedbackCategory,
snapshot: codex_feedback::CodexLogSnapshot,
rollout_path: Option<PathBuf>,
app_event_tx: AppEventSender,
include_logs: bool,
pub(crate) struct FeedbackView;
// UI state
textarea: TextArea,
textarea_state: RefCell<TextAreaState>,
complete: bool,
}
impl FeedbackNoteView {
pub(crate) fn new(
category: FeedbackCategory,
impl FeedbackView {
pub fn show(
bottom_pane: &mut BottomPane,
file_path: PathBuf,
snapshot: codex_feedback::CodexLogSnapshot,
rollout_path: Option<PathBuf>,
app_event_tx: AppEventSender,
include_logs: bool,
) -> Self {
Self {
category,
snapshot,
rollout_path,
app_event_tx,
include_logs,
textarea: TextArea::new(),
textarea_state: RefCell::new(TextAreaState::default()),
complete: false,
}
) {
bottom_pane.show_selection_view(Self::selection_params(file_path, snapshot));
}
fn submit(&mut self) {
let note = self.textarea.text().trim().to_string();
let reason_opt = if note.is_empty() {
None
} else {
Some(note.as_str())
};
let rollout_path_ref = self.rollout_path.as_deref();
let classification = feedback_classification(self.category);
fn selection_params(
file_path: PathBuf,
snapshot: codex_feedback::CodexLogSnapshot,
) -> SelectionViewParams {
let header = FeedbackHeader::new(file_path);
let cli_version = crate::version::CODEX_CLI_VERSION;
let mut thread_id = self.snapshot.thread_id.clone();
let thread_id = snapshot.thread_id.clone();
let result = self.snapshot.upload_feedback(
classification,
reason_opt,
cli_version,
self.include_logs,
if self.include_logs {
rollout_path_ref
} else {
None
},
);
match result {
Ok(()) => {
let issue_url = format!("{BASE_ISSUE_URL}&steps=Uploaded%20thread:%20{thread_id}");
let prefix = if self.include_logs {
"• Feedback uploaded."
} else {
"• Feedback recorded (no logs)."
};
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::PlainHistoryCell::new(vec![
Line::from(format!(
"{prefix} Please open an issue using the following URL:"
)),
let upload_action_tread_id = thread_id.clone();
let upload_action: SelectionAction = Box::new(move |tx: &AppEventSender| {
match snapshot.upload_to_sentry() {
Ok(()) => {
let issue_url = format!(
"{BASE_ISSUE_URL}&steps=Uploaded%20thread:%20{upload_action_tread_id}",
);
tx.send(AppEvent::InsertHistoryCell(Box::new(PlainHistoryCell::new(vec![
Line::from(
"• Codex logs uploaded. Please open an issue using the following URL:",
),
"".into(),
Line::from(vec![" ".into(), issue_url.cyan().underlined()]),
"".into(),
Line::from(vec![
" Or mention your thread ID ".into(),
std::mem::take(&mut thread_id).bold(),
" in an existing issue.".into(),
]),
]),
)));
Line::from(vec![" Or mention your thread ID ".into(), upload_action_tread_id.clone().bold(), " in an existing issue.".into()])
]))));
}
Err(e) => {
tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_error_event(format!("Failed to upload logs: {e}")),
)));
}
}
Err(e) => {
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_error_event(format!("Failed to upload feedback: {e}")),
)));
}
}
self.complete = true;
}
}
});
impl BottomPaneView for FeedbackNoteView {
fn handle_key_event(&mut self, key_event: KeyEvent) {
match key_event {
KeyEvent {
code: KeyCode::Esc, ..
} => {
self.on_ctrl_c();
}
KeyEvent {
code: KeyCode::Enter,
modifiers: KeyModifiers::NONE,
..
} => {
self.submit();
}
KeyEvent {
code: KeyCode::Enter,
..
} => {
self.textarea.input(key_event);
}
other => {
self.textarea.input(other);
}
}
}
fn on_ctrl_c(&mut self) -> CancellationEvent {
self.complete = true;
CancellationEvent::Handled
}
fn is_complete(&self) -> bool {
self.complete
}
fn handle_paste(&mut self, pasted: String) -> bool {
if pasted.is_empty() {
return false;
}
self.textarea.insert_str(&pasted);
true
}
fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
if area.height < 2 || area.width <= 2 {
return None;
}
let text_area_height = self.input_height(area.width).saturating_sub(1);
if text_area_height == 0 {
return None;
}
let top_line_count = 1u16; // title only
let textarea_rect = Rect {
x: area.x.saturating_add(2),
y: area.y.saturating_add(top_line_count).saturating_add(1),
width: area.width.saturating_sub(2),
height: text_area_height,
let upload_item = SelectionItem {
name: "Yes".to_string(),
description: Some(
"Share the current Codex session logs with the team for troubleshooting."
.to_string(),
),
actions: vec![upload_action],
dismiss_on_select: true,
..Default::default()
};
let state = *self.textarea_state.borrow();
self.textarea.cursor_pos_with_state(textarea_rect, state)
let no_action: SelectionAction = Box::new(move |tx: &AppEventSender| {
let issue_url = format!("{BASE_ISSUE_URL}&steps=Thread%20ID:%20{thread_id}",);
tx.send(AppEvent::InsertHistoryCell(Box::new(
PlainHistoryCell::new(vec![
Line::from("• Please open an issue using the following URL:"),
"".into(),
Line::from(vec![" ".into(), issue_url.cyan().underlined()]),
"".into(),
Line::from(vec![
" Or mention your thread ID ".into(),
thread_id.clone().bold(),
" in an existing issue.".into(),
]),
]),
)));
});
let no_item = SelectionItem {
name: "No".to_string(),
actions: vec![no_action],
dismiss_on_select: true,
..Default::default()
};
let cancel_item = SelectionItem {
name: "Cancel".to_string(),
dismiss_on_select: true,
..Default::default()
};
SelectionViewParams {
header: Box::new(header),
items: vec![upload_item, no_item, cancel_item],
..Default::default()
}
}
}
impl Renderable for FeedbackNoteView {
fn desired_height(&self, width: u16) -> u16 {
1u16 + self.input_height(width) + 3u16
struct FeedbackHeader {
file_path: PathBuf,
}
impl FeedbackHeader {
fn new(file_path: PathBuf) -> Self {
Self { file_path }
}
fn lines(&self) -> Vec<Line<'static>> {
vec![
Line::from("Do you want to upload logs before reporting issue?".bold()),
"".into(),
Line::from(
"Logs may include the full conversation history of this Codex process, including prompts, tool calls, and their results.",
),
Line::from(
"These logs are retained for 90 days and are used solely for troubleshooting and diagnostic purposes.",
),
"".into(),
Line::from(vec![
"You can review the exact content of the logs before theyre uploaded at:".into(),
]),
Line::from(self.file_path.display().to_string().dim()),
"".into(),
]
}
}
impl Renderable for FeedbackHeader {
fn render(&self, area: Rect, buf: &mut Buffer) {
if area.height == 0 || area.width == 0 {
if area.width == 0 || area.height == 0 {
return;
}
let (title, placeholder) = feedback_title_and_placeholder(self.category);
let input_height = self.input_height(area.width);
// Title line
let title_area = Rect {
x: area.x,
y: area.y,
width: area.width,
height: 1,
};
let title_spans: Vec<Span<'static>> = vec![gutter(), title.bold()];
Paragraph::new(Line::from(title_spans)).render(title_area, buf);
// Input line
let input_area = Rect {
x: area.x,
y: area.y.saturating_add(1),
width: area.width,
height: input_height,
};
if input_area.width >= 2 {
for row in 0..input_area.height {
Paragraph::new(Line::from(vec![gutter()])).render(
Rect {
x: input_area.x,
y: input_area.y.saturating_add(row),
width: 2,
height: 1,
},
buf,
);
for (i, line) in self.lines().into_iter().enumerate() {
let y = area.y.saturating_add(i as u16);
if y >= area.y.saturating_add(area.height) {
break;
}
let text_area_height = input_area.height.saturating_sub(1);
if text_area_height > 0 {
if input_area.width > 2 {
let blank_rect = Rect {
x: input_area.x.saturating_add(2),
y: input_area.y,
width: input_area.width.saturating_sub(2),
height: 1,
};
Clear.render(blank_rect, buf);
}
let textarea_rect = Rect {
x: input_area.x.saturating_add(2),
y: input_area.y.saturating_add(1),
width: input_area.width.saturating_sub(2),
height: text_area_height,
};
let mut state = self.textarea_state.borrow_mut();
StatefulWidgetRef::render_ref(&(&self.textarea), textarea_rect, buf, &mut state);
if self.textarea.text().is_empty() {
Paragraph::new(Line::from(placeholder.dim())).render(textarea_rect, buf);
}
}
}
let hint_blank_y = input_area.y.saturating_add(input_height);
if hint_blank_y < area.y.saturating_add(area.height) {
let blank_area = Rect {
x: area.x,
y: hint_blank_y,
width: area.width,
height: 1,
};
Clear.render(blank_area, buf);
}
let hint_y = hint_blank_y.saturating_add(1);
if hint_y < area.y.saturating_add(area.height) {
Paragraph::new(standard_popup_hint_line()).render(
Rect {
x: area.x,
y: hint_y,
width: area.width,
height: 1,
},
buf,
);
let line_area = Rect::new(area.x, y, area.width, 1).intersection(area);
line.render(line_area, buf);
}
}
}
impl FeedbackNoteView {
fn input_height(&self, width: u16) -> u16 {
let usable_width = width.saturating_sub(2);
let text_height = self.textarea.desired_height(usable_width).clamp(1, 8);
text_height.saturating_add(1).min(9)
}
}
fn gutter() -> Span<'static> {
"".cyan()
}
fn feedback_title_and_placeholder(category: FeedbackCategory) -> (String, String) {
match category {
FeedbackCategory::BadResult => (
"Tell us more (bad result)".to_string(),
"(optional) Write a short description to help us further".to_string(),
),
FeedbackCategory::GoodResult => (
"Tell us more (good result)".to_string(),
"(optional) Write a short description to help us further".to_string(),
),
FeedbackCategory::Bug => (
"Tell us more (bug)".to_string(),
"(optional) Write a short description to help us further".to_string(),
),
FeedbackCategory::Other => (
"Tell us more (other)".to_string(),
"(optional) Write a short description to help us further".to_string(),
),
}
}
fn feedback_classification(category: FeedbackCategory) -> &'static str {
match category {
FeedbackCategory::BadResult => "bad_result",
FeedbackCategory::GoodResult => "good_result",
FeedbackCategory::Bug => "bug",
FeedbackCategory::Other => "other",
}
}
// Build the selection popup params for feedback categories.
pub(crate) fn feedback_selection_params(
app_event_tx: AppEventSender,
) -> super::SelectionViewParams {
super::SelectionViewParams {
title: Some("How was this?".to_string()),
items: vec![
make_feedback_item(
app_event_tx.clone(),
"bug",
"Crash, error message, hang, or broken UI/behavior.",
FeedbackCategory::Bug,
),
make_feedback_item(
app_event_tx.clone(),
"bad result",
"Output was off-target, incorrect, incomplete, or unhelpful.",
FeedbackCategory::BadResult,
),
make_feedback_item(
app_event_tx.clone(),
"good result",
"Helpful, correct, highquality, or delightful result worth celebrating.",
FeedbackCategory::GoodResult,
),
make_feedback_item(
app_event_tx,
"other",
"Slowness, feature suggestion, UX feedback, or anything else.",
FeedbackCategory::Other,
),
],
..Default::default()
}
}
fn make_feedback_item(
app_event_tx: AppEventSender,
name: &str,
description: &str,
category: FeedbackCategory,
) -> super::SelectionItem {
let action: super::SelectionAction = Box::new(move |_sender: &AppEventSender| {
app_event_tx.send(AppEvent::OpenFeedbackConsent { category });
});
super::SelectionItem {
name: name.to_string(),
description: Some(description.to_string()),
actions: vec![action],
dismiss_on_select: true,
..Default::default()
}
}
/// Build the upload consent popup params for a given feedback category.
pub(crate) fn feedback_upload_consent_params(
app_event_tx: AppEventSender,
category: FeedbackCategory,
rollout_path: Option<std::path::PathBuf>,
) -> super::SelectionViewParams {
use super::popup_consts::standard_popup_hint_line;
let yes_action: super::SelectionAction = Box::new({
let tx = app_event_tx.clone();
move |sender: &AppEventSender| {
let _ = sender;
tx.send(AppEvent::OpenFeedbackNote {
category,
include_logs: true,
});
}
});
let no_action: super::SelectionAction = Box::new({
let tx = app_event_tx;
move |sender: &AppEventSender| {
let _ = sender;
tx.send(AppEvent::OpenFeedbackNote {
category,
include_logs: false,
});
}
});
// Build header listing files that would be sent if user consents.
let mut header_lines: Vec<Box<dyn crate::render::renderable::Renderable>> = vec![
Line::from("Upload logs?".bold()).into(),
Line::from("").into(),
Line::from("The following files will be sent:".dim()).into(),
Line::from(vec!["".into(), "codex-logs.log".into()]).into(),
];
if let Some(path) = rollout_path.as_deref()
&& let Some(name) = path.file_name().map(|s| s.to_string_lossy().to_string())
{
header_lines.push(Line::from(vec!["".into(), name.into()]).into());
}
super::SelectionViewParams {
footer_hint: Some(standard_popup_hint_line()),
items: vec![
super::SelectionItem {
name: "Yes".to_string(),
description: Some(
"Share the current Codex session logs with the team for troubleshooting."
.to_string(),
),
actions: vec![yes_action],
dismiss_on_select: true,
..Default::default()
},
super::SelectionItem {
name: "No".to_string(),
description: Some("".to_string()),
actions: vec![no_action],
dismiss_on_select: true,
..Default::default()
},
],
header: Box::new(crate::render::renderable::ColumnRenderable::with(
header_lines,
)),
..Default::default()
fn desired_height(&self, width: u16) -> u16 {
self.lines()
.iter()
.map(|line| line.desired_height(width))
.sum()
}
}
@@ -450,19 +167,22 @@ pub(crate) fn feedback_upload_consent_params(
mod tests {
use super::*;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use crate::bottom_pane::list_selection_view::ListSelectionView;
use crate::style::user_message_style;
use codex_feedback::CodexFeedback;
use codex_protocol::ConversationId;
use insta::assert_snapshot;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::style::Color;
use tokio::sync::mpsc::unbounded_channel;
fn render(view: &FeedbackNoteView, width: u16) -> String {
let height = view.desired_height(width);
let area = Rect::new(0, 0, width, height);
let mut buf = Buffer::empty(area);
view.render(area, &mut buf);
let mut lines: Vec<String> = (0..area.height)
fn buffer_to_string(buffer: &Buffer) -> String {
(0..buffer.area.height)
.map(|row| {
let mut line = String::new();
for col in 0..area.width {
let symbol = buf[(area.x + col, area.y + row)].symbol();
for col in 0..buffer.area.width {
let symbol = buffer[(buffer.area.x + col, buffer.area.y + row)].symbol();
if symbol.is_empty() {
line.push(' ');
} else {
@@ -471,49 +191,34 @@ mod tests {
}
line.trim_end().to_string()
})
.collect();
while lines.first().is_some_and(|l| l.trim().is_empty()) {
lines.remove(0);
}
while lines.last().is_some_and(|l| l.trim().is_empty()) {
lines.pop();
}
lines.join("\n")
}
fn make_view(category: FeedbackCategory) -> FeedbackNoteView {
let (tx_raw, _rx) = tokio::sync::mpsc::unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let snapshot = codex_feedback::CodexFeedback::new().snapshot(None);
FeedbackNoteView::new(category, snapshot, None, tx, true)
.collect::<Vec<_>>()
.join("\n")
}
#[test]
fn feedback_view_bad_result() {
let view = make_view(FeedbackCategory::BadResult);
let rendered = render(&view, 60);
insta::assert_snapshot!("feedback_view_bad_result", rendered);
}
fn renders_feedback_view_header() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let app_event_tx = AppEventSender::new(tx_raw);
let snapshot = CodexFeedback::new().snapshot(Some(
ConversationId::from_string("550e8400-e29b-41d4-a716-446655440000").unwrap(),
));
let file_path = PathBuf::from("/tmp/codex-feedback.log");
#[test]
fn feedback_view_good_result() {
let view = make_view(FeedbackCategory::GoodResult);
let rendered = render(&view, 60);
insta::assert_snapshot!("feedback_view_good_result", rendered);
}
let params = FeedbackView::selection_params(file_path.clone(), snapshot);
let view = ListSelectionView::new(params, app_event_tx);
#[test]
fn feedback_view_bug() {
let view = make_view(FeedbackCategory::Bug);
let rendered = render(&view, 60);
insta::assert_snapshot!("feedback_view_bug", rendered);
}
let width = 72;
let height = view.desired_height(width).max(1);
let area = Rect::new(0, 0, width, height);
let mut buf = Buffer::empty(area);
view.render(area, &mut buf);
#[test]
fn feedback_view_other() {
let view = make_view(FeedbackCategory::Other);
let rendered = render(&view, 60);
insta::assert_snapshot!("feedback_view_other", rendered);
let rendered =
buffer_to_string(&buf).replace(&file_path.display().to_string(), "<LOG_PATH>");
assert_snapshot!("feedback_view_render", rendered);
let cell_style = buf[(area.x, area.y)].style();
let expected_bg = user_message_style().bg.unwrap_or(Color::Reset);
assert_eq!(cell_style.bg.unwrap_or(Color::Reset), expected_bg);
}
}

View File

@@ -28,14 +28,12 @@ mod list_selection_view;
mod prompt_args;
pub(crate) use list_selection_view::SelectionViewParams;
mod feedback_view;
pub(crate) use feedback_view::feedback_selection_params;
pub(crate) use feedback_view::feedback_upload_consent_params;
mod paste_burst;
pub mod popup_consts;
mod scroll_state;
mod selection_popup_common;
mod textarea;
pub(crate) use feedback_view::FeedbackNoteView;
pub(crate) use feedback_view::FeedbackView;
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) enum CancellationEvent {

View File

@@ -1,9 +0,0 @@
---
source: tui/src/bottom_pane/feedback_view.rs
expression: rendered
---
▌ Tell us more (bad result)
▌ (optional) Write a short description to help us further
Press enter to confirm or esc to go back

View File

@@ -1,9 +0,0 @@
---
source: tui/src/bottom_pane/feedback_view.rs
expression: rendered
---
▌ Tell us more (bug)
▌ (optional) Write a short description to help us further
Press enter to confirm or esc to go back

View File

@@ -1,9 +0,0 @@
---
source: tui/src/bottom_pane/feedback_view.rs
expression: rendered
---
▌ Tell us more (good result)
▌ (optional) Write a short description to help us further
Press enter to confirm or esc to go back

View File

@@ -1,9 +0,0 @@
---
source: tui/src/bottom_pane/feedback_view.rs
expression: rendered
---
▌ Tell us more (other)
▌ (optional) Write a short description to help us further
Press enter to confirm or esc to go back

View File

@@ -276,8 +276,6 @@ pub(crate) struct ChatWidget {
last_rendered_width: std::cell::Cell<Option<usize>>,
// Feedback sink for /feedback
feedback: codex_feedback::CodexFeedback,
// Current session rollout path (if known)
current_rollout_path: Option<PathBuf>,
}
struct UserMessage {
@@ -324,7 +322,6 @@ impl ChatWidget {
self.bottom_pane
.set_history_metadata(event.history_log_id, event.history_entry_count);
self.conversation_id = Some(event.session_id);
self.current_rollout_path = Some(event.rollout_path.clone());
let initial_messages = event.initial_messages.clone();
let model_for_header = event.model.clone();
self.session_header.set_model(&model_for_header);
@@ -346,39 +343,6 @@ impl ChatWidget {
}
}
pub(crate) fn open_feedback_note(
&mut self,
category: crate::app_event::FeedbackCategory,
include_logs: bool,
) {
// Build a fresh snapshot at the time of opening the note overlay.
let snapshot = self.feedback.snapshot(self.conversation_id);
let rollout = if include_logs {
self.current_rollout_path.clone()
} else {
None
};
let view = crate::bottom_pane::FeedbackNoteView::new(
category,
snapshot,
rollout,
self.app_event_tx.clone(),
include_logs,
);
self.bottom_pane.show_view(Box::new(view));
self.request_redraw();
}
pub(crate) fn open_feedback_consent(&mut self, category: crate::app_event::FeedbackCategory) {
let params = crate::bottom_pane::feedback_upload_consent_params(
self.app_event_tx.clone(),
category,
self.current_rollout_path.clone(),
);
self.bottom_pane.show_selection_view(params);
self.request_redraw();
}
fn on_agent_message(&mut self, message: String) {
// If we have a stream_controller, then the final agent message is redundant and will be a
// duplicate of what has already been streamed.
@@ -532,7 +496,7 @@ impl ChatWidget {
if reason != TurnAbortReason::ReviewEnded {
self.add_to_history(history_cell::new_error_event(
"Conversation interrupted - tell the model what to do differently. Something went wrong? Hit `/feedback` to report the issue.".to_owned(),
"Conversation interrupted - tell the model what to do differently".to_owned(),
));
}
@@ -994,7 +958,6 @@ impl ChatWidget {
needs_final_message_separator: false,
last_rendered_width: std::cell::Cell::new(None),
feedback,
current_rollout_path: None,
}
}
@@ -1062,7 +1025,6 @@ impl ChatWidget {
needs_final_message_separator: false,
last_rendered_width: std::cell::Cell::new(None),
feedback,
current_rollout_path: None,
}
}
@@ -1167,11 +1129,23 @@ impl ChatWidget {
}
match cmd {
SlashCommand::Feedback => {
// Step 1: pick a category (UI built in feedback_view)
let params =
crate::bottom_pane::feedback_selection_params(self.app_event_tx.clone());
self.bottom_pane.show_selection_view(params);
self.request_redraw();
let snapshot = self.feedback.snapshot(self.conversation_id);
match snapshot.save_to_temp_file() {
Ok(path) => {
crate::bottom_pane::FeedbackView::show(
&mut self.bottom_pane,
path,
snapshot,
);
self.request_redraw();
}
Err(e) => {
self.add_to_history(history_cell::new_error_event(format!(
"Failed to save feedback logs: {e}"
)));
self.request_redraw();
}
}
}
SlashCommand::New => {
self.app_event_tx.send(AppEvent::NewSession);
@@ -1524,9 +1498,7 @@ impl ChatWidget {
self.on_entered_review_mode(review_request)
}
EventMsg::ExitedReviewMode(review) => self.on_exited_review_mode(review),
EventMsg::RawResponseItem(_)
| EventMsg::ItemStarted(_)
| EventMsg::ItemCompleted(_) => {}
EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) => {}
}
}

View File

@@ -1,11 +0,0 @@
---
source: tui/src/chatwidget/tests.rs
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, highquality, or delightful result worth
celebrating.
4. other Slowness, feature suggestion, UX feedback, or anything else.

View File

@@ -1,14 +0,0 @@
---
source: tui/src/chatwidget/tests.rs
expression: popup
---
Upload logs?
The following files will be sent:
• codex-logs.log
1. Yes Share the current Codex session logs with the team for
troubleshooting.
2. No
Press enter to confirm or esc to go back

View File

@@ -1,5 +0,0 @@
---
source: tui/src/chatwidget/tests.rs
expression: last
---
■ Conversation interrupted - tell the model what to do differently. Something went wrong? Hit `/feedback` to report the issue.

View File

@@ -299,7 +299,6 @@ fn make_chatwidget_manual() -> (
needs_final_message_separator: false,
last_rendered_width: std::cell::Cell::new(None),
feedback: codex_feedback::CodexFeedback::new(),
current_rollout_path: None,
};
(widget, rx, op_rx)
}
@@ -999,37 +998,6 @@ fn interrupt_exec_marks_failed_snapshot() {
assert_snapshot!("interrupt_exec_marks_failed", exec_blob);
}
// Snapshot test: after an interrupted turn, a gentle error message is inserted
// suggesting the user to tell the model what to do differently and to use /feedback.
#[test]
fn interrupted_turn_error_message_snapshot() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
// Simulate an in-progress task so the widget is in a running state.
chat.handle_codex_event(Event {
id: "task-1".into(),
msg: EventMsg::TaskStarted(TaskStartedEvent {
model_context_window: None,
}),
});
// Abort the turn (like pressing Esc) and drain inserted history.
chat.handle_codex_event(Event {
id: "task-1".into(),
msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent {
reason: TurnAbortReason::Interrupted,
}),
});
let cells = drain_insert_history(&mut rx);
assert!(
!cells.is_empty(),
"expected error message to be inserted after interruption"
);
let last = lines_to_single_string(cells.last().unwrap());
assert_snapshot!("interrupted_turn_error_message", last);
}
/// Opening custom prompt from the review popup, pressing Esc returns to the
/// parent popup, pressing Esc again dismisses all panels (back to normal mode).
#[test]
@@ -1207,28 +1175,6 @@ fn model_reasoning_selection_popup_snapshot() {
assert_snapshot!("model_reasoning_selection_popup", popup);
}
#[test]
fn feedback_selection_popup_snapshot() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();
// Open the feedback category selection popup via slash command.
chat.dispatch_command(SlashCommand::Feedback);
let popup = render_bottom_popup(&chat, 80);
assert_snapshot!("feedback_selection_popup", popup);
}
#[test]
fn feedback_upload_consent_popup_snapshot() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();
// Open the consent popup directly for a chosen category.
chat.open_feedback_consent(crate::app_event::FeedbackCategory::Bug);
let popup = render_bottom_popup(&chat, 80);
assert_snapshot!("feedback_upload_consent_popup", popup);
}
#[test]
fn reasoning_popup_escape_returns_to_model_popup() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();

View File

@@ -187,36 +187,12 @@ pub fn normalize_pasted_path(pasted: &str) -> Option<PathBuf> {
// shell-escaped single path → unescaped
let parts: Vec<String> = shlex::Shlex::new(pasted).collect();
if parts.len() == 1 {
let mut path = parts.into_iter().next()?;
#[cfg(not(windows))]
{
path = fixup_unix_root_relative_path(path);
}
return Some(PathBuf::from(path));
return parts.into_iter().next().map(PathBuf::from);
}
None
}
#[cfg(not(windows))]
fn fixup_unix_root_relative_path(mut path: String) -> String {
use std::path::Path;
if Path::new(&path).has_root() {
return path;
}
const ROOT_PREFIXES: [&str; 5] = ["Applications/", "Library/", "System/", "Users/", "Volumes/"];
if ROOT_PREFIXES.iter().any(|prefix| path.starts_with(prefix)) {
path.insert(0, '/');
}
path
}
/// Infer an image format for the provided path based on its extension.
pub fn pasted_image_format(path: &Path) -> EncodedImageFormat {
match path
@@ -279,25 +255,6 @@ mod pasted_paths_tests {
assert!(result.is_none());
}
#[cfg(not(windows))]
#[test]
fn normalize_dragged_finder_users_path() {
let input = "'Users/alice/Pictures/example.png'";
let result = normalize_pasted_path(input).expect("should add leading slash for Users/");
assert_eq!(result, PathBuf::from("/Users/alice/Pictures/example.png"));
}
#[cfg(not(windows))]
#[test]
fn normalize_dragged_finder_volumes_path() {
let input = "'Volumes/ExternalDrive/photos/image.jpg'";
let result = normalize_pasted_path(input).expect("should add leading slash for Volumes/");
assert_eq!(
result,
PathBuf::from("/Volumes/ExternalDrive/photos/image.jpg")
);
}
#[test]
fn pasted_image_format_png_jpeg_unknown() {
assert_eq!(

View File

@@ -1047,10 +1047,7 @@ pub(crate) fn new_mcp_tools_output(
return PlainHistoryCell { lines };
}
let mut servers: Vec<_> = config.mcp_servers.iter().collect();
servers.sort_by(|(a, _), (b, _)| a.cmp(b));
for (server, cfg) in servers {
for (server, cfg) in config.mcp_servers.iter() {
let prefix = format!("mcp__{server}__");
let mut names: Vec<String> = tools
.keys()
@@ -1114,7 +1111,7 @@ pub(crate) fn new_mcp_tools_output(
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
let display = pairs
.into_iter()
.map(|(name, _)| format!("{name}=*****"))
.map(|(name, value)| format!("{name}={value}"))
.collect::<Vec<_>>()
.join(", ");
lines.push(vec![" • HTTP headers: ".into(), display.into()].into());
@@ -1126,7 +1123,7 @@ pub(crate) fn new_mcp_tools_output(
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
let display = pairs
.into_iter()
.map(|(name, var)| format!("{name}={var}"))
.map(|(name, env_var)| format!("{name}={env_var}"))
.collect::<Vec<_>>()
.join(", ");
lines.push(vec![" • Env HTTP headers: ".into(), display.into()].into());
@@ -1418,20 +1415,14 @@ mod tests {
use codex_core::config::Config;
use codex_core::config::ConfigOverrides;
use codex_core::config::ConfigToml;
use codex_core::config_types::McpServerConfig;
use codex_core::config_types::McpServerTransportConfig;
use codex_core::protocol::McpAuthStatus;
use codex_protocol::parse_command::ParsedCommand;
use dirs::home_dir;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::collections::HashMap;
use mcp_types::CallToolResult;
use mcp_types::ContentBlock;
use mcp_types::TextContent;
use mcp_types::Tool;
use mcp_types::ToolInputSchema;
fn test_config() -> Config {
Config::load_from_base_config_with_overrides(
@@ -1458,91 +1449,6 @@ mod tests {
render_lines(&cell.transcript_lines(u16::MAX))
}
#[test]
fn mcp_tools_output_masks_sensitive_values() {
let mut config = test_config();
let mut env = HashMap::new();
env.insert("TOKEN".to_string(), "secret".to_string());
let stdio_config = McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: "docs-server".to_string(),
args: vec![],
env: Some(env),
env_vars: vec!["APP_TOKEN".to_string()],
cwd: None,
},
enabled: true,
startup_timeout_sec: None,
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
};
config.mcp_servers.insert("docs".to_string(), stdio_config);
let mut headers = HashMap::new();
headers.insert("Authorization".to_string(), "Bearer secret".to_string());
let mut env_headers = HashMap::new();
env_headers.insert("X-API-Key".to_string(), "API_KEY_ENV".to_string());
let http_config = McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url: "https://example.com/mcp".to_string(),
bearer_token_env_var: Some("MCP_TOKEN".to_string()),
http_headers: Some(headers),
env_http_headers: Some(env_headers),
},
enabled: true,
startup_timeout_sec: None,
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
};
config.mcp_servers.insert("http".to_string(), http_config);
let mut tools: HashMap<String, Tool> = HashMap::new();
tools.insert(
"mcp__docs__list".to_string(),
Tool {
annotations: None,
description: None,
input_schema: ToolInputSchema {
properties: None,
required: None,
r#type: "object".to_string(),
},
name: "list".to_string(),
output_schema: None,
title: None,
},
);
tools.insert(
"mcp__http__ping".to_string(),
Tool {
annotations: None,
description: None,
input_schema: ToolInputSchema {
properties: None,
required: None,
r#type: "object".to_string(),
},
name: "ping".to_string(),
output_schema: None,
title: None,
},
);
let auth_statuses: HashMap<String, McpAuthStatus> = HashMap::new();
let cell = new_mcp_tools_output(
&config,
tools,
HashMap::new(),
HashMap::new(),
&auth_statuses,
);
let rendered = render_lines(&cell.display_lines(120)).join("\n");
insta::assert_snapshot!(rendered);
}
#[test]
fn empty_agent_message_cell_transcript() {
let cell = AgentMessageCell::new(vec![Line::default()], false);

View File

@@ -1,27 +0,0 @@
---
source: tui/src/history_cell.rs
assertion_line: 1540
expression: rendered
---
/mcp
🔌 MCP Tools
• docs
• Status: enabled
• Auth: Unsupported
• Command: docs-server
• Env: TOKEN=*****, APP_TOKEN=*****
• Tools: list
• Resources: (none)
• Resource templates: (none)
• http
• Status: enabled
• Auth: Unsupported
• URL: https://example.com/mcp
• HTTP headers: Authorization=*****
• Env HTTP headers: X-API-Key=API_KEY_ENV
• Tools: ping
• Resources: (none)
• Resource templates: (none)

View File

@@ -417,7 +417,7 @@ cwd = "/Users/<user>/code/my-server"
[mcp_servers.figma]
url = "https://mcp.linear.app/mcp"
# Optional environment variable containing a bearer token to use for auth
bearer_token_env_var = "ENV_VAR"
bearer_token_env_var = "<token>"
# Optional map of headers with hard-coded values.
http_headers = { "HEADER_NAME" = "HEADER_VALUE" }
# Optional map of headers whose values will be replaced with the environment variable.

View File

@@ -3,7 +3,7 @@ import os from "node:os";
import path from "node:path";
import { codexExecSpy } from "./codexExecSpy";
import { describe, expect, it, xit } from "@jest/globals";
import { describe, expect, it } from "@jest/globals";
import { Codex } from "../src/codex";
@@ -308,8 +308,7 @@ describe("Codex", () => {
await close();
}
});
// TODO(pakrym): unskip the test
xit("forwards images to exec", async () => {
it("forwards images to exec", async () => {
const { url, close } = await startResponsesTestProxy({
statusCode: 200,
responseBodies: [