diff --git a/.github/scripts/install-musl-build-tools.sh b/.github/scripts/install-musl-build-tools.sh index e2af7528d5..634fe04d71 100644 --- a/.github/scripts/install-musl-build-tools.sh +++ b/.github/scripts/install-musl-build-tools.sh @@ -32,30 +32,132 @@ case "${TARGET}" in ;; esac -if command -v clang++ >/dev/null; then - cxx="$(command -v clang++)" - echo "CXXFLAGS=--target=${TARGET} -stdlib=libc++ -pthread" >> "$GITHUB_ENV" - echo "CFLAGS=--target=${TARGET} -pthread" >> "$GITHUB_ENV" - if command -v clang >/dev/null; then - cc="$(command -v clang)" - echo "CC=${cc}" >> "$GITHUB_ENV" - echo "TARGET_CC=${cc}" >> "$GITHUB_ENV" - target_cc_var="CC_${TARGET}" - target_cc_var="${target_cc_var//-/_}" - echo "${target_cc_var}=${cc}" >> "$GITHUB_ENV" - fi -elif command -v "${arch}-linux-musl-g++" >/dev/null; then - cxx="$(command -v "${arch}-linux-musl-g++")" -elif command -v musl-g++ >/dev/null; then - cxx="$(command -v musl-g++)" +# Use the musl toolchain as the Rust linker to avoid Zig injecting its own CRT. +if command -v "${arch}-linux-musl-gcc" >/dev/null; then + musl_linker="$(command -v "${arch}-linux-musl-gcc")" elif command -v musl-gcc >/dev/null; then - cxx="$(command -v musl-gcc)" - echo "CFLAGS=-pthread" >> "$GITHUB_ENV" + musl_linker="$(command -v musl-gcc)" else - echo "musl g++ not found after install; arch=${arch}" >&2 + echo "musl gcc not found after install; arch=${arch}" >&2 exit 1 fi +zig_target="${TARGET/-unknown-linux-musl/-linux-musl}" +runner_temp="${RUNNER_TEMP:-/tmp}" +tool_root="${runner_temp}/codex-musl-tools-${TARGET}" +mkdir -p "${tool_root}" + +sysroot="" +if command -v zig >/dev/null; then + zig_bin="$(command -v zig)" + cc="${tool_root}/zigcc" + cxx="${tool_root}/zigcxx" + + cat >"${cc}" <"${cxx}" </dev/null || true)" +else + cc="${musl_linker}" + + if command -v "${arch}-linux-musl-g++" >/dev/null; then + cxx="$(command -v "${arch}-linux-musl-g++")" + elif command -v musl-g++ >/dev/null; then + cxx="$(command -v musl-g++)" + else + cxx="${cc}" + fi +fi + +if [[ -n "${sysroot}" && "${sysroot}" != "/" ]]; then + echo "BORING_BSSL_SYSROOT=${sysroot}" >> "$GITHUB_ENV" + boring_sysroot_var="BORING_BSSL_SYSROOT_${TARGET}" + boring_sysroot_var="${boring_sysroot_var//-/_}" + echo "${boring_sysroot_var}=${sysroot}" >> "$GITHUB_ENV" +fi + +cflags="-pthread" +cxxflags="-pthread" +if [[ "${TARGET}" == "aarch64-unknown-linux-musl" ]]; then + # BoringSSL enables -Wframe-larger-than=25344 under clang and treats warnings as errors. + cflags="${cflags} -Wno-error=frame-larger-than" + cxxflags="${cxxflags} -Wno-error=frame-larger-than" +fi + +echo "CFLAGS=${cflags}" >> "$GITHUB_ENV" +echo "CXXFLAGS=${cxxflags}" >> "$GITHUB_ENV" +echo "CC=${cc}" >> "$GITHUB_ENV" +echo "TARGET_CC=${cc}" >> "$GITHUB_ENV" +target_cc_var="CC_${TARGET}" +target_cc_var="${target_cc_var//-/_}" +echo "${target_cc_var}=${cc}" >> "$GITHUB_ENV" echo "CXX=${cxx}" >> "$GITHUB_ENV" +echo "TARGET_CXX=${cxx}" >> "$GITHUB_ENV" +target_cxx_var="CXX_${TARGET}" +target_cxx_var="${target_cxx_var//-/_}" +echo "${target_cxx_var}=${cxx}" >> "$GITHUB_ENV" + +cargo_linker_var="CARGO_TARGET_${TARGET^^}_LINKER" +cargo_linker_var="${cargo_linker_var//-/_}" +echo "${cargo_linker_var}=${musl_linker}" >> "$GITHUB_ENV" + +echo "CMAKE_C_COMPILER=${cc}" >> "$GITHUB_ENV" echo "CMAKE_CXX_COMPILER=${cxx}" >> "$GITHUB_ENV" echo "CMAKE_ARGS=-DCMAKE_HAVE_THREADS_LIBRARY=1 -DCMAKE_USE_PTHREADS_INIT=1 -DCMAKE_THREAD_LIBS_INIT=-pthread -DTHREADS_PREFER_PTHREAD_FLAG=ON" >> "$GITHUB_ENV" diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index a6bf51369b..a8efec1089 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -261,6 +261,12 @@ jobs: /var/cache/apt key: apt-${{ matrix.runner }}-${{ matrix.target }}-v1 + - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} + name: Install Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.14.0 + - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} name: Install musl build tools env: diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 9033d058d7..3625c830dd 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -104,6 +104,12 @@ jobs: ${{ github.workspace }}/codex-rs/target/ key: cargo-${{ matrix.runner }}-${{ matrix.target }}-release-${{ hashFiles('**/Cargo.lock') }} + - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} + name: Install Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.14.0 + - if: ${{ matrix.target == 'x86_64-unknown-linux-musl' || matrix.target == 'aarch64-unknown-linux-musl'}} name: Install musl build tools env: diff --git a/.github/workflows/shell-tool-mcp.yml b/.github/workflows/shell-tool-mcp.yml index 0e03fea43c..a8a01b5f6a 100644 --- a/.github/workflows/shell-tool-mcp.yml +++ b/.github/workflows/shell-tool-mcp.yml @@ -97,6 +97,12 @@ jobs: with: targets: ${{ matrix.target }} + - if: ${{ matrix.install_musl }} + name: Install Zig + uses: mlugg/setup-zig@v2 + with: + version: 0.14.0 + - if: ${{ matrix.install_musl }} name: Install musl build dependencies env: diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 633899b68e..d0b3a0ec37 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1096,6 +1096,7 @@ dependencies = [ "serial_test", "shlex", "tempfile", + "time", "tokio", "toml 0.9.5", "tracing", diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index acf08be990..e282e924fc 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -5,6 +5,7 @@ use crate::protocol::common::AuthMode; use codex_protocol::account::PlanType; use codex_protocol::approvals::ExecPolicyAmendment as CoreExecPolicyAmendment; use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::ForcedLoginMethod; use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary; @@ -30,6 +31,7 @@ use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo; use codex_protocol::protocol::SkillInterface as CoreSkillInterface; use codex_protocol::protocol::SkillMetadata as CoreSkillMetadata; use codex_protocol::protocol::SkillScope as CoreSkillScope; +use codex_protocol::protocol::SubAgentSource as CoreSubAgentSource; use codex_protocol::protocol::TokenUsage as CoreTokenUsage; use codex_protocol::protocol::TokenUsageInfo as CoreTokenUsageInfo; use codex_protocol::user_input::ByteRange as CoreByteRange; @@ -700,6 +702,7 @@ pub enum SessionSource { VsCode, Exec, AppServer, + SubAgent(CoreSubAgentSource), #[serde(other)] Unknown, } @@ -711,7 +714,7 @@ impl From for SessionSource { CoreSessionSource::VSCode => SessionSource::VsCode, CoreSessionSource::Exec => SessionSource::Exec, CoreSessionSource::Mcp => SessionSource::AppServer, - CoreSessionSource::SubAgent(_) => SessionSource::Unknown, + CoreSessionSource::SubAgent(sub) => SessionSource::SubAgent(sub), CoreSessionSource::Unknown => SessionSource::Unknown, } } @@ -724,6 +727,7 @@ impl From for CoreSessionSource { SessionSource::VsCode => CoreSessionSource::VSCode, SessionSource::Exec => CoreSessionSource::Exec, SessionSource::AppServer => CoreSessionSource::Mcp, + SessionSource::SubAgent(sub) => CoreSessionSource::SubAgent(sub), SessionSource::Unknown => CoreSessionSource::Unknown, } } @@ -934,7 +938,7 @@ pub struct CollaborationModeListParams {} #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct CollaborationModeListResponse { - pub data: Vec, + pub data: Vec, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -1083,6 +1087,7 @@ pub struct ThreadStartParams { pub base_instructions: Option, pub developer_instructions: Option, pub personality: Option, + pub ephemeral: Option, /// If true, opt into emitting raw response items on the event stream. /// /// This is for internal use only (e.g. Codex Cloud). @@ -1466,7 +1471,7 @@ pub struct Thread { #[ts(type = "number")] pub updated_at: i64, /// [UNSTABLE] Path to the thread on disk. - pub path: PathBuf, + pub path: Option, /// Working directory captured for the thread. pub cwd: PathBuf, /// Version of the CLI that created the thread. diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index f31df68b3f..9fa2044058 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -35,6 +35,7 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } mcp-types = { workspace = true } tempfile = { workspace = true } +time = { workspace = true } toml = { workspace = true } tokio = { workspace = true, features = [ "io-std", diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 931c1073aa..68e233b843 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -1006,7 +1006,15 @@ pub(crate) async fn apply_bespoke_event_handling( }; if let Some(request_id) = pending { - let rollout_path = conversation.rollout_path(); + let Some(rollout_path) = conversation.rollout_path() else { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: "thread has no persisted rollout".to_string(), + data: None, + }; + outgoing.send_error(request_id, error).await; + return; + }; let response = match read_summary_from_rollout( rollout_path.as_path(), fallback_model_provider.as_str(), diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index daf0fb3d73..d0b8c7f949 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -134,6 +134,7 @@ use codex_core::InitialHistory; use codex_core::NewThread; use codex_core::RolloutRecorder; use codex_core::SessionMeta; +use codex_core::ThreadConfigSnapshot; use codex_core::ThreadManager; use codex_core::ThreadSortKey as CoreThreadSortKey; use codex_core::auth::CLIENT_ID; @@ -172,6 +173,7 @@ use codex_protocol::config_types::ForcedLoginMethod; use codex_protocol::config_types::Personality; use codex_protocol::items::TurnItem; use codex_protocol::models::ResponseItem; +use codex_protocol::protocol::AgentStatus; use codex_protocol::protocol::GitInfo as CoreGitInfo; use codex_protocol::protocol::McpAuthStatus as CoreMcpAuthStatus; use codex_protocol::protocol::McpServerRefreshConfig; @@ -192,7 +194,6 @@ use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; use std::time::Duration; -use tokio::select; use tokio::sync::Mutex; use tokio::sync::broadcast; use tokio::sync::oneshot; @@ -1378,11 +1379,23 @@ impl CodexMessageProcessor { session_configured, .. } = new_thread; + let rollout_path = match session_configured.rollout_path { + Some(path) => path, + None => { + let error = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: "rollout path missing for v1 conversation".to_string(), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + }; let response = NewConversationResponse { conversation_id: thread_id, model: session_configured.model, reasoning_effort: session_configured.reasoning_effort, - rollout_path: session_configured.rollout_path, + rollout_path, }; self.outgoing.send_response(request_id, response).await; } @@ -1398,7 +1411,7 @@ impl CodexMessageProcessor { } async fn thread_start(&mut self, request_id: RequestId, params: ThreadStartParams) { - let typesafe_overrides = self.build_thread_config_overrides( + let mut typesafe_overrides = self.build_thread_config_overrides( params.model, params.model_provider, params.cwd, @@ -1408,6 +1421,7 @@ impl CodexMessageProcessor { params.developer_instructions, params.personality, ); + typesafe_overrides.ephemeral = Some(params.ephemeral.unwrap_or_default()); let config = match derive_config_from_params(&self.cli_overrides, params.config, typesafe_overrides) @@ -1429,50 +1443,45 @@ impl CodexMessageProcessor { Ok(new_conv) => { let NewThread { thread_id, + thread, session_configured, .. } = new_conv; - let rollout_path = session_configured.rollout_path.clone(); + let config_snapshot = thread.config_snapshot().await; let fallback_provider = self.config.model_provider_id.as_str(); // A bit hacky, but the summary contains a lot of useful information for the thread // that unfortunately does not get returned from thread_manager.start_thread(). - let thread = match read_summary_from_rollout( - rollout_path.as_path(), - fallback_provider, - ) - .await - { - Ok(summary) => summary_to_thread(summary), - Err(err) => { - self.send_internal_error( - request_id, - format!( - "failed to load rollout `{}` for thread {thread_id}: {err}", - rollout_path.display() - ), - ) - .await; - return; + let thread = match session_configured.rollout_path.as_ref() { + Some(rollout_path) => { + match read_summary_from_rollout(rollout_path.as_path(), fallback_provider) + .await + { + Ok(summary) => summary_to_thread(summary), + Err(err) => { + self.send_internal_error( + request_id, + format!( + "failed to load rollout `{}` for thread {thread_id}: {err}", + rollout_path.display() + ), + ) + .await; + return; + } + } } + None => build_ephemeral_thread(thread_id, &config_snapshot), }; - let SessionConfiguredEvent { - model, - model_provider_id, - cwd, - approval_policy, - sandbox_policy, - .. - } = session_configured; let response = ThreadStartResponse { thread: thread.clone(), - model, - model_provider: model_provider_id, - cwd, - approval_policy: approval_policy.into(), - sandbox: sandbox_policy.into(), - reasoning_effort: session_configured.reasoning_effort, + model: config_snapshot.model, + model_provider: config_snapshot.model_provider_id, + cwd: config_snapshot.cwd, + approval_policy: config_snapshot.approval_policy.into(), + sandbox: config_snapshot.sandbox_policy.into(), + reasoning_effort: config_snapshot.reasoning_effort, }; // Auto-attach a thread listener when starting a thread. @@ -1725,7 +1734,7 @@ impl CodexMessageProcessor { self.outgoing.send_response(request_id, response).await; } - async fn thread_read(&self, request_id: RequestId, params: ThreadReadParams) { + async fn thread_read(&mut self, request_id: RequestId, params: ThreadReadParams) { let ThreadReadParams { thread_id, include_turns, @@ -1744,15 +1753,8 @@ impl CodexMessageProcessor { match find_thread_path_by_id_str(&self.config.codex_home, &thread_uuid.to_string()) .await { - Ok(Some(path)) => path, - Ok(None) => { - self.send_invalid_request_error( - request_id, - format!("no rollout found for thread id {thread_uuid}"), - ) - .await; - return; - } + Ok(Some(path)) => Some(path), + Ok(None) => None, Err(err) => { self.send_invalid_request_error( request_id, @@ -1763,24 +1765,45 @@ impl CodexMessageProcessor { } }; - let fallback_provider = self.config.model_provider_id.as_str(); - let mut thread = match read_summary_from_rollout(&rollout_path, fallback_provider).await { - Ok(summary) => summary_to_thread(summary), - Err(err) => { - self.send_internal_error( + let mut thread = if let Some(rollout_path) = rollout_path.as_ref() { + let fallback_provider = self.config.model_provider_id.as_str(); + match read_summary_from_rollout(rollout_path, fallback_provider).await { + Ok(summary) => summary_to_thread(summary), + Err(err) => { + self.send_internal_error( + request_id, + format!( + "failed to load rollout `{}` for thread {thread_uuid}: {err}", + rollout_path.display() + ), + ) + .await; + return; + } + } + } else { + let Ok(thread) = self.thread_manager.get_thread(thread_uuid).await else { + self.send_invalid_request_error( request_id, - format!( - "failed to load rollout `{}` for thread {thread_uuid}: {err}", - rollout_path.display() - ), + format!("thread not loaded: {thread_uuid}"), + ) + .await; + return; + }; + let config_snapshot = thread.config_snapshot().await; + if include_turns { + self.send_invalid_request_error( + request_id, + "ephemeral threads do not support includeTurns".to_string(), ) .await; return; } + build_ephemeral_thread(thread_uuid, &config_snapshot) }; - if include_turns { - match read_event_msgs_from_rollout(&rollout_path).await { + if include_turns && let Some(rollout_path) = rollout_path.as_ref() { + match read_event_msgs_from_rollout(rollout_path).await { Ok(events) => { thread.turns = build_turns_from_event_msgs(&events); } @@ -1967,6 +1990,14 @@ impl CodexMessageProcessor { initial_messages, .. } = session_configured; + let Some(rollout_path) = rollout_path else { + self.send_internal_error( + request_id, + format!("rollout path missing for thread {thread_id}"), + ) + .await; + return; + }; // Auto-attach a thread listener when resuming a thread. if let Err(err) = self .attach_conversation_listener(thread_id, false, ApiVersion::V2) @@ -2170,6 +2201,14 @@ impl CodexMessageProcessor { initial_messages, .. } = session_configured; + let Some(rollout_path) = rollout_path else { + self.send_internal_error( + request_id, + format!("rollout path missing for thread {thread_id}"), + ) + .await; + return; + }; // Auto-attach a conversation listener when forking a thread. if let Err(err) = self .attach_conversation_listener(thread_id, false, ApiVersion::V2) @@ -2919,6 +2958,18 @@ impl CodexMessageProcessor { session_configured, .. }) => { + let rollout_path = match session_configured.rollout_path.clone() { + Some(path) => path, + None => { + let error = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: "rollout path missing for resumed conversation".to_string(), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + }; self.outgoing .send_server_notification(ServerNotification::SessionConfigured( SessionConfiguredNotification { @@ -2928,7 +2979,7 @@ impl CodexMessageProcessor { history_log_id: session_configured.history_log_id, history_entry_count: session_configured.history_entry_count, initial_messages: session_configured.initial_messages.clone(), - rollout_path: session_configured.rollout_path.clone(), + rollout_path: rollout_path.clone(), }, )) .await; @@ -2941,7 +2992,7 @@ impl CodexMessageProcessor { conversation_id: thread_id, model: session_configured.model.clone(), initial_messages, - rollout_path: session_configured.rollout_path.clone(), + rollout_path, }; self.outgoing.send_response(request_id, response).await; } @@ -3117,6 +3168,19 @@ impl CodexMessageProcessor { } }; + let rollout_path = match session_configured.rollout_path.clone() { + Some(path) => path, + None => { + let error = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: "rollout path missing for forked conversation".to_string(), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + }; + self.outgoing .send_server_notification(ServerNotification::SessionConfigured( SessionConfiguredNotification { @@ -3126,7 +3190,7 @@ impl CodexMessageProcessor { history_log_id: session_configured.history_log_id, history_entry_count: session_configured.history_entry_count, initial_messages: session_configured.initial_messages.clone(), - rollout_path: session_configured.rollout_path.clone(), + rollout_path: rollout_path.clone(), }, )) .await; @@ -3139,7 +3203,7 @@ impl CodexMessageProcessor { conversation_id: thread_id, model: session_configured.model.clone(), initial_messages, - rollout_path: session_configured.rollout_path.clone(), + rollout_path, }; self.outgoing.send_response(request_id, response).await; } @@ -3250,51 +3314,27 @@ impl CodexMessageProcessor { // If the thread is active, request shutdown and wait briefly. if let Some(conversation) = self.thread_manager.remove_thread(&thread_id).await { info!("thread {thread_id} was active; shutting down"); - let conversation_clone = conversation.clone(); - let notify = Arc::new(tokio::sync::Notify::new()); - let notify_clone = notify.clone(); - - // Establish the listener for ShutdownComplete before submitting - // Shutdown so it is not missed. - let is_shutdown = tokio::spawn(async move { - // Create the notified future outside the loop to avoid losing notifications. - let notified = notify_clone.notified(); - tokio::pin!(notified); - loop { - select! { - _ = &mut notified => { break; } - event = conversation_clone.next_event() => { - match event { - Ok(event) => { - if matches!(event.msg, EventMsg::ShutdownComplete) { break; } - } - // Break on errors to avoid tight loops when the agent loop has exited. - Err(_) => { break; } - } - } - } - } - }); // Request shutdown. match conversation.submit(Op::Shutdown).await { Ok(_) => { - // Successfully submitted Shutdown; wait before proceeding. - select! { - _ = is_shutdown => { - // Normal shutdown: proceed with archive. - } - _ = tokio::time::sleep(Duration::from_secs(10)) => { - warn!("thread {thread_id} shutdown timed out; proceeding with archive"); - // Wake any waiter; use notify_waiters to avoid missing the signal. - notify.notify_waiters(); - // Perhaps we lost a shutdown race, so let's continue to - // clean up the .jsonl file. + // Poll agent status rather than consuming events so attached listeners do not block shutdown. + let wait_for_shutdown = async { + loop { + if matches!(conversation.agent_status().await, AgentStatus::Shutdown) { + break; + } + tokio::time::sleep(Duration::from_millis(50)).await; } + }; + if tokio::time::timeout(Duration::from_secs(10), wait_for_shutdown) + .await + .is_err() + { + warn!("thread {thread_id} shutdown timed out; proceeding with archive"); } } Err(err) => { error!("failed to submit Shutdown to thread {thread_id}: {err}"); - notify.notify_waiters(); } } } @@ -3809,23 +3849,29 @@ impl CodexMessageProcessor { ); } - let rollout_path = review_thread.rollout_path(); let fallback_provider = self.config.model_provider_id.as_str(); - match read_summary_from_rollout(rollout_path.as_path(), fallback_provider).await { - Ok(summary) => { - let thread = summary_to_thread(summary); - let notif = ThreadStartedNotification { thread }; - self.outgoing - .send_server_notification(ServerNotification::ThreadStarted(notif)) - .await; - } - Err(err) => { - tracing::warn!( - "failed to load summary for review thread {}: {}", - session_configured.session_id, - err - ); + if let Some(rollout_path) = review_thread.rollout_path() { + match read_summary_from_rollout(rollout_path.as_path(), fallback_provider).await { + Ok(summary) => { + let thread = summary_to_thread(summary); + let notif = ThreadStartedNotification { thread }; + self.outgoing + .send_server_notification(ServerNotification::ThreadStarted(notif)) + .await; + } + Err(err) => { + tracing::warn!( + "failed to load summary for review thread {}: {}", + session_configured.session_id, + err + ); + } } + } else { + tracing::warn!( + "review thread {} has no rollout path", + session_configured.session_id + ); } let turn_id = review_thread @@ -4228,7 +4274,7 @@ impl CodexMessageProcessor { async fn resolve_rollout_path(&self, conversation_id: ThreadId) -> Option { match self.thread_manager.get_thread(conversation_id).await { - Ok(conv) => Some(conv.rollout_path()), + Ok(conv) => conv.rollout_path(), Err(_) => None, } } @@ -4493,6 +4539,23 @@ async fn read_updated_at(path: &Path, created_at: Option<&str>) -> Option Thread { + let now = time::OffsetDateTime::now_utc().unix_timestamp(); + Thread { + id: thread_id.to_string(), + preview: String::new(), + model_provider: config_snapshot.model_provider_id.clone(), + created_at: now, + updated_at: now, + path: None, + cwd: config_snapshot.cwd.clone(), + cli_version: env!("CARGO_PKG_VERSION").to_string(), + source: config_snapshot.session_source.clone().into(), + git_info: None, + turns: Vec::new(), + } +} + pub(crate) fn summary_to_thread(summary: ConversationSummary) -> Thread { let ConversationSummary { conversation_id, @@ -4521,7 +4584,7 @@ pub(crate) fn summary_to_thread(summary: ConversationSummary) -> Thread { model_provider, created_at: created_at.map(|dt| dt.timestamp()).unwrap_or(0), updated_at: updated_at.map(|dt| dt.timestamp()).unwrap_or(0), - path, + path: Some(path), cwd, cli_version, source: source.into(), diff --git a/codex-rs/app-server/tests/suite/v2/collaboration_mode_list.rs b/codex-rs/app-server/tests/suite/v2/collaboration_mode_list.rs index cb9ff3e18e..7d25810261 100644 --- a/codex-rs/app-server/tests/suite/v2/collaboration_mode_list.rs +++ b/codex-rs/app-server/tests/suite/v2/collaboration_mode_list.rs @@ -16,7 +16,7 @@ use codex_app_server_protocol::CollaborationModeListResponse; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_core::models_manager::test_builtin_collaboration_mode_presets; -use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::ModeKind; use pretty_assertions::assert_eq; use tempfile::TempDir; @@ -45,13 +45,23 @@ async fn list_collaboration_modes_returns_presets() -> Result<()> { let CollaborationModeListResponse { data: items } = to_response::(response)?; - let expected = vec![ + let expected = [ plan_preset(), code_preset(), pair_programming_preset(), execute_preset(), ]; - assert_eq!(expected, items); + assert_eq!(expected.len(), items.len()); + for (expected_mask, actual_mask) in expected.iter().zip(items.iter()) { + assert_eq!(expected_mask.name, actual_mask.name); + assert_eq!(expected_mask.mode, actual_mask.mode); + assert_eq!(expected_mask.model, actual_mask.model); + assert_eq!(expected_mask.reasoning_effort, actual_mask.reasoning_effort); + assert_eq!( + expected_mask.developer_instructions, + actual_mask.developer_instructions + ); + } Ok(()) } @@ -59,11 +69,11 @@ async fn list_collaboration_modes_returns_presets() -> Result<()> { /// /// If the defaults change in the app server, this helper should be updated alongside the /// contract, or the test will fail in ways that imply a regression in the API. -fn plan_preset() -> CollaborationMode { +fn plan_preset() -> CollaborationModeMask { let presets = test_builtin_collaboration_mode_presets(); presets .into_iter() - .find(|p| p.mode == ModeKind::Plan) + .find(|p| p.mode == Some(ModeKind::Plan)) .unwrap() } @@ -71,20 +81,20 @@ fn plan_preset() -> CollaborationMode { /// /// The helper keeps the expected model and reasoning defaults co-located with the test /// so that mismatches point directly at the API contract being exercised. -fn pair_programming_preset() -> CollaborationMode { +fn pair_programming_preset() -> CollaborationModeMask { let presets = test_builtin_collaboration_mode_presets(); presets .into_iter() - .find(|p| p.mode == ModeKind::PairProgramming) + .find(|p| p.mode == Some(ModeKind::PairProgramming)) .unwrap() } /// Builds the code preset that the list response is expected to return. -fn code_preset() -> CollaborationMode { +fn code_preset() -> CollaborationModeMask { let presets = test_builtin_collaboration_mode_presets(); presets .into_iter() - .find(|p| p.mode == ModeKind::Code) + .find(|p| p.mode == Some(ModeKind::Code)) .unwrap() } @@ -92,10 +102,10 @@ fn code_preset() -> CollaborationMode { /// /// The execute preset uses a different reasoning effort to capture the higher-effort /// execution contract the server currently exposes. -fn execute_preset() -> CollaborationMode { +fn execute_preset() -> CollaborationModeMask { let presets = test_builtin_collaboration_mode_presets(); presets .into_iter() - .find(|p| p.mode == ModeKind::Execute) + .find(|p| p.mode == Some(ModeKind::Execute)) .unwrap() } diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index 32abc237e3..c06f387fd2 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -77,8 +77,9 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> { assert_ne!(thread.id, conversation_id); assert_eq!(thread.preview, preview); assert_eq!(thread.model_provider, "mock_provider"); - assert!(thread.path.is_absolute()); - assert_ne!(thread.path, original_path); + let thread_path = thread.path.clone().expect("thread path"); + assert!(thread_path.is_absolute()); + assert_ne!(thread_path, original_path); assert!(thread.cwd.is_absolute()); assert_eq!(thread.source, SessionSource::VsCode); diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index 8704a25468..d8ca3aa696 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -64,7 +64,7 @@ async fn thread_read_returns_summary_without_turns() -> Result<()> { assert_eq!(thread.id, conversation_id); assert_eq!(thread.preview, preview); assert_eq!(thread.model_provider, "mock_provider"); - assert!(thread.path.is_absolute()); + assert!(thread.path.as_ref().expect("thread path").is_absolute()); assert_eq!(thread.cwd, PathBuf::from("/")); assert_eq!(thread.cli_version, "0.0.0"); assert_eq!(thread.source, SessionSource::Cli); diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 183532ea7d..6129c0de2f 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -117,7 +117,7 @@ async fn thread_resume_returns_rollout_history() -> Result<()> { assert_eq!(thread.id, conversation_id); assert_eq!(thread.preview, preview); assert_eq!(thread.model_provider, "mock_provider"); - assert!(thread.path.is_absolute()); + assert!(thread.path.as_ref().expect("thread path").is_absolute()); assert_eq!(thread.cwd, PathBuf::from("/")); assert_eq!(thread.cli_version, "0.0.0"); assert_eq!(thread.source, SessionSource::Cli); @@ -169,7 +169,7 @@ async fn thread_resume_prefers_path_over_thread_id() -> Result<()> { .await??; let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; - let thread_path = thread.path.clone(); + let thread_path = thread.path.clone().expect("thread path"); let resume_id = mcp .send_thread_resume_request(ThreadResumeParams { thread_id: "not-a-valid-thread-id".to_string(), diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 332f6314b5..103cc4b349 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -396,8 +396,7 @@ fn run_update_action(action: UpdateAction) -> anyhow::Result<()> { if !status.success() { anyhow::bail!("`{cmd_str}` failed with status {status}"); } - println!(); - println!("šŸŽ‰ Update ran successfully! Please restart Codex."); + println!("\nšŸŽ‰ Update ran successfully! Please restart Codex."); Ok(()) } diff --git a/codex-rs/codex-api/src/requests/headers.rs b/codex-rs/codex-api/src/requests/headers.rs index 02f08724ff..02f8c61c31 100644 --- a/codex-rs/codex-api/src/requests/headers.rs +++ b/codex-rs/codex-api/src/requests/headers.rs @@ -15,13 +15,12 @@ pub(crate) fn subagent_header(source: &Option) -> Option return None; }; match sub { + codex_protocol::protocol::SubAgentSource::Review => Some("review".to_string()), + codex_protocol::protocol::SubAgentSource::Compact => Some("compact".to_string()), + codex_protocol::protocol::SubAgentSource::ThreadSpawn { .. } => { + Some("collab_spawn".to_string()) + } codex_protocol::protocol::SubAgentSource::Other(label) => Some(label.clone()), - other => Some( - serde_json::to_value(other) - .ok() - .and_then(|v| v.as_str().map(std::string::ToString::to_string)) - .unwrap_or_else(|| "other".to_string()), - ), } } diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 6c5ebf9f02..611c0d164c 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -39,12 +39,20 @@ impl AgentControl { &self, config: crate::config::Config, prompt: String, + session_source: Option, ) -> CodexResult { let state = self.upgrade()?; let reservation = self.state.reserve_spawn_slot(config.agent_max_threads)?; // The same `AgentControl` is sent to spawn the thread. - let new_thread = state.spawn_new_thread(config, self.clone()).await?; + let new_thread = match session_source { + Some(session_source) => { + state + .spawn_new_thread_with_source(config, self.clone(), session_source) + .await? + } + None => state.spawn_new_thread(config, self.clone()).await?, + }; reservation.commit(new_thread.thread_id); // Notify a new thread has been created. This notification will be processed by clients @@ -268,7 +276,7 @@ mod tests { let control = AgentControl::default(); let (_home, config) = test_config().await; let err = control - .spawn_agent(config, "hello".to_string()) + .spawn_agent(config, "hello".to_string(), None) .await .expect_err("spawn_agent should fail without a manager"); assert_eq!( @@ -370,7 +378,7 @@ mod tests { let harness = AgentControlHarness::new().await; let thread_id = harness .control - .spawn_agent(harness.config.clone(), "spawned".to_string()) + .spawn_agent(harness.config.clone(), "spawned".to_string(), None) .await .expect("spawn_agent should succeed"); let _thread = harness @@ -417,12 +425,12 @@ mod tests { .expect("start thread"); let first_agent_id = control - .spawn_agent(config.clone(), "hello".to_string()) + .spawn_agent(config.clone(), "hello".to_string(), None) .await .expect("spawn_agent should succeed"); let err = control - .spawn_agent(config, "hello again".to_string()) + .spawn_agent(config, "hello again".to_string(), None) .await .expect_err("spawn_agent should respect max threads"); let CodexErr::AgentLimitReached { @@ -455,7 +463,7 @@ mod tests { let control = manager.agent_control(); let first_agent_id = control - .spawn_agent(config.clone(), "hello".to_string()) + .spawn_agent(config.clone(), "hello".to_string(), None) .await .expect("spawn_agent should succeed"); let _ = control @@ -464,7 +472,7 @@ mod tests { .expect("shutdown agent"); let second_agent_id = control - .spawn_agent(config.clone(), "hello again".to_string()) + .spawn_agent(config.clone(), "hello again".to_string(), None) .await .expect("spawn_agent should succeed after shutdown"); let _ = control @@ -490,12 +498,12 @@ mod tests { let cloned = control.clone(); let first_agent_id = cloned - .spawn_agent(config.clone(), "hello".to_string()) + .spawn_agent(config.clone(), "hello".to_string(), None) .await .expect("spawn_agent should succeed"); let err = control - .spawn_agent(config, "hello again".to_string()) + .spawn_agent(config, "hello again".to_string(), None) .await .expect_err("spawn_agent should respect shared guard"); let CodexErr::AgentLimitReached { max_threads } = err else { diff --git a/codex-rs/core/src/agent/guards.rs b/codex-rs/core/src/agent/guards.rs index c384ed7cd8..2f146f2f80 100644 --- a/codex-rs/core/src/agent/guards.rs +++ b/codex-rs/core/src/agent/guards.rs @@ -1,6 +1,8 @@ use crate::error::CodexErr; use crate::error::Result; use codex_protocol::ThreadId; +use codex_protocol::protocol::SessionSource; +use codex_protocol::protocol::SubAgentSource; use std::collections::HashSet; use std::sync::Arc; use std::sync::Mutex; @@ -19,6 +21,25 @@ pub(crate) struct Guards { total_count: AtomicUsize, } +/// Initial agent is depth 0. +pub(crate) const MAX_THREAD_SPAWN_DEPTH: i32 = 1; + +fn session_depth(session_source: &SessionSource) -> i32 { + match session_source { + SessionSource::SubAgent(SubAgentSource::ThreadSpawn { depth, .. }) => *depth, + SessionSource::SubAgent(_) => 0, + _ => 0, + } +} + +pub(crate) fn next_thread_spawn_depth(session_source: &SessionSource) -> i32 { + session_depth(session_source).saturating_add(1) +} + +pub(crate) fn exceeds_thread_spawn_depth_limit(depth: i32) -> bool { + depth > MAX_THREAD_SPAWN_DEPTH +} + impl Guards { pub(crate) fn reserve_spawn_slot( self: &Arc, @@ -102,6 +123,30 @@ mod tests { use super::*; use pretty_assertions::assert_eq; + #[test] + fn session_depth_defaults_to_zero_for_root_sources() { + assert_eq!(session_depth(&SessionSource::Cli), 0); + } + + #[test] + fn thread_spawn_depth_increments_and_enforces_limit() { + let session_source = SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id: ThreadId::new(), + depth: 1, + }); + let child_depth = next_thread_spawn_depth(&session_source); + assert_eq!(child_depth, 2); + assert!(exceeds_thread_spawn_depth_limit(child_depth)); + } + + #[test] + fn non_thread_spawn_subagents_default_to_depth_zero() { + let session_source = SessionSource::SubAgent(SubAgentSource::Review); + assert_eq!(session_depth(&session_source), 0); + assert_eq!(next_thread_spawn_depth(&session_source), 1); + assert!(!exceeds_thread_spawn_depth_limit(1)); + } + #[test] fn reservation_drop_releases_slot() { let guards = Arc::new(Guards::default()); diff --git a/codex-rs/core/src/agent/mod.rs b/codex-rs/core/src/agent/mod.rs index 180f70dbe6..03652e43e5 100644 --- a/codex-rs/core/src/agent/mod.rs +++ b/codex-rs/core/src/agent/mod.rs @@ -1,10 +1,12 @@ pub(crate) mod control; -// Do not put in `pub` or `pub(crate)`. This code should not be used somewhere else. mod guards; pub(crate) mod role; pub(crate) mod status; pub(crate) use codex_protocol::protocol::AgentStatus; pub(crate) use control::AgentControl; +pub(crate) use guards::MAX_THREAD_SPAWN_DEPTH; +pub(crate) use guards::exceeds_thread_spawn_depth_limit; +pub(crate) use guards::next_thread_spawn_depth; pub(crate) use role::AgentRole; pub(crate) use status::agent_status_from_event; diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index f0f2b125fd..437380deef 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -226,13 +226,11 @@ impl ModelClient { let mut extra_headers = ApiHeaderMap::new(); if let SessionSource::SubAgent(sub) = &self.state.session_source { - let subagent = if let crate::protocol::SubAgentSource::Other(label) = sub { - label.clone() - } else { - serde_json::to_value(sub) - .ok() - .and_then(|v| v.as_str().map(std::string::ToString::to_string)) - .unwrap_or_else(|| "other".to_string()) + let subagent = match sub { + crate::protocol::SubAgentSource::Review => "review".to_string(), + crate::protocol::SubAgentSource::Compact => "compact".to_string(), + crate::protocol::SubAgentSource::ThreadSpawn { .. } => "collab_spawn".to_string(), + crate::protocol::SubAgentSource::Other(label) => label.clone(), }; if let Ok(val) = HeaderValue::from_str(&subagent) { extra_headers.insert("x-openai-subagent", val); diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 25c2b225a3..9fc2bae165 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -89,6 +89,7 @@ use crate::client::ModelClient; use crate::client::ModelClientSession; use crate::client_common::Prompt; use crate::client_common::ResponseEvent; +use crate::codex_thread::ThreadConfigSnapshot; use crate::compact::collect_user_messages; use crate::config::Config; use crate::config::Constrained; @@ -192,6 +193,7 @@ pub struct Codex { pub(crate) rx_event: Receiver, // Last known status of the agent. pub(crate) agent_status: watch::Receiver, + pub(crate) session: Arc, } /// Wrapper returned by [`Codex::spawn`] containing the spawned [`Codex`], @@ -345,12 +347,13 @@ impl Codex { let thread_id = session.conversation_id; // This task will run until Op::Shutdown is received. - tokio::spawn(submission_loop(session, config, rx_sub)); + tokio::spawn(submission_loop(Arc::clone(&session), config, rx_sub)); let codex = Codex { next_id: AtomicU64::new(0), tx_sub, rx_event, agent_status: agent_status_rx, + session, }; #[allow(deprecated)] @@ -394,6 +397,11 @@ impl Codex { pub(crate) async fn agent_status(&self) -> AgentStatus { self.agent_status.borrow().clone() } + + pub(crate) async fn thread_config_snapshot(&self) -> ThreadConfigSnapshot { + let state = self.session.state.lock().await; + state.session_configuration.thread_config_snapshot() + } } /// Context for an initialized model agent @@ -495,6 +503,19 @@ pub(crate) struct SessionConfiguration { } impl SessionConfiguration { + fn thread_config_snapshot(&self) -> ThreadConfigSnapshot { + ThreadConfigSnapshot { + model: self.collaboration_mode.model().to_string(), + model_provider_id: self.original_config_do_not_use.model_provider_id.clone(), + approval_policy: self.approval_policy.value(), + sandbox_policy: self.sandbox_policy.get().clone(), + cwd: self.cwd.clone(), + reasoning_effort: self.collaboration_mode.reasoning_effort(), + personality: self.personality, + session_source: self.session_source.clone(), + } + } + pub(crate) fn apply(&self, updates: &SessionSettingsUpdate) -> ConstraintResult { let mut next_configuration = self.clone(); if let Some(collaboration_mode) = updates.collaboration_mode.clone() { @@ -652,7 +673,15 @@ impl Session { // - initialize RolloutRecorder with new or resumed session info // - perform default shell discovery // - load history metadata - let rollout_fut = RolloutRecorder::new(&config, rollout_params); + let rollout_fut = async { + if config.ephemeral { + Ok(None) + } else { + RolloutRecorder::new(&config, rollout_params) + .await + .map(Some) + } + }; let history_meta_fut = crate::message_history::history_metadata(&config); let auth_manager_clone = Arc::clone(&auth_manager); @@ -679,7 +708,9 @@ impl Session { error!("failed to initialize rollout recorder: {e:#}"); anyhow::Error::from(e) })?; - let rollout_path = rollout_recorder.rollout_path.clone(); + let rollout_path = rollout_recorder + .as_ref() + .map(|rec| rec.rollout_path.clone()); let mut post_session_configured_events = Vec::::new(); @@ -768,7 +799,7 @@ impl Session { mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::default(), notifier: UserNotifier::new(config.notify.clone()), - rollout: Mutex::new(Some(rollout_recorder)), + rollout: Mutex::new(rollout_recorder), user_shell: Arc::new(default_shell), show_raw_agent_reasoning: config.show_raw_agent_reasoning, exec_policy, diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index b855612113..9a94f988d5 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -92,6 +92,7 @@ pub(crate) async fn run_codex_thread_interactive( tx_sub: tx_ops, rx_event: rx_sub, agent_status: codex.agent_status.clone(), + session: Arc::clone(&codex.session), }) } @@ -134,6 +135,7 @@ pub(crate) async fn run_codex_thread_one_shot( let (tx_bridge, rx_bridge) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let ops_tx = io.tx_sub.clone(); let agent_status = io.agent_status.clone(); + let session = Arc::clone(&io.session); let io_for_bridge = io; tokio::spawn(async move { while let Ok(event) = io_for_bridge.next_event().await { @@ -166,6 +168,7 @@ pub(crate) async fn run_codex_thread_one_shot( rx_event: rx_bridge, tx_sub: tx_closed, agent_status, + session, }) } @@ -442,15 +445,15 @@ mod tests { let (tx_events, rx_events) = bounded(1); let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY); let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit); + let (session, ctx, _rx_evt) = crate::codex::make_session_and_context_with_rx().await; let codex = Arc::new(Codex { next_id: AtomicU64::new(0), tx_sub, rx_event: rx_events, agent_status, + session: Arc::clone(&session), }); - let (session, ctx, _rx_evt) = crate::codex::make_session_and_context_with_rx().await; - let (tx_out, rx_out) = bounded(1); tx_out .send(Event { diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 6d132a9256..152679d146 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -4,18 +4,35 @@ use crate::error::Result as CodexResult; use crate::protocol::Event; use crate::protocol::Op; use crate::protocol::Submission; +use codex_protocol::config_types::Personality; +use codex_protocol::openai_models::ReasoningEffort; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::protocol::SessionSource; use std::path::PathBuf; use tokio::sync::watch; +#[derive(Clone, Debug)] +pub struct ThreadConfigSnapshot { + pub model: String, + pub model_provider_id: String, + pub approval_policy: AskForApproval, + pub sandbox_policy: SandboxPolicy, + pub cwd: PathBuf, + pub reasoning_effort: Option, + pub personality: Option, + pub session_source: SessionSource, +} + pub struct CodexThread { codex: Codex, - rollout_path: PathBuf, + rollout_path: Option, } /// Conduit for the bidirectional stream of messages that compose a thread /// (formerly called a conversation) in Codex. impl CodexThread { - pub(crate) fn new(codex: Codex, rollout_path: PathBuf) -> Self { + pub(crate) fn new(codex: Codex, rollout_path: Option) -> Self { Self { codex, rollout_path, @@ -43,7 +60,11 @@ impl CodexThread { self.codex.agent_status.clone() } - pub fn rollout_path(&self) -> PathBuf { + pub fn rollout_path(&self) -> Option { self.rollout_path.clone() } + + pub async fn config_snapshot(&self) -> ThreadConfigSnapshot { + self.codex.thread_config_snapshot().await + } } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index fae370ec0e..ca3ccc1ffd 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -89,7 +89,7 @@ pub use codex_git::GhostSnapshotConfig; /// files are *silently truncated* to this size so we do not take up too much of /// the context window. pub(crate) const PROJECT_DOC_MAX_BYTES: usize = 32 * 1024; // 32 KiB -pub(crate) const DEFAULT_AGENT_MAX_THREADS: Option = None; +pub(crate) const DEFAULT_AGENT_MAX_THREADS: Option = Some(6); pub const CONFIG_TOML_FILE: &str = "config.toml"; @@ -261,6 +261,9 @@ pub struct Config { /// Settings that govern if and what will be written to `~/.codex/history.jsonl`. pub history: History, + /// When true, session is not persisted on disk. Default to `false` + pub ephemeral: bool, + /// Optional URI-based file opener. If set, citations to files in the model /// output will be hyperlinked using the specified URI scheme. pub file_opener: UriBasedFileOpener, @@ -1158,6 +1161,7 @@ pub struct ConfigOverrides { pub include_apply_patch_tool: Option, pub show_raw_agent_reasoning: Option, pub tools_web_search_request: Option, + pub ephemeral: Option, /// Additional directories that should be treated as writable roots for this session. pub additional_writable_roots: Vec, } @@ -1246,6 +1250,7 @@ impl Config { include_apply_patch_tool: include_apply_patch_tool_override, show_raw_agent_reasoning, tools_web_search_request: override_tools_web_search_request, + ephemeral, additional_writable_roots, } = overrides; @@ -1530,6 +1535,7 @@ impl Config { codex_home, config_layer_stack, history, + ephemeral: ephemeral.unwrap_or_default(), file_opener: cfg.file_opener.unwrap_or(UriBasedFileOpener::VsCode), codex_linux_sandbox_exe, @@ -3687,10 +3693,11 @@ model_verbosity = "high" project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), tool_output_token_limit: None, - agent_max_threads: None, + agent_max_threads: DEFAULT_AGENT_MAX_THREADS, codex_home: fixture.codex_home(), config_layer_stack: Default::default(), history: History::default(), + ephemeral: false, file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, hide_agent_reasoning: false, @@ -3768,10 +3775,11 @@ model_verbosity = "high" project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), tool_output_token_limit: None, - agent_max_threads: None, + agent_max_threads: DEFAULT_AGENT_MAX_THREADS, codex_home: fixture.codex_home(), config_layer_stack: Default::default(), history: History::default(), + ephemeral: false, file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, hide_agent_reasoning: false, @@ -3864,10 +3872,11 @@ model_verbosity = "high" project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), tool_output_token_limit: None, - agent_max_threads: None, + agent_max_threads: DEFAULT_AGENT_MAX_THREADS, codex_home: fixture.codex_home(), config_layer_stack: Default::default(), history: History::default(), + ephemeral: false, file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, hide_agent_reasoning: false, @@ -3946,10 +3955,11 @@ model_verbosity = "high" project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), tool_output_token_limit: None, - agent_max_threads: None, + agent_max_threads: DEFAULT_AGENT_MAX_THREADS, codex_home: fixture.codex_home(), config_layer_stack: Default::default(), history: History::default(), + ephemeral: false, file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, hide_agent_reasoning: false, diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 9a9527aa7a..5e2a0d5d50 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -434,11 +434,7 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::Collab, key: "collab", - stage: Stage::Experimental { - name: "Multi-agents", - menu_description: "Allow Codex to spawn and collaborate with other agents on request (formerly named `collab`).", - announcement: "NEW! Codex can now spawn other agents and work with them to solve your problems. Enable in /experimental!", - }, + stage: Stage::Beta, default_enabled: false, }, FeatureSpec { diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 41d33a4ce4..2042e330ef 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -15,6 +15,7 @@ pub mod codex; mod codex_thread; mod compact_remote; pub use codex_thread::CodexThread; +pub use codex_thread::ThreadConfigSnapshot; mod agent; mod codex_delegate; mod command_safety; diff --git a/codex-rs/core/src/models_manager/collaboration_mode_presets.rs b/codex-rs/core/src/models_manager/collaboration_mode_presets.rs index ac1334c56c..aff111b1ad 100644 --- a/codex-rs/core/src/models_manager/collaboration_mode_presets.rs +++ b/codex-rs/core/src/models_manager/collaboration_mode_presets.rs @@ -1,6 +1,5 @@ -use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::ModeKind; -use codex_protocol::config_types::Settings; use codex_protocol::openai_models::ReasoningEffort; const COLLABORATION_MODE_PLAN: &str = include_str!("../../templates/collaboration_mode/plan.md"); @@ -10,7 +9,7 @@ const COLLABORATION_MODE_PAIR_PROGRAMMING: &str = const COLLABORATION_MODE_EXECUTE: &str = include_str!("../../templates/collaboration_mode/execute.md"); -pub(super) fn builtin_collaboration_mode_presets() -> Vec { +pub(super) fn builtin_collaboration_mode_presets() -> Vec { vec![ plan_preset(), code_preset(), @@ -20,50 +19,46 @@ pub(super) fn builtin_collaboration_mode_presets() -> Vec { } #[cfg(any(test, feature = "test-support"))] -pub fn test_builtin_collaboration_mode_presets() -> Vec { +pub fn test_builtin_collaboration_mode_presets() -> Vec { builtin_collaboration_mode_presets() } -fn plan_preset() -> CollaborationMode { - CollaborationMode { - mode: ModeKind::Plan, - settings: Settings { - model: "gpt-5.2-codex".to_string(), - reasoning_effort: Some(ReasoningEffort::High), - developer_instructions: Some(COLLABORATION_MODE_PLAN.to_string()), - }, +fn plan_preset() -> CollaborationModeMask { + CollaborationModeMask { + name: "Plan".to_string(), + mode: Some(ModeKind::Plan), + model: None, + reasoning_effort: Some(Some(ReasoningEffort::High)), + developer_instructions: Some(Some(COLLABORATION_MODE_PLAN.to_string())), } } -fn code_preset() -> CollaborationMode { - CollaborationMode { - mode: ModeKind::Code, - settings: Settings { - model: "gpt-5.2-codex".to_string(), - reasoning_effort: Some(ReasoningEffort::Medium), - developer_instructions: Some(COLLABORATION_MODE_CODE.to_string()), - }, +fn code_preset() -> CollaborationModeMask { + CollaborationModeMask { + name: "Code".to_string(), + mode: Some(ModeKind::Code), + model: None, + reasoning_effort: None, + developer_instructions: Some(Some(COLLABORATION_MODE_CODE.to_string())), } } -fn pair_programming_preset() -> CollaborationMode { - CollaborationMode { - mode: ModeKind::PairProgramming, - settings: Settings { - model: "gpt-5.2-codex".to_string(), - reasoning_effort: Some(ReasoningEffort::Medium), - developer_instructions: Some(COLLABORATION_MODE_PAIR_PROGRAMMING.to_string()), - }, +fn pair_programming_preset() -> CollaborationModeMask { + CollaborationModeMask { + name: "Pair Programming".to_string(), + mode: Some(ModeKind::PairProgramming), + model: None, + reasoning_effort: Some(Some(ReasoningEffort::Medium)), + developer_instructions: Some(Some(COLLABORATION_MODE_PAIR_PROGRAMMING.to_string())), } } -fn execute_preset() -> CollaborationMode { - CollaborationMode { - mode: ModeKind::Execute, - settings: Settings { - model: "gpt-5.2-codex".to_string(), - reasoning_effort: Some(ReasoningEffort::High), - developer_instructions: Some(COLLABORATION_MODE_EXECUTE.to_string()), - }, +fn execute_preset() -> CollaborationModeMask { + CollaborationModeMask { + name: "Execute".to_string(), + mode: Some(ModeKind::Execute), + model: None, + reasoning_effort: Some(Some(ReasoningEffort::High)), + developer_instructions: Some(Some(COLLABORATION_MODE_EXECUTE.to_string())), } } diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index efa60507ea..58049b4747 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -14,7 +14,7 @@ use crate::models_manager::model_presets::builtin_model_presets; use codex_api::ModelsClient; use codex_api::ReqwestTransport; use codex_app_server_protocol::AuthMode; -use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ModelsResponse; @@ -91,7 +91,7 @@ impl ModelsManager { /// List collaboration mode presets. /// /// Returns a static set of presets seeded with the configured model. - pub fn list_collaboration_modes(&self) -> Vec { + pub fn list_collaboration_modes(&self) -> Vec { builtin_collaboration_mode_presets() } diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 70d8cb7900..bc5cd4f19e 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -19,7 +19,7 @@ use crate::rollout::RolloutRecorder; use crate::rollout::truncation; use crate::skills::SkillsManager; use codex_protocol::ThreadId; -use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::openai_models::ModelPreset; use codex_protocol::protocol::InitialHistory; use codex_protocol::protocol::McpServerRefreshConfig; @@ -158,7 +158,7 @@ impl ThreadManager { .await } - pub fn list_collaboration_modes(&self) -> Vec { + pub fn list_collaboration_modes(&self) -> Vec { self.state.models_manager.list_collaboration_modes() } @@ -314,11 +314,22 @@ impl ThreadManagerState { config: Config, agent_control: AgentControl, ) -> CodexResult { - self.spawn_thread( + self.spawn_new_thread_with_source(config, agent_control, self.session_source.clone()) + .await + } + + pub(crate) async fn spawn_new_thread_with_source( + &self, + config: Config, + agent_control: AgentControl, + session_source: SessionSource, + ) -> CodexResult { + self.spawn_thread_with_source( config, InitialHistory::New, Arc::clone(&self.auth_manager), agent_control, + session_source, ) .await } @@ -330,6 +341,24 @@ impl ThreadManagerState { initial_history: InitialHistory, auth_manager: Arc, agent_control: AgentControl, + ) -> CodexResult { + self.spawn_thread_with_source( + config, + initial_history, + auth_manager, + agent_control, + self.session_source.clone(), + ) + .await + } + + pub(crate) async fn spawn_thread_with_source( + &self, + config: Config, + initial_history: InitialHistory, + auth_manager: Arc, + agent_control: AgentControl, + session_source: SessionSource, ) -> CodexResult { let CodexSpawnOk { codex, thread_id, .. @@ -339,7 +368,7 @@ impl ThreadManagerState { Arc::clone(&self.models_manager), Arc::clone(&self.skills_manager), initial_history, - self.session_source.clone(), + session_source, agent_control, ) .await?; diff --git a/codex-rs/core/src/tools/handlers/collab.rs b/codex-rs/core/src/tools/handlers/collab.rs index 56463e644a..83ee53905b 100644 --- a/codex-rs/core/src/tools/handlers/collab.rs +++ b/codex-rs/core/src/tools/handlers/collab.rs @@ -78,6 +78,11 @@ impl ToolHandler for CollabHandler { mod spawn { use super::*; use crate::agent::AgentRole; + use crate::agent::MAX_THREAD_SPAWN_DEPTH; + use crate::agent::exceeds_thread_spawn_depth_limit; + use crate::agent::next_thread_spawn_depth; + use codex_protocol::protocol::SessionSource; + use codex_protocol::protocol::SubAgentSource; use std::sync::Arc; #[derive(Debug, Deserialize)] @@ -105,6 +110,13 @@ mod spawn { "Empty message can't be sent to an agent".to_string(), )); } + let session_source = turn.client.get_session_source(); + let child_depth = next_thread_spawn_depth(&session_source); + if exceeds_thread_spawn_depth_limit(child_depth) { + return Err(FunctionCallError::RespondToModel(format!( + "agent depth limit reached: max depth is {MAX_THREAD_SPAWN_DEPTH}" + ))); + } session .send_event( &turn, @@ -125,7 +137,14 @@ mod spawn { let result = session .services .agent_control - .spawn_agent(config, prompt.clone()) + .spawn_agent( + config, + prompt.clone(), + Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id: session.conversation_id, + depth: child_depth, + })), + ) .await .map_err(collab_spawn_error); let (new_thread_id, status) = match &result { @@ -573,7 +592,6 @@ fn build_agent_spawn_config( config.model_reasoning_summary = turn.client.get_reasoning_summary(); config.developer_instructions = turn.developer_instructions.clone(); config.compact_prompt = turn.compact_prompt.clone(); - config.user_instructions = turn.user_instructions.clone(); config.shell_environment_policy = turn.shell_environment_policy.clone(); config.codex_linux_sandbox_exe = turn.codex_linux_sandbox_exe.clone(); config.cwd = turn.cwd.clone(); @@ -597,13 +615,17 @@ mod tests { use super::*; use crate::CodexAuth; use crate::ThreadManager; + use crate::agent::MAX_THREAD_SPAWN_DEPTH; use crate::built_in_model_providers; + use crate::client::ModelClient; use crate::codex::make_session_and_context; use crate::config::types::ShellEnvironmentPolicy; use crate::function_tool::FunctionCallError; use crate::protocol::AskForApproval; use crate::protocol::Op; use crate::protocol::SandboxPolicy; + use crate::protocol::SessionSource; + use crate::protocol::SubAgentSource; use crate::turn_diff_tracker::TurnDiffTracker; use codex_protocol::ThreadId; use pretty_assertions::assert_eq; @@ -723,6 +745,45 @@ mod tests { ); } + #[tokio::test] + async fn spawn_agent_rejects_when_depth_limit_exceeded() { + let (mut session, mut turn) = make_session_and_context().await; + let manager = thread_manager(); + session.services.agent_control = manager.agent_control(); + + let session_source = SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id: session.conversation_id, + depth: MAX_THREAD_SPAWN_DEPTH, + }); + turn.client = ModelClient::new( + turn.client.config(), + Some(session.services.auth_manager.clone()), + turn.client.get_model_info(), + turn.client.get_otel_manager(), + turn.client.get_provider(), + turn.client.get_reasoning_effort(), + turn.client.get_reasoning_summary(), + session.conversation_id, + session_source, + ); + + let invocation = invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({"message": "hello"})), + ); + let Err(err) = CollabHandler.handle(invocation).await else { + panic!("spawn should fail when depth limit exceeded"); + }; + assert_eq!( + err, + FunctionCallError::RespondToModel(format!( + "agent depth limit reached: max depth is {MAX_THREAD_SPAWN_DEPTH}" + )) + ); + } + #[tokio::test] async fn send_input_rejects_empty_message() { let (session, turn) = make_session_and_context().await; @@ -1073,7 +1134,6 @@ mod tests { }; turn.developer_instructions = Some("dev".to_string()); turn.compact_prompt = Some("compact".to_string()); - turn.user_instructions = Some("user".to_string()); turn.shell_environment_policy = ShellEnvironmentPolicy { use_profile: true, ..ShellEnvironmentPolicy::default() @@ -1093,7 +1153,6 @@ mod tests { expected.model_reasoning_summary = turn.client.get_reasoning_summary(); expected.developer_instructions = turn.developer_instructions.clone(); expected.compact_prompt = turn.compact_prompt.clone(); - expected.user_instructions = turn.user_instructions.clone(); expected.shell_environment_policy = turn.shell_environment_policy.clone(); expected.codex_linux_sandbox_exe = turn.codex_linux_sandbox_exe.clone(); expected.cwd = turn.cwd.clone(); @@ -1107,4 +1166,31 @@ mod tests { .expect("sandbox policy set"); assert_eq!(config, expected); } + + #[tokio::test] + async fn build_agent_spawn_config_preserves_base_user_instructions() { + let (session, mut turn) = make_session_and_context().await; + let session_source = turn.client.get_session_source(); + let mut base_config = (*turn.client.config()).clone(); + base_config.user_instructions = Some("base-user".to_string()); + turn.user_instructions = Some("resolved-user".to_string()); + turn.client = ModelClient::new( + Arc::new(base_config.clone()), + Some(session.services.auth_manager.clone()), + turn.client.get_model_info(), + turn.client.get_otel_manager(), + turn.client.get_provider(), + turn.client.get_reasoning_effort(), + turn.client.get_reasoning_summary(), + session.conversation_id, + session_source, + ); + let base_instructions = BaseInstructions { + text: "base".to_string(), + }; + + let config = build_agent_spawn_config(&base_instructions, &turn).expect("spawn config"); + + assert_eq!(config.user_instructions, base_config.user_instructions); + } } diff --git a/codex-rs/core/templates/collab/experimental_prompt.md b/codex-rs/core/templates/collab/experimental_prompt.md index 9763592e27..c6cd6f7ac4 100644 --- a/codex-rs/core/templates/collab/experimental_prompt.md +++ b/codex-rs/core/templates/collab/experimental_prompt.md @@ -13,6 +13,3 @@ This feature must be used wisely. For simple or straightforward tasks, you don't * When you're done with a sub-agent, don't forget to close it using `close_agent`. * Be careful on the `timeout_ms` parameter you choose for `wait`. It should be wisely scaled. * Sub-agents have access to the same set of tools as you do so you must tell them if they are allowed to spawn sub-agents themselves or not. - -## Global -You're name is Batman diff --git a/codex-rs/core/templates/collaboration_mode/plan.md b/codex-rs/core/templates/collaboration_mode/plan.md index 7d78645de2..3d0447c729 100644 --- a/codex-rs/core/templates/collaboration_mode/plan.md +++ b/codex-rs/core/templates/collaboration_mode/plan.md @@ -1,283 +1,41 @@ -# Collaboration Style: Plan +# Plan Mode (Conversational) -You work in **two phases**: +You work in 2 phases and you should *chat your way* to a great plan before finalizing it. -- **PHASE 1 — Understand user intent**: Align on what the user is trying to accomplish and what ā€œsuccessā€ means. Focus on intent, scope, constraints, and preference tradeoffs. -- **PHASE 2 — Technical spec & implementation plan**: Convert the intent into a decision‑complete technical spec and an implementation plan detailed enough that another agent could execute with minimal follow‑ups. +PHASE 1 — Intent chat (what they actually want) +- Keep asking until you can clearly state: goal + success criteria, audience, in/out of scope, constraints, current state, and the key preferences/tradeoffs. +- Bias toward questions over guessing: if any high‑impact ambiguity remains, do NOT plan yet—ask. +- Include a ā€œConfirm my understandingā€ question in each round (so the user can correct you early). ---- +PHASE 2 — Implementation chat (what/how we’ll build) +- Once intent is stable, keep asking until the spec is decision‑complete: approach, interfaces (APIs/schemas/I/O), data flow, edge cases/failure modes, testing + acceptance criteria, rollout/monitoring, and any migrations/compat constraints. ## Hard interaction rule (critical) - -Every assistant turn MUST be **exactly one** of: - -**A) A `request_user_input` tool call** (to gather requirements and iterate), OR -**B) The final plan output** (**plan‑only**, with a good title). - -Constraints: -- **Do NOT ask questions in free text.** All questions MUST be asked via `request_user_input`. -- **Do NOT mix** a `request_user_input` call with plan content in the same turn. -- You may use internal tools to explore (repo search, file reading, environment inspection) **before** emitting either A or B, but the user‑visible output must still be exactly A or B. - ---- - -## Two types of uncertainty (treat differently) - -### Type 1 — Discoverable facts (repo/system truth) -Examples: ā€œWhere is app‑server 2 defined?ā€, ā€œWhich config sets turn duration?ā€, ā€œWhich service emits this metric?ā€ - -Rule: **Evidence-first exploration applies.** Don’t ask the user until you’ve searched. - -### Type 2 — Preferences & tradeoffs (product and engineering intent) - -Rule: **Ask early** These are often *not discoverable* and should not be silently assumed when multiple approaches are viable. - ---- - -## Evidence‑first exploration (precondition to asking discoverable questions) - -When a repo / codebase / workspace is available (or implied), you MUST attempt to resolve discoverable questions by **exploring first**. - -Before calling `request_user_input` for a discoverable fact, do a quick investigation pass: -- Run at least **2 targeted searches** (exact match + a likely variant/synonym). -- Check the most likely ā€œsource of truthā€ surfaces (service manifests, infra configs, env/config files, entrypoints, schemas/types/constants). - -You may ask the user ONLY if, after exploration: -- There are **multiple plausible candidates** and picking wrong would materially change the implementation, OR -- Nothing is found and you need a **missing identifier**, environment name, external dependency, or non-repo context, OR -- The repo reveals ambiguity that must be resolved by product intent (not code). - -If you found a **single best match**, DO NOT ask the user — proceed and record it as an assumption in the final plan. - -If you must ask, incorporate what you already found: -- Provide **options listing the candidates** you discovered (paths/service names), with a **recommended** option. -- Do NOT ask the user to ā€œpoint to the pathā€ unless you have **zero candidates** after searching. - ---- - -## Preference capture (you SHOULD ask when it changes the plan) - -If there are **multiple reasonable implementation approaches** with meaningful tradeoffs, you SHOULD ask the user to choose their preference even if you could assume a default. - -Treat tradeoff choice as **high-impact** unless the user explicitly said: -- ā€œUse your best judgement,ā€ or -- ā€œPick whatever is simplest,ā€ or -- ā€œI don’t care—ship fast.ā€ - -When asking a preference question: -- Provide **2–4 mutually exclusive options**. -- Include a **recommended default** that matches the user’s apparent goals. -- If the user doesn’t answer, proceed with the recommended option and record it as an assumption. - ---- - -## No‑trivia rule for questions (guardrail) - -You MUST NOT ask questions whose answers are likely to be found by: -- repo text search, -- reading config/infra manifests, -- following imports/types/constants, -unless you already attempted those and can summarize what you found. - -Every `request_user_input` question must: -- materially change an implementation decision, OR -- disambiguate between **concrete candidates** you already found, OR -- capture a **preference/tradeoff** that is not discoverable from the repo. - ---- - -## PHASE 1 — Understand user intent - -### Purpose -Identify what the user actually wants, what matters most, and what constraints + preferences shape the solution. - -### Phase 1 principles -- State what you think the user cares about (speed vs quality, prototype vs production, etc.). -- Think out loud briefly when it helps weigh tradeoffs. -- Use reasonable suggestions with explicit assumptions; make it easy to accept/override. -- Ask fewer, better questions. Ask only what materially changes the spec/plan OR captures a real tradeoff. -- Think ahead: propose helpful suggestions the user may need (testing, debug mode, observability, migration path). - -### Phase 1 exit criteria (Intent gate) -Before moving to Phase 2, ensure you have either a **user answer** OR an **explicit assumption** for: - -**Intent basics** -- Primary goal + success criteria (how we know it worked) -- Primary user / audience -- In-scope and out-of-scope -- Constraints (time, budget, platform, security/compliance) -- Current context (what exists today: code/system/data) - -**Preference profile (don’t silently assume if unclear and high-impact)** -- Risk posture: prototype vs production quality bar -- Tradeoff priority: ship fast vs robust/maintainable -- Compatibility expectations: backward compatibility / migrations / downtime tolerance (if relevant) - -Use `request_user_input` to deeply understand the user's intent after exploring your environment. - ---- - -## PHASE 2 — Technical spec & implementation plan - -### Purpose -Turn the intent into a buildable, decision-complete technical spec. - -### Phase 2 exit criteria (Spec gate) -Before finalizing the plan, ensure you’ve pinned down (answer or assumption): -- Chosen approach + 1–2 alternatives with tradeoffs -- Interfaces (APIs, schemas, inputs/outputs) -- Data flow + key edge cases / failure modes -- Testing + acceptance criteria -- Rollout/monitoring expectations -- Any key preference/tradeoff decisions (and rationale) - -If something is high-impact and unknown, ask via `request_user_input`. Otherwise assume defaults and proceed. - ---- - -## Using `request_user_input` in Plan Mode - -Use `request_user_input` when either: -1) You are genuinely blocked on a decision that materially changes the plan and cannot be resolved via evidence-first exploration, OR -2) There is a meaningful **preference/tradeoff** the user should choose among. -3) When an answer is skipped, assume the recommended path. - +Every assistant turn MUST be exactly one of: +A) a `request_user_input` tool call (questions/options only), OR +B) the final output: a titled, plan‑only document. Rules: -- **Default to options** when there are ≤ 4 common outcomes; include a **recommended** option. -- Use **free-form only** when truly unbounded (e.g., ā€œpaste schemaā€, ā€œshare constraintsā€, ā€œprovide examplesā€). -- Every question must be tied to a decision that changes the spec (A→X, B→Y). -- If you found candidates in the repo, options MUST reference them (paths/service names) so the user chooses among concrete items. +- No questions in free text (only via `request_user_input`). +- Never mix a `request_user_input` call with plan content. +- Internal tool/repo exploration is allowed privately before A or B. -Do **not** use `request_user_input` to ask: -- ā€œis my plan ready?ā€ / ā€œshould I proceed?ā€ -- ā€œwhere is X?ā€ when repo search can answer it. +## Ask a lot, but never ask trivia +You SHOULD ask many questions, but each question must: +- materially change the spec/plan, OR +- confirm/lock an assumption, OR +- choose between meaningful tradeoffs. +Batch questions (e.g., 4–10) per `request_user_input` call to keep momentum. -(If your environment enforces a limit, aim to resolve within ~5 `request_user_input` calls; if still blocked, ask only the most decision-critical remaining question(s) and proceed with explicit assumptions.) +## Two kinds of unknowns (treat differently) +1) Discoverable facts (repo/system truth): explore first. + - Before asking, run ≄2 targeted searches (exact + variant) and check likely sources of truth (configs/manifests/entrypoints/schemas/types/constants). + - Ask only if: multiple plausible candidates; nothing found but you need a missing identifier/context; or ambiguity is actually product intent. + - If asking, present concrete candidates (paths/service names) + recommend one. -### Examples (technical, schema-populated) +2) Preferences/tradeoffs (not discoverable): ask early. + - Provide 2–4 mutually exclusive options + a recommended default. + - If unanswered, proceed with the recommended option and record it as an assumption in the final plan. -**1) Boolean (yes/no), no free-form** -```json -{ - "questions": [ - { - "id": "enable_migration", - "header": "Migrate", - "question": "Enable the database migration in this release?", - "options": [ - { "label": "Yes (Recommended)", "description": "Ship the migration with this rollout." }, - { "label": "No", "description": "Defer the migration to a later release." } - ] - } - ] -} -```` - -**2) Preference/tradeoff question (recommended + options)** - -```json -{ - "questions": [ - { - "id": "tradeoff_priority", - "header": "Tradeoff", - "question": "Which priority should guide the implementation?", - "options": [ - { "label": "Ship fast (Recommended)", "description": "Minimal changes, pragmatic shortcuts, faster delivery." }, - { "label": "Robust & maintainable", "description": "Cleaner abstractions, more refactor, better long-term stability." }, - { "label": "Performance-first", "description": "Optimize latency/throughput even if complexity rises." }, - { "label": "Other", "description": "Specify a different priority or constraint." } - ] - } - ] -} -``` - -**3) Free-form only (no options)** - -```json -{ - "questions": [ - { - "id": "acceptance_criteria", - "header": "Success", - "question": "What are the acceptance criteria or success metrics we should optimize for?" - } - ] -} -``` - ---- - -## Iterating and final output - -Only AFTER you have all the information (or explicit assumptions for remaining low-impact unknowns), write the full plan. - -A good plan here is **decision-complete**: it contains the concrete choices, interfaces, acceptance criteria, and rollout details needed for another agent to execute with minimal back-and-forth. - -### Plan output (what to include) - -Your plan MUST include the sections below. Keep them concise but specific; include only what’s relevant to the task. - -1. **Title** - -* A clear, specific title describing what will be built/delivered. - -2. **Goal & Success Criteria** - -* What outcome we’re driving. -* Concrete acceptance criteria (tests, metrics, or observable behavior). Prefer ā€œdone when ā€¦ā€. - -3. **Non-goals / Out of Scope** - -* Explicit boundaries to prevent scope creep. - -4. **Assumptions** - -* Any defaults you assumed due to missing info, labeled clearly. - -5. **Proposed Solution** - -* The chosen approach (with rationale). -* 1–2 alternatives considered and why they were not chosen (brief tradeoffs). - -6. **System Design** - -* Architecture / components / data flow (only as deep as needed). -* Key invariants, edge cases, and failure modes (and how they’re handled). - -7. **Interfaces & Data Contracts** - -* APIs, schemas, inputs/outputs, event formats, config flags, etc. -* Validation rules and backward/forward compatibility expectations if applicable. - -8. **Execution Details** - -* Concrete implementation steps and ordering. -* **Codebase specifics are conditional**: include file/module/function names, directories, migrations, and dependencies **only when relevant and known** (or when you can reasonably infer them). -* If unknown, specify what to discover and how (e.g., ā€œsearch for X symbolā€, ā€œlocate Y service entrypointā€). - -9. **Testing & Quality** - -* Test strategy (unit/integration/e2e) proportional to risk. -* How to verify locally and in staging; include any test data or harness needs. - -10. **Rollout, Observability, and Ops** - -* Release strategy (flags, gradual rollout, migration plan). -* Monitoring/alerts/logging and dashboards to add or update. -* Rollback strategy and operational playbook notes (brief). - -11. **Risks & Mitigations** - -* Top risks (technical, product, security, privacy, performance). -* Specific mitigations and ā€œwatch-outsā€. - -12. **Open Questions** - -* Only if something truly must be resolved later; include how to resolve and what decision it affects. - -### Plan output (strict) - -**The final output should contain the plan and plan only with a good title.** -PLEASE DO NOT confirm the plan with the user before ending. The user will be responsible for telling us to update, iterate or execute the plan. \ No newline at end of file +## Finalization rule +Only output the final plan when remaining unknowns are low‑impact and explicitly listed as assumptions. +Final output must be plan‑only with a good title (no ā€œshould I proceed?ā€). \ No newline at end of file diff --git a/codex-rs/core/tests/suite/collaboration_instructions.rs b/codex-rs/core/tests/suite/collaboration_instructions.rs index 3c6619fcea..f7183b817e 100644 --- a/codex-rs/core/tests/suite/collaboration_instructions.rs +++ b/codex-rs/core/tests/suite/collaboration_instructions.rs @@ -422,7 +422,11 @@ async fn resume_replays_collaboration_instructions() -> Result<()> { let mut builder = test_codex(); let initial = builder.build(&server).await?; - let rollout_path = initial.session_configured.rollout_path.clone(); + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .expect("rollout path"); let home = initial.home.clone(); let collab_text = "resume instructions"; diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 230ab57840..1ecac3afe0 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -154,7 +154,7 @@ async fn summarize_context_three_requests_and_instructions() { session_configured, .. } = thread_manager.start_thread(config).await.unwrap(); - let rollout_path = session_configured.rollout_path; + let rollout_path = session_configured.rollout_path.expect("rollout path"); // 1) Normal user input – should hit server once. codex @@ -1237,7 +1237,11 @@ async fn auto_compact_runs_after_resume_when_token_usage_is_over_limit() { }); let initial = builder.build(&server).await.unwrap(); let home = initial.home.clone(); - let rollout_path = initial.session_configured.rollout_path.clone(); + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .expect("rollout path"); // A single over-limit completion should not auto-compact until the next user message. mount_sse_once( @@ -1429,7 +1433,7 @@ async fn auto_compact_persists_rollout_entries() { codex.submit(Op::Shutdown).await.unwrap(); wait_for_event(&codex, |ev| matches!(ev, EventMsg::ShutdownComplete)).await; - let rollout_path = session_configured.rollout_path; + let rollout_path = session_configured.rollout_path.expect("rollout path"); let text = std::fs::read_to_string(&rollout_path).unwrap_or_else(|e| { panic!( "failed to read rollout file {}: {e}", diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index a2cf73e110..2fc5ba53c2 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -230,7 +230,12 @@ async fn remote_compact_persists_replacement_history_in_rollout() -> Result<()> ) .await?; let codex = harness.test().codex.clone(); - let rollout_path = harness.test().session_configured.rollout_path.clone(); + let rollout_path = harness + .test() + .session_configured + .rollout_path + .clone() + .expect("rollout path"); let responses_mock = responses::mount_sse_once( harness.server(), diff --git a/codex-rs/core/tests/suite/compact_resume_fork.rs b/codex-rs/core/tests/suite/compact_resume_fork.rs index b7ce49abbe..f6047ac733 100644 --- a/codex-rs/core/tests/suite/compact_resume_fork.rs +++ b/codex-rs/core/tests/suite/compact_resume_fork.rs @@ -1013,7 +1013,7 @@ async fn compact_conversation(conversation: &Arc) { } async fn fetch_conversation_path(conversation: &Arc) -> std::path::PathBuf { - conversation.rollout_path() + conversation.rollout_path().expect("rollout path") } async fn resume_conversation( diff --git a/codex-rs/core/tests/suite/fork_thread.rs b/codex-rs/core/tests/suite/fork_thread.rs index 2a9f604c3d..8698991846 100644 --- a/codex-rs/core/tests/suite/fork_thread.rs +++ b/codex-rs/core/tests/suite/fork_thread.rs @@ -80,7 +80,7 @@ async fn fork_thread_twice_drops_to_first_message() { } // Request history from the base conversation to obtain rollout path. - let base_path = codex.rollout_path(); + let base_path = codex.rollout_path().expect("rollout path"); // GetHistory flushes before returning the path; no wait needed. @@ -135,7 +135,7 @@ async fn fork_thread_twice_drops_to_first_message() { .await .expect("fork 1"); - let fork1_path = codex_fork1.rollout_path(); + let fork1_path = codex_fork1.rollout_path().expect("rollout path"); // GetHistory on fork1 flushed; the file is ready. let fork1_items = read_items(&fork1_path); @@ -154,7 +154,7 @@ async fn fork_thread_twice_drops_to_first_message() { .await .expect("fork 2"); - let fork2_path = codex_fork2.rollout_path(); + let fork2_path = codex_fork2.rollout_path().expect("rollout path"); // GetHistory on fork2 flushed; the file is ready. let fork1_items = read_items(&fork1_path); let fork1_user_inputs = find_user_input_positions(&fork1_items); diff --git a/codex-rs/core/tests/suite/image_rollout.rs b/codex-rs/core/tests/suite/image_rollout.rs index b6dad6a227..6aebe6630d 100644 --- a/codex-rs/core/tests/suite/image_rollout.rs +++ b/codex-rs/core/tests/suite/image_rollout.rs @@ -136,7 +136,7 @@ async fn copy_paste_local_image_persists_rollout_request_shape() -> anyhow::Resu codex.submit(Op::Shutdown).await?; wait_for_event(&codex, |event| matches!(event, EventMsg::ShutdownComplete)).await; - let rollout_path = codex.rollout_path(); + let rollout_path = codex.rollout_path().expect("rollout path"); let rollout_text = read_rollout_text(&rollout_path).await?; let actual = find_user_message_with_image(&rollout_text) .expect("expected user message with input image in rollout"); @@ -217,7 +217,7 @@ async fn drag_drop_image_persists_rollout_request_shape() -> anyhow::Result<()> codex.submit(Op::Shutdown).await?; wait_for_event(&codex, |event| matches!(event, EventMsg::ShutdownComplete)).await; - let rollout_path = codex.rollout_path(); + let rollout_path = codex.rollout_path().expect("rollout path"); let rollout_text = read_rollout_text(&rollout_path).await?; let actual = find_user_message_with_image(&rollout_text) .expect("expected user message with input image in rollout"); diff --git a/codex-rs/core/tests/suite/override_updates.rs b/codex-rs/core/tests/suite/override_updates.rs index 3ceb847daa..d995446864 100644 --- a/codex-rs/core/tests/suite/override_updates.rs +++ b/codex-rs/core/tests/suite/override_updates.rs @@ -129,7 +129,7 @@ async fn override_turn_context_records_permissions_update() -> Result<()> { test.codex.submit(Op::Shutdown).await?; wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::ShutdownComplete)).await; - let rollout_path = test.codex.rollout_path(); + let rollout_path = test.codex.rollout_path().expect("rollout path"); let rollout_text = read_rollout_text(&rollout_path).await?; let developer_texts = rollout_developer_texts(&rollout_text); let approval_texts: Vec<&String> = developer_texts @@ -172,7 +172,7 @@ async fn override_turn_context_records_environment_update() -> Result<()> { test.codex.submit(Op::Shutdown).await?; wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::ShutdownComplete)).await; - let rollout_path = test.codex.rollout_path(); + let rollout_path = test.codex.rollout_path().expect("rollout path"); let rollout_text = read_rollout_text(&rollout_path).await?; let env_texts = rollout_environment_texts(&rollout_text); let new_cwd_text = new_cwd.path().display().to_string(); @@ -209,7 +209,7 @@ async fn override_turn_context_records_collaboration_update() -> Result<()> { test.codex.submit(Op::Shutdown).await?; wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::ShutdownComplete)).await; - let rollout_path = test.codex.rollout_path(); + let rollout_path = test.codex.rollout_path().expect("rollout path"); let rollout_text = read_rollout_text(&rollout_path).await?; let developer_texts = rollout_developer_texts(&rollout_text); let collab_text = collab_xml(collab_text); diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index 9a32a7d9f9..f3d9e8d47b 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -202,7 +202,11 @@ async fn resume_replays_permissions_messages() -> Result<()> { config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); }); let initial = builder.build(&server).await?; - let rollout_path = initial.session_configured.rollout_path.clone(); + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .expect("rollout path"); let home = initial.home.clone(); initial @@ -280,7 +284,11 @@ async fn resume_and_fork_append_permissions_messages() -> Result<()> { config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); }); let initial = builder.build(&server).await?; - let rollout_path = initial.session_configured.rollout_path.clone(); + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .expect("rollout path"); let home = initial.home.clone(); initial diff --git a/codex-rs/core/tests/suite/resume.rs b/codex-rs/core/tests/suite/resume.rs index d0718a334d..a7be42f430 100644 --- a/codex-rs/core/tests/suite/resume.rs +++ b/codex-rs/core/tests/suite/resume.rs @@ -26,7 +26,11 @@ async fn resume_includes_initial_messages_from_rollout_events() -> Result<()> { let initial = builder.build(&server).await?; let codex = Arc::clone(&initial.codex); let home = initial.home.clone(); - let rollout_path = initial.session_configured.rollout_path.clone(); + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .expect("rollout path"); let initial_sse = sse(vec![ ev_response_created("resp-initial"), @@ -85,7 +89,11 @@ async fn resume_includes_initial_messages_from_reasoning_events() -> Result<()> let initial = builder.build(&server).await?; let codex = Arc::clone(&initial.codex); let home = initial.home.clone(); - let rollout_path = initial.session_configured.rollout_path.clone(); + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .expect("rollout path"); let initial_sse = sse(vec![ ev_response_created("resp-initial"), @@ -143,7 +151,11 @@ async fn resume_switches_models_preserves_base_instructions() -> Result<()> { let initial = builder.build(&server).await?; let codex = Arc::clone(&initial.codex); let home = initial.home.clone(); - let rollout_path = initial.session_configured.rollout_path.clone(); + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .expect("rollout path"); let initial_sse = sse(vec![ ev_response_created("resp-initial"), diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index 3a66e320b6..df678f9e9d 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -120,7 +120,7 @@ async fn review_op_emits_lifecycle_and_review_output() { // Also verify that a user message with the header and a formatted finding // was recorded back in the parent session's rollout. - let path = codex.rollout_path(); + let path = codex.rollout_path().expect("rollout path"); let text = std::fs::read_to_string(&path).expect("read rollout file"); let mut saw_header = false; @@ -627,7 +627,7 @@ async fn review_input_isolated_from_parent_history() { assert_eq!(instructions, REVIEW_PROMPT); // Also verify that a user interruption note was recorded in the rollout. - let path = codex.rollout_path(); + let path = codex.rollout_path().expect("rollout path"); let text = std::fs::read_to_string(&path).expect("read rollout file"); let mut saw_interruption_message = false; for line in text.lines() { diff --git a/codex-rs/docs/codex_mcp_interface.md b/codex-rs/docs/codex_mcp_interface.md index 19e1d91581..1aa5aa12ae 100644 --- a/codex-rs/docs/codex_mcp_interface.md +++ b/codex-rs/docs/codex_mcp_interface.md @@ -101,7 +101,8 @@ Each response yields: Fetch the built-in collaboration mode presets with `collaborationMode/list`. This endpoint does not accept pagination and returns the full list in one response: -- `data` – ordered list of collaboration mode presets +- `data` – ordered list of collaboration mode masks (partial settings to apply on top of the base mode) + - For tri-state fields like `reasoning_effort` and `developer_instructions`, omit the field to keep the current value, set it to `null` to clear it, or set a concrete value to update it. ## Event stream diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 7679891287..441041aa68 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -229,6 +229,7 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any include_apply_patch_tool: None, show_raw_agent_reasoning: oss.then_some(true), tools_web_search_request: None, + ephemeral: None, additional_writable_roots: add_dir, }; diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index e0b2509667..43de1f95f8 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -85,7 +85,7 @@ fn session_configured_produces_thread_started_event() { history_log_id: 0, history_entry_count: 0, initial_messages: None, - rollout_path, + rollout_path: Some(rollout_path), }), ); let out = ep.collect_thread_events(&ev); diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index b7c66592ff..2069db69d0 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -267,7 +267,7 @@ mod tests { history_log_id: 1, history_entry_count: 1000, initial_messages: None, - rollout_path: rollout_file.path().to_path_buf(), + rollout_path: Some(rollout_file.path().to_path_buf()), }), }; @@ -307,7 +307,7 @@ mod tests { history_log_id: 1, history_entry_count: 1000, initial_messages: None, - rollout_path: rollout_file.path().to_path_buf(), + rollout_path: Some(rollout_file.path().to_path_buf()), }; let event = Event { id: "1".to_string(), @@ -371,7 +371,7 @@ mod tests { history_log_id: 1, history_entry_count: 1000, initial_messages: None, - rollout_path: rollout_file.path().to_path_buf(), + rollout_path: Some(rollout_file.path().to_path_buf()), }; let event = Event { id: "1".to_string(), diff --git a/codex-rs/protocol/src/config_types.rs b/codex-rs/protocol/src/config_types.rs index 444901bf75..ae956797ea 100644 --- a/codex-rs/protocol/src/config_types.rs +++ b/codex-rs/protocol/src/config_types.rs @@ -188,21 +188,21 @@ impl CollaborationMode { /// /// - `model`: `Some(s)` to update the model, `None` to keep the current model /// - `effort`: `Some(Some(e))` to set effort to `e`, `Some(None)` to clear effort, `None` to keep current effort - /// - `developer_instructions`: `Some(s)` to update developer instructions, `None` to keep current + /// - `developer_instructions`: `Some(Some(s))` to set instructions, `Some(None)` to clear them, `None` to keep current /// /// Returns a new `CollaborationMode` with updated values, preserving the mode. pub fn with_updates( &self, model: Option, effort: Option>, - developer_instructions: Option, + developer_instructions: Option>, ) -> Self { let settings = self.settings_ref(); let updated_settings = Settings { model: model.unwrap_or_else(|| settings.model.clone()), reasoning_effort: effort.unwrap_or(settings.reasoning_effort), developer_instructions: developer_instructions - .or_else(|| settings.developer_instructions.clone()), + .unwrap_or_else(|| settings.developer_instructions.clone()), }; CollaborationMode { @@ -210,6 +210,26 @@ impl CollaborationMode { settings: updated_settings, } } + + /// Applies a mask to this collaboration mode, returning a new collaboration mode + /// with the mask values applied. Fields in the mask that are `Some` will override + /// the corresponding fields, while `None` values will preserve the original values. + /// + /// The `name` field in the mask is ignored as it's metadata for the mask itself. + pub fn apply_mask(&self, mask: &CollaborationModeMask) -> Self { + let settings = self.settings_ref(); + CollaborationMode { + mode: mask.mode.unwrap_or(self.mode), + settings: Settings { + model: mask.model.clone().unwrap_or_else(|| settings.model.clone()), + reasoning_effort: mask.reasoning_effort.unwrap_or(settings.reasoning_effort), + developer_instructions: mask + .developer_instructions + .clone() + .unwrap_or_else(|| settings.developer_instructions.clone()), + }, + } + } } /// Settings for a collaboration mode. @@ -219,3 +239,49 @@ pub struct Settings { pub reasoning_effort: Option, pub developer_instructions: Option, } + +/// A mask for collaboration mode settings, allowing partial updates. +/// All fields except `name` are optional, enabling selective updates. +#[derive(Clone, PartialEq, Eq, Hash, Debug, Serialize, Deserialize, JsonSchema, TS)] +pub struct CollaborationModeMask { + pub name: String, + pub mode: Option, + pub model: Option, + pub reasoning_effort: Option>, + pub developer_instructions: Option>, +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn apply_mask_can_clear_optional_fields() { + let mode = CollaborationMode { + mode: ModeKind::Code, + settings: Settings { + model: "gpt-5.2-codex".to_string(), + reasoning_effort: Some(ReasoningEffort::High), + developer_instructions: Some("stay focused".to_string()), + }, + }; + let mask = CollaborationModeMask { + name: "Clear".to_string(), + mode: None, + model: None, + reasoning_effort: Some(None), + developer_instructions: Some(None), + }; + + let expected = CollaborationMode { + mode: ModeKind::Code, + settings: Settings { + model: "gpt-5.2-codex".to_string(), + reasoning_effort: None, + developer_instructions: None, + }, + }; + assert_eq!(expected, mode.apply_mask(&mask)); + } +} diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 1f35fed60e..7bf3c90eab 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1518,6 +1518,10 @@ pub enum SessionSource { pub enum SubAgentSource { Review, Compact, + ThreadSpawn { + parent_thread_id: ThreadId, + depth: i32, + }, Other(String), } @@ -1539,6 +1543,12 @@ impl fmt::Display for SubAgentSource { match self { SubAgentSource::Review => f.write_str("review"), SubAgentSource::Compact => f.write_str("compact"), + SubAgentSource::ThreadSpawn { + parent_thread_id, + depth, + } => { + write!(f, "thread_spawn_{parent_thread_id}_d{depth}") + } SubAgentSource::Other(other) => f.write_str(other), } } @@ -2139,7 +2149,9 @@ pub struct SessionConfiguredEvent { #[serde(skip_serializing_if = "Option::is_none")] pub initial_messages: Option>, - pub rollout_path: PathBuf, + /// Path in which the rollout is stored. Can be `None` for ephemeral threads + #[serde(skip_serializing_if = "Option::is_none")] + pub rollout_path: Option, } /// User's decision in response to an ExecApprovalRequest. @@ -2491,7 +2503,7 @@ mod tests { history_log_id: 0, history_entry_count: 0, initial_messages: None, - rollout_path: rollout_file.path().to_path_buf(), + rollout_path: Some(rollout_file.path().to_path_buf()), }), }; diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 7063f8f5ba..1d53f5d70d 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -12,6 +12,7 @@ use crate::bottom_pane::SelectionViewParams; use crate::bottom_pane::popup_consts::standard_popup_hint_line; use crate::chatwidget::ChatWidget; use crate::chatwidget::ExternalEditorState; +use crate::cwd_prompt::CwdPromptAction; use crate::diff_render::DiffSummary; use crate::exec_command::strip_bash_lc_and_escape; use crate::external_editor; @@ -36,6 +37,8 @@ use codex_core::AuthManager; use codex_core::CodexAuth; use codex_core::ThreadManager; use codex_core::config::Config; +use codex_core::config::ConfigBuilder; +use codex_core::config::ConfigOverrides; use codex_core::config::edit::ConfigEdit; use codex_core::config::edit::ConfigEditsBuilder; use codex_core::config_loader::ConfigLayerStackOrdering; @@ -44,12 +47,14 @@ use codex_core::features::Feature; use codex_core::models_manager::manager::RefreshStrategy; use codex_core::models_manager::model_presets::HIDE_GPT_5_1_CODEX_MAX_MIGRATION_PROMPT_CONFIG; use codex_core::models_manager::model_presets::HIDE_GPT5_1_MIGRATION_PROMPT_CONFIG; +use codex_core::protocol::AskForApproval; use codex_core::protocol::DeprecationNoticeEvent; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::FinalOutput; use codex_core::protocol::ListSkillsResponseEvent; use codex_core::protocol::Op; +use codex_core::protocol::SandboxPolicy; use codex_core::protocol::SessionSource; use codex_core::protocol::SkillErrorInfo; use codex_core::protocol::TokenUsage; @@ -60,6 +65,7 @@ use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ModelUpgrade; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::protocol::SessionConfiguredEvent; +use codex_utils_absolute_path::AbsolutePathBuf; use color_eyre::eyre::Result; use color_eyre::eyre::WrapErr; use crossterm::event::KeyCode; @@ -87,6 +93,7 @@ use tokio::sync::broadcast; use tokio::sync::mpsc; use tokio::sync::mpsc::error::TryRecvError; use tokio::sync::mpsc::unbounded_channel; +use toml::Value as TomlValue; const EXTERNAL_EDITOR_HINT: &str = "Save and close external editor to continue."; const THREAD_EVENT_CHANNEL_CAPACITY: usize = 1024; @@ -498,6 +505,10 @@ pub(crate) struct App { /// Config is stored here so we can recreate ChatWidgets as needed. pub(crate) config: Config, pub(crate) active_profile: Option, + cli_kv_overrides: Vec<(String, TomlValue)>, + harness_overrides: ConfigOverrides, + runtime_approval_policy_override: Option, + runtime_sandbox_policy_override: Option, pub(crate) file_search: FileSearchManager, @@ -545,6 +556,23 @@ struct WindowsSandboxState { skip_world_writable_scan_once: bool, } +fn normalize_harness_overrides_for_cwd( + mut overrides: ConfigOverrides, + base_cwd: &Path, +) -> Result { + if overrides.additional_writable_roots.is_empty() { + return Ok(overrides); + } + + let mut normalized = Vec::with_capacity(overrides.additional_writable_roots.len()); + for root in overrides.additional_writable_roots.drain(..) { + let absolute = AbsolutePathBuf::resolve_path_against_base(root, base_cwd)?; + normalized.push(absolute.into_path_buf()); + } + overrides.additional_writable_roots = normalized; + Ok(overrides) +} + impl App { pub fn chatwidget_init_for_forked_or_resumed_thread( &self, @@ -567,6 +595,38 @@ impl App { } } + async fn rebuild_config_for_cwd(&self, cwd: PathBuf) -> Result { + let mut overrides = self.harness_overrides.clone(); + overrides.cwd = Some(cwd.clone()); + let cwd_display = cwd.display().to_string(); + ConfigBuilder::default() + .codex_home(self.config.codex_home.clone()) + .cli_overrides(self.cli_kv_overrides.clone()) + .harness_overrides(overrides) + .build() + .await + .wrap_err_with(|| format!("Failed to rebuild config for cwd {cwd_display}")) + } + + fn apply_runtime_policy_overrides(&mut self, config: &mut Config) { + if let Some(policy) = self.runtime_approval_policy_override.as_ref() + && let Err(err) = config.approval_policy.set(*policy) + { + tracing::warn!(%err, "failed to carry forward approval policy override"); + self.chat_widget.add_error_message(format!( + "Failed to carry forward approval policy override: {err}" + )); + } + if let Some(policy) = self.runtime_sandbox_policy_override.as_ref() + && let Err(err) = config.sandbox_policy.set(policy.clone()) + { + tracing::warn!(%err, "failed to carry forward sandbox policy override"); + self.chat_widget.add_error_message(format!( + "Failed to carry forward sandbox policy override: {err}" + )); + } + } + async fn shutdown_current_thread(&mut self) { if let Some(thread_id) = self.chat_widget.thread_id() { // Clear any in-flight rollback guard when switching threads. @@ -824,6 +884,8 @@ impl App { tui: &mut tui::Tui, auth_manager: Arc, mut config: Config, + cli_kv_overrides: Vec<(String, TomlValue)>, + harness_overrides: ConfigOverrides, active_profile: Option, initial_prompt: Option, initial_images: Vec, @@ -838,6 +900,8 @@ impl App { emit_deprecation_notice(&app_event_tx, ollama_chat_support_notice); emit_project_config_warnings(&app_event_tx, &config); + let harness_overrides = + normalize_harness_overrides_for_cwd(harness_overrides, &config.cwd)?; let thread_manager = Arc::new(ThreadManager::new( config.codex_home.clone(), auth_manager.clone(), @@ -979,6 +1043,10 @@ impl App { auth_manager: auth_manager.clone(), config, active_profile, + cli_kv_overrides, + harness_overrides, + runtime_approval_policy_override: None, + runtime_sandbox_policy_override: None, file_search, enhanced_keys_supported, transcript_cells: Vec::new(), @@ -1203,6 +1271,34 @@ impl App { .await? { SessionSelection::Resume(path) => { + let current_cwd = self.config.cwd.clone(); + let resume_cwd = match crate::resolve_cwd_for_resume_or_fork( + tui, + ¤t_cwd, + &path, + CwdPromptAction::Resume, + true, + ) + .await? + { + Some(cwd) => cwd, + None => current_cwd.clone(), + }; + let mut resume_config = if crate::cwds_differ(¤t_cwd, &resume_cwd) { + match self.rebuild_config_for_cwd(resume_cwd).await { + Ok(cfg) => cfg, + Err(err) => { + self.chat_widget.add_error_message(format!( + "Failed to rebuild configuration for resume: {err}" + )); + return Ok(AppRunControl::Continue); + } + } + } else { + // No rebuild needed: current_cwd comes from self.config.cwd. + self.config.clone() + }; + self.apply_runtime_policy_overrides(&mut resume_config); let summary = session_summary( self.chat_widget.token_usage(), self.chat_widget.thread_id(), @@ -1210,7 +1306,7 @@ impl App { match self .server .resume_thread_from_rollout( - self.config.clone(), + resume_config.clone(), path.clone(), self.auth_manager.clone(), ) @@ -1218,6 +1314,11 @@ impl App { { Ok(resumed) => { self.shutdown_current_thread().await; + self.config = resume_config; + self.file_search = FileSearchManager::new( + self.config.cwd.clone(), + self.app_event_tx.clone(), + ); let init = self.chatwidget_init_for_forked_or_resumed_thread( tui, self.config.clone(), @@ -1401,10 +1502,8 @@ impl App { AppEvent::UpdateModel(model) => { self.chat_widget.set_model(&model); } - AppEvent::UpdateCollaborationMode(mode) => { - let model = mode.model().to_string(); - self.chat_widget.set_collaboration_mode(mode); - self.chat_widget.set_model(&model); + AppEvent::UpdateCollaborationMode(mask) => { + self.chat_widget.set_collaboration_mask(mask); } AppEvent::OpenReasoningPopup { model } => { self.chat_widget.open_reasoning_popup(model); @@ -1660,6 +1759,13 @@ impl App { } } AppEvent::UpdateAskForApprovalPolicy(policy) => { + self.runtime_approval_policy_override = Some(policy); + if let Err(err) = self.config.approval_policy.set(policy) { + tracing::warn!(%err, "failed to set approval policy on app config"); + self.chat_widget + .add_error_message(format!("Failed to set approval policy: {err}")); + return Ok(AppRunControl::Continue); + } self.chat_widget.set_approval_policy(policy); } AppEvent::UpdateSandboxPolicy(policy) => { @@ -1688,6 +1794,8 @@ impl App { .add_error_message(format!("Failed to set sandbox policy: {err}")); return Ok(AppRunControl::Continue); } + self.runtime_sandbox_policy_override = + Some(self.config.sandbox_policy.get().clone()); // If sandbox policy becomes workspace-write or read-only, run the Windows world-writable scan. #[cfg(target_os = "windows")] @@ -2014,14 +2122,14 @@ impl App { history_log_id: 0, history_entry_count: 0, initial_messages: None, - rollout_path: PathBuf::new(), + rollout_path: Some(PathBuf::new()), }); session_configured.session_id = thread_id; session_configured.forked_from_id = None; session_configured.history_log_id = 0; session_configured.history_entry_count = 0; session_configured.initial_messages = None; - session_configured.rollout_path = PathBuf::new(); + session_configured.rollout_path = Some(PathBuf::new()); session_configured } @@ -2236,6 +2344,7 @@ mod tests { use codex_core::CodexAuth; use codex_core::ThreadManager; use codex_core::config::ConfigBuilder; + use codex_core::config::ConfigOverrides; use codex_core::models_manager::manager::ModelsManager; use codex_core::protocol::AskForApproval; use codex_core::protocol::Event; @@ -2254,6 +2363,25 @@ mod tests { use std::sync::atomic::AtomicBool; use tempfile::tempdir; + #[test] + fn normalize_harness_overrides_resolves_relative_add_dirs() -> Result<()> { + let temp_dir = tempdir()?; + let base_cwd = temp_dir.path().join("base"); + std::fs::create_dir_all(&base_cwd)?; + + let overrides = ConfigOverrides { + additional_writable_roots: vec![PathBuf::from("rel")], + ..Default::default() + }; + let normalized = normalize_harness_overrides_for_cwd(overrides, &base_cwd)?; + + assert_eq!( + normalized.additional_writable_roots, + vec![base_cwd.join("rel")] + ); + Ok(()) + } + async fn make_test_app() -> App { let (chat_widget, app_event_tx, _rx, _op_rx) = make_chatwidget_manual_with_sender().await; let config = chat_widget.config_ref().clone(); @@ -2275,6 +2403,10 @@ mod tests { auth_manager, config, active_profile: None, + cli_kv_overrides: Vec::new(), + harness_overrides: ConfigOverrides::default(), + runtime_approval_policy_override: None, + runtime_sandbox_policy_override: None, file_search, transcript_cells: Vec::new(), overlay: None, @@ -2323,6 +2455,10 @@ mod tests { auth_manager, config, active_profile: None, + cli_kv_overrides: Vec::new(), + harness_overrides: ConfigOverrides::default(), + runtime_approval_policy_override: None, + runtime_sandbox_policy_override: None, file_search, transcript_cells: Vec::new(), overlay: None, @@ -2576,7 +2712,7 @@ mod tests { history_log_id: 0, history_entry_count: 0, initial_messages: None, - rollout_path: PathBuf::new(), + rollout_path: Some(PathBuf::new()), }; Arc::new(new_session_info( app.chat_widget.config_ref(), @@ -2628,7 +2764,7 @@ mod tests { history_log_id: 0, history_entry_count: 0, initial_messages: None, - rollout_path: PathBuf::new(), + rollout_path: Some(PathBuf::new()), }), }); @@ -2673,7 +2809,7 @@ mod tests { history_log_id: 0, history_entry_count: 0, initial_messages: None, - rollout_path: PathBuf::new(), + rollout_path: Some(PathBuf::new()), }; app.chat_widget.handle_codex_event(Event { diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index f6ed6e8abc..9a2d979f1c 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -23,7 +23,7 @@ use crate::history_cell::HistoryCell; use codex_core::features::Feature; use codex_core::protocol::AskForApproval; use codex_core::protocol::SandboxPolicy; -use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::openai_models::ReasoningEffort; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -103,8 +103,8 @@ pub(crate) enum AppEvent { /// Update the current model slug in the running app and widget. UpdateModel(String), - /// Update the current collaboration mode in the running app and widget. - UpdateCollaborationMode(CollaborationMode), + /// Update the active collaboration mask in the running app and widget. + UpdateCollaborationMode(CollaborationModeMask), /// Persist the selected model and reasoning effort to the appropriate config. PersistModelSelection { @@ -240,10 +240,10 @@ pub(crate) enum AppEvent { /// Open the custom prompt option from the review popup. OpenReviewCustomPrompt, - /// Submit a user message with an explicit collaboration mode. + /// Submit a user message with an explicit collaboration mask. SubmitUserMessageWithMode { text: String, - collaboration_mode: CollaborationMode, + collaboration_mode: CollaborationModeMask, }, /// Open the approval popup. diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 47c2c46251..d1f8d691a2 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -2039,15 +2039,6 @@ impl ChatComposer { { self.handle_paste(pasted); } - // Backspace at the start of an image placeholder should delete that placeholder (rather - // than deleting content before it). Do this without scanning the full text by consulting - // the textarea's element list. - if matches!(input.code, KeyCode::Backspace) - && self.try_remove_image_element_at_cursor_start() - { - return (InputResult::None, true); - } - // For non-char inputs (or after flushing), handle normally. // Track element removals so we can drop any corresponding placeholders without scanning // the full text. (Placeholders are atomic elements; when deleted, the element disappears.) @@ -2086,29 +2077,6 @@ impl ChatComposer { (InputResult::None, true) } - fn try_remove_image_element_at_cursor_start(&mut self) -> bool { - if self.attached_images.is_empty() { - return false; - } - - let p = self.textarea.cursor(); - let Some(payload) = self.textarea.element_payload_starting_at(p) else { - return false; - }; - let Some(idx) = self - .attached_images - .iter() - .position(|img| img.placeholder == payload) - else { - return false; - }; - - self.textarea.replace_range(p..p + payload.len(), ""); - self.attached_images.remove(idx); - self.relabel_attached_images_and_update_placeholders(); - true - } - fn reconcile_deleted_elements(&mut self, elements_before: Vec) { let elements_after: HashSet = self.textarea.element_payloads().into_iter().collect(); @@ -4662,8 +4630,7 @@ mod tests { assert!(!composer.textarea.text().contains(&placeholder)); assert!(composer.attached_images.is_empty()); - // Re-add and test backspace in middle: should break the placeholder string - // and drop the image mapping (same as text placeholder behavior). + // Re-add and ensure backspace at element start does not delete the placeholder. composer.attach_image(path); let placeholder2 = composer.attached_images[0].placeholder.clone(); // Move cursor to roughly middle of placeholder @@ -4671,8 +4638,8 @@ mod tests { let mid_pos = start_pos + (placeholder2.len() / 2); composer.textarea.set_cursor(mid_pos); composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)); - assert!(!composer.textarea.text().contains(&placeholder2)); - assert!(composer.attached_images.is_empty()); + assert!(composer.textarea.text().contains(&placeholder2)); + assert_eq!(composer.attached_images.len(), 1); } else { panic!("Placeholder not found in textarea"); } @@ -4852,7 +4819,7 @@ mod tests { assert_eq!(composer.textarea.text(), "[Image #1][Image #2]"); assert_eq!(composer.attached_images.len(), 2); - // Delete the first element using normal textarea editing (Delete at cursor start). + // Delete the first element using normal textarea editing (forward Delete at cursor start). composer.textarea.set_cursor(0); composer.handle_key_event(KeyEvent::new(KeyCode::Delete, KeyModifiers::NONE)); diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 3a849822b1..0235f7a9c1 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -766,12 +766,6 @@ impl TextArea { .collect() } - pub fn element_payload_starting_at(&self, pos: usize) -> Option { - let pos = pos.min(self.text.len()); - let elem = self.elements.iter().find(|e| e.range.start == pos)?; - self.text.get(elem.range.clone()).map(str::to_string) - } - /// Renames a single text element in-place, keeping it atomic. /// /// Use this when the element payload is an identifier (e.g. a placeholder) that must be @@ -1327,6 +1321,21 @@ mod tests { assert_eq!(t.text(), "b"); } + #[test] + fn delete_forward_deletes_element_at_left_edge() { + let mut t = TextArea::new(); + t.insert_str("a"); + t.insert_element(""); + t.insert_str("b"); + + let elem_start = t.elements[0].range.start; + t.set_cursor(elem_start); + t.delete_forward(1); + + assert_eq!(t.text(), "ab"); + assert_eq!(t.cursor(), elem_start); + } + #[test] fn delete_backward_word_and_kill_line_variants() { // delete backward word at end removes the whole previous word diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 1f7fad3ea7..ce87b33eed 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -93,6 +93,7 @@ use codex_protocol::ThreadId; use codex_protocol::account::PlanType; use codex_protocol::approvals::ElicitationRequestEvent; use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; use codex_protocol::models::local_image_label_text; @@ -415,12 +416,12 @@ pub(crate) struct ChatWidget { /// where the overlay may briefly treat new tail content as already cached. active_cell_revision: u64, config: Config, - /// Stored collaboration mode with model and reasoning effort. + /// The unmasked collaboration mode settings (always Custom mode). /// - /// When collaboration modes feature is enabled, this is initialized to the first preset. - /// When disabled, this is Custom. The model and reasoning effort are stored here instead of - /// being read from config or current_model. - stored_collaboration_mode: CollaborationMode, + /// Masks are applied on top of this base mode to derive the effective mode. + current_collaboration_mode: CollaborationMode, + /// The currently active collaboration mask, if any. + active_collaboration_mask: Option, auth_manager: Arc, models_manager: Arc, otel_manager: OtelManager, @@ -727,20 +728,16 @@ impl ChatWidget { self.set_skills(None); self.thread_id = Some(event.session_id); self.forked_from = event.forked_from_id; - self.current_rollout_path = Some(event.rollout_path.clone()); + self.current_rollout_path = event.rollout_path.clone(); let initial_messages = event.initial_messages.clone(); let model_for_header = event.model.clone(); self.session_header.set_model(&model_for_header); - // Only update stored collaboration settings when collaboration modes are disabled. - // When enabled, we preserve the selected variant (Plan/Pair/Execute/Custom) and its - // instructions as-is; the session configured event should not override it. - if !self.collaboration_modes_enabled() { - self.stored_collaboration_mode = self.stored_collaboration_mode.with_updates( - Some(model_for_header.clone()), - Some(event.reasoning_effort), - None, - ); - } + self.current_collaboration_mode = self.current_collaboration_mode.with_updates( + Some(model_for_header.clone()), + Some(event.reasoning_effort), + None, + ); + self.refresh_model_display(); let session_info_cell = history_cell::new_session_info( &self.config, &model_for_header, @@ -910,7 +907,7 @@ impl ChatWidget { if !self.queued_user_messages.is_empty() { return; } - if self.stored_collaboration_mode.mode != ModeKind::Plan { + if self.active_mode_kind() != ModeKind::Plan { return; } let has_message = last_agent_message.is_some_and(|message| !message.trim().is_empty()); @@ -932,14 +929,14 @@ impl ChatWidget { } fn open_plan_implementation_prompt(&mut self) { - let code_mode = collaboration_modes::code_mode(self.models_manager.as_ref()); - let (implement_actions, implement_disabled_reason) = match code_mode { - Some(collaboration_mode) => { + let code_mask = collaboration_modes::code_mask(self.models_manager.as_ref()); + let (implement_actions, implement_disabled_reason) = match code_mask { + Some(mask) => { let user_text = PLAN_IMPLEMENTATION_CODING_MESSAGE.to_string(); let actions: Vec = vec![Box::new(move |tx| { tx.send(AppEvent::SubmitUserMessageWithMode { text: user_text.clone(), - collaboration_mode: collaboration_mode.clone(), + collaboration_mode: mask.clone(), }); })]; (actions, None) @@ -1930,23 +1927,25 @@ impl ChatWidget { let placeholder = PLACEHOLDERS[rng.random_range(0..PLACEHOLDERS.len())].to_string(); let codex_op_tx = spawn_agent(config.clone(), app_event_tx.clone(), thread_manager); - let model_for_header = model.unwrap_or_else(|| DEFAULT_MODEL_DISPLAY_NAME.to_string()); + let model_override = model.as_deref(); + let model_for_header = model + .clone() + .unwrap_or_else(|| DEFAULT_MODEL_DISPLAY_NAME.to_string()); + let active_collaboration_mask = + Self::initial_collaboration_mask(&config, models_manager.as_ref(), model_override); + let header_model = active_collaboration_mask + .as_ref() + .and_then(|mask| mask.model.clone()) + .unwrap_or_else(|| model_for_header.clone()); let fallback_custom = Settings { - model: model_for_header.clone(), + model: header_model.clone(), reasoning_effort: None, developer_instructions: None, }; - let stored_collaboration_mode = if config.features.enabled(Feature::CollaborationModes) { - initial_collaboration_mode( - models_manager.as_ref(), - fallback_custom, - config.experimental_mode, - ) - } else { - CollaborationMode { - mode: ModeKind::Custom, - settings: fallback_custom, - } + // Collaboration modes start in Custom mode (not activated). + let current_collaboration_mode = CollaborationMode { + mode: ModeKind::Custom, + settings: fallback_custom, }; let active_cell = Some(Self::placeholder_session_header_cell(&config)); @@ -1970,11 +1969,12 @@ impl ChatWidget { config, skills_all: Vec::new(), skills_initial_state: None, - stored_collaboration_mode, + current_collaboration_mode, + active_collaboration_mask, auth_manager, models_manager, otel_manager, - session_header: SessionHeader::new(model_for_header), + session_header: SessionHeader::new(header_model), initial_user_message, token_info: None, rate_limit_snapshot: None, @@ -2051,23 +2051,25 @@ impl ChatWidget { let mut rng = rand::rng(); let placeholder = PLACEHOLDERS[rng.random_range(0..PLACEHOLDERS.len())].to_string(); - let model_for_header = model.unwrap_or_else(|| DEFAULT_MODEL_DISPLAY_NAME.to_string()); + let model_override = model.as_deref(); + let model_for_header = model + .clone() + .unwrap_or_else(|| DEFAULT_MODEL_DISPLAY_NAME.to_string()); + let active_collaboration_mask = + Self::initial_collaboration_mask(&config, models_manager.as_ref(), model_override); + let header_model = active_collaboration_mask + .as_ref() + .and_then(|mask| mask.model.clone()) + .unwrap_or_else(|| model_for_header.clone()); let fallback_custom = Settings { - model: model_for_header.clone(), + model: header_model.clone(), reasoning_effort: None, developer_instructions: None, }; - let stored_collaboration_mode = if config.features.enabled(Feature::CollaborationModes) { - initial_collaboration_mode( - models_manager.as_ref(), - fallback_custom, - config.experimental_mode, - ) - } else { - CollaborationMode { - mode: ModeKind::Custom, - settings: fallback_custom, - } + // Collaboration modes start in Custom mode (not activated). + let current_collaboration_mode = CollaborationMode { + mode: ModeKind::Custom, + settings: fallback_custom, }; let active_cell = Some(Self::placeholder_session_header_cell(&config)); @@ -2091,11 +2093,12 @@ impl ChatWidget { config, skills_all: Vec::new(), skills_initial_state: None, - stored_collaboration_mode, + current_collaboration_mode, + active_collaboration_mask, auth_manager, models_manager, otel_manager, - session_header: SessionHeader::new(model_for_header), + session_header: SessionHeader::new(header_model), initial_user_message, token_info: None, rate_limit_snapshot: None, @@ -2171,7 +2174,16 @@ impl ChatWidget { let mut rng = rand::rng(); let placeholder = PLACEHOLDERS[rng.random_range(0..PLACEHOLDERS.len())].to_string(); - let header_model = model.unwrap_or_else(|| session_configured.model.clone()); + let model_override = model.as_deref(); + let header_model = model + .clone() + .unwrap_or_else(|| session_configured.model.clone()); + let active_collaboration_mask = + Self::initial_collaboration_mask(&config, models_manager.as_ref(), model_override); + let header_model = active_collaboration_mask + .as_ref() + .and_then(|mask| mask.model.clone()) + .unwrap_or(header_model); let codex_op_tx = spawn_agent_from_existing(conversation, session_configured, app_event_tx.clone()); @@ -2181,17 +2193,10 @@ impl ChatWidget { reasoning_effort: None, developer_instructions: None, }; - let stored_collaboration_mode = if config.features.enabled(Feature::CollaborationModes) { - initial_collaboration_mode( - models_manager.as_ref(), - fallback_custom, - config.experimental_mode, - ) - } else { - CollaborationMode { - mode: ModeKind::Custom, - settings: fallback_custom, - } + // Collaboration modes start in Custom mode (not activated). + let current_collaboration_mode = CollaborationMode { + mode: ModeKind::Custom, + settings: fallback_custom, }; let mut widget = Self { @@ -2213,7 +2218,8 @@ impl ChatWidget { config, skills_all: Vec::new(), skills_initial_state: None, - stored_collaboration_mode, + current_collaboration_mode, + active_collaboration_mask, auth_manager, models_manager, otel_manager, @@ -2816,18 +2822,24 @@ impl ChatWidget { } } + let effective_mode = self.effective_collaboration_mode(); + let collaboration_mode = if self.collaboration_modes_enabled() { + self.active_collaboration_mask + .as_ref() + .map(|_| effective_mode.clone()) + } else { + None + }; let op = Op::UserTurn { items, cwd: self.config.cwd.clone(), approval_policy: self.config.approval_policy.value(), sandbox_policy: self.config.sandbox_policy.get().clone(), - model: self.stored_collaboration_mode.model().to_string(), - effort: self.stored_collaboration_mode.reasoning_effort(), + model: effective_mode.model().to_string(), + effort: effective_mode.reasoning_effort(), summary: self.config.model_reasoning_summary, final_output_json_schema: None, - collaboration_mode: self - .collaboration_modes_enabled() - .then(|| self.stored_collaboration_mode.clone()), + collaboration_mode, personality: None, }; @@ -3180,7 +3192,7 @@ impl ChatWidget { .map(|ti| &ti.total_token_usage) .unwrap_or(&default_usage); let collaboration_mode = self.collaboration_mode_label(); - let reasoning_effort_override = Some(self.stored_collaboration_mode.reasoning_effort()); + let reasoning_effort_override = Some(self.effective_reasoning_effort()); self.add_to_history(crate::status::new_status_output( &self.config, self.auth_manager.as_ref(), @@ -3558,23 +3570,24 @@ impl ChatWidget { return; } + let current_kind = self + .active_collaboration_mask + .as_ref() + .and_then(|mask| mask.mode) + .or_else(|| { + collaboration_modes::default_mask(self.models_manager.as_ref()) + .and_then(|mask| mask.mode) + }); let items: Vec = presets .into_iter() - .map(|preset| { - let name = match preset.mode { - ModeKind::Plan => "Plan", - ModeKind::Code => "Code", - ModeKind::PairProgramming => "Pair Programming", - ModeKind::Execute => "Execute", - ModeKind::Custom => "Custom", - }; - let is_current = - collaboration_modes::same_variant(&self.stored_collaboration_mode, &preset); + .map(|mask| { + let name = mask.name.clone(); + let is_current = current_kind == mask.mode; let actions: Vec = vec![Box::new(move |tx| { - tx.send(AppEvent::UpdateCollaborationMode(preset.clone())); + tx.send(AppEvent::UpdateCollaborationMode(mask.clone())); })]; SelectionItem { - name: name.to_string(), + name, is_current, actions, dismiss_on_select: true, @@ -3690,7 +3703,7 @@ impl ChatWidget { let model_slug = preset.model.to_string(); let is_current_model = self.current_model() == preset.model.as_str(); let highlight_choice = if is_current_model { - self.stored_collaboration_mode.reasoning_effort() + self.effective_reasoning_effort() } else { default_choice }; @@ -4548,21 +4561,15 @@ impl ChatWidget { } if feature == Feature::CollaborationModes { self.bottom_pane.set_collaboration_modes_enabled(enabled); - let settings = self.stored_collaboration_mode.settings.clone(); - let fallback_custom = settings.clone(); - self.stored_collaboration_mode = if enabled { - initial_collaboration_mode( - self.models_manager.as_ref(), - fallback_custom, - self.config.experimental_mode, - ) - } else { - CollaborationMode { - mode: ModeKind::Custom, - settings, - } + let settings = self.current_collaboration_mode.settings.clone(); + self.current_collaboration_mode = CollaborationMode { + mode: ModeKind::Custom, + settings, }; + self.active_collaboration_mask = None; self.update_collaboration_mode_indicator(); + self.refresh_model_display(); + self.request_redraw(); } } @@ -4591,31 +4598,52 @@ impl ChatWidget { /// Set the reasoning effort in the stored collaboration mode. pub(crate) fn set_reasoning_effort(&mut self, effort: Option) { - self.stored_collaboration_mode = - self.stored_collaboration_mode + self.current_collaboration_mode = + self.current_collaboration_mode .with_updates(None, Some(effort), None); + if self.collaboration_modes_enabled() + && let Some(mask) = self.active_collaboration_mask.as_mut() + { + mask.reasoning_effort = Some(effort); + } } /// Set the model in the widget's config copy and stored collaboration mode. pub(crate) fn set_model(&mut self, model: &str) { - self.session_header.set_model(model); - self.stored_collaboration_mode = - self.stored_collaboration_mode + self.current_collaboration_mode = + self.current_collaboration_mode .with_updates(Some(model.to_string()), None, None); + if self.collaboration_modes_enabled() + && let Some(mask) = self.active_collaboration_mask.as_mut() + { + mask.model = Some(model.to_string()); + } + self.refresh_model_display(); } pub(crate) fn current_model(&self) -> &str { - self.stored_collaboration_mode.model() + if !self.collaboration_modes_enabled() { + return self.current_collaboration_mode.model(); + } + self.active_collaboration_mask + .as_ref() + .and_then(|mask| mask.model.as_deref()) + .unwrap_or_else(|| self.current_collaboration_mode.model()) } #[allow(dead_code)] // Used in tests - pub(crate) fn stored_collaboration_mode(&self) -> &CollaborationMode { - &self.stored_collaboration_mode + pub(crate) fn current_collaboration_mode(&self) -> &CollaborationMode { + &self.current_collaboration_mode } #[cfg(test)] pub(crate) fn current_reasoning_effort(&self) -> Option { - self.stored_collaboration_mode.reasoning_effort() + self.effective_reasoning_effort() + } + + #[cfg(test)] + pub(crate) fn active_collaboration_mode_kind(&self) -> ModeKind { + self.active_mode_kind() } fn is_session_configured(&self) -> bool { @@ -4626,6 +4654,57 @@ impl ChatWidget { self.config.features.enabled(Feature::CollaborationModes) } + fn initial_collaboration_mask( + config: &Config, + models_manager: &ModelsManager, + model_override: Option<&str>, + ) -> Option { + if !config.features.enabled(Feature::CollaborationModes) { + return None; + } + let mut mask = match config.experimental_mode { + Some(kind) => collaboration_modes::mask_for_kind(models_manager, kind)?, + None => collaboration_modes::default_mask(models_manager)?, + }; + if let Some(model_override) = model_override { + mask.model = Some(model_override.to_string()); + } + Some(mask) + } + + fn active_mode_kind(&self) -> ModeKind { + self.active_collaboration_mask + .as_ref() + .and_then(|mask| mask.mode) + .unwrap_or(ModeKind::Custom) + } + + fn effective_reasoning_effort(&self) -> Option { + if !self.collaboration_modes_enabled() { + return self.current_collaboration_mode.reasoning_effort(); + } + let current_effort = self.current_collaboration_mode.reasoning_effort(); + self.active_collaboration_mask + .as_ref() + .and_then(|mask| mask.reasoning_effort) + .unwrap_or(current_effort) + } + + fn effective_collaboration_mode(&self) -> CollaborationMode { + if !self.collaboration_modes_enabled() { + return self.current_collaboration_mode.clone(); + } + self.active_collaboration_mask.as_ref().map_or_else( + || self.current_collaboration_mode.clone(), + |mask| self.current_collaboration_mode.apply_mask(mask), + ) + } + + fn refresh_model_display(&mut self) { + let effective = self.effective_collaboration_mode(); + self.session_header.set_model(effective.model()); + } + fn model_display_name(&self) -> &str { let model = self.current_model(); if model.is_empty() { @@ -4640,7 +4719,7 @@ impl ChatWidget { if !self.collaboration_modes_enabled() { return None; } - match self.stored_collaboration_mode.mode { + match self.active_mode_kind() { ModeKind::Plan => Some("Plan"), ModeKind::Code => Some("Code"), ModeKind::PairProgramming => Some("Pair Programming"), @@ -4653,7 +4732,7 @@ impl ChatWidget { if !self.collaboration_modes_enabled() { return None; } - match self.stored_collaboration_mode.mode { + match self.active_mode_kind() { ModeKind::Plan => Some(CollaborationModeIndicator::Plan), ModeKind::Code => Some(CollaborationModeIndicator::Code), ModeKind::PairProgramming => Some(CollaborationModeIndicator::PairProgramming), @@ -4673,26 +4752,25 @@ impl ChatWidget { return; } - if let Some(next_mode) = collaboration_modes::next_mode( + if let Some(next_mask) = collaboration_modes::next_mask( self.models_manager.as_ref(), - &self.stored_collaboration_mode, + self.active_collaboration_mask.as_ref(), ) { - self.set_collaboration_mode(next_mode); + self.set_collaboration_mask(next_mask); } } - /// Update the stored collaboration mode. + /// Update the active collaboration mask. /// - /// When collaboration modes are enabled, the current mode is attached to *every* - /// submission as `Op::UserTurn { collaboration_mode: Some(...) }`. - pub(crate) fn set_collaboration_mode(&mut self, mode: CollaborationMode) { + /// When collaboration modes are enabled and a preset is selected (not Custom), + /// the current mode is attached to submissions as `Op::UserTurn { collaboration_mode: Some(...) }`. + pub(crate) fn set_collaboration_mask(&mut self, mask: CollaborationModeMask) { if !self.collaboration_modes_enabled() { return; } - let old_model = self.stored_collaboration_mode.model().to_string(); - let mode = mode.with_updates(Some(old_model), None, None); - self.stored_collaboration_mode = mode; + self.active_collaboration_mask = Some(mask); self.update_collaboration_mode_indicator(); + self.refresh_model_display(); self.request_redraw(); } @@ -4875,11 +4953,9 @@ impl ChatWidget { pub(crate) fn submit_user_message_with_mode( &mut self, text: String, - collaboration_mode: CollaborationMode, + collaboration_mode: CollaborationModeMask, ) { - let model = collaboration_mode.model().to_string(); - self.set_collaboration_mode(collaboration_mode); - self.set_model(&model); + self.set_collaboration_mask(collaboration_mode); self.submit_user_message(text.into()); } @@ -5306,29 +5382,6 @@ fn extract_first_bold(s: &str) -> Option { None } -fn initial_collaboration_mode( - models_manager: &ModelsManager, - fallback_custom: Settings, - desired_mode: Option, -) -> CollaborationMode { - if let Some(kind) = desired_mode { - if kind == ModeKind::Custom { - return CollaborationMode { - mode: ModeKind::Custom, - settings: fallback_custom, - }; - } - if let Some(mode) = collaboration_modes::mode_for_kind(models_manager, kind) { - return mode; - } - } - - collaboration_modes::default_mode(models_manager).unwrap_or(CollaborationMode { - mode: ModeKind::Custom, - settings: fallback_custom, - }) -} - async fn fetch_rate_limits(base_url: String, auth: CodexAuth) -> Option { match BackendClient::from_auth(base_url, &auth) { Ok(client) => match client.get_rate_limits().await { diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 6a7401ce6c..e76abdbd7a 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -89,6 +89,7 @@ use tempfile::NamedTempFile; use tempfile::tempdir; use tokio::sync::mpsc::error::TryRecvError; use tokio::sync::mpsc::unbounded_channel; +use toml::Value as TomlValue; #[cfg(target_os = "windows")] fn set_windows_sandbox_enabled(enabled: bool) { @@ -160,7 +161,7 @@ async fn resumed_initial_messages_render_history() { message: "assistant reply".to_string(), }), ]), - rollout_path: rollout_file.path().to_path_buf(), + rollout_path: Some(rollout_file.path().to_path_buf()), }; chat.handle_codex_event(Event { @@ -221,7 +222,7 @@ async fn replayed_user_message_preserves_text_elements_and_local_images() { text_elements: text_elements.clone(), local_images: local_images.clone(), })]), - rollout_path: rollout_file.path().to_path_buf(), + rollout_path: Some(rollout_file.path().to_path_buf()), }; chat.handle_codex_event(Event { @@ -268,7 +269,7 @@ async fn submission_preserves_text_elements_and_local_images() { history_log_id: 0, history_entry_count: 0, initial_messages: None, - rollout_path: rollout_file.path().to_path_buf(), + rollout_path: Some(rollout_file.path().to_path_buf()), }; chat.handle_codex_event(Event { id: "initial".into(), @@ -777,29 +778,16 @@ async fn make_chatwidget_manual( let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("test")); let codex_home = cfg.codex_home.clone(); let models_manager = Arc::new(ModelsManager::new(codex_home, auth_manager.clone())); - let collaboration_modes_enabled = cfg.features.enabled(Feature::CollaborationModes); let reasoning_effort = None; - let stored_collaboration_mode = if collaboration_modes_enabled { - collaboration_modes::default_mode(models_manager.as_ref()).unwrap_or_else(|| { - CollaborationMode { - mode: ModeKind::Custom, - settings: Settings { - model: resolved_model.clone(), - reasoning_effort, - developer_instructions: None, - }, - } - }) - } else { - CollaborationMode { - mode: ModeKind::Custom, - settings: Settings { - model: resolved_model.clone(), - reasoning_effort, - developer_instructions: None, - }, - } + let base_mode = CollaborationMode { + mode: ModeKind::Custom, + settings: Settings { + model: resolved_model.clone(), + reasoning_effort, + developer_instructions: None, + }, }; + let current_collaboration_mode = base_mode; let widget = ChatWidget { app_event_tx, codex_op_tx: op_tx, @@ -807,7 +795,8 @@ async fn make_chatwidget_manual( active_cell: None, active_cell_revision: 0, config: cfg, - stored_collaboration_mode, + current_collaboration_mode, + active_collaboration_mask: None, auth_manager, models_manager, otel_manager, @@ -1214,7 +1203,7 @@ async fn plan_implementation_popup_yes_emits_submit_message_event() { panic!("expected SubmitUserMessageWithMode, got {event:?}"); }; assert_eq!(text, PLAN_IMPLEMENTATION_CODING_MESSAGE); - assert_eq!(collaboration_mode.mode, ModeKind::Code); + assert_eq!(collaboration_mode.mode, Some(ModeKind::Code)); } #[tokio::test] @@ -1223,7 +1212,7 @@ async fn submit_user_message_with_mode_sets_coding_collaboration_mode() { chat.thread_id = Some(ThreadId::new()); chat.set_feature_enabled(Feature::CollaborationModes, true); - let code_mode = collaboration_modes::code_mode(chat.models_manager.as_ref()) + let code_mode = collaboration_modes::code_mask(chat.models_manager.as_ref()) .expect("expected code collaboration mode"); chat.submit_user_message_with_mode("Implement the plan.".to_string(), code_mode); @@ -1247,14 +1236,10 @@ async fn submit_user_message_with_mode_sets_coding_collaboration_mode() { async fn plan_implementation_popup_skips_replayed_turn_complete() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5")).await; chat.set_feature_enabled(Feature::CollaborationModes, true); - chat.stored_collaboration_mode = CollaborationMode { - mode: ModeKind::Plan, - settings: Settings { - model: chat.current_model().to_string(), - reasoning_effort: None, - developer_instructions: None, - }, - }; + let plan_mask = + collaboration_modes::mask_for_kind(chat.models_manager.as_ref(), ModeKind::Plan) + .expect("expected plan collaboration mask"); + chat.set_collaboration_mask(plan_mask); chat.replay_initial_messages(vec![EventMsg::TurnComplete(TurnCompleteEvent { last_agent_message: Some("Plan details".to_string()), @@ -1271,14 +1256,10 @@ async fn plan_implementation_popup_skips_replayed_turn_complete() { async fn plan_implementation_popup_skips_when_messages_queued() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5")).await; chat.set_feature_enabled(Feature::CollaborationModes, true); - chat.stored_collaboration_mode = CollaborationMode { - mode: ModeKind::Plan, - settings: Settings { - model: chat.current_model().to_string(), - reasoning_effort: None, - developer_instructions: None, - }, - }; + let plan_mask = + collaboration_modes::mask_for_kind(chat.models_manager.as_ref(), ModeKind::Plan) + .expect("expected plan collaboration mask"); + chat.set_collaboration_mask(plan_mask); chat.bottom_pane.set_task_running(true); chat.queue_user_message("Queued message".into()); @@ -1295,14 +1276,10 @@ async fn plan_implementation_popup_skips_when_messages_queued() { async fn plan_implementation_popup_shows_on_plan_update_without_message() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5")).await; chat.set_feature_enabled(Feature::CollaborationModes, true); - chat.stored_collaboration_mode = CollaborationMode { - mode: ModeKind::Plan, - settings: Settings { - model: chat.current_model().to_string(), - reasoning_effort: None, - developer_instructions: None, - }, - }; + let plan_mask = + collaboration_modes::mask_for_kind(chat.models_manager.as_ref(), ModeKind::Plan) + .expect("expected plan collaboration mask"); + chat.set_collaboration_mask(plan_mask); chat.on_task_started(); chat.on_plan_update(UpdatePlanArgs { @@ -1327,14 +1304,10 @@ async fn plan_implementation_popup_skips_when_rate_limit_prompt_pending() { chat.auth_manager = AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing()); chat.set_feature_enabled(Feature::CollaborationModes, true); - chat.stored_collaboration_mode = CollaborationMode { - mode: ModeKind::Plan, - settings: Settings { - model: chat.current_model().to_string(), - reasoning_effort: None, - developer_instructions: None, - }, - }; + let plan_mask = + collaboration_modes::mask_for_kind(chat.models_manager.as_ref(), ModeKind::Plan) + .expect("expected plan collaboration mask"); + chat.set_collaboration_mask(plan_mask); chat.on_task_started(); chat.on_plan_update(UpdatePlanArgs { @@ -2222,22 +2195,25 @@ async fn collab_mode_shift_tab_cycles_only_when_enabled_and_idle() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.set_feature_enabled(Feature::CollaborationModes, false); - let initial = chat.stored_collaboration_mode.clone(); + let initial = chat.current_collaboration_mode().clone(); chat.handle_key_event(KeyEvent::from(KeyCode::BackTab)); - assert_eq!(chat.stored_collaboration_mode, initial); + assert_eq!(chat.current_collaboration_mode(), &initial); + assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Custom); chat.set_feature_enabled(Feature::CollaborationModes, true); chat.handle_key_event(KeyEvent::from(KeyCode::BackTab)); - assert_eq!(chat.stored_collaboration_mode.mode, ModeKind::Plan); + assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Plan); + assert_eq!(chat.current_collaboration_mode(), &initial); chat.handle_key_event(KeyEvent::from(KeyCode::BackTab)); - assert_eq!(chat.stored_collaboration_mode.mode, ModeKind::Code); + assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Code); + assert_eq!(chat.current_collaboration_mode(), &initial); chat.on_task_started(); - let before = chat.stored_collaboration_mode.clone(); + let before = chat.active_collaboration_mode_kind(); chat.handle_key_event(KeyEvent::from(KeyCode::BackTab)); - assert_eq!(chat.stored_collaboration_mode, before); + assert_eq!(chat.active_collaboration_mode_kind(), before); } #[tokio::test] @@ -2254,11 +2230,11 @@ async fn collab_slash_command_opens_picker_and_updates_mode() { ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); - let selected_mode = match rx.try_recv() { - Ok(AppEvent::UpdateCollaborationMode(mode)) => mode, + let selected_mask = match rx.try_recv() { + Ok(AppEvent::UpdateCollaborationMode(mask)) => mask, other => panic!("expected UpdateCollaborationMode event, got {other:?}"), }; - chat.set_collaboration_mode(selected_mode); + chat.set_collaboration_mask(selected_mask); chat.bottom_pane .set_composer_text("hello".to_string(), Vec::new(), Vec::new()); @@ -2298,7 +2274,119 @@ async fn collab_slash_command_opens_picker_and_updates_mode() { } #[tokio::test] -async fn collab_mode_defaults_to_coding_when_enabled() { +async fn collaboration_modes_defaults_to_code_on_startup() { + let codex_home = tempdir().expect("tempdir"); + let cfg = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .cli_overrides(vec![( + "features.collaboration_modes".to_string(), + TomlValue::Boolean(true), + )]) + .build() + .await + .expect("config"); + let resolved_model = ModelsManager::get_model_offline(cfg.model.as_deref()); + let otel_manager = test_otel_manager(&cfg, resolved_model.as_str()); + let thread_manager = Arc::new(ThreadManager::with_models_provider( + CodexAuth::from_api_key("test"), + cfg.model_provider.clone(), + )); + let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("test")); + let init = ChatWidgetInit { + config: cfg, + frame_requester: FrameRequester::test_dummy(), + app_event_tx: AppEventSender::new(unbounded_channel::().0), + initial_user_message: None, + enhanced_keys_supported: false, + auth_manager, + models_manager: thread_manager.get_models_manager(), + feedback: codex_feedback::CodexFeedback::new(), + is_first_run: true, + model: Some(resolved_model.clone()), + otel_manager, + }; + + let chat = ChatWidget::new(init, thread_manager); + assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Code); + assert_eq!(chat.current_model(), resolved_model); +} + +#[tokio::test] +async fn experimental_mode_plan_applies_on_startup() { + let codex_home = tempdir().expect("tempdir"); + let cfg = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .cli_overrides(vec![ + ( + "features.collaboration_modes".to_string(), + TomlValue::Boolean(true), + ), + ( + "tui.experimental_mode".to_string(), + TomlValue::String("plan".to_string()), + ), + ]) + .build() + .await + .expect("config"); + let resolved_model = ModelsManager::get_model_offline(cfg.model.as_deref()); + let otel_manager = test_otel_manager(&cfg, resolved_model.as_str()); + let thread_manager = Arc::new(ThreadManager::with_models_provider( + CodexAuth::from_api_key("test"), + cfg.model_provider.clone(), + )); + let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("test")); + let init = ChatWidgetInit { + config: cfg, + frame_requester: FrameRequester::test_dummy(), + app_event_tx: AppEventSender::new(unbounded_channel::().0), + initial_user_message: None, + enhanced_keys_supported: false, + auth_manager, + models_manager: thread_manager.get_models_manager(), + feedback: codex_feedback::CodexFeedback::new(), + is_first_run: true, + model: Some(resolved_model.clone()), + otel_manager, + }; + + let chat = ChatWidget::new(init, thread_manager); + assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Plan); + assert_eq!(chat.current_model(), resolved_model); +} + +#[tokio::test] +async fn set_model_updates_active_collaboration_mask() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1")).await; + chat.set_feature_enabled(Feature::CollaborationModes, true); + let plan_mask = + collaboration_modes::mask_for_kind(chat.models_manager.as_ref(), ModeKind::Plan) + .expect("expected plan collaboration mask"); + chat.set_collaboration_mask(plan_mask); + + chat.set_model("gpt-5.1-codex-mini"); + + assert_eq!(chat.current_model(), "gpt-5.1-codex-mini"); + assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Plan); +} + +#[tokio::test] +async fn set_reasoning_effort_updates_active_collaboration_mask() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1")).await; + chat.set_feature_enabled(Feature::CollaborationModes, true); + let plan_mask = + collaboration_modes::mask_for_kind(chat.models_manager.as_ref(), ModeKind::Plan) + .expect("expected plan collaboration mask"); + chat.set_collaboration_mask(plan_mask); + + chat.set_reasoning_effort(None); + + assert_eq!(chat.current_reasoning_effort(), None); + assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Plan); +} + +#[tokio::test] +async fn collab_mode_is_not_sent_until_selected() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await; chat.thread_id = Some(ThreadId::new()); chat.set_feature_enabled(Feature::CollaborationModes, true); @@ -2308,25 +2396,24 @@ async fn collab_mode_defaults_to_coding_when_enabled() { chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); match next_submit_op(&mut op_rx) { Op::UserTurn { - collaboration_mode: - Some(CollaborationMode { - mode: ModeKind::Code, - .. - }), + collaboration_mode, personality: None, .. - } => {} + } => { + assert_eq!(collaboration_mode, None); + } other => { - panic!("expected Op::UserTurn with code collab mode, got {other:?}") + panic!("expected Op::UserTurn, got {other:?}") } } } #[tokio::test] -async fn collab_mode_enabling_sets_coding_default() { +async fn collab_mode_enabling_keeps_custom_until_selected() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.set_feature_enabled(Feature::CollaborationModes, true); - assert_eq!(chat.stored_collaboration_mode.mode, ModeKind::Code); + assert_eq!(chat.active_collaboration_mode_kind(), ModeKind::Custom); + assert_eq!(chat.current_collaboration_mode().mode, ModeKind::Custom); } #[tokio::test] diff --git a/codex-rs/tui/src/collaboration_modes.rs b/codex-rs/tui/src/collaboration_modes.rs index c66010b57a..95908d8e90 100644 --- a/codex-rs/tui/src/collaboration_modes.rs +++ b/codex-rs/tui/src/collaboration_modes.rs @@ -1,70 +1,61 @@ use codex_core::models_manager::manager::ModelsManager; -use codex_protocol::config_types::CollaborationMode; +use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::ModeKind; -fn mode_kind(mode: &CollaborationMode) -> ModeKind { - mode.mode -} - fn is_tui_mode(kind: ModeKind) -> bool { matches!(kind, ModeKind::Plan | ModeKind::Code) } -fn filtered_presets(models_manager: &ModelsManager) -> Vec { +fn filtered_presets(models_manager: &ModelsManager) -> Vec { models_manager .list_collaboration_modes() .into_iter() - .filter(|preset| is_tui_mode(mode_kind(preset))) + .filter(|mask| mask.mode.is_some_and(is_tui_mode)) .collect() } -pub(crate) fn presets_for_tui(models_manager: &ModelsManager) -> Vec { +pub(crate) fn presets_for_tui(models_manager: &ModelsManager) -> Vec { filtered_presets(models_manager) } -pub(crate) fn default_mode(models_manager: &ModelsManager) -> Option { +pub(crate) fn default_mask(models_manager: &ModelsManager) -> Option { let presets = filtered_presets(models_manager); presets .iter() - .find(|preset| preset.mode == ModeKind::Code) + .find(|mask| mask.mode == Some(ModeKind::Code)) .cloned() .or_else(|| presets.into_iter().next()) } -pub(crate) fn mode_for_kind( +pub(crate) fn mask_for_kind( models_manager: &ModelsManager, kind: ModeKind, -) -> Option { +) -> Option { if !is_tui_mode(kind) { return None; } - let presets = filtered_presets(models_manager); - presets.into_iter().find(|preset| mode_kind(preset) == kind) -} - -pub(crate) fn same_variant(a: &CollaborationMode, b: &CollaborationMode) -> bool { - mode_kind(a) == mode_kind(b) + filtered_presets(models_manager) + .into_iter() + .find(|mask| mask.mode == Some(kind)) } /// Cycle to the next collaboration mode preset in list order. -pub(crate) fn next_mode( +pub(crate) fn next_mask( models_manager: &ModelsManager, - current: &CollaborationMode, -) -> Option { + current: Option<&CollaborationModeMask>, +) -> Option { let presets = filtered_presets(models_manager); if presets.is_empty() { return None; } - let current_kind = mode_kind(current); + let current_kind = current.and_then(|mask| mask.mode); let next_index = presets .iter() - .position(|preset| mode_kind(preset) == current_kind) + .position(|mask| mask.mode == current_kind) .map_or(0, |idx| (idx + 1) % presets.len()); presets.get(next_index).cloned() } -pub(crate) fn code_mode(models_manager: &ModelsManager) -> Option { - filtered_presets(models_manager) - .into_iter() - .find(|preset| mode_kind(preset) == ModeKind::Code) +pub(crate) fn code_mask(models_manager: &ModelsManager) -> Option { + mask_for_kind(models_manager, ModeKind::Code) } diff --git a/codex-rs/tui/src/cwd_prompt.rs b/codex-rs/tui/src/cwd_prompt.rs new file mode 100644 index 0000000000..2a9c016a1e --- /dev/null +++ b/codex-rs/tui/src/cwd_prompt.rs @@ -0,0 +1,286 @@ +use std::path::Path; + +use crate::key_hint; +use crate::render::Insets; +use crate::render::renderable::ColumnRenderable; +use crate::render::renderable::Renderable; +use crate::render::renderable::RenderableExt as _; +use crate::selection_list::selection_option_row; +use crate::tui::FrameRequester; +use crate::tui::Tui; +use crate::tui::TuiEvent; +use color_eyre::Result; +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 as _; +use ratatui::text::Line; +use ratatui::widgets::Clear; +use ratatui::widgets::WidgetRef; +use tokio_stream::StreamExt; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum CwdPromptAction { + Resume, + Fork, +} + +impl CwdPromptAction { + fn verb(self) -> &'static str { + match self { + CwdPromptAction::Resume => "resume", + CwdPromptAction::Fork => "fork", + } + } + + fn past_participle(self) -> &'static str { + match self { + CwdPromptAction::Resume => "resumed", + CwdPromptAction::Fork => "forked", + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum CwdSelection { + Current, + Session, +} + +impl CwdSelection { + fn next(self) -> Self { + match self { + CwdSelection::Current => CwdSelection::Session, + CwdSelection::Session => CwdSelection::Current, + } + } + + fn prev(self) -> Self { + match self { + CwdSelection::Current => CwdSelection::Session, + CwdSelection::Session => CwdSelection::Current, + } + } +} + +pub(crate) async fn run_cwd_selection_prompt( + tui: &mut Tui, + action: CwdPromptAction, + current_cwd: &Path, + session_cwd: &Path, +) -> Result { + let mut screen = CwdPromptScreen::new( + tui.frame_requester(), + action, + current_cwd.display().to_string(), + session_cwd.display().to_string(), + ); + tui.draw(u16::MAX, |frame| { + frame.render_widget_ref(&screen, frame.area()); + })?; + + let events = tui.event_stream(); + tokio::pin!(events); + + while !screen.is_done() { + if let Some(event) = events.next().await { + match event { + TuiEvent::Key(key_event) => screen.handle_key(key_event), + TuiEvent::Paste(_) => {} + TuiEvent::Draw => { + tui.draw(u16::MAX, |frame| { + frame.render_widget_ref(&screen, frame.area()); + })?; + } + } + } else { + break; + } + } + + Ok(screen.selection().unwrap_or(CwdSelection::Session)) +} + +struct CwdPromptScreen { + request_frame: FrameRequester, + action: CwdPromptAction, + current_cwd: String, + session_cwd: String, + highlighted: CwdSelection, + selection: Option, +} + +impl CwdPromptScreen { + fn new( + request_frame: FrameRequester, + action: CwdPromptAction, + current_cwd: String, + session_cwd: String, + ) -> Self { + Self { + request_frame, + action, + current_cwd, + session_cwd, + highlighted: CwdSelection::Session, + selection: None, + } + } + + fn handle_key(&mut self, key_event: KeyEvent) { + if key_event.kind == KeyEventKind::Release { + return; + } + if key_event.modifiers.contains(KeyModifiers::CONTROL) + && matches!(key_event.code, KeyCode::Char('c') | KeyCode::Char('d')) + { + self.select(CwdSelection::Session); + return; + } + match key_event.code { + KeyCode::Up | KeyCode::Char('k') => self.set_highlight(self.highlighted.prev()), + KeyCode::Down | KeyCode::Char('j') => self.set_highlight(self.highlighted.next()), + KeyCode::Char('1') => self.select(CwdSelection::Session), + KeyCode::Char('2') => self.select(CwdSelection::Current), + KeyCode::Enter => self.select(self.highlighted), + KeyCode::Esc => self.select(CwdSelection::Session), + _ => {} + } + } + + fn set_highlight(&mut self, highlight: CwdSelection) { + if self.highlighted != highlight { + self.highlighted = highlight; + self.request_frame.schedule_frame(); + } + } + + fn select(&mut self, selection: CwdSelection) { + self.highlighted = selection; + self.selection = Some(selection); + self.request_frame.schedule_frame(); + } + + fn is_done(&self) -> bool { + self.selection.is_some() + } + + fn selection(&self) -> Option { + self.selection + } +} + +impl WidgetRef for &CwdPromptScreen { + fn render_ref(&self, area: Rect, buf: &mut Buffer) { + Clear.render(area, buf); + let mut column = ColumnRenderable::new(); + + let action_verb = self.action.verb(); + let action_past = self.action.past_participle(); + let current_cwd = self.current_cwd.as_str(); + let session_cwd = self.session_cwd.as_str(); + + column.push(""); + column.push(Line::from(vec![ + "Choose working directory to ".into(), + action_verb.bold(), + " this session".into(), + ])); + column.push(""); + column.push( + Line::from(format!( + "Session = latest cwd recorded in the {action_past} session" + )) + .dim() + .inset(Insets::tlbr(0, 2, 0, 0)), + ); + column.push( + Line::from("Current = your current working directory".dim()) + .inset(Insets::tlbr(0, 2, 0, 0)), + ); + column.push(""); + column.push(selection_option_row( + 0, + format!("Use session directory ({session_cwd})"), + self.highlighted == CwdSelection::Session, + )); + column.push(selection_option_row( + 1, + format!("Use current directory ({current_cwd})"), + self.highlighted == CwdSelection::Current, + )); + column.push(""); + column.push( + Line::from(vec![ + "Press ".dim(), + key_hint::plain(KeyCode::Enter).into(), + " to continue".dim(), + ]) + .inset(Insets::tlbr(0, 2, 0, 0)), + ); + column.render(area, buf); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_backend::VT100Backend; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + use pretty_assertions::assert_eq; + use ratatui::Terminal; + + fn new_prompt() -> CwdPromptScreen { + CwdPromptScreen::new( + FrameRequester::test_dummy(), + CwdPromptAction::Resume, + "/Users/example/current".to_string(), + "/Users/example/session".to_string(), + ) + } + + #[test] + fn cwd_prompt_snapshot() { + let screen = new_prompt(); + let mut terminal = Terminal::new(VT100Backend::new(80, 14)).expect("terminal"); + terminal + .draw(|frame| frame.render_widget_ref(&screen, frame.area())) + .expect("render cwd prompt"); + insta::assert_snapshot!("cwd_prompt_modal", terminal.backend()); + } + + #[test] + fn cwd_prompt_fork_snapshot() { + let screen = CwdPromptScreen::new( + FrameRequester::test_dummy(), + CwdPromptAction::Fork, + "/Users/example/current".to_string(), + "/Users/example/session".to_string(), + ); + let mut terminal = Terminal::new(VT100Backend::new(80, 14)).expect("terminal"); + terminal + .draw(|frame| frame.render_widget_ref(&screen, frame.area())) + .expect("render cwd prompt"); + insta::assert_snapshot!("cwd_prompt_fork_modal", terminal.backend()); + } + + #[test] + fn cwd_prompt_selects_session_by_default() { + let mut screen = new_prompt(); + screen.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert_eq!(screen.selection(), Some(CwdSelection::Session)); + } + + #[test] + fn cwd_prompt_can_select_current() { + let mut screen = new_prompt(); + screen.handle_key(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + screen.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert_eq!(screen.selection(), Some(CwdSelection::Current)); + } +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index e3ee2ed4e6..0fac506d32 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -27,13 +27,19 @@ use codex_core::config_loader::ConfigLoadError; use codex_core::config_loader::format_config_error_with_source; use codex_core::find_thread_path_by_id_str; use codex_core::get_platform_sandbox; +use codex_core::path_utils; 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; +use codex_protocol::protocol::RolloutItem; +use codex_protocol::protocol::RolloutLine; use codex_utils_absolute_path::AbsolutePathBuf; +use cwd_prompt::CwdPromptAction; +use cwd_prompt::CwdSelection; use std::fs::OpenOptions; +use std::path::Path; use std::path::PathBuf; use tracing::error; use tracing_appender::non_blocking; @@ -54,6 +60,7 @@ mod collab; mod collaboration_modes; mod color; pub mod custom_terminal; +mod cwd_prompt; mod diff_render; mod exec_cell; mod exec_command; @@ -577,25 +584,27 @@ async fn run_ratatui_app( resume_picker::SessionSelection::StartFresh }; + let current_cwd = config.cwd.clone(); + let allow_prompt = cli.cwd.is_none(); + let action_and_path_if_resume_or_fork = match &session_selection { + resume_picker::SessionSelection::Resume(path) => Some((CwdPromptAction::Resume, path)), + resume_picker::SessionSelection::Fork(path) => Some((CwdPromptAction::Fork, path)), + _ => None, + }; + let fallback_cwd = match action_and_path_if_resume_or_fork { + Some((action, path)) => { + resolve_cwd_for_resume_or_fork(&mut tui, ¤t_cwd, path, action, allow_prompt) + .await? + } + None => None, + }; + 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 - } - }; + resume_picker::SessionSelection::Resume(_) | resume_picker::SessionSelection::Fork(_) => { load_config_or_exit_with_fallback_cwd( cli_kv_overrides.clone(), overrides.clone(), - history_cwd, + fallback_cwd, ) .await } @@ -618,6 +627,8 @@ async fn run_ratatui_app( &mut tui, auth_manager, config, + cli_kv_overrides.clone(), + overrides.clone(), active_profile, prompt, images, @@ -635,6 +646,77 @@ async fn run_ratatui_app( app_result } +pub(crate) async fn read_session_cwd(path: &Path) -> Option { + // Prefer the latest TurnContext cwd so resume/fork reflects the most recent + // session directory (for the changed-cwd prompt). The alternative would be + // mutating the SessionMeta line when the session cwd changes, but the rollout + // is an append-only JSONL log and rewriting the head would be error-prone. + // When rollouts move to SQLite, we can drop this scan. + if let Some(cwd) = parse_latest_turn_context_cwd(path).await { + return Some(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 + } + } +} + +async fn parse_latest_turn_context_cwd(path: &Path) -> Option { + let text = tokio::fs::read_to_string(path).await.ok()?; + for line in text.lines().rev() { + let trimmed = line.trim(); + if trimmed.is_empty() { + continue; + } + let Ok(rollout_line) = serde_json::from_str::(trimmed) else { + continue; + }; + if let RolloutItem::TurnContext(item) = rollout_line.item { + return Some(item.cwd); + } + } + None +} + +pub(crate) fn cwds_differ(current_cwd: &Path, session_cwd: &Path) -> bool { + match ( + path_utils::normalize_for_path_comparison(current_cwd), + path_utils::normalize_for_path_comparison(session_cwd), + ) { + (Ok(current), Ok(session)) => current != session, + _ => current_cwd != session_cwd, + } +} + +pub(crate) async fn resolve_cwd_for_resume_or_fork( + tui: &mut Tui, + current_cwd: &Path, + path: &Path, + action: CwdPromptAction, + allow_prompt: bool, +) -> color_eyre::Result> { + let Some(history_cwd) = read_session_cwd(path).await else { + return Ok(None); + }; + if allow_prompt && cwds_differ(current_cwd, &history_cwd) { + let selection = + cwd_prompt::run_cwd_selection_prompt(tui, action, current_cwd, &history_cwd).await?; + return Ok(Some(match selection { + CwdSelection::Current => current_cwd.to_path_buf(), + CwdSelection::Session => history_cwd, + })); + } + Ok(Some(history_cwd)) +} + #[expect( clippy::print_stderr, reason = "TUI should no longer be displayed, so we can write to stderr." @@ -772,7 +854,14 @@ fn should_show_login_screen(login_status: LoginStatus, config: &Config) -> bool mod tests { use super::*; use codex_core::config::ConfigBuilder; + use codex_core::config::ConfigOverrides; use codex_core::config::ProjectConfig; + use codex_core::protocol::AskForApproval; + use codex_protocol::protocol::RolloutItem; + use codex_protocol::protocol::RolloutLine; + use codex_protocol::protocol::SessionMeta; + use codex_protocol::protocol::SessionMetaLine; + use codex_protocol::protocol::TurnContextItem; use serial_test::serial; use tempfile::TempDir; @@ -846,4 +935,180 @@ mod tests { ); Ok(()) } + + fn build_turn_context(config: &Config, cwd: PathBuf) -> TurnContextItem { + let model = config + .model + .clone() + .unwrap_or_else(|| "gpt-5.1".to_string()); + TurnContextItem { + cwd, + approval_policy: config.approval_policy.value(), + sandbox_policy: config.sandbox_policy.get().clone(), + model, + personality: None, + collaboration_mode: None, + effort: config.model_reasoning_effort, + summary: config.model_reasoning_summary, + user_instructions: None, + developer_instructions: None, + final_output_json_schema: None, + truncation_policy: None, + } + } + + #[tokio::test] + async fn read_session_cwd_prefers_latest_turn_context() -> std::io::Result<()> { + let temp_dir = TempDir::new()?; + let config = build_config(&temp_dir).await?; + let first = temp_dir.path().join("first"); + let second = temp_dir.path().join("second"); + std::fs::create_dir_all(&first)?; + std::fs::create_dir_all(&second)?; + + let rollout_path = temp_dir.path().join("rollout.jsonl"); + let lines = vec![ + RolloutLine { + timestamp: "t0".to_string(), + item: RolloutItem::TurnContext(build_turn_context(&config, first)), + }, + RolloutLine { + timestamp: "t1".to_string(), + item: RolloutItem::TurnContext(build_turn_context(&config, second.clone())), + }, + ]; + let mut text = String::new(); + for line in lines { + text.push_str(&serde_json::to_string(&line).expect("serialize rollout")); + text.push('\n'); + } + std::fs::write(&rollout_path, text)?; + + let cwd = read_session_cwd(&rollout_path).await.expect("expected cwd"); + assert_eq!(cwd, second); + Ok(()) + } + + #[tokio::test] + async fn should_prompt_when_meta_matches_current_but_latest_turn_differs() -> std::io::Result<()> + { + let temp_dir = TempDir::new()?; + let config = build_config(&temp_dir).await?; + let current = temp_dir.path().join("current"); + let latest = temp_dir.path().join("latest"); + std::fs::create_dir_all(¤t)?; + std::fs::create_dir_all(&latest)?; + + let rollout_path = temp_dir.path().join("rollout.jsonl"); + let session_meta = SessionMeta { + cwd: current.clone(), + ..SessionMeta::default() + }; + let lines = vec![ + RolloutLine { + timestamp: "t0".to_string(), + item: RolloutItem::SessionMeta(SessionMetaLine { + meta: session_meta, + git: None, + }), + }, + RolloutLine { + timestamp: "t1".to_string(), + item: RolloutItem::TurnContext(build_turn_context(&config, latest.clone())), + }, + ]; + let mut text = String::new(); + for line in lines { + text.push_str(&serde_json::to_string(&line).expect("serialize rollout")); + text.push('\n'); + } + std::fs::write(&rollout_path, text)?; + + let session_cwd = read_session_cwd(&rollout_path).await.expect("expected cwd"); + assert_eq!(session_cwd, latest); + assert!(cwds_differ(¤t, &session_cwd)); + Ok(()) + } + + #[tokio::test] + async fn config_rebuild_changes_trust_defaults_with_cwd() -> std::io::Result<()> { + let temp_dir = TempDir::new()?; + let codex_home = temp_dir.path().to_path_buf(); + let trusted = temp_dir.path().join("trusted"); + let untrusted = temp_dir.path().join("untrusted"); + std::fs::create_dir_all(&trusted)?; + std::fs::create_dir_all(&untrusted)?; + + // TOML keys need escaped backslashes on Windows paths. + let trusted_display = trusted.display().to_string().replace('\\', "\\\\"); + let untrusted_display = untrusted.display().to_string().replace('\\', "\\\\"); + let config_toml = format!( + r#"[projects."{trusted_display}"] +trust_level = "trusted" + +[projects."{untrusted_display}"] +trust_level = "untrusted" +"# + ); + std::fs::write(temp_dir.path().join("config.toml"), config_toml)?; + + let trusted_overrides = ConfigOverrides { + cwd: Some(trusted.clone()), + ..Default::default() + }; + let trusted_config = ConfigBuilder::default() + .codex_home(codex_home.clone()) + .harness_overrides(trusted_overrides.clone()) + .build() + .await?; + assert_eq!( + trusted_config.approval_policy.value(), + AskForApproval::OnRequest + ); + + let untrusted_overrides = ConfigOverrides { + cwd: Some(untrusted), + ..trusted_overrides + }; + let untrusted_config = ConfigBuilder::default() + .codex_home(codex_home) + .harness_overrides(untrusted_overrides) + .build() + .await?; + assert_eq!( + untrusted_config.approval_policy.value(), + AskForApproval::UnlessTrusted + ); + Ok(()) + } + + #[tokio::test] + async fn read_session_cwd_falls_back_to_session_meta() -> std::io::Result<()> { + let temp_dir = TempDir::new()?; + let _config = build_config(&temp_dir).await?; + let session_cwd = temp_dir.path().join("session"); + std::fs::create_dir_all(&session_cwd)?; + + let rollout_path = temp_dir.path().join("rollout.jsonl"); + let session_meta = SessionMeta { + cwd: session_cwd.clone(), + ..SessionMeta::default() + }; + let meta_line = RolloutLine { + timestamp: "t0".to_string(), + item: RolloutItem::SessionMeta(SessionMetaLine { + meta: session_meta, + git: None, + }), + }; + let text = format!( + "{}\n", + serde_json::to_string(&meta_line).expect("serialize meta") + ); + std::fs::write(&rollout_path, text)?; + + let cwd = read_session_cwd(&rollout_path).await.expect("expected cwd"); + assert_eq!(cwd, session_cwd); + Ok(()) + } } diff --git a/codex-rs/tui/src/onboarding/onboarding_screen.rs b/codex-rs/tui/src/onboarding/onboarding_screen.rs index 7e076ae160..78a7f0ce85 100644 --- a/codex-rs/tui/src/onboarding/onboarding_screen.rs +++ b/codex-rs/tui/src/onboarding/onboarding_screen.rs @@ -311,6 +311,9 @@ impl WidgetRef for &OnboardingScreen { } let scratch_area = Rect::new(0, 0, width, max_h); let mut scratch = Buffer::empty(scratch_area); + if let Step::Welcome(widget) = step { + widget.update_layout_area(scratch_area); + } step.render_ref(scratch_area, &mut scratch); let h = used_rows(&scratch, width, max_h).min(max_h); if h > 0 { diff --git a/codex-rs/tui/src/onboarding/welcome.rs b/codex-rs/tui/src/onboarding/welcome.rs index 8ff5e81982..bdf4f91408 100644 --- a/codex-rs/tui/src/onboarding/welcome.rs +++ b/codex-rs/tui/src/onboarding/welcome.rs @@ -11,6 +11,7 @@ use ratatui::widgets::Clear; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; +use std::cell::Cell; use crate::ascii_animation::AsciiAnimation; use crate::onboarding::onboarding_screen::KeyboardHandler; @@ -19,13 +20,14 @@ use crate::tui::FrameRequester; use super::onboarding_screen::StepState; -const MIN_ANIMATION_HEIGHT: u16 = 20; +const MIN_ANIMATION_HEIGHT: u16 = 37; const MIN_ANIMATION_WIDTH: u16 = 60; pub(crate) struct WelcomeWidget { pub is_logged_in: bool, animation: AsciiAnimation, animations_enabled: bool, + layout_area: Cell>, } impl KeyboardHandler for WelcomeWidget { @@ -53,8 +55,13 @@ impl WelcomeWidget { is_logged_in, animation: AsciiAnimation::new(request_frame), animations_enabled, + layout_area: Cell::new(None), } } + + pub(crate) fn update_layout_area(&self, area: Rect) { + self.layout_area.set(Some(area)); + } } impl WidgetRef for &WelcomeWidget { @@ -64,12 +71,14 @@ impl WidgetRef for &WelcomeWidget { self.animation.schedule_next_frame(); } + let layout_area = self.layout_area.get().unwrap_or(area); // 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 show_animation = self.animations_enabled + && layout_area.height >= MIN_ANIMATION_HEIGHT + && layout_area.width >= MIN_ANIMATION_WIDTH; let mut lines: Vec = Vec::new(); - if show_animation && self.animations_enabled { + if show_animation { let frame = self.animation.current_frame(); lines.extend(frame.lines().map(Into::into)); lines.push("".into()); @@ -99,6 +108,7 @@ impl StepStateProvider for WelcomeWidget { #[cfg(test)] mod tests { use super::*; + use pretty_assertions::assert_eq; use ratatui::buffer::Buffer; use ratatui::layout::Rect; @@ -106,31 +116,37 @@ mod tests { static VARIANT_B: [&str; 1] = ["frame-b"]; static VARIANTS: [&[&str]; 2] = [&VARIANT_A, &VARIANT_B]; + fn row_containing(buf: &Buffer, needle: &str) -> Option { + (0..buf.area.height).find(|&y| { + let mut row = String::new(); + for x in 0..buf.area.width { + row.push_str(buf[(x, y)].symbol()); + } + row.contains(needle) + }) + } + #[test] fn welcome_renders_animation_on_first_draw() { let widget = WelcomeWidget::new(false, FrameRequester::test_dummy(), true); let area = Rect::new(0, 0, MIN_ANIMATION_WIDTH, MIN_ANIMATION_HEIGHT); let mut buf = Buffer::empty(area); + let frame_lines = widget.animation.current_frame().lines().count() as u16; (&widget).render(area, &mut buf); - let mut found = false; - let mut last_non_empty: Option = 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; - } - } - } + let welcome_row = row_containing(&buf, "Welcome"); + assert_eq!(welcome_row, Some(frame_lines + 1)); + } - 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 welcome_skips_animation_below_height_breakpoint() { + let widget = WelcomeWidget::new(false, FrameRequester::test_dummy(), true); + let area = Rect::new(0, 0, MIN_ANIMATION_WIDTH, MIN_ANIMATION_HEIGHT - 1); + let mut buf = Buffer::empty(area); + (&widget).render(area, &mut buf); + + let welcome_row = row_containing(&buf, "Welcome"); + assert_eq!(welcome_row, Some(0)); } #[test] @@ -139,6 +155,7 @@ mod tests { is_logged_in: false, animation: AsciiAnimation::with_variants(FrameRequester::test_dummy(), &VARIANTS, 0), animations_enabled: true, + layout_area: Cell::new(None), }; let before = widget.animation.current_frame(); diff --git a/codex-rs/tui/src/snapshots/codex_tui__cwd_prompt__tests__cwd_prompt_fork_modal.snap b/codex-rs/tui/src/snapshots/codex_tui__cwd_prompt__tests__cwd_prompt_fork_modal.snap new file mode 100644 index 0000000000..38b712e872 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__cwd_prompt__tests__cwd_prompt_fork_modal.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/cwd_prompt.rs +expression: terminal.backend() +--- + +Choose working directory to fork this session + + Session = latest cwd recorded in the forked session + Current = your current working directory + +› 1. Use session directory (/Users/example/session) + 2. Use current directory (/Users/example/current) + + Press enter to continue diff --git a/codex-rs/tui/src/snapshots/codex_tui__cwd_prompt__tests__cwd_prompt_modal.snap b/codex-rs/tui/src/snapshots/codex_tui__cwd_prompt__tests__cwd_prompt_modal.snap new file mode 100644 index 0000000000..4e87cfdfc7 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__cwd_prompt__tests__cwd_prompt_modal.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/cwd_prompt.rs +expression: terminal.backend() +--- + +Choose working directory to resume this session + + Session = latest cwd recorded in the resumed session + Current = your current working directory + +› 1. Use session directory (/Users/example/session) + 2. Use current directory (/Users/example/current) + + Press enter to continue diff --git a/codex-rs/windows-sandbox-rs/src/command_runner_win.rs b/codex-rs/windows-sandbox-rs/src/command_runner_win.rs index 1739399901..a016c00c75 100644 --- a/codex-rs/windows-sandbox-rs/src/command_runner_win.rs +++ b/codex-rs/windows-sandbox-rs/src/command_runner_win.rs @@ -16,6 +16,7 @@ use codex_windows_sandbox::SandboxPolicy; use serde::Deserialize; use std::collections::HashMap; use std::ffi::c_void; +use std::path::Path; use std::path::PathBuf; use windows_sys::Win32::Foundation::CloseHandle; use windows_sys::Win32::Foundation::GetLastError; @@ -78,13 +79,20 @@ unsafe fn create_job_kill_on_close() -> Result { Ok(h) } +fn read_request_file(req_path: &Path) -> Result { + let content = std::fs::read_to_string(req_path) + .with_context(|| format!("read request file {}", req_path.display())); + let _ = std::fs::remove_file(req_path); + content +} + pub fn main() -> Result<()> { let mut input = String::new(); let mut args = std::env::args().skip(1); if let Some(first) = args.next() { if let Some(rest) = first.strip_prefix("--request-file=") { let req_path = PathBuf::from(rest); - input = std::fs::read_to_string(&req_path).context("read request file")?; + input = read_request_file(&req_path)?; } } if input.is_empty() { @@ -265,3 +273,21 @@ pub fn main() -> Result<()> { } std::process::exit(exit_code); } + +#[cfg(test)] +mod tests { + use super::read_request_file; + use pretty_assertions::assert_eq; + use std::fs; + + #[test] + fn removes_request_file_after_read() { + let dir = tempfile::tempdir().expect("tempdir"); + let req_path = dir.path().join("request.json"); + fs::write(&req_path, "{\"ok\":true}").expect("write request"); + + let content = read_request_file(&req_path).expect("read request"); + assert_eq!(content, "{\"ok\":true}"); + assert!(!req_path.exists(), "request file should be removed"); + } +}