diff --git a/.github/actions/codex/bun.lock b/.github/actions/codex/bun.lock index 82e12cc4b6..15b55fafa7 100644 --- a/.github/actions/codex/bun.lock +++ b/.github/actions/codex/bun.lock @@ -8,8 +8,8 @@ "@actions/github": "^6.0.1", }, "devDependencies": { - "@types/bun": "^1.2.19", - "@types/node": "^24.1.0", + "@types/bun": "^1.2.20", + "@types/node": "^24.2.1", "prettier": "^3.6.2", "typescript": "^5.9.2", }, @@ -48,15 +48,15 @@ "@octokit/types": ["@octokit/types@13.10.0", "", { "dependencies": { "@octokit/openapi-types": "^24.2.0" } }, "sha512-ifLaO34EbbPj0Xgro4G5lP5asESjwHracYJvVaPIyXMuiuXLlhic3S47cBdTb+jfODkTE5YtGCLt3Ay3+J97sA=="], - "@types/bun": ["@types/bun@1.2.19", "", { "dependencies": { "bun-types": "1.2.19" } }, "sha512-d9ZCmrH3CJ2uYKXQIUuZ/pUnTqIvLDS0SK7pFmbx8ma+ziH/FRMoAq5bYpRG7y+w1gl+HgyNZbtqgMq4W4e2Lg=="], + "@types/bun": ["@types/bun@1.2.20", "", { "dependencies": { "bun-types": "1.2.20" } }, "sha512-dX3RGzQ8+KgmMw7CsW4xT5ITBSCrSbfHc36SNT31EOUg/LA9JWq0VDdEXDRSe1InVWpd2yLUM1FUF/kEOyTzYA=="], - "@types/node": ["@types/node@24.1.0", "", { "dependencies": { "undici-types": "~7.8.0" } }, "sha512-ut5FthK5moxFKH2T1CUOC6ctR67rQRvvHdFLCD2Ql6KXmMuCrjsSsRI9UsLCm9M18BMwClv4pn327UvB7eeO1w=="], + "@types/node": ["@types/node@24.2.1", "", { "dependencies": { "undici-types": "~7.10.0" } }, "sha512-DRh5K+ka5eJic8CjH7td8QpYEV6Zo10gfRkjHCO3weqZHWDtAaSTFtl4+VMqOJ4N5jcuhZ9/l+yy8rVgw7BQeQ=="], "@types/react": ["@types/react@19.1.8", "", { "dependencies": { "csstype": "^3.0.2" } }, "sha512-AwAfQ2Wa5bCx9WP8nZL2uMZWod7J7/JSplxbTmBQ5ms6QpqNYm672H0Vu9ZVKVngQ+ii4R/byguVEUZQyeg44g=="], "before-after-hook": ["before-after-hook@2.2.3", "", {}, "sha512-NzUnlZexiaH/46WDhANlyR2bXRopNg4F/zuSA3OpZnllCUgRaOF2znDioDWrmbNVsuZk6l9pMquQB38cfBZwkQ=="], - "bun-types": ["bun-types@1.2.19", "", { "dependencies": { "@types/node": "*" }, "peerDependencies": { "@types/react": "^19" } }, "sha512-uAOTaZSPuYsWIXRpj7o56Let0g/wjihKCkeRqUBhlLVM/Bt+Fj9xTo+LhC1OV1XDaGkz4hNC80et5xgy+9KTHQ=="], + "bun-types": ["bun-types@1.2.20", "", { "dependencies": { "@types/node": "*" }, "peerDependencies": { "@types/react": "^19" } }, "sha512-pxTnQYOrKvdOwyiyd/7sMt9yFOenN004Y6O4lCcCUoKVej48FS5cvTw9geRaEcB9TsDZaJKAxPTVvi8tFsVuXA=="], "csstype": ["csstype@3.1.3", "", {}, "sha512-M1uQkMl8rQK/szD0LNhtqxIPLpimGm8sOBwU7lLnCpSbTyY3yeU1Vc7l4KT5zT4s/yOxHH5O7tIuuLOCnLADRw=="], @@ -72,7 +72,7 @@ "undici": ["undici@5.29.0", "", { "dependencies": { "@fastify/busboy": "^2.0.0" } }, "sha512-raqeBD6NQK4SkWhQzeYKd1KmIG6dllBOTt55Rmkt4HtI9mwdWtJljnrXjAFUBLTSN67HWrOIZ3EPF4kjUw80Bg=="], - "undici-types": ["undici-types@7.8.0", "", {}, "sha512-9UJ2xGDvQ43tYyVMpuHlsgApydB8ZKfVYTsLDhXkFL/6gfkp+U8xTGdh8pMJv1SpZna0zxG1DwsKZsreLbXBxw=="], + "undici-types": ["undici-types@7.10.0", "", {}, "sha512-t5Fy/nfn+14LuOc2KNYg75vZqClpAiqscVvMygNnlsHBFpSXdJaYtXMcdNLpl/Qvc3P2cB3s6lOV51nqsFq4ag=="], "universal-user-agent": ["universal-user-agent@6.0.1", "", {}, "sha512-yCzhz6FN2wU1NiiQRogkTQszlQSlpWaw8SvVegAc+bDxbzHgh1vX8uIe8OYyMH6DwH+sdTJsgMl36+mSMdRJIQ=="], @@ -82,8 +82,6 @@ "@octokit/plugin-rest-endpoint-methods/@octokit/types": ["@octokit/types@12.6.0", "", { "dependencies": { "@octokit/openapi-types": "^20.0.0" } }, "sha512-1rhSOfRa6H9w4YwK0yrf5faDaDTb+yLyBUKOCV4xtCDB5VmIPqd/v9yr9o6SAzOAlRxMiRiCic6JVM1/kunVkw=="], - "bun-types/@types/node": ["@types/node@24.0.13", "", { "dependencies": { "undici-types": "~7.8.0" } }, "sha512-Qm9OYVOFHFYg3wJoTSrz80hoec5Lia/dPp84do3X7dZvLikQvM1YpmvTBEdIr/e+U8HTkFjLHLnl78K/qjf+jQ=="], - "@octokit/plugin-paginate-rest/@octokit/types/@octokit/openapi-types": ["@octokit/openapi-types@20.0.0", "", {}, "sha512-EtqRBEjp1dL/15V7WiX5LJMIxxkdiGJnabzYx5Apx4FkQIFgAfKumXeYAqqJCj1s+BMX4cPFIFC4OLCR6stlnA=="], "@octokit/plugin-rest-endpoint-methods/@octokit/types/@octokit/openapi-types": ["@octokit/openapi-types@20.0.0", "", {}, "sha512-EtqRBEjp1dL/15V7WiX5LJMIxxkdiGJnabzYx5Apx4FkQIFgAfKumXeYAqqJCj1s+BMX4cPFIFC4OLCR6stlnA=="], diff --git a/.github/actions/codex/package.json b/.github/actions/codex/package.json index 6c7ae9002b..208d9fef78 100644 --- a/.github/actions/codex/package.json +++ b/.github/actions/codex/package.json @@ -13,8 +13,8 @@ "@actions/github": "^6.0.1" }, "devDependencies": { - "@types/bun": "^1.2.19", - "@types/node": "^24.1.0", + "@types/bun": "^1.2.20", + "@types/node": "^24.2.1", "prettier": "^3.6.2", "typescript": "^5.9.2" } diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 5dbc2421fa..9a90df712f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -520,9 +520,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.41" +version = "4.5.43" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be92d32e80243a54711e5d7ce823c35c41c9d929dc4ab58e1276f625841aadf9" +checksum = "50fd97c9dc2399518aa331917ac6f274280ec5eb34e555dd291899745c48ec6f" dependencies = [ "clap_builder", "clap_derive", @@ -530,9 +530,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.41" +version = "4.5.43" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "707eab41e9622f9139419d573eca0900137718000c517d47da73045f54331c3d" +checksum = "c35b5830294e1fa0462034af85cc95225a4cb07092c088c55bda3147cfcd8f65" dependencies = [ "anstream", "anstyle", @@ -543,9 +543,9 @@ dependencies = [ [[package]] name = "clap_complete" -version = "4.5.55" +version = "4.5.56" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a5abde44486daf70c5be8b8f8f1b66c49f86236edf6fa2abadb4d961c4c6229a" +checksum = "67e4efcbb5da11a92e8a609233aa1e8a7d91e38de0be865f016d14700d45a7fd" dependencies = [ "clap", ] @@ -661,7 +661,7 @@ dependencies = [ "clap", "codex-core", "serde", - "toml 0.9.4", + "toml 0.9.5", ] [[package]] @@ -707,7 +707,7 @@ dependencies = [ "tokio", "tokio-test", "tokio-util", - "toml 0.9.4", + "toml 0.9.5", "toml_edit 0.23.3", "tracing", "tree-sitter", @@ -836,7 +836,7 @@ dependencies = [ "tempfile", "tokio", "tokio-test", - "toml 0.9.4", + "toml 0.9.5", "tracing", "tracing-subscriber", "uuid", @@ -855,11 +855,23 @@ dependencies = [ "serde_json", "tempfile", "tokio", - "toml 0.9.4", + "toml 0.9.5", "tracing", "wiremock", ] +[[package]] +name = "codex-protocol" +version = "0.0.0" +dependencies = [ + "pretty_assertions", + "serde", + "serde_json", + "strum 0.27.2", + "strum_macros 0.27.2", + "uuid", +] + [[package]] name = "codex-tui" version = "0.0.0" @@ -881,6 +893,7 @@ dependencies = [ "image", "insta", "lazy_static", + "libc", "mcp-types", "path-clean", "pretty_assertions", @@ -4806,9 +4819,9 @@ dependencies = [ [[package]] name = "toml" -version = "0.9.4" +version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41ae868b5a0f67631c14589f7e250c1ea2c574ee5ba21c6c8dd4b1485705a5a1" +checksum = "75129e1dc5000bfbaa9fee9d1b21f974f9fbad9daec557a521ee6e080825f6e8" dependencies = [ "indexmap 2.10.0", "serde", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 0ed8852228..2fb9b9271e 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -15,6 +15,7 @@ members = [ "mcp-server", "mcp-types", "ollama", + "protocol", "tui", ] resolver = "2" diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 0f03c3b647..ee527f3b72 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -47,7 +47,7 @@ tokio = { version = "1", features = [ "signal", ] } tokio-util = "0.7.16" -toml = "0.9.4" +toml = "0.9.5" toml_edit = "0.23.3" tracing = { version = "0.1.41", features = ["log"] } tree-sitter = "0.25.8" diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index ad08782b6d..8ab5ad9636 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -169,6 +169,7 @@ impl ModelClient { store, stream: true, include, + prompt_cache_key: Some(self.session_id.to_string()), }; let mut attempt = 0; diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index b37b1e3f80..f05a57b6a2 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -215,6 +215,8 @@ pub(crate) struct ResponsesApiRequest<'a> { pub(crate) store: bool, pub(crate) stream: bool, pub(crate) include: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) prompt_cache_key: Option, } pub(crate) fn create_reasoning_param_for_request( diff --git a/codex-rs/core/tests/prompt_caching.rs b/codex-rs/core/tests/prompt_caching.rs new file mode 100644 index 0000000000..f460fc3004 --- /dev/null +++ b/codex-rs/core/tests/prompt_caching.rs @@ -0,0 +1,137 @@ +#![allow(clippy::expect_used, clippy::unwrap_used)] + +use codex_core::Codex; +use codex_core::CodexSpawnOk; +use codex_core::ModelProviderInfo; +use codex_core::built_in_model_providers; +use codex_core::protocol::EventMsg; +use codex_core::protocol::InputItem; +use codex_core::protocol::Op; +use codex_login::CodexAuth; +use core_test_support::load_default_config_for_test; +use core_test_support::load_sse_fixture_with_id; +use core_test_support::wait_for_event; +use tempfile::TempDir; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; + +/// Build minimal SSE stream with completed marker using the JSON fixture. +fn sse_completed(id: &str) -> String { + load_sse_fixture_with_id("tests/fixtures/completed_template.json", id) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn prefixes_context_and_instructions_once_and_consistently_across_requests() { + #![allow(clippy::unwrap_used)] + use pretty_assertions::assert_eq; + + let server = MockServer::start().await; + + let sse = sse_completed("resp"); + let template = ResponseTemplate::new(200) + .insert_header("content-type", "text/event-stream") + .set_body_raw(sse, "text/event-stream"); + + // Expect two POSTs to /v1/responses + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with(template) + .expect(2) + .mount(&server) + .await; + + let model_provider = ModelProviderInfo { + base_url: Some(format!("{}/v1", server.uri())), + ..built_in_model_providers()["openai"].clone() + }; + + let cwd = TempDir::new().unwrap(); + let codex_home = TempDir::new().unwrap(); + let mut config = load_default_config_for_test(&codex_home); + config.cwd = cwd.path().to_path_buf(); + config.model_provider = model_provider; + config.user_instructions = Some("be consistent and helpful".to_string()); + + let ctrl_c = std::sync::Arc::new(tokio::sync::Notify::new()); + let CodexSpawnOk { codex, .. } = Codex::spawn( + config, + Some(CodexAuth::from_api_key("Test API Key")), + ctrl_c.clone(), + ) + .await + .unwrap(); + + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello 1".into(), + }], + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + codex + .submit(Op::UserInput { + items: vec![InputItem::Text { + text: "hello 2".into(), + }], + }) + .await + .unwrap(); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 2, "expected two POST requests"); + + let expected_env_text = format!( + "\n\nCurrent working directory: {}\nApproval policy: on-request\nSandbox policy: read-only\nNetwork access: restricted\n\n\n", + cwd.path().to_string_lossy() + ); + let expected_ui_text = + "\n\nbe consistent and helpful\n\n"; + + let expected_env_msg = serde_json::json!({ + "type": "message", + "id": serde_json::Value::Null, + "role": "user", + "content": [ { "type": "input_text", "text": expected_env_text } ] + }); + let expected_ui_msg = serde_json::json!({ + "type": "message", + "id": serde_json::Value::Null, + "role": "user", + "content": [ { "type": "input_text", "text": expected_ui_text } ] + }); + + let expected_user_message_1 = serde_json::json!({ + "type": "message", + "id": serde_json::Value::Null, + "role": "user", + "content": [ { "type": "input_text", "text": "hello 1" } ] + }); + let body1 = requests[0].body_json::().unwrap(); + assert_eq!( + body1["input"], + serde_json::json!([expected_env_msg, expected_ui_msg, expected_user_message_1]) + ); + + let expected_user_message_2 = serde_json::json!({ + "type": "message", + "id": serde_json::Value::Null, + "role": "user", + "content": [ { "type": "input_text", "text": "hello 2" } ] + }); + let body2 = requests[1].body_json::().unwrap(); + let expected_body2 = serde_json::json!( + [ + body1["input"].as_array().unwrap().as_slice(), + [expected_user_message_2].as_slice(), + ] + .concat() + ); + assert_eq!(body2["input"], expected_body2); +} diff --git a/codex-rs/login/src/login_with_chatgpt.py b/codex-rs/login/src/login_with_chatgpt.py index ddcc6e66c7..252c4e06ae 100644 --- a/codex-rs/login/src/login_with_chatgpt.py +++ b/codex-rs/login/src/login_with_chatgpt.py @@ -45,11 +45,54 @@ DEFAULT_ISSUER = "https://auth.openai.com" EXIT_CODE_WHEN_ADDRESS_ALREADY_IN_USE = 13 CA_CONTEXT = None -try: - import ssl - import certifi as _certifi +CODEX_LOGIN_TRACE = os.environ.get("CODEX_LOGIN_TRACE", "false") in ["true", "1"] + +try: + + def trace(msg: str) -> None: + if CODEX_LOGIN_TRACE: + print(msg) + + def attempt_request(method: str) -> bool: + try: + with urllib.request.urlopen( + urllib.request.Request( + f"{DEFAULT_ISSUER}/.well-known/openid-configuration", + method="GET", + ), + context=CA_CONTEXT, + ) as resp: + if resp.status != 200: + trace(f"Request using {method} failed: {resp.status}") + return False + + trace(f"Request using {method} succeeded") + return True + except Exception as e: + trace(f"Request using {method} failed: {e}") + return False + + status = attempt_request("default settings") + if not status: + try: + import truststore + + truststore.inject_into_ssl() + status = attempt_request("truststore") + except Exception as e: + trace(f"Failed to use truststore: {e}") + + if not status: + try: + import ssl + import certifi as _certifi + + CA_CONTEXT = ssl.create_default_context(cafile=_certifi.where()) + status = attempt_request("certify") + except Exception as e: + trace(f"Failed to use certify: {e}") + - CA_CONTEXT = ssl.create_default_context(cafile=_certifi.where()) except Exception: pass diff --git a/codex-rs/ollama/Cargo.toml b/codex-rs/ollama/Cargo.toml index ead9a06494..7fa1328385 100644 --- a/codex-rs/ollama/Cargo.toml +++ b/codex-rs/ollama/Cargo.toml @@ -24,7 +24,7 @@ tokio = { version = "1", features = [ "rt-multi-thread", "signal", ] } -toml = "0.9.2" +toml = "0.9.5" tracing = { version = "0.1.41", features = ["log"] } wiremock = "0.6" diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml new file mode 100644 index 0000000000..2f122fe3f7 --- /dev/null +++ b/codex-rs/protocol/Cargo.toml @@ -0,0 +1,21 @@ +[package] +edition = "2024" +name = "codex-protocol" +version = { workspace = true } + +[lib] +name = "codex_protocol" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +serde = { version = "1", features = ["derive"] } +strum = "0.27.2" +strum_macros = "0.27.2" +uuid = { version = "1", features = ["serde", "v4"] } + +[dev-dependencies] +pretty_assertions = "1.4.1" +serde_json = "1" diff --git a/codex-rs/protocol/src/lib.rs b/codex-rs/protocol/src/lib.rs new file mode 100644 index 0000000000..c82dfba556 --- /dev/null +++ b/codex-rs/protocol/src/lib.rs @@ -0,0 +1 @@ +mod notification; diff --git a/codex-rs/protocol/src/notification.rs b/codex-rs/protocol/src/notification.rs new file mode 100644 index 0000000000..f1bbace01a --- /dev/null +++ b/codex-rs/protocol/src/notification.rs @@ -0,0 +1,147 @@ +use serde::Deserialize; +use serde::Serialize; +use strum_macros::Display; +use uuid::Uuid; + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Display)] +#[serde(tag = "method", rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] +pub enum NotificationMessage { + Conversation(ConversationNotification), + ShutdownComplete, +} + +/// Notification associated with a conversation. The `conversation_id` is key +/// so clients can dispatch messages to the correct conversation. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +pub struct ConversationNotification { + conversation_id: Uuid, + #[serde(flatten)] + message: ConversationNotificationMessage, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Display)] +#[serde(tag = "type", rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] +enum ConversationNotificationMessage { + Initialized(ConversationInitialized), +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +struct ConversationInitialized { + model: String, +} + +#[allow(clippy::unwrap_used)] +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use serde::de::Error as _; + use serde_json::json; + + // The idea is that the way we map `NotificationMessage` to an MCP + // notification is to use the tuple type as the `params`. + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] + struct JsonrpcNotification { + jsonrpc: String, + method: String, + params: serde_json::Value, + } + + // Example of how JSON-RPC serialization could work. + fn to_jsonrpc_message( + notification: NotificationMessage, + ) -> Result { + let method = notification.to_string(); + let params = match notification { + NotificationMessage::Conversation(notification) => serde_json::to_value(notification)?, + NotificationMessage::ShutdownComplete => serde_json::Value::Null, + }; + + let jsonrpc_notification = JsonrpcNotification { + jsonrpc: "2.0".to_string(), + method, + params, + }; + + serde_json::to_value(jsonrpc_notification) + } + + fn from_jsonrpc_message( + jsonrpc_notification: JsonrpcNotification, + ) -> Result { + let JsonrpcNotification { + jsonrpc: _, + method, + params, + } = jsonrpc_notification; + + match method.as_str() { + "conversation" => { + let conversation_notification: ConversationNotification = + serde_json::from_value(params)?; + Ok(NotificationMessage::Conversation(conversation_notification)) + } + "shutdown_complete" => Ok(NotificationMessage::ShutdownComplete), + _ => Err(serde_json::Error::custom(format!( + "Unknown method: {method}" + ))), + } + } + + #[test] + fn test_serialize_notification_message_conversation() { + let conversation_id = Uuid::parse_str("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let message = NotificationMessage::Conversation(ConversationNotification { + conversation_id, + message: ConversationNotificationMessage::Initialized(ConversationInitialized { + model: "gpt-5".to_string(), + }), + }); + assert_eq!( + json!({ + "method": "conversation", + "conversation_id": conversation_id.to_string(), + "type": "initialized", + "model": "gpt-5", + }), + serde_json::to_value(message.clone()).unwrap() + ); + + let expected_jsonrpc_message = json!({ + "jsonrpc": "2.0", + "method": "conversation", + "params": { + "conversation_id": conversation_id.to_string(), + "type": "initialized", + "model": "gpt-5", + } + }); + assert_eq!( + expected_jsonrpc_message, + to_jsonrpc_message(message.clone()).unwrap() + ); + + let serialized_json_rpc_message = serde_json::to_string(&expected_jsonrpc_message).unwrap(); + let deserialized_json_rpc_message = + serde_json::from_str::(&serialized_json_rpc_message).unwrap(); + assert_eq!( + JsonrpcNotification { + jsonrpc: "2.0".to_string(), + method: "conversation".to_string(), + params: json!({ + "conversation_id": conversation_id.to_string(), + "type": "initialized", + "model": "gpt-5", + }), + }, + deserialized_json_rpc_message + ); + + assert_eq!( + message, + from_jsonrpc_message(deserialized_json_rpc_message).unwrap() + ); + } +} diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 719c631149..31f198f543 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -72,6 +72,9 @@ unicode-segmentation = "1.12.0" unicode-width = "0.1" uuid = "1" +[target.'cfg(unix)'.dependencies] +libc = "0.2" + [dev-dependencies] chrono = { version = "0.4", features = ["serde"] } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index e3290e10a8..ebce2699e8 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -255,9 +255,11 @@ impl App<'_> { kind: KeyEventKind::Press, .. } => { - if let AppState::Chat { widget } = &mut self.app_state { - widget.on_ctrl_z(); + #[cfg(unix)] + { + self.suspend(terminal)?; } + // No-op on non-Unix platforms. } KeyEvent { code: KeyCode::Char('d'), @@ -459,6 +461,23 @@ impl App<'_> { Ok(()) } + #[cfg(unix)] + fn suspend(&mut self, terminal: &mut tui::Tui) -> Result<()> { + tui::restore()?; + // SAFETY: Unix-only code path. We intentionally send SIGTSTP to the + // current process group (pid 0) to trigger standard job-control + // suspension semantics. This FFI does not involve any raw pointers, + // is not called from a signal handler, and uses a constant signal. + // Errors from kill are acceptable (e.g., if already stopped) — the + // subsequent re-init path will still leave the terminal in a good state. + // We considered `nix`, but didn't think it was worth pulling in for this one call. + unsafe { libc::kill(0, libc::SIGTSTP) }; + *terminal = tui::init(&self.config)?; + terminal.clear()?; + self.app_event_tx.send(AppEvent::RequestRedraw); + Ok(()) + } + pub(crate) fn token_usage(&self) -> codex_core::protocol::TokenUsage { match &self.app_state { AppState::Chat { widget } => widget.token_usage().clone(), diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 173ab64af2..3d312ffce0 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -46,6 +46,7 @@ use crate::bottom_pane::BottomPane; use crate::bottom_pane::BottomPaneParams; use crate::bottom_pane::CancellationEvent; use crate::bottom_pane::InputResult; +use crate::common::DEFAULT_WRAP_COLS; use crate::history_cell::CommandOutput; use crate::history_cell::ExecCell; use crate::history_cell::HistoryCell; @@ -223,7 +224,7 @@ impl ChatWidget<'_> { content_buffer: String::new(), answer_buffer: String::new(), running_commands: HashMap::new(), - live_builder: RowBuilder::new(80), + live_builder: RowBuilder::new(DEFAULT_WRAP_COLS.into()), current_stream: None, stream_header_emitted: false, live_max_rows: 3, @@ -651,10 +652,6 @@ impl ChatWidget<'_> { } } - pub(crate) fn on_ctrl_z(&mut self) { - self.interrupt_running_task(); - } - pub(crate) fn composer_is_empty(&self) -> bool { self.bottom_pane.composer_is_empty() } diff --git a/codex-rs/tui/src/common.rs b/codex-rs/tui/src/common.rs new file mode 100644 index 0000000000..2c19b58706 --- /dev/null +++ b/codex-rs/tui/src/common.rs @@ -0,0 +1 @@ +pub(crate) const DEFAULT_WRAP_COLS: u16 = 80; diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs index f536681732..ada84b89d6 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -1,3 +1,4 @@ +use crossterm::terminal; use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; @@ -6,36 +7,44 @@ use ratatui::text::Span as RtSpan; use std::collections::HashMap; use std::path::PathBuf; +use crate::common::DEFAULT_WRAP_COLS; use codex_core::protocol::FileChange; -struct FileSummary { - display_path: String, - added: usize, - removed: usize, +use crate::history_cell::PatchEventType; + +const SPACES_AFTER_LINE_NUMBER: usize = 6; + +// Internal representation for diff line rendering +enum DiffLineType { + Insert, + Delete, + Context, } pub(crate) fn create_diff_summary( title: &str, - changes: HashMap, + changes: &HashMap, + event_type: PatchEventType, ) -> Vec> { - let mut files: Vec = Vec::new(); + struct FileSummary { + display_path: String, + added: usize, + removed: usize, + } - // Count additions/deletions from a unified diff body let count_from_unified = |diff: &str| -> (usize, usize) { if let Ok(patch) = diffy::Patch::from_str(diff) { - let mut adds = 0usize; - let mut dels = 0usize; - for hunk in patch.hunks() { - for line in hunk.lines() { - match line { - diffy::Line::Insert(_) => adds += 1, - diffy::Line::Delete(_) => dels += 1, - _ => {} - } - } - } - (adds, dels) + patch + .hunks() + .iter() + .flat_map(|h| h.lines()) + .fold((0, 0), |(a, d), l| match l { + diffy::Line::Insert(_) => (a + 1, d), + diffy::Line::Delete(_) => (a, d + 1), + _ => (a, d), + }) } else { + // Fallback: manual scan to preserve counts even for unparsable diffs let mut adds = 0usize; let mut dels = 0usize; for l in diff.lines() { @@ -52,29 +61,23 @@ pub(crate) fn create_diff_summary( } }; - for (path, change) in &changes { - use codex_core::protocol::FileChange::*; + let mut files: Vec = Vec::new(); + for (path, change) in changes.iter() { match change { - Add { content } => { - let added = content.lines().count(); - files.push(FileSummary { - display_path: path.display().to_string(), - added, - removed: 0, - }); - } - Delete => { - let removed = std::fs::read_to_string(path) + FileChange::Add { content } => files.push(FileSummary { + display_path: path.display().to_string(), + added: content.lines().count(), + removed: 0, + }), + FileChange::Delete => files.push(FileSummary { + display_path: path.display().to_string(), + added: 0, + removed: std::fs::read_to_string(path) .ok() .map(|s| s.lines().count()) - .unwrap_or(0); - files.push(FileSummary { - display_path: path.display().to_string(), - added: 0, - removed, - }); - } - Update { + .unwrap_or(0), + }), + FileChange::Update { unified_diff, move_path, } => { @@ -142,11 +145,278 @@ pub(crate) fn create_diff_summary( let mut line = RtLine::from(spans); let prefix = if idx == 0 { " ⎿ " } else { " " }; line.spans.insert(0, prefix.into()); - line.spans.iter_mut().for_each(|span| { - span.style = span.style.add_modifier(Modifier::DIM); - }); + line.spans + .iter_mut() + .for_each(|span| span.style = span.style.add_modifier(Modifier::DIM)); out.push(line); } + let show_details = matches!( + event_type, + PatchEventType::ApplyBegin { + auto_approved: true + } | PatchEventType::ApprovalRequest + ); + + if show_details { + out.extend(render_patch_details(changes)); + } + out } + +fn render_patch_details(changes: &HashMap) -> Vec> { + let mut out: Vec> = Vec::new(); + let term_cols: usize = terminal::size() + .map(|(w, _)| w as usize) + .unwrap_or(DEFAULT_WRAP_COLS.into()); + + for (index, (path, change)) in changes.iter().enumerate() { + let is_first_file = index == 0; + // Add separator only between files (not at the very start) + if !is_first_file { + out.push(RtLine::from(vec![ + RtSpan::raw(" "), + RtSpan::styled("...", style_dim()), + ])); + } + match change { + FileChange::Add { content } => { + for (i, raw) in content.lines().enumerate() { + let ln = i + 1; + out.extend(push_wrapped_diff_line( + ln, + DiffLineType::Insert, + raw, + term_cols, + )); + } + } + FileChange::Delete => { + let original = std::fs::read_to_string(path).unwrap_or_default(); + for (i, raw) in original.lines().enumerate() { + let ln = i + 1; + out.extend(push_wrapped_diff_line( + ln, + DiffLineType::Delete, + raw, + term_cols, + )); + } + } + FileChange::Update { + unified_diff, + move_path: _, + } => { + if let Ok(patch) = diffy::Patch::from_str(unified_diff) { + for h in patch.hunks() { + let mut old_ln = h.old_range().start(); + let mut new_ln = h.new_range().start(); + for l in h.lines() { + match l { + diffy::Line::Insert(text) => { + let s = text.trim_end_matches('\n'); + out.extend(push_wrapped_diff_line( + new_ln, + DiffLineType::Insert, + s, + term_cols, + )); + new_ln += 1; + } + diffy::Line::Delete(text) => { + let s = text.trim_end_matches('\n'); + out.extend(push_wrapped_diff_line( + old_ln, + DiffLineType::Delete, + s, + term_cols, + )); + old_ln += 1; + } + diffy::Line::Context(text) => { + let s = text.trim_end_matches('\n'); + out.extend(push_wrapped_diff_line( + new_ln, + DiffLineType::Context, + s, + term_cols, + )); + old_ln += 1; + new_ln += 1; + } + } + } + } + } + } + } + + out.push(RtLine::from(RtSpan::raw(""))); + } + + out +} + +fn push_wrapped_diff_line( + line_number: usize, + kind: DiffLineType, + text: &str, + term_cols: usize, +) -> Vec> { + let indent = " "; + let ln_str = line_number.to_string(); + let mut remaining_text: &str = text; + + // Reserve a fixed number of spaces after the line number so that content starts + // at a consistent column. The sign ("+"/"-") is rendered as part of the content + // with the same background as the edit, not as a separate dimmed column. + let gap_after_ln = SPACES_AFTER_LINE_NUMBER.saturating_sub(ln_str.len()); + let first_prefix_cols = indent.len() + ln_str.len() + gap_after_ln; + let cont_prefix_cols = indent.len() + ln_str.len() + gap_after_ln; + + let mut first = true; + let (sign_opt, bg_style) = match kind { + DiffLineType::Insert => (Some('+'), Some(style_add())), + DiffLineType::Delete => (Some('-'), Some(style_del())), + DiffLineType::Context => (None, None), + }; + let mut lines: Vec> = Vec::new(); + while !remaining_text.is_empty() { + let prefix_cols = if first { + first_prefix_cols + } else { + cont_prefix_cols + }; + // Fit the content for the current terminal row: + // compute how many columns are available after the prefix, then split + // at a UTF-8 character boundary so this row's chunk fits exactly. + let available_content_cols = term_cols.saturating_sub(prefix_cols).max(1); + let split_at_byte_index = remaining_text + .char_indices() + .nth(available_content_cols) + .map(|(i, _)| i) + .unwrap_or_else(|| remaining_text.len()); + let (chunk, rest) = remaining_text.split_at(split_at_byte_index); + remaining_text = rest; + + if first { + let mut spans: Vec> = Vec::new(); + spans.push(RtSpan::raw(indent)); + spans.push(RtSpan::styled(ln_str.clone(), style_dim())); + spans.push(RtSpan::raw(" ".repeat(gap_after_ln))); + + // Prefix the content with the sign if it is an insertion or deletion, and color + // the sign with the same background as the edited text. + let display_chunk = match sign_opt { + Some(sign_char) => format!("{sign_char}{chunk}"), + None => chunk.to_string(), + }; + + let content_span = match bg_style { + Some(style) => RtSpan::styled(display_chunk, style), + None => RtSpan::raw(display_chunk), + }; + spans.push(content_span); + lines.push(RtLine::from(spans)); + first = false; + } else { + let hang_prefix = format!( + "{indent}{}{}", + " ".repeat(ln_str.len()), + " ".repeat(gap_after_ln) + ); + let content_span = match bg_style { + Some(style) => RtSpan::styled(chunk.to_string(), style), + None => RtSpan::raw(chunk.to_string()), + }; + lines.push(RtLine::from(vec![RtSpan::raw(hang_prefix), content_span])); + } + } + lines +} + +fn style_dim() -> Style { + Style::default().add_modifier(Modifier::DIM) +} + +fn style_add() -> Style { + Style::default().bg(Color::Green) +} + +fn style_del() -> Style { + Style::default().bg(Color::Red) +} + +#[allow(clippy::expect_used)] +#[cfg(test)] +mod tests { + use super::*; + use crate::history_cell::HistoryCell; + use crate::text_block::TextBlock; + use insta::assert_snapshot; + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + fn snapshot_lines(name: &str, lines: Vec>, width: u16, height: u16) { + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("terminal"); + let cell = HistoryCell::PendingPatch { + view: TextBlock::new(lines), + }; + terminal + .draw(|f| f.render_widget_ref(&cell, f.area())) + .expect("draw"); + assert_snapshot!(name, terminal.backend()); + } + + #[test] + fn ui_snapshot_add_details() { + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("README.md"), + FileChange::Add { + content: "first line\nsecond line\n".to_string(), + }, + ); + + let lines = + create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest); + + snapshot_lines("add_details", lines, 80, 10); + } + + #[test] + fn ui_snapshot_update_details_with_rename() { + let mut changes: HashMap = HashMap::new(); + + let original = "line one\nline two\nline three\n"; + let modified = "line one\nline two changed\nline three\n"; + let patch = diffy::create_patch(original, modified).to_string(); + + changes.insert( + PathBuf::from("src/lib.rs"), + FileChange::Update { + unified_diff: patch, + move_path: Some(PathBuf::from("src/lib_new.rs")), + }, + ); + + let lines = + create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest); + + snapshot_lines("update_details_with_rename", lines, 80, 12); + } + + #[test] + fn ui_snapshot_wrap_behavior_insert() { + // Narrow width to force wrapping within our diff line rendering + let long_line = "this is a very long line that should wrap across multiple terminal columns and continue"; + + // Call the wrapping function directly so we can precisely control the width + let lines = + push_wrapped_diff_line(1, DiffLineType::Insert, long_line, DEFAULT_WRAP_COLS.into()); + + // Render into a small terminal to capture the visual layout + snapshot_lines("wrap_behavior_insert", lines, DEFAULT_WRAP_COLS + 10, 8); + } +} diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 1236d4b550..b49e59972c 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -335,7 +335,7 @@ impl HistoryCell { let mut lines: Vec = vec![Line::from("⚙︎ Working")]; for (i, parsed) in parsed_commands.iter().enumerate() { - let str = match parsed { + let text = match parsed { ParsedCommand::Read { name, .. } => format!("📖 {name}"), ParsedCommand::ListFiles { cmd, path } => match path { Some(p) => format!("📂 {p}"), @@ -353,11 +353,14 @@ impl HistoryCell { ParsedCommand::Unknown { cmd } => format!("⌨️ {}", shlex_join_safe(cmd)), }; - let prefix = if i == 0 { " L " } else { " " }; - lines.push(Line::from(vec![ - Span::styled(prefix, Style::default().add_modifier(Modifier::DIM)), - Span::styled(str, Style::default().fg(LIGHT_BLUE)), - ])); + let first_prefix = if i == 0 { " L " } else { " " }; + for (j, line_text) in text.lines().enumerate() { + let prefix = if j == 0 { first_prefix } else { " " }; + lines.push(Line::from(vec![ + Span::styled(prefix, Style::default().add_modifier(Modifier::DIM)), + Span::styled(line_text.to_string(), Style::default().fg(LIGHT_BLUE)), + ])); + } } lines.extend(output_lines(output, true, false)); @@ -446,7 +449,7 @@ impl HistoryCell { } pub(crate) fn new_completed_mcp_tool_call( - num_cols: u16, + num_cols: usize, invocation: McpInvocation, duration: Duration, success: bool, @@ -484,7 +487,7 @@ impl HistoryCell { format_and_truncate_tool_result( &text.text, TOOL_CALL_MAX_LINES, - num_cols as usize, + num_cols, ) } mcp_types::ContentBlock::ImageContent(_) => { @@ -845,7 +848,9 @@ impl HistoryCell { } }; - let lines: Vec> = create_diff_summary(title, changes); + let mut lines: Vec> = create_diff_summary(title, &changes, event_type); + + lines.push(Line::from("")); HistoryCell::PendingPatch { view: TextBlock::new(lines), @@ -974,3 +979,19 @@ fn shlex_join_safe(command: &[String]) -> String { Err(_) => command.join(" "), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parsed_command_with_newlines_starts_each_line_at_origin() { + let parsed = vec![ParsedCommand::Unknown { + cmd: vec!["printf".into(), "foo\nbar".into()], + }]; + let lines = HistoryCell::exec_command_lines(&[], &parsed, None); + assert!(lines.len() >= 3); + assert_eq!(lines[1].spans[0].content, " L "); + assert_eq!(lines[2].spans[0].content, " "); + } +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 27c850ca61..8f64f3247d 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -30,6 +30,7 @@ mod chatwidget; mod citation_regex; mod cli; mod colors; +mod common; pub mod custom_terminal; mod diff_render; mod exec_command; diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__add_details.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__add_details.snap new file mode 100644 index 0000000000..06fc8a68e8 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__add_details.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +"proposed patch to 1 file (+2 -0) " +" ⎿ README.md (+2 -0) " +" 1 +first line " +" 2 +second line " +" " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__update_details_with_rename.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__update_details_with_rename.snap new file mode 100644 index 0000000000..0eebe09d73 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__update_details_with_rename.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +"proposed patch to 1 file (+1 -1) " +" ⎿ src/lib.rs → src/lib_new.rs (+1 -1) " +" 1 line one " +" 2 -line two " +" 2 +line two changed " +" 3 line three " +" " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap new file mode 100644 index 0000000000..641552d5a1 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +" 1 +this is a very long line that should wrap across multiple terminal col " +" umns and continue " +" " +" " +" " +" " +" " +" "