diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 32318cd62..596fe6d2f 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -53,14 +53,6 @@ "experimental_compact_prompt_file": { "$ref": "#/definitions/AbsolutePathBuf" }, - "experimental_instructions_file": { - "description": "Legacy, now use features", - "allOf": [ - { - "$ref": "#/definitions/AbsolutePathBuf" - } - ] - }, "experimental_use_freeform_apply_patch": { "type": "boolean" }, @@ -240,6 +232,14 @@ "type": "integer", "format": "int64" }, + "model_instructions_file": { + "description": "Optional path to a file containing model instructions that will override the built-in instructions for the selected model. Users are STRONGLY DISCOURAGED from using this field, as deviating from the instructions sanctioned by Codex will likely degrade model performance.", + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ] + }, "model_personality": { "description": "EXPERIMENTAL Optionally specify a personality for the model", "allOf": [ @@ -545,9 +545,6 @@ "experimental_compact_prompt_file": { "$ref": "#/definitions/AbsolutePathBuf" }, - "experimental_instructions_file": { - "$ref": "#/definitions/AbsolutePathBuf" - }, "experimental_use_freeform_apply_patch": { "type": "boolean" }, @@ -640,6 +637,14 @@ "model": { "type": "string" }, + "model_instructions_file": { + "description": "Optional path to a file containing model instructions.", + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ] + }, "model_personality": { "$ref": "#/definitions/Personality" }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b7dbd0796..5bc6156c8 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -666,6 +666,19 @@ impl Session { msg: EventMsg::DeprecationNotice(DeprecationNoticeEvent { summary, details }), }); } + if crate::config::uses_deprecated_instructions_file(&config.config_layer_stack) { + post_session_configured_events.push(Event { + id: INITIAL_SUBMIT_ID.to_owned(), + msg: EventMsg::DeprecationNotice(DeprecationNoticeEvent { + summary: "`experimental_instructions_file` is deprecated and ignored. Use `model_instructions_file` instead." + .to_string(), + details: Some( + "Move the setting to `model_instructions_file` in config.toml (or under a profile) to load instructions from a file." + .to_string(), + ), + }), + }); + } maybe_push_chat_wire_api_deprecation(&config, &mut post_session_configured_events); let auth = auth_manager.auth().await; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 7d1426e7a..934c54263 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -820,6 +820,12 @@ pub struct ConfigToml { #[serde(default)] pub developer_instructions: Option, + /// Optional path to a file containing model instructions that will override + /// the built-in instructions for the selected model. Users are STRONGLY + /// DISCOURAGED from using this field, as deviating from the instructions + /// sanctioned by Codex will likely degrade model performance. + pub model_instructions_file: Option, + /// Compact prompt used for history compaction. pub compact_prompt: Option, @@ -965,6 +971,8 @@ pub struct ConfigToml { pub notice: Option, /// Legacy, now use features + /// Deprecated: ignored. Use `model_instructions_file`. + #[schemars(skip)] pub experimental_instructions_file: Option, pub experimental_compact_prompt_file: Option, pub experimental_use_unified_exec_tool: Option, @@ -1432,14 +1440,12 @@ impl Config { // Load base instructions override from a file if specified. If the // path is relative, resolve it against the effective cwd so the // behaviour matches other path-like config values. - let experimental_instructions_path = config_profile - .experimental_instructions_file + let model_instructions_path = config_profile + .model_instructions_file .as_ref() - .or(cfg.experimental_instructions_file.as_ref()); - let file_base_instructions = Self::try_read_non_empty_file( - experimental_instructions_path, - "experimental instructions file", - )?; + .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 developer_instructions = developer_instructions.or(cfg.developer_instructions); @@ -1682,6 +1688,30 @@ impl Config { } } +pub(crate) fn uses_deprecated_instructions_file(config_layer_stack: &ConfigLayerStack) -> bool { + config_layer_stack + .layers_high_to_low() + .into_iter() + .any(|layer| toml_uses_deprecated_instructions_file(&layer.config)) +} + +fn toml_uses_deprecated_instructions_file(value: &TomlValue) -> bool { + let Some(table) = value.as_table() else { + return false; + }; + if table.contains_key("experimental_instructions_file") { + return true; + } + let Some(profiles) = table.get("profiles").and_then(TomlValue::as_table) else { + return false; + }; + profiles.values().any(|profile| { + profile.as_table().is_some_and(|profile_table| { + profile_table.contains_key("experimental_instructions_file") + }) + }) +} + /// Returns the path to the Codex configuration directory, which can be /// specified by the `CODEX_HOME` environment variable. If not set, defaults to /// `~/.codex`. diff --git a/codex-rs/core/src/config/profile.rs b/codex-rs/core/src/config/profile.rs index 4f1a964eb..e78b02374 100644 --- a/codex-rs/core/src/config/profile.rs +++ b/codex-rs/core/src/config/profile.rs @@ -27,6 +27,10 @@ pub struct ConfigProfile { pub model_verbosity: Option, pub model_personality: Option, pub chatgpt_base_url: Option, + /// Optional path to a file containing model instructions. + pub model_instructions_file: Option, + /// Deprecated: ignored. Use `model_instructions_file`. + #[schemars(skip)] pub experimental_instructions_file: Option, pub experimental_compact_prompt_file: Option, pub include_apply_patch_tool: Option, diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 0460e0157..ffde19f1f 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -656,7 +656,7 @@ mod unit_tests { let contents = r#" # This is a field recognized by config.toml that is an AbsolutePathBuf in # the ConfigToml struct. -experimental_instructions_file = "./some_file.md" +model_instructions_file = "./some_file.md" # This is a field recognized by config.toml. model = "gpt-1000" @@ -669,7 +669,7 @@ foo = "xyzzy" let normalized_toml_value = resolve_relative_paths_in_config_toml(user_config, base_dir)?; let mut expected_toml_value = toml::map::Map::new(); expected_toml_value.insert( - "experimental_instructions_file".to_string(), + "model_instructions_file".to_string(), TomlValue::String( AbsolutePathBuf::resolve_path_against_base("./some_file.md", base_dir)? .as_path() diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 773d9951d..365fcdc59 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -438,10 +438,10 @@ async fn project_paths_resolve_relative_to_dot_codex_and_override_in_order() -> tokio::fs::write(project_root.join(".git"), "gitdir: here").await?; let root_cfg = r#" -experimental_instructions_file = "root.txt" +model_instructions_file = "root.txt" "#; let nested_cfg = r#" -experimental_instructions_file = "child.txt" +model_instructions_file = "child.txt" "#; tokio::fs::write(project_root.join(".codex").join(CONFIG_TOML_FILE), root_cfg).await?; tokio::fs::write(nested.join(".codex").join(CONFIG_TOML_FILE), nested_cfg).await?; @@ -591,7 +591,7 @@ async fn cli_overrides_with_relative_paths_do_not_break_trust_check() -> std::io let cwd = AbsolutePathBuf::from_absolute_path(&nested)?; let cli_overrides = vec![( - "experimental_instructions_file".to_string(), + "model_instructions_file".to_string(), TomlValue::String("relative.md".to_string()), )]; diff --git a/codex-rs/core/tests/suite/cli_stream.rs b/codex-rs/core/tests/suite/cli_stream.rs index 12be98723..57142b07f 100644 --- a/codex-rs/core/tests/suite/cli_stream.rs +++ b/codex-rs/core/tests/suite/cli_stream.rs @@ -109,11 +109,11 @@ async fn chat_mode_stream_cli() { ); } -/// Verify that passing `-c experimental_instructions_file=...` to the CLI +/// Verify that passing `-c model_instructions_file=...` to the CLI /// overrides the built-in base instructions by inspecting the request body /// received by a mock OpenAI Responses endpoint. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn exec_cli_applies_experimental_instructions_file() { +async fn exec_cli_applies_model_instructions_file() { skip_if_no_network!(); // Start mock server which will capture the request and return a minimal @@ -128,7 +128,7 @@ async fn exec_cli_applies_experimental_instructions_file() { // Create a temporary instructions file with a unique marker we can assert // appears in the outbound request payload. let custom = TempDir::new().unwrap(); - let marker = "cli-experimental-instructions-marker"; + let marker = "cli-model-instructions-file-marker"; let custom_path = custom.path().join("instr.md"); std::fs::write(&custom_path, marker).unwrap(); let custom_path_str = custom_path.to_string_lossy().replace('\\', "/"); @@ -151,9 +151,7 @@ async fn exec_cli_applies_experimental_instructions_file() { .arg("-c") .arg("model_provider=\"mock\"") .arg("-c") - .arg(format!( - "experimental_instructions_file=\"{custom_path_str}\"" - )) + .arg(format!("model_instructions_file=\"{custom_path_str}\"")) .arg("-C") .arg(&repo_root) .arg("hello?\n"); diff --git a/codex-rs/core/tests/suite/deprecation_notice.rs b/codex-rs/core/tests/suite/deprecation_notice.rs index bab715ebd..d401a64cd 100644 --- a/codex-rs/core/tests/suite/deprecation_notice.rs +++ b/codex-rs/core/tests/suite/deprecation_notice.rs @@ -1,15 +1,22 @@ #![cfg(not(target_os = "windows"))] use anyhow::Ok; +use codex_app_server_protocol::ConfigLayerSource; +use codex_core::config_loader::ConfigLayerEntry; +use codex_core::config_loader::ConfigLayerStack; +use codex_core::config_loader::ConfigRequirements; +use codex_core::config_loader::ConfigRequirementsToml; use codex_core::features::Feature; use codex_core::protocol::DeprecationNoticeEvent; use codex_core::protocol::EventMsg; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; +use core_test_support::test_absolute_path; use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event_match; use pretty_assertions::assert_eq; +use toml::Value as TomlValue; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn emits_deprecation_notice_for_legacy_feature_flag() -> anyhow::Result<()> { @@ -48,3 +55,58 @@ async fn emits_deprecation_notice_for_legacy_feature_flag() -> anyhow::Result<() Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn emits_deprecation_notice_for_experimental_instructions_file() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + + let mut builder = test_codex().with_config(|config| { + let mut table = toml::map::Map::new(); + table.insert( + "experimental_instructions_file".to_string(), + TomlValue::String("legacy.md".to_string()), + ); + let config_layer = ConfigLayerEntry::new( + ConfigLayerSource::User { + file: test_absolute_path("/tmp/config.toml"), + }, + TomlValue::Table(table), + ); + let config_layer_stack = ConfigLayerStack::new( + vec![config_layer], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("build config layer stack"); + config.config_layer_stack = config_layer_stack; + }); + + let TestCodex { codex, .. } = builder.build(&server).await?; + + let notice = wait_for_event_match(&codex, |event| match event { + EventMsg::DeprecationNotice(ev) + if ev.summary.contains("experimental_instructions_file") => + { + Some(ev.clone()) + } + _ => None, + }) + .await; + + let DeprecationNoticeEvent { summary, details } = notice; + assert_eq!( + summary, + "`experimental_instructions_file` is deprecated and ignored. Use `model_instructions_file` instead." + .to_string(), + ); + assert_eq!( + details.as_deref(), + Some( + "Move the setting to `model_instructions_file` in config.toml (or under a profile) to load instructions from a file." + ), + ); + + Ok(()) +}