Merge branch 'main' into jif/review-as-collab

This commit is contained in:
jif-oai
2026-01-20 23:15:46 +00:00
committed by GitHub
51 changed files with 1325 additions and 436 deletions
Regular → Executable
View File
@@ -503,17 +503,14 @@ impl From<CoreTextElement> for V1TextElement {
fn from(value: CoreTextElement) -> Self {
Self {
byte_range: value.byte_range.into(),
placeholder: value.placeholder,
placeholder: value._placeholder_for_conversion_only().map(str::to_string),
}
}
}
impl From<V1TextElement> for CoreTextElement {
fn from(value: V1TextElement) -> Self {
Self {
byte_range: value.byte_range.into(),
placeholder: value.placeholder,
}
Self::new(value.byte_range.into(), value.placeholder)
}
}
@@ -1665,24 +1665,38 @@ pub struct TextElement {
/// Byte range in the parent `text` buffer that this element occupies.
pub byte_range: ByteRange,
/// Optional human-readable placeholder for the element, displayed in the UI.
pub placeholder: Option<String>,
placeholder: Option<String>,
}
impl TextElement {
pub fn new(byte_range: ByteRange, placeholder: Option<String>) -> Self {
Self {
byte_range,
placeholder,
}
}
pub fn set_placeholder(&mut self, placeholder: Option<String>) {
self.placeholder = placeholder;
}
pub fn placeholder(&self) -> Option<&str> {
self.placeholder.as_deref()
}
}
impl From<CoreTextElement> for TextElement {
fn from(value: CoreTextElement) -> Self {
Self {
byte_range: value.byte_range.into(),
placeholder: value.placeholder,
}
Self::new(
value.byte_range.into(),
value._placeholder_for_conversion_only().map(str::to_string),
)
}
}
impl From<TextElement> for CoreTextElement {
fn from(value: TextElement) -> Self {
Self {
byte_range: value.byte_range.into(),
placeholder: value.placeholder,
}
Self::new(value.byte_range.into(), value.placeholder)
}
}
@@ -74,10 +74,10 @@ async fn thread_resume_returns_rollout_history() -> Result<()> {
create_config_toml(codex_home.path(), &server.uri())?;
let preview = "Saved user message";
let text_elements = vec![TextElement {
byte_range: ByteRange { start: 0, end: 5 },
placeholder: Some("<note>".into()),
}];
let text_elements = vec![TextElement::new(
ByteRange { start: 0, end: 5 },
Some("<note>".into()),
)];
let conversation_id = create_fake_rollout_with_text_elements(
codex_home.path(),
"2025-01-05T12-00-00",
@@ -142,10 +142,10 @@ async fn turn_start_emits_user_message_item_with_text_elements() -> Result<()> {
.await??;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
let text_elements = vec![TextElement {
byte_range: ByteRange { start: 0, end: 5 },
placeholder: Some("<note>".to_string()),
}];
let text_elements = vec![TextElement::new(
ByteRange { start: 0, end: 5 },
Some("<note>".to_string()),
)];
let turn_req = mcp
.send_turn_start_request(TurnStartParams {
thread_id: thread.id.clone(),
+1
View File
@@ -18,6 +18,7 @@ codex_rust_crate(
),
integration_compile_data_extra = [
"//codex-rs/apply-patch:apply_patch_tool_instructions.md",
"models.json",
"prompt.md",
],
test_data_extra = [
+61 -27
View File
@@ -4335,6 +4335,8 @@ mod tests {
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
// Interrupts persist a model-visible `<turn_aborted>` marker into history, but there is no
// separate client-visible event for that marker (only `EventMsg::TurnAborted`).
let evt = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for event")
@@ -4343,6 +4345,7 @@ mod tests {
EventMsg::TurnAborted(e) => assert_eq!(TurnAbortReason::Interrupted, e.reason),
other => panic!("unexpected event: {other:?}"),
}
// No extra events should be emitted after an abort.
assert!(rx.try_recv().is_err());
}
@@ -4365,11 +4368,17 @@ mod tests {
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
let evt = rx.recv().await.expect("event");
// Even if tasks handle cancellation gracefully, interrupts still result in `TurnAborted`
// being the only client-visible signal.
let evt = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for event")
.expect("event");
match evt.msg {
EventMsg::TurnAborted(e) => assert_eq!(TurnAbortReason::Interrupted, e.reason),
other => panic!("unexpected event: {other:?}"),
}
// No extra events should be emitted after an abort.
assert!(rx.try_recv().is_err());
}
@@ -4391,42 +4400,67 @@ mod tests {
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
// Drain events until we observe ExitedReviewMode; earlier
// RawResponseItem entries (e.g., environment context) may arrive first.
loop {
let evt = tokio::time::timeout(std::time::Duration::from_secs(1), rx.recv())
// Aborting a review task should exit review mode before surfacing the abort to the client.
// We scan for these events (rather than relying on fixed ordering) since unrelated events
// may interleave.
let mut exited_review_mode_idx = None;
let mut turn_aborted_idx = None;
let mut idx = 0usize;
let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(3);
while tokio::time::Instant::now() < deadline {
let remaining = deadline.saturating_duration_since(tokio::time::Instant::now());
let evt = tokio::time::timeout(remaining, rx.recv())
.await
.expect("timeout waiting for first event")
.expect("first event");
.expect("timeout waiting for event")
.expect("event");
let event_idx = idx;
idx = idx.saturating_add(1);
match evt.msg {
EventMsg::ExitedReviewMode(ev) => {
assert!(ev.review_output.is_none());
exited_review_mode_idx = Some(event_idx);
}
EventMsg::TurnAborted(ev) => {
assert_eq!(TurnAbortReason::Interrupted, ev.reason);
turn_aborted_idx = Some(event_idx);
break;
}
// Ignore any non-critical events before exit.
_ => continue,
}
}
loop {
let evt = tokio::time::timeout(std::time::Duration::from_secs(2), rx.recv())
.await
.expect("timeout waiting for next event")
.expect("event");
match evt.msg {
EventMsg::RawResponseItem(_) => continue,
EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) => continue,
EventMsg::AgentMessage(_) => continue,
EventMsg::TurnAborted(e) => {
assert_eq!(TurnAbortReason::Interrupted, e.reason);
break;
}
other => panic!("unexpected second event: {other:?}"),
_ => {}
}
}
assert!(
exited_review_mode_idx.is_some(),
"expected ExitedReviewMode after abort"
);
assert!(
turn_aborted_idx.is_some(),
"expected TurnAborted after abort"
);
assert!(
exited_review_mode_idx.unwrap() < turn_aborted_idx.unwrap(),
"expected ExitedReviewMode before TurnAborted"
);
// TODO(jif) investigate what is this?
let history = sess.clone_history().await;
let _ = history.raw_items();
// The `<turn_aborted>` marker is silent in the event stream, so verify it is still
// recorded in history for the model.
assert!(
history.raw_items().iter().any(|item| {
let ResponseItem::Message { role, content, .. } = item else {
return false;
};
if role != "user" {
return false;
}
content.iter().any(|content_item| {
let ContentItem::InputText { text } = content_item else {
return false;
};
text.contains(crate::session_prefix::TURN_ABORTED_OPEN_TAG)
})
}),
"expected a model-visible turn aborted marker in history after interrupt"
);
}
#[tokio::test]
+60 -1
View File
@@ -15,6 +15,7 @@ use crate::protocol::EventMsg;
use crate::protocol::TurnContextItem;
use crate::protocol::TurnStartedEvent;
use crate::protocol::WarningEvent;
use crate::session_prefix::TURN_ABORTED_OPEN_TAG;
use crate::truncate::TruncationPolicy;
use crate::truncate::approx_token_count;
use crate::truncate::truncate_text;
@@ -223,11 +224,31 @@ pub(crate) fn collect_user_messages(items: &[ResponseItem]) -> Vec<String> {
Some(user.message())
}
}
_ => None,
_ => collect_turn_aborted_marker(item),
})
.collect()
}
fn collect_turn_aborted_marker(item: &ResponseItem) -> Option<String> {
let ResponseItem::Message { role, content, .. } = item else {
return None;
};
if role != "user" {
return None;
}
let text = content_items_to_text(content)?;
if text
.trim_start()
.to_ascii_lowercase()
.starts_with(TURN_ABORTED_OPEN_TAG)
{
Some(text)
} else {
None
}
}
pub(crate) fn is_summary_message(message: &str) -> bool {
message.starts_with(format!("{SUMMARY_PREFIX}\n").as_str())
}
@@ -337,6 +358,7 @@ async fn drain_to_completed(
mod tests {
use super::*;
use crate::session_prefix::TURN_ABORTED_OPEN_TAG;
use pretty_assertions::assert_eq;
#[test]
@@ -489,4 +511,41 @@ mod tests {
};
assert_eq!(summary, summary_text);
}
#[test]
fn build_compacted_history_preserves_turn_aborted_markers() {
let marker = format!(
"{TURN_ABORTED_OPEN_TAG}\n <turn_id>turn-1</turn_id>\n <reason>interrupted</reason>\n</turn_aborted>"
);
let items = vec![
ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText {
text: marker.clone(),
}],
},
ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText {
text: "real user message".to_string(),
}],
},
];
let user_messages = collect_user_messages(&items);
let history = build_compacted_history(Vec::new(), &user_messages, "SUMMARY");
let found_marker = history.iter().any(|item| match item {
ResponseItem::Message { role, content, .. } if role == "user" => {
content_items_to_text(content).is_some_and(|text| text == marker)
}
_ => false,
});
assert!(
found_marker,
"expected compacted history to retain <turn_aborted> marker"
);
}
}
+57 -2
View File
@@ -399,6 +399,7 @@ pub struct ConfigBuilder {
cli_overrides: Option<Vec<(String, TomlValue)>>,
harness_overrides: Option<ConfigOverrides>,
loader_overrides: Option<LoaderOverrides>,
fallback_cwd: Option<PathBuf>,
}
impl ConfigBuilder {
@@ -422,21 +423,29 @@ impl ConfigBuilder {
self
}
pub fn fallback_cwd(mut self, fallback_cwd: Option<PathBuf>) -> Self {
self.fallback_cwd = fallback_cwd;
self
}
pub async fn build(self) -> std::io::Result<Config> {
let Self {
codex_home,
cli_overrides,
harness_overrides,
loader_overrides,
fallback_cwd,
} = self;
let codex_home = codex_home.map_or_else(find_codex_home, std::io::Result::Ok)?;
let cli_overrides = cli_overrides.unwrap_or_default();
let harness_overrides = harness_overrides.unwrap_or_default();
let mut harness_overrides = harness_overrides.unwrap_or_default();
let loader_overrides = loader_overrides.unwrap_or_default();
let cwd = match harness_overrides.cwd.as_deref() {
let cwd_override = harness_overrides.cwd.as_deref().or(fallback_cwd.as_deref());
let cwd = match cwd_override {
Some(path) => AbsolutePathBuf::try_from(path)?,
None => AbsolutePathBuf::current_dir()?,
};
harness_overrides.cwd = Some(cwd.to_path_buf());
let config_layer_stack =
load_config_layers_state(&codex_home, Some(cwd), &cli_overrides, loader_overrides)
.await?;
@@ -2301,6 +2310,52 @@ trust_level = "trusted"
Ok(())
}
#[tokio::test]
async fn project_profile_overrides_user_profile() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let workspace = TempDir::new()?;
let workspace_key = workspace.path().to_string_lossy().replace('\\', "\\\\");
std::fs::write(
codex_home.path().join(CONFIG_TOML_FILE),
format!(
r#"
profile = "global"
[profiles.global]
model = "gpt-global"
[profiles.project]
model = "gpt-project"
[projects."{workspace_key}"]
trust_level = "trusted"
"#,
),
)?;
let project_config_dir = workspace.path().join(".codex");
std::fs::create_dir_all(&project_config_dir)?;
std::fs::write(
project_config_dir.join(CONFIG_TOML_FILE),
r#"
profile = "project"
"#,
)?;
let config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.harness_overrides(ConfigOverrides {
cwd: Some(workspace.path().to_path_buf()),
..Default::default()
})
.build()
.await?;
assert_eq!(config.active_profile.as_deref(), Some("project"));
assert_eq!(config.model.as_deref(), Some("gpt-project"));
Ok(())
}
#[test]
fn profile_sandbox_mode_overrides_base() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
+1 -6
View File
@@ -2,6 +2,7 @@ use crate::codex::TurnContext;
use crate::context_manager::normalize;
use crate::instructions::SkillInstructions;
use crate::instructions::UserInstructions;
use crate::session_prefix::is_session_prefix;
use crate::truncate::TruncationPolicy;
use crate::truncate::approx_token_count;
use crate::truncate::approx_tokens_from_byte_count;
@@ -328,12 +329,6 @@ fn estimate_reasoning_length(encoded_len: usize) -> usize {
.saturating_sub(650)
}
fn is_session_prefix(text: &str) -> bool {
let trimmed = text.trim_start();
let lowered = trimmed.to_ascii_lowercase();
lowered.starts_with("<environment_context>")
}
pub(crate) fn is_user_turn_boundary(item: &ResponseItem) -> bool {
let ResponseItem::Message { role, content, .. } = item else {
return false;
+1 -6
View File
@@ -19,14 +19,9 @@ use uuid::Uuid;
use crate::instructions::SkillInstructions;
use crate::instructions::UserInstructions;
use crate::session_prefix::is_session_prefix;
use crate::user_shell_command::is_user_shell_command_text;
fn is_session_prefix(text: &str) -> bool {
let trimmed = text.trim_start();
let lowered = trimmed.to_ascii_lowercase();
lowered.starts_with("<environment_context>")
}
fn parse_user_message(message: &[ContentItem]) -> Option<UserMessageItem> {
if UserInstructions::is_user_instructions(message)
|| SkillInstructions::is_skill_instructions(message)
+59 -29
View File
@@ -47,6 +47,12 @@ const EXEC_TIMEOUT_EXIT_CODE: i32 = 124; // conventional timeout exit code
const READ_CHUNK_SIZE: usize = 8192; // bytes per read
const AGGREGATE_BUFFER_INITIAL_CAPACITY: usize = 8 * 1024; // 8 KiB
/// Hard cap on bytes retained from exec stdout/stderr/aggregated output.
///
/// This mirrors unified exec's output cap so a single runaway command cannot
/// OOM the process by dumping huge amounts of data to stdout/stderr.
const EXEC_OUTPUT_MAX_BYTES: usize = 1024 * 1024; // 1 MiB
/// Limit the number of ExecCommandOutputDelta events emitted per exec call.
/// Aggregation still collects full output; only the live event stream is capped.
pub(crate) const MAX_EXEC_OUTPUT_DELTAS_PER_CALL: usize = 10_000;
@@ -290,18 +296,32 @@ async fn exec_windows_sandbox(
};
let exit_status = synthetic_exit_status(capture.exit_code);
let mut stdout_text = capture.stdout;
if stdout_text.len() > EXEC_OUTPUT_MAX_BYTES {
stdout_text.truncate(EXEC_OUTPUT_MAX_BYTES);
}
let mut stderr_text = capture.stderr;
if stderr_text.len() > EXEC_OUTPUT_MAX_BYTES {
stderr_text.truncate(EXEC_OUTPUT_MAX_BYTES);
}
let stdout = StreamOutput {
text: capture.stdout,
text: stdout_text,
truncated_after_lines: None,
};
let stderr = StreamOutput {
text: capture.stderr,
text: stderr_text,
truncated_after_lines: None,
};
// Best-effort aggregate: stdout then stderr
let mut aggregated = Vec::with_capacity(stdout.text.len() + stderr.text.len());
append_all(&mut aggregated, &stdout.text);
append_all(&mut aggregated, &stderr.text);
// Best-effort aggregate: stdout then stderr (capped).
let mut aggregated = Vec::with_capacity(
stdout
.text
.len()
.saturating_add(stderr.text.len())
.min(EXEC_OUTPUT_MAX_BYTES),
);
append_capped(&mut aggregated, &stdout.text, EXEC_OUTPUT_MAX_BYTES);
append_capped(&mut aggregated, &stderr.text, EXEC_OUTPUT_MAX_BYTES);
let aggregated_output = StreamOutput {
text: aggregated,
truncated_after_lines: None,
@@ -490,8 +510,13 @@ impl StreamOutput<Vec<u8>> {
}
#[inline]
fn append_all(dst: &mut Vec<u8>, src: &[u8]) {
dst.extend_from_slice(src);
fn append_capped(dst: &mut Vec<u8>, src: &[u8], max_bytes: usize) {
if dst.len() >= max_bytes {
return;
}
let remaining = max_bytes.saturating_sub(dst.len());
let take = remaining.min(src.len());
dst.extend_from_slice(&src[..take]);
}
#[derive(Clone, Debug)]
@@ -584,19 +609,15 @@ async fn consume_truncated_output(
))
})?;
let (agg_tx, agg_rx) = async_channel::unbounded::<Vec<u8>>();
let stdout_handle = tokio::spawn(read_capped(
BufReader::new(stdout_reader),
stdout_stream.clone(),
false,
Some(agg_tx.clone()),
));
let stderr_handle = tokio::spawn(read_capped(
BufReader::new(stderr_reader),
stdout_stream.clone(),
true,
Some(agg_tx.clone()),
));
let (exit_status, timed_out) = tokio::select! {
@@ -662,15 +683,18 @@ async fn consume_truncated_output(
Duration::from_millis(IO_DRAIN_TIMEOUT_MS),
)
.await?;
drop(agg_tx);
let mut combined_buf = Vec::with_capacity(AGGREGATE_BUFFER_INITIAL_CAPACITY);
while let Ok(chunk) = agg_rx.recv().await {
append_all(&mut combined_buf, &chunk);
}
// Best-effort aggregate: stdout then stderr (capped).
let mut aggregated = Vec::with_capacity(
stdout
.text
.len()
.saturating_add(stderr.text.len())
.min(EXEC_OUTPUT_MAX_BYTES),
);
append_capped(&mut aggregated, &stdout.text, EXEC_OUTPUT_MAX_BYTES);
append_capped(&mut aggregated, &stderr.text, EXEC_OUTPUT_MAX_BYTES * 2);
let aggregated_output = StreamOutput {
text: combined_buf,
text: aggregated,
truncated_after_lines: None,
};
@@ -687,14 +711,11 @@ async fn read_capped<R: AsyncRead + Unpin + Send + 'static>(
mut reader: R,
stream: Option<StdoutStream>,
is_stderr: bool,
aggregate_tx: Option<Sender<Vec<u8>>>,
) -> io::Result<StreamOutput<Vec<u8>>> {
let mut buf = Vec::with_capacity(AGGREGATE_BUFFER_INITIAL_CAPACITY);
let mut buf = Vec::with_capacity(AGGREGATE_BUFFER_INITIAL_CAPACITY.min(EXEC_OUTPUT_MAX_BYTES));
let mut tmp = [0u8; READ_CHUNK_SIZE];
let mut emitted_deltas: usize = 0;
// No caps: append all bytes
loop {
let n = reader.read(&mut tmp).await?;
if n == 0 {
@@ -723,11 +744,7 @@ async fn read_capped<R: AsyncRead + Unpin + Send + 'static>(
emitted_deltas += 1;
}
if let Some(tx) = &aggregate_tx {
let _ = tx.send(tmp[..n].to_vec()).await;
}
append_all(&mut buf, &tmp[..n]);
append_capped(&mut buf, &tmp[..n], EXEC_OUTPUT_MAX_BYTES);
// Continue reading to EOF to avoid back-pressure
}
@@ -755,6 +772,7 @@ fn synthetic_exit_status(code: i32) -> ExitStatus {
mod tests {
use super::*;
use std::time::Duration;
use tokio::io::AsyncWriteExt;
fn make_exec_output(
exit_code: i32,
@@ -816,6 +834,18 @@ mod tests {
));
}
#[tokio::test]
async fn read_capped_limits_retained_bytes() {
let (mut writer, reader) = tokio::io::duplex(1024);
let bytes = vec![b'a'; EXEC_OUTPUT_MAX_BYTES.saturating_add(128 * 1024)];
tokio::spawn(async move {
writer.write_all(&bytes).await.expect("write");
});
let out = read_capped(reader, None, false).await.expect("read");
assert_eq!(out.text.len(), EXEC_OUTPUT_MAX_BYTES);
}
#[cfg(unix)]
#[test]
fn sandbox_detection_flags_sigsys_exit_code() {
+2
View File
@@ -45,6 +45,7 @@ pub mod parse_command;
pub mod path_utils;
pub mod powershell;
pub mod sandboxing;
mod session_prefix;
mod stream_events_utils;
mod text_encoding;
pub mod token_data;
@@ -103,6 +104,7 @@ pub use rollout::list::ThreadSortKey;
pub use rollout::list::ThreadsPage;
pub use rollout::list::parse_cursor;
pub use rollout::list::read_head_for_summary;
pub use rollout::list::read_session_meta_line;
mod function_tool;
mod state;
mod tasks;
+37 -33
View File
@@ -239,7 +239,18 @@ impl ModelsManager {
/// Replace the cached remote models and rebuild the derived presets list.
async fn apply_remote_models(&self, models: Vec<ModelInfo>) {
*self.remote_models.write().await = models;
let mut existing_models = Self::load_remote_models_from_file().unwrap_or_default();
for model in models {
if let Some(existing_index) = existing_models
.iter()
.position(|existing| existing.slug == model.slug)
{
existing_models[existing_index] = model;
} else {
existing_models.push(model);
}
}
*self.remote_models.write().await = existing_models;
}
fn load_remote_models_from_file() -> Result<Vec<ModelInfo>, std::io::Error> {
@@ -272,16 +283,16 @@ impl ModelsManager {
let chatgpt_mode = self.auth_manager.get_auth_mode() == Some(AuthMode::ChatGPT);
merged_presets = ModelPreset::filter_by_auth(merged_presets, chatgpt_mode);
let has_default = merged_presets.iter().any(|preset| preset.is_default);
if !has_default {
if let Some(default) = merged_presets
.iter_mut()
.find(|preset| preset.show_in_picker)
{
default.is_default = true;
} else if let Some(default) = merged_presets.first_mut() {
default.is_default = true;
}
for preset in &mut merged_presets {
preset.is_default = false;
}
if let Some(default) = merged_presets
.iter_mut()
.find(|preset| preset.show_in_picker)
{
default.is_default = true;
} else if let Some(default) = merged_presets.first_mut() {
default.is_default = true;
}
merged_presets
@@ -396,6 +407,16 @@ mod tests {
.expect("valid model")
}
fn assert_models_contain(actual: &[ModelInfo], expected: &[ModelInfo]) {
for model in expected {
assert!(
actual.iter().any(|candidate| candidate.slug == model.slug),
"expected model {} in cached list",
model.slug
);
}
}
fn provider_for(base_url: String) -> ModelProviderInfo {
ModelProviderInfo {
name: "mock".into(),
@@ -415,7 +436,7 @@ mod tests {
}
#[tokio::test]
async fn refresh_available_models_sorts_and_marks_default() {
async fn refresh_available_models_sorts_by_priority() {
let server = MockServer::start().await;
let remote_models = vec![
remote_model("priority-low", "Low", 1),
@@ -447,7 +468,7 @@ mod tests {
.await
.expect("refresh succeeds");
let cached_remote = manager.get_remote_models(&config).await;
assert_eq!(cached_remote, remote_models);
assert_models_contain(&cached_remote, &remote_models);
let available = manager
.list_models(&config, RefreshStrategy::OnlineIfUncached)
@@ -464,11 +485,6 @@ mod tests {
high_idx < low_idx,
"higher priority should be listed before lower priority"
);
assert!(
available[high_idx].is_default,
"highest priority should be default"
);
assert!(!available[low_idx].is_default);
assert_eq!(
models_mock.requests().len(),
1,
@@ -508,22 +524,14 @@ mod tests {
.refresh_available_models(&config, RefreshStrategy::OnlineIfUncached)
.await
.expect("first refresh succeeds");
assert_eq!(
manager.get_remote_models(&config).await,
remote_models,
"remote cache should store fetched models"
);
assert_models_contain(&manager.get_remote_models(&config).await, &remote_models);
// Second call should read from cache and avoid the network.
manager
.refresh_available_models(&config, RefreshStrategy::OnlineIfUncached)
.await
.expect("cached refresh succeeds");
assert_eq!(
manager.get_remote_models(&config).await,
remote_models,
"cache path should not mutate stored models"
);
assert_models_contain(&manager.get_remote_models(&config).await, &remote_models);
assert_eq!(
models_mock.requests().len(),
1,
@@ -587,11 +595,7 @@ mod tests {
.refresh_available_models(&config, RefreshStrategy::OnlineIfUncached)
.await
.expect("second refresh succeeds");
assert_eq!(
manager.get_remote_models(&config).await,
updated_models,
"stale cache should trigger refetch"
);
assert_models_contain(&manager.get_remote_models(&config).await, &updated_models);
assert_eq!(
initial_mock.requests().len(),
1,
+19
View File
@@ -20,6 +20,7 @@ use crate::protocol::EventMsg;
use codex_file_search as file_search;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::RolloutLine;
use codex_protocol::protocol::SessionMetaLine;
use codex_protocol::protocol::SessionSource;
/// Returned page of thread (thread) summaries.
@@ -744,6 +745,24 @@ pub async fn read_head_for_summary(path: &Path) -> io::Result<Vec<serde_json::Va
Ok(summary.head)
}
/// Read the SessionMetaLine from the head of a rollout file for reuse by
/// callers that need the session metadata (e.g. to derive a cwd for config).
pub async fn read_session_meta_line(path: &Path) -> io::Result<SessionMetaLine> {
let head = read_head_for_summary(path).await?;
let Some(first) = head.first() else {
return Err(io::Error::other(format!(
"rollout at {} is empty",
path.display()
)));
};
serde_json::from_value::<SessionMetaLine>(first.clone()).map_err(|_| {
io::Error::other(format!(
"rollout at {} does not start with session metadata",
path.display()
))
})
}
async fn file_modified_time(path: &Path) -> io::Result<Option<OffsetDateTime>> {
let meta = tokio::fs::metadata(path).await?;
let modified = meta.modified().ok();
+8 -20
View File
@@ -1,12 +1,10 @@
//! Persist Codex session rollouts (.jsonl) so sessions can be replayed or inspected later.
use std::fs::File;
use std::fs::FileTimes;
use std::fs::{self};
use std::io::Error as IoError;
use std::path::Path;
use std::path::PathBuf;
use std::time::SystemTime;
use codex_protocol::ThreadId;
use codex_protocol::models::BaseInstructions;
@@ -197,17 +195,14 @@ impl RolloutRecorder {
}),
)
}
RolloutRecorderParams::Resume { path } => {
touch_rollout_file(&path)?;
(
tokio::fs::OpenOptions::new()
.append(true)
.open(&path)
.await?,
path,
None,
)
}
RolloutRecorderParams::Resume { path } => (
tokio::fs::OpenOptions::new()
.append(true)
.open(&path)
.await?,
path,
None,
),
};
// Clone the cwd for the spawned task to collect git info asynchronously
@@ -392,13 +387,6 @@ fn create_log_file(config: &Config, conversation_id: ThreadId) -> std::io::Resul
})
}
fn touch_rollout_file(path: &Path) -> std::io::Result<()> {
let file = fs::OpenOptions::new().append(true).open(path)?;
let times = FileTimes::new().set_modified(SystemTime::now());
file.set_times(times)?;
Ok(())
}
async fn rollout_writer(
file: tokio::fs::File,
mut rx: mpsc::Receiver<RolloutCmd>,
+15
View File
@@ -0,0 +1,15 @@
/// Helpers for identifying model-visible "session prefix" messages.
///
/// A session prefix is a user-role message that carries configuration or state needed by
/// follow-up turns (e.g. `<environment_context>`, `<turn_aborted>`). These items are persisted in
/// history so the model can see them, but they are not user intent and must not create user-turn
/// boundaries.
pub(crate) const ENVIRONMENT_CONTEXT_OPEN_TAG: &str = "<environment_context>";
pub(crate) const TURN_ABORTED_OPEN_TAG: &str = "<turn_aborted>";
/// Returns true if `text` starts with a session prefix marker (case-insensitive).
pub(crate) fn is_session_prefix(text: &str) -> bool {
let trimmed = text.trim_start();
let lowered = trimmed.to_ascii_lowercase();
lowered.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG) || lowered.starts_with(TURN_ABORTED_OPEN_TAG)
}
+24
View File
@@ -23,9 +23,13 @@ use crate::protocol::EventMsg;
use crate::protocol::TurnAbortReason;
use crate::protocol::TurnAbortedEvent;
use crate::protocol::TurnCompleteEvent;
use crate::session_prefix::TURN_ABORTED_OPEN_TAG;
use crate::state::ActiveTurn;
use crate::state::RunningTask;
use crate::state::TaskKind;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::user_input::UserInput;
pub(crate) use compact::CompactTask;
@@ -36,6 +40,7 @@ pub(crate) use undo::UndoTask;
pub(crate) use user_shell::UserShellCommandTask;
const GRACEFULL_INTERRUPTION_TIMEOUT_MS: u64 = 100;
const TURN_ABORTED_INTERRUPTED_GUIDANCE: &str = "The user interrupted the previous turn. Do not continue or repeat work from that turn unless the user explicitly asks. If any tools/commands were aborted, they may have partially executed; verify current state before retrying.";
/// Thin wrapper that exposes the parts of [`Session`] task runners need.
#[derive(Clone)]
@@ -237,6 +242,25 @@ impl Session {
.abort(session_ctx, Arc::clone(&task.turn_context))
.await;
if reason == TurnAbortReason::Interrupted {
let marker = ResponseItem::Message {
id: None,
role: "user".to_string(),
content: vec![ContentItem::InputText {
text: format!(
"{TURN_ABORTED_OPEN_TAG}\n <turn_id>{sub_id}</turn_id>\n <reason>interrupted</reason>\n <guidance>{TURN_ABORTED_INTERRUPTED_GUIDANCE}</guidance>\n</turn_aborted>"
),
}],
};
self.record_into_history(std::slice::from_ref(&marker), task.turn_context.as_ref())
.await;
self.persist_rollout_items(&[RolloutItem::ResponseItem(marker)])
.await;
// Ensure the marker is durably visible before emitting TurnAborted: some clients
// synchronously re-read the rollout on receipt of the abort event.
self.flush_rollout().await;
}
let event = EventMsg::TurnAborted(TurnAbortedEvent { reason });
self.send_event(task.turn_context.as_ref(), event).await;
}
+76
View File
@@ -163,3 +163,79 @@ async fn interrupt_tool_records_history_entries() {
"expected at least one tenth of a second of elapsed time, got {secs}"
);
}
/// After an interrupt we persist a model-visible `<turn_aborted>` marker in the conversation
/// history. This test asserts that the marker is included in the next `/responses` request.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn interrupt_persists_turn_aborted_marker_in_next_request() {
let command = "sleep 60";
let call_id = "call-turn-aborted-marker";
let args = json!({
"command": command,
"timeout_ms": 60_000
})
.to_string();
let first_body = sse(vec![
ev_response_created("resp-marker"),
ev_function_call(call_id, "shell_command", &args),
ev_completed("resp-marker"),
]);
let follow_up_body = sse(vec![
ev_response_created("resp-followup"),
ev_completed("resp-followup"),
]);
let server = start_mock_server().await;
let response_mock = mount_sse_sequence(&server, vec![first_body, follow_up_body]).await;
let fixture = test_codex()
.with_model("gpt-5.1")
.build(&server)
.await
.unwrap();
let codex = Arc::clone(&fixture.codex);
codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "start interrupt marker".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await
.unwrap();
wait_for_event(&codex, |ev| matches!(ev, EventMsg::ExecCommandBegin(_))).await;
tokio::time::sleep(Duration::from_secs_f32(0.1)).await;
codex.submit(Op::Interrupt).await.unwrap();
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnAborted(_))).await;
codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "follow up".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await
.unwrap();
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
let requests = response_mock.requests();
assert_eq!(requests.len(), 2, "expected two calls to the responses API");
let follow_up_request = &requests[1];
let user_texts = follow_up_request.message_input_texts("user");
assert!(
user_texts
.iter()
.any(|text| text.contains("<turn_aborted>")),
"expected <turn_aborted> marker in follow-up request"
);
}
+4 -4
View File
@@ -40,10 +40,10 @@ async fn user_message_item_is_emitted() -> anyhow::Result<()> {
let first_response = sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]);
mount_sse_once(&server, first_response).await;
let text_elements = vec![TextElement {
byte_range: ByteRange { start: 0, end: 6 },
placeholder: Some("<file>".into()),
}];
let text_elements = vec![TextElement::new(
ByteRange { start: 0, end: 6 },
Some("<file>".into()),
)];
let expected_input = UserInput::Text {
text: "please inspect sample.txt".into(),
text_elements: text_elements.clone(),
+165 -1
View File
@@ -1,4 +1,5 @@
#![cfg(not(target_os = "windows"))]
#![allow(clippy::expect_used)]
// unified exec is not supported on Windows OS
use std::sync::Arc;
@@ -434,8 +435,18 @@ async fn remote_models_preserve_builtin_presets() -> Result<()> {
.find(|model| model.model == "remote-alpha")
.expect("remote model should be listed");
let mut expected_remote: ModelPreset = remote_model.into();
expected_remote.is_default = true;
expected_remote.is_default = remote.is_default;
assert_eq!(*remote, expected_remote);
let default_model = available
.iter()
.find(|model| model.show_in_picker)
.expect("default model should be set");
assert!(default_model.is_default);
assert_eq!(
available.iter().filter(|model| model.is_default).count(),
1,
"expected a single default model"
);
assert!(
available
.iter()
@@ -451,6 +462,148 @@ async fn remote_models_preserve_builtin_presets() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_models_merge_adds_new_high_priority_first() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = MockServer::start().await;
let remote_model = test_remote_model("remote-top", ModelVisibility::List, -10_000);
let models_mock = mount_models_once(
&server,
ModelsResponse {
models: vec![remote_model],
},
)
.await;
let codex_home = TempDir::new()?;
let mut config = load_default_config_for_test(&codex_home).await;
config.features.enable(Feature::RemoteModels);
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
let provider = ModelProviderInfo {
base_url: Some(format!("{}/v1", server.uri())),
..built_in_model_providers()["openai"].clone()
};
let manager = ModelsManager::with_provider(
codex_home.path().to_path_buf(),
codex_core::auth::AuthManager::from_auth_for_testing(auth),
provider,
);
let available = manager
.list_models(&config, RefreshStrategy::OnlineIfUncached)
.await;
assert_eq!(
available.first().map(|model| model.model.as_str()),
Some("remote-top")
);
assert_eq!(
models_mock.requests().len(),
1,
"expected a single /models request"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_models_merge_replaces_overlapping_model() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = MockServer::start().await;
let slug = bundled_model_slug();
let mut remote_model = test_remote_model(&slug, ModelVisibility::List, 0);
remote_model.display_name = "Overridden".to_string();
remote_model.description = Some("Overridden description".to_string());
let models_mock = mount_models_once(
&server,
ModelsResponse {
models: vec![remote_model.clone()],
},
)
.await;
let codex_home = TempDir::new()?;
let mut config = load_default_config_for_test(&codex_home).await;
config.features.enable(Feature::RemoteModels);
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
let provider = ModelProviderInfo {
base_url: Some(format!("{}/v1", server.uri())),
..built_in_model_providers()["openai"].clone()
};
let manager = ModelsManager::with_provider(
codex_home.path().to_path_buf(),
codex_core::auth::AuthManager::from_auth_for_testing(auth),
provider,
);
let available = manager
.list_models(&config, RefreshStrategy::OnlineIfUncached)
.await;
let overridden = available
.iter()
.find(|model| model.model == slug)
.expect("overlapping model should be listed");
assert_eq!(overridden.display_name, remote_model.display_name);
assert_eq!(
overridden.description,
remote_model
.description
.expect("remote model should include description")
);
assert_eq!(
models_mock.requests().len(),
1,
"expected a single /models request"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_models_merge_preserves_bundled_models_on_empty_response() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
let server = MockServer::start().await;
let models_mock = mount_models_once(&server, ModelsResponse { models: Vec::new() }).await;
let codex_home = TempDir::new()?;
let mut config = load_default_config_for_test(&codex_home).await;
config.features.enable(Feature::RemoteModels);
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
let provider = ModelProviderInfo {
base_url: Some(format!("{}/v1", server.uri())),
..built_in_model_providers()["openai"].clone()
};
let manager = ModelsManager::with_provider(
codex_home.path().to_path_buf(),
codex_core::auth::AuthManager::from_auth_for_testing(auth),
provider,
);
let available = manager
.list_models(&config, RefreshStrategy::OnlineIfUncached)
.await;
let bundled_slug = bundled_model_slug();
assert!(
available.iter().any(|model| model.model == bundled_slug),
"bundled models should remain available after empty remote response"
);
assert_eq!(
models_mock.requests().len(),
1,
"expected a single /models request"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_models_request_times_out_after_5s() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -588,6 +741,17 @@ async fn wait_for_model_available(
}
}
fn bundled_model_slug() -> String {
let response: ModelsResponse = serde_json::from_str(include_str!("../../models.json"))
.expect("bundled models.json should deserialize");
response
.models
.first()
.expect("bundled models.json should include at least one model")
.slug
.clone()
}
fn test_remote_model(slug: &str, visibility: ModelVisibility, priority: i32) -> ModelInfo {
test_remote_model_with_policy(
slug,
+4 -4
View File
@@ -35,10 +35,10 @@ async fn resume_includes_initial_messages_from_rollout_events() -> Result<()> {
]);
mount_sse_once(&server, initial_sse).await;
let text_elements = vec![TextElement {
byte_range: ByteRange { start: 0, end: 6 },
placeholder: Some("<note>".into()),
}];
let text_elements = vec![TextElement::new(
ByteRange { start: 0, end: 6 },
Some("<note>".into()),
)];
codex
.submit(Op::UserInput {
+8 -7
View File
@@ -97,13 +97,14 @@ impl UserMessageItem {
// Text element ranges are relative to each text chunk; offset them so they align
// with the concatenated message returned by `message()`.
for elem in text_elements {
out.push(TextElement {
byte_range: ByteRange {
start: offset + elem.byte_range.start,
end: offset + elem.byte_range.end,
},
placeholder: elem.placeholder.clone(),
});
let byte_range = ByteRange {
start: offset + elem.byte_range.start,
end: offset + elem.byte_range.end,
};
out.push(TextElement::new(
byte_range,
elem.placeholder(text).map(str::to_string),
));
}
offset += text.len();
}
+29 -1
View File
@@ -36,7 +36,35 @@ pub struct TextElement {
/// Byte range in the parent `text` buffer that this element occupies.
pub byte_range: ByteRange,
/// Optional human-readable placeholder for the element, displayed in the UI.
pub placeholder: Option<String>,
placeholder: Option<String>,
}
impl TextElement {
pub fn new(byte_range: ByteRange, placeholder: Option<String>) -> Self {
Self {
byte_range,
placeholder,
}
}
pub fn set_placeholder(&mut self, placeholder: Option<String>) {
self.placeholder = placeholder;
}
/// Returns the stored placeholder without falling back to the text buffer.
///
/// This is intended only for protocol conversions where the full text is not
/// available; prefer `placeholder(text)` for UI logic.
#[doc(hidden)]
pub fn _placeholder_for_conversion_only(&self) -> Option<&str> {
self.placeholder.as_deref()
}
pub fn placeholder<'a>(&'a self, text: &'a str) -> Option<&'a str> {
self.placeholder
.as_deref()
.or_else(|| text.get(self.byte_range.start..self.byte_range.end))
}
}
#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, TS, JsonSchema)]
+22 -30
View File
@@ -557,12 +557,7 @@ impl ChatComposer {
let image_placeholders: HashSet<String> = text_elements
.iter()
.filter_map(|elem| {
elem.placeholder.as_ref().cloned().or_else(|| {
text.get(elem.byte_range.start..elem.byte_range.end)
.map(str::to_string)
})
})
.filter_map(|elem| elem.placeholder(&text).map(str::to_string))
.collect();
for (idx, path) in local_image_paths.into_iter().enumerate() {
let placeholder = local_image_label_text(idx + 1);
@@ -1240,13 +1235,13 @@ impl ChatComposer {
return None;
}
let placeholder = trimmed.get(new_start..new_end).map(str::to_string);
Some(TextElement {
byte_range: ByteRange {
Some(TextElement::new(
ByteRange {
start: new_start,
end: new_end,
},
placeholder,
})
))
})
.collect()
}
@@ -1287,7 +1282,7 @@ impl ChatComposer {
rebuilt.push_str(&text[cursor..start]);
}
let elem_text = &text[start..end];
let placeholder = elem.placeholder;
let placeholder = elem.placeholder(text).map(str::to_string);
let replacement = placeholder
.as_deref()
.and_then(|ph| pending_by_placeholder.get_mut(ph))
@@ -1301,13 +1296,13 @@ impl ChatComposer {
rebuilt.push_str(elem_text);
let new_end = rebuilt.len();
let placeholder = placeholder.or_else(|| Some(elem_text.to_string()));
rebuilt_elements.push(TextElement {
byte_range: ByteRange {
rebuilt_elements.push(TextElement::new(
ByteRange {
start: new_start,
end: new_end,
},
placeholder,
});
));
}
cursor = end;
}
@@ -4232,7 +4227,7 @@ mod tests {
} => {
assert_eq!(text, "[Image #1] hi");
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4261,10 +4256,7 @@ mod tests {
let placeholder = local_image_label_text(1);
let text = format!("{placeholder} restored");
let text_elements = vec![TextElement {
byte_range: (0..placeholder.len()).into(),
placeholder: None,
}];
let text_elements = vec![TextElement::new((0..placeholder.len()).into(), None)];
let path = PathBuf::from("/tmp/image1.png");
composer.set_text_content(text, text_elements, vec![path.clone()]);
@@ -4301,7 +4293,7 @@ mod tests {
let expected = format!("{large_content} [Image #1]");
assert_eq!(text, expected);
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4345,7 +4337,7 @@ mod tests {
let trimmed = large_content.trim().to_string();
assert_eq!(text, format!("{trimmed} [Image #1]"));
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4389,7 +4381,7 @@ mod tests {
assert_eq!(text, "line1\nline2\n [Image #1]");
assert!(!text.contains('\r'));
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4474,7 +4466,7 @@ mod tests {
} => {
assert_eq!(text, "[Image #1]");
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4664,20 +4656,20 @@ mod tests {
let start2 = text.find(&placeholder2).expect("placeholder2 present");
let start1 = text.find(&placeholder1).expect("placeholder1 present");
let text_elements = vec![
TextElement {
byte_range: ByteRange {
TextElement::new(
ByteRange {
start: start2,
end: start2 + placeholder2.len(),
},
placeholder: Some(placeholder2),
},
TextElement {
byte_range: ByteRange {
Some(placeholder2),
),
TextElement::new(
ByteRange {
start: start1,
end: start1 + placeholder1.len(),
},
placeholder: Some(placeholder1.clone()),
},
Some(placeholder1.clone()),
),
];
composer.set_text_content(text, text_elements, vec![path1, path2.clone()]);
+61 -4
View File
@@ -701,11 +701,14 @@ impl BottomPane {
if !self.unified_exec_footer.is_empty() {
flex.push(0, RenderableItem::Borrowed(&self.unified_exec_footer));
}
let has_queued_messages = !self.queued_user_messages.messages.is_empty();
let has_status_or_footer =
self.status.is_some() || !self.unified_exec_footer.is_empty();
if has_queued_messages && has_status_or_footer {
flex.push(0, RenderableItem::Owned("".into()));
}
flex.push(1, RenderableItem::Borrowed(&self.queued_user_messages));
if self.status.is_some()
|| !self.unified_exec_footer.is_empty()
|| !self.queued_user_messages.messages.is_empty()
{
if !has_queued_messages && has_status_or_footer {
flex.push(0, RenderableItem::Owned("".into()));
}
let mut flex2 = FlexRenderable::new();
@@ -951,6 +954,60 @@ mod tests {
);
}
#[test]
fn status_only_snapshot() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let mut pane = BottomPane::new(BottomPaneParams {
app_event_tx: tx,
frame_requester: FrameRequester::test_dummy(),
has_input_focus: true,
enhanced_keys_supported: false,
placeholder_text: "Ask Codex to do anything".to_string(),
disable_paste_burst: false,
animations_enabled: true,
skills: Some(Vec::new()),
});
pane.set_task_running(true);
let width = 48;
let height = pane.desired_height(width);
let area = Rect::new(0, 0, width, height);
assert_snapshot!("status_only_snapshot", render_snapshot(&pane, area));
}
#[test]
fn status_with_details_and_queued_messages_snapshot() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let mut pane = BottomPane::new(BottomPaneParams {
app_event_tx: tx,
frame_requester: FrameRequester::test_dummy(),
has_input_focus: true,
enhanced_keys_supported: false,
placeholder_text: "Ask Codex to do anything".to_string(),
disable_paste_burst: false,
animations_enabled: true,
skills: Some(Vec::new()),
});
pane.set_task_running(true);
pane.update_status(
"Working".to_string(),
Some("First detail line\nSecond detail line".to_string()),
);
pane.set_queued_user_messages(vec!["Queued follow-up question".to_string()]);
let width = 48;
let height = pane.desired_height(width);
let area = Rect::new(0, 0, width, height);
assert_snapshot!(
"status_with_details_and_queued_messages_snapshot",
render_snapshot(&pane, area)
);
}
#[test]
fn queued_messages_visible_when_status_hidden_snapshot() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
@@ -5,7 +5,6 @@ expression: "render_snapshot(&pane, area)"
↳ Queued follow-up question
⌥ + ↑ edit
› Ask Codex to do anything
100% context left · ? for shortcuts
@@ -3,10 +3,10 @@ source: tui/src/bottom_pane/mod.rs
expression: "render_snapshot(&pane, area)"
---
• Working (0s • esc to interrupt)
↳ Queued follow-up question
⌥ + ↑ edit
› Ask Codex to do anything
100% context left · ? for shortcuts
@@ -0,0 +1,10 @@
---
source: tui/src/bottom_pane/mod.rs
expression: "render_snapshot(&pane, area)"
---
• Working (0s • esc to interrupt)
› Ask Codex to do anything
100% context left · ? for shortcuts
@@ -0,0 +1,14 @@
---
source: tui/src/bottom_pane/mod.rs
expression: "render_snapshot(&pane, area)"
---
• Working (0s • esc to interrupt)
└ First detail line
Second detail line
↳ Queued follow-up question
⌥ + ↑ edit
› Ask Codex to do anything
100% context left · ? for shortcuts
+3 -3
View File
@@ -755,13 +755,13 @@ impl TextArea {
.iter()
.map(|e| {
let placeholder = self.text.get(e.range.clone()).map(str::to_string);
UserTextElement {
byte_range: ByteRange {
UserTextElement::new(
ByteRange {
start: e.range.start,
end: e.range.end,
},
placeholder,
}
)
})
.collect()
}
+5 -5
View File
@@ -614,9 +614,9 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize)
}
let original = text.get(start..end).unwrap_or("");
let placeholder_key = elem.placeholder.as_deref().unwrap_or(original);
let replacement = mapping
.get(placeholder_key)
let placeholder = elem.placeholder(&text);
let replacement = placeholder
.and_then(|ph| mapping.get(ph))
.map(String::as_str)
.unwrap_or(original);
@@ -624,8 +624,8 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize)
rebuilt.push_str(replacement);
let elem_end = rebuilt.len();
if let Some(remapped) = mapping.get(placeholder_key) {
elem.placeholder = Some(remapped.clone());
if let Some(remapped) = placeholder.and_then(|ph| mapping.get(ph)) {
elem.set_placeholder(Some(remapped.clone()));
}
elem.byte_range = (elem_start..elem_end).into();
rebuilt_elements.push(elem);
@@ -2,7 +2,9 @@
source: tui/src/chatwidget/tests.rs
expression: term.backend().vt100().screen().contents()
---
• Working (0s • esc to interrupt)
↳ Hello, world! 0
↳ Hello, world! 1
↳ Hello, world! 2
@@ -21,7 +23,6 @@ expression: term.backend().vt100().screen().contents()
↳ Hello, world! 15
↳ Hello, world! 16
› Ask Codex to do anything
100% context left · ? for shortcuts
@@ -1,6 +1,5 @@
---
source: tui/src/chatwidget/tests.rs
assertion_line: 3840
expression: term.backend().vt100().screen().contents()
---
@@ -14,10 +13,10 @@ expression: term.backend().vt100().screen().contents()
• Working (0s • esc to interrupt)
↳ Queued while /review is running.
⌥ + ↑ edit
› Ask Codex to do anything
100% context left · ? for shortcuts
+61 -66
View File
@@ -193,10 +193,10 @@ async fn replayed_user_message_preserves_text_elements_and_local_images() {
let placeholder = "[Image #1]";
let message = format!("{placeholder} replayed");
let text_elements = vec![TextElement {
byte_range: (0..placeholder.len()).into(),
placeholder: Some(placeholder.to_string()),
}];
let text_elements = vec![TextElement::new(
(0..placeholder.len()).into(),
Some(placeholder.to_string()),
)];
let local_images = vec![PathBuf::from("/tmp/replay.png")];
let conversation_id = ThreadId::new();
@@ -275,10 +275,10 @@ async fn submission_preserves_text_elements_and_local_images() {
let placeholder = "[Image #1]";
let text = format!("{placeholder} submit");
let text_elements = vec![TextElement {
byte_range: (0..placeholder.len()).into(),
placeholder: Some(placeholder.to_string()),
}];
let text_elements = vec![TextElement::new(
(0..placeholder.len()).into(),
Some(placeholder.to_string()),
)];
let local_images = vec![PathBuf::from("/tmp/submitted.png")];
chat.bottom_pane
@@ -331,26 +331,26 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() {
let first_placeholder = "[Image #1]";
let first_text = format!("{first_placeholder} first");
let first_elements = vec![TextElement {
byte_range: (0..first_placeholder.len()).into(),
placeholder: Some(first_placeholder.to_string()),
}];
let first_elements = vec![TextElement::new(
(0..first_placeholder.len()).into(),
Some(first_placeholder.to_string()),
)];
let first_images = [PathBuf::from("/tmp/first.png")];
let second_placeholder = "[Image #1]";
let second_text = format!("{second_placeholder} second");
let second_elements = vec![TextElement {
byte_range: (0..second_placeholder.len()).into(),
placeholder: Some(second_placeholder.to_string()),
}];
let second_elements = vec![TextElement::new(
(0..second_placeholder.len()).into(),
Some(second_placeholder.to_string()),
)];
let second_images = [PathBuf::from("/tmp/second.png")];
let existing_placeholder = "[Image #1]";
let existing_text = format!("{existing_placeholder} existing");
let existing_elements = vec![TextElement {
byte_range: (0..existing_placeholder.len()).into(),
placeholder: Some(existing_placeholder.to_string()),
}];
let existing_elements = vec![TextElement::new(
(0..existing_placeholder.len()).into(),
Some(existing_placeholder.to_string()),
)];
let existing_images = vec![PathBuf::from("/tmp/existing.png")];
chat.queued_user_messages.push_back(UserMessage {
@@ -393,18 +393,18 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() {
let second_start = first.len() + 1;
let third_start = second_start + second.len() + 1;
let expected_elements = vec![
TextElement {
byte_range: (first_start..first_start + "[Image #1]".len()).into(),
placeholder: Some("[Image #1]".to_string()),
},
TextElement {
byte_range: (second_start..second_start + "[Image #2]".len()).into(),
placeholder: Some("[Image #2]".to_string()),
},
TextElement {
byte_range: (third_start..third_start + "[Image #3]".len()).into(),
placeholder: Some("[Image #3]".to_string()),
},
TextElement::new(
(first_start..first_start + "[Image #1]".len()).into(),
Some("[Image #1]".to_string()),
),
TextElement::new(
(second_start..second_start + "[Image #2]".len()).into(),
Some("[Image #2]".to_string()),
),
TextElement::new(
(third_start..third_start + "[Image #3]".len()).into(),
Some("[Image #3]".to_string()),
),
];
assert_eq!(chat.bottom_pane.composer_text_elements(), expected_elements);
assert_eq!(
@@ -423,14 +423,14 @@ async fn remap_placeholders_uses_attachment_labels() {
let placeholder_two = "[Image #2]";
let text = format!("{placeholder_two} before {placeholder_one}");
let elements = vec![
TextElement {
byte_range: (0..placeholder_two.len()).into(),
placeholder: Some(placeholder_two.to_string()),
},
TextElement {
byte_range: ("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
placeholder: Some(placeholder_one.to_string()),
},
TextElement::new(
(0..placeholder_two.len()).into(),
Some(placeholder_two.to_string()),
),
TextElement::new(
("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
Some(placeholder_one.to_string()),
),
];
let attachments = vec![
@@ -455,15 +455,14 @@ async fn remap_placeholders_uses_attachment_labels() {
assert_eq!(
remapped.text_elements,
vec![
TextElement {
byte_range: (0.."[Image #4]".len()).into(),
placeholder: Some("[Image #4]".to_string()),
},
TextElement {
byte_range: ("[Image #4] before ".len().."[Image #4] before [Image #3]".len())
.into(),
placeholder: Some("[Image #3]".to_string()),
},
TextElement::new(
(0.."[Image #4]".len()).into(),
Some("[Image #4]".to_string()),
),
TextElement::new(
("[Image #4] before ".len().."[Image #4] before [Image #3]".len()).into(),
Some("[Image #3]".to_string()),
),
]
);
assert_eq!(
@@ -487,14 +486,11 @@ async fn remap_placeholders_uses_byte_ranges_when_placeholder_missing() {
let placeholder_two = "[Image #2]";
let text = format!("{placeholder_two} before {placeholder_one}");
let elements = vec![
TextElement {
byte_range: (0..placeholder_two.len()).into(),
placeholder: None,
},
TextElement {
byte_range: ("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
placeholder: None,
},
TextElement::new((0..placeholder_two.len()).into(), None),
TextElement::new(
("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
None,
),
];
let attachments = vec![
@@ -519,15 +515,14 @@ async fn remap_placeholders_uses_byte_ranges_when_placeholder_missing() {
assert_eq!(
remapped.text_elements,
vec![
TextElement {
byte_range: (0.."[Image #4]".len()).into(),
placeholder: Some("[Image #4]".to_string()),
},
TextElement {
byte_range: ("[Image #4] before ".len().."[Image #4] before [Image #3]".len())
.into(),
placeholder: Some("[Image #3]".to_string()),
},
TextElement::new(
(0.."[Image #4]".len()).into(),
Some("[Image #4]".to_string()),
),
TextElement::new(
("[Image #4] before ".len().."[Image #4] before [Image #3]".len()).into(),
Some("[Image #3]".to_string()),
),
]
);
assert_eq!(
+51 -17
View File
@@ -18,6 +18,7 @@ use codex_core::RolloutRecorder;
use codex_core::ThreadSortKey;
use codex_core::auth::enforce_login_restrictions;
use codex_core::config::Config;
use codex_core::config::ConfigBuilder;
use codex_core::config::ConfigOverrides;
use codex_core::config::find_codex_home;
use codex_core::config::load_config_as_toml_with_cli_overrides;
@@ -25,6 +26,7 @@ use codex_core::config::resolve_oss_provider;
use codex_core::find_thread_path_by_id_str;
use codex_core::get_platform_sandbox;
use codex_core::protocol::AskForApproval;
use codex_core::read_session_meta_line;
use codex_core::terminal::Multiplexer;
use codex_protocol::config_types::AltScreenMode;
use codex_protocol::config_types::SandboxMode;
@@ -247,7 +249,6 @@ pub async fn run_main(
std::process::exit(1);
}
let active_profile = config.active_profile.clone();
let log_dir = codex_core::config::log_dir(&config)?;
std::fs::create_dir_all(&log_dir)?;
// Open (or create) your log file, appending to it.
@@ -337,16 +338,9 @@ pub async fn run_main(
.with(otel_tracing_layer)
.try_init();
run_ratatui_app(
cli,
config,
overrides,
cli_kv_overrides,
active_profile,
feedback,
)
.await
.map_err(|err| std::io::Error::other(err.to_string()))
run_ratatui_app(cli, config, overrides, cli_kv_overrides, feedback)
.await
.map_err(|err| std::io::Error::other(err.to_string()))
}
async fn run_ratatui_app(
@@ -354,7 +348,6 @@ async fn run_ratatui_app(
initial_config: Config,
overrides: ConfigOverrides,
cli_kv_overrides: Vec<(String, toml::Value)>,
active_profile: Option<String>,
feedback: codex_feedback::CodexFeedback,
) -> color_eyre::Result<AppExitInfo> {
color_eyre::install()?;
@@ -405,15 +398,15 @@ async fn run_ratatui_app(
initial_config.cli_auth_credentials_store_mode,
);
let login_status = get_login_status(&initial_config);
let should_show_trust_screen = should_show_trust_screen(&initial_config);
let should_show_trust_screen_flag = should_show_trust_screen(&initial_config);
let should_show_onboarding =
should_show_onboarding(login_status, &initial_config, should_show_trust_screen);
should_show_onboarding(login_status, &initial_config, should_show_trust_screen_flag);
let config = if should_show_onboarding {
let onboarding_result = run_onboarding_app(
OnboardingScreenArgs {
show_login_screen: should_show_login_screen(login_status, &initial_config),
show_trust_screen: should_show_trust_screen,
show_trust_screen: should_show_trust_screen_flag,
login_status,
auth_manager: auth_manager.clone(),
config: initial_config.clone(),
@@ -438,7 +431,7 @@ async fn run_ratatui_app(
.map(|d| d == TrustDirectorySelection::Trust)
.unwrap_or(false)
{
load_config_or_exit(cli_kv_overrides, overrides).await
load_config_or_exit(cli_kv_overrides.clone(), overrides.clone()).await
} else {
initial_config
}
@@ -571,6 +564,33 @@ async fn run_ratatui_app(
resume_picker::SessionSelection::StartFresh
};
let config = match &session_selection {
resume_picker::SessionSelection::Resume(path)
| resume_picker::SessionSelection::Fork(path) => {
let history_cwd = match read_session_meta_line(path).await {
Ok(meta_line) => Some(meta_line.meta.cwd),
Err(err) => {
let rollout_path = path.display().to_string();
tracing::warn!(
%rollout_path,
%err,
"Failed to read session metadata from rollout"
);
None
}
};
load_config_or_exit_with_fallback_cwd(
cli_kv_overrides.clone(),
overrides.clone(),
history_cwd,
)
.await
}
_ => config,
};
let active_profile = config.active_profile.clone();
let should_show_trust_screen = should_show_trust_screen(&config);
let Cli {
prompt,
images,
@@ -672,9 +692,23 @@ fn get_login_status(config: &Config) -> LoginStatus {
async fn load_config_or_exit(
cli_kv_overrides: Vec<(String, toml::Value)>,
overrides: ConfigOverrides,
) -> Config {
load_config_or_exit_with_fallback_cwd(cli_kv_overrides, overrides, None).await
}
async fn load_config_or_exit_with_fallback_cwd(
cli_kv_overrides: Vec<(String, toml::Value)>,
overrides: ConfigOverrides,
fallback_cwd: Option<PathBuf>,
) -> Config {
#[allow(clippy::print_stderr)]
match Config::load_with_cli_overrides_and_harness_overrides(cli_kv_overrides, overrides).await {
match ConfigBuilder::default()
.cli_overrides(cli_kv_overrides)
.harness_overrides(overrides)
.fallback_cwd(fallback_cwd)
.build()
.await
{
Ok(config) => config,
Err(err) => {
eprintln!("Error loading configuration: {err}");
+22 -30
View File
@@ -489,12 +489,7 @@ impl ChatComposer {
let image_placeholders: HashSet<String> = text_elements
.iter()
.filter_map(|elem| {
elem.placeholder.as_ref().cloned().or_else(|| {
text.get(elem.byte_range.start..elem.byte_range.end)
.map(str::to_string)
})
})
.filter_map(|elem| elem.placeholder(&text).map(str::to_string))
.collect();
for (idx, path) in local_image_paths.into_iter().enumerate() {
let placeholder = local_image_label_text(idx + 1);
@@ -1174,13 +1169,13 @@ impl ChatComposer {
return None;
}
let placeholder = trimmed.get(new_start..new_end).map(str::to_string);
Some(TextElement {
byte_range: ByteRange {
Some(TextElement::new(
ByteRange {
start: new_start,
end: new_end,
},
placeholder,
})
))
})
.collect()
}
@@ -1219,7 +1214,7 @@ impl ChatComposer {
rebuilt.push_str(&text[cursor..start]);
}
let elem_text = &text[start..end];
let placeholder = elem.placeholder;
let placeholder = elem.placeholder(text).map(str::to_string);
let replacement = placeholder
.as_deref()
.and_then(|ph| pending_by_placeholder.get_mut(ph))
@@ -1231,13 +1226,13 @@ impl ChatComposer {
rebuilt.push_str(elem_text);
let new_end = rebuilt.len();
let placeholder = placeholder.or_else(|| Some(elem_text.to_string()));
rebuilt_elements.push(TextElement {
byte_range: ByteRange {
rebuilt_elements.push(TextElement::new(
ByteRange {
start: new_start,
end: new_end,
},
placeholder,
});
));
}
cursor = end;
}
@@ -4223,7 +4218,7 @@ mod tests {
} => {
assert_eq!(text, "[Image #1] hi");
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4252,10 +4247,7 @@ mod tests {
let placeholder = local_image_label_text(1);
let text = format!("{placeholder} restored");
let text_elements = vec![TextElement {
byte_range: (0..placeholder.len()).into(),
placeholder: None,
}];
let text_elements = vec![TextElement::new((0..placeholder.len()).into(), None)];
let path = PathBuf::from("/tmp/image1.png");
composer.set_text_content(text, text_elements, vec![path.clone()]);
@@ -4292,7 +4284,7 @@ mod tests {
let expected = format!("{large_content} [Image #1]");
assert_eq!(text, expected);
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4336,7 +4328,7 @@ mod tests {
let trimmed = large_content.trim().to_string();
assert_eq!(text, format!("{trimmed} [Image #1]"));
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4380,7 +4372,7 @@ mod tests {
assert_eq!(text, "line1\nline2\n [Image #1]");
assert!(!text.contains('\r'));
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4465,7 +4457,7 @@ mod tests {
} => {
assert_eq!(text, "[Image #1]");
assert_eq!(text_elements.len(), 1);
assert_eq!(text_elements[0].placeholder.as_deref(), Some("[Image #1]"));
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
assert_eq!(
text_elements[0].byte_range,
ByteRange {
@@ -4627,20 +4619,20 @@ mod tests {
let start2 = text.find(&placeholder2).expect("placeholder2 present");
let start1 = text.find(&placeholder1).expect("placeholder1 present");
let text_elements = vec![
TextElement {
byte_range: ByteRange {
TextElement::new(
ByteRange {
start: start2,
end: start2 + placeholder2.len(),
},
placeholder: Some(placeholder2),
},
TextElement {
byte_range: ByteRange {
Some(placeholder2),
),
TextElement::new(
ByteRange {
start: start1,
end: start1 + placeholder1.len(),
},
placeholder: Some(placeholder1.clone()),
},
Some(placeholder1.clone()),
),
];
composer.set_text_content(text, text_elements, vec![path1, path2.clone()]);
+60 -1
View File
@@ -688,8 +688,13 @@ impl BottomPane {
if let Some(status) = &self.status {
flex.push(0, RenderableItem::Borrowed(status));
}
let has_queued_messages = !self.queued_user_messages.messages.is_empty();
let has_status = self.status.is_some();
if has_queued_messages && has_status {
flex.push(0, RenderableItem::Owned("".into()));
}
flex.push(1, RenderableItem::Borrowed(&self.queued_user_messages));
if self.status.is_some() || !self.queued_user_messages.messages.is_empty() {
if !has_queued_messages && has_status {
flex.push(0, RenderableItem::Owned("".into()));
}
let mut flex2 = FlexRenderable::new();
@@ -931,6 +936,60 @@ mod tests {
);
}
#[test]
fn status_only_snapshot() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let mut pane = BottomPane::new(BottomPaneParams {
app_event_tx: tx,
frame_requester: FrameRequester::test_dummy(),
has_input_focus: true,
enhanced_keys_supported: false,
placeholder_text: "Ask Codex to do anything".to_string(),
disable_paste_burst: false,
animations_enabled: true,
skills: Some(Vec::new()),
});
pane.set_task_running(true);
let width = 48;
let height = pane.desired_height(width);
let area = Rect::new(0, 0, width, height);
assert_snapshot!("status_only_snapshot", render_snapshot(&pane, area));
}
#[test]
fn status_with_details_and_queued_messages_snapshot() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let mut pane = BottomPane::new(BottomPaneParams {
app_event_tx: tx,
frame_requester: FrameRequester::test_dummy(),
has_input_focus: true,
enhanced_keys_supported: false,
placeholder_text: "Ask Codex to do anything".to_string(),
disable_paste_burst: false,
animations_enabled: true,
skills: Some(Vec::new()),
});
pane.set_task_running(true);
pane.update_status(
"Working".to_string(),
Some("First detail line\nSecond detail line".to_string()),
);
pane.set_queued_user_messages(vec!["Queued follow-up question".to_string()]);
let width = 48;
let height = pane.desired_height(width);
let area = Rect::new(0, 0, width, height);
assert_snapshot!(
"status_with_details_and_queued_messages_snapshot",
render_snapshot(&pane, area)
);
}
#[test]
fn queued_messages_visible_when_status_hidden_snapshot() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
@@ -5,7 +5,6 @@ expression: "render_snapshot(&pane, area)"
↳ Queued follow-up question
⌥ + ↑ edit
› Ask Codex to do anything
100% context left · ? for shortcuts
@@ -3,10 +3,10 @@ source: tui2/src/bottom_pane/mod.rs
expression: "render_snapshot(&pane, area)"
---
• Working (0s • esc to interrupt)
↳ Queued follow-up question
⌥ + ↑ edit
› Ask Codex to do anything
100% context left · ? for shortcuts
@@ -0,0 +1,10 @@
---
source: tui2/src/bottom_pane/mod.rs
expression: "render_snapshot(&pane, area)"
---
• Working (0s • esc to interrupt)
› Ask Codex to do anything
100% context left · ? for shortcuts
@@ -0,0 +1,14 @@
---
source: tui2/src/bottom_pane/mod.rs
expression: "render_snapshot(&pane, area)"
---
• Working (0s • esc to interrupt)
└ First detail line
Second detail line
↳ Queued follow-up question
⌥ + ↑ edit
› Ask Codex to do anything
100% context left · ? for shortcuts
+3 -3
View File
@@ -752,13 +752,13 @@ impl TextArea {
.iter()
.map(|e| {
let placeholder = self.text.get(e.range.clone()).map(str::to_string);
UserTextElement {
byte_range: ByteRange {
UserTextElement::new(
ByteRange {
start: e.range.start,
end: e.range.end,
},
placeholder,
}
)
})
.collect()
}
+5 -5
View File
@@ -556,9 +556,9 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize)
}
let original = text.get(start..end).unwrap_or("");
let placeholder_key = elem.placeholder.as_deref().unwrap_or(original);
let replacement = mapping
.get(placeholder_key)
let placeholder = elem.placeholder(&text);
let replacement = placeholder
.and_then(|ph| mapping.get(ph))
.map(String::as_str)
.unwrap_or(original);
@@ -566,8 +566,8 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize)
rebuilt.push_str(replacement);
let elem_end = rebuilt.len();
if let Some(remapped) = mapping.get(placeholder_key) {
elem.placeholder = Some(remapped.clone());
if let Some(remapped) = placeholder.and_then(|ph| mapping.get(ph)) {
elem.set_placeholder(Some(remapped.clone()));
}
elem.byte_range = (elem_start..elem_end).into();
rebuilt_elements.push(elem);
@@ -2,7 +2,9 @@
source: tui2/src/chatwidget/tests.rs
expression: term.backend().vt100().screen().contents()
---
• Working (0s • esc to interrupt)
↳ Hello, world! 0
↳ Hello, world! 1
↳ Hello, world! 2
@@ -21,7 +23,6 @@ expression: term.backend().vt100().screen().contents()
↳ Hello, world! 15
↳ Hello, world! 16
› Ask Codex to do anything
100% context left · ? for shortcuts
+61 -66
View File
@@ -182,10 +182,10 @@ async fn replayed_user_message_preserves_text_elements_and_local_images() {
let placeholder = "[Image #1]";
let message = format!("{placeholder} replayed");
let text_elements = vec![TextElement {
byte_range: (0..placeholder.len()).into(),
placeholder: Some(placeholder.to_string()),
}];
let text_elements = vec![TextElement::new(
(0..placeholder.len()).into(),
Some(placeholder.to_string()),
)];
let local_images = vec![PathBuf::from("/tmp/replay.png")];
let conversation_id = ThreadId::new();
@@ -264,10 +264,10 @@ async fn submission_preserves_text_elements_and_local_images() {
let placeholder = "[Image #1]";
let text = format!("{placeholder} submit");
let text_elements = vec![TextElement {
byte_range: (0..placeholder.len()).into(),
placeholder: Some(placeholder.to_string()),
}];
let text_elements = vec![TextElement::new(
(0..placeholder.len()).into(),
Some(placeholder.to_string()),
)];
let local_images = vec![PathBuf::from("/tmp/submitted.png")];
chat.bottom_pane
@@ -320,26 +320,26 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() {
let first_placeholder = "[Image #1]";
let first_text = format!("{first_placeholder} first");
let first_elements = vec![TextElement {
byte_range: (0..first_placeholder.len()).into(),
placeholder: Some(first_placeholder.to_string()),
}];
let first_elements = vec![TextElement::new(
(0..first_placeholder.len()).into(),
Some(first_placeholder.to_string()),
)];
let first_images = [PathBuf::from("/tmp/first.png")];
let second_placeholder = "[Image #1]";
let second_text = format!("{second_placeholder} second");
let second_elements = vec![TextElement {
byte_range: (0..second_placeholder.len()).into(),
placeholder: Some(second_placeholder.to_string()),
}];
let second_elements = vec![TextElement::new(
(0..second_placeholder.len()).into(),
Some(second_placeholder.to_string()),
)];
let second_images = [PathBuf::from("/tmp/second.png")];
let existing_placeholder = "[Image #1]";
let existing_text = format!("{existing_placeholder} existing");
let existing_elements = vec![TextElement {
byte_range: (0..existing_placeholder.len()).into(),
placeholder: Some(existing_placeholder.to_string()),
}];
let existing_elements = vec![TextElement::new(
(0..existing_placeholder.len()).into(),
Some(existing_placeholder.to_string()),
)];
let existing_images = vec![PathBuf::from("/tmp/existing.png")];
chat.queued_user_messages.push_back(UserMessage {
@@ -382,18 +382,18 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() {
let second_start = first.len() + 1;
let third_start = second_start + second.len() + 1;
let expected_elements = vec![
TextElement {
byte_range: (first_start..first_start + "[Image #1]".len()).into(),
placeholder: Some("[Image #1]".to_string()),
},
TextElement {
byte_range: (second_start..second_start + "[Image #2]".len()).into(),
placeholder: Some("[Image #2]".to_string()),
},
TextElement {
byte_range: (third_start..third_start + "[Image #3]".len()).into(),
placeholder: Some("[Image #3]".to_string()),
},
TextElement::new(
(first_start..first_start + "[Image #1]".len()).into(),
Some("[Image #1]".to_string()),
),
TextElement::new(
(second_start..second_start + "[Image #2]".len()).into(),
Some("[Image #2]".to_string()),
),
TextElement::new(
(third_start..third_start + "[Image #3]".len()).into(),
Some("[Image #3]".to_string()),
),
];
assert_eq!(chat.bottom_pane.composer_text_elements(), expected_elements);
assert_eq!(
@@ -412,14 +412,14 @@ async fn remap_placeholders_uses_attachment_labels() {
let placeholder_two = "[Image #2]";
let text = format!("{placeholder_two} before {placeholder_one}");
let elements = vec![
TextElement {
byte_range: (0..placeholder_two.len()).into(),
placeholder: Some(placeholder_two.to_string()),
},
TextElement {
byte_range: ("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
placeholder: Some(placeholder_one.to_string()),
},
TextElement::new(
(0..placeholder_two.len()).into(),
Some(placeholder_two.to_string()),
),
TextElement::new(
("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
Some(placeholder_one.to_string()),
),
];
let attachments = vec![
@@ -444,15 +444,14 @@ async fn remap_placeholders_uses_attachment_labels() {
assert_eq!(
remapped.text_elements,
vec![
TextElement {
byte_range: (0.."[Image #4]".len()).into(),
placeholder: Some("[Image #4]".to_string()),
},
TextElement {
byte_range: ("[Image #4] before ".len().."[Image #4] before [Image #3]".len())
.into(),
placeholder: Some("[Image #3]".to_string()),
},
TextElement::new(
(0.."[Image #4]".len()).into(),
Some("[Image #4]".to_string()),
),
TextElement::new(
("[Image #4] before ".len().."[Image #4] before [Image #3]".len()).into(),
Some("[Image #3]".to_string()),
),
]
);
assert_eq!(
@@ -476,14 +475,11 @@ async fn remap_placeholders_uses_byte_ranges_when_placeholder_missing() {
let placeholder_two = "[Image #2]";
let text = format!("{placeholder_two} before {placeholder_one}");
let elements = vec![
TextElement {
byte_range: (0..placeholder_two.len()).into(),
placeholder: None,
},
TextElement {
byte_range: ("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
placeholder: None,
},
TextElement::new((0..placeholder_two.len()).into(), None),
TextElement::new(
("[Image #2] before ".len().."[Image #2] before [Image #1]".len()).into(),
None,
),
];
let attachments = vec![
@@ -508,15 +504,14 @@ async fn remap_placeholders_uses_byte_ranges_when_placeholder_missing() {
assert_eq!(
remapped.text_elements,
vec![
TextElement {
byte_range: (0.."[Image #4]".len()).into(),
placeholder: Some("[Image #4]".to_string()),
},
TextElement {
byte_range: ("[Image #4] before ".len().."[Image #4] before [Image #3]".len())
.into(),
placeholder: Some("[Image #3]".to_string()),
},
TextElement::new(
(0.."[Image #4]".len()).into(),
Some("[Image #4]".to_string()),
),
TextElement::new(
("[Image #4] before ".len().."[Image #4] before [Image #3]".len()).into(),
Some("[Image #3]".to_string()),
),
]
);
assert_eq!(
+51 -17
View File
@@ -18,6 +18,7 @@ use codex_core::RolloutRecorder;
use codex_core::ThreadSortKey;
use codex_core::auth::enforce_login_restrictions;
use codex_core::config::Config;
use codex_core::config::ConfigBuilder;
use codex_core::config::ConfigOverrides;
use codex_core::config::find_codex_home;
use codex_core::config::load_config_as_toml_with_cli_overrides;
@@ -25,6 +26,7 @@ use codex_core::config::resolve_oss_provider;
use codex_core::find_thread_path_by_id_str;
use codex_core::get_platform_sandbox;
use codex_core::protocol::AskForApproval;
use codex_core::read_session_meta_line;
use codex_core::terminal::Multiplexer;
use codex_protocol::config_types::AltScreenMode;
use codex_protocol::config_types::SandboxMode;
@@ -263,7 +265,6 @@ pub async fn run_main(
std::process::exit(1);
}
let active_profile = config.active_profile.clone();
let log_dir = codex_core::config::log_dir(&config)?;
std::fs::create_dir_all(&log_dir)?;
// Open (or create) your log file, appending to it.
@@ -356,16 +357,9 @@ pub async fn run_main(
let terminal_info = codex_core::terminal::terminal_info();
tracing::info!(terminal = ?terminal_info, "Detected terminal info");
run_ratatui_app(
cli,
config,
overrides,
cli_kv_overrides,
active_profile,
feedback,
)
.await
.map_err(|err| std::io::Error::other(err.to_string()))
run_ratatui_app(cli, config, overrides, cli_kv_overrides, feedback)
.await
.map_err(|err| std::io::Error::other(err.to_string()))
}
async fn run_ratatui_app(
@@ -373,7 +367,6 @@ async fn run_ratatui_app(
initial_config: Config,
overrides: ConfigOverrides,
cli_kv_overrides: Vec<(String, toml::Value)>,
active_profile: Option<String>,
feedback: codex_feedback::CodexFeedback,
) -> color_eyre::Result<AppExitInfo> {
color_eyre::install()?;
@@ -425,15 +418,15 @@ async fn run_ratatui_app(
initial_config.cli_auth_credentials_store_mode,
);
let login_status = get_login_status(&initial_config);
let should_show_trust_screen = should_show_trust_screen(&initial_config);
let should_show_trust_screen_flag = should_show_trust_screen(&initial_config);
let should_show_onboarding =
should_show_onboarding(login_status, &initial_config, should_show_trust_screen);
should_show_onboarding(login_status, &initial_config, should_show_trust_screen_flag);
let config = if should_show_onboarding {
let onboarding_result = run_onboarding_app(
OnboardingScreenArgs {
show_login_screen: should_show_login_screen(login_status, &initial_config),
show_trust_screen: should_show_trust_screen,
show_trust_screen: should_show_trust_screen_flag,
login_status,
auth_manager: auth_manager.clone(),
config: initial_config.clone(),
@@ -459,7 +452,7 @@ async fn run_ratatui_app(
.map(|d| d == TrustDirectorySelection::Trust)
.unwrap_or(false)
{
load_config_or_exit(cli_kv_overrides, overrides).await
load_config_or_exit(cli_kv_overrides.clone(), overrides.clone()).await
} else {
initial_config
}
@@ -595,6 +588,33 @@ async fn run_ratatui_app(
resume_picker::SessionSelection::StartFresh
};
let config = match &session_selection {
resume_picker::SessionSelection::Resume(path)
| resume_picker::SessionSelection::Fork(path) => {
let history_cwd = match read_session_meta_line(path).await {
Ok(meta_line) => Some(meta_line.meta.cwd),
Err(err) => {
let rollout_path = path.display().to_string();
tracing::warn!(
%rollout_path,
%err,
"Failed to read session metadata from rollout"
);
None
}
};
load_config_or_exit_with_fallback_cwd(
cli_kv_overrides.clone(),
overrides.clone(),
history_cwd,
)
.await
}
_ => config,
};
let active_profile = config.active_profile.clone();
let should_show_trust_screen = should_show_trust_screen(&config);
let Cli {
prompt,
images,
@@ -700,9 +720,23 @@ fn get_login_status(config: &Config) -> LoginStatus {
async fn load_config_or_exit(
cli_kv_overrides: Vec<(String, toml::Value)>,
overrides: ConfigOverrides,
) -> Config {
load_config_or_exit_with_fallback_cwd(cli_kv_overrides, overrides, None).await
}
async fn load_config_or_exit_with_fallback_cwd(
cli_kv_overrides: Vec<(String, toml::Value)>,
overrides: ConfigOverrides,
fallback_cwd: Option<PathBuf>,
) -> Config {
#[allow(clippy::print_stderr)]
match Config::load_with_cli_overrides_and_harness_overrides(cli_kv_overrides, overrides).await {
match ConfigBuilder::default()
.cli_overrides(cli_kv_overrides)
.harness_overrides(overrides)
.fallback_cwd(fallback_cwd)
.build()
.await
{
Ok(config) => config,
Err(err) => {
eprintln!("Error loading configuration: {err}");
+30 -3
View File
@@ -52,9 +52,9 @@ pub fn compute_allow_paths(
let canonical = canonicalize(&candidate).unwrap_or(candidate);
add_allow(canonical.clone());
let git_dir = canonical.join(".git");
if git_dir.is_dir() {
add_deny(git_dir);
let git_entry = canonical.join(".git");
if git_entry.exists() {
add_deny(git_entry);
}
};
@@ -178,6 +178,33 @@ mod tests {
assert_eq!(expected_deny, paths.deny);
}
#[test]
fn denies_git_file_inside_writable_root() {
let tmp = TempDir::new().expect("tempdir");
let command_cwd = tmp.path().join("workspace");
let git_file = command_cwd.join(".git");
let _ = fs::create_dir_all(&command_cwd);
let _ = fs::write(&git_file, "gitdir: .git/worktrees/example");
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: false,
};
let paths = compute_allow_paths(&policy, &command_cwd, &command_cwd, &HashMap::new());
let expected_allow: HashSet<PathBuf> = [dunce::canonicalize(&command_cwd).unwrap()]
.into_iter()
.collect();
let expected_deny: HashSet<PathBuf> = [dunce::canonicalize(&git_file).unwrap()]
.into_iter()
.collect();
assert_eq!(expected_allow, paths.allow);
assert_eq!(expected_deny, paths.deny);
}
#[test]
fn skips_git_dir_when_missing() {
let tmp = TempDir::new().expect("tempdir");
+42 -3
View File
@@ -10,6 +10,7 @@ use anyhow::anyhow;
use anyhow::Result;
use std::collections::HashSet;
use std::ffi::c_void;
use std::ffi::OsStr;
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
@@ -67,9 +68,9 @@ fn gather_candidates(cwd: &Path, env: &std::collections::HashMap<String, String>
.cloned()
.or_else(|| std::env::var("PATH").ok())
{
for part in path.split(std::path::MAIN_SEPARATOR) {
if !part.is_empty() {
unique_push(&mut set, &mut out, PathBuf::from(part));
for part in std::env::split_paths(OsStr::new(&path)) {
if !part.as_os_str().is_empty() {
unique_push(&mut set, &mut out, part);
}
}
}
@@ -298,3 +299,41 @@ pub fn apply_capability_denies_for_world_writable(
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::gather_candidates;
use std::collections::HashMap;
use std::fs;
#[test]
fn gathers_path_entries_by_list_separator() {
let tmp = tempfile::tempdir().expect("tempdir");
let dir_a = tmp.path().join("Tools");
let dir_b = tmp.path().join("Bin");
let dir_space = tmp.path().join("Program Files");
fs::create_dir_all(&dir_a).expect("dir a");
fs::create_dir_all(&dir_b).expect("dir b");
fs::create_dir_all(&dir_space).expect("dir space");
let mut env_map = HashMap::new();
env_map.insert(
"PATH".to_string(),
format!(
"{};{};{}",
dir_a.display(),
dir_b.display(),
dir_space.display()
),
);
let candidates = gather_candidates(tmp.path(), &env_map);
let canon_a = dir_a.canonicalize().expect("canon a");
let canon_b = dir_b.canonicalize().expect("canon b");
let canon_space = dir_space.canonicalize().expect("canon space");
assert!(candidates.contains(&canon_a));
assert!(candidates.contains(&canon_b));
assert!(candidates.contains(&canon_space));
}
}
+128 -11
View File
@@ -13,6 +13,7 @@ use std::fs::File;
use std::path::Path;
use std::path::PathBuf;
use windows_sys::Win32::Foundation::GetLastError;
use windows_sys::Win32::Foundation::LocalFree;
use windows_sys::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER;
use windows_sys::Win32::NetworkManagement::NetManagement::NERR_Success;
use windows_sys::Win32::NetworkManagement::NetManagement::NetLocalGroupAdd;
@@ -27,7 +28,10 @@ use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1;
use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_1003;
use windows_sys::Win32::NetworkManagement::NetManagement::USER_PRIV_USER;
use windows_sys::Win32::Security::Authorization::ConvertStringSidToSidW;
use windows_sys::Win32::Security::CopySid;
use windows_sys::Win32::Security::GetLengthSid;
use windows_sys::Win32::Security::LookupAccountNameW;
use windows_sys::Win32::Security::LookupAccountSidW;
use windows_sys::Win32::Security::SID_NAME_USE;
use codex_windows_sandbox::dpapi_protect;
@@ -38,6 +42,11 @@ use codex_windows_sandbox::SETUP_VERSION;
pub const SANDBOX_USERS_GROUP: &str = "CodexSandboxUsers";
const SANDBOX_USERS_GROUP_COMMENT: &str = "Codex sandbox internal group (managed)";
const SID_ADMINISTRATORS: &str = "S-1-5-32-544";
const SID_USERS: &str = "S-1-5-32-545";
const SID_AUTHENTICATED_USERS: &str = "S-1-5-11";
const SID_EVERYONE: &str = "S-1-1-0";
const SID_SYSTEM: &str = "S-1-5-18";
pub fn ensure_sandbox_users_group(log: &mut File) -> Result<()> {
ensure_local_group(SANDBOX_USERS_GROUP, SANDBOX_USERS_GROUP_COMMENT, log)
@@ -119,17 +128,24 @@ pub fn ensure_local_user(name: &str, password: &str, log: &mut File) -> Result<(
}
// Ensure the principal is a regular local user account.
let group = to_wide(OsStr::new("Users"));
let member = LOCALGROUP_MEMBERS_INFO_3 {
lgrmi3_domainandname: name_w.as_ptr() as *mut u16,
};
let _ = NetLocalGroupAddMembers(
std::ptr::null(),
group.as_ptr(),
3,
&member as *const _ as *mut u8,
1,
);
if let Ok(group_name) = lookup_account_name_for_sid(SID_USERS) {
let group = to_wide(OsStr::new(&group_name));
let member = LOCALGROUP_MEMBERS_INFO_3 {
lgrmi3_domainandname: name_w.as_ptr() as *mut u16,
};
let _ = NetLocalGroupAddMembers(
std::ptr::null(),
group.as_ptr(),
3,
&member as *const _ as *mut u8,
1,
);
} else {
super::log_line(
log,
"LookupAccountSidW failed for Users SID; skipping Users group membership",
)?;
}
}
Ok(())
}
@@ -184,6 +200,9 @@ pub fn ensure_local_group_member(group_name: &str, member_name: &str) -> Result<
}
pub fn resolve_sid(name: &str) -> Result<Vec<u8>> {
if let Some(sid_str) = well_known_sid_str(name) {
return sid_bytes_from_string(sid_str);
}
let name_w = to_wide(OsStr::new(name));
let mut sid_buffer = vec![0u8; 68];
let mut sid_len: u32 = sid_buffer.len() as u32;
@@ -218,6 +237,104 @@ pub fn resolve_sid(name: &str) -> Result<Vec<u8>> {
}
}
fn well_known_sid_str(name: &str) -> Option<&'static str> {
match name {
"Administrators" => Some(SID_ADMINISTRATORS),
"Users" => Some(SID_USERS),
"Authenticated Users" => Some(SID_AUTHENTICATED_USERS),
"Everyone" => Some(SID_EVERYONE),
"SYSTEM" => Some(SID_SYSTEM),
_ => None,
}
}
fn sid_bytes_from_string(sid_str: &str) -> Result<Vec<u8>> {
let sid_w = to_wide(OsStr::new(sid_str));
let mut psid: *mut c_void = std::ptr::null_mut();
if unsafe { ConvertStringSidToSidW(sid_w.as_ptr(), &mut psid) } == 0 {
return Err(anyhow::anyhow!(
"ConvertStringSidToSidW failed for {sid_str}: {}",
unsafe { GetLastError() }
));
}
let sid_len = unsafe { GetLengthSid(psid) };
if sid_len == 0 {
unsafe {
LocalFree(psid as _);
}
return Err(anyhow::anyhow!("GetLengthSid failed for {sid_str}"));
}
let mut out = vec![0u8; sid_len as usize];
let ok = unsafe { CopySid(sid_len, out.as_mut_ptr() as *mut c_void, psid) };
unsafe {
LocalFree(psid as _);
}
if ok == 0 {
return Err(anyhow::anyhow!("CopySid failed for {sid_str}"));
}
Ok(out)
}
fn lookup_account_name_for_sid(sid_str: &str) -> Result<String> {
let sid_w = to_wide(OsStr::new(sid_str));
let mut psid: *mut c_void = std::ptr::null_mut();
if unsafe { ConvertStringSidToSidW(sid_w.as_ptr(), &mut psid) } == 0 {
return Err(anyhow::anyhow!(
"ConvertStringSidToSidW failed for {sid_str}: {}",
unsafe { GetLastError() }
));
}
let mut name_len: u32 = 0;
let mut domain_len: u32 = 0;
let mut use_type: SID_NAME_USE = 0;
let ok = unsafe {
LookupAccountSidW(
std::ptr::null(),
psid,
std::ptr::null_mut(),
&mut name_len,
std::ptr::null_mut(),
&mut domain_len,
&mut use_type,
)
};
if ok == 0 {
let err = unsafe { GetLastError() };
if err != ERROR_INSUFFICIENT_BUFFER {
unsafe {
LocalFree(psid as _);
}
return Err(anyhow::anyhow!(
"LookupAccountSidW preflight failed for {sid_str}: {err}"
));
}
}
let mut name_buf: Vec<u16> = vec![0u16; name_len as usize];
let mut domain_buf: Vec<u16> = vec![0u16; domain_len as usize];
let ok = unsafe {
LookupAccountSidW(
std::ptr::null(),
psid,
name_buf.as_mut_ptr(),
&mut name_len,
domain_buf.as_mut_ptr(),
&mut domain_len,
&mut use_type,
)
};
unsafe {
LocalFree(psid as _);
}
if ok == 0 {
return Err(anyhow::anyhow!(
"LookupAccountSidW failed for {sid_str}: {}",
unsafe { GetLastError() }
));
}
let name = String::from_utf16_lossy(&name_buf);
Ok(name.trim_end_matches('\0').to_string())
}
pub fn sid_bytes_to_psid(sid: &[u8]) -> Result<*mut c_void> {
let sid_str = string_from_sid_bytes(sid).map_err(anyhow::Error::msg)?;
let sid_w = to_wide(OsStr::new(&sid_str));
@@ -26,7 +26,7 @@ use windows_sys::Win32::Security::CheckTokenMembership;
use windows_sys::Win32::Security::FreeSid;
use windows_sys::Win32::Security::SECURITY_NT_AUTHORITY;
pub const SETUP_VERSION: u32 = 3;
pub const SETUP_VERSION: u32 = 4;
pub const OFFLINE_USERNAME: &str = "CodexSandboxOffline";
pub const ONLINE_USERNAME: &str = "CodexSandboxOnline";
const SECURITY_BUILTIN_DOMAIN_RID: u32 = 0x0000_0020;