mirror of
https://github.com/openai/codex.git
synced 2026-11-03 19:53:25 +00:00
Merge 1e35665fc9 into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -4,6 +4,7 @@ In the codex-rs folder where the rust code lives:
|
||||
|
||||
- Crate names are prefixed with `codex-`. For example, the `core` folder's crate is named `codex-core`
|
||||
- When using format! and you can inline variables into {}, always do that.
|
||||
- Install any commands the repo relies on (for example `just`, `rg`, or `cargo-insta`) if they aren't already available before running instructions here.
|
||||
- Never add or modify any code related to `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` or `CODEX_SANDBOX_ENV_VAR`.
|
||||
- You operate in a sandbox where `CODEX_SANDBOX_NETWORK_DISABLED=1` will be set whenever you use the `shell` tool. Any existing code that uses `CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR` was authored with this fact in mind. It is often used to early exit out of tests that the author knew you would not be able to run given your sandbox limitations.
|
||||
- Similarly, when you spawn a process using Seatbelt (`/usr/bin/sandbox-exec`), `CODEX_SANDBOX=seatbelt` will be set on the child process. Integration tests that want to run Seatbelt themselves cannot be run under Seatbelt, so checks for `CODEX_SANDBOX=seatbelt` are also often used to early exit out of tests, as appropriate.
|
||||
|
||||
Generated
+1
@@ -929,6 +929,7 @@ dependencies = [
|
||||
"libc",
|
||||
"mcp-types",
|
||||
"once_cell",
|
||||
"owo-colors",
|
||||
"path-clean",
|
||||
"pathdiff",
|
||||
"pretty_assertions",
|
||||
|
||||
@@ -80,8 +80,15 @@ async fn run_command_under_sandbox(
|
||||
|
||||
let mut child = match sandbox_type {
|
||||
SandboxType::Seatbelt => {
|
||||
spawn_command_under_seatbelt(command, &config.sandbox_policy, cwd, stdio_policy, env)
|
||||
.await?
|
||||
spawn_command_under_seatbelt(
|
||||
command,
|
||||
cwd.clone(),
|
||||
&config.sandbox_policy,
|
||||
config.cwd.clone(),
|
||||
stdio_policy,
|
||||
env,
|
||||
)
|
||||
.await?
|
||||
}
|
||||
SandboxType::Landlock => {
|
||||
#[expect(clippy::expect_used)]
|
||||
@@ -91,8 +98,9 @@ async fn run_command_under_sandbox(
|
||||
spawn_command_under_linux_sandbox(
|
||||
codex_linux_sandbox_exe,
|
||||
command,
|
||||
&config.sandbox_policy,
|
||||
cwd,
|
||||
&config.sandbox_policy,
|
||||
config.cwd.clone(),
|
||||
stdio_policy,
|
||||
env,
|
||||
)
|
||||
|
||||
@@ -177,8 +177,11 @@ async fn cli_main(codex_linux_sandbox_exe: Option<PathBuf>) -> anyhow::Result<()
|
||||
root_config_overrides.clone(),
|
||||
);
|
||||
let usage = codex_tui::run_main(interactive, codex_linux_sandbox_exe).await?;
|
||||
if !usage.is_zero() {
|
||||
println!("{}", codex_core::protocol::FinalOutput::from(usage));
|
||||
if !usage.token_usage.is_zero() {
|
||||
println!(
|
||||
"{}",
|
||||
codex_core::protocol::FinalOutput::from(usage.token_usage)
|
||||
);
|
||||
}
|
||||
}
|
||||
Some(Subcommand::Exec(mut exec_cli)) => {
|
||||
|
||||
@@ -4,9 +4,9 @@ name = "codex-core"
|
||||
version = { workspace = true }
|
||||
|
||||
[lib]
|
||||
doctest = false
|
||||
name = "codex_core"
|
||||
path = "src/lib.rs"
|
||||
doctest = false
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
@@ -41,7 +41,12 @@ similar = "2.7.0"
|
||||
strum_macros = "0.27.2"
|
||||
tempfile = "3"
|
||||
thiserror = "2.0.16"
|
||||
time = { version = "0.3", features = ["formatting", "parsing", "local-offset", "macros"] }
|
||||
time = { version = "0.3", features = [
|
||||
"formatting",
|
||||
"parsing",
|
||||
"local-offset",
|
||||
"macros",
|
||||
] }
|
||||
tokio = { version = "1", features = [
|
||||
"io-std",
|
||||
"macros",
|
||||
|
||||
@@ -26,37 +26,41 @@ When using the planning tool:
|
||||
|
||||
## Codex CLI harness, sandboxing, and approvals
|
||||
|
||||
The Codex CLI harness supports several different sandboxing, and approval configurations that the user can choose from.
|
||||
The Codex CLI harness supports several different configurations for sandboxing and escalation approvals that the user can choose from.
|
||||
|
||||
Filesystem sandboxing defines which files can be read or written. The options are:
|
||||
- **read-only**: You can only read files.
|
||||
- **workspace-write**: You can read files. You can write to files in this folder, but not outside it.
|
||||
- **danger-full-access**: No filesystem sandboxing.
|
||||
Filesystem sandboxing defines which files can be read or written. The options for `sandbox_mode` are:
|
||||
- **read-only**: The sandbox only permits reading files.
|
||||
- **workspace-write**: The sandbox permits reading files, and editing files in `cwd` and `writable_roots`. Editing files in other directories requires approval.
|
||||
- **danger-full-access**: No filesystem sandboxing - all commands are permitted.
|
||||
|
||||
Network sandboxing defines whether network can be accessed without approval. Options are
|
||||
Network sandboxing defines whether network can be accessed without approval. Options for `network_access` are:
|
||||
- **restricted**: Requires approval
|
||||
- **enabled**: No approval needed
|
||||
|
||||
Approvals are your mechanism to get user consent to perform more privileged actions. Although they introduce friction to the user because your work is paused until the user responds, you should leverage them to accomplish your important work. Do not let these settings or the sandbox deter you from attempting to accomplish the user's task unless it is set to "never", in which case never ask for approvals.
|
||||
|
||||
Approval options are
|
||||
Approvals are your mechanism to get user consent to run shell commands without the sandbox. Possible configuration options for `approval_policy` are
|
||||
- **untrusted**: The harness will escalate most commands for user approval, apart from a limited allowlist of safe "read" commands.
|
||||
- **on-failure**: The harness will allow all commands to run in the sandbox (if enabled), and failures will be escalated to the user for approval to run again without the sandbox.
|
||||
- **on-request**: Commands will be run in the sandbox by default, and you can specify in your tool call if you want to escalate a command to run without sandboxing. (Note that this mode is not always available. If it is, you'll see parameters for it in the `shell` command description.)
|
||||
- **never**: This is a non-interactive mode where you may NEVER ask the user for approval to run commands. Instead, you must always persist and work around constraints to solve the task for the user. You MUST do your utmost best to finish the task and validate your work before yielding. If this mode is paired with `danger-full-access`, take advantage of it to deliver the best outcome for the user. Further, in this mode, your default testing philosophy is overridden: Even if you don't see local patterns for testing, you may add tests and scripts to validate your work. Just remove them before yielding.
|
||||
|
||||
When you are running with approvals `on-request`, and sandboxing enabled, here are scenarios where you'll need to request approval:
|
||||
- You need to run a command that writes to a directory that requires it (e.g. running tests that write to /tmp)
|
||||
When you are running with `approval_policy == on-request`, and sandboxing enabled, here are scenarios where you'll need to request approval:
|
||||
- You need to run a command that writes to a directory that requires it (e.g. running tests that write to /var)
|
||||
- You need to run a GUI app (e.g., open/xdg-open/osascript) to open browsers or files.
|
||||
- You are running sandboxed and need to run a command that requires network access (e.g. installing packages)
|
||||
- If you run a command that is important to solving the user's query, but it fails because of sandboxing, rerun the command with approval.
|
||||
- If you run a command that is important to solving the user's query, but it fails because of sandboxing, rerun the command with approval. ALWAYS proceed to use the `with_escalated_permissions` and `justification` parameters - do not message the user before requesting approval for the command.
|
||||
- You are about to take a potentially destructive action such as an `rm` or `git reset` that the user did not explicitly ask for
|
||||
- (for all of these, you should weigh alternative paths that do not require approval)
|
||||
|
||||
When sandboxing is set to read-only, you'll need to request approval for any command that isn't a read.
|
||||
When `sandbox_mode` is set to read-only, you'll need to request approval for any command that isn't a read.
|
||||
|
||||
You will be told what filesystem sandboxing, network sandboxing, and approval mode are active in a developer or user message. If you are not told about this, assume that you are running with workspace-write, network sandboxing enabled, and approval on-failure.
|
||||
|
||||
Although they introduce friction to the user because your work is paused until the user responds, you should leverage them when necessary to accomplish important work. If the completing the task requires escalated permissions, Do not let these settings or the sandbox deter you from attempting to accomplish the user's task unless it is set to "never", in which case never ask for approvals.
|
||||
|
||||
When requesting approval to execute a command that will require escalated privileges:
|
||||
- Provide the `with_escalated_permissions` parameter with the boolean value true
|
||||
- Include a short, 1 sentence explanation for why you need to enable `with_escalated_permissions` in the justification parameter
|
||||
|
||||
## Special user requests
|
||||
|
||||
- If the user makes a simple request (such as asking for the time) which you can fulfill by running a terminal command (such as `date`), you should do so.
|
||||
|
||||
@@ -34,13 +34,11 @@ pub struct Prompt {
|
||||
}
|
||||
|
||||
impl Prompt {
|
||||
pub(crate) fn get_full_instructions(&self, model: &ModelFamily) -> Cow<'_, str> {
|
||||
pub(crate) fn get_full_instructions<'a>(&'a self, model: &'a ModelFamily) -> Cow<'a, str> {
|
||||
let base = self
|
||||
.base_instructions_override
|
||||
.as_deref()
|
||||
.unwrap_or(model.base_instructions.deref());
|
||||
let mut sections: Vec<&str> = vec![base];
|
||||
|
||||
// When there are no custom instructions, add apply_patch_tool_instructions if:
|
||||
// - the model needs special instructions (4.1)
|
||||
// AND
|
||||
@@ -54,9 +52,10 @@ impl Prompt {
|
||||
&& model.needs_special_apply_patch_instructions
|
||||
&& !is_apply_patch_tool_present
|
||||
{
|
||||
sections.push(APPLY_PATCH_TOOL_INSTRUCTIONS);
|
||||
Cow::Owned(format!("{base}\n{APPLY_PATCH_TOOL_INSTRUCTIONS}"))
|
||||
} else {
|
||||
Cow::Borrowed(base)
|
||||
}
|
||||
Cow::Owned(sections.join("\n"))
|
||||
}
|
||||
|
||||
pub(crate) fn get_formatted_input(&self) -> Vec<ResponseItem> {
|
||||
|
||||
+92
-76
@@ -1,10 +1,9 @@
|
||||
use std::borrow::Cow;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::sync::MutexGuard;
|
||||
use std::sync::atomic::AtomicU64;
|
||||
use std::time::Duration;
|
||||
|
||||
@@ -31,6 +30,7 @@ use mcp_types::CallToolResult;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde_json;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::sync::oneshot;
|
||||
use tokio::task::AbortHandle;
|
||||
use tracing::debug;
|
||||
@@ -135,21 +135,6 @@ mod compact;
|
||||
use self::compact::build_compacted_history;
|
||||
use self::compact::collect_user_messages;
|
||||
|
||||
// A convenience extension trait for acquiring mutex locks where poisoning is
|
||||
// unrecoverable and should abort the program. This avoids scattered `.unwrap()`
|
||||
// calls on `lock()` while still surfacing a clear panic message when a lock is
|
||||
// poisoned.
|
||||
trait MutexExt<T> {
|
||||
fn lock_unchecked(&self) -> MutexGuard<'_, T>;
|
||||
}
|
||||
|
||||
impl<T> MutexExt<T> for Mutex<T> {
|
||||
fn lock_unchecked(&self) -> MutexGuard<'_, T> {
|
||||
#[expect(clippy::expect_used)]
|
||||
self.lock().expect("poisoned lock")
|
||||
}
|
||||
}
|
||||
|
||||
/// The high-level interface to the Codex system.
|
||||
/// It operates as a queue pair where you send submissions and receive events.
|
||||
pub struct Codex {
|
||||
@@ -272,7 +257,6 @@ struct State {
|
||||
pending_input: Vec<ResponseInputItem>,
|
||||
history: ConversationHistory,
|
||||
token_info: Option<TokenUsageInfo>,
|
||||
next_internal_sub_id: u64,
|
||||
}
|
||||
|
||||
/// Context for an initialized model agent
|
||||
@@ -298,6 +282,7 @@ pub(crate) struct Session {
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
user_shell: shell::Shell,
|
||||
show_raw_agent_reasoning: bool,
|
||||
next_internal_sub_id: AtomicU64,
|
||||
}
|
||||
|
||||
/// The context needed for a single turn of the conversation.
|
||||
@@ -500,6 +485,7 @@ impl Session {
|
||||
codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(),
|
||||
user_shell: default_shell,
|
||||
show_raw_agent_reasoning: config.show_raw_agent_reasoning,
|
||||
next_internal_sub_id: AtomicU64::new(0),
|
||||
});
|
||||
|
||||
// Dispatch the SessionConfiguredEvent first and then report any errors.
|
||||
@@ -528,16 +514,16 @@ impl Session {
|
||||
Ok((sess, turn_context))
|
||||
}
|
||||
|
||||
pub fn set_task(&self, task: AgentTask) {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
pub async fn set_task(&self, task: AgentTask) {
|
||||
let mut state = self.state.lock().await;
|
||||
if let Some(current_task) = state.current_task.take() {
|
||||
current_task.abort(TurnAbortReason::Replaced);
|
||||
}
|
||||
state.current_task = Some(task);
|
||||
}
|
||||
|
||||
pub fn remove_task(&self, sub_id: &str) {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
pub async fn remove_task(&self, sub_id: &str) {
|
||||
let mut state = self.state.lock().await;
|
||||
if let Some(task) = &state.current_task
|
||||
&& task.sub_id == sub_id
|
||||
{
|
||||
@@ -546,9 +532,9 @@ impl Session {
|
||||
}
|
||||
|
||||
fn next_internal_sub_id(&self) -> String {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
let id = state.next_internal_sub_id;
|
||||
state.next_internal_sub_id += 1;
|
||||
let id = self
|
||||
.next_internal_sub_id
|
||||
.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
|
||||
format!("auto-compact-{id}")
|
||||
}
|
||||
|
||||
@@ -571,7 +557,7 @@ impl Session {
|
||||
let reconstructed_history =
|
||||
self.reconstruct_history_from_rollout(turn_context, &rollout_items);
|
||||
if !reconstructed_history.is_empty() {
|
||||
self.record_into_history(&reconstructed_history);
|
||||
self.record_into_history(&reconstructed_history).await;
|
||||
}
|
||||
|
||||
// If persisting, persist all rollout items as-is (recorder filters)
|
||||
@@ -604,7 +590,7 @@ impl Session {
|
||||
let (tx_approve, rx_approve) = oneshot::channel();
|
||||
let event_id = sub_id.clone();
|
||||
let prev_entry = {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
let mut state = self.state.lock().await;
|
||||
state.pending_approvals.insert(sub_id, tx_approve)
|
||||
};
|
||||
if prev_entry.is_some() {
|
||||
@@ -636,7 +622,7 @@ impl Session {
|
||||
let (tx_approve, rx_approve) = oneshot::channel();
|
||||
let event_id = sub_id.clone();
|
||||
let prev_entry = {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
let mut state = self.state.lock().await;
|
||||
state.pending_approvals.insert(sub_id, tx_approve)
|
||||
};
|
||||
if prev_entry.is_some() {
|
||||
@@ -656,9 +642,9 @@ impl Session {
|
||||
rx_approve
|
||||
}
|
||||
|
||||
pub fn notify_approval(&self, sub_id: &str, decision: ReviewDecision) {
|
||||
pub async fn notify_approval(&self, sub_id: &str, decision: ReviewDecision) {
|
||||
let entry = {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
let mut state = self.state.lock().await;
|
||||
state.pending_approvals.remove(sub_id)
|
||||
};
|
||||
match entry {
|
||||
@@ -671,15 +657,15 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn add_approved_command(&self, cmd: Vec<String>) {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
pub async fn add_approved_command(&self, cmd: Vec<String>) {
|
||||
let mut state = self.state.lock().await;
|
||||
state.approved_commands.insert(cmd);
|
||||
}
|
||||
|
||||
/// Records input items: always append to conversation history and
|
||||
/// persist these response items to rollout.
|
||||
async fn record_conversation_items(&self, items: &[ResponseItem]) {
|
||||
self.record_into_history(items);
|
||||
self.record_into_history(items).await;
|
||||
self.persist_rollout_response_items(items).await;
|
||||
}
|
||||
|
||||
@@ -711,11 +697,9 @@ impl Session {
|
||||
}
|
||||
|
||||
/// Append ResponseItems to the in-memory conversation history only.
|
||||
fn record_into_history(&self, items: &[ResponseItem]) {
|
||||
self.state
|
||||
.lock_unchecked()
|
||||
.history
|
||||
.record_items(items.iter());
|
||||
async fn record_into_history(&self, items: &[ResponseItem]) {
|
||||
let mut state = self.state.lock().await;
|
||||
state.history.record_items(items.iter());
|
||||
}
|
||||
|
||||
async fn persist_rollout_response_items(&self, items: &[ResponseItem]) {
|
||||
@@ -743,8 +727,8 @@ impl Session {
|
||||
|
||||
async fn persist_rollout_items(&self, items: &[RolloutItem]) {
|
||||
let recorder = {
|
||||
let guard = self.rollout.lock_unchecked();
|
||||
guard.as_ref().cloned()
|
||||
let guard = self.rollout.lock().await;
|
||||
guard.clone()
|
||||
};
|
||||
if let Some(rec) = recorder
|
||||
&& let Err(e) = rec.record_items(items).await
|
||||
@@ -753,12 +737,12 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
fn update_token_usage_info(
|
||||
async fn update_token_usage_info(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
token_usage: &Option<TokenUsage>,
|
||||
) -> Option<TokenUsageInfo> {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
let mut state = self.state.lock().await;
|
||||
let info = TokenUsageInfo::new_or_append(
|
||||
&state.token_info,
|
||||
token_usage,
|
||||
@@ -915,6 +899,7 @@ impl Session {
|
||||
exec_args.params,
|
||||
exec_args.sandbox_type,
|
||||
exec_args.sandbox_policy,
|
||||
exec_args.sandbox_cwd,
|
||||
exec_args.codex_linux_sandbox_exe,
|
||||
exec_args.stdout_stream,
|
||||
)
|
||||
@@ -973,13 +958,17 @@ impl Session {
|
||||
|
||||
/// Build the full turn input by concatenating the current conversation
|
||||
/// history with additional items for this turn.
|
||||
pub fn turn_input_with_history(&self, extra: Vec<ResponseItem>) -> Vec<ResponseItem> {
|
||||
[self.state.lock_unchecked().history.contents(), extra].concat()
|
||||
pub async fn turn_input_with_history(&self, extra: Vec<ResponseItem>) -> Vec<ResponseItem> {
|
||||
let history = {
|
||||
let state = self.state.lock().await;
|
||||
state.history.contents()
|
||||
};
|
||||
[history, extra].concat()
|
||||
}
|
||||
|
||||
/// Returns the input if there was no task running to inject into
|
||||
pub fn inject_input(&self, input: Vec<InputItem>) -> Result<(), Vec<InputItem>> {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
pub async fn inject_input(&self, input: Vec<InputItem>) -> Result<(), Vec<InputItem>> {
|
||||
let mut state = self.state.lock().await;
|
||||
if state.current_task.is_some() {
|
||||
state.pending_input.push(input.into());
|
||||
Ok(())
|
||||
@@ -988,8 +977,8 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get_pending_input(&self) -> Vec<ResponseInputItem> {
|
||||
let mut state = self.state.lock_unchecked();
|
||||
pub async fn get_pending_input(&self) -> Vec<ResponseInputItem> {
|
||||
let mut state = self.state.lock().await;
|
||||
if state.pending_input.is_empty() {
|
||||
Vec::with_capacity(0)
|
||||
} else {
|
||||
@@ -1011,9 +1000,9 @@ impl Session {
|
||||
.await
|
||||
}
|
||||
|
||||
fn interrupt_task(&self) {
|
||||
pub async fn interrupt_task(&self) {
|
||||
info!("interrupt received: abort current task, if any");
|
||||
let mut state = self.state.lock_unchecked();
|
||||
let mut state = self.state.lock().await;
|
||||
state.pending_approvals.clear();
|
||||
state.pending_input.clear();
|
||||
if let Some(task) = state.current_task.take() {
|
||||
@@ -1021,6 +1010,16 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
fn interrupt_task_sync(&self) {
|
||||
if let Ok(mut state) = self.state.try_lock() {
|
||||
state.pending_approvals.clear();
|
||||
state.pending_input.clear();
|
||||
if let Some(task) = state.current_task.take() {
|
||||
task.abort(TurnAbortReason::Interrupted);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Spawn the configured notifier (if any) with the given JSON payload as
|
||||
/// the last argument. Failures are logged but otherwise ignored so that
|
||||
/// notification issues do not interfere with the main workflow.
|
||||
@@ -1053,7 +1052,7 @@ impl Session {
|
||||
|
||||
impl Drop for Session {
|
||||
fn drop(&mut self) {
|
||||
self.interrupt_task();
|
||||
self.interrupt_task_sync();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1184,7 +1183,7 @@ async fn submission_loop(
|
||||
debug!(?sub, "Submission");
|
||||
match sub.op {
|
||||
Op::Interrupt => {
|
||||
sess.interrupt_task();
|
||||
sess.interrupt_task().await;
|
||||
}
|
||||
Op::OverrideTurnContext {
|
||||
cwd,
|
||||
@@ -1277,11 +1276,11 @@ async fn submission_loop(
|
||||
}
|
||||
Op::UserInput { items } => {
|
||||
// attempt to inject input into current task
|
||||
if let Err(items) = sess.inject_input(items) {
|
||||
if let Err(items) = sess.inject_input(items).await {
|
||||
// no current task, spawn a new one
|
||||
let task =
|
||||
AgentTask::spawn(sess.clone(), Arc::clone(&turn_context), sub.id, items);
|
||||
sess.set_task(task);
|
||||
sess.set_task(task).await;
|
||||
}
|
||||
}
|
||||
Op::UserTurn {
|
||||
@@ -1294,7 +1293,7 @@ async fn submission_loop(
|
||||
summary,
|
||||
} => {
|
||||
// attempt to inject input into current task
|
||||
if let Err(items) = sess.inject_input(items) {
|
||||
if let Err(items) = sess.inject_input(items).await {
|
||||
// Derive a fresh TurnContext for this turn using the provided overrides.
|
||||
let provider = turn_context.client.get_provider();
|
||||
let auth_manager = turn_context.client.get_auth_manager();
|
||||
@@ -1360,20 +1359,20 @@ async fn submission_loop(
|
||||
// no current task, spawn a new one with the per‑turn context
|
||||
let task =
|
||||
AgentTask::spawn(sess.clone(), Arc::clone(&turn_context), sub.id, items);
|
||||
sess.set_task(task);
|
||||
sess.set_task(task).await;
|
||||
}
|
||||
}
|
||||
Op::ExecApproval { id, decision } => match decision {
|
||||
ReviewDecision::Abort => {
|
||||
sess.interrupt_task();
|
||||
sess.interrupt_task().await;
|
||||
}
|
||||
other => sess.notify_approval(&id, other),
|
||||
other => sess.notify_approval(&id, other).await,
|
||||
},
|
||||
Op::PatchApproval { id, decision } => match decision {
|
||||
ReviewDecision::Abort => {
|
||||
sess.interrupt_task();
|
||||
sess.interrupt_task().await;
|
||||
}
|
||||
other => sess.notify_approval(&id, other),
|
||||
other => sess.notify_approval(&id, other).await,
|
||||
},
|
||||
Op::AddToHistory { text } => {
|
||||
let id = sess.conversation_id;
|
||||
@@ -1452,15 +1451,19 @@ async fn submission_loop(
|
||||
}
|
||||
Op::Compact => {
|
||||
// Attempt to inject input into current task
|
||||
if let Err(items) = sess.inject_input(vec![InputItem::Text {
|
||||
text: compact::COMPACT_TRIGGER_TEXT.to_string(),
|
||||
}]) {
|
||||
if let Err(items) = sess
|
||||
.inject_input(vec![InputItem::Text {
|
||||
text: compact::COMPACT_TRIGGER_TEXT.to_string(),
|
||||
}])
|
||||
.await
|
||||
{
|
||||
compact::spawn_compact_task(
|
||||
sess.clone(),
|
||||
Arc::clone(&turn_context),
|
||||
sub.id,
|
||||
items,
|
||||
);
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
Op::Shutdown => {
|
||||
@@ -1468,7 +1471,10 @@ async fn submission_loop(
|
||||
|
||||
// Gracefully flush and shutdown rollout recorder on session end so tests
|
||||
// that inspect the rollout file do not race with the background writer.
|
||||
let recorder_opt = sess.rollout.lock_unchecked().take();
|
||||
let recorder_opt = {
|
||||
let mut guard = sess.rollout.lock().await;
|
||||
guard.take()
|
||||
};
|
||||
if let Some(rec) = recorder_opt
|
||||
&& let Err(e) = rec.shutdown().await
|
||||
{
|
||||
@@ -1493,7 +1499,7 @@ async fn submission_loop(
|
||||
let sub_id = sub.id.clone();
|
||||
// Flush rollout writes before returning the path so readers observe a consistent file.
|
||||
let (path, rec_opt) = {
|
||||
let guard = sess.rollout.lock_unchecked();
|
||||
let guard = sess.rollout.lock().await;
|
||||
match guard.as_ref() {
|
||||
Some(rec) => (rec.get_rollout_path(), Some(rec.clone())),
|
||||
None => {
|
||||
@@ -1604,7 +1610,7 @@ async fn spawn_review_thread(
|
||||
// Clone sub_id for the upcoming announcement before moving it into the task.
|
||||
let sub_id_for_event = sub_id.clone();
|
||||
let task = AgentTask::review(sess.clone(), tc.clone(), sub_id, input);
|
||||
sess.set_task(task);
|
||||
sess.set_task(task).await;
|
||||
|
||||
// Announce entering review mode so UIs can switch modes.
|
||||
sess.send_event(Event {
|
||||
@@ -1675,6 +1681,7 @@ async fn run_task(
|
||||
// may support this, the model might not.
|
||||
let pending_input = sess
|
||||
.get_pending_input()
|
||||
.await
|
||||
.into_iter()
|
||||
.map(ResponseItem::from)
|
||||
.collect::<Vec<ResponseItem>>();
|
||||
@@ -1696,7 +1703,7 @@ async fn run_task(
|
||||
review_thread_history.clone()
|
||||
} else {
|
||||
sess.record_conversation_items(&pending_input).await;
|
||||
sess.turn_input_with_history(pending_input)
|
||||
sess.turn_input_with_history(pending_input).await
|
||||
};
|
||||
|
||||
let turn_input_messages: Vec<String> = turn_input
|
||||
@@ -1908,7 +1915,7 @@ async fn run_task(
|
||||
.await;
|
||||
}
|
||||
|
||||
sess.remove_task(&sub_id);
|
||||
sess.remove_task(&sub_id).await;
|
||||
let event = Event {
|
||||
id: sub_id,
|
||||
msg: EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }),
|
||||
@@ -2141,7 +2148,9 @@ async fn try_run_turn(
|
||||
response_id: _,
|
||||
token_usage,
|
||||
} => {
|
||||
let info = sess.update_token_usage_info(turn_context, &token_usage);
|
||||
let info = sess
|
||||
.update_token_usage_info(turn_context, &token_usage)
|
||||
.await;
|
||||
let _ = sess
|
||||
.send_event(Event {
|
||||
id: sub_id.to_string(),
|
||||
@@ -2475,7 +2484,10 @@ async fn handle_function_call(
|
||||
}
|
||||
};
|
||||
let abs = turn_context.resolve_path(Some(args.path));
|
||||
let output = match sess.inject_input(vec![InputItem::LocalImage { path: abs }]) {
|
||||
let output = match sess
|
||||
.inject_input(vec![InputItem::LocalImage { path: abs }])
|
||||
.await
|
||||
{
|
||||
Ok(()) => FunctionCallOutputPayload {
|
||||
content: "attached local image path".to_string(),
|
||||
success: Some(true),
|
||||
@@ -2681,6 +2693,7 @@ pub struct ExecInvokeArgs<'a> {
|
||||
pub params: ExecParams,
|
||||
pub sandbox_type: SandboxType,
|
||||
pub sandbox_policy: &'a SandboxPolicy,
|
||||
pub sandbox_cwd: &'a Path,
|
||||
pub codex_linux_sandbox_exe: &'a Option<PathBuf>,
|
||||
pub stdout_stream: Option<StdoutStream>,
|
||||
}
|
||||
@@ -2789,7 +2802,7 @@ async fn handle_container_exec_with_params(
|
||||
}
|
||||
None => {
|
||||
let safety = {
|
||||
let state = sess.state.lock_unchecked();
|
||||
let state = sess.state.lock().await;
|
||||
assess_command_safety(
|
||||
¶ms.command,
|
||||
turn_context.approval_policy,
|
||||
@@ -2818,7 +2831,7 @@ async fn handle_container_exec_with_params(
|
||||
match rx_approve.await.unwrap_or_default() {
|
||||
ReviewDecision::Approved => (),
|
||||
ReviewDecision::ApprovedForSession => {
|
||||
sess.add_approved_command(params.command.clone());
|
||||
sess.add_approved_command(params.command.clone()).await;
|
||||
}
|
||||
ReviewDecision::Denied | ReviewDecision::Abort => {
|
||||
return ResponseInputItem::FunctionCallOutput {
|
||||
@@ -2872,6 +2885,7 @@ async fn handle_container_exec_with_params(
|
||||
params: params.clone(),
|
||||
sandbox_type,
|
||||
sandbox_policy: &turn_context.sandbox_policy,
|
||||
sandbox_cwd: &turn_context.cwd,
|
||||
codex_linux_sandbox_exe: &sess.codex_linux_sandbox_exe,
|
||||
stdout_stream: if exec_command_context.apply_patch.is_some() {
|
||||
None
|
||||
@@ -2991,7 +3005,7 @@ async fn handle_sandbox_error(
|
||||
// remainder of the session so future
|
||||
// executions skip the sandbox directly.
|
||||
// TODO(ragona): Isn't this a bug? It always saves the command in an | fork?
|
||||
sess.add_approved_command(params.command.clone());
|
||||
sess.add_approved_command(params.command.clone()).await;
|
||||
// Inform UI we are retrying without sandbox.
|
||||
sess.notify_background_event(&sub_id, "retrying command without sandbox")
|
||||
.await;
|
||||
@@ -3006,6 +3020,7 @@ async fn handle_sandbox_error(
|
||||
params,
|
||||
sandbox_type: SandboxType::None,
|
||||
sandbox_policy: &turn_context.sandbox_policy,
|
||||
sandbox_cwd: &turn_context.cwd,
|
||||
codex_linux_sandbox_exe: &sess.codex_linux_sandbox_exe,
|
||||
stdout_stream: if exec_command_context.apply_patch.is_some() {
|
||||
None
|
||||
@@ -3356,7 +3371,7 @@ mod tests {
|
||||
}),
|
||||
));
|
||||
|
||||
let actual = session.state.lock_unchecked().history.contents();
|
||||
let actual = tokio_test::block_on(async { session.state.lock().await.history.contents() });
|
||||
assert_eq!(expected, actual);
|
||||
}
|
||||
|
||||
@@ -3369,7 +3384,7 @@ mod tests {
|
||||
session.record_initial_history(&turn_context, InitialHistory::Forked(rollout_items)),
|
||||
);
|
||||
|
||||
let actual = session.state.lock_unchecked().history.contents();
|
||||
let actual = tokio_test::block_on(async { session.state.lock().await.history.contents() });
|
||||
assert_eq!(expected, actual);
|
||||
}
|
||||
|
||||
@@ -3611,6 +3626,7 @@ mod tests {
|
||||
codex_linux_sandbox_exe: None,
|
||||
user_shell: shell::Shell::Unknown,
|
||||
show_raw_agent_reasoning: config.show_raw_agent_reasoning,
|
||||
next_internal_sub_id: AtomicU64::new(0),
|
||||
};
|
||||
(session, turn_context)
|
||||
}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use super::AgentTask;
|
||||
use super::MutexExt;
|
||||
use super::Session;
|
||||
use super::TurnContext;
|
||||
use super::get_last_assistant_message_from_turn;
|
||||
@@ -37,7 +36,7 @@ struct HistoryBridgeTemplate<'a> {
|
||||
summary_text: &'a str,
|
||||
}
|
||||
|
||||
pub(super) fn spawn_compact_task(
|
||||
pub(super) async fn spawn_compact_task(
|
||||
sess: Arc<Session>,
|
||||
turn_context: Arc<TurnContext>,
|
||||
sub_id: String,
|
||||
@@ -50,7 +49,7 @@ pub(super) fn spawn_compact_task(
|
||||
input,
|
||||
SUMMARIZATION_PROMPT.to_string(),
|
||||
);
|
||||
sess.set_task(task);
|
||||
sess.set_task(task).await;
|
||||
}
|
||||
|
||||
pub(super) async fn run_inline_auto_compact_task(
|
||||
@@ -79,15 +78,29 @@ pub(super) async fn run_compact_task(
|
||||
input: Vec<InputItem>,
|
||||
compact_instructions: String,
|
||||
) {
|
||||
let start_event = Event {
|
||||
id: sub_id.clone(),
|
||||
msg: EventMsg::TaskStarted(TaskStartedEvent {
|
||||
model_context_window: turn_context.client.get_model_context_window(),
|
||||
}),
|
||||
};
|
||||
sess.send_event(start_event).await;
|
||||
run_compact_task_inner(
|
||||
sess,
|
||||
sess.clone(),
|
||||
turn_context,
|
||||
sub_id,
|
||||
sub_id.clone(),
|
||||
input,
|
||||
compact_instructions,
|
||||
true,
|
||||
)
|
||||
.await;
|
||||
let event = Event {
|
||||
id: sub_id,
|
||||
msg: EventMsg::TaskComplete(TaskCompleteEvent {
|
||||
last_agent_message: None,
|
||||
}),
|
||||
};
|
||||
sess.send_event(event).await;
|
||||
}
|
||||
|
||||
async fn run_compact_task_inner(
|
||||
@@ -98,18 +111,11 @@ async fn run_compact_task_inner(
|
||||
compact_instructions: String,
|
||||
remove_task_on_completion: bool,
|
||||
) {
|
||||
let model_context_window = turn_context.client.get_model_context_window();
|
||||
let start_event = Event {
|
||||
id: sub_id.clone(),
|
||||
msg: EventMsg::TaskStarted(TaskStartedEvent {
|
||||
model_context_window,
|
||||
}),
|
||||
};
|
||||
sess.send_event(start_event).await;
|
||||
|
||||
let initial_input_for_turn: ResponseInputItem = ResponseInputItem::from(input);
|
||||
let instructions_override = compact_instructions;
|
||||
let turn_input = sess.turn_input_with_history(vec![initial_input_for_turn.clone().into()]);
|
||||
let turn_input = sess
|
||||
.turn_input_with_history(vec![initial_input_for_turn.clone().into()])
|
||||
.await;
|
||||
|
||||
let prompt = Prompt {
|
||||
input: turn_input,
|
||||
@@ -168,10 +174,10 @@ async fn run_compact_task_inner(
|
||||
}
|
||||
|
||||
if remove_task_on_completion {
|
||||
sess.remove_task(&sub_id);
|
||||
sess.remove_task(&sub_id).await;
|
||||
}
|
||||
let history_snapshot = {
|
||||
let state = sess.state.lock_unchecked();
|
||||
let state = sess.state.lock().await;
|
||||
state.history.contents()
|
||||
};
|
||||
let summary_text = get_last_assistant_message_from_turn(&history_snapshot).unwrap_or_default();
|
||||
@@ -179,7 +185,7 @@ async fn run_compact_task_inner(
|
||||
let initial_context = sess.build_initial_context(turn_context.as_ref());
|
||||
let new_history = build_compacted_history(initial_context, &user_messages, &summary_text);
|
||||
{
|
||||
let mut state = sess.state.lock_unchecked();
|
||||
let mut state = sess.state.lock().await;
|
||||
state.history.replace(new_history);
|
||||
}
|
||||
|
||||
@@ -195,13 +201,6 @@ async fn run_compact_task_inner(
|
||||
}),
|
||||
};
|
||||
sess.send_event(event).await;
|
||||
let event = Event {
|
||||
id: sub_id.clone(),
|
||||
msg: EventMsg::TaskComplete(TaskCompleteEvent {
|
||||
last_agent_message: None,
|
||||
}),
|
||||
};
|
||||
sess.send_event(event).await;
|
||||
}
|
||||
|
||||
fn content_items_to_text(content: &[ContentItem]) -> Option<String> {
|
||||
@@ -290,7 +289,7 @@ async fn drain_to_completed(
|
||||
};
|
||||
match event {
|
||||
Ok(ResponseEvent::OutputItemDone(item)) => {
|
||||
let mut state = sess.state.lock_unchecked();
|
||||
let mut state = sess.state.lock().await;
|
||||
state.history.record_items(std::slice::from_ref(&item));
|
||||
}
|
||||
Ok(ResponseEvent::Completed { .. }) => {
|
||||
|
||||
@@ -1163,6 +1163,7 @@ pub fn log_dir(cfg: &Config) -> std::io::Result<PathBuf> {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::config_types::HistoryPersistence;
|
||||
use crate::config_types::Notifications;
|
||||
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -1201,6 +1202,19 @@ persistence = "none"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tui_config_missing_notifications_field_defaults_to_disabled() {
|
||||
let cfg = r#"
|
||||
[tui]
|
||||
"#;
|
||||
|
||||
let parsed = toml::from_str::<ConfigToml>(cfg)
|
||||
.expect("TUI config without notifications should succeed");
|
||||
let tui = parsed.tui.expect("config should include tui section");
|
||||
|
||||
assert_eq!(tui.notifications, Notifications::Enabled(false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sandbox_config_parsing() {
|
||||
let sandbox_full_access = r#"
|
||||
|
||||
@@ -94,6 +94,7 @@ impl Default for Notifications {
|
||||
pub struct Tui {
|
||||
/// Enable desktop notifications from the TUI when the terminal is unfocused.
|
||||
/// Defaults to `false`.
|
||||
#[serde(default)]
|
||||
pub notifications: Notifications,
|
||||
}
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ use std::os::unix::process::ExitStatusExt;
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::ExitStatus;
|
||||
use std::time::Duration;
|
||||
@@ -82,6 +83,7 @@ pub async fn process_exec_tool_call(
|
||||
params: ExecParams,
|
||||
sandbox_type: SandboxType,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
sandbox_cwd: &Path,
|
||||
codex_linux_sandbox_exe: &Option<PathBuf>,
|
||||
stdout_stream: Option<StdoutStream>,
|
||||
) -> Result<ExecToolCallOutput> {
|
||||
@@ -94,12 +96,16 @@ pub async fn process_exec_tool_call(
|
||||
SandboxType::None => exec(params, sandbox_policy, stdout_stream.clone()).await,
|
||||
SandboxType::MacosSeatbelt => {
|
||||
let ExecParams {
|
||||
command, cwd, env, ..
|
||||
command,
|
||||
cwd: command_cwd,
|
||||
env,
|
||||
..
|
||||
} = params;
|
||||
let child = spawn_command_under_seatbelt(
|
||||
command,
|
||||
command_cwd,
|
||||
sandbox_policy,
|
||||
cwd,
|
||||
sandbox_cwd.to_path_buf(),
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
env,
|
||||
)
|
||||
@@ -108,7 +114,10 @@ pub async fn process_exec_tool_call(
|
||||
}
|
||||
SandboxType::LinuxSeccomp => {
|
||||
let ExecParams {
|
||||
command, cwd, env, ..
|
||||
command,
|
||||
cwd: command_cwd,
|
||||
env,
|
||||
..
|
||||
} = params;
|
||||
|
||||
let codex_linux_sandbox_exe = codex_linux_sandbox_exe
|
||||
@@ -117,8 +126,9 @@ pub async fn process_exec_tool_call(
|
||||
let child = spawn_command_under_linux_sandbox(
|
||||
codex_linux_sandbox_exe,
|
||||
command,
|
||||
command_cwd,
|
||||
sandbox_policy,
|
||||
cwd,
|
||||
sandbox_cwd.to_path_buf(),
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
env,
|
||||
)
|
||||
|
||||
@@ -16,21 +16,23 @@ use tokio::process::Child;
|
||||
pub async fn spawn_command_under_linux_sandbox<P>(
|
||||
codex_linux_sandbox_exe: P,
|
||||
command: Vec<String>,
|
||||
command_cwd: PathBuf,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
cwd: PathBuf,
|
||||
sandbox_policy_cwd: PathBuf,
|
||||
stdio_policy: StdioPolicy,
|
||||
env: HashMap<String, String>,
|
||||
) -> std::io::Result<Child>
|
||||
where
|
||||
P: AsRef<Path>,
|
||||
{
|
||||
let args = create_linux_sandbox_command_args(command, sandbox_policy, &cwd);
|
||||
let args =
|
||||
create_linux_sandbox_command_args(command, sandbox_policy, sandbox_policy_cwd.as_path());
|
||||
let arg0 = Some("codex-linux-sandbox");
|
||||
spawn_child_async(
|
||||
codex_linux_sandbox_exe.as_ref().to_path_buf(),
|
||||
args,
|
||||
arg0,
|
||||
cwd,
|
||||
command_cwd,
|
||||
sandbox_policy,
|
||||
stdio_policy,
|
||||
env,
|
||||
@@ -42,10 +44,13 @@ where
|
||||
fn create_linux_sandbox_command_args(
|
||||
command: Vec<String>,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
cwd: &Path,
|
||||
sandbox_policy_cwd: &Path,
|
||||
) -> Vec<String> {
|
||||
#[expect(clippy::expect_used)]
|
||||
let sandbox_policy_cwd = cwd.to_str().expect("cwd must be valid UTF-8").to_string();
|
||||
let sandbox_policy_cwd = sandbox_policy_cwd
|
||||
.to_str()
|
||||
.expect("cwd must be valid UTF-8")
|
||||
.to_string();
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
let sandbox_policy_json =
|
||||
|
||||
@@ -333,7 +333,7 @@ pub(crate) struct ApplyPatchToolArgs {
|
||||
/// Responses API:
|
||||
/// https://platform.openai.com/docs/guides/function-calling?api-mode=responses
|
||||
pub fn create_tools_json_for_responses_api(
|
||||
tools: &Vec<OpenAiTool>,
|
||||
tools: &[OpenAiTool],
|
||||
) -> crate::error::Result<Vec<serde_json::Value>> {
|
||||
let mut tools_json = Vec::new();
|
||||
|
||||
@@ -348,7 +348,7 @@ pub fn create_tools_json_for_responses_api(
|
||||
/// Chat Completions API:
|
||||
/// https://platform.openai.com/docs/guides/function-calling?api-mode=chat
|
||||
pub(crate) fn create_tools_json_for_chat_completions_api(
|
||||
tools: &Vec<OpenAiTool>,
|
||||
tools: &[OpenAiTool],
|
||||
) -> crate::error::Result<Vec<serde_json::Value>> {
|
||||
// We start with the JSON for the Responses API and than rewrite it to match
|
||||
// the chat completions tool call format.
|
||||
|
||||
@@ -18,19 +18,20 @@ const MACOS_PATH_TO_SEATBELT_EXECUTABLE: &str = "/usr/bin/sandbox-exec";
|
||||
|
||||
pub async fn spawn_command_under_seatbelt(
|
||||
command: Vec<String>,
|
||||
command_cwd: PathBuf,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
cwd: PathBuf,
|
||||
sandbox_policy_cwd: PathBuf,
|
||||
stdio_policy: StdioPolicy,
|
||||
mut env: HashMap<String, String>,
|
||||
) -> std::io::Result<Child> {
|
||||
let args = create_seatbelt_command_args(command, sandbox_policy, &cwd);
|
||||
let args = create_seatbelt_command_args(command, sandbox_policy, sandbox_policy_cwd.as_path());
|
||||
let arg0 = None;
|
||||
env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string());
|
||||
spawn_child_async(
|
||||
PathBuf::from(MACOS_PATH_TO_SEATBELT_EXECUTABLE),
|
||||
args,
|
||||
arg0,
|
||||
cwd,
|
||||
command_cwd,
|
||||
sandbox_policy,
|
||||
stdio_policy,
|
||||
env,
|
||||
@@ -41,7 +42,7 @@ pub async fn spawn_command_under_seatbelt(
|
||||
fn create_seatbelt_command_args(
|
||||
command: Vec<String>,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
cwd: &Path,
|
||||
sandbox_policy_cwd: &Path,
|
||||
) -> Vec<String> {
|
||||
let (file_write_policy, extra_cli_args) = {
|
||||
if sandbox_policy.has_full_disk_write_access() {
|
||||
@@ -51,7 +52,7 @@ fn create_seatbelt_command_args(
|
||||
Vec::<String>::new(),
|
||||
)
|
||||
} else {
|
||||
let writable_roots = sandbox_policy.get_writable_roots_with_cwd(cwd);
|
||||
let writable_roots = sandbox_policy.get_writable_roots_with_cwd(sandbox_policy_cwd);
|
||||
|
||||
let mut writable_folder_policies: Vec<String> = Vec::new();
|
||||
let mut cli_args: Vec<String> = Vec::new();
|
||||
|
||||
+16
-10
@@ -32,15 +32,19 @@ pub enum Shell {
|
||||
impl Shell {
|
||||
pub fn format_default_shell_invocation(&self, command: Vec<String>) -> Option<Vec<String>> {
|
||||
match self {
|
||||
Shell::Zsh(zsh) => {
|
||||
format_shell_invocation_with_rc(&command, &zsh.shell_path, &zsh.zshrc_path)
|
||||
}
|
||||
Shell::Bash(bash) => {
|
||||
format_shell_invocation_with_rc(&command, &bash.shell_path, &bash.bashrc_path)
|
||||
}
|
||||
Shell::Zsh(zsh) => format_shell_invocation_with_rc(
|
||||
command.as_slice(),
|
||||
&zsh.shell_path,
|
||||
&zsh.zshrc_path,
|
||||
),
|
||||
Shell::Bash(bash) => format_shell_invocation_with_rc(
|
||||
command.as_slice(),
|
||||
&bash.shell_path,
|
||||
&bash.bashrc_path,
|
||||
),
|
||||
Shell::PowerShell(ps) => {
|
||||
// If model generated a bash command, prefer a detected bash fallback
|
||||
if let Some(script) = strip_bash_lc(&command) {
|
||||
if let Some(script) = strip_bash_lc(command.as_slice()) {
|
||||
return match &ps.bash_exe_fallback {
|
||||
Some(bash) => Some(vec![
|
||||
bash.to_string_lossy().to_string(),
|
||||
@@ -102,7 +106,7 @@ impl Shell {
|
||||
}
|
||||
|
||||
fn format_shell_invocation_with_rc(
|
||||
command: &Vec<String>,
|
||||
command: &[String],
|
||||
shell_path: &str,
|
||||
rc_path: &str,
|
||||
) -> Option<Vec<String>> {
|
||||
@@ -118,8 +122,8 @@ fn format_shell_invocation_with_rc(
|
||||
Some(vec![shell_path.to_string(), "-lc".to_string(), rc_command])
|
||||
}
|
||||
|
||||
fn strip_bash_lc(command: &Vec<String>) -> Option<String> {
|
||||
match command.as_slice() {
|
||||
fn strip_bash_lc(command: &[String]) -> Option<String> {
|
||||
match command {
|
||||
// exactly three items
|
||||
[first, second, third]
|
||||
// first two must be "bash", "-lc"
|
||||
@@ -345,6 +349,7 @@ mod tests {
|
||||
},
|
||||
SandboxType::None,
|
||||
&SandboxPolicy::DangerFullAccess,
|
||||
temp_home.path(),
|
||||
&None,
|
||||
None,
|
||||
)
|
||||
@@ -451,6 +456,7 @@ mod macos_tests {
|
||||
},
|
||||
SandboxType::None,
|
||||
&SandboxPolicy::DangerFullAccess,
|
||||
temp_home.path(),
|
||||
&None,
|
||||
None,
|
||||
)
|
||||
|
||||
@@ -557,6 +557,7 @@ mod tests {
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
#[ignore] // Ignored while we have a better way to test this.
|
||||
async fn requests_with_large_timeout_are_capped() -> Result<(), UnifiedExecError> {
|
||||
let manager = UnifiedExecSessionManager::default();
|
||||
|
||||
@@ -578,6 +579,7 @@ mod tests {
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
#[ignore] // Ignored while we have a better way to test this.
|
||||
async fn completed_commands_do_not_persist_sessions() -> Result<(), UnifiedExecError> {
|
||||
let manager = UnifiedExecSessionManager::default();
|
||||
let result = manager
|
||||
|
||||
@@ -861,8 +861,18 @@ async fn auto_compact_allows_multiple_attempts_when_interleaved_with_other_turn_
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let mut auto_compact_lifecycle_events = Vec::new();
|
||||
loop {
|
||||
let event = codex.next_event().await.unwrap();
|
||||
if event.id.starts_with("auto-compact-")
|
||||
&& matches!(
|
||||
event.msg,
|
||||
EventMsg::TaskStarted(_) | EventMsg::TaskComplete(_)
|
||||
)
|
||||
{
|
||||
auto_compact_lifecycle_events.push(event);
|
||||
continue;
|
||||
}
|
||||
if let EventMsg::TaskComplete(_) = &event.msg
|
||||
&& !event.id.starts_with("auto-compact-")
|
||||
{
|
||||
@@ -870,6 +880,11 @@ async fn auto_compact_allows_multiple_attempts_when_interleaved_with_other_turn_
|
||||
}
|
||||
}
|
||||
|
||||
assert!(
|
||||
auto_compact_lifecycle_events.is_empty(),
|
||||
"auto compact should not emit task lifecycle events"
|
||||
);
|
||||
|
||||
let request_bodies: Vec<String> = responder
|
||||
.recorded_requests()
|
||||
.into_iter()
|
||||
|
||||
@@ -39,7 +39,7 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result<ExecToolCallOutput
|
||||
|
||||
let policy = SandboxPolicy::new_read_only_policy();
|
||||
|
||||
process_exec_tool_call(params, sandbox_type, &policy, &None, None).await
|
||||
process_exec_tool_call(params, sandbox_type, &policy, tmp.path(), &None, None).await
|
||||
}
|
||||
|
||||
/// Command succeeds with exit code 0 normally
|
||||
|
||||
@@ -49,9 +49,10 @@ async fn test_exec_stdout_stream_events_echo() {
|
||||
"printf 'hello-world\n'".to_string(),
|
||||
];
|
||||
|
||||
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
|
||||
let params = ExecParams {
|
||||
command: cmd,
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
cwd: cwd.clone(),
|
||||
timeout_ms: Some(5_000),
|
||||
env: HashMap::new(),
|
||||
with_escalated_permissions: None,
|
||||
@@ -64,6 +65,7 @@ async fn test_exec_stdout_stream_events_echo() {
|
||||
params,
|
||||
SandboxType::None,
|
||||
&policy,
|
||||
cwd.as_path(),
|
||||
&None,
|
||||
Some(stdout_stream),
|
||||
)
|
||||
@@ -99,9 +101,10 @@ async fn test_exec_stderr_stream_events_echo() {
|
||||
"printf 'oops\n' 1>&2".to_string(),
|
||||
];
|
||||
|
||||
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
|
||||
let params = ExecParams {
|
||||
command: cmd,
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
cwd: cwd.clone(),
|
||||
timeout_ms: Some(5_000),
|
||||
env: HashMap::new(),
|
||||
with_escalated_permissions: None,
|
||||
@@ -114,6 +117,7 @@ async fn test_exec_stderr_stream_events_echo() {
|
||||
params,
|
||||
SandboxType::None,
|
||||
&policy,
|
||||
cwd.as_path(),
|
||||
&None,
|
||||
Some(stdout_stream),
|
||||
)
|
||||
@@ -152,9 +156,10 @@ async fn test_aggregated_output_interleaves_in_order() {
|
||||
"printf 'O1\\n'; sleep 0.01; printf 'E1\\n' 1>&2; sleep 0.01; printf 'O2\\n'; sleep 0.01; printf 'E2\\n' 1>&2".to_string(),
|
||||
];
|
||||
|
||||
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
|
||||
let params = ExecParams {
|
||||
command: cmd,
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
cwd: cwd.clone(),
|
||||
timeout_ms: Some(5_000),
|
||||
env: HashMap::new(),
|
||||
with_escalated_permissions: None,
|
||||
@@ -163,9 +168,16 @@ async fn test_aggregated_output_interleaves_in_order() {
|
||||
|
||||
let policy = SandboxPolicy::new_read_only_policy();
|
||||
|
||||
let result = process_exec_tool_call(params, SandboxType::None, &policy, &None, None)
|
||||
.await
|
||||
.expect("process_exec_tool_call");
|
||||
let result = process_exec_tool_call(
|
||||
params,
|
||||
SandboxType::None,
|
||||
&policy,
|
||||
cwd.as_path(),
|
||||
&None,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
.expect("process_exec_tool_call");
|
||||
|
||||
assert_eq!(result.exit_code, 0);
|
||||
assert_eq!(result.stdout.text, "O1\nO2\n");
|
||||
@@ -182,9 +194,10 @@ async fn test_exec_timeout_returns_partial_output() {
|
||||
"printf 'before\\n'; sleep 2; printf 'after\\n'".to_string(),
|
||||
];
|
||||
|
||||
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
|
||||
let params = ExecParams {
|
||||
command: cmd,
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
cwd: cwd.clone(),
|
||||
timeout_ms: Some(200),
|
||||
env: HashMap::new(),
|
||||
with_escalated_permissions: None,
|
||||
@@ -193,7 +206,15 @@ async fn test_exec_timeout_returns_partial_output() {
|
||||
|
||||
let policy = SandboxPolicy::new_read_only_policy();
|
||||
|
||||
let result = process_exec_tool_call(params, SandboxType::None, &policy, &None, None).await;
|
||||
let result = process_exec_tool_call(
|
||||
params,
|
||||
SandboxType::None,
|
||||
&policy,
|
||||
cwd.as_path(),
|
||||
&None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
|
||||
let Err(CodexErr::Sandbox(SandboxErr::Timeout { output })) = result else {
|
||||
panic!("expected timeout error");
|
||||
|
||||
@@ -171,6 +171,8 @@ async fn python_getpwuid_works_under_seatbelt() {
|
||||
|
||||
// ReadOnly is sufficient here since we are only exercising user lookup.
|
||||
let policy = SandboxPolicy::ReadOnly;
|
||||
let sandbox_cwd = std::env::current_dir().expect("should be able to get current dir");
|
||||
let command_cwd = sandbox_cwd.clone();
|
||||
|
||||
let mut child = spawn_command_under_seatbelt(
|
||||
vec![
|
||||
@@ -179,8 +181,9 @@ async fn python_getpwuid_works_under_seatbelt() {
|
||||
// Print the passwd struct; success implies lookup worked.
|
||||
"import pwd, os; print(pwd.getpwuid(os.getuid()))".to_string(),
|
||||
],
|
||||
command_cwd,
|
||||
&policy,
|
||||
std::env::current_dir().expect("should be able to get current dir"),
|
||||
sandbox_cwd,
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
HashMap::new(),
|
||||
)
|
||||
@@ -216,13 +219,16 @@ fn create_test_scenario(tmp: &TempDir) -> TestScenario {
|
||||
/// Note that `path` must be absolute.
|
||||
async fn touch(path: &Path, policy: &SandboxPolicy) -> bool {
|
||||
assert!(path.is_absolute(), "Path must be absolute: {path:?}");
|
||||
let sandbox_cwd = std::env::current_dir().expect("should be able to get current dir");
|
||||
let command_cwd = sandbox_cwd.clone();
|
||||
let mut child = spawn_command_under_seatbelt(
|
||||
vec![
|
||||
"/usr/bin/touch".to_string(),
|
||||
path.to_string_lossy().to_string(),
|
||||
],
|
||||
command_cwd,
|
||||
policy,
|
||||
std::env::current_dir().expect("should be able to get current dir"),
|
||||
sandbox_cwd,
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
HashMap::new(),
|
||||
)
|
||||
|
||||
@@ -4,27 +4,39 @@ use codex_core::spawn::StdioPolicy;
|
||||
use std::collections::HashMap;
|
||||
use std::future::Future;
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::process::ExitStatus;
|
||||
use tokio::fs::create_dir_all;
|
||||
use tokio::process::Child;
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
async fn spawn_command_under_sandbox(
|
||||
command: Vec<String>,
|
||||
command_cwd: PathBuf,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
cwd: PathBuf,
|
||||
sandbox_cwd: &Path,
|
||||
stdio_policy: StdioPolicy,
|
||||
env: HashMap<String, String>,
|
||||
) -> std::io::Result<Child> {
|
||||
use codex_core::seatbelt::spawn_command_under_seatbelt;
|
||||
spawn_command_under_seatbelt(command, sandbox_policy, cwd, stdio_policy, env).await
|
||||
spawn_command_under_seatbelt(
|
||||
command,
|
||||
command_cwd,
|
||||
sandbox_policy,
|
||||
sandbox_cwd.to_path_buf(),
|
||||
stdio_policy,
|
||||
env,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
async fn spawn_command_under_sandbox(
|
||||
command: Vec<String>,
|
||||
command_cwd: PathBuf,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
cwd: PathBuf,
|
||||
sandbox_cwd: &Path,
|
||||
stdio_policy: StdioPolicy,
|
||||
env: HashMap<String, String>,
|
||||
) -> std::io::Result<Child> {
|
||||
@@ -33,8 +45,9 @@ async fn spawn_command_under_sandbox(
|
||||
spawn_command_under_linux_sandbox(
|
||||
codex_linux_sandbox_exe,
|
||||
command,
|
||||
command_cwd,
|
||||
sandbox_policy,
|
||||
cwd,
|
||||
sandbox_cwd.to_path_buf(),
|
||||
stdio_policy,
|
||||
env,
|
||||
)
|
||||
@@ -74,15 +87,18 @@ if __name__ == '__main__':
|
||||
p.join()
|
||||
"#;
|
||||
|
||||
let command_cwd = std::env::current_dir().expect("should be able to get current dir");
|
||||
let sandbox_cwd = command_cwd.clone();
|
||||
let mut child = spawn_command_under_sandbox(
|
||||
vec![
|
||||
"python3".to_string(),
|
||||
"-c".to_string(),
|
||||
python_code.to_string(),
|
||||
],
|
||||
command_cwd,
|
||||
&policy,
|
||||
std::env::current_dir().expect("should be able to get current dir"),
|
||||
StdioPolicy::Inherit,
|
||||
sandbox_cwd.as_path(),
|
||||
StdioPolicy::RedirectForShellTool,
|
||||
HashMap::new(),
|
||||
)
|
||||
.await
|
||||
@@ -92,6 +108,92 @@ if __name__ == '__main__':
|
||||
assert!(status.success(), "python exited with {status:?}");
|
||||
}
|
||||
|
||||
#[cfg_attr(
|
||||
target_os = "macos",
|
||||
ignore = "Seatbelt does not yet honor distinct sandbox cwd in this scenario"
|
||||
)]
|
||||
#[tokio::test]
|
||||
async fn sandbox_distinguishes_command_and_policy_cwds() {
|
||||
let temp = tempfile::tempdir().expect("should be able to create temp dir");
|
||||
let sandbox_root = temp.path().join("sandbox");
|
||||
let command_root = temp.path().join("command");
|
||||
create_dir_all(&sandbox_root).await.expect("mkdir");
|
||||
create_dir_all(&command_root).await.expect("mkdir");
|
||||
let canonical_sandbox_root = tokio::fs::canonicalize(&sandbox_root)
|
||||
.await
|
||||
.expect("canonicalize sandbox root");
|
||||
let canonical_allowed_path = canonical_sandbox_root.join("allowed.txt");
|
||||
|
||||
let disallowed_path = command_root.join("forbidden.txt");
|
||||
|
||||
// Note writable_roots is empty: verify that `canonical_allowed_path` is
|
||||
// writable only because it is under the sandbox policy cwd, not because it
|
||||
// is under a writable root.
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
// Attempt to write inside the command cwd, which is outside of the sandbox policy cwd.
|
||||
let mut child = spawn_command_under_sandbox(
|
||||
vec![
|
||||
"bash".to_string(),
|
||||
"-lc".to_string(),
|
||||
"echo forbidden > forbidden.txt".to_string(),
|
||||
],
|
||||
command_root.clone(),
|
||||
&policy,
|
||||
canonical_sandbox_root.as_path(),
|
||||
StdioPolicy::Inherit,
|
||||
HashMap::new(),
|
||||
)
|
||||
.await
|
||||
.expect("should spawn command writing to forbidden path");
|
||||
|
||||
let status = child
|
||||
.wait()
|
||||
.await
|
||||
.expect("should wait for forbidden command");
|
||||
assert!(
|
||||
!status.success(),
|
||||
"sandbox unexpectedly allowed writing to command cwd: {status:?}"
|
||||
);
|
||||
let forbidden_exists = tokio::fs::try_exists(&disallowed_path)
|
||||
.await
|
||||
.expect("try_exists failed");
|
||||
assert!(
|
||||
!forbidden_exists,
|
||||
"forbidden path should not have been created"
|
||||
);
|
||||
|
||||
// Writing to the sandbox policy cwd after changing directories into it should succeed.
|
||||
let mut child = spawn_command_under_sandbox(
|
||||
vec![
|
||||
"/usr/bin/touch".to_string(),
|
||||
canonical_allowed_path.to_string_lossy().into_owned(),
|
||||
],
|
||||
command_root,
|
||||
&policy,
|
||||
canonical_sandbox_root.as_path(),
|
||||
StdioPolicy::Inherit,
|
||||
HashMap::new(),
|
||||
)
|
||||
.await
|
||||
.expect("should spawn command writing to sandbox root");
|
||||
|
||||
let status = child.wait().await.expect("should wait for allowed command");
|
||||
assert!(
|
||||
status.success(),
|
||||
"sandbox blocked allowed write: {status:?}"
|
||||
);
|
||||
let allowed_exists = tokio::fs::try_exists(&canonical_allowed_path)
|
||||
.await
|
||||
.expect("try_exists allowed failed");
|
||||
assert!(allowed_exists, "allowed path should exist");
|
||||
}
|
||||
|
||||
fn unix_sock_body() {
|
||||
unsafe {
|
||||
let mut fds = [0i32; 2];
|
||||
@@ -200,10 +302,13 @@ where
|
||||
cmds.push(test_selector.into());
|
||||
|
||||
// Your existing launcher:
|
||||
let command_cwd = std::env::current_dir().expect("should be able to get current dir");
|
||||
let sandbox_cwd = command_cwd.clone();
|
||||
let mut child = spawn_command_under_sandbox(
|
||||
cmds,
|
||||
command_cwd,
|
||||
policy,
|
||||
std::env::current_dir().expect("should be able to get current dir"),
|
||||
sandbox_cwd.as_path(),
|
||||
stdio_policy,
|
||||
HashMap::from([("IN_SANDBOX".into(), "1".into())]),
|
||||
)
|
||||
|
||||
@@ -35,9 +35,11 @@ fn create_env_from_core_vars() -> HashMap<String, String> {
|
||||
|
||||
#[expect(clippy::print_stdout, clippy::expect_used, clippy::unwrap_used)]
|
||||
async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) {
|
||||
let cwd = std::env::current_dir().expect("cwd should exist");
|
||||
let sandbox_cwd = cwd.clone();
|
||||
let params = ExecParams {
|
||||
command: cmd.iter().map(|elm| elm.to_string()).collect(),
|
||||
cwd: std::env::current_dir().expect("cwd should exist"),
|
||||
cwd,
|
||||
timeout_ms: Some(timeout_ms),
|
||||
env: create_env_from_core_vars(),
|
||||
with_escalated_permissions: None,
|
||||
@@ -59,6 +61,7 @@ async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) {
|
||||
params,
|
||||
SandboxType::LinuxSeccomp,
|
||||
&sandbox_policy,
|
||||
sandbox_cwd.as_path(),
|
||||
&codex_linux_sandbox_exe,
|
||||
None,
|
||||
)
|
||||
@@ -133,6 +136,7 @@ async fn test_timeout() {
|
||||
#[expect(clippy::expect_used)]
|
||||
async fn assert_network_blocked(cmd: &[&str]) {
|
||||
let cwd = std::env::current_dir().expect("cwd should exist");
|
||||
let sandbox_cwd = cwd.clone();
|
||||
let params = ExecParams {
|
||||
command: cmd.iter().map(|s| s.to_string()).collect(),
|
||||
cwd,
|
||||
@@ -151,6 +155,7 @@ async fn assert_network_blocked(cmd: &[&str]) {
|
||||
params,
|
||||
SandboxType::LinuxSeccomp,
|
||||
&sandbox_policy,
|
||||
sandbox_cwd.as_path(),
|
||||
&codex_linux_sandbox_exe,
|
||||
None,
|
||||
)
|
||||
|
||||
@@ -36,7 +36,7 @@ tokio = { version = "1", features = [
|
||||
toml = "0.9"
|
||||
tracing = { version = "0.1.41", features = ["log"] }
|
||||
tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt"] }
|
||||
uuid = { version = "1", features = ["serde", "v4"] }
|
||||
uuid = { version = "1", features = ["serde", "v7"] }
|
||||
|
||||
[dev-dependencies]
|
||||
assert_cmd = "2"
|
||||
|
||||
@@ -589,12 +589,14 @@ impl CodexMessageProcessor {
|
||||
let codex_linux_sandbox_exe = self.config.codex_linux_sandbox_exe.clone();
|
||||
let outgoing = self.outgoing.clone();
|
||||
let req_id = request_id;
|
||||
let sandbox_cwd = self.config.cwd.clone();
|
||||
|
||||
tokio::spawn(async move {
|
||||
match codex_core::exec::process_exec_tool_call(
|
||||
exec_params,
|
||||
sandbox_type,
|
||||
&effective_policy,
|
||||
sandbox_cwd.as_path(),
|
||||
&codex_linux_sandbox_exe,
|
||||
None,
|
||||
)
|
||||
@@ -814,7 +816,7 @@ impl CodexMessageProcessor {
|
||||
return;
|
||||
};
|
||||
|
||||
let required_suffix = format!("{}.jsonl", conversation_id.0);
|
||||
let required_suffix = format!("{conversation_id}.jsonl");
|
||||
let Some(file_name) = canonical_rollout_path.file_name().map(OsStr::to_owned) else {
|
||||
let error = JSONRPCErrorError {
|
||||
code: INVALID_REQUEST_ERROR_CODE,
|
||||
@@ -1414,13 +1416,13 @@ mod tests {
|
||||
#[test]
|
||||
fn extract_conversation_summary_prefers_plain_user_messages() {
|
||||
let conversation_id =
|
||||
ConversationId(Uuid::parse_str("3f941c35-29b3-493b-b0a4-e25800d9aeb0").unwrap());
|
||||
ConversationId::from_string("3f941c35-29b3-493b-b0a4-e25800d9aeb0").unwrap();
|
||||
let timestamp = Some("2025-09-05T16:53:11.850Z".to_string());
|
||||
let path = PathBuf::from("rollout.jsonl");
|
||||
|
||||
let head = vec![
|
||||
json!({
|
||||
"id": conversation_id.0,
|
||||
"id": conversation_id.to_string(),
|
||||
"timestamp": timestamp,
|
||||
"cwd": "/",
|
||||
"originator": "codex",
|
||||
|
||||
@@ -36,7 +36,6 @@ use serde_json::json;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::task;
|
||||
use uuid::Uuid;
|
||||
|
||||
pub(crate) struct MessageProcessor {
|
||||
codex_message_processor: CodexMessageProcessor,
|
||||
@@ -484,8 +483,8 @@ impl MessageProcessor {
|
||||
return;
|
||||
}
|
||||
};
|
||||
let conversation_id = match Uuid::parse_str(&conversation_id) {
|
||||
Ok(id) => ConversationId::from(id),
|
||||
let conversation_id = match ConversationId::from_string(&conversation_id) {
|
||||
Ok(id) => id,
|
||||
Err(e) => {
|
||||
tracing::error!("Failed to parse conversation_id: {e}");
|
||||
let result = CallToolResult {
|
||||
|
||||
@@ -142,7 +142,7 @@ async fn test_list_and_resume_conversations() {
|
||||
} = to_response::<ResumeConversationResponse>(resume_resp)
|
||||
.expect("deserialize resumeConversation response");
|
||||
// conversation id should be a valid UUID
|
||||
let _: uuid::Uuid = conversation_id.into();
|
||||
assert!(!conversation_id.to_string().is_empty());
|
||||
}
|
||||
|
||||
fn create_fake_rollout(codex_home: &Path, filename_ts: &str, meta_rfc3339: &str, preview: &str) {
|
||||
|
||||
@@ -136,7 +136,7 @@ async fn test_send_message_session_not_found() {
|
||||
.expect("timeout")
|
||||
.expect("init");
|
||||
|
||||
let unknown = ConversationId(uuid::Uuid::new_v4());
|
||||
let unknown = ConversationId::new();
|
||||
let req_id = mcp
|
||||
.send_send_user_message_request(SendUserMessageParams {
|
||||
conversation_id: unknown,
|
||||
|
||||
@@ -23,8 +23,12 @@ strum = "0.27.2"
|
||||
strum_macros = "0.27.2"
|
||||
sys-locale = "0.3.2"
|
||||
tracing = "0.1.41"
|
||||
ts-rs = { version = "11", features = ["uuid-impl", "serde-json-impl", "no-serde-warnings"] }
|
||||
uuid = { version = "1", features = ["serde", "v4"] }
|
||||
ts-rs = { version = "11", features = [
|
||||
"uuid-impl",
|
||||
"serde-json-impl",
|
||||
"no-serde-warnings",
|
||||
] }
|
||||
uuid = { version = "1", features = ["serde", "v7"] }
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions = "1.4.1"
|
||||
|
||||
@@ -19,13 +19,23 @@ use strum_macros::Display;
|
||||
use ts_rs::TS;
|
||||
use uuid::Uuid;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, TS, Hash)]
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, TS, Hash)]
|
||||
#[ts(type = "string")]
|
||||
pub struct ConversationId(pub Uuid);
|
||||
pub struct ConversationId {
|
||||
uuid: Uuid,
|
||||
}
|
||||
|
||||
impl ConversationId {
|
||||
pub fn new() -> Self {
|
||||
Self(Uuid::new_v4())
|
||||
Self {
|
||||
uuid: Uuid::now_v7(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn from_string(s: &str) -> Result<Self, uuid::Error> {
|
||||
Ok(Self {
|
||||
uuid: Uuid::parse_str(s)?,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -37,19 +47,27 @@ impl Default for ConversationId {
|
||||
|
||||
impl Display for ConversationId {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
write!(f, "{}", self.0)
|
||||
write!(f, "{}", self.uuid)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<Uuid> for ConversationId {
|
||||
fn from(value: Uuid) -> Self {
|
||||
Self(value)
|
||||
impl Serialize for ConversationId {
|
||||
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
|
||||
where
|
||||
S: serde::Serializer,
|
||||
{
|
||||
serializer.collect_str(&self.uuid)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<ConversationId> for Uuid {
|
||||
fn from(value: ConversationId) -> Self {
|
||||
value.0
|
||||
impl<'de> Deserialize<'de> for ConversationId {
|
||||
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
let value = String::deserialize(deserializer)?;
|
||||
let uuid = Uuid::parse_str(&value).map_err(serde::de::Error::custom)?;
|
||||
Ok(Self { uuid })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -719,6 +737,27 @@ mod tests {
|
||||
#[test]
|
||||
fn test_conversation_id_default_is_not_zeroes() {
|
||||
let id = ConversationId::default();
|
||||
assert_ne!(id.0, Uuid::nil());
|
||||
assert_ne!(id.uuid, Uuid::nil());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn conversation_id_serializes_as_plain_string() {
|
||||
let id = ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap();
|
||||
|
||||
assert_eq!(
|
||||
json!("67e55044-10b1-426f-9247-bb680e5fe0c8"),
|
||||
serde_json::to_value(id).unwrap()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn conversation_id_deserializes_from_plain_string() {
|
||||
let id: ConversationId =
|
||||
serde_json::from_value(json!("67e55044-10b1-426f-9247-bb680e5fe0c8")).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap(),
|
||||
id,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1252,7 +1252,8 @@ mod tests {
|
||||
/// amount of nesting.
|
||||
#[test]
|
||||
fn serialize_event() {
|
||||
let conversation_id = ConversationId(uuid::uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8"));
|
||||
let conversation_id =
|
||||
ConversationId::from_string("67e55044-10b1-426f-9247-bb680e5fe0c8").unwrap();
|
||||
let rollout_file = NamedTempFile::new().unwrap();
|
||||
let event = Event {
|
||||
id: "1234".to_string(),
|
||||
|
||||
@@ -85,6 +85,7 @@ unicode-segmentation = "1.12.0"
|
||||
unicode-width = "0.1"
|
||||
url = "2"
|
||||
pathdiff = "0.2"
|
||||
owo-colors = "4.2.0"
|
||||
|
||||
[target.'cfg(unix)'.dependencies]
|
||||
libc = "0.2"
|
||||
|
||||
+12
-2
@@ -15,6 +15,7 @@ use codex_core::config::persist_model_selection;
|
||||
use codex_core::model_family::find_family_for_model;
|
||||
use codex_core::protocol::TokenUsage;
|
||||
use codex_core::protocol_config_types::ReasoningEffort as ReasoningEffortConfig;
|
||||
use codex_protocol::mcp_protocol::ConversationId;
|
||||
use color_eyre::eyre::Result;
|
||||
use color_eyre::eyre::WrapErr;
|
||||
use crossterm::event::KeyCode;
|
||||
@@ -33,6 +34,12 @@ use tokio::select;
|
||||
use tokio::sync::mpsc::unbounded_channel;
|
||||
// use uuid::Uuid;
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct AppExitInfo {
|
||||
pub token_usage: TokenUsage,
|
||||
pub conversation_id: Option<ConversationId>,
|
||||
}
|
||||
|
||||
pub(crate) struct App {
|
||||
pub(crate) server: Arc<ConversationManager>,
|
||||
pub(crate) app_event_tx: AppEventSender,
|
||||
@@ -70,7 +77,7 @@ impl App {
|
||||
initial_prompt: Option<String>,
|
||||
initial_images: Vec<PathBuf>,
|
||||
resume_selection: ResumeSelection,
|
||||
) -> Result<TokenUsage> {
|
||||
) -> Result<AppExitInfo> {
|
||||
use tokio_stream::StreamExt;
|
||||
let (app_event_tx, mut app_event_rx) = unbounded_channel();
|
||||
let app_event_tx = AppEventSender::new(app_event_tx);
|
||||
@@ -153,7 +160,10 @@ impl App {
|
||||
}
|
||||
} {}
|
||||
tui.terminal.clear()?;
|
||||
Ok(app.token_usage())
|
||||
Ok(AppExitInfo {
|
||||
token_usage: app.token_usage(),
|
||||
conversation_id: app.chat_widget.conversation_id(),
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) async fn handle_tui_event(
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
use std::convert::TryFrom;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use rand::Rng as _;
|
||||
|
||||
use crate::frames::ALL_VARIANTS;
|
||||
use crate::frames::FRAME_TICK_DEFAULT;
|
||||
use crate::tui::FrameRequester;
|
||||
|
||||
/// Drives ASCII art animations shared across popups and onboarding widgets.
|
||||
pub(crate) struct AsciiAnimation {
|
||||
request_frame: FrameRequester,
|
||||
variants: &'static [&'static [&'static str]],
|
||||
variant_idx: usize,
|
||||
frame_tick: Duration,
|
||||
start: Instant,
|
||||
}
|
||||
|
||||
impl AsciiAnimation {
|
||||
pub(crate) fn new(request_frame: FrameRequester) -> Self {
|
||||
Self::with_variants(request_frame, ALL_VARIANTS, 0)
|
||||
}
|
||||
|
||||
pub(crate) fn with_variants(
|
||||
request_frame: FrameRequester,
|
||||
variants: &'static [&'static [&'static str]],
|
||||
variant_idx: usize,
|
||||
) -> Self {
|
||||
assert!(
|
||||
!variants.is_empty(),
|
||||
"AsciiAnimation requires at least one animation variant",
|
||||
);
|
||||
let clamped_idx = variant_idx.min(variants.len() - 1);
|
||||
Self {
|
||||
request_frame,
|
||||
variants,
|
||||
variant_idx: clamped_idx,
|
||||
frame_tick: FRAME_TICK_DEFAULT,
|
||||
start: Instant::now(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn schedule_next_frame(&self) {
|
||||
let tick_ms = self.frame_tick.as_millis();
|
||||
if tick_ms == 0 {
|
||||
self.request_frame.schedule_frame();
|
||||
return;
|
||||
}
|
||||
let elapsed_ms = self.start.elapsed().as_millis();
|
||||
let rem_ms = elapsed_ms % tick_ms;
|
||||
let delay_ms = if rem_ms == 0 {
|
||||
tick_ms
|
||||
} else {
|
||||
tick_ms - rem_ms
|
||||
};
|
||||
if let Ok(delay_ms_u64) = u64::try_from(delay_ms) {
|
||||
self.request_frame
|
||||
.schedule_frame_in(Duration::from_millis(delay_ms_u64));
|
||||
} else {
|
||||
self.request_frame.schedule_frame();
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn current_frame(&self) -> &'static str {
|
||||
let frames = self.frames();
|
||||
if frames.is_empty() {
|
||||
return "";
|
||||
}
|
||||
let tick_ms = self.frame_tick.as_millis();
|
||||
if tick_ms == 0 {
|
||||
return frames[0];
|
||||
}
|
||||
let elapsed_ms = self.start.elapsed().as_millis();
|
||||
let idx = ((elapsed_ms / tick_ms) % frames.len() as u128) as usize;
|
||||
frames[idx]
|
||||
}
|
||||
|
||||
pub(crate) fn pick_random_variant(&mut self) -> bool {
|
||||
if self.variants.len() <= 1 {
|
||||
return false;
|
||||
}
|
||||
let mut rng = rand::rng();
|
||||
let mut next = self.variant_idx;
|
||||
while next == self.variant_idx {
|
||||
next = rng.random_range(0..self.variants.len());
|
||||
}
|
||||
self.variant_idx = next;
|
||||
self.request_frame.schedule_frame();
|
||||
true
|
||||
}
|
||||
|
||||
pub(crate) fn request_frame(&self) {
|
||||
self.request_frame.schedule_frame();
|
||||
}
|
||||
|
||||
fn frames(&self) -> &'static [&'static str] {
|
||||
self.variants[self.variant_idx]
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn frame_tick_must_be_nonzero() {
|
||||
assert!(FRAME_TICK_DEFAULT.as_millis() > 0);
|
||||
}
|
||||
}
|
||||
@@ -255,7 +255,7 @@ impl ChatWidget {
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
fn on_task_complete(&mut self) {
|
||||
fn on_task_complete(&mut self, last_agent_message: Option<String>) {
|
||||
// If a stream is currently active, finalize only that stream to flush any tail
|
||||
// without emitting stray headers for other streams.
|
||||
if self.stream.is_write_cycle_active() {
|
||||
@@ -270,7 +270,9 @@ impl ChatWidget {
|
||||
// If there is a queued user message, send exactly one now to begin the next turn.
|
||||
self.maybe_send_next_queued_input();
|
||||
// Emit a notification when the turn completes (suppressed if focused).
|
||||
self.notify(Notification::AgentTurnComplete);
|
||||
self.notify(Notification::AgentTurnComplete {
|
||||
response: last_agent_message.unwrap_or_default(),
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) fn set_token_info(&mut self, info: Option<TokenUsageInfo>) {
|
||||
@@ -1082,7 +1084,9 @@ impl ChatWidget {
|
||||
}
|
||||
EventMsg::AgentReasoningSectionBreak(_) => self.on_reasoning_section_break(),
|
||||
EventMsg::TaskStarted(_) => self.on_task_started(),
|
||||
EventMsg::TaskComplete(TaskCompleteEvent { .. }) => self.on_task_complete(),
|
||||
EventMsg::TaskComplete(TaskCompleteEvent { last_agent_message }) => {
|
||||
self.on_task_complete(last_agent_message)
|
||||
}
|
||||
EventMsg::TokenCount(ev) => self.set_token_info(ev.info),
|
||||
EventMsg::Error(ErrorEvent { message }) => self.on_error(message),
|
||||
EventMsg::TurnAborted(ev) => match ev.reason {
|
||||
@@ -1479,7 +1483,7 @@ impl WidgetRef for &ChatWidget {
|
||||
}
|
||||
|
||||
enum Notification {
|
||||
AgentTurnComplete,
|
||||
AgentTurnComplete { response: String },
|
||||
ExecApprovalRequested { command: String },
|
||||
EditApprovalRequested { cwd: PathBuf, changes: Vec<PathBuf> },
|
||||
}
|
||||
@@ -1487,7 +1491,10 @@ enum Notification {
|
||||
impl Notification {
|
||||
fn display(&self) -> String {
|
||||
match self {
|
||||
Notification::AgentTurnComplete => "Agent turn complete".to_string(),
|
||||
Notification::AgentTurnComplete { response } => {
|
||||
Notification::agent_turn_preview(response)
|
||||
.unwrap_or_else(|| "Agent turn complete".to_string())
|
||||
}
|
||||
Notification::ExecApprovalRequested { command } => {
|
||||
format!("Approval requested: {}", truncate_text(command, 30))
|
||||
}
|
||||
@@ -1507,7 +1514,7 @@ impl Notification {
|
||||
|
||||
fn type_name(&self) -> &str {
|
||||
match self {
|
||||
Notification::AgentTurnComplete => "agent-turn-complete",
|
||||
Notification::AgentTurnComplete { .. } => "agent-turn-complete",
|
||||
Notification::ExecApprovalRequested { .. }
|
||||
| Notification::EditApprovalRequested { .. } => "approval-requested",
|
||||
}
|
||||
@@ -1519,8 +1526,26 @@ impl Notification {
|
||||
Notifications::Custom(allowed) => allowed.iter().any(|a| a == self.type_name()),
|
||||
}
|
||||
}
|
||||
|
||||
fn agent_turn_preview(response: &str) -> Option<String> {
|
||||
let mut normalized = String::new();
|
||||
for part in response.split_whitespace() {
|
||||
if !normalized.is_empty() {
|
||||
normalized.push(' ');
|
||||
}
|
||||
normalized.push_str(part);
|
||||
}
|
||||
let trimmed = normalized.trim();
|
||||
if trimmed.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(truncate_text(trimmed, AGENT_NOTIFICATION_PREVIEW_GRAPHEMES))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const AGENT_NOTIFICATION_PREVIEW_GRAPHEMES: usize = 200;
|
||||
|
||||
const EXAMPLE_PROMPTS: [&str; 6] = [
|
||||
"Explain this codebase",
|
||||
"Summarize recent commits",
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
#![deny(clippy::print_stdout, clippy::print_stderr)]
|
||||
#![deny(clippy::disallowed_methods)]
|
||||
use app::App;
|
||||
pub use app::AppExitInfo;
|
||||
use codex_core::AuthManager;
|
||||
use codex_core::BUILT_IN_OSS_MODEL_PROVIDER_ID;
|
||||
use codex_core::CodexAuth;
|
||||
@@ -32,6 +33,7 @@ mod app;
|
||||
mod app_backtrack;
|
||||
mod app_event;
|
||||
mod app_event_sender;
|
||||
mod ascii_animation;
|
||||
mod backtrack_helpers;
|
||||
mod bottom_pane;
|
||||
mod chatwidget;
|
||||
@@ -85,7 +87,7 @@ use codex_core::internal_storage::InternalStorage;
|
||||
pub async fn run_main(
|
||||
cli: Cli,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
) -> std::io::Result<codex_core::protocol::TokenUsage> {
|
||||
) -> std::io::Result<AppExitInfo> {
|
||||
let (sandbox_mode, approval_policy) = if cli.full_auto {
|
||||
(
|
||||
Some(SandboxMode::WorkspaceWrite),
|
||||
@@ -257,7 +259,7 @@ async fn run_ratatui_app(
|
||||
mut internal_storage: InternalStorage,
|
||||
active_profile: Option<String>,
|
||||
should_show_trust_screen: bool,
|
||||
) -> color_eyre::Result<codex_core::protocol::TokenUsage> {
|
||||
) -> color_eyre::Result<AppExitInfo> {
|
||||
let mut config = config;
|
||||
color_eyre::install()?;
|
||||
|
||||
@@ -369,7 +371,10 @@ async fn run_ratatui_app(
|
||||
resume_picker::ResumeSelection::Exit => {
|
||||
restore();
|
||||
session_log::log_session_end();
|
||||
return Ok(codex_core::protocol::TokenUsage::default());
|
||||
return Ok(AppExitInfo {
|
||||
token_usage: codex_core::protocol::TokenUsage::default(),
|
||||
conversation_id: None,
|
||||
});
|
||||
}
|
||||
other => other,
|
||||
}
|
||||
|
||||
@@ -3,6 +3,8 @@ use codex_arg0::arg0_dispatch_or_else;
|
||||
use codex_common::CliConfigOverrides;
|
||||
use codex_tui::Cli;
|
||||
use codex_tui::run_main;
|
||||
use owo_colors::OwoColorize;
|
||||
use supports_color::Stream;
|
||||
|
||||
#[derive(Parser, Debug)]
|
||||
struct TopCli {
|
||||
@@ -21,9 +23,21 @@ fn main() -> anyhow::Result<()> {
|
||||
.config_overrides
|
||||
.raw_overrides
|
||||
.splice(0..0, top_cli.config_overrides.raw_overrides);
|
||||
let usage = run_main(inner, codex_linux_sandbox_exe).await?;
|
||||
if !usage.is_zero() {
|
||||
println!("{}", codex_core::protocol::FinalOutput::from(usage));
|
||||
let exit_info = run_main(inner, codex_linux_sandbox_exe).await?;
|
||||
let token_usage = exit_info.token_usage;
|
||||
let conversation_id = exit_info.conversation_id;
|
||||
if !token_usage.is_zero() {
|
||||
println!("{}", codex_core::protocol::FinalOutput::from(token_usage),);
|
||||
if let Some(session_id) = conversation_id {
|
||||
let command = format!("codex resume {session_id}");
|
||||
let prefix = "To continue this session, run ";
|
||||
let suffix = ".";
|
||||
if supports_color::on(Stream::Stdout).is_some() {
|
||||
println!("{}{}{}", prefix, command.cyan(), suffix);
|
||||
} else {
|
||||
println!("{prefix}{command}{suffix}");
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
})
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use crate::frames::ALL_VARIANTS as FRAME_VARIANTS;
|
||||
use crate::frames::FRAME_TICK_DEFAULT;
|
||||
use crate::ascii_animation::AsciiAnimation;
|
||||
use crate::tui::FrameRequester;
|
||||
use crate::tui::Tui;
|
||||
use crate::tui::TuiEvent;
|
||||
@@ -7,7 +6,6 @@ use color_eyre::eyre::Result;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use rand::Rng as _;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::prelude::Widget;
|
||||
@@ -17,10 +15,8 @@ use ratatui::widgets::Clear;
|
||||
use ratatui::widgets::Paragraph;
|
||||
use ratatui::widgets::WidgetRef;
|
||||
use ratatui::widgets::Wrap;
|
||||
use std::time::Duration;
|
||||
use tokio_stream::StreamExt;
|
||||
|
||||
const FRAME_TICK: Duration = FRAME_TICK_DEFAULT;
|
||||
const MIN_ANIMATION_HEIGHT: u16 = 24;
|
||||
const MIN_ANIMATION_WIDTH: u16 = 60;
|
||||
|
||||
@@ -39,9 +35,7 @@ enum ModelUpgradeOption {
|
||||
struct ModelUpgradePopup {
|
||||
highlighted: ModelUpgradeOption,
|
||||
decision: Option<ModelUpgradeDecision>,
|
||||
request_frame: FrameRequester,
|
||||
frame_idx: usize,
|
||||
variant_idx: usize,
|
||||
animation: AsciiAnimation,
|
||||
}
|
||||
|
||||
impl ModelUpgradePopup {
|
||||
@@ -49,9 +43,7 @@ impl ModelUpgradePopup {
|
||||
Self {
|
||||
highlighted: ModelUpgradeOption::TryNewModel,
|
||||
decision: None,
|
||||
request_frame,
|
||||
frame_idx: 0,
|
||||
variant_idx: 0,
|
||||
animation: AsciiAnimation::new(request_frame),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -65,7 +57,7 @@ impl ModelUpgradePopup {
|
||||
KeyCode::Esc => self.select(ModelUpgradeOption::KeepCurrent),
|
||||
KeyCode::Char('.') => {
|
||||
if key_event.modifiers.contains(KeyModifiers::CONTROL) {
|
||||
self.pick_random_variant();
|
||||
let _ = self.animation.pick_random_variant();
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
@@ -75,37 +67,13 @@ impl ModelUpgradePopup {
|
||||
fn highlight(&mut self, option: ModelUpgradeOption) {
|
||||
if self.highlighted != option {
|
||||
self.highlighted = option;
|
||||
self.request_frame.schedule_frame();
|
||||
self.animation.request_frame();
|
||||
}
|
||||
}
|
||||
|
||||
fn select(&mut self, option: ModelUpgradeOption) {
|
||||
self.decision = Some(option.into());
|
||||
self.request_frame.schedule_frame();
|
||||
}
|
||||
|
||||
fn advance_animation(&mut self) {
|
||||
let len = self.frames().len();
|
||||
self.frame_idx = (self.frame_idx + 1) % len;
|
||||
self.request_frame.schedule_frame_in(FRAME_TICK);
|
||||
}
|
||||
|
||||
fn frames(&self) -> &'static [&'static str] {
|
||||
FRAME_VARIANTS[self.variant_idx]
|
||||
}
|
||||
|
||||
fn pick_random_variant(&mut self) {
|
||||
let total = FRAME_VARIANTS.len();
|
||||
if total <= 1 {
|
||||
return;
|
||||
}
|
||||
let mut rng = rand::rng();
|
||||
let mut next = self.variant_idx;
|
||||
while next == self.variant_idx {
|
||||
next = rng.random_range(0..total);
|
||||
}
|
||||
self.variant_idx = next;
|
||||
self.request_frame.schedule_frame();
|
||||
self.animation.request_frame();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -121,6 +89,7 @@ impl From<ModelUpgradeOption> for ModelUpgradeDecision {
|
||||
impl WidgetRef for &ModelUpgradePopup {
|
||||
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
|
||||
Clear.render(area, buf);
|
||||
self.animation.schedule_next_frame();
|
||||
|
||||
// Skip the animation entirely when the viewport is too small so we don't clip frames.
|
||||
let show_animation =
|
||||
@@ -128,7 +97,7 @@ impl WidgetRef for &ModelUpgradePopup {
|
||||
|
||||
let mut lines: Vec<Line> = Vec::new();
|
||||
if show_animation {
|
||||
let frame = self.frames()[self.frame_idx];
|
||||
let frame = self.animation.current_frame();
|
||||
lines.extend(frame.lines().map(|l| l.into()));
|
||||
// Spacer between animation and text content.
|
||||
lines.push("".into());
|
||||
@@ -188,8 +157,6 @@ pub(crate) async fn run_model_upgrade_popup(tui: &mut Tui) -> Result<ModelUpgrad
|
||||
frame.render_widget_ref(&popup, frame.area());
|
||||
})?;
|
||||
|
||||
popup.advance_animation();
|
||||
|
||||
let events = tui.event_stream();
|
||||
tokio::pin!(events);
|
||||
while popup.decision.is_none() {
|
||||
@@ -197,7 +164,6 @@ pub(crate) async fn run_model_upgrade_popup(tui: &mut Tui) -> Result<ModelUpgrad
|
||||
match event {
|
||||
TuiEvent::Key(key_event) => popup.handle_key_event(key_event),
|
||||
TuiEvent::Draw => {
|
||||
popup.advance_animation();
|
||||
let _ = tui.draw(u16::MAX, |frame| {
|
||||
frame.render_widget_ref(&popup, frame.area());
|
||||
});
|
||||
|
||||
@@ -7,6 +7,7 @@ use crossterm::event::KeyEventKind;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::prelude::Widget;
|
||||
use ratatui::style::Color;
|
||||
use ratatui::widgets::Clear;
|
||||
use ratatui::widgets::WidgetRef;
|
||||
|
||||
@@ -24,7 +25,6 @@ use crate::tui::TuiEvent;
|
||||
use color_eyre::eyre::Result;
|
||||
use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
use std::time::Instant;
|
||||
|
||||
#[allow(clippy::large_enum_variant)]
|
||||
enum Step {
|
||||
@@ -73,11 +73,10 @@ impl OnboardingScreen {
|
||||
} = args;
|
||||
let cwd = config.cwd.clone();
|
||||
let codex_home = config.codex_home;
|
||||
let mut steps: Vec<Step> = vec![Step::Welcome(WelcomeWidget {
|
||||
is_logged_in: !matches!(login_status, LoginStatus::NotAuthenticated),
|
||||
request_frame: tui.frame_requester(),
|
||||
start: Instant::now(),
|
||||
})];
|
||||
let mut steps: Vec<Step> = vec![Step::Welcome(WelcomeWidget::new(
|
||||
!matches!(login_status, LoginStatus::NotAuthenticated),
|
||||
tui.frame_requester(),
|
||||
))];
|
||||
if show_login_screen {
|
||||
steps.push(Step::Auth(AuthModeWidget {
|
||||
request_frame: tui.frame_requester(),
|
||||
@@ -189,6 +188,13 @@ impl KeyboardHandler for OnboardingScreen {
|
||||
self.is_done = true;
|
||||
}
|
||||
_ => {
|
||||
if let Some(Step::Welcome(widget)) = self
|
||||
.steps
|
||||
.iter_mut()
|
||||
.find(|step| matches!(step, Step::Welcome(_)))
|
||||
{
|
||||
widget.handle_key_event(key_event);
|
||||
}
|
||||
if let Some(active_step) = self.current_steps_mut().into_iter().last() {
|
||||
active_step.handle_key_event(key_event);
|
||||
}
|
||||
@@ -226,8 +232,12 @@ impl WidgetRef for &OnboardingScreen {
|
||||
for yy in 0..height {
|
||||
let mut any = false;
|
||||
for xx in 0..width {
|
||||
let sym = tmp[(xx, yy)].symbol();
|
||||
if !sym.trim().is_empty() {
|
||||
let cell = &tmp[(xx, yy)];
|
||||
let has_symbol = !cell.symbol().trim().is_empty();
|
||||
let has_style = cell.fg != Color::Reset
|
||||
|| cell.bg != Color::Reset
|
||||
|| !cell.modifier.is_empty();
|
||||
if has_symbol || has_style {
|
||||
any = true;
|
||||
break;
|
||||
}
|
||||
@@ -271,7 +281,7 @@ impl WidgetRef for &OnboardingScreen {
|
||||
impl KeyboardHandler for Step {
|
||||
fn handle_key_event(&mut self, key_event: KeyEvent) {
|
||||
match self {
|
||||
Step::Welcome(_) => (),
|
||||
Step::Welcome(widget) => widget.handle_key_event(key_event),
|
||||
Step::Auth(widget) => widget.handle_key_event(key_event),
|
||||
Step::TrustDirectory(widget) => widget.handle_key_event(key_event),
|
||||
}
|
||||
|
||||
@@ -1,60 +1,68 @@
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyEventKind;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::prelude::Widget;
|
||||
use ratatui::style::Stylize;
|
||||
use ratatui::text::Line;
|
||||
use ratatui::widgets::Clear;
|
||||
use ratatui::widgets::Paragraph;
|
||||
use ratatui::widgets::WidgetRef;
|
||||
use ratatui::widgets::Wrap;
|
||||
|
||||
use crate::frames::FRAME_TICK_DEFAULT;
|
||||
use crate::frames::FRAMES_DEFAULT;
|
||||
use crate::ascii_animation::AsciiAnimation;
|
||||
use crate::onboarding::onboarding_screen::KeyboardHandler;
|
||||
use crate::onboarding::onboarding_screen::StepStateProvider;
|
||||
use crate::tui::FrameRequester;
|
||||
|
||||
use super::onboarding_screen::StepState;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
const FRAME_TICK: Duration = FRAME_TICK_DEFAULT;
|
||||
const MIN_ANIMATION_HEIGHT: u16 = 20;
|
||||
const MIN_ANIMATION_WIDTH: u16 = 60;
|
||||
|
||||
pub(crate) struct WelcomeWidget {
|
||||
pub is_logged_in: bool,
|
||||
pub request_frame: FrameRequester,
|
||||
pub start: Instant,
|
||||
animation: AsciiAnimation,
|
||||
}
|
||||
|
||||
impl KeyboardHandler for WelcomeWidget {
|
||||
fn handle_key_event(&mut self, key_event: KeyEvent) {
|
||||
if key_event.kind == KeyEventKind::Press
|
||||
&& key_event.code == KeyCode::Char('.')
|
||||
&& key_event.modifiers.contains(KeyModifiers::CONTROL)
|
||||
{
|
||||
tracing::warn!("Welcome background to press '.'");
|
||||
let _ = self.animation.pick_random_variant();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl WelcomeWidget {
|
||||
pub(crate) fn new(is_logged_in: bool, request_frame: FrameRequester) -> Self {
|
||||
Self {
|
||||
is_logged_in,
|
||||
animation: AsciiAnimation::new(request_frame),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl WidgetRef for &WelcomeWidget {
|
||||
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
|
||||
let elapsed_ms = self.start.elapsed().as_millis();
|
||||
Clear.render(area, buf);
|
||||
self.animation.schedule_next_frame();
|
||||
|
||||
// Align next draw to the next FRAME_TICK boundary to reduce jitter.
|
||||
{
|
||||
let tick_ms = FRAME_TICK.as_millis();
|
||||
let rem_ms = elapsed_ms % tick_ms;
|
||||
let delay_ms = if rem_ms == 0 {
|
||||
tick_ms
|
||||
} else {
|
||||
tick_ms - rem_ms
|
||||
};
|
||||
// Safe cast: delay_ms < tick_ms and FRAME_TICK is small.
|
||||
self.request_frame
|
||||
.schedule_frame_in(Duration::from_millis(delay_ms as u64));
|
||||
}
|
||||
|
||||
let frames = &FRAMES_DEFAULT;
|
||||
let idx = ((elapsed_ms / FRAME_TICK.as_millis()) % frames.len() as u128) as usize;
|
||||
// Skip the animation entirely when the viewport is too small so we don't clip frames.
|
||||
let show_animation =
|
||||
area.height >= MIN_ANIMATION_HEIGHT && area.width >= MIN_ANIMATION_WIDTH;
|
||||
|
||||
let mut lines: Vec<Line> = Vec::new();
|
||||
if show_animation {
|
||||
let frame_line_count = frames[idx].lines().count();
|
||||
lines.reserve(frame_line_count + 2);
|
||||
lines.extend(frames[idx].lines().map(|l| l.into()));
|
||||
let frame = self.animation.current_frame();
|
||||
// let frame_line_count = frame.lines().count();
|
||||
// lines.reserve(frame_line_count + 2);
|
||||
lines.extend(frame.lines().map(|l| l.into()));
|
||||
lines.push("".into());
|
||||
}
|
||||
lines.push(Line::from(vec![
|
||||
@@ -82,10 +90,54 @@ impl StepStateProvider for WelcomeWidget {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
|
||||
static VARIANT_A: [&str; 1] = ["frame-a"];
|
||||
static VARIANT_B: [&str; 1] = ["frame-b"];
|
||||
static VARIANTS: [&[&str]; 2] = [&VARIANT_A, &VARIANT_B];
|
||||
|
||||
/// A number of things break down if FRAME_TICK is zero.
|
||||
#[test]
|
||||
fn frame_tick_must_be_nonzero() {
|
||||
assert!(FRAME_TICK.as_millis() > 0);
|
||||
fn welcome_renders_animation_on_first_draw() {
|
||||
let widget = WelcomeWidget::new(false, FrameRequester::test_dummy());
|
||||
let area = Rect::new(0, 0, MIN_ANIMATION_WIDTH, MIN_ANIMATION_HEIGHT);
|
||||
let mut buf = Buffer::empty(area);
|
||||
(&widget).render(area, &mut buf);
|
||||
|
||||
let mut found = false;
|
||||
let mut last_non_empty: Option<u16> = None;
|
||||
for y in 0..area.height {
|
||||
for x in 0..area.width {
|
||||
if !buf[(x, y)].symbol().trim().is_empty() {
|
||||
found = true;
|
||||
last_non_empty = Some(y);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
assert!(found, "expected welcome animation to render characters");
|
||||
let measured_rows = last_non_empty.map(|v| v + 2).unwrap_or(0);
|
||||
assert!(
|
||||
measured_rows >= MIN_ANIMATION_HEIGHT,
|
||||
"expected measurement to report at least {MIN_ANIMATION_HEIGHT} rows, got {measured_rows}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ctrl_dot_changes_animation_variant() {
|
||||
let mut widget = WelcomeWidget {
|
||||
is_logged_in: false,
|
||||
animation: AsciiAnimation::with_variants(FrameRequester::test_dummy(), &VARIANTS, 0),
|
||||
};
|
||||
|
||||
let before = widget.animation.current_frame();
|
||||
widget.handle_key_event(KeyEvent::new(KeyCode::Char('.'), KeyModifiers::CONTROL));
|
||||
let after = widget.animation.current_frame();
|
||||
|
||||
assert_ne!(
|
||||
before, after,
|
||||
"expected ctrl+. to switch welcome animation variant"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user