diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index f65c555a..157c09a2 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -973,11 +973,13 @@ dependencies = [ "diffy", "image", "insta", + "itertools 0.14.0", "lazy_static", "libc", "mcp-types", "once_cell", "path-clean", + "pathdiff", "pretty_assertions", "rand 0.9.2", "ratatui", @@ -3377,6 +3379,12 @@ dependencies = [ "once_cell", ] +[[package]] +name = "pathdiff" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df94ce210e5bc13cb6651479fa48d14f601d9858cfe0467f43ae157023b938d3" + [[package]] name = "percent-encoding" version = "2.3.1" diff --git a/codex-rs/apply-patch/src/lib.rs b/codex-rs/apply-patch/src/lib.rs index 84cb9120..46ddf056 100644 --- a/codex-rs/apply-patch/src/lib.rs +++ b/codex-rs/apply-patch/src/lib.rs @@ -116,7 +116,9 @@ pub enum ApplyPatchFileChange { Add { content: String, }, - Delete, + Delete { + content: String, + }, Update { unified_diff: String, move_path: Option, @@ -210,7 +212,18 @@ pub fn maybe_parse_apply_patch_verified(argv: &[String], cwd: &Path) -> MaybeApp changes.insert(path, ApplyPatchFileChange::Add { content: contents }); } Hunk::DeleteFile { .. } => { - changes.insert(path, ApplyPatchFileChange::Delete); + let content = match std::fs::read_to_string(&path) { + Ok(content) => content, + Err(e) => { + return MaybeApplyPatchVerified::CorrectnessError( + ApplyPatchError::IoError(IoError { + context: format!("Failed to read {}", path.display()), + source: e, + }), + ); + } + }; + changes.insert(path, ApplyPatchFileChange::Delete { content }); } Hunk::UpdateFile { move_path, chunks, .. diff --git a/codex-rs/core/src/apply_patch.rs b/codex-rs/core/src/apply_patch.rs index 56fd01fe..0afc06e9 100644 --- a/codex-rs/core/src/apply_patch.rs +++ b/codex-rs/core/src/apply_patch.rs @@ -109,7 +109,9 @@ pub(crate) fn convert_apply_patch_to_protocol( ApplyPatchFileChange::Add { content } => FileChange::Add { content: content.clone(), }, - ApplyPatchFileChange::Delete => FileChange::Delete, + ApplyPatchFileChange::Delete { content } => FileChange::Delete { + content: content.clone(), + }, ApplyPatchFileChange::Update { unified_diff, move_path, diff --git a/codex-rs/core/src/git_info.rs b/codex-rs/core/src/git_info.rs index 0290323c..79a8647a 100644 --- a/codex-rs/core/src/git_info.rs +++ b/codex-rs/core/src/git_info.rs @@ -10,7 +10,35 @@ use tokio::process::Command; use tokio::time::Duration as TokioDuration; use tokio::time::timeout; -use crate::util::is_inside_git_repo; +/// Return `true` if the project folder specified by the `Config` is inside a +/// Git repository. +/// +/// The check walks up the directory hierarchy looking for a `.git` file or +/// directory (note `.git` can be a file that contains a `gitdir` entry). This +/// approach does **not** require the `git` binary or the `git2` crate and is +/// therefore fairly lightweight. +/// +/// Note that this does **not** detect *work‑trees* created with +/// `git worktree add` where the checkout lives outside the main repository +/// directory. If you need Codex to work from such a checkout simply pass the +/// `--allow-no-git-exec` CLI flag that disables the repo requirement. +pub fn get_git_repo_root(base_dir: &Path) -> Option { + let mut dir = base_dir.to_path_buf(); + + loop { + if dir.join(".git").exists() { + return Some(dir); + } + + // Pop one component (go up one directory). `pop` returns false when + // we have reached the filesystem root. + if !dir.pop() { + break; + } + } + + None +} /// Timeout for git commands to prevent freezing on large repositories const GIT_COMMAND_TIMEOUT: TokioDuration = TokioDuration::from_secs(5); @@ -94,9 +122,7 @@ pub async fn collect_git_info(cwd: &Path) -> Option { /// Returns the closest git sha to HEAD that is on a remote as well as the diff to that sha. pub async fn git_diff_to_remote(cwd: &Path) -> Option { - if !is_inside_git_repo(cwd) { - return None; - } + get_git_repo_root(cwd)?; let remotes = get_git_remotes(cwd).await?; let branches = branch_ancestry(cwd).await?; @@ -440,7 +466,7 @@ async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option { } /// Resolve the path that should be used for trust checks. Similar to -/// `[utils::is_inside_git_repo]`, but resolves to the root of the main +/// `[get_git_repo_root]`, but resolves to the root of the main /// repository. Handles worktrees. pub fn resolve_root_git_project_for_trust(cwd: &Path) -> Option { let base = if cwd.is_dir() { cwd } else { cwd.parent()? }; diff --git a/codex-rs/core/src/parse_command.rs b/codex-rs/core/src/parse_command.rs index 7348af43..bcf0dba7 100644 --- a/codex-rs/core/src/parse_command.rs +++ b/codex-rs/core/src/parse_command.rs @@ -20,22 +20,6 @@ pub enum ParsedCommand { query: Option, path: Option, }, - Format { - cmd: String, - tool: Option, - targets: Option>, - }, - Test { - cmd: String, - }, - Lint { - cmd: String, - tool: Option, - targets: Option>, - }, - Noop { - cmd: String, - }, Unknown { cmd: String, }, @@ -50,10 +34,6 @@ impl From for codex_protocol::parse_command::ParsedCommand { ParsedCommand::Read { cmd, name } => P::Read { cmd, name }, ParsedCommand::ListFiles { cmd, path } => P::ListFiles { cmd, path }, ParsedCommand::Search { cmd, query, path } => P::Search { cmd, query, path }, - ParsedCommand::Format { cmd, tool, targets } => P::Format { cmd, tool, targets }, - ParsedCommand::Test { cmd } => P::Test { cmd }, - ParsedCommand::Lint { cmd, tool, targets } => P::Lint { cmd, tool, targets }, - ParsedCommand::Noop { cmd } => P::Noop { cmd }, ParsedCommand::Unknown { cmd } => P::Unknown { cmd }, } } @@ -122,7 +102,7 @@ mod tests { assert_parsed( &vec_str(&["bash", "-lc", inner]), vec![ParsedCommand::Unknown { - cmd: "git status | wc -l".to_string(), + cmd: "git status".to_string(), }], ); } @@ -244,6 +224,17 @@ mod tests { ); } + #[test] + fn cd_then_cat_is_single_read() { + assert_parsed( + &shlex_split_safe("cd foo && cat foo.txt"), + vec![ParsedCommand::Read { + cmd: "cat foo.txt".to_string(), + name: "foo.txt".to_string(), + }], + ); + } + #[test] fn supports_ls_with_pipe() { let inner = "ls -la | sed -n '1,120p'"; @@ -315,27 +306,6 @@ mod tests { ); } - #[test] - fn supports_npm_run_with_forwarded_args() { - assert_parsed( - &vec_str(&[ - "npm", - "run", - "lint", - "--", - "--max-warnings", - "0", - "--format", - "json", - ]), - vec![ParsedCommand::Lint { - cmd: "npm run lint -- --max-warnings 0 --format json".to_string(), - tool: Some("npm-script:lint".to_string()), - targets: None, - }], - ); - } - #[test] fn supports_grep_recursive_current_dir() { assert_parsed( @@ -396,173 +366,10 @@ mod tests { fn supports_cd_and_rg_files() { assert_parsed( &shlex_split_safe("cd codex-rs && rg --files"), - vec![ - ParsedCommand::Unknown { - cmd: "cd codex-rs".to_string(), - }, - ParsedCommand::Search { - cmd: "rg --files".to_string(), - query: None, - path: None, - }, - ], - ); - } - - #[test] - fn echo_then_cargo_test_sequence() { - assert_parsed( - &shlex_split_safe("echo Running tests... && cargo test --all-features --quiet"), - vec![ParsedCommand::Test { - cmd: "cargo test --all-features --quiet".to_string(), - }], - ); - } - - #[test] - fn supports_cargo_fmt_and_test_with_config() { - assert_parsed( - &shlex_split_safe( - "cargo fmt -- --config imports_granularity=Item && cargo test -p core --all-features", - ), - vec![ - ParsedCommand::Format { - cmd: "cargo fmt -- --config 'imports_granularity=Item'".to_string(), - tool: Some("cargo fmt".to_string()), - targets: None, - }, - ParsedCommand::Test { - cmd: "cargo test -p core --all-features".to_string(), - }, - ], - ); - } - - #[test] - fn recognizes_rustfmt_and_clippy() { - assert_parsed( - &shlex_split_safe("rustfmt src/main.rs"), - vec![ParsedCommand::Format { - cmd: "rustfmt src/main.rs".to_string(), - tool: Some("rustfmt".to_string()), - targets: Some(vec!["src/main.rs".to_string()]), - }], - ); - - assert_parsed( - &shlex_split_safe("cargo clippy -p core --all-features -- -D warnings"), - vec![ParsedCommand::Lint { - cmd: "cargo clippy -p core --all-features -- -D warnings".to_string(), - tool: Some("cargo clippy".to_string()), - targets: None, - }], - ); - } - - #[test] - fn recognizes_pytest_go_and_tools() { - assert_parsed( - &shlex_split_safe( - "pytest -k 'Login and not slow' tests/test_login.py::TestLogin::test_ok", - ), - vec![ParsedCommand::Test { - cmd: "pytest -k 'Login and not slow' tests/test_login.py::TestLogin::test_ok" - .to_string(), - }], - ); - - assert_parsed( - &shlex_split_safe("go fmt ./..."), - vec![ParsedCommand::Format { - cmd: "go fmt ./...".to_string(), - tool: Some("go fmt".to_string()), - targets: Some(vec!["./...".to_string()]), - }], - ); - - assert_parsed( - &shlex_split_safe("go test ./pkg -run TestThing"), - vec![ParsedCommand::Test { - cmd: "go test ./pkg -run TestThing".to_string(), - }], - ); - - assert_parsed( - &shlex_split_safe("eslint . --max-warnings 0"), - vec![ParsedCommand::Lint { - cmd: "eslint . --max-warnings 0".to_string(), - tool: Some("eslint".to_string()), - targets: Some(vec![".".to_string()]), - }], - ); - - assert_parsed( - &shlex_split_safe("prettier -w ."), - vec![ParsedCommand::Format { - cmd: "prettier -w .".to_string(), - tool: Some("prettier".to_string()), - targets: Some(vec![".".to_string()]), - }], - ); - } - - #[test] - fn recognizes_jest_and_vitest_filters() { - assert_parsed( - &shlex_split_safe("jest -t 'should work' src/foo.test.ts"), - vec![ParsedCommand::Test { - cmd: "jest -t 'should work' src/foo.test.ts".to_string(), - }], - ); - - assert_parsed( - &shlex_split_safe("vitest -t 'runs' src/foo.test.tsx"), - vec![ParsedCommand::Test { - cmd: "vitest -t runs src/foo.test.tsx".to_string(), - }], - ); - } - - #[test] - fn recognizes_npx_and_scripts() { - assert_parsed( - &shlex_split_safe("npx eslint src"), - vec![ParsedCommand::Lint { - cmd: "npx eslint src".to_string(), - tool: Some("eslint".to_string()), - targets: Some(vec!["src".to_string()]), - }], - ); - - assert_parsed( - &shlex_split_safe("npx prettier -c ."), - vec![ParsedCommand::Format { - cmd: "npx prettier -c .".to_string(), - tool: Some("prettier".to_string()), - targets: Some(vec![".".to_string()]), - }], - ); - - assert_parsed( - &shlex_split_safe("pnpm run lint -- --max-warnings 0"), - vec![ParsedCommand::Lint { - cmd: "pnpm run lint -- --max-warnings 0".to_string(), - tool: Some("pnpm-script:lint".to_string()), - targets: None, - }], - ); - - assert_parsed( - &shlex_split_safe("npm test"), - vec![ParsedCommand::Test { - cmd: "npm test".to_string(), - }], - ); - - assert_parsed( - &shlex_split_safe("yarn test"), - vec![ParsedCommand::Test { - cmd: "yarn test".to_string(), + vec![ParsedCommand::Search { + cmd: "rg --files".to_string(), + query: None, + path: None, }], ); } @@ -770,6 +577,51 @@ mod tests { ); } + #[test] + fn parses_mixed_sequence_with_pipes_semicolons_and_or() { + // Provided long command sequence combining sequencing, pipelines, and ORs. + let inner = "pwd; ls -la; rg --files -g '!target' | wc -l; rg -n '^\\[workspace\\]' -n Cargo.toml || true; rg -n '^\\[package\\]' -n */Cargo.toml || true; cargo --version; rustc --version; cargo clippy --workspace --all-targets --all-features -q"; + let args = vec_str(&["bash", "-lc", inner]); + + let expected = vec![ + ParsedCommand::Unknown { + cmd: "pwd".to_string(), + }, + ParsedCommand::ListFiles { + cmd: shlex_join(&shlex_split_safe("ls -la")), + path: None, + }, + ParsedCommand::Search { + cmd: shlex_join(&shlex_split_safe("rg --files -g '!target'")), + query: None, + path: Some("!target".to_string()), + }, + ParsedCommand::Search { + cmd: shlex_join(&shlex_split_safe("rg -n '^\\[workspace\\]' -n Cargo.toml")), + query: Some("^\\[workspace\\]".to_string()), + path: Some("Cargo.toml".to_string()), + }, + ParsedCommand::Search { + cmd: shlex_join(&shlex_split_safe("rg -n '^\\[package\\]' -n */Cargo.toml")), + query: Some("^\\[package\\]".to_string()), + path: Some("Cargo.toml".to_string()), + }, + ParsedCommand::Unknown { + cmd: shlex_join(&shlex_split_safe("cargo --version")), + }, + ParsedCommand::Unknown { + cmd: shlex_join(&shlex_split_safe("rustc --version")), + }, + ParsedCommand::Unknown { + cmd: shlex_join(&shlex_split_safe( + "cargo clippy --workspace --all-targets --all-features -q", + )), + }, + ]; + + assert_parsed(&args, expected); + } + #[test] fn strips_true_in_sequence() { // `true` should be dropped from parsed sequences @@ -867,159 +719,6 @@ mod tests { ); } - #[test] - fn pnpm_test_is_parsed_as_test() { - assert_parsed( - &shlex_split_safe("pnpm test"), - vec![ParsedCommand::Test { - cmd: "pnpm test".to_string(), - }], - ); - } - - #[test] - fn pnpm_exec_vitest_is_unknown() { - // From commands_combined: cd codex-cli && pnpm exec vitest run tests/... --threads=false --passWithNoTests - let inner = "cd codex-cli && pnpm exec vitest run tests/file-tag-utils.test.ts --threads=false --passWithNoTests"; - assert_parsed( - &shlex_split_safe(inner), - vec![ - ParsedCommand::Unknown { - cmd: "cd codex-cli".to_string(), - }, - ParsedCommand::Unknown { - cmd: "pnpm exec vitest run tests/file-tag-utils.test.ts '--threads=false' --passWithNoTests".to_string(), - }, - ], - ); - } - - #[test] - fn cargo_test_with_crate() { - assert_parsed( - &shlex_split_safe("cargo test -p codex-core parse_command::"), - vec![ParsedCommand::Test { - cmd: "cargo test -p codex-core parse_command::".to_string(), - }], - ); - } - - #[test] - fn cargo_test_with_crate_2() { - assert_parsed( - &shlex_split_safe( - "cd core && cargo test -q parse_command::tests::bash_dash_c_pipeline_parsing parse_command::tests::fd_file_finder_variants", - ), - vec![ParsedCommand::Test { - cmd: "cargo test -q parse_command::tests::bash_dash_c_pipeline_parsing parse_command::tests::fd_file_finder_variants".to_string(), - }], - ); - } - - #[test] - fn cargo_test_with_crate_3() { - assert_parsed( - &shlex_split_safe("cd core && cargo test -q parse_command::tests"), - vec![ParsedCommand::Test { - cmd: "cargo test -q parse_command::tests".to_string(), - }], - ); - } - - #[test] - fn cargo_test_with_crate_4() { - assert_parsed( - &shlex_split_safe("cd core && cargo test --all-features parse_command -- --nocapture"), - vec![ParsedCommand::Test { - cmd: "cargo test --all-features parse_command -- --nocapture".to_string(), - }], - ); - } - - // Additional coverage for other common tools/frameworks - #[test] - fn recognizes_black_and_ruff() { - // black formats Python code - assert_parsed( - &shlex_split_safe("black src"), - vec![ParsedCommand::Format { - cmd: "black src".to_string(), - tool: Some("black".to_string()), - targets: Some(vec!["src".to_string()]), - }], - ); - - // ruff check is a linter; ensure we collect targets - assert_parsed( - &shlex_split_safe("ruff check ."), - vec![ParsedCommand::Lint { - cmd: "ruff check .".to_string(), - tool: Some("ruff".to_string()), - targets: Some(vec![".".to_string()]), - }], - ); - - // ruff format is a formatter - assert_parsed( - &shlex_split_safe("ruff format pkg/"), - vec![ParsedCommand::Format { - cmd: "ruff format pkg/".to_string(), - tool: Some("ruff".to_string()), - targets: Some(vec!["pkg/".to_string()]), - }], - ); - } - - #[test] - fn recognizes_pnpm_monorepo_test_and_npm_format_script() { - // pnpm -r test in a monorepo should still parse as a test action - assert_parsed( - &shlex_split_safe("pnpm -r test"), - vec![ParsedCommand::Test { - cmd: "pnpm -r test".to_string(), - }], - ); - - // npm run format should be recognized as a format action - assert_parsed( - &shlex_split_safe("npm run format -- -w ."), - vec![ParsedCommand::Format { - cmd: "npm run format -- -w .".to_string(), - tool: Some("npm-script:format".to_string()), - targets: None, - }], - ); - } - - #[test] - fn yarn_test_is_parsed_as_test() { - assert_parsed( - &shlex_split_safe("yarn test"), - vec![ParsedCommand::Test { - cmd: "yarn test".to_string(), - }], - ); - } - - #[test] - fn pytest_file_only_and_go_run_regex() { - // pytest invoked with a file path should be captured as a filter - assert_parsed( - &shlex_split_safe("pytest tests/test_example.py"), - vec![ParsedCommand::Test { - cmd: "pytest tests/test_example.py".to_string(), - }], - ); - - // go test with -run regex should capture the filter - assert_parsed( - &shlex_split_safe("go test ./... -run '^TestFoo$'"), - vec![ParsedCommand::Test { - cmd: "go test ./... -run '^TestFoo$'".to_string(), - }], - ); - } - #[test] fn grep_with_query_and_path() { assert_parsed( @@ -1090,30 +789,6 @@ mod tests { ); } - #[test] - fn eslint_with_config_path_and_target() { - assert_parsed( - &shlex_split_safe("eslint -c .eslintrc.json src"), - vec![ParsedCommand::Lint { - cmd: "eslint -c .eslintrc.json src".to_string(), - tool: Some("eslint".to_string()), - targets: Some(vec!["src".to_string()]), - }], - ); - } - - #[test] - fn npx_eslint_with_config_path_and_target() { - assert_parsed( - &shlex_split_safe("npx eslint -c .eslintrc src"), - vec![ParsedCommand::Lint { - cmd: "npx eslint -c .eslintrc src".to_string(), - tool: Some("eslint".to_string()), - targets: Some(vec!["src".to_string()]), - }], - ); - } - #[test] fn fd_file_finder_variants() { assert_parsed( @@ -1202,16 +877,13 @@ fn simplify_once(commands: &[ParsedCommand]) -> Option> { return Some(commands[1..].to_vec()); } - // cd foo && [any Test command] => [any Test command] + // cd foo && [any command] => [any command] (keep non-cd when a cd is followed by something) if let Some(idx) = commands.iter().position(|pc| match pc { ParsedCommand::Unknown { cmd } => { shlex_split(cmd).is_some_and(|t| t.first().map(|s| s.as_str()) == Some("cd")) } _ => false, - }) && commands - .iter() - .skip(idx + 1) - .any(|pc| matches!(pc, ParsedCommand::Test { .. })) + }) && commands.len() > idx + 1 { let mut out = Vec::with_capacity(commands.len() - 1); out.extend_from_slice(&commands[..idx]); @@ -1220,10 +892,10 @@ fn simplify_once(commands: &[ParsedCommand]) -> Option> { } // cmd || true => cmd - if let Some(idx) = commands.iter().position(|pc| match pc { - ParsedCommand::Noop { cmd } => cmd == "true", - _ => false, - }) { + if let Some(idx) = commands + .iter() + .position(|pc| matches!(pc, ParsedCommand::Unknown { cmd } if cmd == "true")) + { let mut out = Vec::with_capacity(commands.len() - 1); out.extend_from_slice(&commands[..idx]); out.extend_from_slice(&commands[idx + 1..]); @@ -1377,75 +1049,6 @@ fn skip_flag_values<'a>(args: &'a [String], flags_with_vals: &[&str]) -> Vec<&'a out } -/// Common flags for ESLint that take a following value and should not be -/// considered positional targets. -const ESLINT_FLAGS_WITH_VALUES: &[&str] = &[ - "-c", - "--config", - "--parser", - "--parser-options", - "--rulesdir", - "--plugin", - "--max-warnings", - "--format", -]; - -fn collect_non_flag_targets(args: &[String]) -> Option> { - let mut targets = Vec::new(); - let mut skip_next = false; - for (i, a) in args.iter().enumerate() { - if a == "--" { - break; - } - if skip_next { - skip_next = false; - continue; - } - if a == "-p" - || a == "--package" - || a == "--features" - || a == "-C" - || a == "--config" - || a == "--config-path" - || a == "--out-dir" - || a == "-o" - || a == "--run" - || a == "--max-warnings" - || a == "--format" - { - if i + 1 < args.len() { - skip_next = true; - } - continue; - } - if a.starts_with('-') { - continue; - } - targets.push(a.clone()); - } - if targets.is_empty() { - None - } else { - Some(targets) - } -} - -fn collect_non_flag_targets_with_flags( - args: &[String], - flags_with_vals: &[&str], -) -> Option> { - let targets: Vec = skip_flag_values(args, flags_with_vals) - .into_iter() - .filter(|a| !a.starts_with('-')) - .cloned() - .collect(); - if targets.is_empty() { - None - } else { - Some(targets) - } -} - fn is_pathish(s: &str) -> bool { s == "." || s == ".." @@ -1514,47 +1117,6 @@ fn parse_find_query_and_path(tail: &[String]) -> (Option, Option (query, path) } -fn classify_npm_like(tool: &str, tail: &[String], full_cmd: &[String]) -> Option { - let mut r = tail; - if tool == "pnpm" && r.first().map(|s| s.as_str()) == Some("-r") { - r = &r[1..]; - } - let mut script_name: Option = None; - if r.first().map(|s| s.as_str()) == Some("run") { - script_name = r.get(1).cloned(); - } else { - let is_test_cmd = (tool == "npm" && r.first().map(|s| s.as_str()) == Some("t")) - || ((tool == "npm" || tool == "pnpm" || tool == "yarn") - && r.first().map(|s| s.as_str()) == Some("test")); - if is_test_cmd { - script_name = Some("test".to_string()); - } - } - if let Some(name) = script_name { - let lname = name.to_lowercase(); - if lname == "test" || lname == "unit" || lname == "jest" || lname == "vitest" { - return Some(ParsedCommand::Test { - cmd: shlex_join(full_cmd), - }); - } - if lname == "lint" || lname == "eslint" { - return Some(ParsedCommand::Lint { - cmd: shlex_join(full_cmd), - tool: Some(format!("{tool}-script:{name}")), - targets: None, - }); - } - if lname == "format" || lname == "fmt" || lname == "prettier" { - return Some(ParsedCommand::Format { - cmd: shlex_join(full_cmd), - tool: Some(format!("{tool}-script:{name}")), - targets: None, - }); - } - } - None -} - fn parse_bash_lc_commands(original: &[String]) -> Option> { let [bash, flag, script] = original else { return None; @@ -1586,7 +1148,7 @@ fn parse_bash_lc_commands(original: &[String]) -> Option> { .map(|tokens| summarize_main_tokens(&tokens)) .collect(); if commands.len() > 1 { - commands.retain(|pc| !matches!(pc, ParsedCommand::Noop { .. })); + commands.retain(|pc| !matches!(pc, ParsedCommand::Unknown { cmd } if cmd == "true")); } if commands.len() == 1 { // If we reduced to a single command, attribute the full original script @@ -1655,27 +1217,7 @@ fn parse_bash_lc_commands(original: &[String]) -> Option> { } } } - ParsedCommand::Format { - tool, targets, cmd, .. - } => ParsedCommand::Format { - cmd: cmd.clone(), - tool, - targets, - }, - ParsedCommand::Test { cmd, .. } => ParsedCommand::Test { cmd: cmd.clone() }, - ParsedCommand::Lint { - tool, targets, cmd, .. - } => ParsedCommand::Lint { - cmd: cmd.clone(), - tool, - targets, - }, - ParsedCommand::Unknown { .. } => ParsedCommand::Unknown { - cmd: script.clone(), - }, - ParsedCommand::Noop { .. } => ParsedCommand::Noop { - cmd: script.clone(), - }, + other => other, }) .collect(); } @@ -1728,124 +1270,6 @@ fn drop_small_formatting_commands(mut commands: Vec>) -> Vec ParsedCommand { match main_cmd.split_first() { - Some((head, tail)) if head == "true" && tail.is_empty() => ParsedCommand::Noop { - cmd: shlex_join(main_cmd), - }, - // (sed-specific logic handled below in dedicated arm returning Read) - Some((head, tail)) - if head == "cargo" && tail.first().map(|s| s.as_str()) == Some("fmt") => - { - ParsedCommand::Format { - cmd: shlex_join(main_cmd), - tool: Some("cargo fmt".to_string()), - targets: collect_non_flag_targets(&tail[1..]), - } - } - Some((head, tail)) - if head == "cargo" && tail.first().map(|s| s.as_str()) == Some("clippy") => - { - ParsedCommand::Lint { - cmd: shlex_join(main_cmd), - tool: Some("cargo clippy".to_string()), - targets: collect_non_flag_targets(&tail[1..]), - } - } - Some((head, tail)) - if head == "cargo" && tail.first().map(|s| s.as_str()) == Some("test") => - { - ParsedCommand::Test { - cmd: shlex_join(main_cmd), - } - } - Some((head, tail)) if head == "rustfmt" => ParsedCommand::Format { - cmd: shlex_join(main_cmd), - tool: Some("rustfmt".to_string()), - targets: collect_non_flag_targets(tail), - }, - Some((head, tail)) if head == "go" && tail.first().map(|s| s.as_str()) == Some("fmt") => { - ParsedCommand::Format { - cmd: shlex_join(main_cmd), - tool: Some("go fmt".to_string()), - targets: collect_non_flag_targets(&tail[1..]), - } - } - Some((head, tail)) if head == "go" && tail.first().map(|s| s.as_str()) == Some("test") => { - ParsedCommand::Test { - cmd: shlex_join(main_cmd), - } - } - Some((head, _)) if head == "pytest" => ParsedCommand::Test { - cmd: shlex_join(main_cmd), - }, - Some((head, tail)) if head == "eslint" => { - // Treat configuration flags with values (e.g. `-c .eslintrc`) as non-targets. - let targets = collect_non_flag_targets_with_flags(tail, ESLINT_FLAGS_WITH_VALUES); - ParsedCommand::Lint { - cmd: shlex_join(main_cmd), - tool: Some("eslint".to_string()), - targets, - } - } - Some((head, tail)) if head == "prettier" => ParsedCommand::Format { - cmd: shlex_join(main_cmd), - tool: Some("prettier".to_string()), - targets: collect_non_flag_targets(tail), - }, - Some((head, tail)) if head == "black" => ParsedCommand::Format { - cmd: shlex_join(main_cmd), - tool: Some("black".to_string()), - targets: collect_non_flag_targets(tail), - }, - Some((head, tail)) - if head == "ruff" && tail.first().map(|s| s.as_str()) == Some("check") => - { - ParsedCommand::Lint { - cmd: shlex_join(main_cmd), - tool: Some("ruff".to_string()), - targets: collect_non_flag_targets(&tail[1..]), - } - } - Some((head, tail)) - if head == "ruff" && tail.first().map(|s| s.as_str()) == Some("format") => - { - ParsedCommand::Format { - cmd: shlex_join(main_cmd), - tool: Some("ruff".to_string()), - targets: collect_non_flag_targets(&tail[1..]), - } - } - Some((head, _)) if (head == "jest" || head == "vitest") => ParsedCommand::Test { - cmd: shlex_join(main_cmd), - }, - Some((head, tail)) - if head == "npx" && tail.first().map(|s| s.as_str()) == Some("eslint") => - { - let targets = collect_non_flag_targets_with_flags(&tail[1..], ESLINT_FLAGS_WITH_VALUES); - ParsedCommand::Lint { - cmd: shlex_join(main_cmd), - tool: Some("eslint".to_string()), - targets, - } - } - Some((head, tail)) - if head == "npx" && tail.first().map(|s| s.as_str()) == Some("prettier") => - { - ParsedCommand::Format { - cmd: shlex_join(main_cmd), - tool: Some("prettier".to_string()), - targets: collect_non_flag_targets(&tail[1..]), - } - } - // NPM-like scripts including yarn - Some((tool, tail)) if (tool == "pnpm" || tool == "npm" || tool == "yarn") => { - if let Some(cmd) = classify_npm_like(tool, tail, main_cmd) { - cmd - } else { - ParsedCommand::Unknown { - cmd: shlex_join(main_cmd), - } - } - } Some((head, tail)) if head == "ls" => { // Avoid treating option values as paths (e.g., ls -I "*.test.js"). let candidates = skip_flag_values( diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 37cb8f4e..752177ed 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -222,7 +222,7 @@ fn is_write_patch_constrained_to_writable_paths( for (path, change) in action.changes() { match change { - ApplyPatchFileChange::Add { .. } | ApplyPatchFileChange::Delete => { + ApplyPatchFileChange::Add { .. } | ApplyPatchFileChange::Delete { .. } => { if !is_path_writable(path) { return false; } diff --git a/codex-rs/core/src/turn_diff_tracker.rs b/codex-rs/core/src/turn_diff_tracker.rs index 5e73fa06..2157ed17 100644 --- a/codex-rs/core/src/turn_diff_tracker.rs +++ b/codex-rs/core/src/turn_diff_tracker.rs @@ -578,7 +578,12 @@ index {ZERO_OID}..{right_oid} fs::write(&file, "x\n").unwrap(); let mut acc = TurnDiffTracker::new(); - let del_changes = HashMap::from([(file.clone(), FileChange::Delete)]); + let del_changes = HashMap::from([( + file.clone(), + FileChange::Delete { + content: "x\n".to_string(), + }, + )]); acc.on_patch_begin(&del_changes); // Simulate apply: delete the file from disk. @@ -741,7 +746,12 @@ index {left_oid}..{right_oid} assert_eq!(first, expected_first); // Next: introduce a brand-new path b.txt into baseline snapshots via a delete change. - let del_b = HashMap::from([(b.clone(), FileChange::Delete)]); + let del_b = HashMap::from([( + b.clone(), + FileChange::Delete { + content: "z\n".to_string(), + }, + )]); acc.on_patch_begin(&del_b); // Simulate apply: delete b.txt. let baseline_mode = file_mode_for_path(&b).unwrap_or(FileMode::Regular); diff --git a/codex-rs/core/src/util.rs b/codex-rs/core/src/util.rs index e1248a48..8ef258d9 100644 --- a/codex-rs/core/src/util.rs +++ b/codex-rs/core/src/util.rs @@ -1,4 +1,3 @@ -use std::path::Path; use std::time::Duration; use rand::Rng; @@ -12,33 +11,3 @@ pub(crate) fn backoff(attempt: u64) -> Duration { let jitter = rand::rng().random_range(0.9..1.1); Duration::from_millis((base as f64 * jitter) as u64) } - -/// Return `true` if the project folder specified by the `Config` is inside a -/// Git repository. -/// -/// The check walks up the directory hierarchy looking for a `.git` file or -/// directory (note `.git` can be a file that contains a `gitdir` entry). This -/// approach does **not** require the `git` binary or the `git2` crate and is -/// therefore fairly lightweight. -/// -/// Note that this does **not** detect *work‑trees* created with -/// `git worktree add` where the checkout lives outside the main repository -/// directory. If you need Codex to work from such a checkout simply pass the -/// `--allow-no-git-exec` CLI flag that disables the repo requirement. -pub fn is_inside_git_repo(base_dir: &Path) -> bool { - let mut dir = base_dir.to_path_buf(); - - loop { - if dir.join(".git").exists() { - return true; - } - - // Pop one component (go up one directory). `pop` returns false when - // we have reached the filesystem root. - if !dir.pop() { - break; - } - } - - false -} diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 8403bbde..cc633277 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -404,13 +404,16 @@ impl EventProcessor for EventProcessorWithHumanOutput { println!("{}", line.style(self.green)); } } - FileChange::Delete => { + FileChange::Delete { content } => { let header = format!( "{} {}", format_file_change(change), path.to_string_lossy() ); println!("{}", header.style(self.magenta)); + for line in content.lines() { + println!("{}", line.style(self.red)); + } } FileChange::Update { unified_diff, @@ -560,7 +563,7 @@ fn escape_command(command: &[String]) -> String { fn format_file_change(change: &FileChange) -> &'static str { match change { FileChange::Add { .. } => "A", - FileChange::Delete => "D", + FileChange::Delete { .. } => "D", FileChange::Update { move_path: Some(_), .. } => "R", diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 785272a6..124d3390 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -13,13 +13,13 @@ use codex_core::ConversationManager; use codex_core::NewConversation; use codex_core::config::Config; use codex_core::config::ConfigOverrides; +use codex_core::git_info::get_git_repo_root; use codex_core::protocol::AskForApproval; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; use codex_core::protocol::TaskCompleteEvent; -use codex_core::util::is_inside_git_repo; use codex_login::AuthManager; use codex_ollama::DEFAULT_OSS_MODEL; use codex_protocol::config_types::SandboxMode; @@ -183,7 +183,7 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any // is using. event_processor.print_config_summary(&config, &prompt); - if !skip_git_repo_check && !is_inside_git_repo(&config.cwd.to_path_buf()) { + if !skip_git_repo_check && get_git_repo_root(&config.cwd.to_path_buf()).is_none() { eprintln!("Not inside a trusted directory and --skip-git-repo-check was not specified."); std::process::exit(1); } diff --git a/codex-rs/protocol/src/parse_command.rs b/codex-rs/protocol/src/parse_command.rs index 63399acc..326f6cf5 100644 --- a/codex-rs/protocol/src/parse_command.rs +++ b/codex-rs/protocol/src/parse_command.rs @@ -17,22 +17,6 @@ pub enum ParsedCommand { query: Option, path: Option, }, - Format { - cmd: String, - tool: Option, - targets: Option>, - }, - Test { - cmd: String, - }, - Lint { - cmd: String, - tool: Option, - targets: Option>, - }, - Noop { - cmd: String, - }, Unknown { cmd: String, }, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 1f6cbe07..315ec09f 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -869,7 +869,9 @@ pub enum FileChange { Add { content: String, }, - Delete, + Delete { + content: String, + }, Update { unified_diff: String, move_path: Option, diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 8e5fdf1a..21fe3b3d 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -49,6 +49,7 @@ image = { version = "^0.25.6", default-features = false, features = [ "jpeg", "png", ] } +itertools = "0.14.0" lazy_static = "1" mcp-types = { path = "../mcp-types" } once_cell = "1" @@ -87,6 +88,7 @@ unicode-segmentation = "1.12.0" unicode-width = "0.1" url = "2" uuid = "1" +pathdiff = "0.2" [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 6d107d67..de548337 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -43,6 +43,7 @@ pub(crate) struct App { // Pager overlay state (Transcript or Static like Diff) pub(crate) overlay: Option, pub(crate) deferred_history_lines: Vec>, + has_emitted_history_lines: bool, pub(crate) enhanced_keys_supported: bool, @@ -91,6 +92,7 @@ impl App { transcript_lines: Vec::new(), overlay: None, deferred_history_lines: Vec::new(), + has_emitted_history_lines: false, commit_anim_running: Arc::new(AtomicBool::new(false)), backtrack: BacktrackState::default(), }; @@ -177,27 +179,23 @@ impl App { ); tui.frame_requester().schedule_frame(); } - AppEvent::InsertHistoryLines(lines) => { - if let Some(Overlay::Transcript(t)) = &mut self.overlay { - t.insert_lines(lines.clone()); - tui.frame_requester().schedule_frame(); - } - self.transcript_lines.extend(lines.clone()); - if self.overlay.is_some() { - self.deferred_history_lines.extend(lines); - } else { - tui.insert_history_lines(lines); - } - } AppEvent::InsertHistoryCell(cell) => { - let cell_transcript = cell.transcript_lines(); + let mut cell_transcript = cell.transcript_lines(); + if !cell.is_stream_continuation() && !self.transcript_lines.is_empty() { + cell_transcript.insert(0, Line::from("")); + } if let Some(Overlay::Transcript(t)) = &mut self.overlay { t.insert_lines(cell_transcript.clone()); tui.frame_requester().schedule_frame(); } self.transcript_lines.extend(cell_transcript.clone()); - let display = cell.display_lines(); + let mut display = cell.display_lines(tui.terminal.last_known_screen_size.width); if !display.is_empty() { + if self.has_emitted_history_lines { + display.insert(0, Line::from("")); + } else { + self.has_emitted_history_lines = true; + } if self.overlay.is_some() { self.deferred_history_lines.extend(display); } else { diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 439edbfd..1896f0a4 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -1,7 +1,6 @@ use codex_core::protocol::ConversationHistoryResponseEvent; use codex_core::protocol::Event; use codex_file_search::FileMatch; -use ratatui::text::Line; use crate::history_cell::HistoryCell; @@ -40,7 +39,6 @@ pub(crate) enum AppEvent { /// Result of computing a `/diff` command. DiffResult(String), - InsertHistoryLines(Vec>), InsertHistoryCell(Box), StartCommitAnimation, diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 68f9a76f..fc44d5ef 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -27,6 +27,7 @@ use super::command_popup::CommandPopup; use super::file_search_popup::FileSearchPopup; use super::paste_burst::CharDecision; use super::paste_burst::PasteBurst; +use crate::bottom_pane::paste_burst::FlushResult; use crate::slash_command::SlashCommand; use codex_protocol::custom_prompts::CustomPrompt; @@ -223,7 +224,7 @@ impl ChatComposer { let placeholder = format!("[Pasted Content {char_count} chars]"); self.textarea.insert_element(&placeholder); self.pending_pastes.push((placeholder, pasted)); - } else if self.handle_paste_image_path(pasted.clone()) { + } else if char_count > 1 && self.handle_paste_image_path(pasted.clone()) { self.textarea.insert_str(" "); } else { self.textarea.insert_str(&pasted); @@ -298,12 +299,7 @@ impl ChatComposer { } pub(crate) fn flush_paste_burst_if_due(&mut self) -> bool { - let now = Instant::now(); - if let Some(pasted) = self.paste_burst.flush_if_due(now) { - let _ = self.handle_paste(pasted); - return true; - } - false + self.handle_paste_burst_flush(Instant::now()) } pub(crate) fn is_in_paste_burst(&self) -> bool { @@ -396,9 +392,11 @@ impl ChatComposer { KeyEvent { code: KeyCode::Tab, .. } => { + // Ensure popup filtering/selection reflects the latest composer text + // before applying completion. + let first_line = self.textarea.text().lines().next().unwrap_or(""); + popup.on_composer_text_change(first_line.to_string()); if let Some(sel) = popup.selected_item() { - let first_line = self.textarea.text().lines().next().unwrap_or(""); - match sel { CommandItem::Builtin(cmd) => { let starts_with_cmd = first_line @@ -853,15 +851,36 @@ impl ChatComposer { } } + fn handle_paste_burst_flush(&mut self, now: Instant) -> bool { + match self.paste_burst.flush_if_due(now) { + FlushResult::Paste(pasted) => { + self.handle_paste(pasted); + true + } + FlushResult::Typed(ch) => { + // Mirror insert_str() behavior so popups stay in sync when a + // pending fast char flushes as normal typed input. + self.textarea.insert_str(ch.to_string().as_str()); + // Keep popup sync consistent with key handling: prefer slash popup; only + // sync file popup when slash popup is NOT active. + self.sync_command_popup(); + if matches!(self.active_popup, ActivePopup::Command(_)) { + self.dismissed_file_popup_token = None; + } else { + self.sync_file_search_popup(); + } + true + } + FlushResult::None => false, + } + } + /// Handle generic Input events that modify the textarea content. fn handle_input_basic(&mut self, input: KeyEvent) -> (InputResult, bool) { // If we have a buffered non-bracketed paste burst and enough time has // elapsed since the last char, flush it before handling a new input. let now = Instant::now(); - if let Some(pasted) = self.paste_burst.flush_if_due(now) { - // Reuse normal paste path (handles large-paste placeholders). - self.handle_paste(pasted); - } + self.handle_paste_burst_flush(now); // If we're capturing a burst and receive Enter, accumulate it instead of inserting. if matches!(input.code, KeyCode::Enter) @@ -1672,6 +1691,66 @@ mod tests { } } + #[test] + fn slash_popup_model_first_for_mo_ui() { + use insta::assert_snapshot; + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + // Type "/mo" humanlike so paste-burst doesn’t interfere. + type_chars_humanlike(&mut composer, &['/', 'm', 'o']); + + let mut terminal = match Terminal::new(TestBackend::new(60, 4)) { + Ok(t) => t, + Err(e) => panic!("Failed to create terminal: {e}"), + }; + terminal + .draw(|f| f.render_widget_ref(composer, f.area())) + .unwrap_or_else(|e| panic!("Failed to draw composer: {e}")); + + // Visual snapshot should show the slash popup with /model as the first entry. + assert_snapshot!("slash_popup_mo", terminal.backend()); + } + + #[test] + fn slash_popup_model_first_for_mo_logic() { + use super::super::command_popup::CommandItem; + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + type_chars_humanlike(&mut composer, &['/', 'm', 'o']); + + match &composer.active_popup { + ActivePopup::Command(popup) => match popup.selected_item() { + Some(CommandItem::Builtin(cmd)) => { + assert_eq!(cmd.command(), "model") + } + Some(CommandItem::UserPrompt(_)) => { + panic!("unexpected prompt selected for '/mo'") + } + None => panic!("no selected command for '/mo'"), + }, + _ => panic!("slash popup not active after typing '/mo'"), + } + } + // Test helper: simulate human typing with a brief delay and flush the paste-burst buffer fn type_chars_humanlike(composer: &mut ChatComposer, chars: &[char]) { use crossterm::event::KeyCode; diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 4e60e7ea..4a0f3c13 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -238,6 +238,20 @@ mod tests { } } + #[test] + fn model_is_first_suggestion_for_mo() { + let mut popup = CommandPopup::new(Vec::new()); + popup.on_composer_text_change("/mo".to_string()); + let matches = popup.filtered_items(); + match matches.first() { + Some(CommandItem::Builtin(cmd)) => assert_eq!(cmd.command(), "model"), + Some(CommandItem::UserPrompt(_)) => { + panic!("unexpected prompt ranked before '/model' for '/mo'") + } + None => panic!("expected at least one match for '/mo'"), + } + } + #[test] fn prompt_discovery_lists_custom_prompts() { let prompts = vec![ diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index b94db79f..5ab32849 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -100,53 +100,47 @@ impl BottomPane { } pub fn desired_height(&self, width: u16) -> u16 { - let top_margin = if self.active_view.is_some() { 0 } else { 1 }; + // Always reserve one blank row above the pane for visual spacing. + let top_margin = 1; // Base height depends on whether a modal/overlay is active. - let mut base = if let Some(view) = self.active_view.as_ref() { - view.desired_height(width) - } else { - self.composer.desired_height(width) + let base = match self.active_view.as_ref() { + Some(view) => view.desired_height(width), + None => self.composer.desired_height(width).saturating_add( + self.status + .as_ref() + .map_or(0, |status| status.desired_height(width)), + ), }; - // If a status indicator is active and no modal is covering the composer, - // include its height above the composer. - if self.active_view.is_none() - && let Some(status) = self.status.as_ref() - { - base = base.saturating_add(status.desired_height(width)); - } // Account for bottom padding rows. Top spacing is handled in layout(). base.saturating_add(Self::BOTTOM_PAD_LINES) .saturating_add(top_margin) } fn layout(&self, area: Rect) -> [Rect; 2] { - // Prefer showing the status header when space is extremely tight. - // Drop the top spacer if there is only one row available. - let mut top_margin = if self.active_view.is_some() { 0 } else { 1 }; - if area.height <= 1 { - top_margin = 0; - } - - let status_height = if self.active_view.is_none() { - if let Some(status) = self.status.as_ref() { - status.desired_height(area.width) - } else { - 0 - } + // At small heights, bottom pane takes the entire height. + let (top_margin, bottom_margin) = if area.height <= BottomPane::BOTTOM_PAD_LINES + 1 { + (0, 0) } else { - 0 + (1, BottomPane::BOTTOM_PAD_LINES) }; - let [_, status, content, _] = Layout::vertical([ - Constraint::Max(top_margin), - Constraint::Max(status_height), - Constraint::Min(1), - Constraint::Max(BottomPane::BOTTOM_PAD_LINES), - ]) - .areas(area); - - [status, content] + let area = Rect { + x: area.x, + y: area.y + top_margin, + width: area.width, + height: area.height - top_margin - bottom_margin, + }; + match self.active_view.as_ref() { + Some(_) => [Rect::ZERO, area], + None => { + let status_height = self + .status + .as_ref() + .map_or(0, |status| status.desired_height(area.width)); + Layout::vertical([Constraint::Max(status_height), Constraint::Min(1)]).areas(area) + } + } } pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { @@ -705,7 +699,7 @@ mod tests { pane.set_task_running(true); - // Height=2 → composer visible; status is hidden to preserve composer. Spacer may collapse. + // Height=2 → status on one row, composer on the other. let area2 = Rect::new(0, 0, 20, 2); let mut buf2 = Buffer::empty(area2); (&pane).render_ref(area2, &mut buf2); @@ -721,8 +715,8 @@ mod tests { "expected composer to be visible on one of the rows: row0={row0:?}, row1={row1:?}" ); assert!( - !row0.contains("Working") && !row1.contains("Working"), - "status header should be hidden when height=2" + row0.contains("Working") || row1.contains("Working"), + "expected status header to be visible at height=2: row0={row0:?}, row1={row1:?}" ); // Height=1 → no padding; single row is the composer (status hidden). diff --git a/codex-rs/tui/src/bottom_pane/paste_burst.rs b/codex-rs/tui/src/bottom_pane/paste_burst.rs index b353d867..d507dcae 100644 --- a/codex-rs/tui/src/bottom_pane/paste_burst.rs +++ b/codex-rs/tui/src/bottom_pane/paste_burst.rs @@ -35,6 +35,12 @@ pub(crate) struct RetroGrab { pub grabbed: String, } +pub(crate) enum FlushResult { + Paste(String), + Typed(char), + None, +} + impl PasteBurst { /// Recommended delay to wait between simulated keypresses (or before /// scheduling a UI tick) so that a pending fast keystroke is flushed @@ -95,24 +101,24 @@ impl PasteBurst { /// now emit that char as normal typed input. /// /// Returns None if the timeout has not elapsed or there is nothing to flush. - pub fn flush_if_due(&mut self, now: Instant) -> Option { + pub fn flush_if_due(&mut self, now: Instant) -> FlushResult { let timed_out = self .last_plain_char_time .is_some_and(|t| now.duration_since(t) > PASTE_BURST_CHAR_INTERVAL); if timed_out && self.is_active_internal() { self.active = false; let out = std::mem::take(&mut self.buffer); - Some(out) + FlushResult::Paste(out) } else if timed_out { // If we were saving a single fast char and no burst followed, // flush it as normal typed input. if let Some((ch, _at)) = self.pending_first_char.take() { - Some(ch.to_string()) + FlushResult::Typed(ch) } else { - None + FlushResult::None } } else { - None + FlushResult::None } } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap new file mode 100644 index 00000000..63f9b3a9 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__slash_popup_mo.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +"▌/mo " +"▌ " +"▌/model choose what model and reasoning effort to use " +"▌/mention mention a file " diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 2ad8c440..cb6ef501 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -245,7 +245,6 @@ impl TextArea { } => self.delete_backward_word(), KeyEvent { code: KeyCode::Backspace, - modifiers: KeyModifiers::NONE, .. } | KeyEvent { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 946d6de9..87d14117 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -101,7 +101,6 @@ pub(crate) struct ChatWidget { // Stream lifecycle controller stream: StreamController, running_commands: HashMap, - pending_exec_completions: Vec<(Vec, Vec, CommandOutput)>, task_complete_pending: bool, // Queue of interruptive UI events deferred during an active write cycle interrupts: InterruptManager, @@ -113,7 +112,6 @@ pub(crate) struct ChatWidget { frame_requester: FrameRequester, // Whether to include the initial welcome banner on session configured show_welcome_banner: bool, - last_history_was_exec: bool, // User messages queued while a turn is in progress queued_user_messages: VecDeque, } @@ -333,6 +331,7 @@ impl ChatWidget { auto_approved: event.auto_approved, }, event.changes, + &self.config.cwd, )); } @@ -442,14 +441,14 @@ impl ChatWidget { self.task_complete_pending = false; } // A completed stream indicates non-exec content was just inserted. - // Reset the exec header grouping so the next exec shows its header. - self.last_history_was_exec = false; self.flush_interrupt_queue(); } } #[inline] fn handle_streaming_delta(&mut self, delta: String) { + // Before streaming agent content, flush any active exec cell group. + self.flush_active_exec_cell(); let sink = AppEventHistorySink(self.app_event_tx.clone()); self.stream.begin(&sink); self.stream.push_and_maybe_commit(&delta, &sink); @@ -462,31 +461,29 @@ impl ChatWidget { Some(rc) => (rc.command, rc.parsed_cmd), None => (vec![ev.call_id.clone()], Vec::new()), }; - self.pending_exec_completions.push(( - command, - parsed, - CommandOutput { - exit_code: ev.exit_code, - stdout: ev.stdout.clone(), - stderr: ev.stderr.clone(), - formatted_output: ev.formatted_output.clone(), - }, - )); - if self.running_commands.is_empty() { - self.active_exec_cell = None; - let pending = std::mem::take(&mut self.pending_exec_completions); - for (command, parsed, output) in pending { - let include_header = !self.last_history_was_exec; - let cell = history_cell::new_completed_exec_command( - command, - parsed, - output, - include_header, - ev.duration, - ); - self.add_to_history(cell); - self.last_history_was_exec = true; + if self.active_exec_cell.is_none() { + // This should have been created by handle_exec_begin_now, but in case it wasn't, + // create it now. + self.active_exec_cell = Some(history_cell::new_active_exec_command( + ev.call_id.clone(), + command, + parsed, + )); + } + if let Some(cell) = self.active_exec_cell.as_mut() { + cell.complete_call( + &ev.call_id, + CommandOutput { + exit_code: ev.exit_code, + stdout: ev.stdout.clone(), + stderr: ev.stderr.clone(), + formatted_output: ev.formatted_output.clone(), + }, + ev.duration, + ); + if cell.should_flush() { + self.flush_active_exec_cell(); } } } @@ -495,9 +492,9 @@ impl ChatWidget { &mut self, event: codex_core::protocol::PatchApplyEndEvent, ) { - if event.success { - self.add_to_history(history_cell::new_patch_apply_success(event.stdout)); - } else { + // If the patch was successful, just let the "Edited" block stand. + // Otherwise, add a failure block. + if !event.success { self.add_to_history(history_cell::new_patch_apply_failure(event.stderr)); } } @@ -523,6 +520,7 @@ impl ChatWidget { self.add_to_history(history_cell::new_patch_event( PatchEventType::ApprovalRequest, ev.changes.clone(), + &self.config.cwd, )); let request = ApprovalRequest::ApplyPatch { @@ -543,19 +541,28 @@ impl ChatWidget { parsed_cmd: ev.parsed_cmd.clone(), }, ); - // Accumulate parsed commands into a single active Exec cell so they stack - match self.active_exec_cell.as_mut() { - Some(exec) => { - exec.parsed.extend(ev.parsed_cmd); - } - _ => { - let include_header = !self.last_history_was_exec; + if let Some(exec) = &self.active_exec_cell { + if let Some(new_exec) = exec.with_added_call( + ev.call_id.clone(), + ev.command.clone(), + ev.parsed_cmd.clone(), + ) { + self.active_exec_cell = Some(new_exec); + } else { + // Make a new cell. + self.flush_active_exec_cell(); self.active_exec_cell = Some(history_cell::new_active_exec_command( - ev.command, - ev.parsed_cmd, - include_header, + ev.call_id.clone(), + ev.command.clone(), + ev.parsed_cmd.clone(), )); } + } else { + self.active_exec_cell = Some(history_cell::new_active_exec_command( + ev.call_id.clone(), + ev.command.clone(), + ev.parsed_cmd.clone(), + )); } // Request a redraw so the working header and command list are visible immediately. @@ -585,7 +592,7 @@ impl ChatWidget { Constraint::Max( self.active_exec_cell .as_ref() - .map_or(0, |c| c.desired_height(area.width)), + .map_or(0, |c| c.desired_height(area.width) + 1), ), Constraint::Min(self.bottom_pane.desired_height(area.width)), ]) @@ -627,13 +634,11 @@ impl ChatWidget { last_token_usage: TokenUsage::default(), stream: StreamController::new(config), running_commands: HashMap::new(), - pending_exec_completions: Vec::new(), task_complete_pending: false, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), session_id: None, - last_history_was_exec: false, queued_user_messages: VecDeque::new(), show_welcome_banner: true, } @@ -673,13 +678,11 @@ impl ChatWidget { last_token_usage: TokenUsage::default(), stream: StreamController::new(config), running_commands: HashMap::new(), - pending_exec_completions: Vec::new(), task_complete_pending: false, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), session_id: None, - last_history_was_exec: false, queued_user_messages: VecDeque::new(), show_welcome_banner: false, } @@ -690,7 +693,7 @@ impl ChatWidget { + self .active_exec_cell .as_ref() - .map_or(0, |c| c.desired_height(width)) + .map_or(0, |c| c.desired_height(width) + 1) } pub(crate) fn handle_key_event(&mut self, key_event: KeyEvent) { @@ -891,18 +894,15 @@ impl ChatWidget { fn flush_active_exec_cell(&mut self) { if let Some(active) = self.active_exec_cell.take() { - self.last_history_was_exec = true; self.app_event_tx .send(AppEvent::InsertHistoryCell(Box::new(active))); } } fn add_to_history(&mut self, cell: impl HistoryCell + 'static) { - // Only break exec grouping if the cell renders visible lines. - let has_display_lines = !cell.display_lines().is_empty(); - self.flush_active_exec_cell(); - if has_display_lines { - self.last_history_was_exec = false; + if !cell.display_lines(u16::MAX).is_empty() { + // Only break exec grouping if the cell renders visible lines. + self.flush_active_exec_cell(); } self.app_event_tx .send(AppEvent::InsertHistoryCell(Box::new(cell))); @@ -1028,7 +1028,6 @@ impl ChatWidget { let cell = cell.into_failed(); // Insert finalized exec into history and keep grouping consistent. self.add_to_history(cell); - self.last_history_was_exec = true; } } @@ -1284,6 +1283,9 @@ impl WidgetRef for &ChatWidget { let [active_cell_area, bottom_pane_area] = self.layout_areas(area); (&self.bottom_pane).render(bottom_pane_area, buf); if let Some(cell) = &self.active_exec_cell { + let mut active_cell_area = active_cell_area; + active_cell_area.y += 1; + active_cell_area.height -= 1; cell.render_ref(active_cell_area, buf); } } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_approved.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_approved.snap new file mode 100644 index 00000000..3ed1e075 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_approved.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: lines_to_single_string(&approved_lines) +--- +• Change Approved foo.txt (+1 -0) diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_proposed.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_proposed.snap new file mode 100644 index 00000000..2d0c73ca --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_proposed.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: lines_to_single_string(&proposed_lines) +--- +• Proposed Change foo.txt (+1 -0) + 1 +hello diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap index e3289220..e190ef4d 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap @@ -3,6 +3,7 @@ source: tui/src/chatwidget/tests.rs assertion_line: 728 expression: terminal.backend() --- +" " "? Codex wants to run echo hello world " " " "Model wants to run a command " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap new file mode 100644 index 00000000..f8aee639 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +" " +"? Codex wants to run echo hello world " +" " +"▌Allow command? " +"▌ Yes Always No, provide feedback " +"▌ Approve and run the command " +" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap index b4373640..0b9b10f3 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap @@ -1,8 +1,9 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 763 +assertion_line: 794 expression: terminal.backend() --- +" " "The model wants to apply changes " " " "This will grant write access to /tmp for the remainder of this session. " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap new file mode 100644 index 00000000..5d6d5cb4 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__binary_size_ideal_response.snap @@ -0,0 +1,57 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: visible_after +--- +> I’m going to scan the workspace and Cargo manifests to see build profiles and + dependencies that impact binary size. Then I’ll summarize the main causes. + +• Explored + └ List ls -la + Read Cargo.toml + +• Ran + └ for d in ansi-escape apply-patch arg0 cli common core exec execpolicy + file-search linux-sandbox login mcp-client mcp-server mcp-types ollama + tui; do echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; + done + +• Explored + └ Read Cargo.toml + +> Here’s what’s driving size in this workspace’s binaries. + + Main Causes + + - Static linking style: Each bin (codex, codex-tui, codex-exec, + codex-mcp-server, etc.) statically links its full dependency graph, so common + code isn’t shared at runtime across executables. + - Heavy deps (HTTP/TLS): reqwest brings in Hyper, HTTP/2, compressors, + and a TLS stack (rustls by default; OpenSSL on musl). In core, login, tui, + and ollama you enable reqwest with json/stream, which still pulls a large + transitive set. + - Image/terminal stack: codex-tui includes image (with jpeg), ratatui, + crossterm, and ratatui-image, which together contribute significant code size + (decoders, pixel ops, terminal backends). + - Parsers/VMs: tree-sitter + tree-sitter-bash (in core and apply-patch) and + starlark (in execpolicy) include sizeable parser/VM tables and runtimes. + - Tokio runtime: Broad tokio features (rt-multi-thread, macros, process, + signal) across many crates inflate code size even if only subsets are used + per bin. + - Panic + backtraces: Default panic = unwind and backtrace support keep + unwinding tables and symbols that add weight. + - Per-target OpenSSL (musl): For *-unknown-linux-musl, core enables + openssl-sys with vendored, compiling OpenSSL into the binary—this adds + multiple megabytes per executable. + + Build-Mode Notes + + - Release settings: You use lto = "fat" and codegen-units = 1 (good for size), + but strip = "symbols" keeps debuginfo. Debuginfo is often the largest single + contributor; if you build in release with that setting, binaries can still + be large. + - Debug builds: cargo build (dev profile) includes full debuginfo, no LTO, and + assertions—outputs are much larger than cargo build --release. + + If you want, I can outline targeted trims (e.g., strip = "debuginfo", + opt-level = "z", panic abort, tighter tokio/reqwest features) and estimate + impact per binary. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap index bf8bd08b..aeb69e84 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap @@ -1,7 +1,6 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 779 expression: terminal.backend() --- "▌ Ask Codex to do anything " -" " +" ⏎ send Ctrl+J newline Ctrl+T transc" diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap index 11047736..c77477a0 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h3.snap @@ -1,6 +1,5 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 779 expression: terminal.backend() --- " " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap index 7f84982a..f5097286 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h2.snap @@ -1,7 +1,6 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 807 expression: terminal.backend() --- +" Thinking (0s • Esc to interrupt) " "▌ Ask Codex to do anything " -" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap index dbe9cbed..c77477a0 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_running_h3.snap @@ -1,6 +1,5 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 807 expression: terminal.backend() --- " " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap new file mode 100644 index 00000000..77087030 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap @@ -0,0 +1,15 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: visual +--- +> I’m going to search the repo for where “Change Approved” is rendered to update + that view. + +• Explored + └ Search Change Approved + Read diff_render.rs + + Investigating rendering code (0s • Esc to interrupt) + +▌Summarize recent commits + ⏎ send Ctrl+J newline Ctrl+T transcript Ctrl+C quit diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__deltas_then_same_final_message_are_rendered_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__deltas_then_same_final_message_are_rendered_snapshot.snap index 2bc5c75e..ae86ddc8 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__deltas_then_same_final_message_are_rendered_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__deltas_then_same_final_message_are_rendered_snapshot.snap @@ -2,5 +2,4 @@ source: tui/src/chatwidget/tests.rs expression: combined --- -codex -Here is the result. +> Here is the result. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step1_start_ls.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step1_start_ls.snap new file mode 100644 index 00000000..3b338006 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step1_start_ls.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: blob1 +--- +⠋ Exploring + └ List ls -la diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step2_finish_ls.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step2_finish_ls.snap new file mode 100644 index 00000000..492e8b77 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step2_finish_ls.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: blob2 +--- +• Explored + └ List ls -la diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step3_start_cat_foo.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step3_start_cat_foo.snap new file mode 100644 index 00000000..ddcd484b --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step3_start_cat_foo.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: blob3 +--- +⠋ Exploring + └ List ls -la + Read foo.txt diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step4_finish_cat_foo.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step4_finish_cat_foo.snap new file mode 100644 index 00000000..9e29785f --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step4_finish_cat_foo.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: blob4 +--- +• Explored + └ List ls -la + Read foo.txt diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step5_finish_sed_range.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step5_finish_sed_range.snap new file mode 100644 index 00000000..296b00f9 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step5_finish_sed_range.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: blob5 +--- +• Explored + └ List ls -la + Read foo.txt diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step6_finish_cat_bar.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step6_finish_cat_bar.snap new file mode 100644 index 00000000..55fa9791 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exploring_step6_finish_cat_bar.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: blob6 +--- +• Explored + └ List ls -la + Read foo.txt, bar.txt diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__final_reasoning_then_message_without_deltas_are_rendered.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__final_reasoning_then_message_without_deltas_are_rendered.snap index 2bc5c75e..ae86ddc8 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__final_reasoning_then_message_without_deltas_are_rendered.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__final_reasoning_then_message_without_deltas_are_rendered.snap @@ -2,5 +2,4 @@ source: tui/src/chatwidget/tests.rs expression: combined --- -codex -Here is the result. +> Here is the result. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_exec_marks_failed.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_exec_marks_failed.snap index 1554e94f..5098b824 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_exec_marks_failed.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupt_exec_marks_failed.snap @@ -2,5 +2,4 @@ source: tui/src/chatwidget/tests.rs expression: exec_blob --- ->_ - ✗ ⌨️ sleep 1 +• Ran sleep 1 diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap index c598dbe1..72bb3c34 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap @@ -1,6 +1,5 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 878 expression: terminal.backend() --- " " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap index dc7619c6..645ddb8e 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap @@ -1,8 +1,9 @@ --- source: tui/src/chatwidget/tests.rs -assertion_line: 851 +assertion_line: 921 expression: terminal.backend() --- +" " "? Codex wants to run echo 'hello world' " " " "Codex wants to run a command " diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a1adfaef..4ad82c75 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -32,7 +32,6 @@ use pretty_assertions::assert_eq; use std::fs::File; use std::io::BufRead; use std::io::BufReader; -use std::io::Read; use std::path::PathBuf; use tokio::sync::mpsc::unbounded_channel; @@ -177,7 +176,6 @@ fn make_chatwidget_manual() -> ( last_token_usage: TokenUsage::default(), stream: StreamController::new(cfg), running_commands: HashMap::new(), - pending_exec_completions: Vec::new(), task_complete_pending: false, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), @@ -185,7 +183,6 @@ fn make_chatwidget_manual() -> ( session_id: None, frame_requester: crate::tui::FrameRequester::test_dummy(), show_welcome_banner: true, - last_history_was_exec: false, queued_user_messages: std::collections::VecDeque::new(), }; (widget, rx, op_rx) @@ -196,10 +193,12 @@ fn drain_insert_history( ) -> Vec>> { let mut out = Vec::new(); while let Ok(ev) = rx.try_recv() { - match ev { - AppEvent::InsertHistoryLines(lines) => out.push(lines), - AppEvent::InsertHistoryCell(cell) => out.push(cell.display_lines()), - _ => {} + if let AppEvent::InsertHistoryCell(cell) = ev { + let mut lines = cell.display_lines(80); + if !cell.is_stream_continuation() && !out.is_empty() && !lines.is_empty() { + lines.insert(0, "".into()); + } + out.push(lines) } } out @@ -216,23 +215,54 @@ fn lines_to_single_string(lines: &[ratatui::text::Line<'static>]) -> String { s } -#[test] -fn disabled_slash_command_while_task_running_snapshot() { - // Build a chat widget and simulate an active task - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); - chat.bottom_pane.set_task_running(true); +// --- Small helpers to tersely drive exec begin/end and snapshot active cell --- +fn begin_exec(chat: &mut ChatWidget, call_id: &str, raw_cmd: &str) { + // Build the full command vec and parse it using core's parser, + // then convert to protocol variants for the event payload. + let command = vec!["bash".to_string(), "-lc".to_string(), raw_cmd.to_string()]; + let parsed_cmd: Vec = + codex_core::parse_command::parse_command(&command) + .into_iter() + .map(Into::into) + .collect(); + chat.handle_codex_event(Event { + id: call_id.to_string(), + msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id: call_id.to_string(), + command, + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + parsed_cmd, + }), + }); +} - // Dispatch a command that is unavailable while a task runs (e.g., /model) - chat.dispatch_command(SlashCommand::Model); +fn end_exec(chat: &mut ChatWidget, call_id: &str, stdout: &str, stderr: &str, exit_code: i32) { + let aggregated = if stderr.is_empty() { + stdout.to_string() + } else { + format!("{stdout}{stderr}") + }; + chat.handle_codex_event(Event { + id: call_id.to_string(), + msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id: call_id.to_string(), + stdout: stdout.to_string(), + stderr: stderr.to_string(), + aggregated_output: aggregated.clone(), + exit_code, + duration: std::time::Duration::from_millis(5), + formatted_output: aggregated, + }), + }); +} - // Drain history and snapshot the rendered error line(s) - let cells = drain_insert_history(&mut rx); - assert!( - !cells.is_empty(), - "expected an error message history cell to be emitted", - ); - let blob = lines_to_single_string(cells.last().unwrap()); - assert_snapshot!(blob); +fn active_blob(chat: &ChatWidget) -> String { + let lines = chat + .active_exec_cell + .as_ref() + .expect("active exec cell present") + .display_lines(80); + lines_to_single_string(&lines) } fn open_fixture(name: &str) -> std::fs::File { @@ -308,45 +338,24 @@ fn exec_history_cell_shows_working_then_completed() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); // Begin command - chat.handle_codex_event(Event { - id: "call-1".into(), - msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { - call_id: "call-1".into(), - command: vec!["bash".into(), "-lc".into(), "echo done".into()], - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), - parsed_cmd: vec![ - codex_core::parse_command::ParsedCommand::Unknown { - cmd: "echo done".into(), - } - .into(), - ], - }), - }); - - // End command successfully - chat.handle_codex_event(Event { - id: "call-1".into(), - msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { - call_id: "call-1".into(), - stdout: "done".into(), - stderr: String::new(), - aggregated_output: "done".into(), - exit_code: 0, - duration: std::time::Duration::from_millis(5), - formatted_output: "done".into(), - }), - }); + begin_exec(&mut chat, "call-1", "echo done"); let cells = drain_insert_history(&mut rx); - assert_eq!( - cells.len(), - 1, - "expected only the completed exec cell to be inserted into history" - ); - let blob = lines_to_single_string(&cells[0]); + assert_eq!(cells.len(), 0, "no exec cell should have been flushed yet"); + + // End command successfully + end_exec(&mut chat, "call-1", "done", "", 0); + + let cells = drain_insert_history(&mut rx); + // Exec end now finalizes and flushes the exec cell immediately. + assert_eq!(cells.len(), 1, "expected finalized exec cell to flush"); + // Inspect the flushed exec cell rendering. + let lines = &cells[0]; + let blob = lines_to_single_string(lines); + // New behavior: no glyph markers; ensure command is shown and no panic. assert!( - blob.contains('✓'), - "expected completed exec cell to show success marker: {blob:?}" + blob.contains("• Ran"), + "expected summary header present: {blob:?}" ); assert!( blob.contains("echo done"), @@ -359,50 +368,23 @@ fn exec_history_cell_shows_working_then_failed() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); // Begin command - chat.handle_codex_event(Event { - id: "call-2".into(), - msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { - call_id: "call-2".into(), - command: vec!["bash".into(), "-lc".into(), "false".into()], - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), - parsed_cmd: vec![ - codex_core::parse_command::ParsedCommand::Unknown { - cmd: "false".into(), - } - .into(), - ], - }), - }); + begin_exec(&mut chat, "call-2", "false"); + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 0, "no exec cell should have been flushed yet"); // End command with failure - chat.handle_codex_event(Event { - id: "call-2".into(), - msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { - call_id: "call-2".into(), - stdout: String::new(), - stderr: "error".into(), - aggregated_output: "error".into(), - exit_code: 2, - duration: std::time::Duration::from_millis(7), - formatted_output: "".into(), - }), - }); + end_exec(&mut chat, "call-2", "", "Bloop", 2); let cells = drain_insert_history(&mut rx); - assert_eq!( - cells.len(), - 1, - "expected only the completed exec cell to be inserted into history" - ); - let blob = lines_to_single_string(&cells[0]); + // Exec end with failure should also flush immediately. + assert_eq!(cells.len(), 1, "expected finalized exec cell to flush"); + let lines = &cells[0]; + let blob = lines_to_single_string(lines); assert!( - blob.contains('✗'), - "expected failure marker present: {blob:?}" - ); - assert!( - blob.contains("false"), - "expected command text present: {blob:?}" + blob.contains("• Ran false"), + "expected command and header text present: {blob:?}" ); + assert!(blob.to_lowercase().contains("bloop"), "expected error text"); } // Snapshot test: interrupting a running exec finalizes the active cell with a red ✗ @@ -412,20 +394,7 @@ fn interrupt_exec_marks_failed_snapshot() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); // Begin a long-running command so we have an active exec cell with a spinner. - chat.handle_codex_event(Event { - id: "call-int".into(), - msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { - call_id: "call-int".into(), - command: vec!["bash".into(), "-lc".into(), "sleep 1".into()], - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), - parsed_cmd: vec![ - codex_core::parse_command::ParsedCommand::Unknown { - cmd: "sleep 1".into(), - } - .into(), - ], - }), - }); + begin_exec(&mut chat, "call-int", "sleep 1"); // Simulate the task being aborted (as if ESC was pressed), which should // cause the active exec cell to be finalized as failed and flushed. @@ -449,75 +418,56 @@ fn interrupt_exec_marks_failed_snapshot() { #[test] fn exec_history_extends_previous_when_consecutive() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + + // 1) Start "ls -la" (List) + begin_exec(&mut chat, "call-ls", "ls -la"); + assert_snapshot!("exploring_step1_start_ls", active_blob(&chat)); + + // 2) Finish "ls -la" + end_exec(&mut chat, "call-ls", "", "", 0); + assert_snapshot!("exploring_step2_finish_ls", active_blob(&chat)); + + // 3) Start "cat foo.txt" (Read) + begin_exec(&mut chat, "call-cat-foo", "cat foo.txt"); + assert_snapshot!("exploring_step3_start_cat_foo", active_blob(&chat)); + + // 4) Complete "cat foo.txt" + end_exec(&mut chat, "call-cat-foo", "hello from foo", "", 0); + assert_snapshot!("exploring_step4_finish_cat_foo", active_blob(&chat)); + + // 5) Start & complete "sed -n 100,200p foo.txt" (treated as Read of foo.txt) + begin_exec(&mut chat, "call-sed-range", "sed -n 100,200p foo.txt"); + end_exec(&mut chat, "call-sed-range", "chunk", "", 0); + assert_snapshot!("exploring_step5_finish_sed_range", active_blob(&chat)); + + // 6) Start & complete "cat bar.txt" + begin_exec(&mut chat, "call-cat-bar", "cat bar.txt"); + end_exec(&mut chat, "call-cat-bar", "hello from bar", "", 0); + assert_snapshot!("exploring_step6_finish_cat_bar", active_blob(&chat)); +} + +#[test] +fn disabled_slash_command_while_task_running_snapshot() { + // Build a chat widget and simulate an active task let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + chat.bottom_pane.set_task_running(true); - // First command - chat.handle_codex_event(Event { - id: "call-a".into(), - msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { - call_id: "call-a".into(), - command: vec!["bash".into(), "-lc".into(), "echo one".into()], - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), - parsed_cmd: vec![ - codex_core::parse_command::ParsedCommand::Unknown { - cmd: "echo one".into(), - } - .into(), - ], - }), - }); - chat.handle_codex_event(Event { - id: "call-a".into(), - msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { - call_id: "call-a".into(), - stdout: "one".into(), - stderr: String::new(), - aggregated_output: "one".into(), - exit_code: 0, - duration: std::time::Duration::from_millis(5), - formatted_output: "one".into(), - }), - }); - let first_cells = drain_insert_history(&mut rx); - assert_eq!(first_cells.len(), 1, "first exec should insert history"); + // Dispatch a command that is unavailable while a task runs (e.g., /model) + chat.dispatch_command(SlashCommand::Model); - // Second command - chat.handle_codex_event(Event { - id: "call-b".into(), - msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { - call_id: "call-b".into(), - command: vec!["bash".into(), "-lc".into(), "echo two".into()], - cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), - parsed_cmd: vec![ - codex_core::parse_command::ParsedCommand::Unknown { - cmd: "echo two".into(), - } - .into(), - ], - }), - }); - chat.handle_codex_event(Event { - id: "call-b".into(), - msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { - call_id: "call-b".into(), - stdout: "two".into(), - stderr: String::new(), - aggregated_output: "two".into(), - exit_code: 0, - duration: std::time::Duration::from_millis(5), - formatted_output: "two".into(), - }), - }); - let second_cells = drain_insert_history(&mut rx); - assert_eq!(second_cells.len(), 1, "second exec should extend history"); - let first_blob = lines_to_single_string(&first_cells[0]); - let second_blob = lines_to_single_string(&second_cells[0]); - assert!(first_blob.contains('✓')); - assert!(second_blob.contains("echo two")); + // Drain history and snapshot the rendered error line(s) + let cells = drain_insert_history(&mut rx); + assert!( + !cells.is_empty(), + "expected an error message history cell to be emitted", + ); + let blob = lines_to_single_string(cells.last().unwrap()); + assert_snapshot!(blob); } #[tokio::test(flavor = "current_thread")] -async fn binary_size_transcript_matches_ideal_fixture() { +async fn binary_size_transcript_snapshot() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); // Set up a VT100 test terminal to capture ANSI visual output @@ -534,6 +484,7 @@ async fn binary_size_transcript_matches_ideal_fixture() { let reader = BufReader::new(file); let mut transcript = String::new(); let mut ansi: Vec = Vec::new(); + let mut has_emitted_history = false; for line in reader.lines() { let line = line.expect("read line"); @@ -559,27 +510,42 @@ async fn binary_size_transcript_matches_ideal_fixture() { let ev: Event = serde_json::from_value(upgrade_event_payload_for_tests(payload.clone())) .expect("parse"); + let ev = match ev { + Event { + msg: EventMsg::ExecCommandBegin(e), + .. + } => { + // Re-parse the command + let parsed_cmd = codex_core::parse_command::parse_command(&e.command); + Event { + id: ev.id, + msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id: e.call_id.clone(), + command: e.command, + cwd: e.cwd, + parsed_cmd: parsed_cmd.into_iter().map(|c| c.into()).collect(), + }), + } + } + _ => ev, + }; chat.handle_codex_event(ev); while let Ok(app_ev) = rx.try_recv() { - match app_ev { - AppEvent::InsertHistoryLines(lines) => { - transcript.push_str(&lines_to_single_string(&lines)); - crate::insert_history::insert_history_lines_to_writer( - &mut terminal, - &mut ansi, - lines, - ); + if let AppEvent::InsertHistoryCell(cell) = app_ev { + let mut lines = cell.display_lines(width); + if has_emitted_history + && !cell.is_stream_continuation() + && !lines.is_empty() + { + lines.insert(0, "".into()); } - AppEvent::InsertHistoryCell(cell) => { - let lines = cell.display_lines(); - transcript.push_str(&lines_to_single_string(&lines)); - crate::insert_history::insert_history_lines_to_writer( - &mut terminal, - &mut ansi, - lines, - ); - } - _ => {} + has_emitted_history = true; + transcript.push_str(&lines_to_single_string(&lines)); + crate::insert_history::insert_history_lines_to_writer( + &mut terminal, + &mut ansi, + lines, + ); } } } @@ -590,25 +556,21 @@ async fn binary_size_transcript_matches_ideal_fixture() { { chat.on_commit_tick(); while let Ok(app_ev) = rx.try_recv() { - match app_ev { - AppEvent::InsertHistoryLines(lines) => { - transcript.push_str(&lines_to_single_string(&lines)); - crate::insert_history::insert_history_lines_to_writer( - &mut terminal, - &mut ansi, - lines, - ); + if let AppEvent::InsertHistoryCell(cell) = app_ev { + let mut lines = cell.display_lines(width); + if has_emitted_history + && !cell.is_stream_continuation() + && !lines.is_empty() + { + lines.insert(0, "".into()); } - AppEvent::InsertHistoryCell(cell) => { - let lines = cell.display_lines(); - transcript.push_str(&lines_to_single_string(&lines)); - crate::insert_history::insert_history_lines_to_writer( - &mut terminal, - &mut ansi, - lines, - ); - } - _ => {} + has_emitted_history = true; + transcript.push_str(&lines_to_single_string(&lines)); + crate::insert_history::insert_history_lines_to_writer( + &mut terminal, + &mut ansi, + lines, + ); } } } @@ -617,19 +579,6 @@ async fn binary_size_transcript_matches_ideal_fixture() { } } - // Read the ideal fixture as-is - let mut f = open_fixture("ideal-binary-response.txt"); - let mut ideal = String::new(); - f.read_to_string(&mut ideal) - .expect("read ideal-binary-response.txt"); - // Normalize line endings for Windows vs. Unix checkouts - let ideal = ideal.replace("\r\n", "\n"); - let ideal_first_line = ideal - .lines() - .find(|l| !l.trim().is_empty()) - .unwrap_or("") - .to_string(); - // Build the final VT100 visual by parsing the ANSI stream. Trim trailing spaces per line // and drop trailing empty lines so the shape matches the ideal fixture exactly. let mut parser = vt100::Parser::new(height, width, 0); @@ -654,24 +603,19 @@ async fn binary_size_transcript_matches_ideal_fixture() { while lines.last().is_some_and(|l| l.is_empty()) { lines.pop(); } - // Compare only after the last session banner marker. Skip the transient - // 'thinking' header if present, and start from the first non-empty line - // of content that follows. + // Consider content only after the last session banner marker. Skip the transient + // 'thinking' header if present, and start from the first non-empty content line + // that follows. This keeps the snapshot stable across sessions. const MARKER_PREFIX: &str = ">_ You are using OpenAI Codex in "; let last_marker_line_idx = lines .iter() .rposition(|l| l.starts_with(MARKER_PREFIX)) .expect("marker not found in visible output"); - // Anchor to the first ideal line if present; otherwise use heuristics. + // Prefer the first assistant content line (blockquote '>' prefix) after the marker; + // fallback to the first non-empty, non-'thinking' line. let start_idx = (last_marker_line_idx + 1..lines.len()) - .find(|&idx| lines[idx].trim_start() == ideal_first_line) - .or_else(|| { - // Prefer the first assistant content line (blockquote '>' prefix) after the marker. - (last_marker_line_idx + 1..lines.len()) - .find(|&idx| lines[idx].trim_start().starts_with('>')) - }) + .find(|&idx| lines[idx].trim_start().starts_with('>')) .unwrap_or_else(|| { - // Fallback: first non-empty, non-'thinking' line (last_marker_line_idx + 1..lines.len()) .find(|&idx| { let t = lines[idx].trim_start(); @@ -686,8 +630,8 @@ async fn binary_size_transcript_matches_ideal_fixture() { compare_lines.extend(lines[(start_idx + 1)..].iter().cloned()); let visible_after = compare_lines.join("\n"); - // Normalize: drop a leading 'thinking' line if present in either side to - // avoid coupling to whether the reasoning header is rendered in history. + // Normalize: drop a leading 'thinking' line if present to avoid coupling + // to whether the reasoning header is rendered in history. fn drop_leading_thinking(s: &str) -> String { let mut it = s.lines(); let first = it.next(); @@ -699,35 +643,9 @@ async fn binary_size_transcript_matches_ideal_fixture() { } } let visible_after = drop_leading_thinking(&visible_after); - let ideal = drop_leading_thinking(&ideal); - // Normalize: strip leading Markdown blockquote markers ('>' or '> ') which - // may be present in rendered transcript lines but not in the ideal text. - fn strip_blockquotes(s: &str) -> String { - s.lines() - .map(|l| { - l.strip_prefix("> ") - .or_else(|| l.strip_prefix('>')) - .unwrap_or(l) - }) - .collect::>() - .join("\n") - } - let visible_after = strip_blockquotes(&visible_after); - let ideal = strip_blockquotes(&ideal); - - // Optionally update the fixture when env var is set - if std::env::var("UPDATE_IDEAL").as_deref() == Ok("1") { - let mut p = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - p.push("tests"); - p.push("fixtures"); - p.push("ideal-binary-response.txt"); - std::fs::write(&p, &visible_after).expect("write updated ideal fixture"); - return; - } - - // Exact equality with pretty diff on failure - assert_eq!(visible_after, ideal); + // Snapshot the normalized visible transcript following the banner. + assert_snapshot!("binary_size_ideal_response", visible_after); } // @@ -763,6 +681,33 @@ fn approval_modal_exec_snapshot() { assert_snapshot!("approval_modal_exec", terminal.backend()); } +// Snapshot test: command approval modal without a reason +// Ensures spacing looks correct when no reason text is provided. +#[test] +fn approval_modal_exec_without_reason_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(); + chat.config.approval_policy = codex_core::protocol::AskForApproval::OnRequest; + + let ev = ExecApprovalRequestEvent { + call_id: "call-approve-cmd-noreason".into(), + command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + reason: None, + }; + chat.handle_codex_event(Event { + id: "sub-approve-noreason".into(), + msg: EventMsg::ExecApprovalRequest(ev), + }); + + let height = chat.desired_height(80); + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height)) + .expect("create terminal"); + terminal + .draw(|f| f.render_widget_ref(&chat, f.area())) + .expect("draw approval modal (no reason)"); + assert_snapshot!("approval_modal_exec_no_reason", terminal.backend()); +} + // Snapshot test: patch approval modal #[test] fn approval_modal_patch_snapshot() { @@ -984,11 +929,11 @@ fn apply_patch_events_emit_history_cells() { assert!(!cells.is_empty(), "expected pending patch cell to be sent"); let blob = lines_to_single_string(cells.last().unwrap()); assert!( - blob.contains("proposed patch"), - "missing proposed patch header: {blob:?}" + blob.contains("Proposed Change"), + "missing proposed change header: {blob:?}" ); - // 2) Begin apply -> applying patch cell + // 2) Begin apply -> per-file apply block cell (no global header) let mut changes2 = HashMap::new(); changes2.insert( PathBuf::from("foo.txt"), @@ -1006,11 +951,11 @@ fn apply_patch_events_emit_history_cells() { msg: EventMsg::PatchApplyBegin(begin), }); let cells = drain_insert_history(&mut rx); - assert!(!cells.is_empty(), "expected applying patch cell to be sent"); + assert!(!cells.is_empty(), "expected apply block cell to be sent"); let blob = lines_to_single_string(cells.last().unwrap()); assert!( - blob.contains("Applying patch"), - "missing applying patch header: {blob:?}" + blob.contains("Added foo.txt") || blob.contains("Edited foo.txt"), + "expected single-file header with filename (Added/Edited): {blob:?}" ); // 3) End apply success -> success cell @@ -1025,11 +970,111 @@ fn apply_patch_events_emit_history_cells() { msg: EventMsg::PatchApplyEnd(end), }); let cells = drain_insert_history(&mut rx); - assert!(!cells.is_empty(), "expected applied patch cell to be sent"); + assert!( + cells.is_empty(), + "no success cell should be emitted anymore" + ); +} + +#[test] +fn apply_patch_manual_approval_adjusts_header() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + let mut proposed_changes = HashMap::new(); + proposed_changes.insert( + PathBuf::from("foo.txt"), + FileChange::Add { + content: "hello\n".to_string(), + }, + ); + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { + call_id: "c1".into(), + changes: proposed_changes, + reason: None, + grant_root: None, + }), + }); + drain_insert_history(&mut rx); + + let mut apply_changes = HashMap::new(); + apply_changes.insert( + PathBuf::from("foo.txt"), + FileChange::Add { + content: "hello\n".to_string(), + }, + ); + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::PatchApplyBegin(PatchApplyBeginEvent { + call_id: "c1".into(), + auto_approved: false, + changes: apply_changes, + }), + }); + + let cells = drain_insert_history(&mut rx); + assert!(!cells.is_empty(), "expected apply block cell to be sent"); let blob = lines_to_single_string(cells.last().unwrap()); assert!( - blob.contains("Applied patch"), - "missing applied patch header: {blob:?}" + blob.contains("Change Approved foo.txt"), + "expected change approved summary: {blob:?}" + ); +} + +#[test] +fn apply_patch_manual_flow_snapshot() { + use insta::assert_snapshot; + + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + let mut proposed_changes = HashMap::new(); + proposed_changes.insert( + PathBuf::from("foo.txt"), + FileChange::Add { + content: "hello\n".to_string(), + }, + ); + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { + call_id: "c1".into(), + changes: proposed_changes, + reason: Some("Manual review required".into()), + grant_root: None, + }), + }); + let proposed_lines = drain_insert_history(&mut rx) + .pop() + .expect("proposed patch cell"); + + let mut apply_changes = HashMap::new(); + apply_changes.insert( + PathBuf::from("foo.txt"), + FileChange::Add { + content: "hello\n".to_string(), + }, + ); + chat.handle_codex_event(Event { + id: "s1".into(), + msg: EventMsg::PatchApplyBegin(PatchApplyBeginEvent { + call_id: "c1".into(), + auto_approved: false, + changes: apply_changes, + }), + }); + let approved_lines = drain_insert_history(&mut rx) + .pop() + .expect("approved patch cell"); + + assert_snapshot!( + "apply_patch_manual_flow_history_proposed", + lines_to_single_string(&proposed_lines) + ); + assert_snapshot!( + "apply_patch_manual_flow_history_approved", + lines_to_single_string(&approved_lines) ); } @@ -1226,7 +1271,7 @@ fn apply_patch_request_shows_diff_summary() { // Header should summarize totals assert!( - blob.contains("proposed patch to 1 file (+2 -0)"), + blob.contains("Proposed Change README.md (+2 -0)"), "missing or incorrect diff header: {blob:?}" ); @@ -1265,7 +1310,7 @@ fn plan_update_renders_history_cell() { assert!(!cells.is_empty(), "expected plan update cell to be sent"); let blob = lines_to_single_string(cells.last().unwrap()); assert!( - blob.contains("Update plan"), + blob.contains("Updated Plan"), "missing plan header: {blob:?}" ); assert!(blob.contains("Explore codebase")); @@ -1287,99 +1332,11 @@ fn stream_error_is_rendered_to_history() { let cells = drain_insert_history(&mut rx); assert!(!cells.is_empty(), "expected a history cell for StreamError"); let blob = lines_to_single_string(cells.last().unwrap()); - assert!(blob.contains("⚠\u{200A} ")); + assert!(blob.contains('⚠')); assert!(blob.contains("stream error:")); assert!(blob.contains("idle timeout waiting for SSE")); } -#[test] -fn headers_emitted_on_stream_begin_for_answer_and_not_for_reasoning() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); - - // Answer: no header until a newline commit - chat.handle_codex_event(Event { - id: "sub-a".into(), - msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { - delta: "Hello".into(), - }), - }); - let mut saw_codex_pre = false; - while let Ok(ev) = rx.try_recv() { - if let AppEvent::InsertHistoryLines(lines) = ev { - let s = lines - .iter() - .flat_map(|l| l.spans.iter()) - .map(|sp| sp.content.clone()) - .collect::>() - .join(""); - if s.contains("codex") { - saw_codex_pre = true; - break; - } - } - } - assert!( - !saw_codex_pre, - "answer header should not be emitted before first newline commit" - ); - - // Newline arrives, then header is emitted - chat.handle_codex_event(Event { - id: "sub-a".into(), - msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { - delta: "!\n".into(), - }), - }); - chat.on_commit_tick(); - let mut saw_codex_post = false; - while let Ok(ev) = rx.try_recv() { - if let AppEvent::InsertHistoryLines(lines) = ev { - let s = lines - .iter() - .flat_map(|l| l.spans.iter()) - .map(|sp| sp.content.clone()) - .collect::>() - .join(""); - if s.contains("codex") { - saw_codex_post = true; - break; - } - } - } - assert!( - saw_codex_post, - "expected 'codex' header to be emitted after first newline commit" - ); - - // Reasoning: do NOT emit a history header; status text is updated instead - let (mut chat2, mut rx2, _op_rx2) = make_chatwidget_manual(); - chat2.handle_codex_event(Event { - id: "sub-b".into(), - msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { - delta: "Thinking".into(), - }), - }); - let mut saw_thinking = false; - while let Ok(ev) = rx2.try_recv() { - if let AppEvent::InsertHistoryLines(lines) = ev { - let s = lines - .iter() - .flat_map(|l| l.spans.iter()) - .map(|sp| sp.content.clone()) - .collect::>() - .join(""); - if s.contains("thinking") { - saw_thinking = true; - break; - } - } - } - assert!( - !saw_thinking, - "reasoning deltas should not emit history headers" - ); -} - #[test] fn multiple_agent_messages_in_single_turn_emit_multiple_headers() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); @@ -1417,26 +1374,10 @@ fn multiple_agent_messages_in_single_turn_emit_multiple_headers() { }); let cells = drain_insert_history(&mut rx); - let mut header_count = 0usize; - let mut combined = String::new(); - for lines in &cells { - for l in lines { - for sp in &l.spans { - let s = &sp.content; - if s == "codex" { - header_count += 1; - } - combined.push_str(s); - } - combined.push('\n'); - } - } - assert_eq!( - header_count, - 2, - "expected two 'codex' headers for two AgentMessage events in one turn; cells={:?}", - cells.len() - ); + let combined: String = cells + .iter() + .map(|lines| lines_to_single_string(lines)) + .collect(); assert!( combined.contains("First message"), "missing first message: {combined}" @@ -1537,3 +1478,124 @@ fn deltas_then_same_final_message_are_rendered_snapshot() { .collect::(); assert_snapshot!(combined); } + +// Combined visual snapshot using vt100 for history + direct buffer overlay for UI. +// This renders the final visual as seen in a terminal: history above, then a blank line, +// then the exec block, another blank line, the status line, a blank line, and the composer. +#[test] +fn chatwidget_exec_and_status_layout_vt100_snapshot() { + // Setup identical scenario + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + chat.handle_codex_event(Event { + id: "c1".into(), + msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id: "c1".into(), + command: vec!["bash".into(), "-lc".into(), "rg \"Change Approved\"".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + parsed_cmd: vec![ + codex_core::parse_command::ParsedCommand::Search { + query: Some("Change Approved".into()), + path: None, + cmd: "rg \"Change Approved\"".into(), + } + .into(), + codex_core::parse_command::ParsedCommand::Read { + name: "diff_render.rs".into(), + cmd: "cat diff_render.rs".into(), + } + .into(), + ], + }), + }); + chat.handle_codex_event(Event { + id: "c1".into(), + msg: EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id: "c1".into(), + stdout: String::new(), + stderr: String::new(), + aggregated_output: String::new(), + exit_code: 0, + duration: std::time::Duration::from_millis(16000), + formatted_output: String::new(), + }), + }); + chat.handle_codex_event(Event { + id: "t1".into(), + msg: EventMsg::TaskStarted(TaskStartedEvent { + model_context_window: None, + }), + }); + chat.handle_codex_event(Event { + id: "t1".into(), + msg: EventMsg::AgentReasoningDelta(AgentReasoningDeltaEvent { + delta: "**Investigating rendering code**".into(), + }), + }); + chat.bottom_pane + .set_composer_text("Summarize recent commits".to_string()); + chat.handle_codex_event(Event { + id: "t1".into(), + msg: EventMsg::AgentMessage(AgentMessageEvent { message: "I’m going to search the repo for where “Change Approved” is rendered to update that view.".into() }), + }); + + // Dimensions + let width: u16 = 80; + let ui_height: u16 = chat.desired_height(width); + let vt_height: u16 = 40; + let viewport = ratatui::layout::Rect::new(0, vt_height - ui_height, width, ui_height); + + // Use TestBackend for the terminal (no real ANSI emitted by drawing), + // but capture VT100 escape stream for history insertion with a separate writer. + let backend = ratatui::backend::TestBackend::new(width, vt_height); + let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); + term.set_viewport_area(viewport); + + // 1) Apply any pending history insertions by emitting ANSI to a buffer via insert_history_lines_to_writer + let mut ansi: Vec = Vec::new(); + for lines in drain_insert_history(&mut rx) { + crate::insert_history::insert_history_lines_to_writer(&mut term, &mut ansi, lines); + } + + // 2) Render the ChatWidget UI into an off-screen buffer using WidgetRef directly + let mut ui_buf = ratatui::buffer::Buffer::empty(viewport); + (&chat).render_ref(viewport, &mut ui_buf); + + // 3) Build VT100 visual from the captured ANSI + let mut parser = vt100::Parser::new(vt_height, width, 0); + parser.process(&ansi); + let mut vt_lines: Vec = (0..vt_height) + .map(|row| { + let mut s = String::with_capacity(width as usize); + for col in 0..width { + if let Some(cell) = parser.screen().cell(row, col) { + if let Some(ch) = cell.contents().chars().next() { + s.push(ch); + } else { + s.push(' '); + } + } else { + s.push(' '); + } + } + s.trim_end().to_string() + }) + .collect(); + + // 4) Overlay UI buffer content into the viewport region of the VT output + for rel_y in 0..viewport.height { + let y = viewport.y + rel_y; + let mut line = String::with_capacity(width as usize); + for x in 0..viewport.width { + let ch = ui_buf[(viewport.x + x, viewport.y + rel_y)] + .symbol() + .chars() + .next() + .unwrap_or(' '); + line.push(ch); + } + vt_lines[y as usize] = line.trim_end().to_string(); + } + + let visual = vt_lines.join("\n"); + insta::assert_snapshot!(visual); +} diff --git a/codex-rs/tui/src/common.rs b/codex-rs/tui/src/common.rs deleted file mode 100644 index 2c19b587..00000000 --- a/codex-rs/tui/src/common.rs +++ /dev/null @@ -1 +0,0 @@ -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 48a16659..06afd85c 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -1,16 +1,17 @@ -use crossterm::terminal; use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; +use ratatui::style::Stylize; use ratatui::text::Line as RtLine; use ratatui::text::Span as RtSpan; use std::collections::HashMap; +use std::path::Path; use std::path::PathBuf; -use crate::common::DEFAULT_WRAP_COLS; -use codex_core::protocol::FileChange; - +use crate::exec_command::relativize_to_home; use crate::history_cell::PatchEventType; +use codex_core::git_info::get_git_repo_root; +use codex_core::protocol::FileChange; const SPACES_AFTER_LINE_NUMBER: usize = 6; @@ -22,205 +23,199 @@ enum DiffLineType { } pub(crate) fn create_diff_summary( - title: &str, changes: &HashMap, event_type: PatchEventType, + cwd: &Path, + wrap_cols: usize, ) -> Vec> { - struct FileSummary { - display_path: String, - added: usize, - removed: usize, - } - - let count_from_unified = |diff: &str| -> (usize, usize) { - if let Ok(patch) = diffy::Patch::from_str(diff) { - 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() { - if l.starts_with("+++") || l.starts_with("---") || l.starts_with("@@") { - continue; - } - match l.as_bytes().first() { - Some(b'+') => adds += 1, - Some(b'-') => dels += 1, - _ => {} - } + let rows = collect_rows(changes); + let header_kind = match event_type { + PatchEventType::ApplyBegin { auto_approved } => { + if auto_approved { + HeaderKind::Edited + } else { + HeaderKind::ChangeApproved } - (adds, dels) } + PatchEventType::ApprovalRequest => HeaderKind::ProposedChange, }; - - let mut files: Vec = Vec::new(); - for (path, change) in changes.iter() { - match change { - 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), - }), - FileChange::Update { - unified_diff, - move_path, - } => { - let (added, removed) = count_from_unified(unified_diff); - let display_path = if let Some(new_path) = move_path { - format!("{} → {}", path.display(), new_path.display()) - } else { - path.display().to_string() - }; - files.push(FileSummary { - display_path, - added, - removed, - }); - } - } - } - - let file_count = files.len(); - let total_added: usize = files.iter().map(|f| f.added).sum(); - let total_removed: usize = files.iter().map(|f| f.removed).sum(); - let noun = if file_count == 1 { "file" } else { "files" }; - - let mut out: Vec> = Vec::new(); - - // Header - let mut header_spans: Vec> = Vec::new(); - header_spans.push(RtSpan::styled( - title.to_owned(), - Style::default() - .fg(Color::Magenta) - .add_modifier(Modifier::BOLD), - )); - header_spans.push(RtSpan::raw(" to ")); - header_spans.push(RtSpan::raw(format!("{file_count} {noun} "))); - header_spans.push(RtSpan::raw("(")); - header_spans.push(RtSpan::styled( - format!("+{total_added}"), - Style::default().fg(Color::Green), - )); - header_spans.push(RtSpan::raw(" ")); - header_spans.push(RtSpan::styled( - format!("-{total_removed}"), - Style::default().fg(Color::Red), - )); - header_spans.push(RtSpan::raw(")")); - out.push(RtLine::from(header_spans)); - - // Dimmed per-file lines with prefix - for (idx, f) in files.iter().enumerate() { - let mut spans: Vec> = Vec::new(); - spans.push(RtSpan::raw(f.display_path.clone())); - // Show per-file +/- counts only when there are multiple files - if file_count > 1 { - spans.push(RtSpan::raw(" (")); - spans.push(RtSpan::styled( - format!("+{}", f.added), - Style::default().fg(Color::Green), - )); - spans.push(RtSpan::raw(" ")); - spans.push(RtSpan::styled( - format!("-{}", f.removed), - Style::default().fg(Color::Red), - )); - spans.push(RtSpan::raw(")")); - } - - 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)); - 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 + render_changes_block(rows, wrap_cols, header_kind, cwd) } -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()); +// Shared row for per-file presentation +#[derive(Clone)] +struct Row { + #[allow(dead_code)] + path: PathBuf, + move_path: Option, + added: usize, + removed: usize, + change: FileChange, +} - 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()), - ])); +fn collect_rows(changes: &HashMap) -> Vec { + let mut rows: Vec = Vec::new(); + for (path, change) in changes.iter() { + let (added, removed) = match change { + FileChange::Add { content } => (content.lines().count(), 0), + FileChange::Delete { content } => (0, content.lines().count()), + FileChange::Update { unified_diff, .. } => calculate_add_remove_from_diff(unified_diff), + }; + let move_path = match change { + FileChange::Update { + move_path: Some(new), + .. + } => Some(new.clone()), + _ => None, + }; + rows.push(Row { + path: path.clone(), + move_path, + added, + removed, + change: change.clone(), + }); + } + rows.sort_by_key(|r| r.path.clone()); + rows +} + +enum HeaderKind { + ProposedChange, + Edited, + ChangeApproved, +} + +fn render_changes_block( + rows: Vec, + wrap_cols: usize, + header_kind: HeaderKind, + cwd: &Path, +) -> Vec> { + let mut out: Vec> = Vec::new(); + let term_cols = wrap_cols; + + fn render_line_count_summary(added: usize, removed: usize) -> Vec> { + let mut spans = Vec::new(); + spans.push("(".into()); + spans.push(format!("+{added}").green()); + spans.push(" ".into()); + spans.push(format!("-{removed}").red()); + spans.push(")".into()); + spans + } + + let render_path = |row: &Row| -> Vec> { + let mut spans = Vec::new(); + spans.push(display_path_for(&row.path, cwd).into()); + if let Some(move_path) = &row.move_path { + spans.push(format!(" → {}", display_path_for(move_path, cwd)).into()); } - match change { + spans + }; + + // Header + let total_added: usize = rows.iter().map(|r| r.added).sum(); + let total_removed: usize = rows.iter().map(|r| r.removed).sum(); + let file_count = rows.len(); + let noun = if file_count == 1 { "file" } else { "files" }; + let mut header_spans: Vec> = vec!["• ".into()]; + match header_kind { + HeaderKind::ProposedChange => { + header_spans.push("Proposed Change".bold()); + if let [row] = &rows[..] { + header_spans.push(" ".into()); + header_spans.extend(render_path(row)); + header_spans.push(" ".into()); + header_spans.extend(render_line_count_summary(row.added, row.removed)); + } else { + header_spans.push(format!(" to {file_count} {noun} ").into()); + header_spans.extend(render_line_count_summary(total_added, total_removed)); + } + } + HeaderKind::Edited => { + if let [row] = &rows[..] { + let verb = match &row.change { + FileChange::Add { .. } => "Added", + FileChange::Delete { .. } => "Deleted", + _ => "Edited", + }; + header_spans.push(verb.bold()); + header_spans.push(" ".into()); + header_spans.extend(render_path(row)); + header_spans.push(" ".into()); + header_spans.extend(render_line_count_summary(row.added, row.removed)); + } else { + header_spans.push("Edited".bold()); + header_spans.push(format!(" {file_count} {noun} ").into()); + header_spans.extend(render_line_count_summary(total_added, total_removed)); + } + } + HeaderKind::ChangeApproved => { + header_spans.push("Change Approved".bold()); + if let [row] = &rows[..] { + header_spans.push(" ".into()); + header_spans.extend(render_path(row)); + header_spans.push(" ".into()); + header_spans.extend(render_line_count_summary(row.added, row.removed)); + } else { + header_spans.push(format!(" {file_count} {noun} ").into()); + header_spans.extend(render_line_count_summary(total_added, total_removed)); + } + } + } + out.push(RtLine::from(header_spans)); + + // For Change Approved, we only show the header summary and no per-file/diff details. + if matches!(header_kind, HeaderKind::ChangeApproved) { + return out; + } + + for (idx, r) in rows.into_iter().enumerate() { + // Insert a blank separator between file chunks (except before the first) + if idx > 0 { + out.push("".into()); + } + // File header line (skip when single-file header already shows the name) + let skip_file_header = + matches!(header_kind, HeaderKind::ProposedChange | HeaderKind::Edited) + && file_count == 1; + if !skip_file_header { + let mut header: Vec> = Vec::new(); + header.push(" └ ".dim()); + header.extend(render_path(&r)); + header.push(" ".into()); + header.extend(render_line_count_summary(r.added, r.removed)); + out.push(RtLine::from(header)); + } + + match r.change { FileChange::Add { content } => { for (i, raw) in content.lines().enumerate() { - let ln = i + 1; out.extend(push_wrapped_diff_line( - ln, + i + 1, 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; + FileChange::Delete { content } => { + for (i, raw) in content.lines().enumerate() { out.extend(push_wrapped_diff_line( - ln, + i + 1, DiffLineType::Delete, raw, term_cols, )); } } - FileChange::Update { - unified_diff, - move_path: _, - } => { - if let Ok(patch) = diffy::Patch::from_str(unified_diff) { + FileChange::Update { unified_diff, .. } => { + if let Ok(patch) = diffy::Patch::from_str(&unified_diff) { let mut is_first_hunk = true; for h in patch.hunks() { - // Render a simple separator between non-contiguous hunks - // instead of diff-style @@ headers. if !is_first_hunk { - out.push(RtLine::from(vec![ - RtSpan::raw(" "), - RtSpan::styled("⋮", style_dim()), - ])); + out.push(RtLine::from(vec![" ".into(), "⋮".dim()])); } is_first_hunk = false; @@ -265,13 +260,41 @@ fn render_patch_details(changes: &HashMap) -> Vec String { + let path_in_same_repo = match (get_git_repo_root(cwd), get_git_repo_root(path)) { + (Some(cwd_repo), Some(path_repo)) => cwd_repo == path_repo, + _ => false, + }; + let chosen = if path_in_same_repo { + pathdiff::diff_paths(path, cwd).unwrap_or_else(|| path.to_path_buf()) + } else { + relativize_to_home(path).unwrap_or_else(|| path.to_path_buf()) + }; + chosen.display().to_string() +} + +fn calculate_add_remove_from_diff(diff: &str) -> (usize, usize) { + if let Ok(patch) = diffy::Patch::from_str(diff) { + 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), + diffy::Line::Context(_) => (a, d), + }) + } else { + // For unparsable diffs, return 0 for both counts. + (0, 0) + } +} + fn push_wrapped_diff_line( line_number: usize, kind: DiffLineType, @@ -290,10 +313,10 @@ fn push_wrapped_diff_line( let prefix_cols = indent.len() + ln_str.len() + gap_after_ln; let mut first = true; - let (sign_opt, line_style) = match kind { - DiffLineType::Insert => (Some('+'), Some(style_add())), - DiffLineType::Delete => (Some('-'), Some(style_del())), - DiffLineType::Context => (None, None), + let (sign_char, line_style) = match kind { + DiffLineType::Insert => ('+', style_add()), + DiffLineType::Delete => ('-', style_del()), + DiffLineType::Context => (' ', style_context()), }; let mut lines: Vec> = Vec::new(); @@ -301,9 +324,7 @@ fn push_wrapped_diff_line( // 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(if first { prefix_cols + 1 } else { prefix_cols }) - .max(1); + let available_content_cols = term_cols.saturating_sub(prefix_cols + 1).max(1); let split_at_byte_index = remaining_text .char_indices() .nth(available_content_cols) @@ -313,41 +334,22 @@ fn push_wrapped_diff_line( 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))); - // Always include a sign character at the start of the displayed chunk - // ('+' for insert, '-' for delete, ' ' for context) so gutters align. - let sign_char = sign_opt.unwrap_or(' '); - let display_chunk = format!("{sign_char}{chunk}"); - let content_span = match line_style { - Some(style) => RtSpan::styled(display_chunk, style), - None => RtSpan::raw(display_chunk), - }; - spans.push(content_span); - let mut line = RtLine::from(spans); - if let Some(style) = line_style { - line.style = line.style.patch(style); - } - lines.push(line); + // Build gutter (indent + line number + spacing) as a dimmed span + let gutter = format!("{indent}{ln_str}{}", " ".repeat(gap_after_ln)); + // Content with a sign ('+'/'-'/' ') styled per diff kind + let content = format!("{sign_char}{chunk}"); + lines.push(RtLine::from(vec![ + RtSpan::styled(gutter, style_gutter()), + RtSpan::styled(content, line_style), + ])); first = false; } else { // Continuation lines keep a space for the sign column so content aligns - let hang_prefix = format!( - "{indent}{}{} ", - " ".repeat(ln_str.len()), - " ".repeat(gap_after_ln) - ); - let content_span = match line_style { - Some(style) => RtSpan::styled(chunk.to_string(), style), - None => RtSpan::raw(chunk.to_string()), - }; - let mut line = RtLine::from(vec![RtSpan::raw(hang_prefix), content_span]); - if let Some(style) = line_style { - line.style = line.style.patch(style); - } - lines.push(line); + let gutter = format!("{indent}{} ", " ".repeat(ln_str.len() + gap_after_ln)); + lines.push(RtLine::from(vec![ + RtSpan::styled(gutter, style_gutter()), + RtSpan::styled(chunk.to_string(), line_style), + ])); } if remaining_text.is_empty() { break; @@ -356,10 +358,14 @@ fn push_wrapped_diff_line( lines } -fn style_dim() -> Style { +fn style_gutter() -> Style { Style::default().add_modifier(Modifier::DIM) } +fn style_context() -> Style { + Style::default() +} + fn style_add() -> Style { Style::default().fg(Color::Green) } @@ -378,6 +384,12 @@ mod tests { use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; + fn diff_summary_for_tests( + changes: &HashMap, + event_type: PatchEventType, + ) -> Vec> { + create_diff_summary(changes, event_type, &PathBuf::from("/"), 80) + } fn snapshot_lines(name: &str, lines: Vec>, width: u16, height: u16) { let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("terminal"); @@ -391,6 +403,23 @@ mod tests { assert_snapshot!(name, terminal.backend()); } + fn snapshot_lines_text(name: &str, lines: &[RtLine<'static>]) { + // Convert Lines to plain text rows and trim trailing spaces so it's + // easier to validate indentation visually in snapshots. + let text = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .map(|s| s.trim_end().to_string()) + .collect::>() + .join("\n"); + assert_snapshot!(name, text); + } + #[test] fn ui_snapshot_add_details() { let mut changes: HashMap = HashMap::new(); @@ -401,8 +430,7 @@ mod tests { }, ); - let lines = - create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest); + let lines = diff_summary_for_tests(&changes, PatchEventType::ApprovalRequest); snapshot_lines("add_details", lines, 80, 10); } @@ -423,8 +451,7 @@ mod tests { }, ); - let lines = - create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest); + let lines = diff_summary_for_tests(&changes, PatchEventType::ApprovalRequest); snapshot_lines("update_details_with_rename", lines, 80, 12); } @@ -435,11 +462,10 @@ mod tests { 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()); + let lines = push_wrapped_diff_line(1, DiffLineType::Insert, long_line, 80); // Render into a small terminal to capture the visual layout - snapshot_lines("wrap_behavior_insert", lines, DEFAULT_WRAP_COLS + 10, 8); + snapshot_lines("wrap_behavior_insert", lines, 90, 8); } #[test] @@ -458,8 +484,7 @@ mod tests { }, ); - let lines = - create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest); + let lines = diff_summary_for_tests(&changes, PatchEventType::ApprovalRequest); snapshot_lines("single_line_replacement_counts", lines, 80, 8); } @@ -480,8 +505,7 @@ mod tests { }, ); - let lines = - create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest); + let lines = diff_summary_for_tests(&changes, PatchEventType::ApprovalRequest); snapshot_lines("blank_context_line", lines, 80, 10); } @@ -503,10 +527,232 @@ mod tests { }, ); - let lines = - create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest); + let lines = diff_summary_for_tests(&changes, PatchEventType::ApprovalRequest); // Height is large enough to show both hunks and the separator snapshot_lines("vertical_ellipsis_between_hunks", lines, 80, 16); } + + #[test] + fn ui_snapshot_apply_update_block() { + 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("example.txt"), + FileChange::Update { + unified_diff: patch, + move_path: None, + }, + ); + + for (name, auto_approved) in [ + ("apply_update_block", true), + ("apply_update_block_manual", false), + ] { + let lines = + diff_summary_for_tests(&changes, PatchEventType::ApplyBegin { auto_approved }); + + snapshot_lines(name, lines, 80, 12); + } + } + + #[test] + fn ui_snapshot_apply_update_with_rename_block() { + let mut changes: HashMap = HashMap::new(); + let original = "A\nB\nC\n"; + let modified = "A\nB changed\nC\n"; + let patch = diffy::create_patch(original, modified).to_string(); + + changes.insert( + PathBuf::from("old_name.rs"), + FileChange::Update { + unified_diff: patch, + move_path: Some(PathBuf::from("new_name.rs")), + }, + ); + + let lines = diff_summary_for_tests( + &changes, + PatchEventType::ApplyBegin { + auto_approved: true, + }, + ); + + snapshot_lines("apply_update_with_rename_block", lines, 80, 12); + } + + #[test] + fn ui_snapshot_apply_multiple_files_block() { + // Two files: one update and one add, to exercise combined header and per-file rows + let mut changes: HashMap = HashMap::new(); + + // File a.txt: single-line replacement (one delete, one insert) + let patch_a = diffy::create_patch("one\n", "one changed\n").to_string(); + changes.insert( + PathBuf::from("a.txt"), + FileChange::Update { + unified_diff: patch_a, + move_path: None, + }, + ); + + // File b.txt: newly added with one line + changes.insert( + PathBuf::from("b.txt"), + FileChange::Add { + content: "new\n".to_string(), + }, + ); + + let lines = diff_summary_for_tests( + &changes, + PatchEventType::ApplyBegin { + auto_approved: true, + }, + ); + + snapshot_lines("apply_multiple_files_block", lines, 80, 14); + } + + #[test] + fn ui_snapshot_apply_add_block() { + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("new_file.txt"), + FileChange::Add { + content: "alpha\nbeta\n".to_string(), + }, + ); + + let lines = diff_summary_for_tests( + &changes, + PatchEventType::ApplyBegin { + auto_approved: true, + }, + ); + + snapshot_lines("apply_add_block", lines, 80, 10); + } + + #[test] + fn ui_snapshot_apply_delete_block() { + // Write a temporary file so the delete renderer can read original content + let tmp_path = PathBuf::from("tmp_delete_example.txt"); + std::fs::write(&tmp_path, "first\nsecond\nthird\n").expect("write tmp file"); + + let mut changes: HashMap = HashMap::new(); + changes.insert( + tmp_path.clone(), + FileChange::Delete { + content: "first\nsecond\nthird\n".to_string(), + }, + ); + + let lines = diff_summary_for_tests( + &changes, + PatchEventType::ApplyBegin { + auto_approved: true, + }, + ); + + // Cleanup best-effort; rendering has already read the file + let _ = std::fs::remove_file(&tmp_path); + + snapshot_lines("apply_delete_block", lines, 80, 12); + } + + #[test] + fn ui_snapshot_apply_update_block_wraps_long_lines() { + // Create a patch with a long modified line to force wrapping + let original = "line 1\nshort\nline 3\n"; + let modified = "line 1\nshort this_is_a_very_long_modified_line_that_should_wrap_across_multiple_terminal_columns_and_continue_even_further_beyond_eighty_columns_to_force_multiple_wraps\nline 3\n"; + let patch = diffy::create_patch(original, modified).to_string(); + + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("long_example.txt"), + FileChange::Update { + unified_diff: patch, + move_path: None, + }, + ); + + let lines = create_diff_summary( + &changes, + PatchEventType::ApplyBegin { + auto_approved: true, + }, + &PathBuf::from("/"), + 72, + ); + + // Render with backend width wider than wrap width to avoid Paragraph auto-wrap. + snapshot_lines("apply_update_block_wraps_long_lines", lines, 80, 12); + } + + #[test] + fn ui_snapshot_apply_update_block_wraps_long_lines_text() { + // This mirrors the desired layout example: sign only on first inserted line, + // subsequent wrapped pieces start aligned under the line number gutter. + let original = "1\n2\n3\n4\n"; + let modified = "1\nadded long line which wraps and_if_there_is_a_long_token_it_will_be_broken\n3\n4 context line which also wraps across\n"; + let patch = diffy::create_patch(original, modified).to_string(); + + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("wrap_demo.txt"), + FileChange::Update { + unified_diff: patch, + move_path: None, + }, + ); + + let mut lines = create_diff_summary( + &changes, + PatchEventType::ApplyBegin { + auto_approved: true, + }, + &PathBuf::from("/"), + 28, + ); + // Drop the combined header for this text-only snapshot + if !lines.is_empty() { + lines.remove(0); + } + snapshot_lines_text("apply_update_block_wraps_long_lines_text", &lines); + } + + #[test] + fn ui_snapshot_apply_update_block_relativizes_path() { + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("/")); + let abs_old = cwd.join("abs_old.rs"); + let abs_new = cwd.join("abs_new.rs"); + + let original = "X\nY\n"; + let modified = "X changed\nY\n"; + let patch = diffy::create_patch(original, modified).to_string(); + + let mut changes: HashMap = HashMap::new(); + changes.insert( + abs_old.clone(), + FileChange::Update { + unified_diff: patch, + move_path: Some(abs_new.clone()), + }, + ); + + let lines = create_diff_summary( + &changes, + PatchEventType::ApplyBegin { + auto_approved: true, + }, + &cwd, + 80, + ); + + snapshot_lines("apply_update_block_relativizes_path", lines, 80, 10); + } } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index cc05b36f..185767be 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -23,22 +23,26 @@ use codex_login::try_read_auth_json; use codex_protocol::parse_command::ParsedCommand; use image::DynamicImage; use image::ImageReader; +use itertools::Itertools; use mcp_types::EmbeddedResourceResource; use mcp_types::ResourceLink; use ratatui::prelude::*; use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; +use ratatui::style::Styled; use ratatui::style::Stylize; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; use std::collections::HashMap; use std::io::Cursor; +use std::path::Path; use std::path::PathBuf; use std::time::Duration; use std::time::Instant; use tracing::error; +use unicode_width::UnicodeWidthStr; use uuid::Uuid; #[derive(Clone, Debug)] @@ -49,6 +53,7 @@ pub(crate) struct CommandOutput { pub(crate) formatted_output: String, } +#[derive(Clone, Debug)] pub(crate) enum PatchEventType { ApprovalRequest, ApplyBegin { auto_approved: bool }, @@ -58,19 +63,110 @@ pub(crate) enum PatchEventType { /// `Vec>` representation to make it easier to display in a /// scrollable list. pub(crate) trait HistoryCell: std::fmt::Debug + Send + Sync { - fn display_lines(&self) -> Vec>; + fn display_lines(&self, width: u16) -> Vec>; fn transcript_lines(&self) -> Vec> { - self.display_lines() + self.display_lines(u16::MAX) } fn desired_height(&self, width: u16) -> u16 { - Paragraph::new(Text::from(self.display_lines())) + Paragraph::new(Text::from(self.display_lines(width))) .wrap(Wrap { trim: false }) .line_count(width) .try_into() .unwrap_or(0) } + + fn is_stream_continuation(&self) -> bool { + false + } +} + +#[derive(Debug)] +pub(crate) struct UserHistoryCell { + message: String, +} + +impl HistoryCell for UserHistoryCell { + fn display_lines(&self, width: u16) -> Vec> { + let mut lines: Vec> = Vec::new(); + + // Wrap the content first, then prefix each wrapped line with the marker. + let wrap_width = width.saturating_sub(1); // account for the ▌ prefix + let wrapped = textwrap::wrap( + &self.message, + textwrap::Options::new(wrap_width as usize) + .wrap_algorithm(textwrap::WrapAlgorithm::FirstFit) // Match textarea wrap + .word_splitter(textwrap::WordSplitter::NoHyphenation), + ); + + for line in wrapped { + lines.push(Line::from(vec!["▌".cyan().dim(), line.to_string().dim()])); + } + lines + } + + fn transcript_lines(&self) -> Vec> { + let mut lines: Vec> = Vec::new(); + lines.push(Line::from("user".cyan().bold())); + lines.extend(self.message.lines().map(|l| Line::from(l.to_string()))); + lines + } +} + +#[derive(Debug)] +pub(crate) struct AgentMessageCell { + lines: Vec>, + is_first_line: bool, +} + +impl AgentMessageCell { + pub(crate) fn new(lines: Vec>, is_first_line: bool) -> Self { + Self { + lines, + is_first_line, + } + } +} + +impl HistoryCell for AgentMessageCell { + fn display_lines(&self, width: u16) -> Vec> { + let mut out: Vec> = Vec::new(); + // We want: + // - First visual line: "> " prefix (collapse with header logic) + // - All subsequent visual lines: two-space prefix + let mut is_first_visual = true; + let wrap_width = width.saturating_sub(2); // account for prefix + for line in &self.lines { + let wrapped = + crate::insert_history::word_wrap_lines(std::slice::from_ref(line), wrap_width); + for (i, piece) in wrapped.into_iter().enumerate() { + let mut spans = Vec::with_capacity(piece.spans.len() + 1); + spans.push(if is_first_visual && i == 0 && self.is_first_line { + "> ".into() + } else { + " ".into() + }); + spans.extend(piece.spans.into_iter()); + out.push(Line::from(spans)); + } + is_first_visual = false; + } + out + } + + fn transcript_lines(&self) -> Vec> { + let mut out: Vec> = Vec::new(); + if self.is_first_line { + out.push(Line::from("codex".magenta().bold())); + } + out.extend(self.lines.clone()); + out + } + + fn is_stream_continuation(&self) -> bool { + !self.is_first_line + } } #[derive(Debug)] @@ -79,7 +175,7 @@ pub(crate) struct PlainHistoryCell { } impl HistoryCell for PlainHistoryCell { - fn display_lines(&self) -> Vec> { + fn display_lines(&self, _width: u16) -> Vec> { self.lines.clone() } } @@ -90,7 +186,7 @@ pub(crate) struct TranscriptOnlyHistoryCell { } impl HistoryCell for TranscriptOnlyHistoryCell { - fn display_lines(&self) -> Vec> { + fn display_lines(&self, _width: u16) -> Vec> { Vec::new() } @@ -100,60 +196,294 @@ impl HistoryCell for TranscriptOnlyHistoryCell { } #[derive(Debug)] -pub(crate) struct ExecCell { +pub(crate) struct PatchHistoryCell { + event_type: PatchEventType, + changes: HashMap, + cwd: PathBuf, +} + +impl HistoryCell for PatchHistoryCell { + fn display_lines(&self, width: u16) -> Vec> { + create_diff_summary( + &self.changes, + self.event_type.clone(), + &self.cwd, + width as usize, + ) + } +} + +#[derive(Debug, Clone)] +pub(crate) struct ExecCall { + pub(crate) call_id: String, pub(crate) command: Vec, pub(crate) parsed: Vec, pub(crate) output: Option, start_time: Option, duration: Option, - include_header: bool, +} + +#[derive(Debug)] +pub(crate) struct ExecCell { + calls: Vec, } impl HistoryCell for ExecCell { - fn display_lines(&self) -> Vec> { - exec_command_lines( - &self.command, - &self.parsed, - self.output.as_ref(), - self.start_time, - self.include_header, - ) + fn display_lines(&self, width: u16) -> Vec> { + if self.is_exploring_cell() { + self.exploring_display_lines(width) + } else { + self.command_display_lines(width) + } } fn transcript_lines(&self) -> Vec> { - let mut lines: Vec> = vec!["".into()]; + let mut lines: Vec> = vec![]; + for call in &self.calls { + let cmd_display = strip_bash_lc_and_escape(&call.command); + for (i, part) in cmd_display.lines().enumerate() { + if i == 0 { + lines.push(Line::from(vec!["$ ".magenta(), part.to_string().into()])); + } else { + lines.push(Line::from(vec![" ".into(), part.to_string().into()])); + } + } - let cmd_display = strip_bash_lc_and_escape(&self.command); - for (i, part) in cmd_display.lines().enumerate() { - if i == 0 { - lines.push(Line::from(vec!["$ ".magenta(), part.to_string().into()])); + if let Some(output) = call.output.as_ref() { + lines.extend(output.formatted_output.lines().map(ansi_escape_line)); + let duration = call + .duration + .map(format_duration) + .unwrap_or_else(|| "unknown".to_string()); + let mut result = if output.exit_code == 0 { + Line::from("✓".green().bold()) + } else { + Line::from(vec![ + "✗".red().bold(), + format!(" ({})", output.exit_code).into(), + ]) + }; + result.push_span(format!(" • {duration}").dim()); + lines.push(result); + } + lines.push("".into()); + } + lines + } +} + +impl ExecCell { + fn is_active(&self) -> bool { + self.calls.iter().any(|c| c.output.is_none()) + } + + fn exploring_display_lines(&self, width: u16) -> Vec> { + let mut lines: Vec> = Vec::new(); + let active_start_time = self + .calls + .iter() + .find(|c| c.output.is_none()) + .and_then(|c| c.start_time); + lines.push(Line::from(vec![ + if self.is_active() { + // Show an animated spinner while exploring + spinner(active_start_time) } else { - lines.push(Line::from(vec![" ".into(), part.to_string().into()])); + "•".bold() + }, + " ".into(), + if self.is_active() { + "Exploring".bold() + } else { + "Explored".bold() + }, + ])); + let mut calls = self.calls.clone(); + let mut first = true; + while !calls.is_empty() { + let mut call = calls.remove(0); + if call + .parsed + .iter() + .all(|c| matches!(c, ParsedCommand::Read { .. })) + { + while let Some(next) = calls.first() { + if next + .parsed + .iter() + .all(|c| matches!(c, ParsedCommand::Read { .. })) + { + call.parsed.extend(next.parsed.clone()); + calls.remove(0); + } else { + break; + } + } + } + let call_lines: Vec<(&str, Vec>)> = if call + .parsed + .iter() + .all(|c| matches!(c, ParsedCommand::Read { .. })) + { + let names: Vec = call + .parsed + .iter() + .map(|c| match c { + ParsedCommand::Read { name, .. } => name.clone(), + _ => unreachable!(), + }) + .unique() + .collect(); + vec![( + "Read", + itertools::Itertools::intersperse( + names.into_iter().map(|n| n.into()), + ", ".dim(), + ) + .collect(), + )] + } else { + let mut lines = Vec::new(); + for p in call.parsed { + match p { + ParsedCommand::Read { name, .. } => { + lines.push(("Read", vec![name.into()])); + } + ParsedCommand::ListFiles { cmd, path } => { + lines.push(("List", vec![path.unwrap_or(cmd).into()])); + } + ParsedCommand::Search { cmd, query, path } => { + lines.push(( + "Search", + match (query, path) { + (Some(q), Some(p)) => { + vec![q.into(), " in ".dim(), p.into()] + } + (Some(q), None) => vec![q.into()], + _ => vec![cmd.into()], + }, + )); + } + ParsedCommand::Unknown { cmd } => { + lines.push(("Run", vec![cmd.into()])); + } + } + } + lines + }; + for (title, line) in call_lines { + let prefix_len = 4 + title.len() + 1; // " └ " + title + " " + let wrapped = crate::insert_history::word_wrap_lines( + &[Line::from(line)], + width.saturating_sub(prefix_len as u16), + ); + let mut first_sub = true; + for mut line in wrapped { + let mut spans = Vec::with_capacity(line.spans.len() + 1); + spans.push(if first { + first = false; + " └ ".dim() + } else { + " ".into() + }); + if first_sub { + first_sub = false; + spans.push(title.cyan()); + spans.push(" ".into()); + } else { + spans.push(" ".repeat(title.width() + 1).into()); + } + spans.extend(line.spans.into_iter()); + line.spans = spans; + lines.push(line); + } } } + lines + } - // Command output: include full stdout and stderr (no truncation) - if let Some(output) = self.output.as_ref() { - lines.extend(output.formatted_output.lines().map(ansi_escape_line)); + fn command_display_lines(&self, width: u16) -> Vec> { + use textwrap::Options as TwOptions; + use textwrap::WordSplitter; + + let mut lines: Vec> = Vec::new(); + let [call] = &self.calls.as_slice() else { + panic!("Expected exactly one call in a command display cell"); + }; + let success = call.output.as_ref().map(|o| o.exit_code == 0); + let bullet = match success { + Some(true) => "•".green().bold(), + Some(false) => "•".red().bold(), + None => spinner(call.start_time), + }; + let title = if self.is_active() { "Running" } else { "Ran" }; + let cmd_display = strip_bash_lc_and_escape(&call.command); + + // If the command fits on the same line as the header at the current width, + // show a single compact line: "• Ran ". Use the width of + // "• Running " (including trailing space) as the reserved prefix width. + // If the command contains newlines, always use the multi-line variant. + let reserved = "• Running ".width(); + let mut branch_consumed = false; + + if !cmd_display.contains('\n') + && cmd_display.width() < (width as usize).saturating_sub(reserved) + { + lines.push(Line::from(vec![ + bullet, + " ".into(), + title.bold(), + " ".into(), + cmd_display.clone().into(), + ])); + } else { + branch_consumed = true; + lines.push(Line::from(vec![bullet, " ".into(), title.bold()])); + + // Wrap the command line. + for (i, line) in cmd_display.lines().enumerate() { + let wrapped = textwrap::wrap( + line, + TwOptions::new(width as usize) + .initial_indent(" ") + .subsequent_indent(" ") + .word_splitter(WordSplitter::NoHyphenation), + ); + lines.extend(wrapped.into_iter().enumerate().map(|(j, l)| { + if i == 0 && j == 0 { + Line::from(vec![" └ ".dim(), l[4..].to_string().into()]) + } else { + Line::from(l.to_string()) + } + })); + } } - - if let Some(output) = self.output.as_ref() { - let duration = self - .duration - .map(format_duration) - .unwrap_or_else(|| "unknown".to_string()); - let mut result = if output.exit_code == 0 { - Line::from("✓".green().bold()) - } else { - Line::from(vec![ - "✗".red().bold(), - format!(" ({})", output.exit_code).into(), - ]) - }; - - result.push_span(format!(" • {duration}").dim()); - lines.push(result); + if let Some(output) = call.output.as_ref() + && output.exit_code != 0 + { + let out = output_lines(Some(output), false, false, false) + .into_iter() + .join("\n"); + if !out.trim().is_empty() { + // Wrap the output. + for (i, line) in out.lines().enumerate() { + let wrapped = textwrap::wrap( + line, + TwOptions::new(width as usize - 4) + .word_splitter(WordSplitter::NoHyphenation), + ); + lines.extend(wrapped.into_iter().map(|l| { + Line::from(vec![ + if i == 0 && !branch_consumed { + " └ ".dim() + } else { + " ".dim() + }, + l.to_string().dim(), + ]) + })); + } + } } - lines } } @@ -169,7 +499,16 @@ impl WidgetRef for &ExecCell { width: area.width, height: area.height, }; - Paragraph::new(Text::from(self.display_lines())) + let lines = self.display_lines(area.width); + let max_rows = area.height as usize; + let rendered = if lines.len() > max_rows { + // Keep the last `max_rows` lines in original order + lines[lines.len() - max_rows..].to_vec() + } else { + lines + }; + + Paragraph::new(Text::from(rendered)) .wrap(Wrap { trim: false }) .render(content_area, buf); } @@ -177,22 +516,86 @@ impl WidgetRef for &ExecCell { impl ExecCell { /// Convert an active exec cell into a failed, completed exec cell. - /// Replaces the spinner with a red ✗ and sets a zero/elapsed duration. + /// Any call without output is marked as failed with a red ✗. pub(crate) fn into_failed(mut self) -> ExecCell { - let elapsed = self - .start_time - .map(|st| st.elapsed()) - .unwrap_or_else(|| Duration::from_millis(0)); - self.start_time = None; - self.duration = Some(elapsed); - self.output = Some(CommandOutput { - exit_code: 1, - stdout: String::new(), - stderr: String::new(), - formatted_output: String::new(), - }); + for call in self.calls.iter_mut() { + if call.output.is_none() { + let elapsed = call + .start_time + .map(|st| st.elapsed()) + .unwrap_or_else(|| Duration::from_millis(0)); + call.start_time = None; + call.duration = Some(elapsed); + call.output = Some(CommandOutput { + exit_code: 1, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }); + } + } self } + + pub(crate) fn new(call: ExecCall) -> Self { + ExecCell { calls: vec![call] } + } + + fn is_exploring_call(call: &ExecCall) -> bool { + !call.parsed.is_empty() + && call.parsed.iter().all(|p| { + matches!( + p, + ParsedCommand::Read { .. } + | ParsedCommand::ListFiles { .. } + | ParsedCommand::Search { .. } + ) + }) + } + + fn is_exploring_cell(&self) -> bool { + self.calls.iter().all(Self::is_exploring_call) + } + + pub(crate) fn with_added_call( + &self, + call_id: String, + command: Vec, + parsed: Vec, + ) -> Option { + let call = ExecCall { + call_id, + command, + parsed, + output: None, + start_time: Some(Instant::now()), + duration: None, + }; + if self.is_exploring_cell() && Self::is_exploring_call(&call) { + Some(Self { + calls: [self.calls.clone(), vec![call]].concat(), + }) + } else { + None + } + } + + pub(crate) fn complete_call( + &mut self, + call_id: &str, + output: CommandOutput, + duration: Duration, + ) { + if let Some(call) = self.calls.iter_mut().rev().find(|c| c.call_id == call_id) { + call.output = Some(output); + call.duration = Some(duration); + call.start_time = None; + } + } + + pub(crate) fn should_flush(&self) -> bool { + !self.is_exploring_cell() && self.calls.iter().all(|c| c.output.is_some()) + } } #[derive(Debug)] @@ -200,11 +603,8 @@ struct CompletedMcpToolCallWithImageOutput { _image: DynamicImage, } impl HistoryCell for CompletedMcpToolCallWithImageOutput { - fn display_lines(&self) -> Vec> { - vec![ - Line::from(""), - Line::from("tool result (image output omitted)"), - ] + fn display_lines(&self, _width: u16) -> Vec> { + vec![Line::from("tool result (image output omitted)")] } } @@ -237,11 +637,6 @@ fn padded_emoji(emoji: &str) -> String { format!("{emoji}\u{200A}") } -/// Convenience function over `padded_emoji()`. -fn padded_emoji_with(emoji: &str, text: impl AsRef) -> String { - format!("{}{}", padded_emoji(emoji), text.as_ref()) -} - pub(crate) fn new_session_info( config: &Config, event: SessionConfiguredEvent, @@ -264,7 +659,6 @@ pub(crate) fn new_session_info( }; let lines: Vec> = vec![ - Line::from(Span::from("")), Line::from(vec![ Span::raw(">_ ").dim(), Span::styled( @@ -330,7 +724,6 @@ pub(crate) fn new_session_info( PlainHistoryCell { lines: Vec::new() } } else { let lines = vec![ - Line::from(""), Line::from("model changed:".magenta().bold()), Line::from(format!("requested: {}", config.model)), Line::from(format!("used: {model}")), @@ -339,195 +732,47 @@ pub(crate) fn new_session_info( } } -pub(crate) fn new_user_prompt(message: String) -> PlainHistoryCell { - let mut lines: Vec> = Vec::new(); - lines.push(Line::from("")); - lines.push(Line::from("user".cyan().bold())); - lines.extend(message.lines().map(|l| Line::from(l.to_string()))); +pub(crate) fn new_user_prompt(message: String) -> UserHistoryCell { + UserHistoryCell { message } +} +pub(crate) fn new_user_approval_decision(lines: Vec>) -> PlainHistoryCell { PlainHistoryCell { lines } } pub(crate) fn new_active_exec_command( + call_id: String, command: Vec, parsed: Vec, - include_header: bool, ) -> ExecCell { - ExecCell { + ExecCell::new(ExecCall { + call_id, command, parsed, output: None, start_time: Some(Instant::now()), duration: None, - include_header, - } + }) } -pub(crate) fn new_completed_exec_command( - command: Vec, - parsed: Vec, - output: CommandOutput, - include_header: bool, - duration: Duration, -) -> ExecCell { - ExecCell { - command, - parsed, - output: Some(output), - start_time: None, - duration: Some(duration), - include_header, - } -} - -fn exec_command_lines( - command: &[String], - parsed: &[ParsedCommand], - output: Option<&CommandOutput>, - start_time: Option, - include_header: bool, -) -> Vec> { - match parsed.is_empty() { - true => new_exec_command_generic(command, output, start_time, include_header), - false => new_parsed_command(command, parsed, output, start_time, include_header), - } -} -fn new_parsed_command( - _command: &[String], - parsed_commands: &[ParsedCommand], - output: Option<&CommandOutput>, - start_time: Option, - include_header: bool, -) -> Vec> { - let mut lines: Vec = Vec::new(); - // Leading spacer and header line above command list - if include_header { - lines.push(Line::from("")); - lines.push(Line::from(">_".magenta())); - } - - // Determine the leading status marker: spinner while running, ✓ on success, ✗ on failure. - let status_marker: Span<'static> = match output { - None => { - // Animated braille spinner – choose frame based on elapsed time. - const FRAMES: &[char] = &['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; - let idx = start_time - .map(|st| ((st.elapsed().as_millis() / 100) as usize) % FRAMES.len()) - .unwrap_or(0); - let ch = FRAMES[idx]; - Span::raw(format!("{ch}")) - } - Some(o) if o.exit_code == 0 => Span::styled("✓", Style::default().fg(Color::Green)), - Some(_) => Span::styled("✗", Style::default().fg(Color::Red)), - }; - - for parsed in parsed_commands.iter() { - let text = match parsed { - ParsedCommand::Read { name, .. } => padded_emoji_with("📖", name), - ParsedCommand::ListFiles { cmd, path } => match path { - Some(p) => padded_emoji_with("📂", p), - None => padded_emoji_with("📂", cmd), - }, - ParsedCommand::Search { query, path, cmd } => match (query, path) { - (Some(q), Some(p)) => padded_emoji_with("🔎", format!("{q} in {p}")), - (Some(q), None) => padded_emoji_with("🔎", q), - (None, Some(p)) => padded_emoji_with("🔎", p), - (None, None) => padded_emoji_with("🔎", cmd), - }, - ParsedCommand::Format { .. } => padded_emoji_with("✨", "Formatting"), - ParsedCommand::Test { cmd } => padded_emoji_with("🧪", cmd), - ParsedCommand::Lint { cmd, .. } => padded_emoji_with("🧹", cmd), - ParsedCommand::Unknown { cmd } => padded_emoji_with("⌨️", cmd), - ParsedCommand::Noop { cmd } => padded_emoji_with("🔄", cmd), - }; - // Prefix: two spaces, marker, space. Continuations align under the text block. - for (j, line_text) in text.lines().enumerate() { - if j == 0 { - lines.push(Line::from(vec![ - " ".into(), - status_marker.clone(), - " ".into(), - line_text.to_string().light_blue(), - ])); - } else { - lines.push(Line::from(vec![ - " ".into(), - line_text.to_string().light_blue(), - ])); - } - } - } - - lines.extend(output_lines(output, true, false)); - - lines -} - -fn new_exec_command_generic( - command: &[String], - output: Option<&CommandOutput>, - start_time: Option, - include_header: bool, -) -> Vec> { - let mut lines: Vec> = Vec::new(); - // Leading spacer and header line above command list - if include_header { - lines.push(Line::from("")); - lines.push(Line::from(">_".magenta())); - } - let command_escaped = strip_bash_lc_and_escape(command); - - // Determine marker: spinner while running, ✓/✗ when completed - let status_marker: Span<'static> = match output { - None => { - const FRAMES: &[char] = &['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; - let idx = start_time - .map(|st| ((st.elapsed().as_millis() / 100) as usize) % FRAMES.len()) - .unwrap_or(0); - let ch = FRAMES[idx]; - Span::raw(format!("{ch}")) - } - Some(o) if o.exit_code == 0 => Span::styled("✓", Style::default().fg(Color::Green)), - Some(_) => Span::styled("✗", Style::default().fg(Color::Red)), - }; - - for (i, line) in command_escaped.lines().enumerate() { - if i == 0 { - lines.push(Line::from(vec![ - Span::raw(" "), - status_marker.clone(), - Span::raw(" "), - Span::raw(line.to_string()), - ])); - } else { - lines.push(Line::from(vec![ - Span::styled(" ", Style::default().add_modifier(Modifier::DIM)), - Span::raw(line.to_string()), - ])); - } - } - - lines.extend(output_lines(output, false, true)); - - lines +fn spinner(start_time: Option) -> Span<'static> { + const FRAMES: &[char] = &['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; + let idx = start_time + .map(|st| ((st.elapsed().as_millis() / 100) as usize) % FRAMES.len()) + .unwrap_or(0); + let ch = FRAMES[idx]; + ch.to_string().into() } pub(crate) fn new_active_mcp_tool_call(invocation: McpInvocation) -> PlainHistoryCell { let title_line = Line::from(vec!["tool".magenta(), " running...".dim()]); - let lines: Vec = vec![ - Line::from(""), - title_line, - format_mcp_invocation(invocation.clone()), - ]; + let lines: Vec = vec![title_line, format_mcp_invocation(invocation.clone())]; PlainHistoryCell { lines } } pub(crate) fn new_web_search_call(query: String) -> PlainHistoryCell { - let lines: Vec> = vec![ - Line::from(""), - Line::from(vec![padded_emoji("🌐").into(), query.into()]), - ]; + let lines: Vec> = vec![Line::from(vec![padded_emoji("🌐").into(), query.into()])]; PlainHistoryCell { lines } } @@ -647,8 +892,6 @@ pub(crate) fn new_completed_mcp_tool_call( } }; - // Leading blank separator at the start of this cell - lines.insert(0, Line::from("")); Box::new(PlainHistoryCell { lines }) } @@ -658,7 +901,6 @@ pub(crate) fn new_status_output( session_id: &Option, ) -> PlainHistoryCell { let mut lines: Vec> = Vec::new(); - lines.push(Line::from("")); lines.push(Line::from("/status".magenta())); let config_entries = create_config_summary_entries(config); @@ -749,6 +991,7 @@ pub(crate) fn new_status_output( agents_list.join(", ").into(), ])); } + lines.push(Line::from("")); // 👤 Account (only if ChatGPT tokens exist), shown under the first block let auth_file = get_auth_file(&config.codex_home); @@ -849,7 +1092,6 @@ pub(crate) fn new_status_output( /// Render a summary of configured MCP servers from the current `Config`. pub(crate) fn empty_mcp_output() -> PlainHistoryCell { let lines: Vec> = vec![ - Line::from(""), Line::from("/mcp".magenta()), Line::from(""), Line::from(vec!["🔌 ".into(), "MCP Tools".bold()]), @@ -939,124 +1181,104 @@ pub(crate) fn new_error_event(message: String) -> PlainHistoryCell { // Use a hair space (U+200A) to create a subtle, near-invisible separation // before the text. VS16 is intentionally omitted to keep spacing tighter // in terminals like Ghostty. - let lines: Vec> = vec![ - "".into(), - vec![padded_emoji("🖐").red().bold(), " ".into(), message.into()].into(), - ]; + let lines: Vec> = + vec![vec![padded_emoji("🖐").red().bold(), " ".into(), message.into()].into()]; PlainHistoryCell { lines } } pub(crate) fn new_stream_error_event(message: String) -> PlainHistoryCell { - let lines: Vec> = vec![ - vec![ - padded_emoji("⚠").magenta().bold(), - " ".into(), - message.dim(), - ] - .into(), - "".into(), - ]; + let lines: Vec> = vec![vec![padded_emoji("⚠️").into(), message.dim()].into()]; PlainHistoryCell { lines } } /// Render a user‑friendly plan update styled like a checkbox todo list. -pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlainHistoryCell { +pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlanUpdateCell { let UpdatePlanArgs { explanation, plan } = update; + PlanUpdateCell { explanation, plan } +} - let mut lines: Vec> = Vec::new(); - // Leading blank for separation - lines.push(Line::from("")); - // Header with progress summary - let total = plan.len(); - let completed = plan - .iter() - .filter(|p| matches!(p.status, StepStatus::Completed)) - .count(); +#[derive(Debug)] +pub(crate) struct PlanUpdateCell { + explanation: Option, + plan: Vec, +} - let width: usize = 10; - let filled = if total > 0 { - (completed * width + total / 2) / total - } else { - 0 - }; - let empty = width.saturating_sub(filled); +impl HistoryCell for PlanUpdateCell { + fn display_lines(&self, width: u16) -> Vec> { + let render_note = |text: &str| -> Vec> { + let wrap_width = width.saturating_sub(4).max(1) as usize; + textwrap::wrap(text, wrap_width) + .into_iter() + .map(|s| s.to_string().dim().italic().into()) + .collect() + }; - let mut header: Vec = Vec::new(); - header.push(Span::raw("📋")); - header.push(Span::styled( - " Update plan", - Style::default().add_modifier(Modifier::BOLD).magenta(), - )); - header.push(Span::raw(" [")); - if filled > 0 { - header.push(Span::styled( - "█".repeat(filled), - Style::default().fg(Color::Green), - )); - } - if empty > 0 { - header.push(Span::styled( - "░".repeat(empty), - Style::default().add_modifier(Modifier::DIM), - )); - } - header.push(Span::raw("] ")); - header.push(Span::raw(format!("{completed}/{total}"))); - lines.push(Line::from(header)); - - // Optional explanation/note from the model - if let Some(expl) = explanation.and_then(|s| { - let t = s.trim().to_string(); - if t.is_empty() { None } else { Some(t) } - }) { - lines.push(Line::from("note".dim().italic())); - for l in expl.lines() { - lines.push(Line::from(l.to_string()).dim()); - } - } - - // Steps styled as checkbox items - if plan.is_empty() { - lines.push(Line::from("(no steps provided)".dim().italic())); - } else { - for (idx, PlanItemArg { step, status }) in plan.into_iter().enumerate() { - let (box_span, text_span) = match status { - StepStatus::Completed => ( - Span::styled("✔", Style::default().fg(Color::Green)), - Span::styled( - step, - Style::default().add_modifier(Modifier::CROSSED_OUT | Modifier::DIM), - ), - ), - StepStatus::InProgress => ( - Span::raw("□"), - Span::styled( - step, - Style::default() - .fg(Color::Cyan) - .add_modifier(Modifier::BOLD), - ), - ), - StepStatus::Pending => ( - Span::raw("□"), - Span::styled(step, Style::default().add_modifier(Modifier::DIM)), - ), + let render_step = |status: &StepStatus, text: &str| -> Vec> { + let (box_str, step_style) = match status { + StepStatus::Completed => ("✔ ", Style::default().crossed_out().dim()), + StepStatus::InProgress => ("□ ", Style::default().cyan().bold()), + StepStatus::Pending => ("□ ", Style::default().dim()), }; - let prefix = if idx == 0 { - Span::raw(" └ ") - } else { - Span::raw(" ") - }; - lines.push(Line::from(vec![ - prefix, - box_span, - Span::raw(" "), - text_span, - ])); - } - } + let wrap_width = (width as usize) + .saturating_sub(4) + .saturating_sub(box_str.width()) + .max(1); + let parts = textwrap::wrap(text, wrap_width); + let step_text = parts + .into_iter() + .map(|s| s.to_string().set_style(step_style).into()) + .collect(); + prefix_lines(step_text, &box_str.into(), &" ".into()) + }; - PlainHistoryCell { lines } + fn prefix_lines( + lines: Vec>, + initial_prefix: &Span<'static>, + subsequent_prefix: &Span<'static>, + ) -> Vec> { + lines + .into_iter() + .enumerate() + .map(|(i, l)| { + Line::from( + [ + vec![if i == 0 { + initial_prefix.clone() + } else { + subsequent_prefix.clone() + }], + l.spans, + ] + .concat(), + ) + }) + .collect() + } + + let mut lines: Vec> = vec![]; + lines.push(vec!["• ".into(), "Updated Plan".bold()].into()); + + let mut indented_lines = vec![]; + let note = self + .explanation + .as_ref() + .map(|s| s.trim()) + .filter(|t| !t.is_empty()); + if let Some(expl) = note { + indented_lines.extend(render_note(expl)); + }; + + if self.plan.is_empty() { + indented_lines.push(Line::from("(no steps provided)".dim().italic())); + } else { + for PlanItemArg { step, status } in self.plan.iter() { + indented_lines.extend(render_step(status, step)); + } + } + lines.extend(prefix_lines(indented_lines, &" └ ".into(), &" ".into())); + + lines + } } /// Create a new `PendingPatch` cell that lists the file‑level summary of @@ -1065,28 +1287,13 @@ pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlainHistoryCell { pub(crate) fn new_patch_event( event_type: PatchEventType, changes: HashMap, -) -> PlainHistoryCell { - let title = match &event_type { - PatchEventType::ApprovalRequest => "proposed patch", - PatchEventType::ApplyBegin { - auto_approved: true, - } => "✏️ Applying patch", - PatchEventType::ApplyBegin { - auto_approved: false, - } => { - let lines: Vec> = vec![ - Line::from(""), - Line::from("✏️ Applying patch".magenta().bold()), - ]; - return PlainHistoryCell { lines }; - } - }; - - let mut lines: Vec> = create_diff_summary(title, &changes, event_type); - // Add leading blank separator for the cell - lines.insert(0, Line::from("")); - - PlainHistoryCell { lines } + cwd: &Path, +) -> PatchHistoryCell { + PatchHistoryCell { + event_type, + changes, + cwd: cwd.to_path_buf(), + } } pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell { @@ -1105,59 +1312,10 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell { }), true, true, + true, )); } - // Leading blank separator - lines.insert(0, Line::from("")); - PlainHistoryCell { lines } -} - -pub(crate) fn new_patch_apply_success(stdout: String) -> PlainHistoryCell { - let mut lines: Vec> = Vec::new(); - - // Success title - lines.push(Line::from("✓ Applied patch".magenta().bold())); - - if !stdout.trim().is_empty() { - let mut iter = stdout.lines(); - for (i, raw) in iter.by_ref().take(TOOL_CALL_MAX_LINES).enumerate() { - let prefix = if i == 0 { " └ " } else { " " }; - - // First line is the header; dim it entirely. - if i == 0 { - let s = format!("{prefix}{raw}"); - lines.push(ansi_escape_line(&s).dim()); - continue; - } - - // Subsequent lines should look like: "M path/to/file". - // Colorize the status letter like `git status` (e.g., M red). - let status = raw.chars().next(); - let rest = raw.get(1..).unwrap_or(""); - - let status_span = match status { - Some('M') => "M".red(), - Some('A') => "A".green(), - Some('D') => "D".red(), - Some(other) => other.to_string().into(), - None => "".into(), - }; - - lines.push(Line::from(vec![ - prefix.into(), - status_span, - ansi_escape_line(rest).to_string().into(), - ])); - } - let remaining = iter.count(); - if remaining > 0 { - lines.push(Line::from("")); - lines.push(Line::from(format!("... +{remaining} lines")).dim()); - } - } - // Leading blank separator - lines.insert(0, Line::from("")); PlainHistoryCell { lines } } @@ -1166,7 +1324,6 @@ pub(crate) fn new_reasoning_block( config: &Config, ) -> TranscriptOnlyHistoryCell { let mut lines: Vec> = Vec::new(); - lines.push(Line::from("")); lines.push(Line::from("thinking".magenta().italic())); append_markdown(&full_reasoning_buffer, &mut lines, config); TranscriptOnlyHistoryCell { lines } @@ -1176,6 +1333,7 @@ fn output_lines( output: Option<&CommandOutput>, only_err: bool, include_angle_pipe: bool, + include_prefix: bool, ) -> Vec> { let CommandOutput { exit_code, @@ -1198,7 +1356,9 @@ fn output_lines( let head_end = total.min(limit); for (i, raw) in lines[..head_end].iter().enumerate() { let mut line = ansi_escape_line(raw); - let prefix = if i == 0 && include_angle_pipe { + let prefix = if !include_prefix { + "" + } else if i == 0 && include_angle_pipe { " └ " } else { " " @@ -1224,7 +1384,9 @@ fn output_lines( }; for raw in lines[tail_start..].iter() { let mut line = ansi_escape_line(raw); - line.spans.insert(0, " ".into()); + if include_prefix { + line.spans.insert(0, " ".into()); + } line.spans.iter_mut().for_each(|span| { span.style = span.style.add_modifier(Modifier::DIM); }); @@ -1260,18 +1422,552 @@ mod tests { use super::*; #[test] - fn parsed_command_with_newlines_starts_each_line_at_origin() { - let parsed = vec![ParsedCommand::Unknown { - cmd: "printf 'foo\nbar'".to_string(), - }]; - let lines = exec_command_lines(&[], &parsed, None, None, true); - assert!(lines.len() >= 4); - // Leading spacer then header line - assert!(lines[0].spans.is_empty() || lines[0].spans[0].content.is_empty()); - assert_eq!(lines[1].spans[0].content, ">_"); - // First rendered command line starts with two-space + marker. - assert_eq!(lines[2].spans[0].content, " "); - // Continuation lines align under the text block. - assert_eq!(lines[3].spans[0].content, " "); + fn coalesces_sequential_reads_within_one_call() { + // Build one exec cell with a Search followed by two Reads + let call_id = "c1".to_string(); + let mut cell = ExecCell::new(ExecCall { + call_id: call_id.clone(), + command: vec!["bash".into(), "-lc".into(), "echo".into()], + parsed: vec![ + ParsedCommand::Search { + query: Some("shimmer_spans".into()), + path: None, + cmd: "rg shimmer_spans".into(), + }, + ParsedCommand::Read { + name: "shimmer.rs".into(), + cmd: "cat shimmer.rs".into(), + }, + ParsedCommand::Read { + name: "status_indicator_widget.rs".into(), + cmd: "cat status_indicator_widget.rs".into(), + }, + ], + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + // Mark call complete so markers are ✓ + cell.complete_call( + &call_id, + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + + let lines = cell.display_lines(80); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn coalesces_reads_across_multiple_calls() { + let mut cell = ExecCell::new(ExecCall { + call_id: "c1".to_string(), + command: vec!["bash".into(), "-lc".into(), "echo".into()], + parsed: vec![ParsedCommand::Search { + query: Some("shimmer_spans".into()), + path: None, + cmd: "rg shimmer_spans".into(), + }], + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + // Call 1: Search only + cell.complete_call( + "c1", + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + // Call 2: Read A + cell = cell + .with_added_call( + "c2".into(), + vec!["bash".into(), "-lc".into(), "echo".into()], + vec![ParsedCommand::Read { + name: "shimmer.rs".into(), + cmd: "cat shimmer.rs".into(), + }], + ) + .unwrap(); + cell.complete_call( + "c2", + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + // Call 3: Read B + cell = cell + .with_added_call( + "c3".into(), + vec!["bash".into(), "-lc".into(), "echo".into()], + vec![ParsedCommand::Read { + name: "status_indicator_widget.rs".into(), + cmd: "cat status_indicator_widget.rs".into(), + }], + ) + .unwrap(); + cell.complete_call( + "c3", + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + + let lines = cell.display_lines(80); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn coalesced_reads_dedupe_names() { + let mut cell = ExecCell::new(ExecCall { + call_id: "c1".to_string(), + command: vec!["bash".into(), "-lc".into(), "echo".into()], + parsed: vec![ + ParsedCommand::Read { + name: "auth.rs".into(), + cmd: "cat auth.rs".into(), + }, + ParsedCommand::Read { + name: "auth.rs".into(), + cmd: "cat auth.rs".into(), + }, + ParsedCommand::Read { + name: "shimmer.rs".into(), + cmd: "cat shimmer.rs".into(), + }, + ], + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + cell.complete_call( + "c1", + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + let lines = cell.display_lines(80); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn multiline_command_wraps_with_extra_indent_on_subsequent_lines() { + // Create a completed exec cell with a multiline command + let cmd = "set -o pipefail\ncargo test --all-features --quiet".to_string(); + let call_id = "c1".to_string(); + let mut cell = ExecCell::new(ExecCall { + call_id: call_id.clone(), + command: vec!["bash".into(), "-lc".into(), cmd], + parsed: Vec::new(), + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + // Mark call complete so it renders as "Ran" + cell.complete_call( + &call_id, + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + + // Small width to force wrapping on both lines + let width: u16 = 28; + let lines = cell.display_lines(width); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn single_line_command_compact_when_fits() { + let call_id = "c1".to_string(); + let mut cell = ExecCell::new(ExecCall { + call_id: call_id.clone(), + command: vec!["echo".into(), "ok".into()], + parsed: Vec::new(), + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + cell.complete_call( + &call_id, + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + // Wide enough that it fits inline + let lines = cell.display_lines(80); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn single_line_command_wraps_with_four_space_continuation() { + let call_id = "c1".to_string(); + let long = "a_very_long_token_without_spaces_to_force_wrapping".to_string(); + let mut cell = ExecCell::new(ExecCall { + call_id: call_id.clone(), + command: vec!["bash".into(), "-lc".into(), long], + parsed: Vec::new(), + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + cell.complete_call( + &call_id, + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + let lines = cell.display_lines(24); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn multiline_command_without_wrap_uses_branch_then_eight_spaces() { + let call_id = "c1".to_string(); + let cmd = "echo one\necho two".to_string(); + let mut cell = ExecCell::new(ExecCall { + call_id: call_id.clone(), + command: vec!["bash".into(), "-lc".into(), cmd], + parsed: Vec::new(), + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + cell.complete_call( + &call_id, + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + let lines = cell.display_lines(80); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn multiline_command_both_lines_wrap_with_correct_prefixes() { + let call_id = "c1".to_string(); + let cmd = "first_token_is_long_enough_to_wrap\nsecond_token_is_also_long_enough_to_wrap" + .to_string(); + let mut cell = ExecCell::new(ExecCall { + call_id: call_id.clone(), + command: vec!["bash".into(), "-lc".into(), cmd], + parsed: Vec::new(), + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + cell.complete_call( + &call_id, + CommandOutput { + exit_code: 0, + stdout: String::new(), + stderr: String::new(), + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + let lines = cell.display_lines(28); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn stderr_tail_more_than_five_lines_snapshot() { + // Build an exec cell with a non-zero exit and 10 lines on stderr to exercise + // the head/tail rendering and gutter prefixes. + let call_id = "c_err".to_string(); + let mut cell = ExecCell::new(ExecCall { + call_id: call_id.clone(), + command: vec!["bash".into(), "-lc".into(), "seq 1 10 1>&2 && false".into()], + parsed: Vec::new(), + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + let stderr: String = (1..=10) + .map(|n| n.to_string()) + .collect::>() + .join("\n"); + cell.complete_call( + &call_id, + CommandOutput { + exit_code: 1, + stdout: String::new(), + stderr, + formatted_output: String::new(), + }, + Duration::from_millis(1), + ); + + let rendered = cell + .display_lines(80) + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn ran_cell_multiline_with_stderr_snapshot() { + // Build an exec cell that completes (so it renders as "Ran") with a + // command long enough that it must render on its own line under the + // header, and include a couple of stderr lines to verify the output + // block prefixes and wrapping. + let call_id = "c_wrap_err".to_string(); + let long_cmd = + "echo this_is_a_very_long_single_token_that_will_wrap_across_the_available_width"; + let mut cell = ExecCell::new(ExecCall { + call_id: call_id.clone(), + command: vec!["bash".into(), "-lc".into(), long_cmd.to_string()], + parsed: Vec::new(), + output: None, + start_time: Some(Instant::now()), + duration: None, + }); + + let stderr = "error: first line on stderr\nerror: second line on stderr".to_string(); + cell.complete_call( + &call_id, + CommandOutput { + exit_code: 1, + stdout: String::new(), + stderr, + formatted_output: String::new(), + }, + Duration::from_millis(5), + ); + + // Narrow width to force the command to render under the header line. + let width: u16 = 28; + let rendered = cell + .display_lines(width) + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + #[test] + fn user_history_cell_wraps_and_prefixes_each_line_snapshot() { + let msg = "one two three four five six seven"; + let cell = UserHistoryCell { + message: msg.to_string(), + }; + + // Small width to force wrapping more clearly. Effective wrap width is width-1 due to the ▌ prefix. + let width: u16 = 12; + let lines = cell.display_lines(width); + + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + + insta::assert_snapshot!(rendered); + } + + #[test] + fn plan_update_with_note_and_wrapping_snapshot() { + // Long explanation forces wrapping; include long step text to verify step wrapping and alignment. + let update = UpdatePlanArgs { + explanation: Some( + "I’ll update Grafana call error handling by adding retries and clearer messages when the backend is unreachable." + .to_string(), + ), + plan: vec![ + PlanItemArg { + step: "Investigate existing error paths and logging around HTTP timeouts".into(), + status: StepStatus::Completed, + }, + PlanItemArg { + step: "Harden Grafana client error handling with retry/backoff and user‑friendly messages".into(), + status: StepStatus::InProgress, + }, + PlanItemArg { + step: "Add tests for transient failure scenarios and surfacing to the UI".into(), + status: StepStatus::Pending, + }, + ], + }; + + let cell = new_plan_update(update); + // Narrow width to force wrapping for both the note and steps + let lines = cell.display_lines(32); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); + } + + #[test] + fn plan_update_without_note_snapshot() { + let update = UpdatePlanArgs { + explanation: None, + plan: vec![ + PlanItemArg { + step: "Define error taxonomy".into(), + status: StepStatus::InProgress, + }, + PlanItemArg { + step: "Implement mapping to user messages".into(), + status: StepStatus::Pending, + }, + ], + }; + + let cell = new_plan_update(update); + let lines = cell.display_lines(40); + let rendered = lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.as_ref()) + .collect::() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!(rendered); } } diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index b6388709..2c35c877 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -21,7 +21,8 @@ use ratatui::text::Span; use textwrap::Options as TwOptions; use textwrap::WordSplitter; -/// Insert `lines` above the viewport. +/// Insert `lines` above the viewport using the terminal's backend writer +/// (avoids direct stdout references). pub(crate) fn insert_history_lines(terminal: &mut tui::Terminal, lines: Vec) { let mut out = std::io::stdout(); insert_history_lines_to_writer(terminal, &mut out, lines); @@ -262,7 +263,10 @@ where } /// Word-aware wrapping for a list of `Line`s preserving styles. -pub(crate) fn word_wrap_lines(lines: &[Line], width: u16) -> Vec> { +pub(crate) fn word_wrap_lines<'a, I>(lines: I, width: u16) -> Vec> +where + I: IntoIterator>, +{ let mut out = Vec::new(); let w = width.max(1) as usize; for line in lines { diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 544aec27..5da8e0d6 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -34,7 +34,6 @@ mod chatwidget; mod citation_regex; mod cli; mod clipboard_paste; -mod common; pub mod custom_terminal; mod diff_render; mod exec_command; diff --git a/codex-rs/tui/src/onboarding/onboarding_screen.rs b/codex-rs/tui/src/onboarding/onboarding_screen.rs index f0009bb9..58ff5a44 100644 --- a/codex-rs/tui/src/onboarding/onboarding_screen.rs +++ b/codex-rs/tui/src/onboarding/onboarding_screen.rs @@ -1,4 +1,4 @@ -use codex_core::util::is_inside_git_repo; +use codex_core::git_info::get_git_repo_root; use codex_login::AuthManager; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -88,7 +88,7 @@ impl OnboardingScreen { auth_manager, })) } - let is_git_repo = is_inside_git_repo(&cwd); + let is_git_repo = get_git_repo_root(&cwd).is_some(); let highlighted = if is_git_repo { TrustDirectorySelection::Trust } else { diff --git a/codex-rs/tui/src/session_log.rs b/codex-rs/tui/src/session_log.rs index 4937be8b..3118800b 100644 --- a/codex-rs/tui/src/session_log.rs +++ b/codex-rs/tui/src/session_log.rs @@ -140,16 +140,6 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { }); LOGGER.write_json_line(value); } - // Internal UI events; still log for fidelity, but avoid heavy payloads. - AppEvent::InsertHistoryLines(lines) => { - let value = json!({ - "ts": now_ts(), - "dir": "to_tui", - "kind": "insert_history", - "lines": lines.len(), - }); - LOGGER.write_json_line(value); - } AppEvent::InsertHistoryCell(cell) => { let value = json!({ "ts": now_ts(), 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 index 143113fb..1ff57094 100644 --- 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 @@ -1,9 +1,9 @@ --- source: tui/src/diff_render.rs +assertion_line: 765 expression: terminal.backend() --- -"proposed patch to 1 file (+2 -0) " -" └ README.md " +"• Proposed Change README.md (+2 -0) " " 1 +first line " " 2 +second line " " " @@ -12,3 +12,4 @@ expression: terminal.backend() " " " " " " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_add_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_add_block.snap new file mode 100644 index 00000000..254f51b6 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_add_block.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +"• Added new_file.txt (+2 -0) " +" 1 +alpha " +" 2 +beta " +" " +" " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_delete_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_delete_block.snap new file mode 100644 index 00000000..989200b2 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_delete_block.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +"• Deleted tmp_delete_example.txt (+0 -3) " +" 1 -first " +" 2 -second " +" 3 -third " +" " +" " +" " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_multiple_files_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_multiple_files_block.snap new file mode 100644 index 00000000..62e1f58e --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_multiple_files_block.snap @@ -0,0 +1,18 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +"• Edited 2 files (+2 -1) " +" └ a.txt (+1 -1) " +" 1 -one " +" 1 +one changed " +" " +" └ b.txt (+1 -0) " +" 1 +new " +" " +" " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block.snap new file mode 100644 index 00000000..305120d3 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block.snap @@ -0,0 +1,17 @@ +--- +source: tui/src/diff_render.rs +assertion_line: 748 +expression: terminal.backend() +--- +"• Edited example.txt (+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__apply_update_block_manual.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_manual.snap new file mode 100644 index 00000000..12c10ca4 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_manual.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +"• Change Approved example.txt (+1 -1) " +" " +" " +" " +" " +" " +" " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_relativizes_path.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_relativizes_path.snap new file mode 100644 index 00000000..825e50eb --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_relativizes_path.snap @@ -0,0 +1,15 @@ +--- +source: tui/src/diff_render.rs +assertion_line: 748 +expression: terminal.backend() +--- +"• Edited abs_old.rs → abs_new.rs (+1 -1) " +" 1 -X " +" 1 +X changed " +" 2 Y " +" " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines.snap new file mode 100644 index 00000000..d571b006 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines.snap @@ -0,0 +1,17 @@ +--- +source: tui/src/diff_render.rs +assertion_line: 748 +expression: terminal.backend() +--- +"• Edited long_example.txt (+1 -1) " +" 1 line 1 " +" 2 -short " +" 2 +short this_is_a_very_long_modified_line_that_should_wrap_acro " +" ss_multiple_terminal_columns_and_continue_even_further_beyond " +" _eighty_columns_to_force_multiple_wraps " +" 3 line 3 " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines_text.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines_text.snap new file mode 100644 index 00000000..ea495c10 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines_text.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/diff_render.rs +expression: text +--- + 1 1 + 2 -2 + 2 +added long line w + hich wraps and_if + _there_is_a_long_ + token_it_will_be_ + broken + 3 3 + 4 -4 + 4 +4 context line wh + ich also wraps ac + ross diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_with_rename_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_with_rename_block.snap new file mode 100644 index 00000000..89c2ddf1 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_with_rename_block.snap @@ -0,0 +1,17 @@ +--- +source: tui/src/diff_render.rs +assertion_line: 748 +expression: terminal.backend() +--- +"• Edited old_name.rs → new_name.rs (+1 -1) " +" 1 A " +" 2 -B " +" 2 +B changed " +" 3 C " +" " +" " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__blank_context_line.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__blank_context_line.snap index b38704f9..a662041e 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__blank_context_line.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__blank_context_line.snap @@ -1,9 +1,9 @@ --- source: tui/src/diff_render.rs +assertion_line: 765 expression: terminal.backend() --- -"proposed patch to 1 file (+1 -1) " -" └ example.txt " +"• Proposed Change example.txt (+1 -1) " " 1 " " 2 -Y " " 2 +Y changed " @@ -12,3 +12,4 @@ expression: terminal.backend() " " " " " " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__single_line_replacement_counts.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__single_line_replacement_counts.snap index b92dcbd6..d4a7bd8b 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__single_line_replacement_counts.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__single_line_replacement_counts.snap @@ -1,12 +1,13 @@ --- source: tui/src/diff_render.rs +assertion_line: 765 expression: terminal.backend() --- -"proposed patch to 1 file (+1 -1) " -" └ README.md " +"• Proposed Change README.md (+1 -1) " " 1 -# Codex CLI (Rust Implementation) " " 1 +# Codex CLI (Rust Implementation) banana " " " " " " " " " +" " 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 index 459791e0..6e5c34ba 100644 --- 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 @@ -1,9 +1,9 @@ --- source: tui/src/diff_render.rs +assertion_line: 765 expression: terminal.backend() --- -"proposed patch to 1 file (+1 -1) " -" └ src/lib.rs → src/lib_new.rs " +"• Proposed Change src/lib.rs → src/lib_new.rs (+1 -1) " " 1 line one " " 2 -line two " " 2 +line two changed " @@ -14,3 +14,4 @@ expression: terminal.backend() " " " " " " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__vertical_ellipsis_between_hunks.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__vertical_ellipsis_between_hunks.snap index 0f4bfd5a..bc774388 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__vertical_ellipsis_between_hunks.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__vertical_ellipsis_between_hunks.snap @@ -1,9 +1,9 @@ --- source: tui/src/diff_render.rs +assertion_line: 765 expression: terminal.backend() --- -"proposed patch to 1 file (+2 -2) " -" └ example.txt " +"• Proposed Change example.txt (+2 -2) " " 1 line 1 " " 2 -line 2 " " 2 +line two changed " @@ -18,3 +18,4 @@ expression: terminal.backend() " 9 +line nine changed " " 10 line 10 " " " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__coalesced_reads_dedupe_names.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__coalesced_reads_dedupe_names.snap new file mode 100644 index 00000000..2044d41a --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__coalesced_reads_dedupe_names.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Explored + └ Read auth.rs, shimmer.rs diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__coalesces_reads_across_multiple_calls.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__coalesces_reads_across_multiple_calls.snap new file mode 100644 index 00000000..fcfa31ee --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__coalesces_reads_across_multiple_calls.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Explored + └ Search shimmer_spans + Read shimmer.rs, status_indicator_widget.rs diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__coalesces_sequential_reads_within_one_call.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__coalesces_sequential_reads_within_one_call.snap new file mode 100644 index 00000000..533a489e --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__coalesces_sequential_reads_within_one_call.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Explored + └ Search shimmer_spans + Read shimmer.rs + Read status_indicator_widget.rs diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_both_lines_wrap_with_correct_prefixes.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_both_lines_wrap_with_correct_prefixes.snap new file mode 100644 index 00000000..99d6579f --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_both_lines_wrap_with_correct_prefixes.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Ran + └ first_token_is_long_ + enough_to_wrap + second_token_is_also + _long_enough_to_wrap diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_without_wrap_uses_branch_then_eight_spaces.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_without_wrap_uses_branch_then_eight_spaces.snap new file mode 100644 index 00000000..7dc9d572 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_without_wrap_uses_branch_then_eight_spaces.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Ran + └ echo one + echo two diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_wraps_with_extra_indent_on_subsequent_lines.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_wraps_with_extra_indent_on_subsequent_lines.snap new file mode 100644 index 00000000..4e383a5a --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__multiline_command_wraps_with_extra_indent_on_subsequent_lines.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Ran + └ set -o pipefail + cargo test + --all-features + --quiet diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__plan_update_with_note_and_wrapping_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__plan_update_with_note_and_wrapping_snapshot.snap new file mode 100644 index 00000000..27649a11 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__plan_update_with_note_and_wrapping_snapshot.snap @@ -0,0 +1,20 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Updated Plan + └ I’ll update Grafana call + error handling by adding + retries and clearer + messages when the backend is + unreachable. + ✔ Investigate existing error + paths and logging around + HTTP timeouts + □ Harden Grafana client + error handling with retry/ + backoff and user‑friendly + messages + □ Add tests for transient + failure scenarios and + surfacing to the UI diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__plan_update_without_note_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__plan_update_without_note_snapshot.snap new file mode 100644 index 00000000..e3ac8844 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__plan_update_without_note_snapshot.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Updated Plan + └ □ Define error taxonomy + □ Implement mapping to user messages diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ran_cell_multiline_with_stderr_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ran_cell_multiline_with_stderr_snapshot.snap new file mode 100644 index 00000000..4a0ec938 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ran_cell_multiline_with_stderr_snapshot.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Ran + └ echo + this_is_a_very_long_ + single_token_that_wi + ll_wrap_across_the_a + vailable_width + error: first line on + stderr + error: second line on + stderr diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_compact_when_fits.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_compact_when_fits.snap new file mode 100644 index 00000000..b4a79261 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_compact_when_fits.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Ran echo ok diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_wraps_with_four_space_continuation.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_wraps_with_four_space_continuation.snap new file mode 100644 index 00000000..548d50ce --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__single_line_command_wraps_with_four_space_continuation.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Ran + └ a_very_long_toke + n_without_spaces + _to_force_wrappi + ng diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__stderr_tail_more_than_five_lines_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__stderr_tail_more_than_five_lines_snapshot.snap new file mode 100644 index 00000000..ccba8c22 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__stderr_tail_more_than_five_lines_snapshot.snap @@ -0,0 +1,15 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Ran seq 1 10 1>&2 && false + └ 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__user_history_cell_wraps_and_prefixes_each_line_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__user_history_cell_wraps_and_prefixes_each_line_snapshot.snap new file mode 100644 index 00000000..8b8cc38a --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__user_history_cell_wraps_and_prefixes_each_line_snapshot.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +▌one two +▌three four +▌five six +▌seven diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index 9c6db452..4edea186 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -1,3 +1,5 @@ +use crate::history_cell; +use crate::history_cell::HistoryCell; use codex_core::config::Config; use ratatui::text::Line; @@ -6,7 +8,7 @@ use super::StreamState; /// Sink for history insertions and animation control. pub(crate) trait HistorySink { - fn insert_history(&self, lines: Vec>); + fn insert_history_cell(&self, cell: Box); fn start_commit_animation(&self); fn stop_commit_animation(&self); } @@ -15,9 +17,9 @@ pub(crate) trait HistorySink { pub(crate) struct AppEventHistorySink(pub(crate) crate::app_event_sender::AppEventSender); impl HistorySink for AppEventHistorySink { - fn insert_history(&self, lines: Vec>) { + fn insert_history_cell(&self, cell: Box) { self.0 - .send(crate::app_event::AppEvent::InsertHistoryLines(lines)) + .send(crate::app_event::AppEvent::InsertHistoryCell(cell)) } fn start_commit_animation(&self) { self.0 @@ -66,10 +68,6 @@ impl StreamController { // leave header state unchanged; caller decides when to reset } - fn emit_header_if_needed(&mut self, out_lines: &mut Lines) -> bool { - self.header.maybe_emit(out_lines) - } - /// Begin an answer stream. Does not emit header yet; it is emitted on first commit. pub(crate) fn begin(&mut self, _sink: &impl HistorySink) { // Starting a new stream cancels any pending finish-from-previous-stream animation. @@ -124,10 +122,11 @@ impl StreamController { out_lines.extend(step.history); } if !out_lines.is_empty() { - let mut lines_with_header: Lines = Vec::new(); - self.emit_header_if_needed(&mut lines_with_header); - lines_with_header.extend(out_lines); - sink.insert_history(lines_with_header); + // Insert as a HistoryCell so display drops the header while transcript keeps it. + sink.insert_history_cell(Box::new(history_cell::AgentMessageCell::new( + out_lines, + self.header.maybe_emit_header(), + ))); } // Cleanup @@ -159,11 +158,10 @@ impl StreamController { } let step = { self.state.step() }; if !step.history.is_empty() { - let mut lines: Lines = Vec::new(); - self.emit_header_if_needed(&mut lines); - let mut out = lines; - out.extend(step.history); - sink.insert_history(out); + sink.insert_history_cell(Box::new(history_cell::AgentMessageCell::new( + step.history, + self.header.maybe_emit_header(), + ))); } let is_idle = self.state.is_idle(); @@ -244,8 +242,9 @@ mod tests { } } impl HistorySink for TestSink { - fn insert_history(&self, lines: Vec>) { - self.lines.borrow_mut().push(lines); + fn insert_history_cell(&self, cell: Box) { + // For tests, store the transcript representation of the cell. + self.lines.borrow_mut().push(cell.transcript_lines()); } fn start_commit_animation(&self) {} fn stop_commit_animation(&self) {} diff --git a/codex-rs/tui/src/streaming/mod.rs b/codex-rs/tui/src/streaming/mod.rs index eaaeeff4..3fc81360 100644 --- a/codex-rs/tui/src/streaming/mod.rs +++ b/codex-rs/tui/src/streaming/mod.rs @@ -62,20 +62,13 @@ impl HeaderEmitter { self.emitted_this_turn = false; } - pub(crate) fn maybe_emit(&mut self, out_lines: &mut Vec>) -> bool { + pub(crate) fn maybe_emit_header(&mut self) -> bool { if !self.emitted_in_stream && !self.emitted_this_turn { - // Add a leading blank line before the header for visual spacing - out_lines.push(ratatui::text::Line::from("")); - out_lines.push(render_header_line()); self.emitted_in_stream = true; self.emitted_this_turn = true; - return true; + true + } else { + false } - false } } - -fn render_header_line() -> ratatui::text::Line<'static> { - use ratatui::style::Stylize; - ratatui::text::Line::from("codex".magenta().bold()) -} diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 47a858c5..f769791d 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -510,6 +510,7 @@ impl Tui { } } + // Use synchronized update via backend instead of stdout() std::io::stdout().sync_update(|_| { #[cfg(unix)] { @@ -560,8 +561,7 @@ impl Tui { } terminal.draw(|frame| { draw_fn(frame); - })?; - Ok(()) + }) })? } } diff --git a/codex-rs/tui/src/user_approval_widget.rs b/codex-rs/tui/src/user_approval_widget.rs index e323574c..200ba2c0 100644 --- a/codex-rs/tui/src/user_approval_widget.rs +++ b/codex-rs/tui/src/user_approval_widget.rs @@ -29,6 +29,7 @@ use ratatui::widgets::Wrap; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; use crate::exec_command::strip_bash_lc_and_escape; +use crate::history_cell; /// Request coming from the agent that needs user approval. pub(crate) enum ApprovalRequest { @@ -258,12 +259,11 @@ impl UserApprovalWidget { } fn send_decision_with_feedback(&mut self, decision: ReviewDecision, feedback: String) { - let mut lines: Vec> = vec![Line::from("")]; match &self.approval_request { ApprovalRequest::Exec { command, .. } => { let cmd = strip_bash_lc_and_escape(command); - let mut cmd_span: Span = cmd.clone().into(); - cmd_span.style = cmd_span.style.add_modifier(Modifier::DIM); + // TODO: move this rendering into history_cell. + let mut lines: Vec> = vec![]; // Result line based on decision. match decision { @@ -316,18 +316,22 @@ impl UserApprovalWidget { )); } } + + if !feedback.trim().is_empty() { + lines.push(Line::from("feedback:")); + for l in feedback.lines() { + lines.push(Line::from(l.to_string())); + } + } + + self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( + history_cell::new_user_approval_decision(lines), + ))); } ApprovalRequest::ApplyPatch { .. } => { - lines.push(Line::from(format!("patch approval decision: {decision:?}"))); + // No history line for patch approval decisions. } } - if !feedback.trim().is_empty() { - lines.push(Line::from("feedback:")); - for l in feedback.lines() { - lines.push(Line::from(l.to_string())); - } - } - self.app_event_tx.send(AppEvent::InsertHistoryLines(lines)); let op = match &self.approval_request { ApprovalRequest::Exec { id, .. } => Op::ExecApproval { diff --git a/codex-rs/tui/tests/fixtures/ideal-binary-response.txt b/codex-rs/tui/tests/fixtures/ideal-binary-response.txt deleted file mode 100644 index 56ed4a46..00000000 --- a/codex-rs/tui/tests/fixtures/ideal-binary-response.txt +++ /dev/null @@ -1,208 +0,0 @@ -To get started, describe a task or try one of these commands: - - /init - create an AGENTS.md file with instructions for Codex - /status - show current session configuration and token usage - /approvals - choose what Codex can do without approval - /model - choose what model and reasoning effort to use - -codex -I’m going to scan the workspace and Cargo manifests to see build profiles and -dependencies that impact binary size. Then I’ll summarize the main causes. - -_ - ✓ ls -la - └ total 6696 - drwxr-xr-x@ 39 easong staff 1248 Aug 9 08:49 . - drwxr-xr-x@ 29 easong staff 928 Aug 9 08:50 .. - -rw-r--r--@ 1 easong staff 10244 Aug 8 21:13 .DS_Store - -rw-r--r--@ 1 easong staff 212 Aug 4 18:49 .gitignore -… +30 lines - -rw-r--r--@ 1 easong staff 9971 Aug 8 23:07 simpler.md - drwxr-xr-x@ 6 easong staff 192 Aug 4 22:40 target - drwxr-xr-x@ 6 easong staff 192 Aug 7 19:20 tui - -rw-r--r--@ 1 easong staff 8639 Aug 6 23:30 typewriter.md - -rw-r--r--@ 1 easong staff 7673 Aug 6 21:58 wrap-plan.md - ✓ sed -n '1,200p' Cargo.toml - └ [workspace] - members = [ - "ansi-escape", - "apply-patch", - "arg0", -… +38 lines - codegen-units = 1 - - [patch.crates-io] - # ratatui = { path = "../../ratatui" } - ratatui = { git = "https://github.com/nornagon/ratatui", branch = -"nornagon-v0.29.0-patch" } - ✓ for d in ansi-escape apply-patch arg0 cli common core exec execpolicy -file-search linux-sandbox login mcp-client mcp-server mcp-types ollama tui; do -echo "--- $d/Cargo.toml"; sed -n '1,200p' $d/Cargo.toml; echo; done - └ --- ansi-escape/Cargo.toml - [package] - edition = "2024" - name = "codex-ansi-escape" - version = { workspace = true } -… +237 lines - "process", - "rt-multi-thread", - "signal", - ] } - tracing = { version - ✓ sed -n '1,200p' execpolicy/Cargo.toml - └ [package] - name = "codex-execpolicy" - version = { workspace = true } - edition = "2024" - -… +23 lines - serde_json = "1.0.142" - serde_with = { version = "3", features = ["macros"] } - - [dev-dependencies] - tempfile = "3.13.0" - ✓ sed -n '1,200p' file-search/Cargo.toml - └ [package] - edition = "2024" - name = "codex-file-search" - version = { workspace = true } - -… +11 lines - ignore = "0.4.23" - nucleo-matcher = "0.3.1" - serde = { version = "1", features = ["derive"] } - serde_json = "1.0.142" - tokio = { version = "1", features = ["full"] } - ✓ sed -n '1,200p' linux-sandbox/Cargo.toml - └ [package] - edition = "2024" - name = "codex-linux-sandbox" - version = { workspace = true } - -… +24 lines - "macros", - "process", - "rt-multi-thread", - "signal", - ] } - ✓ sed -n '1,200p' login/Cargo.toml - └ [package] - edition = "2024" - name = "codex-login" - version = { workspace = true } - -… +17 lines - ] } - - [dev-dependencies] - pretty_assertions = "1.4.1" - tempfile = "3" - ✓ sed -n '1,200p' mcp-client/Cargo.toml - └ [package] - name = "codex-mcp-client" - version = { workspace = true } - edition = "2024" - -… +13 lines - "process", - "rt-multi-thread", - "sync", - "time", - ] } - ✓ sed -n '1,200p' mcp-server/Cargo.toml - └ [package] - edition = "2024" - name = "codex-mcp-server" - version = { workspace = true } - -… +35 lines - mcp_test_support = { path = "tests/common" } - pretty_assertions = "1.4.1" - tempfile = "3" - tokio-test = "0.4" - wiremock = "0.6" - ✓ sed -n '1,200p' mcp-types/Cargo.toml - └ [package] - edition = "2024" - name = "mcp-types" - version = { workspace = true } - -… +1 lines - workspace = true - - [dependencies] - serde = { version = "1", features = ["derive"] } - serde_json = "1" - ✓ sed -n '1,200p' ollama/Cargo.toml - └ [package] - edition = "2024" - name = "codex-ollama" - version = { workspace = true } - -… +22 lines - tracing = { version = "0.1.41", features = ["log"] } - wiremock = "0.6" - - [dev-dependencies] - tempfile = "3" - ✓ sed -n '1,200p' tui/Cargo.toml - └ [package] - edition = "2024" - name = "codex-tui" - version = { workspace = true } - -… +74 lines - chrono = { version = "0.4", features = ["serde"] } - insta = "1.43.1" - pretty_assertions = "1" - rand = "0.8" - vt100 = "0.16.2" - ✓ sed -n '1,200p' chatgpt/Cargo.toml - └ [package] - edition = "2024" - name = "codex-chatgpt" - version = { workspace = true } - -… +11 lines - serde_json = "1" - tokio = { version = "1", features = ["full"] } - - [dev-dependencies] - tempfile = "3" - -codex -Here’s what’s driving size in this workspace’s binaries. - -Main Causes - -- Static linking style: Each bin (codex, codex-tui, codex-exec, -codex-mcp-server, etc.) statically links its full dependency graph, so common -code isn’t shared at runtime across executables. -- Heavy deps (HTTP/TLS): reqwest brings in Hyper, HTTP/2, compressors, and a TLS -stack (rustls by default; OpenSSL on musl). In core, login, tui, and ollama you -enable reqwest with json/stream, which still pulls a large transitive set. -- Image/terminal stack: codex-tui includes image (with jpeg), ratatui, -crossterm, and ratatui-image, which together contribute significant code size -(decoders, pixel ops, terminal backends). -- Parsers/VMs: tree-sitter + tree-sitter-bash (in core and apply-patch) and -starlark (in execpolicy) include sizeable parser/VM tables and runtimes. -- Tokio runtime: Broad tokio features (rt-multi-thread, macros, process, signal) -across many crates inflate code size even if only subsets are used per bin. -- Panic + backtraces: Default panic = unwind and backtrace support keep -unwinding tables and symbols that add weight. -- Per-target OpenSSL (musl): For *-unknown-linux-musl, core enables openssl-sys -with vendored, compiling OpenSSL into the binary—this adds multiple megabytes -per executable. - -Build-Mode Notes - -- Release settings: You use lto = "fat" and codegen-units = 1 (good for size), -but strip = "symbols" keeps debuginfo. Debuginfo is often the largest single -contributor; if you build in release with that setting, binaries can still be -large. -- Debug builds: cargo build (dev profile) includes full debuginfo, no LTO, and -assertions—outputs are much larger than cargo build --release. - -If you want, I can outline targeted trims (e.g., strip = "debuginfo", opt-level -= "z", panic abort, tighter tokio/reqwest features) and estimate impact per -binary. \ No newline at end of file