Merge branch 'main' into issue-2148-number-format

Author: seratch

.github/workflows/codex.yml

@@ -52,7 +52,7 @@ jobs:
             ~/.cargo/registry/cache/
             ~/.cargo/git/db/
             ${{ github.workspace }}/codex-rs/target/
-          key: cargo-ubuntu-24.04-x86_64-unknown-linux-gnu-${{ hashFiles('**/Cargo.lock') }}
+          key: cargo-ubuntu-24.04-x86_64-unknown-linux-gnu-dev-${{ hashFiles('**/Cargo.lock') }}
 
       # Note it is possible that the `verify` step internal to Run Codex will
       # fail, in which case the work to setup the repo was worthless :(

AGENTS.md

@@ -8,7 +8,7 @@ In the codex-rs folder where the rust code lives:
   - You operate in a sandbox where `CODEX_SANDBOX_NETWORK_DISABLED=1` will be set whenever you use the `shell` tool. Any existing code that uses `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` was authored with this fact in mind. It is often used to early exit out of tests that the author knew you would not be able to run given your sandbox limitations.
   - Similarly, when you spawn a process using Seatbelt (`/usr/bin/sandbox-exec`), `CODEX_SANDBOX=seatbelt` will be set on the child process. Integration tests that want to run Seatbelt themselves cannot be run under Seatbelt, so checks for `CODEX_SANDBOX=seatbelt` are also often used to early exit out of tests, as appropriate.
 
-Before finalizing a change to `codex-rs`, run `just fmt` (in `codex-rs` directory) to format the code and `just fix` (in `codex-rs` directory) to fix any linter issues in the code. Additionally, run the tests:
+Before finalizing a change to `codex-rs`, run `just fmt` (in `codex-rs` directory) to format the code and `just fix -p <project>` (in `codex-rs` directory) to fix any linter issues in the code. Additionally, run the tests:
 1. Run the test for the specific project that was changed. For example, if changes were made in `codex-rs/tui`, run `cargo test -p codex-tui`.
 2. Once those pass, if any changes were made in common, core, or protocol, run the complete test suite with `cargo test --all-features`.
 

codex-rs/Cargo.lock

@@ -22,6 +22,8 @@ use tree_sitter_bash::LANGUAGE as BASH;
 /// Detailed instructions for gpt-4.1 on how to use the `apply_patch` tool.
 pub const APPLY_PATCH_TOOL_INSTRUCTIONS: &str = include_str!("../apply_patch_tool_instructions.md");
 
+const APPLY_PATCH_COMMANDS: [&str; 2] = ["apply_patch", "applypatch"];
+
 #[derive(Debug, Error, PartialEq)]
 pub enum ApplyPatchError {
     #[error(transparent)]
@@ -82,7 +84,6 @@ pub struct ApplyPatchArgs {
 }
 
 pub fn maybe_parse_apply_patch(argv: &[String]) -> MaybeApplyPatch {
-    const APPLY_PATCH_COMMANDS: [&str; 2] = ["apply_patch", "applypatch"];
     match argv {
         [cmd, body] if APPLY_PATCH_COMMANDS.contains(&cmd.as_str()) => match parse_patch(body) {
             Ok(source) => MaybeApplyPatch::Body(source),
@@ -91,7 +92,9 @@ pub fn maybe_parse_apply_patch(argv: &[String]) -> MaybeApplyPatch {
         [bash, flag, script]
             if bash == "bash"
                 && flag == "-lc"
-                && script.trim_start().starts_with("apply_patch") =>
+                && APPLY_PATCH_COMMANDS
+                    .iter()
+                    .any(|cmd| script.trim_start().starts_with(cmd)) =>
         {
             match extract_heredoc_body_from_apply_patch_command(script) {
                 Ok(body) => match parse_patch(&body) {
@@ -262,7 +265,10 @@ pub fn maybe_parse_apply_patch_verified(argv: &[String], cwd: &Path) -> MaybeApp
 fn extract_heredoc_body_from_apply_patch_command(
     src: &str,
 ) -> std::result::Result<String, ExtractHeredocError> {
-    if !src.trim_start().starts_with("apply_patch") {
+    if !APPLY_PATCH_COMMANDS
+        .iter()
+        .any(|cmd| src.trim_start().starts_with(cmd))
+    {
         return Err(ExtractHeredocError::CommandDidNotStartWithApplyPatch);
     }
 
@@ -773,6 +779,33 @@ PATCH"#,
         }
     }
 
+    #[test]
+    fn test_heredoc_applypatch() {
+        let args = strs_to_strings(&[
+            "bash",
+            "-lc",
+            r#"applypatch <<'PATCH'
+*** Begin Patch
+*** Add File: foo
++hi
+*** End Patch
+PATCH"#,
+        ]);
+
+        match maybe_parse_apply_patch(&args) {
+            MaybeApplyPatch::Body(ApplyPatchArgs { hunks, patch: _ }) => {
+                assert_eq!(
+                    hunks,
+                    vec![Hunk::AddFile {
+                        path: PathBuf::from("foo"),
+                        contents: "hi\n".to_string()
+                    }]
+                );
+            }
+            result => panic!("expected MaybeApplyPatch::Body got {result:?}"),
+        }
+    }
+
     #[test]
     fn test_add_file_hunk_creates_file_with_contents() {
         let dir = tempdir().unwrap();

codex-rs/apply-patch/src/lib.rs

@@ -248,12 +248,10 @@ impl ModelClient {
                         .and_then(|v| v.to_str().ok())
                         .and_then(|s| s.parse::<u64>().ok());
 
-                    if status == StatusCode::UNAUTHORIZED {
-                        if let Some(a) = auth.as_ref() {
-                            let _ = a.refresh_token().await;
-                        }
-                        // Retry immediately with refreshed credentials.
-                        continue;
+                    if status == StatusCode::UNAUTHORIZED
+                        && let Some(a) = auth.as_ref()
+                    {
+                        let _ = a.refresh_token().await;
                     }
 
                     // The OpenAI Responses endpoint returns structured JSON bodies even for 4xx/5xx
@@ -263,7 +261,10 @@ impl ModelClient {
                     // exact error message (e.g. "Unknown parameter: 'input[0].metadata'"). The body is
                     // small and this branch only runs on error paths so the extra allocation is
                     // negligible.
-                    if !(status == StatusCode::TOO_MANY_REQUESTS || status.is_server_error()) {
+                    if !(status == StatusCode::TOO_MANY_REQUESTS
+                        || status == StatusCode::UNAUTHORIZED
+                        || status.is_server_error())
+                    {
                         // Surface the error body to callers. Use `unwrap_or_default` per Clippy.
                         let body = res.text().await.unwrap_or_default();
                         return Err(CodexErr::UnexpectedStatus(status, body));

codex-rs/core/src/client.rs

@@ -49,6 +49,7 @@ pub(crate) mod safety;
 pub mod seatbelt;
 pub mod shell;
 pub mod spawn;
+pub mod terminal;
 pub mod turn_diff_tracker;
 pub mod user_agent;
 mod user_notification;

codex-rs/core/src/lib.rs

@@ -0,0 +1,72 @@
+use std::sync::OnceLock;
+
+static TERMINAL: OnceLock<String> = OnceLock::new();
+
+pub fn user_agent() -> String {
+    TERMINAL.get_or_init(detect_terminal).to_string()
+}
+
+/// Sanitize a header value to be used in a User-Agent string.
+///
+/// This function replaces any characters that are not allowed in a User-Agent string with an underscore.
+///
+/// # Arguments
+///
+/// * `value` - The value to sanitize.
+fn is_valid_header_value_char(c: char) -> bool {
+    c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.' || c == '/'
+}
+
+fn sanitize_header_value(value: String) -> String {
+    value.replace(|c| !is_valid_header_value_char(c), "_")
+}
+
+fn detect_terminal() -> String {
+    sanitize_header_value(
+        if let Ok(tp) = std::env::var("TERM_PROGRAM")
+            && !tp.trim().is_empty()
+        {
+            let ver = std::env::var("TERM_PROGRAM_VERSION").ok();
+            match ver {
+                Some(v) if !v.trim().is_empty() => format!("{tp}/{v}"),
+                _ => tp,
+            }
+        } else if let Ok(v) = std::env::var("WEZTERM_VERSION") {
+            if !v.trim().is_empty() {
+                format!("WezTerm/{v}")
+            } else {
+                "WezTerm".to_string()
+            }
+        } else if std::env::var("KITTY_WINDOW_ID").is_ok()
+            || std::env::var("TERM")
+                .map(|t| t.contains("kitty"))
+                .unwrap_or(false)
+        {
+            "kitty".to_string()
+        } else if std::env::var("ALACRITTY_SOCKET").is_ok()
+            || std::env::var("TERM")
+                .map(|t| t == "alacritty")
+                .unwrap_or(false)
+        {
+            "Alacritty".to_string()
+        } else if let Ok(v) = std::env::var("KONSOLE_VERSION") {
+            if !v.trim().is_empty() {
+                format!("Konsole/{v}")
+            } else {
+                "Konsole".to_string()
+            }
+        } else if std::env::var("GNOME_TERMINAL_SCREEN").is_ok() {
+            return "gnome-terminal".to_string();
+        } else if let Ok(v) = std::env::var("VTE_VERSION") {
+            if !v.trim().is_empty() {
+                format!("VTE/{v}")
+            } else {
+                "VTE".to_string()
+            }
+        } else if std::env::var("WT_SESSION").is_ok() {
+            return "WindowsTerminal".to_string();
+        } else {
+            std::env::var("TERM").unwrap_or_else(|_| "unknown".to_string())
+        },
+    )
+}

codex-rs/core/src/terminal.rs

@@ -4,11 +4,12 @@ pub fn get_codex_user_agent(originator: Option<&str>) -> String {
     let build_version = env!("CARGO_PKG_VERSION");
     let os_info = os_info::get();
     format!(
-        "{}/{build_version} ({} {}; {})",
+        "{}/{build_version} ({} {}; {}) {}",
         originator.unwrap_or(DEFAULT_ORIGINATOR),
         os_info.os_type(),
         os_info.version(),
         os_info.architecture().unwrap_or("unknown"),
+        crate::terminal::user_agent()
     )
 }
 
@@ -27,9 +28,10 @@ mod tests {
     fn test_macos() {
         use regex_lite::Regex;
         let user_agent = get_codex_user_agent(None);
-        let re =
-            Regex::new(r"^codex_cli_rs/\d+\.\d+\.\d+ \(Mac OS \d+\.\d+\.\d+; (x86_64|arm64)\)$")
-                .unwrap();
+        let re = Regex::new(
+            r"^codex_cli_rs/\d+\.\d+\.\d+ \(Mac OS \d+\.\d+\.\d+; (x86_64|arm64)\) (\S+)$",
+        )
+        .unwrap();
         assert!(re.is_match(&user_agent));
     }
 }

codex-rs/core/src/user_agent.rs

@@ -24,8 +24,8 @@ file-search *args:
 fmt:
     cargo fmt -- --config imports_granularity=Item
 
-fix:
-    cargo clippy --fix --all-features --tests --allow-dirty
+fix *args:
+    cargo clippy --fix --all-features --tests --allow-dirty "$@"
 
 install:
     rustup show active-toolchain

codex-rs/justfile

@@ -242,7 +242,7 @@ fn load_auth(
         // "refreshable" even if we are using the API key for auth?
         match &tokens {
             Some(tokens) => {
-                if tokens.should_use_api_key(preferred_auth_method) {
+                if tokens.should_use_api_key(preferred_auth_method, tokens.is_openai_email()) {
                     return Ok(Some(CodexAuth::from_api_key(api_key)));
                 } else {
                     // Ignore the API key and fall through to ChatGPT auth.