openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-06 20:20:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge 0728d23c51 into sapling-pr-archive-bolinfest

Browse Source
This commit is contained in:
Michael Bolin
2026-01-10 08:25:45 -08:00
committed by GitHub
parent 513a0c8d36 0728d23c51
commit 37dca075cc
20 changed files with 958 additions and 703 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex.rs
+18
View File
@@ -1540,6 +1540,24 @@ impl Session {
}
}
/// Returns the input if there was no task running to inject into
pub async fn inject_response_items(
&self,
input: Vec<ResponseInputItem>,
) -> Result<(), Vec<ResponseInputItem>> {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
Some(at) => {
let mut ts = at.turn_state.lock().await;
for item in input {
ts.push_pending_input(item);
}
Ok(())
}
None => Err(input),
}
}
pub async fn get_pending_input(&self) -> Vec<ResponseInputItem> {
let mut active = self.active_turn.lock().await;
match active.as_mut() {
codex-rs/core/src/event_mapping.rs
+87 -1
View File
@@ -9,6 +9,10 @@ use codex_protocol::models::ReasoningItemContent;
use codex_protocol::models::ReasoningItemReasoningSummary;
use codex_protocol::models::ResponseItem;
use codex_protocol::models::WebSearchAction;
use codex_protocol::models::is_image_close_tag_text;
use codex_protocol::models::is_image_open_tag_text;
use codex_protocol::models::is_local_image_close_tag_text;
use codex_protocol::models::is_local_image_open_tag_text;
use codex_protocol::user_input::UserInput;
use tracing::warn;
use uuid::Uuid;
@@ -32,9 +36,17 @@ fn parse_user_message(message: &[ContentItem]) -> Option<UserMessageItem> {
let mut content: Vec<UserInput> = Vec::new();
for content_item in message.iter() {
for (idx, content_item) in message.iter().enumerate() {
match content_item {
ContentItem::InputText { text } => {
if (is_local_image_open_tag_text(text) || is_image_open_tag_text(text))
&& (matches!(message.get(idx + 1), Some(ContentItem::InputImage { .. })))
|| (idx > 0
&& (is_local_image_close_tag_text(text) || is_image_close_tag_text(text))
&& matches!(message.get(idx - 1), Some(ContentItem::InputImage { .. })))
{
continue;
}
if is_session_prefix(text) || is_user_shell_command_text(text) {
return None;
}
@@ -177,6 +189,80 @@ mod tests {
}
}
#[test]
fn skips_local_image_label_text() {
let image_url = "data:image/png;base64,abc".to_string();
let label = codex_protocol::models::local_image_open_tag_text(1);
let user_text = "Please review this image.".to_string();
let item = ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![
ContentItem::InputText { text: label },
ContentItem::InputImage {
image_url: image_url.clone(),
},
ContentItem::InputText {
text: "</image>".to_string(),
},
ContentItem::InputText {
text: user_text.clone(),
},
],
};
let turn_item = parse_turn_item(&item).expect("expected user message turn item");
match turn_item {
TurnItem::UserMessage(user) => {
let expected_content = vec![
UserInput::Image { image_url },
UserInput::Text { text: user_text },
];
assert_eq!(user.content, expected_content);
}
other => panic!("expected TurnItem::UserMessage, got {other:?}"),
}
}
#[test]
fn skips_unnamed_image_label_text() {
let image_url = "data:image/png;base64,abc".to_string();
let label = codex_protocol::models::image_open_tag_text();
let user_text = "Please review this image.".to_string();
let item = ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![
ContentItem::InputText { text: label },
ContentItem::InputImage {
image_url: image_url.clone(),
},
ContentItem::InputText {
text: codex_protocol::models::image_close_tag_text(),
},
ContentItem::InputText {
text: user_text.clone(),
},
],
};
let turn_item = parse_turn_item(&item).expect("expected user message turn item");
match turn_item {
TurnItem::UserMessage(user) => {
let expected_content = vec![
UserInput::Image { image_url },
UserInput::Text { text: user_text },
];
assert_eq!(user.content, expected_content);
}
other => panic!("expected TurnItem::UserMessage, got {other:?}"),
}
}
#[test]
fn skips_user_instructions_and_env() {
let items = vec![
codex-rs/core/src/tools/handlers/view_image.rs
+11 -2
View File
@@ -11,7 +11,9 @@ use crate::tools::context::ToolPayload;
use crate::tools::handlers::parse_arguments;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
use codex_protocol::user_input::UserInput;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::local_image_content_items_with_label_number;
pub struct ViewImageHandler;
@@ -63,8 +65,15 @@ impl ToolHandler for ViewImageHandler {
}
let event_path = abs_path.clone();
let content: Vec<ContentItem> =
local_image_content_items_with_label_number(&abs_path, None);
let input = ResponseInputItem::Message {
role: "user".to_string(),
content,
};
session
.inject_input(vec![UserInput::LocalImage { path: abs_path }])
.inject_response_items(vec![input])
.await
.map_err(|_| {
FunctionCallError::RespondToModel(
codex-rs/core/src/tools/spec.rs
+4 -4
View File
@@ -8,6 +8,7 @@ use crate::tools::handlers::apply_patch::create_apply_patch_json_tool;
use crate::tools::handlers::collab::DEFAULT_WAIT_TIMEOUT_MS;
use crate::tools::handlers::collab::MAX_WAIT_TIMEOUT_MS;
use crate::tools::registry::ToolRegistryBuilder;
use codex_protocol::models::VIEW_IMAGE_TOOL_NAME;
use codex_protocol::openai_models::ApplyPatchToolType;
use codex_protocol::openai_models::ConfigShellToolType;
use codex_protocol::openai_models::ModelInfo;
@@ -412,10 +413,9 @@ fn create_view_image_tool() -> ToolSpec {
)]);
ToolSpec::Function(ResponsesApiTool {
name: "view_image".to_string(),
description:
"Attach a local image (by filesystem path) to the thread context for this turn."
.to_string(),
name: VIEW_IMAGE_TOOL_NAME.to_string(),
description: "View a local image from the filesystem (only use if given a full filepath by the user, and the image isn't already attached to the thread context within <image ...> tags)."
.to_string(),
strict: false,
parameters: JsonSchema::Object {
properties,
codex-rs/core/tests/common/responses.rs
+13 -7
View File
@@ -509,12 +509,17 @@ pub fn ev_apply_patch_function_call(call_id: &str, patch: &str) -> Value {
}
pub fn ev_shell_command_call(call_id: &str, command: &str) -> Value {
let args = serde_json::json!({ "command": command });
let args = serde_json::json!({ "command": command, "login": false });
ev_shell_command_call_with_args(call_id, &args)
}
pub fn ev_shell_command_call_with_args(call_id: &str, args: &serde_json::Value) -> Value {
let arguments = serde_json::to_string(args).expect("serialize shell command arguments");
let mut args = args.clone();
if let serde_json::Value::Object(map) = &mut args {
map.entry("login".to_string())
.or_insert_with(|| serde_json::Value::Bool(false));
}
let arguments = serde_json::to_string(&args).expect("serialize shell command arguments");
ev_function_call(call_id, "shell_command", &arguments)
}
@@ -527,17 +532,18 @@ pub fn ev_apply_patch_shell_call(call_id: &str, patch: &str) -> Value {
pub fn ev_apply_patch_shell_call_via_heredoc(call_id: &str, patch: &str) -> Value {
let script = format!("apply_patch <<'EOF'\n{patch}\nEOF\n");
let args = serde_json::json!({ "command": ["bash", "-lc", script] });
let args = serde_json::json!({ "command": ["bash", "-c", script] });
let arguments = serde_json::to_string(&args).expect("serialize apply_patch arguments");
ev_function_call(call_id, "shell", &arguments)
}
pub fn ev_apply_patch_shell_command_call_via_heredoc(call_id: &str, patch: &str) -> Value {
let args = serde_json::json!({ "command": format!("apply_patch <<'EOF'\n{patch}\nEOF\n") });
let arguments = serde_json::to_string(&args).expect("serialize apply_patch arguments");
ev_function_call(call_id, "shell_command", &arguments)
let args = serde_json::json!({
"command": format!("apply_patch <<'EOF'\n{patch}\nEOF\n"),
"login": false,
});
ev_shell_command_call_with_args(call_id, &args)
}
pub fn sse_failed(id: &str, code: &str, message: &str) -> String {
codex-rs/core/tests/suite/approvals.rs
+51 -4
View File
@@ -30,9 +30,11 @@ use pretty_assertions::assert_eq;
use regex_lite::Regex;
use serde_json::Value;
use serde_json::json;
use std::env;
use std::fs;
use std::path::Path;
use std::path::PathBuf;
use std::sync::OnceLock;
use tempfile::Builder;
use wiremock::Mock;
use wiremock::MockServer;
use wiremock::ResponseTemplate;
@@ -45,6 +47,52 @@ enum TargetPath {
OutsideWorkspace(&'static str),
}
fn candidate_is_outside_tmp(candidate: &Path) -> bool {
let temp_dir = std::env::temp_dir();
if candidate.starts_with(&temp_dir) {
return false;
}
if cfg!(unix) && candidate.starts_with(Path::new("/tmp")) {
return false;
}
if let Some(tmpdir) = std::env::var_os("TMPDIR") {
let tmpdir = PathBuf::from(tmpdir);
if candidate.starts_with(&tmpdir) {
return false;
}
}
true
}
fn outside_workspace_root() -> &'static PathBuf {
static OUTSIDE_ROOT: OnceLock<PathBuf> = OnceLock::new();
OUTSIDE_ROOT.get_or_init(|| {
let mut candidates = Vec::new();
if cfg!(unix) {
candidates.push(PathBuf::from("/var/tmp"));
}
if let Some(home) = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")) {
candidates.push(PathBuf::from(home));
}
for candidate in candidates {
if !candidate.is_dir() || !candidate_is_outside_tmp(&candidate) {
continue;
}
if let Ok(dir) = Builder::new()
.prefix("codex-outside-")
.tempdir_in(&candidate)
{
return dir.keep();
}
}
Builder::new()
.prefix("codex-outside-")
.tempdir()
.expect("create outside workspace temp dir")
.keep()
})
}
impl TargetPath {
fn resolve_for_patch(self, test: &TestCodex) -> (PathBuf, String) {
match self {
@@ -53,9 +101,7 @@ impl TargetPath {
(path, name.to_string())
}
TargetPath::OutsideWorkspace(name) => {
let path = env::current_dir()
.expect("current dir should be available")
.join(name);
let path = outside_workspace_root().join(name);
(path.clone(), path.display().to_string())
}
}
@@ -187,6 +233,7 @@ fn shell_event(
) -> Result<Value> {
let mut args = json!({
"command": command,
"login": false,
"timeout_ms": timeout_ms,
});
if sandbox_permissions.requires_escalated_permissions() {
codex-rs/core/tests/suite/shell_serialization.rs
+1
View File
@@ -49,6 +49,7 @@ fn shell_responses(
let command = shlex::try_join(command)?;
let parameters = json!({
"command": command,
"login": false,
"timeout_ms": 2_000,
});
Ok(vec![
codex-rs/core/tests/suite/unified_exec.rs
+93 -84
View File
@@ -46,6 +46,15 @@ fn extract_output_text(item: &Value) -> Option<&str> {
})
}
fn exec_command_args(mut args: Value) -> Value {
if let Value::Object(map) = &mut args {
if map.contains_key("cmd") {
map.entry("login".to_string()).or_insert(Value::Bool(false));
}
}
args
}
#[derive(Debug)]
struct ParsedUnifiedExecOutput {
chunk_id: Option<String>,
@@ -171,10 +180,10 @@ async fn unified_exec_intercepts_apply_patch_exec_command() -> Result<()> {
"*** Begin Patch\n*** Add File: uexec_apply.txt\n+hello from unified exec\n*** End Patch";
let command = format!("apply_patch <<'EOF'\n{patch}\nEOF\n");
let call_id = "uexec-apply-patch";
let args = json!({
let args = exec_command_args(json!({
"cmd": command,
"yield_time_ms": 250,
});
}));
let responses = vec![
sse(vec![
@@ -299,11 +308,11 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
} = builder.build(&server).await?;
let call_id = "uexec-begin-event";
let args = json!({
let args = exec_command_args(json!({
"shell": "bash".to_string(),
"cmd": "/bin/echo hello unified exec".to_string(),
"yield_time_ms": 250,
});
}));
let responses = vec![
sse(vec![
@@ -342,7 +351,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
})
.await;
assert_command(&begin_event.command, "-lc", "/bin/echo hello unified exec");
assert_command(&begin_event.command, "-c", "/bin/echo hello unified exec");
assert_eq!(begin_event.cwd, cwd.path());
@@ -374,11 +383,11 @@ async fn unified_exec_resolves_relative_workdir() -> Result<()> {
std::fs::create_dir_all(cwd.path().join(&workdir_rel))?;
let call_id = "uexec-workdir-relative";
let args = json!({
let args = exec_command_args(json!({
"cmd": "pwd",
"yield_time_ms": 250,
"workdir": workdir_rel.to_string_lossy().to_string(),
});
}));
let responses = vec![
sse(vec![
@@ -452,11 +461,11 @@ async fn unified_exec_respects_workdir_override() -> Result<()> {
std::fs::create_dir_all(&workdir)?;
let call_id = "uexec-workdir";
let args = json!({
let args = exec_command_args(json!({
"cmd": "pwd",
"yield_time_ms": 250,
"workdir": workdir.to_string_lossy().to_string(),
});
}));
let responses = vec![
sse(vec![
@@ -528,16 +537,16 @@ async fn unified_exec_emits_exec_command_end_event() -> Result<()> {
} = builder.build(&server).await?;
let call_id = "uexec-end-event";
let args = json!({
let args = exec_command_args(json!({
"cmd": "/bin/echo END-EVENT".to_string(),
"yield_time_ms": 250,
});
}));
let poll_call_id = "uexec-end-event-poll";
let poll_args = json!({
let poll_args = exec_command_args(json!({
"chars": "",
"session_id": 1000,
"yield_time_ms": 250,
});
}));
let responses = vec![
sse(vec![
@@ -615,10 +624,10 @@ async fn unified_exec_emits_output_delta_for_exec_command() -> Result<()> {
} = builder.build(&server).await?;
let call_id = "uexec-delta-1";
let args = json!({
let args = exec_command_args(json!({
"cmd": "printf 'HELLO-UEXEC'",
"yield_time_ms": 1000,
});
}));
let responses = vec![
sse(vec![
@@ -688,10 +697,10 @@ async fn unified_exec_full_lifecycle_with_background_end_event() -> Result<()> {
let call_id = "uexec-full-lifecycle";
// This timing force the long-standing PTY
let args = json!({
let args = exec_command_args(json!({
"cmd": "sleep 0.5; printf 'HELLO-FULL-LIFECYCLE'",
"yield_time_ms": 1000,
});
}));
let responses = vec![
sse(vec![
@@ -794,17 +803,17 @@ async fn unified_exec_emits_terminal_interaction_for_write_stdin() -> Result<()>
} = builder.build(&server).await?;
let open_call_id = "uexec-open";
let open_args = json!({
let open_args = exec_command_args(json!({
"cmd": "/bin/bash -i",
"yield_time_ms": 200,
});
}));
let stdin_call_id = "uexec-stdin-delta";
let stdin_args = json!({
let stdin_args = exec_command_args(json!({
"chars": "echo WSTDIN-MARK\\n",
"session_id": 1000,
"yield_time_ms": 800,
});
}));
let responses = vec![
sse(vec![
@@ -893,33 +902,33 @@ async fn unified_exec_terminal_interaction_captures_delayed_output() -> Result<(
} = builder.build(&server).await?;
let open_call_id = "uexec-delayed-open";
let open_args = json!({
let open_args = exec_command_args(json!({
"cmd": "sleep 3 && echo MARKER1 && sleep 3 && echo MARKER2",
"yield_time_ms": 10,
});
}));
// Poll stdin three times: first for no output, second after the first marker,
// and a final long poll to capture the second marker.
let first_poll_call_id = "uexec-delayed-poll-1";
let first_poll_args = json!({
let first_poll_args = exec_command_args(json!({
"chars": "",
"session_id": 1000,
"yield_time_ms": 10,
});
}));
let second_poll_call_id = "uexec-delayed-poll-2";
let second_poll_args = json!({
let second_poll_args = exec_command_args(json!({
"chars": "",
"session_id": 1000,
"yield_time_ms": 4000,
});
}));
let third_poll_call_id = "uexec-delayed-poll-3";
let third_poll_args = json!({
let third_poll_args = exec_command_args(json!({
"chars": "",
"session_id": 1000,
"yield_time_ms": 6000,
});
}));
let responses = vec![
sse(vec![
@@ -1083,18 +1092,18 @@ async fn unified_exec_emits_one_begin_and_one_end_event() -> Result<()> {
} = builder.build(&server).await?;
let open_call_id = "uexec-open-session";
let open_args = json!({
let open_args = exec_command_args(json!({
"shell": "bash".to_string(),
"cmd": "sleep 0.1".to_string(),
"yield_time_ms": 10,
});
}));
let poll_call_id = "uexec-poll-empty";
let poll_args = json!({
let poll_args = exec_command_args(json!({
"chars": "",
"session_id": 1000,
"yield_time_ms": 150,
});
}));
let responses = vec![
sse(vec![
@@ -1166,7 +1175,7 @@ async fn unified_exec_emits_one_begin_and_one_end_event() -> Result<()> {
let open_event = &begin_events[0];
assert_command(&open_event.command, "-lc", "sleep 0.1");
assert_command(&open_event.command, "-c", "sleep 0.1");
assert!(
open_event.interaction_input.is_none(),
@@ -1199,11 +1208,11 @@ async fn exec_command_reports_chunk_and_exit_metadata() -> Result<()> {
} = builder.build(&server).await?;
let call_id = "uexec-metadata";
let args = serde_json::json!({
let args = exec_command_args(serde_json::json!({
"cmd": "printf 'token one token two token three token four token five token six token seven'",
"yield_time_ms": 500,
"max_output_tokens": 6,
});
}));
let responses = vec![
sse(vec![
@@ -1306,10 +1315,10 @@ async fn unified_exec_respects_early_exit_notifications() -> Result<()> {
} = builder.build(&server).await?;
let call_id = "uexec-early-exit";
let args = serde_json::json!({
let args = exec_command_args(serde_json::json!({
"cmd": "sleep 0.05",
"yield_time_ms": 31415,
});
}));
let responses = vec![
sse(vec![
@@ -1401,20 +1410,20 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> {
let send_call_id = "uexec-cat-send";
let exit_call_id = "uexec-cat-exit";
let start_args = serde_json::json!({
let start_args = exec_command_args(serde_json::json!({
"cmd": "/bin/cat",
"yield_time_ms": 500,
});
let send_args = serde_json::json!({
}));
let send_args = exec_command_args(serde_json::json!({
"chars": "hello unified exec\n",
"session_id": 1000,
"yield_time_ms": 500,
});
let exit_args = serde_json::json!({
}));
let exit_args = exec_command_args(serde_json::json!({
"chars": "\u{0004}",
"session_id": 1000,
"yield_time_ms": 500,
});
}));
let responses = vec![
sse(vec![
@@ -1560,24 +1569,24 @@ async fn unified_exec_emits_end_event_when_session_dies_via_stdin() -> Result<()
} = builder.build(&server).await?;
let start_call_id = "uexec-end-on-exit-start";
let start_args = serde_json::json!({
let start_args = exec_command_args(serde_json::json!({
"cmd": "/bin/cat",
"yield_time_ms": 200,
});
}));
let echo_call_id = "uexec-end-on-exit-echo";
let echo_args = serde_json::json!({
let echo_args = exec_command_args(serde_json::json!({
"chars": "bye-END\n",
"session_id": 1000,
"yield_time_ms": 300,
});
}));
let exit_call_id = "uexec-end-on-exit";
let exit_args = serde_json::json!({
let exit_args = exec_command_args(serde_json::json!({
"chars": "\u{0004}",
"session_id": 1000,
"yield_time_ms": 500,
});
}));
let responses = vec![
sse(vec![
@@ -1670,10 +1679,10 @@ async fn unified_exec_closes_long_running_session_at_turn_end() -> Result<()> {
let call_id = "uexec-long-running";
let command = format!("printf '%s' $$ > '{pid_path_str}' && exec sleep 3000");
let args = json!({
let args = exec_command_args(json!({
"cmd": command,
"yield_time_ms": 250,
});
}));
let responses = vec![
sse(vec![
@@ -1769,17 +1778,17 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> {
} = builder.build(&server).await?;
let first_call_id = "uexec-start";
let first_args = serde_json::json!({
let first_args = exec_command_args(serde_json::json!({
"cmd": "/bin/cat",
"yield_time_ms": 200,
});
}));
let second_call_id = "uexec-stdin";
let second_args = serde_json::json!({
let second_args = exec_command_args(serde_json::json!({
"chars": "hello unified exec\n",
"session_id": 1000,
"yield_time_ms": 500,
});
}));
let responses = vec![
sse(vec![
@@ -1900,17 +1909,17 @@ PY
"#;
let first_call_id = "uexec-lag-start";
let first_args = serde_json::json!({
let first_args = exec_command_args(serde_json::json!({
"cmd": script,
"yield_time_ms": 25,
});
}));
let second_call_id = "uexec-lag-poll";
let second_args = serde_json::json!({
let second_args = exec_command_args(serde_json::json!({
"chars": "",
"session_id": 1000,
"yield_time_ms": 2_000,
});
}));
let responses = vec![
sse(vec![
@@ -2011,17 +2020,17 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> {
} = builder.build(&server).await?;
let first_call_id = "uexec-timeout";
let first_args = serde_json::json!({
let first_args = exec_command_args(serde_json::json!({
"cmd": "sleep 0.5; echo ready",
"yield_time_ms": 10,
});
}));
let second_call_id = "uexec-poll";
let second_args = serde_json::json!({
let second_args = exec_command_args(serde_json::json!({
"chars": "",
"session_id": 1000,
"yield_time_ms": 800,
});
}));
let responses = vec![
sse(vec![
@@ -2123,11 +2132,11 @@ PY
"#;
let call_id = "uexec-large-output";
let args = serde_json::json!({
let args = exec_command_args(serde_json::json!({
"cmd": script,
"max_output_tokens": 100,
"yield_time_ms": 500,
});
}));
let responses = vec![
sse(vec![
@@ -2202,10 +2211,10 @@ async fn unified_exec_runs_under_sandbox() -> Result<()> {
} = builder.build(&server).await?;
let call_id = "uexec";
let args = serde_json::json!({
let args = exec_command_args(serde_json::json!({
"cmd": "echo 'hello'",
"yield_time_ms": 500,
});
}));
let responses = vec![
sse(vec![
@@ -2282,17 +2291,17 @@ async fn unified_exec_python_prompt_under_seatbelt() -> Result<()> {
} = builder.build(&server).await?;
let startup_call_id = "uexec-python-seatbelt";
let startup_args = serde_json::json!({
let startup_args = exec_command_args(serde_json::json!({
"cmd": format!("{} -i", python.display()),
"yield_time_ms": 1_500,
});
}));
let exit_call_id = "uexec-python-exit";
let exit_args = serde_json::json!({
let exit_args = exec_command_args(serde_json::json!({
"chars": "exit()\n",
"session_id": 1000,
"yield_time_ms": 1_500,
});
}));
let responses = vec![
sse(vec![
@@ -2396,9 +2405,9 @@ async fn unified_exec_runs_on_all_platforms() -> Result<()> {
} = builder.build(&server).await?;
let call_id = "uexec";
let args = serde_json::json!({
let args = exec_command_args(serde_json::json!({
"cmd": "echo 'hello crossplat'",
});
}));
let responses = vec![
sse(vec![
@@ -2472,17 +2481,17 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
const FILLER_SESSIONS: i32 = MAX_SESSIONS_FOR_TEST - 1;
let keep_call_id = "uexec-prune-keep";
let keep_args = serde_json::json!({
let keep_args = exec_command_args(serde_json::json!({
"cmd": "/bin/cat",
"yield_time_ms": 250,
});
}));
let prune_call_id = "uexec-prune-target";
// Give the sleeper time to exit before the filler sessions trigger pruning.
let prune_args = serde_json::json!({
let prune_args = exec_command_args(serde_json::json!({
"cmd": "sleep 1",
"yield_time_ms": 1_250,
});
}));
let mut events = vec![ev_response_created("resp-prune-1")];
events.push(ev_function_call(
@@ -2497,10 +2506,10 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
));
for idx in 0..FILLER_SESSIONS {
let filler_args = serde_json::json!({
let filler_args = exec_command_args(serde_json::json!({
"cmd": format!("echo filler {idx}"),
"yield_time_ms": 250,
});
}));
let call_id = format!("uexec-prune-fill-{idx}");
events.push(ev_function_call(
&call_id,
@@ -2510,11 +2519,11 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
}
let keep_write_call_id = "uexec-prune-keep-write";
let keep_write_args = serde_json::json!({
let keep_write_args = exec_command_args(serde_json::json!({
"chars": "still alive\n",
"session_id": 1000,
"yield_time_ms": 500,
});
}));
events.push(ev_function_call(
keep_write_call_id,
"write_stdin",
@@ -2522,11 +2531,11 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
));
let probe_call_id = "uexec-prune-probe";
let probe_args = serde_json::json!({
let probe_args = exec_command_args(serde_json::json!({
"chars": "should fail\n",
"session_id": 1001,
"yield_time_ms": 500,
});
}));
events.push(ev_function_call(
probe_call_id,
"write_stdin",
codex-rs/core/tests/suite/view_image.rs
+14
View File
@@ -216,6 +216,20 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> {
let image_message =
find_image_message(&body).expect("pending input image message not included in request");
let content_items = image_message
.get("content")
.and_then(Value::as_array)
.expect("image message has content array");
assert_eq!(
content_items.len(),
1,
"view_image should inject only the image content item (no tag/label text)"
);
assert_eq!(
content_items[0].get("type").and_then(Value::as_str),
Some("input_image"),
"view_image should inject only an input_image content item"
);
let image_url = image_message
.get("content")
.and_then(Value::as_array)
codex-rs/protocol/src/models.rs
+133 -31
View File
@@ -180,6 +180,48 @@ fn local_image_error_placeholder(
}
}
pub const VIEW_IMAGE_TOOL_NAME: &str = "view_image";
const IMAGE_OPEN_TAG: &str = "<image>";
const IMAGE_CLOSE_TAG: &str = "</image>";
const LOCAL_IMAGE_OPEN_TAG_PREFIX: &str = "<image name=";
const LOCAL_IMAGE_OPEN_TAG_SUFFIX: &str = ">";
const LOCAL_IMAGE_CLOSE_TAG: &str = IMAGE_CLOSE_TAG;
pub fn image_open_tag_text() -> String {
IMAGE_OPEN_TAG.to_string()
}
pub fn image_close_tag_text() -> String {
IMAGE_CLOSE_TAG.to_string()
}
pub fn local_image_label_text(label_number: usize) -> String {
format!("[Image #{label_number}]")
}
pub fn local_image_open_tag_text(label_number: usize) -> String {
let label = local_image_label_text(label_number);
format!("{LOCAL_IMAGE_OPEN_TAG_PREFIX}{label}{LOCAL_IMAGE_OPEN_TAG_SUFFIX}")
}
pub fn is_local_image_open_tag_text(text: &str) -> bool {
text.strip_prefix(LOCAL_IMAGE_OPEN_TAG_PREFIX)
.is_some_and(|rest| rest.ends_with(LOCAL_IMAGE_OPEN_TAG_SUFFIX))
}
pub fn is_local_image_close_tag_text(text: &str) -> bool {
is_image_close_tag_text(text)
}
pub fn is_image_open_tag_text(text: &str) -> bool {
text == IMAGE_OPEN_TAG
}
pub fn is_image_close_tag_text(text: &str) -> bool {
text == IMAGE_CLOSE_TAG
}
fn invalid_image_error_placeholder(
path: &std::path::Path,
error: impl std::fmt::Display,
@@ -203,6 +245,53 @@ fn unsupported_image_error_placeholder(path: &std::path::Path, mime: &str) -> Co
}
}
pub fn local_image_content_items_with_label_number(
path: &std::path::Path,
label_number: Option<usize>,
) -> Vec<ContentItem> {
match load_and_resize_to_fit(path) {
Ok(image) => {
let mut items = Vec::with_capacity(3);
if let Some(label_number) = label_number {
items.push(ContentItem::InputText {
text: local_image_open_tag_text(label_number),
});
}
items.push(ContentItem::InputImage {
image_url: image.into_data_url(),
});
if label_number.is_some() {
items.push(ContentItem::InputText {
text: LOCAL_IMAGE_CLOSE_TAG.to_string(),
});
}
items
}
Err(err) => {
if matches!(&err, ImageProcessingError::Read { .. }) {
vec![local_image_error_placeholder(path, &err)]
} else if err.is_invalid_image() {
vec![invalid_image_error_placeholder(path, &err)]
} else {
let Some(mime_guess) = mime_guess::from_path(path).first() else {
return vec![local_image_error_placeholder(
path,
"unsupported MIME type (unknown)",
)];
};
let mime = mime_guess.essence_str().to_owned();
if !mime.starts_with("image/") {
return vec![local_image_error_placeholder(
path,
format!("unsupported MIME type `{mime}`"),
)];
}
vec![unsupported_image_error_placeholder(path, &mime)]
}
}
}
}
impl From<ResponseInputItem> for ResponseItem {
fn from(item: ResponseInputItem) -> Self {
match item {
@@ -296,41 +385,27 @@ pub enum ReasoningItemContent {
impl From<Vec<UserInput>> for ResponseInputItem {
fn from(items: Vec<UserInput>) -> Self {
let mut image_index = 0;
Self::Message {
role: "user".to_string(),
content: items
.into_iter()
.filter_map(|c| match c {
UserInput::Text { text } => Some(ContentItem::InputText { text }),
UserInput::Image { image_url } => Some(ContentItem::InputImage { image_url }),
UserInput::LocalImage { path } => match load_and_resize_to_fit(&path) {
Ok(image) => Some(ContentItem::InputImage {
image_url: image.into_data_url(),
}),
Err(err) => {
if matches!(&err, ImageProcessingError::Read { .. }) {
Some(local_image_error_placeholder(&path, &err))
} else if err.is_invalid_image() {
Some(invalid_image_error_placeholder(&path, &err))
} else {
let Some(mime_guess) = mime_guess::from_path(&path).first() else {
return Some(local_image_error_placeholder(
&path,
"unsupported MIME type (unknown)",
));
};
let mime = mime_guess.essence_str().to_owned();
if !mime.starts_with("image/") {
return Some(local_image_error_placeholder(
&path,
format!("unsupported MIME type `{mime}`"),
));
}
Some(unsupported_image_error_placeholder(&path, &mime))
}
}
},
UserInput::Skill { .. } => None, // Skill bodies are injected later in core
.flat_map(|c| match c {
UserInput::Text { text } => vec![ContentItem::InputText { text }],
UserInput::Image { image_url } => vec![
ContentItem::InputText {
text: image_open_tag_text(),
},
ContentItem::InputImage { image_url },
ContentItem::InputText {
text: image_close_tag_text(),
},
],
UserInput::LocalImage { path } => {
image_index += 1;
local_image_content_items_with_label_number(&path, Some(image_index))
}
UserInput::Skill { .. } => Vec::new(), // Skill bodies are injected later in core
})
.collect::<Vec<ContentItem>>(),
}
@@ -770,6 +845,33 @@ mod tests {
Ok(())
}
#[test]
fn wraps_image_user_input_with_tags() -> Result<()> {
let image_url = "data:image/png;base64,abc".to_string();
let item = ResponseInputItem::from(vec![UserInput::Image {
image_url: image_url.clone(),
}]);
match item {
ResponseInputItem::Message { content, .. } => {
let expected = vec![
ContentItem::InputText {
text: image_open_tag_text(),
},
ContentItem::InputImage { image_url },
ContentItem::InputText {
text: image_close_tag_text(),
},
];
assert_eq!(content, expected);
}
other => panic!("expected message response but got {other:?}"),
}
Ok(())
}
#[test]
fn local_image_read_error_adds_placeholder() -> Result<()> {
let dir = tempdir()?;
codex-rs/tui/src/bottom_pane/chat_composer.rs
+176 -270
View File
@@ -46,6 +46,7 @@ use crate::style::user_message_style;
use codex_common::fuzzy_match::fuzzy_match;
use codex_protocol::custom_prompts::CustomPrompt;
use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX;
use codex_protocol::models::local_image_label_text;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
@@ -59,7 +60,7 @@ use codex_core::skills::model::SkillMetadata;
use codex_file_search::FileMatch;
use std::cell::RefCell;
use std::collections::HashMap;
use std::path::Path;
use std::collections::HashSet;
use std::path::PathBuf;
use std::time::Duration;
use std::time::Instant;
@@ -274,10 +275,12 @@ impl ChatComposer {
// normalize_pasted_path already handles Windows → WSL path conversion,
// so we can directly try to read the image dimensions.
match image::image_dimensions(&path_buf) {
Ok((w, h)) => {
Ok((width, height)) => {
tracing::info!("OK: {pasted}");
let format_label = pasted_image_format(&path_buf).label();
self.attach_image(path_buf, w, h, format_label);
tracing::debug!("image dimensions={}x{}", width, height);
let format = pasted_image_format(&path_buf);
tracing::debug!("attached image format={}", format.label());
self.attach_image(path_buf);
true
}
Err(err) => {
@@ -408,13 +411,9 @@ impl ChatComposer {
}
/// Attempt to start a burst by retro-capturing recent chars before the cursor.
pub fn attach_image(&mut self, path: PathBuf, width: u32, height: u32, _format_label: &str) {
let file_label = path
.file_name()
.map(|name| name.to_string_lossy().into_owned())
.unwrap_or_else(|| "image".to_string());
let base_placeholder = format!("{file_label} {width}x{height}");
let placeholder = self.next_image_placeholder(&base_placeholder);
pub fn attach_image(&mut self, path: PathBuf) {
let image_number = self.attached_images.len() + 1;
let placeholder = local_image_label_text(image_number);
// Insert as an element to match large paste placeholder behavior:
// styled distinctly and treated atomically for cursor/mutations.
self.textarea.insert_element(&placeholder);
@@ -477,22 +476,6 @@ impl ChatComposer {
}
}
fn next_image_placeholder(&mut self, base: &str) -> String {
let text = self.textarea.text();
let mut suffix = 1;
loop {
let placeholder = if suffix == 1 {
format!("[{base}]")
} else {
format!("[{base} #{suffix}]")
};
if !text.contains(&placeholder) {
return placeholder;
}
suffix += 1;
}
}
pub(crate) fn insert_str(&mut self, text: &str) {
self.textarea.insert_str(text);
self.sync_popups();
@@ -818,47 +801,43 @@ impl ChatComposer {
if is_image {
// Determine dimensions; if that fails fall back to normal path insertion.
let path_buf = PathBuf::from(&sel_path);
if let Ok((w, h)) = image::image_dimensions(&path_buf) {
// Remove the current @token (mirror logic from insert_selected_path without inserting text)
// using the flat text and byte-offset cursor API.
let cursor_offset = self.textarea.cursor();
let text = self.textarea.text();
// Clamp to a valid char boundary to avoid panics when slicing.
let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset);
let before_cursor = &text[..safe_cursor];
let after_cursor = &text[safe_cursor..];
match image::image_dimensions(&path_buf) {
Ok((width, height)) => {
tracing::debug!("selected image dimensions={}x{}", width, height);
// Remove the current @token (mirror logic from insert_selected_path without inserting text)
// using the flat text and byte-offset cursor API.
let cursor_offset = self.textarea.cursor();
let text = self.textarea.text();
// Clamp to a valid char boundary to avoid panics when slicing.
let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset);
let before_cursor = &text[..safe_cursor];
let after_cursor = &text[safe_cursor..];
// Determine token boundaries in the full text.
let start_idx = before_cursor
.char_indices()
.rfind(|(_, c)| c.is_whitespace())
.map(|(idx, c)| idx + c.len_utf8())
.unwrap_or(0);
let end_rel_idx = after_cursor
.char_indices()
.find(|(_, c)| c.is_whitespace())
.map(|(idx, _)| idx)
.unwrap_or(after_cursor.len());
let end_idx = safe_cursor + end_rel_idx;
// Determine token boundaries in the full text.
let start_idx = before_cursor
.char_indices()
.rfind(|(_, c)| c.is_whitespace())
.map(|(idx, c)| idx + c.len_utf8())
.unwrap_or(0);
let end_rel_idx = after_cursor
.char_indices()
.find(|(_, c)| c.is_whitespace())
.map(|(idx, _)| idx)
.unwrap_or(after_cursor.len());
let end_idx = safe_cursor + end_rel_idx;
self.textarea.replace_range(start_idx..end_idx, "");
self.textarea.set_cursor(start_idx);
self.textarea.replace_range(start_idx..end_idx, "");
self.textarea.set_cursor(start_idx);
let format_label = match Path::new(&sel_path)
.extension()
.and_then(|e| e.to_str())
.map(str::to_ascii_lowercase)
{
Some(ext) if ext == "png" => "PNG",
Some(ext) if ext == "jpg" || ext == "jpeg" => "JPEG",
_ => "IMG",
};
self.attach_image(path_buf, w, h, format_label);
// Add a trailing space to keep typing fluid.
self.textarea.insert_str(" ");
} else {
// Fallback to plain path insertion if metadata read fails.
self.insert_selected_path(&sel_path);
self.attach_image(path_buf);
// Add a trailing space to keep typing fluid.
self.textarea.insert_str(" ");
}
Err(err) => {
tracing::trace!("image dimensions lookup failed: {err}");
// Fallback to plain path insertion if metadata read fails.
self.insert_selected_path(&sel_path);
}
}
} else {
// Non-image: inserting file path.
@@ -1464,20 +1443,29 @@ impl ChatComposer {
}
}
// For non-char inputs (or after flushing), handle normally.
// Special handling for backspace on placeholders
if let KeyEvent {
code: KeyCode::Backspace,
..
} = input
&& self.try_remove_any_placeholder_at_cursor()
// Backspace at the start of an image placeholder should delete that placeholder (rather
// than deleting content before it). Do this without scanning the full text by consulting
// the textarea's element list.
if matches!(input.code, KeyCode::Backspace)
&& self.try_remove_image_element_at_cursor_start()
{
return (InputResult::None, true);
}
// Normal input handling
// For non-char inputs (or after flushing), handle normally.
// Track element removals so we can drop any corresponding placeholders without scanning
// the full text. (Placeholders are atomic elements; when deleted, the element disappears.)
let elements_before = if self.pending_pastes.is_empty() && self.attached_images.is_empty() {
None
} else {
Some(self.textarea.element_payloads())
};
self.textarea.input(input);
let text_after = self.textarea.text();
if let Some(elements_before) = elements_before {
self.reconcile_deleted_elements(elements_before);
}
// Update paste-burst heuristic for plain Char (no Ctrl/Alt) events.
let crossterm::event::KeyEvent {
@@ -1499,176 +1487,69 @@ impl ChatComposer {
}
}
// Check if any placeholders were removed and remove their corresponding pending pastes
self.pending_pastes
.retain(|(placeholder, _)| text_after.contains(placeholder));
// Keep attached images in proportion to how many matching placeholders exist in the text.
// This handles duplicate placeholders that share the same visible label.
if !self.attached_images.is_empty() {
let mut needed: HashMap<String, usize> = HashMap::new();
for img in &self.attached_images {
needed
.entry(img.placeholder.clone())
.or_insert_with(|| text_after.matches(&img.placeholder).count());
}
let mut used: HashMap<String, usize> = HashMap::new();
let mut kept: Vec<AttachedImage> = Vec::with_capacity(self.attached_images.len());
for img in self.attached_images.drain(..) {
let total_needed = *needed.get(&img.placeholder).unwrap_or(&0);
let used_count = used.entry(img.placeholder.clone()).or_insert(0);
if *used_count < total_needed {
kept.push(img);
*used_count += 1;
}
}
self.attached_images = kept;
}
(InputResult::None, true)
}
/// Attempts to remove an image or paste placeholder if the cursor is at the end of one.
/// Returns true if a placeholder was removed.
fn try_remove_any_placeholder_at_cursor(&mut self) -> bool {
// Clamp the cursor to a valid char boundary to avoid panics when slicing.
let text = self.textarea.text();
let p = Self::clamp_to_char_boundary(text, self.textarea.cursor());
// Try image placeholders first
let mut out: Option<(usize, String)> = None;
// Detect if the cursor is at the end of any image placeholder.
// If duplicates exist, remove the specific occurrence's mapping.
for (i, img) in self.attached_images.iter().enumerate() {
let ph = &img.placeholder;
if p < ph.len() {
continue;
}
let start = p - ph.len();
if text.get(start..p) != Some(ph.as_str()) {
continue;
}
// Count the number of occurrences of `ph` before `start`.
let mut occ_before = 0usize;
let mut search_pos = 0usize;
while search_pos < start {
let segment = match text.get(search_pos..start) {
Some(s) => s,
None => break,
};
if let Some(found) = segment.find(ph) {
occ_before += 1;
search_pos += found + ph.len();
} else {
break;
}
}
// Remove the occ_before-th attached image that shares this placeholder label.
out = if let Some((remove_idx, _)) = self
.attached_images
.iter()
.enumerate()
.filter(|(_, img2)| img2.placeholder == *ph)
.nth(occ_before)
{
Some((remove_idx, ph.clone()))
} else {
Some((i, ph.clone()))
};
break;
}
if let Some((idx, placeholder)) = out {
self.textarea.replace_range(p - placeholder.len()..p, "");
self.attached_images.remove(idx);
return true;
fn try_remove_image_element_at_cursor_start(&mut self) -> bool {
if self.attached_images.is_empty() {
return false;
}
// Also handle when the cursor is at the START of an image placeholder.
// let result = 'out: {
let out: Option<(usize, String)> = 'out: {
for (i, img) in self.attached_images.iter().enumerate() {
let ph = &img.placeholder;
if p + ph.len() > text.len() {
continue;
}
if text.get(p..p + ph.len()) != Some(ph.as_str()) {
continue;
}
// Count occurrences of `ph` before `p`.
let mut occ_before = 0usize;
let mut search_pos = 0usize;
while search_pos < p {
let segment = match text.get(search_pos..p) {
Some(s) => s,
None => break 'out None,
};
if let Some(found) = segment.find(ph) {
occ_before += 1;
search_pos += found + ph.len();
} else {
break 'out None;
}
}
if let Some((remove_idx, _)) = self
.attached_images
.iter()
.enumerate()
.filter(|(_, img2)| img2.placeholder == *ph)
.nth(occ_before)
{
break 'out Some((remove_idx, ph.clone()));
} else {
break 'out Some((i, ph.clone()));
}
}
None
let p = self.textarea.cursor();
let Some(payload) = self.textarea.element_payload_starting_at(p) else {
return false;
};
let Some(idx) = self
.attached_images
.iter()
.position(|img| img.placeholder == payload)
else {
return false;
};
if let Some((idx, placeholder)) = out {
self.textarea.replace_range(p..p + placeholder.len(), "");
self.attached_images.remove(idx);
return true;
self.textarea.replace_range(p..p + payload.len(), "");
self.attached_images.remove(idx);
self.relabel_attached_images_and_update_placeholders();
true
}
fn reconcile_deleted_elements(&mut self, elements_before: Vec<String>) {
let elements_after: HashSet<String> =
self.textarea.element_payloads().into_iter().collect();
let mut removed_any_image = false;
for removed in elements_before
.into_iter()
.filter(|payload| !elements_after.contains(payload))
{
self.pending_pastes.retain(|(ph, _)| ph != &removed);
if let Some(idx) = self
.attached_images
.iter()
.position(|img| img.placeholder == removed)
{
self.attached_images.remove(idx);
removed_any_image = true;
}
}
// Then try pasted-content placeholders
if let Some(placeholder) = self.pending_pastes.iter().find_map(|(ph, _)| {
if p < ph.len() {
return None;
}
let start = p - ph.len();
if text.get(start..p) == Some(ph.as_str()) {
Some(ph.clone())
} else {
None
}
}) {
self.textarea.replace_range(p - placeholder.len()..p, "");
self.pending_pastes.retain(|(ph, _)| ph != &placeholder);
return true;
if removed_any_image {
self.relabel_attached_images_and_update_placeholders();
}
}
// Also handle when the cursor is at the START of a pasted-content placeholder.
if let Some(placeholder) = self.pending_pastes.iter().find_map(|(ph, _)| {
if p + ph.len() > text.len() {
return None;
fn relabel_attached_images_and_update_placeholders(&mut self) {
for idx in 0..self.attached_images.len() {
let expected = local_image_label_text(idx + 1);
let current = self.attached_images[idx].placeholder.clone();
if current == expected {
continue;
}
if text.get(p..p + ph.len()) == Some(ph.as_str()) {
Some(ph.clone())
} else {
None
}
}) {
self.textarea.replace_range(p..p + placeholder.len(), "");
self.pending_pastes.retain(|(ph, _)| ph != &placeholder);
return true;
self.attached_images[idx].placeholder = expected.clone();
let _renamed = self.textarea.replace_element_payload(&current, &expected);
}
false
}
fn handle_shortcut_overlay_key(&mut self, key_event: &KeyEvent) -> bool {
@@ -3477,12 +3358,12 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image1.png");
composer.attach_image(path.clone(), 32, 16, "PNG");
composer.attach_image(path.clone());
composer.handle_paste(" hi".into());
let (result, _) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Submitted(text) => assert_eq!(text, "[image1.png 32x16] hi"),
InputResult::Submitted(text) => assert_eq!(text, "[Image #1] hi"),
_ => panic!("expected Submitted"),
}
let imgs = composer.take_recent_submission_images();
@@ -3501,11 +3382,11 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image2.png");
composer.attach_image(path.clone(), 10, 5, "PNG");
composer.attach_image(path.clone());
let (result, _) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Submitted(text) => assert_eq!(text, "[image2.png 10x5]"),
InputResult::Submitted(text) => assert_eq!(text, "[Image #1]"),
_ => panic!("expected Submitted"),
}
let imgs = composer.take_recent_submission_images();
@@ -3526,21 +3407,15 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image_dup.png");
composer.attach_image(path.clone(), 10, 5, "PNG");
composer.attach_image(path.clone());
composer.handle_paste(" ".into());
composer.attach_image(path, 10, 5, "PNG");
composer.attach_image(path);
let text = composer.textarea.text().to_string();
assert!(text.contains("[image_dup.png 10x5]"));
assert!(text.contains("[image_dup.png 10x5 #2]"));
assert_eq!(
composer.attached_images[0].placeholder,
"[image_dup.png 10x5]"
);
assert_eq!(
composer.attached_images[1].placeholder,
"[image_dup.png 10x5 #2]"
);
assert!(text.contains("[Image #1]"));
assert!(text.contains("[Image #2]"));
assert_eq!(composer.attached_images[0].placeholder, "[Image #1]");
assert_eq!(composer.attached_images[1].placeholder, "[Image #2]");
}
#[test]
@@ -3555,7 +3430,7 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image3.png");
composer.attach_image(path.clone(), 20, 10, "PNG");
composer.attach_image(path.clone());
let placeholder = composer.attached_images[0].placeholder.clone();
// Case 1: backspace at end
@@ -3566,7 +3441,7 @@ mod tests {
// Re-add and test backspace in middle: should break the placeholder string
// and drop the image mapping (same as text placeholder behavior).
composer.attach_image(path, 20, 10, "PNG");
composer.attach_image(path);
let placeholder2 = composer.attached_images[0].placeholder.clone();
// Move cursor to roughly middle of placeholder
if let Some(start_pos) = composer.textarea.text().find(&placeholder2) {
@@ -3598,7 +3473,7 @@ mod tests {
// Insert an image placeholder at the start
let path = PathBuf::from("/tmp/image_multibyte.png");
composer.attach_image(path, 10, 5, "PNG");
composer.attach_image(path);
// Add multibyte text after the placeholder
composer.textarea.insert_str("日本語");
@@ -3607,16 +3482,11 @@ mod tests {
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
assert_eq!(composer.attached_images.len(), 1);
assert!(
composer
.textarea
.text()
.starts_with("[image_multibyte.png 10x5]")
);
assert!(composer.textarea.text().starts_with("[Image #1]"));
}
#[test]
fn deleting_one_of_duplicate_image_placeholders_removes_matching_entry() {
fn deleting_one_of_duplicate_image_placeholders_removes_one_entry() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
@@ -3630,10 +3500,10 @@ mod tests {
let path1 = PathBuf::from("/tmp/image_dup1.png");
let path2 = PathBuf::from("/tmp/image_dup2.png");
composer.attach_image(path1, 10, 5, "PNG");
composer.attach_image(path1);
// separate placeholders with a space for clarity
composer.handle_paste(" ".into());
composer.attach_image(path2.clone(), 10, 5, "PNG");
composer.attach_image(path2.clone());
let placeholder1 = composer.attached_images[0].placeholder.clone();
let placeholder2 = composer.attached_images[1].placeholder.clone();
@@ -3647,25 +3517,66 @@ mod tests {
let new_text = composer.textarea.text().to_string();
assert_eq!(
0,
1,
new_text.matches(&placeholder1).count(),
"first placeholder removed"
"one placeholder remains after deletion"
);
assert_eq!(
0,
new_text.matches(&placeholder2).count(),
"second placeholder was relabeled"
);
assert_eq!(
1,
new_text.matches(&placeholder2).count(),
"second placeholder remains"
new_text.matches("[Image #1]").count(),
"remaining placeholder relabeled to #1"
);
assert_eq!(
vec![AttachedImage {
path: path2,
placeholder: "[image_dup2.png 10x5]".to_string()
placeholder: "[Image #1]".to_string()
}],
composer.attached_images,
"one image mapping remains"
);
}
#[test]
fn deleting_first_text_element_renumbers_following_text_element() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
true,
sender,
false,
"Ask Codex to do anything".to_string(),
false,
);
let path1 = PathBuf::from("/tmp/image_first.png");
let path2 = PathBuf::from("/tmp/image_second.png");
// Insert two adjacent atomic elements.
composer.attach_image(path1);
composer.attach_image(path2.clone());
assert_eq!(composer.textarea.text(), "[Image #1][Image #2]");
assert_eq!(composer.attached_images.len(), 2);
// Delete the first element using normal textarea editing (Delete at cursor start).
composer.textarea.set_cursor(0);
composer.handle_key_event(KeyEvent::new(KeyCode::Delete, KeyModifiers::NONE));
// Remaining image should be renumbered and the textarea element updated.
assert_eq!(composer.attached_images.len(), 1);
assert_eq!(composer.attached_images[0].path, path2);
assert_eq!(composer.attached_images[0].placeholder, "[Image #1]");
assert_eq!(composer.textarea.text(), "[Image #1]");
}
#[test]
fn pasting_filepath_attaches_image() {
let tmp = tempdir().expect("create TempDir");
@@ -3686,12 +3597,7 @@ mod tests {
let needs_redraw = composer.handle_paste(tmp_path.to_string_lossy().to_string());
assert!(needs_redraw);
assert!(
composer
.textarea
.text()
.starts_with("[codex_tui_test_paste_image.png 3x2] ")
);
assert!(composer.textarea.text().starts_with("[Image #1] "));
let imgs = composer.take_recent_submission_images();
assert_eq!(imgs, vec![tmp_path]);
codex-rs/tui/src/bottom_pane/mod.rs
+2 -9
View File
@@ -542,16 +542,9 @@ impl BottomPane {
self.request_redraw();
}
pub(crate) fn attach_image(
&mut self,
path: PathBuf,
width: u32,
height: u32,
format_label: &str,
) {
pub(crate) fn attach_image(&mut self, path: PathBuf) {
if self.view_stack.is_empty() {
self.composer
.attach_image(path, width, height, format_label);
self.composer.attach_image(path);
self.request_redraw();
}
}
codex-rs/tui/src/bottom_pane/textarea.rs
+82
View File
@@ -715,6 +715,88 @@ impl TextArea {
// ===== Text elements support =====
pub fn element_payloads(&self) -> Vec<String> {
self.elements
.iter()
.filter_map(|e| self.text.get(e.range.clone()).map(str::to_string))
.collect()
}
pub fn element_payload_starting_at(&self, pos: usize) -> Option<String> {
let pos = pos.min(self.text.len());
let elem = self.elements.iter().find(|e| e.range.start == pos)?;
self.text.get(elem.range.clone()).map(str::to_string)
}
/// Renames a single text element in-place, keeping it atomic.
///
/// This is intended for cases where the element payload is an identifier (e.g. a placeholder)
/// that must be updated without converting the element back into normal text.
pub fn replace_element_payload(&mut self, old: &str, new: &str) -> bool {
let Some(idx) = self
.elements
.iter()
.position(|e| self.text.get(e.range.clone()) == Some(old))
else {
return false;
};
let range = self.elements[idx].range.clone();
let start = range.start;
let end = range.end;
if start > end || end > self.text.len() {
return false;
}
let removed_len = end - start;
let inserted_len = new.len();
let diff = inserted_len as isize - removed_len as isize;
self.text.replace_range(range, new);
self.wrap_cache.replace(None);
self.preferred_col = None;
// Update the modified element's range.
self.elements[idx].range = start..(start + inserted_len);
// Shift element ranges that occur after the replaced element.
if diff != 0 {
for (j, e) in self.elements.iter_mut().enumerate() {
if j == idx {
continue;
}
if e.range.end <= start {
continue;
}
if e.range.start >= end {
e.range.start = ((e.range.start as isize) + diff) as usize;
e.range.end = ((e.range.end as isize) + diff) as usize;
continue;
}
// Elements should not partially overlap each other; degrade gracefully by
// snapping anything intersecting the replaced range to the new bounds.
e.range.start = start.min(e.range.start);
e.range.end = (start + inserted_len).max(e.range.end.saturating_add_signed(diff));
}
}
// Update the cursor position to account for the edit.
self.cursor_pos = if self.cursor_pos < start {
self.cursor_pos
} else if self.cursor_pos <= end {
start + inserted_len
} else {
((self.cursor_pos as isize) + diff) as usize
};
self.cursor_pos = self.clamp_pos_to_nearest_boundary(self.cursor_pos);
// Keep element ordering deterministic.
self.elements.sort_by_key(|e| e.range.start);
true
}
pub fn insert_element(&mut self, text: &str) {
let start = self.clamp_pos_for_insertion(self.cursor_pos);
self.insert_str_at(start, text);
codex-rs/tui/src/chatwidget.rs
+11 -19
View File
@@ -1593,12 +1593,13 @@ impl ChatWidget {
{
match paste_image_to_temp_png() {
Ok((path, info)) => {
self.attach_image(
path,
tracing::debug!(
"pasted image size={}x{} format={}",
info.width,
info.height,
info.encoded_format.label(),
info.encoded_format.label()
);
self.attach_image(path);
}
Err(err) => {
tracing::warn!("failed to paste image: {err}");
@@ -1651,18 +1652,9 @@ impl ChatWidget {
}
}
pub(crate) fn attach_image(
&mut self,
path: PathBuf,
width: u32,
height: u32,
format_label: &str,
) {
tracing::info!(
"attach_image path={path:?} width={width} height={height} format={format_label}",
);
self.bottom_pane
.attach_image(path, width, height, format_label);
pub(crate) fn attach_image(&mut self, path: PathBuf) {
tracing::info!("attach_image path={path:?}");
self.bottom_pane.attach_image(path);
self.request_redraw();
}
@@ -2012,14 +2004,14 @@ impl ChatWidget {
return;
}
if !text.is_empty() {
items.push(UserInput::Text { text: text.clone() });
}
for path in image_paths {
items.push(UserInput::LocalImage { path });
}
if !text.is_empty() {
items.push(UserInput::Text { text: text.clone() });
}
if let Some(skills) = self.bottom_pane.skills() {
let skill_mentions = find_skill_mentions(&text, skills);
for skill in skill_mentions {
codex-rs/tui/src/chatwidget/tests.rs
+2 -2
View File
@@ -1114,8 +1114,8 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
chat.bottom_pane.insert_str("draft message ");
chat.bottom_pane
.attach_image(PathBuf::from("/tmp/preview.png"), 24, 42, "png");
let placeholder = "[preview.png 24x42]";
.attach_image(PathBuf::from("/tmp/preview.png"));
let placeholder = "[Image #1]";
assert!(
chat.bottom_pane.composer_text().ends_with(placeholder),
"expected placeholder {placeholder:?} in composer text"
codex-rs/tui2/src/bottom_pane/chat_composer.rs
+163 -240
View File
@@ -49,6 +49,7 @@ use crate::style::user_message_style;
use codex_common::fuzzy_match::fuzzy_match;
use codex_protocol::custom_prompts::CustomPrompt;
use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX;
use codex_protocol::models::local_image_label_text;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
@@ -62,7 +63,7 @@ use codex_core::skills::model::SkillMetadata;
use codex_file_search::FileMatch;
use std::cell::RefCell;
use std::collections::HashMap;
use std::path::Path;
use std::collections::HashSet;
use std::path::PathBuf;
use std::time::Duration;
use std::time::Instant;
@@ -287,10 +288,12 @@ impl ChatComposer {
// normalize_pasted_path already handles Windows → WSL path conversion,
// so we can directly try to read the image dimensions.
match image::image_dimensions(&path_buf) {
Ok((w, h)) => {
Ok((width, height)) => {
tracing::info!("OK: {pasted}");
let format_label = pasted_image_format(&path_buf).label();
self.attach_image(path_buf, w, h, format_label);
tracing::debug!("image dimensions={}x{}", width, height);
let format = pasted_image_format(&path_buf);
tracing::debug!("attached image format={}", format.label());
self.attach_image(path_buf);
true
}
Err(err) => {
@@ -342,12 +345,9 @@ impl ChatComposer {
}
/// Attempt to start a burst by retro-capturing recent chars before the cursor.
pub fn attach_image(&mut self, path: PathBuf, width: u32, height: u32, _format_label: &str) {
let file_label = path
.file_name()
.map(|name| name.to_string_lossy().into_owned())
.unwrap_or_else(|| "image".to_string());
let placeholder = format!("[{file_label} {width}x{height}]");
pub fn attach_image(&mut self, path: PathBuf) {
let image_number = self.attached_images.len() + 1;
let placeholder = local_image_label_text(image_number);
// Insert as an element to match large paste placeholder behavior:
// styled distinctly and treated atomically for cursor/mutations.
self.textarea.insert_element(&placeholder);
@@ -735,47 +735,43 @@ impl ChatComposer {
if is_image {
// Determine dimensions; if that fails fall back to normal path insertion.
let path_buf = PathBuf::from(&sel_path);
if let Ok((w, h)) = image::image_dimensions(&path_buf) {
// Remove the current @token (mirror logic from insert_selected_path without inserting text)
// using the flat text and byte-offset cursor API.
let cursor_offset = self.textarea.cursor();
let text = self.textarea.text();
// Clamp to a valid char boundary to avoid panics when slicing.
let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset);
let before_cursor = &text[..safe_cursor];
let after_cursor = &text[safe_cursor..];
match image::image_dimensions(&path_buf) {
Ok((width, height)) => {
tracing::debug!("selected image dimensions={}x{}", width, height);
// Remove the current @token (mirror logic from insert_selected_path without inserting text)
// using the flat text and byte-offset cursor API.
let cursor_offset = self.textarea.cursor();
let text = self.textarea.text();
// Clamp to a valid char boundary to avoid panics when slicing.
let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset);
let before_cursor = &text[..safe_cursor];
let after_cursor = &text[safe_cursor..];
// Determine token boundaries in the full text.
let start_idx = before_cursor
.char_indices()
.rfind(|(_, c)| c.is_whitespace())
.map(|(idx, c)| idx + c.len_utf8())
.unwrap_or(0);
let end_rel_idx = after_cursor
.char_indices()
.find(|(_, c)| c.is_whitespace())
.map(|(idx, _)| idx)
.unwrap_or(after_cursor.len());
let end_idx = safe_cursor + end_rel_idx;
// Determine token boundaries in the full text.
let start_idx = before_cursor
.char_indices()
.rfind(|(_, c)| c.is_whitespace())
.map(|(idx, c)| idx + c.len_utf8())
.unwrap_or(0);
let end_rel_idx = after_cursor
.char_indices()
.find(|(_, c)| c.is_whitespace())
.map(|(idx, _)| idx)
.unwrap_or(after_cursor.len());
let end_idx = safe_cursor + end_rel_idx;
self.textarea.replace_range(start_idx..end_idx, "");
self.textarea.set_cursor(start_idx);
self.textarea.replace_range(start_idx..end_idx, "");
self.textarea.set_cursor(start_idx);
let format_label = match Path::new(&sel_path)
.extension()
.and_then(|e| e.to_str())
.map(str::to_ascii_lowercase)
{
Some(ext) if ext == "png" => "PNG",
Some(ext) if ext == "jpg" || ext == "jpeg" => "JPEG",
_ => "IMG",
};
self.attach_image(path_buf, w, h, format_label);
// Add a trailing space to keep typing fluid.
self.textarea.insert_str(" ");
} else {
// Fallback to plain path insertion if metadata read fails.
self.insert_selected_path(&sel_path);
self.attach_image(path_buf);
// Add a trailing space to keep typing fluid.
self.textarea.insert_str(" ");
}
Err(err) => {
tracing::trace!("image dimensions lookup failed: {err}");
// Fallback to plain path insertion if metadata read fails.
self.insert_selected_path(&sel_path);
}
}
} else {
// Non-image: inserting file path.
@@ -1381,20 +1377,28 @@ impl ChatComposer {
}
}
// For non-char inputs (or after flushing), handle normally.
// Special handling for backspace on placeholders
if let KeyEvent {
code: KeyCode::Backspace,
..
} = input
&& self.try_remove_any_placeholder_at_cursor()
// Backspace at the start of an image placeholder should delete that placeholder (rather
// than deleting content before it). Do this without scanning the full text by consulting
// the textarea's element list.
if matches!(input.code, KeyCode::Backspace)
&& self.try_remove_image_element_at_cursor_start()
{
return (InputResult::None, true);
}
// Normal input handling
// Track element removals so we can drop any corresponding placeholders without scanning
// the full text. (Placeholders are atomic elements; when deleted, the element disappears.)
let elements_before = if self.pending_pastes.is_empty() && self.attached_images.is_empty() {
None
} else {
Some(self.textarea.element_payloads())
};
self.textarea.input(input);
let text_after = self.textarea.text();
if let Some(elements_before) = elements_before {
self.reconcile_deleted_elements(elements_before);
}
// Update paste-burst heuristic for plain Char (no Ctrl/Alt) events.
let crossterm::event::KeyEvent {
@@ -1416,176 +1420,69 @@ impl ChatComposer {
}
}
// Check if any placeholders were removed and remove their corresponding pending pastes
self.pending_pastes
.retain(|(placeholder, _)| text_after.contains(placeholder));
// Keep attached images in proportion to how many matching placeholders exist in the text.
// This handles duplicate placeholders that share the same visible label.
if !self.attached_images.is_empty() {
let mut needed: HashMap<String, usize> = HashMap::new();
for img in &self.attached_images {
needed
.entry(img.placeholder.clone())
.or_insert_with(|| text_after.matches(&img.placeholder).count());
}
let mut used: HashMap<String, usize> = HashMap::new();
let mut kept: Vec<AttachedImage> = Vec::with_capacity(self.attached_images.len());
for img in self.attached_images.drain(..) {
let total_needed = *needed.get(&img.placeholder).unwrap_or(&0);
let used_count = used.entry(img.placeholder.clone()).or_insert(0);
if *used_count < total_needed {
kept.push(img);
*used_count += 1;
}
}
self.attached_images = kept;
}
(InputResult::None, true)
}
/// Attempts to remove an image or paste placeholder if the cursor is at the end of one.
/// Returns true if a placeholder was removed.
fn try_remove_any_placeholder_at_cursor(&mut self) -> bool {
// Clamp the cursor to a valid char boundary to avoid panics when slicing.
let text = self.textarea.text();
let p = Self::clamp_to_char_boundary(text, self.textarea.cursor());
// Try image placeholders first
let mut out: Option<(usize, String)> = None;
// Detect if the cursor is at the end of any image placeholder.
// If duplicates exist, remove the specific occurrence's mapping.
for (i, img) in self.attached_images.iter().enumerate() {
let ph = &img.placeholder;
if p < ph.len() {
continue;
}
let start = p - ph.len();
if text.get(start..p) != Some(ph.as_str()) {
continue;
}
// Count the number of occurrences of `ph` before `start`.
let mut occ_before = 0usize;
let mut search_pos = 0usize;
while search_pos < start {
let segment = match text.get(search_pos..start) {
Some(s) => s,
None => break,
};
if let Some(found) = segment.find(ph) {
occ_before += 1;
search_pos += found + ph.len();
} else {
break;
}
}
// Remove the occ_before-th attached image that shares this placeholder label.
out = if let Some((remove_idx, _)) = self
.attached_images
.iter()
.enumerate()
.filter(|(_, img2)| img2.placeholder == *ph)
.nth(occ_before)
{
Some((remove_idx, ph.clone()))
} else {
Some((i, ph.clone()))
};
break;
}
if let Some((idx, placeholder)) = out {
self.textarea.replace_range(p - placeholder.len()..p, "");
self.attached_images.remove(idx);
return true;
fn try_remove_image_element_at_cursor_start(&mut self) -> bool {
if self.attached_images.is_empty() {
return false;
}
// Also handle when the cursor is at the START of an image placeholder.
// let result = 'out: {
let out: Option<(usize, String)> = 'out: {
for (i, img) in self.attached_images.iter().enumerate() {
let ph = &img.placeholder;
if p + ph.len() > text.len() {
continue;
}
if text.get(p..p + ph.len()) != Some(ph.as_str()) {
continue;
}
// Count occurrences of `ph` before `p`.
let mut occ_before = 0usize;
let mut search_pos = 0usize;
while search_pos < p {
let segment = match text.get(search_pos..p) {
Some(s) => s,
None => break 'out None,
};
if let Some(found) = segment.find(ph) {
occ_before += 1;
search_pos += found + ph.len();
} else {
break 'out None;
}
}
if let Some((remove_idx, _)) = self
.attached_images
.iter()
.enumerate()
.filter(|(_, img2)| img2.placeholder == *ph)
.nth(occ_before)
{
break 'out Some((remove_idx, ph.clone()));
} else {
break 'out Some((i, ph.clone()));
}
}
None
let p = self.textarea.cursor();
let Some(payload) = self.textarea.element_payload_starting_at(p) else {
return false;
};
let Some(idx) = self
.attached_images
.iter()
.position(|img| img.placeholder == payload)
else {
return false;
};
if let Some((idx, placeholder)) = out {
self.textarea.replace_range(p..p + placeholder.len(), "");
self.attached_images.remove(idx);
return true;
self.textarea.replace_range(p..p + payload.len(), "");
self.attached_images.remove(idx);
self.relabel_attached_images_and_update_placeholders();
true
}
fn reconcile_deleted_elements(&mut self, elements_before: Vec<String>) {
let elements_after: HashSet<String> =
self.textarea.element_payloads().into_iter().collect();
let mut removed_any_image = false;
for removed in elements_before
.into_iter()
.filter(|payload| !elements_after.contains(payload))
{
self.pending_pastes.retain(|(ph, _)| ph != &removed);
if let Some(idx) = self
.attached_images
.iter()
.position(|img| img.placeholder == removed)
{
self.attached_images.remove(idx);
removed_any_image = true;
}
}
// Then try pasted-content placeholders
if let Some(placeholder) = self.pending_pastes.iter().find_map(|(ph, _)| {
if p < ph.len() {
return None;
}
let start = p - ph.len();
if text.get(start..p) == Some(ph.as_str()) {
Some(ph.clone())
} else {
None
}
}) {
self.textarea.replace_range(p - placeholder.len()..p, "");
self.pending_pastes.retain(|(ph, _)| ph != &placeholder);
return true;
if removed_any_image {
self.relabel_attached_images_and_update_placeholders();
}
}
// Also handle when the cursor is at the START of a pasted-content placeholder.
if let Some(placeholder) = self.pending_pastes.iter().find_map(|(ph, _)| {
if p + ph.len() > text.len() {
return None;
fn relabel_attached_images_and_update_placeholders(&mut self) {
for idx in 0..self.attached_images.len() {
let expected = local_image_label_text(idx + 1);
let current = self.attached_images[idx].placeholder.clone();
if current == expected {
continue;
}
if text.get(p..p + ph.len()) == Some(ph.as_str()) {
Some(ph.clone())
} else {
None
}
}) {
self.textarea.replace_range(p..p + placeholder.len(), "");
self.pending_pastes.retain(|(ph, _)| ph != &placeholder);
return true;
self.attached_images[idx].placeholder = expected.clone();
let _renamed = self.textarea.replace_element_payload(&current, &expected);
}
false
}
fn handle_shortcut_overlay_key(&mut self, key_event: &KeyEvent) -> bool {
@@ -3408,12 +3305,12 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image1.png");
composer.attach_image(path.clone(), 32, 16, "PNG");
composer.attach_image(path.clone());
composer.handle_paste(" hi".into());
let (result, _) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Submitted(text) => assert_eq!(text, "[image1.png 32x16] hi"),
InputResult::Submitted(text) => assert_eq!(text, "[Image #1] hi"),
_ => panic!("expected Submitted"),
}
let imgs = composer.take_recent_submission_images();
@@ -3432,11 +3329,11 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image2.png");
composer.attach_image(path.clone(), 10, 5, "PNG");
composer.attach_image(path.clone());
let (result, _) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match result {
InputResult::Submitted(text) => assert_eq!(text, "[image2.png 10x5]"),
InputResult::Submitted(text) => assert_eq!(text, "[Image #1]"),
_ => panic!("expected Submitted"),
}
let imgs = composer.take_recent_submission_images();
@@ -3457,7 +3354,7 @@ mod tests {
false,
);
let path = PathBuf::from("/tmp/image3.png");
composer.attach_image(path.clone(), 20, 10, "PNG");
composer.attach_image(path.clone());
let placeholder = composer.attached_images[0].placeholder.clone();
// Case 1: backspace at end
@@ -3468,7 +3365,7 @@ mod tests {
// Re-add and test backspace in middle: should break the placeholder string
// and drop the image mapping (same as text placeholder behavior).
composer.attach_image(path, 20, 10, "PNG");
composer.attach_image(path);
let placeholder2 = composer.attached_images[0].placeholder.clone();
// Move cursor to roughly middle of placeholder
if let Some(start_pos) = composer.textarea.text().find(&placeholder2) {
@@ -3500,7 +3397,7 @@ mod tests {
// Insert an image placeholder at the start
let path = PathBuf::from("/tmp/image_multibyte.png");
composer.attach_image(path, 10, 5, "PNG");
composer.attach_image(path);
// Add multibyte text after the placeholder
composer.textarea.insert_str("日本語");
@@ -3509,12 +3406,7 @@ mod tests {
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
assert_eq!(composer.attached_images.len(), 1);
assert!(
composer
.textarea
.text()
.starts_with("[image_multibyte.png 10x5]")
);
assert!(composer.textarea.text().starts_with("[Image #1]"));
}
#[test]
@@ -3532,10 +3424,10 @@ mod tests {
let path1 = PathBuf::from("/tmp/image_dup1.png");
let path2 = PathBuf::from("/tmp/image_dup2.png");
composer.attach_image(path1, 10, 5, "PNG");
composer.attach_image(path1);
// separate placeholders with a space for clarity
composer.handle_paste(" ".into());
composer.attach_image(path2.clone(), 10, 5, "PNG");
composer.attach_image(path2.clone());
let placeholder1 = composer.attached_images[0].placeholder.clone();
let placeholder2 = composer.attached_images[1].placeholder.clone();
@@ -3550,24 +3442,60 @@ mod tests {
let new_text = composer.textarea.text().to_string();
assert_eq!(
0,
new_text.matches(&placeholder1).count(),
"first placeholder removed"
new_text.matches(&placeholder2).count(),
"second placeholder was relabeled"
);
assert_eq!(
1,
new_text.matches(&placeholder2).count(),
"second placeholder remains"
new_text.matches("[Image #1]").count(),
"remaining placeholder relabeled to #1"
);
assert_eq!(
vec![AttachedImage {
path: path2,
placeholder: "[image_dup2.png 10x5]".to_string()
placeholder: "[Image #1]".to_string()
}],
composer.attached_images,
"one image mapping remains"
);
}
#[test]
fn deleting_first_text_element_renumbers_following_text_element() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
true,
sender,
false,
"Ask Codex to do anything".to_string(),
false,
);
let path1 = PathBuf::from("/tmp/image_first.png");
let path2 = PathBuf::from("/tmp/image_second.png");
// Insert two adjacent atomic elements.
composer.attach_image(path1);
composer.attach_image(path2.clone());
assert_eq!(composer.textarea.text(), "[Image #1][Image #2]");
assert_eq!(composer.attached_images.len(), 2);
// Delete the first element using normal textarea editing (Delete at cursor start).
composer.textarea.set_cursor(0);
composer.handle_key_event(KeyEvent::new(KeyCode::Delete, KeyModifiers::NONE));
// Remaining image should be renumbered and the textarea element updated.
assert_eq!(composer.attached_images.len(), 1);
assert_eq!(composer.attached_images[0].path, path2);
assert_eq!(composer.attached_images[0].placeholder, "[Image #1]");
assert_eq!(composer.textarea.text(), "[Image #1]");
}
#[test]
fn pasting_filepath_attaches_image() {
let tmp = tempdir().expect("create TempDir");
@@ -3588,12 +3516,7 @@ mod tests {
let needs_redraw = composer.handle_paste(tmp_path.to_string_lossy().to_string());
assert!(needs_redraw);
assert!(
composer
.textarea
.text()
.starts_with("[codex_tui_test_paste_image.png 3x2] ")
);
assert!(composer.textarea.text().starts_with("[Image #1] "));
let imgs = composer.take_recent_submission_images();
assert_eq!(imgs, vec![tmp_path]);
codex-rs/tui2/src/bottom_pane/mod.rs
+2 -9
View File
@@ -529,16 +529,9 @@ impl BottomPane {
self.request_redraw();
}
pub(crate) fn attach_image(
&mut self,
path: PathBuf,
width: u32,
height: u32,
format_label: &str,
) {
pub(crate) fn attach_image(&mut self, path: PathBuf) {
if self.view_stack.is_empty() {
self.composer
.attach_image(path, width, height, format_label);
self.composer.attach_image(path);
self.request_redraw();
}
}
codex-rs/tui2/src/bottom_pane/textarea.rs
+82
View File
@@ -715,6 +715,88 @@ impl TextArea {
// ===== Text elements support =====
pub fn element_payloads(&self) -> Vec<String> {
self.elements
.iter()
.filter_map(|e| self.text.get(e.range.clone()).map(str::to_string))
.collect()
}
pub fn element_payload_starting_at(&self, pos: usize) -> Option<String> {
let pos = pos.min(self.text.len());
let elem = self.elements.iter().find(|e| e.range.start == pos)?;
self.text.get(elem.range.clone()).map(str::to_string)
}
/// Renames a single text element in-place, keeping it atomic.
///
/// This is intended for cases where the element payload is an identifier (e.g. a placeholder)
/// that must be updated without converting the element back into normal text.
pub fn replace_element_payload(&mut self, old: &str, new: &str) -> bool {
let Some(idx) = self
.elements
.iter()
.position(|e| self.text.get(e.range.clone()) == Some(old))
else {
return false;
};
let range = self.elements[idx].range.clone();
let start = range.start;
let end = range.end;
if start > end || end > self.text.len() {
return false;
}
let removed_len = end - start;
let inserted_len = new.len();
let diff = inserted_len as isize - removed_len as isize;
self.text.replace_range(range, new);
self.wrap_cache.replace(None);
self.preferred_col = None;
// Update the modified element's range.
self.elements[idx].range = start..(start + inserted_len);
// Shift element ranges that occur after the replaced element.
if diff != 0 {
for (j, e) in self.elements.iter_mut().enumerate() {
if j == idx {
continue;
}
if e.range.end <= start {
continue;
}
if e.range.start >= end {
e.range.start = ((e.range.start as isize) + diff) as usize;
e.range.end = ((e.range.end as isize) + diff) as usize;
continue;
}
// Elements should not partially overlap each other; degrade gracefully by
// snapping anything intersecting the replaced range to the new bounds.
e.range.start = start.min(e.range.start);
e.range.end = (start + inserted_len).max(e.range.end.saturating_add_signed(diff));
}
}
// Update the cursor position to account for the edit.
self.cursor_pos = if self.cursor_pos < start {
self.cursor_pos
} else if self.cursor_pos <= end {
start + inserted_len
} else {
((self.cursor_pos as isize) + diff) as usize
};
self.cursor_pos = self.clamp_pos_to_nearest_boundary(self.cursor_pos);
// Keep element ordering deterministic.
self.elements.sort_by_key(|e| e.range.start);
true
}
pub fn insert_element(&mut self, text: &str) {
let start = self.clamp_pos_for_insertion(self.cursor_pos);
self.insert_str_at(start, text);
codex-rs/tui2/src/chatwidget.rs
+11 -19
View File
@@ -1452,12 +1452,13 @@ impl ChatWidget {
{
match paste_image_to_temp_png() {
Ok((path, info)) => {
self.attach_image(
path,
tracing::debug!(
"pasted image size={}x{} format={}",
info.width,
info.height,
info.encoded_format.label(),
info.encoded_format.label()
);
self.attach_image(path);
}
Err(err) => {
tracing::warn!("failed to paste image: {err}");
@@ -1510,18 +1511,9 @@ impl ChatWidget {
}
}
pub(crate) fn attach_image(
&mut self,
path: PathBuf,
width: u32,
height: u32,
format_label: &str,
) {
tracing::info!(
"attach_image path={path:?} width={width} height={height} format={format_label}",
);
self.bottom_pane
.attach_image(path, width, height, format_label);
pub(crate) fn attach_image(&mut self, path: PathBuf) {
tracing::info!("attach_image path={path:?}");
self.bottom_pane.attach_image(path);
self.request_redraw();
}
@@ -1818,14 +1810,14 @@ impl ChatWidget {
return;
}
if !text.is_empty() {
items.push(UserInput::Text { text: text.clone() });
}
for path in image_paths {
items.push(UserInput::LocalImage { path });
}
if !text.is_empty() {
items.push(UserInput::Text { text: text.clone() });
}
if let Some(skills) = self.bottom_pane.skills() {
let skill_mentions = find_skill_mentions(&text, skills);
for skill in skill_mentions {
codex-rs/tui2/src/chatwidget/tests.rs
+2 -2
View File
@@ -1065,8 +1065,8 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
chat.bottom_pane.insert_str("draft message ");
chat.bottom_pane
.attach_image(PathBuf::from("/tmp/preview.png"), 24, 42, "png");
let placeholder = "[preview.png 24x42]";
.attach_image(PathBuf::from("/tmp/preview.png"));
let placeholder = "[Image #1]";
assert!(
chat.bottom_pane.composer_text().ends_with(placeholder),
"expected placeholder {placeholder:?} in composer text"