diff --git a/codex-rs/core/src/context/world_state/environment.rs b/codex-rs/core/src/context/world_state/environment.rs index 93080b9c58..8f38946ede 100644 --- a/codex-rs/core/src/context/world_state/environment.rs +++ b/codex-rs/core/src/context/world_state/environment.rs @@ -5,6 +5,7 @@ use crate::context::environment_context::FileSystemContext; use crate::context::environment_context::NetworkContext; use crate::context::environment_context::push_xml_escaped_text; use crate::environment_selection::TurnEnvironmentSnapshot; +use crate::environment_selection::TurnEnvironmentState; use crate::session::turn_context::TurnContext; use crate::shell::ShellType; use codex_features::Feature; @@ -114,6 +115,7 @@ impl WorldStateSection for EnvironmentsState { EnvironmentSnapshot { cwd: environment.cwd.inferred_native_path_string(), status: environment.status, + error: environment.error.clone(), shell: environment.shell.clone(), is_primary: self.environments.len() > 1 && environment.is_primary, }, @@ -321,6 +323,16 @@ fn push_environment_values(rendered: &mut String, environment: &EnvironmentState rendered.push_str(indent); rendered.push_str("starting\n"); } + if environment.status == EnvironmentStatus::Failed { + rendered.push_str(indent); + rendered.push_str("failed\n"); + } + if let Some(error) = &environment.error { + rendered.push_str(indent); + rendered.push_str(""); + push_xml_escaped_text(rendered, error); + rendered.push_str("\n"); + } if let Some(shell) = &environment.shell { rendered.push_str(indent); rendered.push_str(""); @@ -347,6 +359,7 @@ struct EnvironmentState { cwd: PathUri, status: EnvironmentStatus, shell: Option, + error: Option, is_primary: bool, } @@ -367,6 +380,8 @@ struct EnvironmentSnapshot { cwd: String, status: EnvironmentStatus, shell: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + error: Option, #[serde(default, skip_serializing_if = "std::ops::Not::not")] is_primary: bool, } @@ -375,6 +390,7 @@ impl EnvironmentSnapshot { fn has_same_diff_value(&self, other: &Self) -> bool { self.cwd == other.cwd && self.status == other.status + && self.error == other.error && self.is_primary == other.is_primary && self .shell @@ -389,6 +405,7 @@ impl EnvironmentSnapshot { enum EnvironmentStatus { Starting, Available, + Failed, } async fn powershell_version(shell_path: &Path) -> Option { @@ -441,6 +458,7 @@ fn environment_states(snapshot: &TurnEnvironmentSnapshot) -> BTreeMap BTreeMap) -> (String, Env EnvironmentState { cwd, status: EnvironmentStatus::Available, + error: None, shell: Some(shell.into()), is_primary: false, }, diff --git a/codex-rs/core/src/context/world_state/environment_tests.rs b/codex-rs/core/src/context/world_state/environment_tests.rs index 043dd67839..6bda9b79b6 100644 --- a/codex-rs/core/src/context/world_state/environment_tests.rs +++ b/codex-rs/core/src/context/world_state/environment_tests.rs @@ -105,6 +105,7 @@ fn snapshots() -> Result<()> { EnvironmentState { cwd: PathUri::parse("file:///repo")?, status: EnvironmentStatus::Available, + error: None, shell: None, is_primary: false, }, @@ -308,6 +309,7 @@ fn available(cwd: &str, shell: &str) -> Result { Ok(EnvironmentState { cwd: PathUri::parse(cwd)?, status: EnvironmentStatus::Available, + error: None, shell: Some(shell.to_string()), is_primary: false, }) @@ -324,7 +326,91 @@ fn starting(cwd: &str) -> Result { Ok(EnvironmentState { cwd: PathUri::parse(cwd)?, status: EnvironmentStatus::Starting, + error: None, shell: None, is_primary: false, }) } + +#[test] +fn failure_context_is_escaped_incremental_and_cleared_on_recovery() -> Result<()> { + let failed = EnvironmentsState { + environments: [( + "remote".to_string(), + EnvironmentState { + cwd: PathUri::parse("file:///workspace")?, + status: EnvironmentStatus::Failed, + error: Some("Repository & unavailable".to_string()), + shell: None, + is_primary: false, + }, + )] + .into_iter() + .collect(), + ..Default::default() + }; + insta::assert_snapshot!(failed.body(), @r#" + + + + /workspace + failed + Repository <empty> & unavailable + + + "#); + assert!( + failed + .render_diff(PreviousSectionState::Known(&failed.snapshot())) + .is_none() + ); + let recovered = EnvironmentsState { + environments: [( + "remote".to_string(), + available("file:///workspace", "bash")?, + )] + .into_iter() + .collect(), + ..Default::default() + }; + let update = recovered + .render_diff(PreviousSectionState::Known(&failed.snapshot())) + .expect("recovery must be visible to the model"); + assert!(!update.body().contains("")); + assert!(update.body().contains("bash")); + Ok(()) +} + +#[test] +fn failure_context_limits_total_detail_bytes_at_utf8_boundaries() { + use codex_protocol::protocol::EnvironmentConfigState; + use codex_protocol::protocol::TurnEnvironmentSelection; + let snapshot = TurnEnvironmentSnapshot { + environments: (0..4) + .map(|index| TurnEnvironmentState::Failed { + selection: TurnEnvironmentSelection { + environment_id: format!("remote-{index}"), + cwd: PathUri::parse("file:///workspace").unwrap(), + workspace_roots: Vec::new(), + config: EnvironmentConfigState::FromThread, + }, + error: "界".repeat(300), + }) + .collect(), + }; + let states = environment_states(&snapshot); + let details: Vec<_> = states + .values() + .map(|state| state.error.as_deref()) + .collect(); + let truncated = "界".repeat(85); + assert_eq!( + details, + vec![ + Some(truncated.as_str()), + Some(truncated.as_str()), + None, + None + ] + ); +} diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 1bc9a08750..59375c92a8 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -818,11 +818,11 @@ impl ThreadEnvironments { let selected = self.environments.load_full(); let mut environments = Vec::with_capacity(selected.len()); for environment in selected.iter() { - if matches!( - environment.selection.config, - EnvironmentConfigState::Failed(_) - ) { - environments.push(TurnEnvironmentState::Failed); + if let EnvironmentConfigState::Failed(error) = &environment.selection.config { + environments.push(TurnEnvironmentState::Failed { + selection: environment.selection.clone(), + error: error.clone(), + }); continue; } let pending = matches!( @@ -857,7 +857,10 @@ pub(crate) enum TurnEnvironmentState { Ready(TurnEnvironment), Starting(StartingTurnEnvironment), /// Unavailable for execution, but still selected when evaluating permissions. - Failed, + Failed { + selection: TurnEnvironmentSelection, + error: String, + }, } impl TurnEnvironmentState { @@ -870,7 +873,10 @@ impl TurnEnvironmentState { let mut selection = starting.selection; if matches!(selection.config, EnvironmentConfigState::Pending) { let Some(config) = environment.installed_config else { - return Self::Failed; + return Self::Failed { + selection, + error: "Environment configuration was not supplied.".to_string(), + }; }; selection.config = EnvironmentConfigState::Ready(config); } @@ -896,7 +902,10 @@ impl TurnEnvironmentState { environment_id = %starting.selection.environment_id, "skipping failed turn environment: {err}" ); - Self::Failed + Self::Failed { + selection: starting.selection, + error: err.to_string(), + } } None => Self::Starting(starting), } @@ -925,7 +934,7 @@ impl TurnEnvironmentSnapshot { .iter() .map(|environment| match environment { TurnEnvironmentState::Ready(environment) => &environment.selection.config, - TurnEnvironmentState::Starting(_) | TurnEnvironmentState::Failed => { + TurnEnvironmentState::Starting(_) | TurnEnvironmentState::Failed { .. } => { &EnvironmentConfigState::Pending } }), @@ -947,7 +956,7 @@ impl TurnEnvironmentSnapshot { environment.resolution.clone().now_or_never(), ) } - TurnEnvironmentState::Failed => TurnEnvironmentState::Failed, + TurnEnvironmentState::Failed { .. } => environment.clone(), }) .collect(); Self { environments } @@ -1022,7 +1031,7 @@ impl TurnEnvironmentSnapshot { { environment.selection.cwd.to_abs_path().ok() } - TurnEnvironmentState::Starting(_) | TurnEnvironmentState::Failed => None, + TurnEnvironmentState::Starting(_) | TurnEnvironmentState::Failed { .. } => None, }) } diff --git a/codex-rs/core/src/tools/handlers/wait_for_environment.rs b/codex-rs/core/src/tools/handlers/wait_for_environment.rs index daa9b81d11..95c423fc09 100644 --- a/codex-rs/core/src/tools/handlers/wait_for_environment.rs +++ b/codex-rs/core/src/tools/handlers/wait_for_environment.rs @@ -7,6 +7,7 @@ use serde::Deserialize; use serde_json::json; use std::collections::BTreeMap; +use crate::environment_selection::TurnEnvironmentState; use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; @@ -128,6 +129,22 @@ impl ToolExecutor for WaitForEnvironmentHandler { .turn_environments() .any(|environment| environment.selection.environment_id == environment_id); if !already_ready { + if let Some(error) = + step_context + .environments + .environments + .iter() + .find_map(|state| match state { + TurnEnvironmentState::Failed { selection, error } + if selection.environment_id == environment_id => + { + Some(error) + } + _ => None, + }) + { + return Err(environment_failure(&environment_id, error)); + } let Some(environment) = step_context .environments .starting() @@ -139,11 +156,10 @@ impl ToolExecutor for WaitForEnvironmentHandler { ))); }; - environment.wait_until_ready().await.map_err(|_| { - FunctionCallError::RespondToModel(format!( - "Environment `{environment_id}` failed to start and is unavailable. Continue without it." - )) - })?; + environment + .wait_until_ready() + .await + .map_err(|error| environment_failure(&environment_id, &error.to_string()))?; } Ok(boxed_tool_output(JsonToolOutput::new(json!({ @@ -155,3 +171,12 @@ impl ToolExecutor for WaitForEnvironmentHandler { } impl CoreToolRuntime for WaitForEnvironmentHandler {} + +// Keep failure output small while preserving a UTF-8 boundary. +fn environment_failure(environment_id: &str, error: &str) -> FunctionCallError { + const MAX_ERROR_BYTES: usize = 256; + let reason = &error[..error.floor_char_boundary(MAX_ERROR_BYTES)]; + FunctionCallError::RespondToModel(format!( + "Environment `{environment_id}` failed to start: {reason}" + )) +} diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index ab764061a0..eada6c37ba 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -4023,3 +4023,6 @@ async fn remote_test_env_copy_preserves_symlink_source() -> Result<()> { .await?; Ok(()) } + +#[path = "remote_env_failure_tests.rs"] +mod failure_tests; diff --git a/codex-rs/core/tests/suite/remote_env_failure_tests.rs b/codex-rs/core/tests/suite/remote_env_failure_tests.rs new file mode 100644 index 0000000000..8b0e4b2935 --- /dev/null +++ b/codex-rs/core/tests/suite/remote_env_failure_tests.rs @@ -0,0 +1,106 @@ +//! Provisioning failures reach the model without terminating its turn. +use super::*; +use pretty_assertions::assert_eq; +use test_case::test_case; + +#[test_case(true; "failed_before_turn")] +#[test_case(false; "failed_while_waiting")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn provisioning_failure_reaches_model_and_turn_continues( + fail_before_turn: bool, +) -> Result<()> { + const REASON: &str = "This repository is empty. Push an initial commit, then retry."; + const CALL_ID: &str = "wait-for-failed-environment"; + let server = start_mock_server().await; + let response_mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_function_call( + CALL_ID, + "wait_for_environment", + &json!({ + "environment_id": REMOTE_ENVIRONMENT_ID, + }) + .to_string(), + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_assistant_message( + "message-1", + "I can explain how to initialize the repository.", + ), + ev_completed("resp-2"), + ]), + ], + ) + .await; + let mut builder = test_codex_with_wait_for_environment().with_config(|config| { + assert!(config.features.enable(Feature::DeferredExecutor).is_ok()); + }); + let test = expect_startup(builder.build(&server)).await; + let manager = test.thread_manager.environment_manager(); + let provider = Arc::new(FailingNoiseConnectProvider::default()); + manager.materialize_pending_noise_environment( + REMOTE_ENVIRONMENT_ID.to_string(), + provider.clone(), + )?; + if fail_before_turn { + manager.report_environment_provisioning_status( + REMOTE_ENVIRONMENT_ID.to_string(), + Err(REASON.to_string()), + provider.clone(), + )?; + } + test.codex + .start_or_steer_turn( + TurnInputRequest::user_input(vec![UserInput::Text { + text: "inspect the repository".into(), + text_elements: Vec::new(), + }]) + .with_thread_settings(ThreadSettingsOverrides { + environments: Some(TurnEnvironmentSelections::new( + test.config.cwd.clone(), + vec![TurnEnvironmentSelection { + environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&test.config.cwd), + workspace_roots: vec![PathUri::from_abs_path(&test.config.cwd)], + config: EnvironmentConfigState::FromThread, + }], + )), + ..Default::default() + }), + ) + .await?; + wait_for_response_request_count(&response_mock, /*expected_count*/ 1).await; + if !fail_before_turn { + manager.report_environment_provisioning_status( + REMOTE_ENVIRONMENT_ID.to_string(), + Err(REASON.to_string()), + provider, + )?; + } + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + let requests = response_mock.requests(); + assert_eq!(requests.len(), 2); + let (output, _) = requests[1] + .function_call_output_content_and_success(CALL_ID) + .context("the model should receive the failed wait result")?; + assert!( + output.as_deref().is_some_and(|text| text.contains(REASON)), + "{output:?}" + ); + let context_request = if fail_before_turn { + &requests[0] + } else { + &requests[1] + }; + let context = context_request.message_input_texts("user").join("\n"); + assert!(context.contains("failed"), "{context}"); + assert!(context.contains(REASON), "{context}"); + Ok(()) +}