Fix

codex-rs/core/src/agent/role.rs

@@ -1,11 +1,14 @@
 use crate::config::Config;
 use crate::config::ConfigOverrides;
+use crate::config::ConfigToml;
 use crate::config::deserialize_config_toml_with_base;
 use crate::config::find_codex_home;
 use crate::config_loader::ConfigLayerEntry;
 use crate::config_loader::ConfigLayerStack;
 use crate::config_loader::ConfigLayerStackOrdering;
 use codex_app_server_protocol::ConfigLayerSource;
+use codex_protocol::config_types::SandboxMode;
+use codex_utils_absolute_path::AbsolutePathBuf;
 use serde::Deserialize;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
@@ -95,6 +98,38 @@ pub(crate) async fn apply_role_to_config(
     };
 
     let original = config.clone();
+    let runtime_sandbox_mode = match original.permissions.sandbox_policy.get() {
+        crate::protocol::SandboxPolicy::ReadOnly { .. } => Some(SandboxMode::ReadOnly),
+        crate::protocol::SandboxPolicy::WorkspaceWrite { .. } => Some(SandboxMode::WorkspaceWrite),
+        crate::protocol::SandboxPolicy::DangerFullAccess => Some(SandboxMode::DangerFullAccess),
+        crate::protocol::SandboxPolicy::ExternalSandbox { .. } => None,
+    };
+    let runtime_layer_config = toml::Value::try_from(ConfigToml {
+        model: original.model.clone(),
+        model_provider: Some(original.model_provider_id.clone()),
+        sandbox_mode: runtime_sandbox_mode,
+        instructions: original.base_instructions.clone(),
+        developer_instructions: original.developer_instructions.clone(),
+        js_repl_node_path: original
+            .js_repl_node_path
+            .clone()
+            .and_then(|path| AbsolutePathBuf::try_from(path).ok()),
+        compact_prompt: original.compact_prompt.clone(),
+        show_raw_agent_reasoning: Some(original.show_raw_agent_reasoning),
+        model_reasoning_effort: original.model_reasoning_effort,
+        model_reasoning_summary: Some(original.model_reasoning_summary),
+        model_verbosity: original.model_verbosity,
+        personality: original.personality,
+        ..ConfigToml::default()
+    })
+    .map_err(|err| {
+        tracing::warn!(
+            agent_type = role_name,
+            error = %err,
+            "failed to serialize runtime fallback layer for role"
+        );
+        AGENT_TYPE_UNAVAILABLE_ERROR.to_string()
+    })?;
     let original_stack = &original.config_layer_stack;
     let mut layers = original
         .config_layer_stack
@@ -103,6 +138,13 @@ pub(crate) async fn apply_role_to_config(
         .cloned()
         .collect::<Vec<_>>();
 
+    let runtime_layer =
+        ConfigLayerEntry::new(ConfigLayerSource::SessionFlags, runtime_layer_config);
+    let runtime_layer_precedence = runtime_layer.name.precedence();
+    let runtime_layer_index =
+        layers.partition_point(|layer| layer.name.precedence() <= runtime_layer_precedence);
+    layers.insert(runtime_layer_index, runtime_layer);
+
     let role_layer = ConfigLayerEntry::new(ConfigLayerSource::SessionFlags, agent_config);
     let role_layer_precedence = role_layer.name.precedence();
     let role_layer_index =
@@ -134,11 +176,7 @@ pub(crate) async fn apply_role_to_config(
 
     *config = Config::load_config_with_layer_stack(
         layered_config,
-        ConfigOverrides {
-            cwd: Some(original.cwd.clone()),
-            codex_linux_sandbox_exe: original.codex_linux_sandbox_exe.clone(),
-            ..Default::default()
-        },
+        ConfigOverrides::for_role_reload(&original),
         original.codex_home.clone(),
         layered_stack,
     )
@@ -318,6 +356,7 @@ fn parse_agents_config(contents: &str, source: &str) -> Result<AgentsConfigToml,
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::built_in_model_providers;
     use crate::config::test_config;
     use codex_protocol::openai_models::ReasoningEffort;
     use codex_utils_absolute_path::AbsolutePathBuf;
@@ -573,6 +612,98 @@ enabled_tools = ["search"]
         ));
     }
 
+    /// Preserves runtime-only harness overrides (provider, ephemeral, writable roots)
+    /// when a role config layer forces a reload via `load_config_with_layer_stack`,
+    /// as long as the role does not explicitly set the same fields.
+    #[tokio::test]
+    async fn apply_role_to_config_preserves_runtime_overrides() {
+        let mut config = test_config();
+        config.model_provider_id = "lmstudio".to_string();
+        config.model_provider = built_in_model_providers()["lmstudio"].clone();
+        config.ephemeral = true;
+
+        let dir = TempDir::new().expect("tempdir");
+        let extra_root = AbsolutePathBuf::try_from(dir.path().to_path_buf()).expect("extra root");
+        config
+            .permissions
+            .sandbox_policy
+            .set(crate::protocol::SandboxPolicy::WorkspaceWrite {
+                writable_roots: vec![extra_root.clone()],
+                read_only_access: crate::protocol::ReadOnlyAccess::FullAccess,
+                network_access: false,
+                exclude_tmpdir_env_var: false,
+                exclude_slash_tmp: false,
+            })
+            .expect("sandbox_policy");
+
+        apply_role_to_config(&mut config, Some("explorer"))
+            .await
+            .expect("apply explorer role");
+
+        assert_eq!(config.model_provider_id, "lmstudio".to_string());
+        assert_eq!(
+            config.model_provider,
+            built_in_model_providers()["lmstudio"].clone()
+        );
+        assert_eq!(config.ephemeral, true);
+        assert_eq!(
+            config.permissions.sandbox_policy.get(),
+            &crate::protocol::SandboxPolicy::WorkspaceWrite {
+                writable_roots: vec![extra_root],
+                read_only_access: crate::protocol::ReadOnlyAccess::FullAccess,
+                network_access: false,
+                exclude_tmpdir_env_var: false,
+                exclude_slash_tmp: false,
+            }
+        );
+    }
+
+    #[tokio::test]
+    async fn apply_role_to_config_explicit_role_fields_beat_runtime_overrides() {
+        let dir = TempDir::new().expect("tempdir");
+        let planner_path = write_role_config_file(
+            &dir,
+            "agents/planner.toml",
+            r#"
+model_provider = "lmstudio"
+developer_instructions = "ROLE_DEVELOPER"
+compact_prompt = "ROLE_COMPACT"
+personality = "friendly"
+"#,
+        );
+        write_agents_config(
+            &dir,
+            &format!("[agents.planner]\nconfig_file = {planner_path:?}\n"),
+        );
+
+        let mut config = test_config();
+        config.codex_home = dir.path().to_path_buf();
+        config.model_provider_id = "openai".to_string();
+        config.model_provider = built_in_model_providers()["openai"].clone();
+        config.developer_instructions = Some("RUNTIME_DEVELOPER".to_string());
+        config.compact_prompt = Some("RUNTIME_COMPACT".to_string());
+        config.personality = Some(crate::config::types::Personality::Pragmatic);
+
+        apply_role_to_config(&mut config, Some("planner"))
+            .await
+            .expect("apply planner role");
+
+        assert_eq!(config.model_provider_id, "lmstudio".to_string());
+        assert_eq!(
+            config.model_provider,
+            built_in_model_providers()["lmstudio"].clone()
+        );
+        assert_eq!(
+            config.developer_instructions,
+            Some("ROLE_DEVELOPER".to_string())
+        );
+        assert_eq!(config.compact_prompt, Some("ROLE_COMPACT".to_string()));
+        assert_eq!(
+            config.personality,
+            Some(crate::config::types::Personality::Friendly)
+        );
+    }
+
     #[test]
     fn spawn_tool_spec_build_dedups_and_prefers_user_defined_roles() {
         let built_in_roles = parse_agents_config(

codex-rs/core/src/config/mod.rs

@@ -1323,6 +1323,34 @@ pub struct ConfigOverrides {
     pub additional_writable_roots: Vec<PathBuf>,
 }
 
+impl ConfigOverrides {
+    /// Builds the set of "runtime-only" overrides that must be preserved when
+    /// reloading a `Config` from a modified `ConfigLayerStack` (for example,
+    /// when applying an agent role config layer).
+    ///
+    /// This intentionally excludes fields that an agent role config is
+    /// expected to override (for example `model` or `sandbox_mode`).
+    pub(crate) fn for_role_reload(config: &Config) -> Self {
+        let additional_writable_roots = match config.permissions.sandbox_policy.get() {
+            SandboxPolicy::WorkspaceWrite { writable_roots, .. } => writable_roots
+                .iter()
+                .map(codex_utils_absolute_path::AbsolutePathBuf::to_path_buf)
+                .collect(),
+            SandboxPolicy::DangerFullAccess
+            | SandboxPolicy::ExternalSandbox { .. }
+            | SandboxPolicy::ReadOnly { .. } => Vec::new(),
+        };
+
+        Self {
+            cwd: Some(config.cwd.clone()),
+            codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(),
+            ephemeral: Some(config.ephemeral),
+            additional_writable_roots,
+            ..Default::default()
+        }
+    }
+}
+
 /// Resolves the OSS provider from CLI override, profile config, or global config.
 /// Returns `None` if no provider is configured at any level.
 pub fn resolve_oss_provider(
@@ -1651,7 +1679,9 @@ impl Config {
             .or(cfg.model_instructions_file.as_ref());
         let file_base_instructions =
             Self::try_read_non_empty_file(model_instructions_path, "model instructions file")?;
-        let base_instructions = base_instructions.or(file_base_instructions);
+        let base_instructions = base_instructions
+            .or(file_base_instructions)
+            .or(cfg.instructions);
         let developer_instructions = developer_instructions.or(cfg.developer_instructions);
         let personality = personality
             .or(config_profile.personality)
@@ -3961,6 +3991,60 @@ model = "gpt-5.1-codex"
         Ok(())
     }
 
+    #[test]
+    fn instructions_field_sets_base_instructions() -> std::io::Result<()> {
+        let codex_home = TempDir::new()?;
+        let cfg = ConfigToml {
+            instructions: Some("Use this base instruction".to_string()),
+            ..Default::default()
+        };
+
+        let config = Config::load_from_base_config_with_overrides(
+            cfg,
+            ConfigOverrides::default(),
+            codex_home.path().to_path_buf(),
+        )?;
+
+        assert_eq!(
+            config.base_instructions,
+            Some("Use this base instruction".to_string())
+        );
+
+        Ok(())
+    }
+
+    #[test]
+    fn model_instructions_file_wins_over_instructions_field() -> std::io::Result<()> {
+        let codex_home = TempDir::new()?;
+        let workspace = codex_home.path().join("workspace");
+        std::fs::create_dir_all(&workspace)?;
+
+        let instructions_path = workspace.join("role_instructions.txt");
+        std::fs::write(&instructions_path, "From model instructions file")?;
+
+        let cfg = ConfigToml {
+            instructions: Some("From instructions field".to_string()),
+            model_instructions_file: Some(AbsolutePathBuf::from_absolute_path(instructions_path)?),
+            ..Default::default()
+        };
+
+        let config = Config::load_from_base_config_with_overrides(
+            cfg,
+            ConfigOverrides {
+                cwd: Some(workspace),
+                ..Default::default()
+            },
+            codex_home.path().to_path_buf(),
+        )?;
+
+        assert_eq!(
+            config.base_instructions,
+            Some("From model instructions file".to_string())
+        );
+
+        Ok(())
+    }
+
     fn create_test_fixture() -> std::io::Result<PrecedenceTestFixture> {
         let toml = r#"
 model = "o3"