mirror of
https://github.com/openai/codex.git
synced 2026-02-03 07:23:39 +00:00
Compare commits
11 Commits
pr5657
...
fix/image-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8fb94fb11b | ||
|
|
7aab45e060 | ||
|
|
bcd64c7e72 | ||
|
|
c124f24354 | ||
|
|
c7e4e6d0ee | ||
|
|
88abbf58ce | ||
|
|
71f838389b | ||
|
|
0533bd2e7c | ||
|
|
6af83d86ff | ||
|
|
e2e1b65da6 | ||
|
|
817d1508bc |
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -853,6 +853,7 @@ dependencies = [
|
||||
"pretty_assertions",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"serial_test",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"toml",
|
||||
|
||||
@@ -717,6 +717,8 @@ 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)]
|
||||
|
||||
@@ -47,6 +47,7 @@ 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 }
|
||||
|
||||
@@ -1256,7 +1256,10 @@ impl CodexMessageProcessor {
|
||||
request_id: RequestId,
|
||||
params: AddConversationListenerParams,
|
||||
) {
|
||||
let AddConversationListenerParams { conversation_id } = params;
|
||||
let AddConversationListenerParams {
|
||||
conversation_id,
|
||||
experimental_raw_events,
|
||||
} = params;
|
||||
let Ok(conversation) = self
|
||||
.conversation_manager
|
||||
.get_conversation(conversation_id)
|
||||
@@ -1293,6 +1296,11 @@ 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`
|
||||
|
||||
@@ -103,7 +103,10 @@ async fn test_codex_jsonrpc_conversation_flow() {
|
||||
|
||||
// 2) addConversationListener
|
||||
let add_listener_id = mcp
|
||||
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
|
||||
.send_add_conversation_listener_request(AddConversationListenerParams {
|
||||
conversation_id,
|
||||
experimental_raw_events: false,
|
||||
})
|
||||
.await
|
||||
.expect("send addConversationListener");
|
||||
let add_listener_resp: JSONRPCResponse = timeout(
|
||||
@@ -252,7 +255,10 @@ 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 })
|
||||
.send_add_conversation_listener_request(AddConversationListenerParams {
|
||||
conversation_id,
|
||||
experimental_raw_events: false,
|
||||
})
|
||||
.await
|
||||
.expect("send addConversationListener");
|
||||
let _: AddConversationSubscriptionResponse =
|
||||
@@ -459,7 +465,10 @@ 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 })
|
||||
.send_add_conversation_listener_request(AddConversationListenerParams {
|
||||
conversation_id,
|
||||
experimental_raw_events: false,
|
||||
})
|
||||
.await
|
||||
.expect("send addConversationListener");
|
||||
timeout(
|
||||
|
||||
@@ -67,7 +67,10 @@ 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 })
|
||||
.send_add_conversation_listener_request(AddConversationListenerParams {
|
||||
conversation_id,
|
||||
experimental_raw_events: false,
|
||||
})
|
||||
.await
|
||||
.expect("send addConversationListener");
|
||||
let _sub: AddConversationSubscriptionResponse =
|
||||
|
||||
@@ -88,7 +88,10 @@ async fn shell_command_interruption() -> anyhow::Result<()> {
|
||||
|
||||
// 2) addConversationListener
|
||||
let add_listener_id = mcp
|
||||
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
|
||||
.send_add_conversation_listener_request(AddConversationListenerParams {
|
||||
conversation_id,
|
||||
experimental_raw_events: false,
|
||||
})
|
||||
.await?;
|
||||
let _add_listener_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
|
||||
@@ -13,6 +13,7 @@ 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;
|
||||
|
||||
@@ -94,6 +95,8 @@ 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}"));
|
||||
@@ -208,6 +211,8 @@ 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")
|
||||
|
||||
@@ -15,6 +15,8 @@ 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;
|
||||
@@ -62,7 +64,10 @@ async fn test_send_message_success() {
|
||||
|
||||
// 2) addConversationListener
|
||||
let add_listener_id = mcp
|
||||
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
|
||||
.send_add_conversation_listener_request(AddConversationListenerParams {
|
||||
conversation_id,
|
||||
experimental_raw_events: false,
|
||||
})
|
||||
.await
|
||||
.expect("send addConversationListener");
|
||||
let add_listener_resp: JSONRPCResponse = timeout(
|
||||
@@ -124,6 +129,105 @@ 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]
|
||||
@@ -184,3 +288,108 @@ 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()
|
||||
}
|
||||
|
||||
@@ -274,19 +274,33 @@ 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))
|
||||
{
|
||||
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.");
|
||||
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."
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
@@ -523,10 +537,12 @@ 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_env_var.clone().unwrap_or("-".to_string()),
|
||||
bearer_token_display,
|
||||
status,
|
||||
auth_status,
|
||||
]);
|
||||
@@ -752,15 +768,15 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re
|
||||
} => {
|
||||
println!(" transport: streamable_http");
|
||||
println!(" url: {url}");
|
||||
let env_var = bearer_token_env_var.as_deref().unwrap_or("-");
|
||||
println!(" bearer_token_env_var: {env_var}");
|
||||
let bearer_token_display = bearer_token_env_var.as_deref().unwrap_or("-");
|
||||
println!(" bearer_token_env_var: {bearer_token_display}");
|
||||
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, v)| format!("{k}={v}"))
|
||||
.map(|(k, _)| format!("{k}=*****"))
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ")
|
||||
}
|
||||
@@ -773,7 +789,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, v)| format!("{k}={v}"))
|
||||
.map(|(k, var)| format!("{k}={var}"))
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ")
|
||||
}
|
||||
|
||||
@@ -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=secret"));
|
||||
assert!(stdout.contains("APP_TOKEN=$APP_TOKEN"));
|
||||
assert!(stdout.contains("WORKSPACE_ID=$WORKSPACE_ID"));
|
||||
assert!(stdout.contains("TOKEN=*****"));
|
||||
assert!(stdout.contains("APP_TOKEN=*****"));
|
||||
assert!(stdout.contains("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=secret"));
|
||||
assert!(stdout.contains("APP_TOKEN=$APP_TOKEN"));
|
||||
assert!(stdout.contains("WORKSPACE_ID=$WORKSPACE_ID"));
|
||||
assert!(stdout.contains("env: TOKEN=*****"));
|
||||
assert!(stdout.contains("APP_TOKEN=*****"));
|
||||
assert!(stdout.contains("WORKSPACE_ID=*****"));
|
||||
assert!(stdout.contains("enabled: true"));
|
||||
assert!(stdout.contains("remove: codex mcp remove docs"));
|
||||
|
||||
|
||||
@@ -6,15 +6,11 @@ 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, value)| format!("{key}={value}")),
|
||||
);
|
||||
parts.extend(pairs.into_iter().map(|(key, _)| format!("{key}=*****")));
|
||||
}
|
||||
|
||||
if !env_vars.is_empty() {
|
||||
parts.extend(env_vars.iter().map(|var| format!("{var}=${var}")));
|
||||
parts.extend(env_vars.iter().map(|var| format!("{var}=*****")));
|
||||
}
|
||||
|
||||
if parts.is_empty() {
|
||||
@@ -42,14 +38,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=one, B=two");
|
||||
assert_eq!(format_env_display(Some(&env), &[]), "A=*****, B=*****");
|
||||
}
|
||||
|
||||
#[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=$TOKEN, PATH=$PATH");
|
||||
assert_eq!(format_env_display(None, &vars), "TOKEN=*****, PATH=*****");
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -60,7 +56,7 @@ mod tests {
|
||||
|
||||
assert_eq!(
|
||||
format_env_display(Some(&env), &vars),
|
||||
"HOME=/tmp, TOKEN=$TOKEN"
|
||||
"HOME=*****, TOKEN=*****"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -570,7 +570,6 @@ 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(),
|
||||
@@ -589,6 +588,9 @@ 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)
|
||||
}
|
||||
|
||||
@@ -609,7 +611,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(&items).await;
|
||||
self.record_conversation_items(&turn_context, &items).await;
|
||||
}
|
||||
InitialHistory::Resumed(_) | InitialHistory::Forked(_) => {
|
||||
let rollout_items = conversation_history.get_rollout_items();
|
||||
@@ -886,9 +888,14 @@ 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, items: &[ResponseItem]) {
|
||||
pub(crate) async fn record_conversation_items(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
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(
|
||||
@@ -938,6 +945,13 @@ 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() {
|
||||
@@ -1033,7 +1047,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(std::slice::from_ref(&response_item))
|
||||
self.record_conversation_items(turn_context, std::slice::from_ref(&response_item))
|
||||
.await;
|
||||
|
||||
// Derive user message events and persist only UserMessage to rollout
|
||||
@@ -1224,8 +1238,11 @@ 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(), ¤t_context)
|
||||
{
|
||||
sess.record_conversation_items(std::slice::from_ref(&env_item))
|
||||
.await;
|
||||
sess.record_conversation_items(
|
||||
¤t_context,
|
||||
std::slice::from_ref(&env_item),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
sess.spawn_task(Arc::clone(¤t_context), items, RegularTask)
|
||||
@@ -1597,7 +1614,8 @@ pub(crate) async fn run_task(
|
||||
}
|
||||
review_thread_history.get_history()
|
||||
} else {
|
||||
sess.record_conversation_items(&pending_input).await;
|
||||
sess.record_conversation_items(&turn_context, &pending_input)
|
||||
.await;
|
||||
sess.history_snapshot().await
|
||||
};
|
||||
|
||||
@@ -1644,6 +1662,7 @@ pub(crate) async fn run_task(
|
||||
is_review_mode,
|
||||
&mut review_thread_history,
|
||||
&sess,
|
||||
&turn_context,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -1692,6 +1711,7 @@ 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.
|
||||
@@ -2202,11 +2222,14 @@ pub(crate) async fn exit_review_mode(
|
||||
}
|
||||
|
||||
session
|
||||
.record_conversation_items(&[ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText { text: user_message }],
|
||||
}])
|
||||
.record_conversation_items(
|
||||
&turn_context,
|
||||
&[ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText { text: user_message }],
|
||||
}],
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
@@ -2801,13 +2824,19 @@ mod tests {
|
||||
EventMsg::ExitedReviewMode(ev) => assert!(ev.review_output.is_none()),
|
||||
other => panic!("unexpected first 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:?}"),
|
||||
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 history = sess.history_snapshot().await;
|
||||
|
||||
@@ -200,7 +200,20 @@ pub(crate) fn build_compacted_history(
|
||||
user_messages: &[String],
|
||||
summary_text: &str,
|
||||
) -> Vec<ResponseItem> {
|
||||
let mut history = initial_context;
|
||||
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 user_messages_text = if user_messages.is_empty() {
|
||||
"(none)".to_string()
|
||||
} else {
|
||||
@@ -208,7 +221,6 @@ pub(crate) fn build_compacted_history(
|
||||
};
|
||||
// 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;
|
||||
}
|
||||
@@ -361,11 +373,16 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn build_compacted_history_truncates_overlong_user_messages() {
|
||||
// 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");
|
||||
// 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,
|
||||
);
|
||||
|
||||
// Expect exactly one bridge message added to history (plus any initial context we provided, which is none).
|
||||
assert_eq!(history.len(), 1);
|
||||
|
||||
@@ -1027,9 +1027,11 @@ 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
|
||||
@@ -1219,7 +1221,8 @@ impl Config {
|
||||
.get_active_project(&resolved_cwd)
|
||||
.unwrap_or(ProjectConfig { trust_level: None });
|
||||
|
||||
let mut sandbox_policy = cfg.derive_sandbox_policy(sandbox_mode, &resolved_cwd);
|
||||
let mut sandbox_policy =
|
||||
cfg.derive_sandbox_policy(sandbox_mode, config_profile.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) {
|
||||
@@ -1242,8 +1245,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();
|
||||
@@ -1603,8 +1606,11 @@ 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, &PathBuf::from("/tmp/test"))
|
||||
sandbox_full_access_cfg.derive_sandbox_policy(
|
||||
sandbox_mode_override,
|
||||
None,
|
||||
&PathBuf::from("/tmp/test")
|
||||
)
|
||||
);
|
||||
|
||||
let sandbox_read_only = r#"
|
||||
@@ -1619,8 +1625,11 @@ 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, &PathBuf::from("/tmp/test"))
|
||||
sandbox_read_only_cfg.derive_sandbox_policy(
|
||||
sandbox_mode_override,
|
||||
None,
|
||||
&PathBuf::from("/tmp/test")
|
||||
)
|
||||
);
|
||||
|
||||
let sandbox_workspace_write = r#"
|
||||
@@ -1644,8 +1653,11 @@ exclude_slash_tmp = true
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
},
|
||||
sandbox_workspace_write_cfg
|
||||
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
|
||||
sandbox_workspace_write_cfg.derive_sandbox_policy(
|
||||
sandbox_mode_override,
|
||||
None,
|
||||
&PathBuf::from("/tmp/test")
|
||||
)
|
||||
);
|
||||
|
||||
let sandbox_workspace_write = r#"
|
||||
@@ -1672,8 +1684,11 @@ trust_level = "trusted"
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
},
|
||||
sandbox_workspace_write_cfg
|
||||
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
|
||||
sandbox_workspace_write_cfg.derive_sandbox_policy(
|
||||
sandbox_mode_override,
|
||||
None,
|
||||
&PathBuf::from("/tmp/test")
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1765,6 +1780,75 @@ 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()?;
|
||||
|
||||
@@ -4,6 +4,7 @@ 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
|
||||
@@ -15,6 +16,7 @@ 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>,
|
||||
|
||||
@@ -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")]
|
||||
#[error("turn aborted. Something went wrong? Hit `/feedback` to report the issue.")]
|
||||
TurnAborted {
|
||||
dangling_artifacts: Vec<ProcessedResponseItem>,
|
||||
},
|
||||
@@ -91,7 +91,7 @@ pub enum CodexErr {
|
||||
|
||||
/// Returned by run_command_stream when the user pressed Ctrl‑C (SIGINT). Session uses this to
|
||||
/// surface a polite FunctionCallOutput back to the model instead of crashing the CLI.
|
||||
#[error("interrupted (Ctrl-C)")]
|
||||
#[error("interrupted (Ctrl-C). Something went wrong? Hit `/feedback` to report the issue.")]
|
||||
Interrupted,
|
||||
|
||||
/// Unexpected HTTP status code.
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::conversation_history::ConversationHistory;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
@@ -13,6 +14,7 @@ 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();
|
||||
@@ -104,7 +106,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(&items_to_record_in_conversation_history)
|
||||
sess.record_conversation_items(turn_context, &items_to_record_in_conversation_history)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -50,6 +50,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool {
|
||||
| EventMsg::AgentReasoningDelta(_)
|
||||
| EventMsg::AgentReasoningRawContentDelta(_)
|
||||
| EventMsg::AgentReasoningSectionBreak(_)
|
||||
| EventMsg::RawResponseItem(_)
|
||||
| EventMsg::SessionConfigured(_)
|
||||
| EventMsg::McpToolCallBegin(_)
|
||||
| EventMsg::McpToolCallEnd(_)
|
||||
|
||||
@@ -519,6 +519,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
|
||||
EventMsg::AgentReasoningRawContentDelta(_) => {}
|
||||
EventMsg::ItemStarted(_) => {}
|
||||
EventMsg::ItemCompleted(_) => {}
|
||||
EventMsg::RawResponseItem(_) => {}
|
||||
}
|
||||
CodexStatus::Running
|
||||
}
|
||||
|
||||
@@ -167,8 +167,17 @@ impl CodexLogSnapshot {
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
pub fn upload_to_sentry(&self) -> Result<()> {
|
||||
/// 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<()> {
|
||||
use std::collections::BTreeMap;
|
||||
use std::fs;
|
||||
use std::str::FromStr;
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -182,36 +191,90 @@ 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 tags = BTreeMap::from([(String::from("thread_id"), self.thread_id.to_string())]);
|
||||
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 event = Event {
|
||||
level: Level::Error,
|
||||
message: Some("Codex Log Upload ".to_string() + &self.thread_id),
|
||||
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()),
|
||||
tags,
|
||||
..Default::default()
|
||||
};
|
||||
let mut envelope = Envelope::new();
|
||||
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()
|
||||
}]);
|
||||
}
|
||||
envelope.add_item(EnvelopeItem::Event(event));
|
||||
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 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,
|
||||
}));
|
||||
}
|
||||
|
||||
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(),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
@@ -285,6 +285,7 @@ async fn run_codex_tool_session_inner(
|
||||
| EventMsg::UserMessage(_)
|
||||
| EventMsg::ShutdownComplete
|
||||
| EventMsg::ViewImageToolCall(_)
|
||||
| EventMsg::RawResponseItem(_)
|
||||
| EventMsg::EnteredReviewMode(_)
|
||||
| EventMsg::ItemStarted(_)
|
||||
| EventMsg::ItemCompleted(_)
|
||||
|
||||
@@ -527,6 +527,8 @@ pub enum EventMsg {
|
||||
/// Exited review mode with an optional final result to apply.
|
||||
ExitedReviewMode(ExitedReviewModeEvent),
|
||||
|
||||
RawResponseItem(ResponseItem),
|
||||
|
||||
ItemStarted(ItemStartedEvent),
|
||||
ItemCompleted(ItemCompletedEvent),
|
||||
}
|
||||
|
||||
@@ -360,6 +360,15 @@ 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)
|
||||
|
||||
@@ -101,4 +101,23 @@ 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,
|
||||
}
|
||||
|
||||
@@ -52,6 +52,7 @@ 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;
|
||||
@@ -61,6 +62,34 @@ 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 {
|
||||
@@ -275,11 +304,11 @@ impl ChatComposer {
|
||||
return false;
|
||||
};
|
||||
|
||||
match image::image_dimensions(&path_buf) {
|
||||
Ok((w, h)) => {
|
||||
match Self::resolve_image_path_with_fallback(path_buf) {
|
||||
Ok((resolved_path, w, h)) => {
|
||||
tracing::info!("OK: {pasted}");
|
||||
let format_label = pasted_image_format(&path_buf).label();
|
||||
self.attach_image(path_buf, w, h, format_label);
|
||||
let format_label = pasted_image_format(&resolved_path).label();
|
||||
self.attach_image(resolved_path, w, h, format_label);
|
||||
true
|
||||
}
|
||||
Err(err) => {
|
||||
@@ -289,6 +318,34 @@ 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;
|
||||
@@ -3448,4 +3505,20 @@ 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());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,165 +1,448 @@
|
||||
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 std::cell::RefCell;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::style::Stylize;
|
||||
use ratatui::text::Line;
|
||||
use std::path::PathBuf;
|
||||
use ratatui::text::Span;
|
||||
use ratatui::widgets::Clear;
|
||||
use ratatui::widgets::Paragraph;
|
||||
use ratatui::widgets::StatefulWidgetRef;
|
||||
use ratatui::widgets::Widget;
|
||||
|
||||
use super::BottomPane;
|
||||
use super::SelectionAction;
|
||||
use super::SelectionItem;
|
||||
use super::SelectionViewParams;
|
||||
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;
|
||||
|
||||
const BASE_ISSUE_URL: &str = "https://github.com/openai/codex/issues/new?template=2-bug-report.yml";
|
||||
|
||||
pub(crate) struct FeedbackView;
|
||||
/// 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,
|
||||
|
||||
impl FeedbackView {
|
||||
pub fn show(
|
||||
bottom_pane: &mut BottomPane,
|
||||
file_path: PathBuf,
|
||||
// UI state
|
||||
textarea: TextArea,
|
||||
textarea_state: RefCell<TextAreaState>,
|
||||
complete: bool,
|
||||
}
|
||||
|
||||
impl FeedbackNoteView {
|
||||
pub(crate) fn new(
|
||||
category: FeedbackCategory,
|
||||
snapshot: codex_feedback::CodexLogSnapshot,
|
||||
) {
|
||||
bottom_pane.show_selection_view(Self::selection_params(file_path, snapshot));
|
||||
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,
|
||||
}
|
||||
}
|
||||
|
||||
fn selection_params(
|
||||
file_path: PathBuf,
|
||||
snapshot: codex_feedback::CodexLogSnapshot,
|
||||
) -> SelectionViewParams {
|
||||
let header = FeedbackHeader::new(file_path);
|
||||
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);
|
||||
|
||||
let thread_id = snapshot.thread_id.clone();
|
||||
let cli_version = crate::version::CODEX_CLI_VERSION;
|
||||
let mut thread_id = self.snapshot.thread_id.clone();
|
||||
|
||||
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:",
|
||||
),
|
||||
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:"
|
||||
)),
|
||||
"".into(),
|
||||
Line::from(vec![" ".into(), issue_url.cyan().underlined()]),
|
||||
"".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}")),
|
||||
)));
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
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 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(),
|
||||
Line::from(vec![
|
||||
" Or mention your thread ID ".into(),
|
||||
std::mem::take(&mut thread_id).bold(),
|
||||
" in an existing issue.".into(),
|
||||
]),
|
||||
]),
|
||||
]),
|
||||
)));
|
||||
});
|
||||
)));
|
||||
}
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
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 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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
struct FeedbackHeader {
|
||||
file_path: PathBuf,
|
||||
}
|
||||
|
||||
impl FeedbackHeader {
|
||||
fn new(file_path: PathBuf) -> Self {
|
||||
Self { file_path }
|
||||
fn on_ctrl_c(&mut self) -> CancellationEvent {
|
||||
self.complete = true;
|
||||
CancellationEvent::Handled
|
||||
}
|
||||
|
||||
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 they’re uploaded at:".into(),
|
||||
]),
|
||||
Line::from(self.file_path.display().to_string().dim()),
|
||||
"".into(),
|
||||
]
|
||||
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 state = *self.textarea_state.borrow();
|
||||
self.textarea.cursor_pos_with_state(textarea_rect, state)
|
||||
}
|
||||
}
|
||||
|
||||
impl Renderable for FeedbackHeader {
|
||||
impl Renderable for FeedbackNoteView {
|
||||
fn desired_height(&self, width: u16) -> u16 {
|
||||
1u16 + self.input_height(width) + 3u16
|
||||
}
|
||||
|
||||
fn render(&self, area: Rect, buf: &mut Buffer) {
|
||||
if area.width == 0 || area.height == 0 {
|
||||
if area.height == 0 || area.width == 0 {
|
||||
return;
|
||||
}
|
||||
|
||||
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 (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,
|
||||
);
|
||||
}
|
||||
let line_area = Rect::new(area.x, y, area.width, 1).intersection(area);
|
||||
line.render(line_area, buf);
|
||||
|
||||
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,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn desired_height(&self, width: u16) -> u16 {
|
||||
self.lines()
|
||||
.iter()
|
||||
.map(|line| line.desired_height(width))
|
||||
.sum()
|
||||
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, high‑quality, 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()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -167,22 +450,19 @@ impl Renderable for FeedbackHeader {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::app_event::AppEvent;
|
||||
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;
|
||||
use crate::app_event_sender::AppEventSender;
|
||||
|
||||
fn buffer_to_string(buffer: &Buffer) -> String {
|
||||
(0..buffer.area.height)
|
||||
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)
|
||||
.map(|row| {
|
||||
let mut line = String::new();
|
||||
for col in 0..buffer.area.width {
|
||||
let symbol = buffer[(buffer.area.x + col, buffer.area.y + row)].symbol();
|
||||
for col in 0..area.width {
|
||||
let symbol = buf[(area.x + col, area.y + row)].symbol();
|
||||
if symbol.is_empty() {
|
||||
line.push(' ');
|
||||
} else {
|
||||
@@ -191,34 +471,49 @@ mod tests {
|
||||
}
|
||||
line.trim_end().to_string()
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
.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)
|
||||
}
|
||||
|
||||
#[test]
|
||||
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");
|
||||
fn feedback_view_bad_result() {
|
||||
let view = make_view(FeedbackCategory::BadResult);
|
||||
let rendered = render(&view, 60);
|
||||
insta::assert_snapshot!("feedback_view_bad_result", rendered);
|
||||
}
|
||||
|
||||
let params = FeedbackView::selection_params(file_path.clone(), snapshot);
|
||||
let view = ListSelectionView::new(params, app_event_tx);
|
||||
#[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 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_bug() {
|
||||
let view = make_view(FeedbackCategory::Bug);
|
||||
let rendered = render(&view, 60);
|
||||
insta::assert_snapshot!("feedback_view_bug", 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);
|
||||
#[test]
|
||||
fn feedback_view_other() {
|
||||
let view = make_view(FeedbackCategory::Other);
|
||||
let rendered = render(&view, 60);
|
||||
insta::assert_snapshot!("feedback_view_other", rendered);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -28,12 +28,14 @@ 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::FeedbackView;
|
||||
pub(crate) use feedback_view::FeedbackNoteView;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum CancellationEvent {
|
||||
|
||||
@@ -0,0 +1,9 @@
|
||||
---
|
||||
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
|
||||
@@ -0,0 +1,9 @@
|
||||
---
|
||||
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
|
||||
@@ -0,0 +1,9 @@
|
||||
---
|
||||
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
|
||||
@@ -0,0 +1,9 @@
|
||||
---
|
||||
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
|
||||
@@ -276,6 +276,8 @@ 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 {
|
||||
@@ -322,6 +324,7 @@ 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);
|
||||
@@ -343,6 +346,39 @@ 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.
|
||||
@@ -496,7 +532,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".to_owned(),
|
||||
"Conversation interrupted - tell the model what to do differently. Something went wrong? Hit `/feedback` to report the issue.".to_owned(),
|
||||
));
|
||||
}
|
||||
|
||||
@@ -958,6 +994,7 @@ impl ChatWidget {
|
||||
needs_final_message_separator: false,
|
||||
last_rendered_width: std::cell::Cell::new(None),
|
||||
feedback,
|
||||
current_rollout_path: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1025,6 +1062,7 @@ impl ChatWidget {
|
||||
needs_final_message_separator: false,
|
||||
last_rendered_width: std::cell::Cell::new(None),
|
||||
feedback,
|
||||
current_rollout_path: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1129,23 +1167,11 @@ impl ChatWidget {
|
||||
}
|
||||
match cmd {
|
||||
SlashCommand::Feedback => {
|
||||
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();
|
||||
}
|
||||
}
|
||||
// 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();
|
||||
}
|
||||
SlashCommand::New => {
|
||||
self.app_event_tx.send(AppEvent::NewSession);
|
||||
@@ -1498,7 +1524,9 @@ impl ChatWidget {
|
||||
self.on_entered_review_mode(review_request)
|
||||
}
|
||||
EventMsg::ExitedReviewMode(review) => self.on_exited_review_mode(review),
|
||||
EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) => {}
|
||||
EventMsg::RawResponseItem(_)
|
||||
| EventMsg::ItemStarted(_)
|
||||
| EventMsg::ItemCompleted(_) => {}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
---
|
||||
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, high‑quality, or delightful result worth
|
||||
celebrating.
|
||||
4. other Slowness, feature suggestion, UX feedback, or anything else.
|
||||
@@ -0,0 +1,14 @@
|
||||
---
|
||||
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
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
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.
|
||||
@@ -299,6 +299,7 @@ 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)
|
||||
}
|
||||
@@ -998,6 +999,37 @@ 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]
|
||||
@@ -1175,6 +1207,28 @@ 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();
|
||||
|
||||
@@ -187,12 +187,36 @@ 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 {
|
||||
return parts.into_iter().next().map(PathBuf::from);
|
||||
let mut path = parts.into_iter().next()?;
|
||||
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
path = fixup_unix_root_relative_path(path);
|
||||
}
|
||||
|
||||
return Some(PathBuf::from(path));
|
||||
}
|
||||
|
||||
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
|
||||
@@ -255,6 +279,25 @@ 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!(
|
||||
|
||||
@@ -1047,7 +1047,10 @@ pub(crate) fn new_mcp_tools_output(
|
||||
return PlainHistoryCell { lines };
|
||||
}
|
||||
|
||||
for (server, cfg) in config.mcp_servers.iter() {
|
||||
let mut servers: Vec<_> = config.mcp_servers.iter().collect();
|
||||
servers.sort_by(|(a, _), (b, _)| a.cmp(b));
|
||||
|
||||
for (server, cfg) in servers {
|
||||
let prefix = format!("mcp__{server}__");
|
||||
let mut names: Vec<String> = tools
|
||||
.keys()
|
||||
@@ -1111,7 +1114,7 @@ pub(crate) fn new_mcp_tools_output(
|
||||
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
||||
let display = pairs
|
||||
.into_iter()
|
||||
.map(|(name, value)| format!("{name}={value}"))
|
||||
.map(|(name, _)| format!("{name}=*****"))
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ");
|
||||
lines.push(vec![" • HTTP headers: ".into(), display.into()].into());
|
||||
@@ -1123,7 +1126,7 @@ pub(crate) fn new_mcp_tools_output(
|
||||
pairs.sort_by(|(a, _), (b, _)| a.cmp(b));
|
||||
let display = pairs
|
||||
.into_iter()
|
||||
.map(|(name, env_var)| format!("{name}={env_var}"))
|
||||
.map(|(name, var)| format!("{name}={var}"))
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ");
|
||||
lines.push(vec![" • Env HTTP headers: ".into(), display.into()].into());
|
||||
@@ -1415,14 +1418,20 @@ 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(
|
||||
@@ -1449,6 +1458,91 @@ 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);
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
---
|
||||
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)
|
||||
@@ -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 = "<token>"
|
||||
bearer_token_env_var = "ENV_VAR"
|
||||
# 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.
|
||||
|
||||
@@ -3,7 +3,7 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import { codexExecSpy } from "./codexExecSpy";
|
||||
import { describe, expect, it } from "@jest/globals";
|
||||
import { describe, expect, it, xit } from "@jest/globals";
|
||||
|
||||
import { Codex } from "../src/codex";
|
||||
|
||||
@@ -308,7 +308,8 @@ describe("Codex", () => {
|
||||
await close();
|
||||
}
|
||||
});
|
||||
it("forwards images to exec", async () => {
|
||||
// TODO(pakrym): unskip the test
|
||||
xit("forwards images to exec", async () => {
|
||||
const { url, close } = await startResponsesTestProxy({
|
||||
statusCode: 200,
|
||||
responseBodies: [
|
||||
|
||||
Reference in New Issue
Block a user