From 8d1964686196b7231e7fc2a2f522d715476b5b0c Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Fri, 3 Apr 2026 13:47:56 -1000 Subject: [PATCH] [codex] allow disabling prompt instruction blocks (#16735) This PR adds root and profile config switches to omit the generated `` and `` prompt blocks while keeping both enabled by default, and it gates both the initial developer-context injection and later permissions diff injection so turning the permissions block off stays effective across turn-context overrides. Also added a prompt debug tool that can be used as `codex debug prompt-input "hello"` and dumps the constructed items list. --- codex-rs/cli/src/main.rs | 122 ++++++++++++++ codex-rs/core/config.schema.json | 16 +- codex-rs/core/src/codex.rs | 37 +++-- codex-rs/core/src/config/config_tests.rs | 34 ++++ codex-rs/core/src/config/mod.rs | 22 +++ codex-rs/core/src/config/profile.rs | 2 + codex-rs/core/src/context_manager/updates.rs | 4 + codex-rs/core/src/lib.rs | 2 + codex-rs/core/src/prompt_debug.rs | 152 ++++++++++++++++++ codex-rs/core/tests/suite/client.rs | 120 ++++++++------ .../core/tests/suite/permissions_messages.rs | 140 ++++++++++------ 11 files changed, 531 insertions(+), 120 deletions(-) create mode 100644 codex-rs/core/src/prompt_debug.rs diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index f889e2256..1c4f5068f 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -51,6 +51,8 @@ use codex_core::config::find_codex_home; use codex_features::FEATURES; use codex_features::Stage; use codex_features::is_known_feature_key; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::user_input::UserInput; use codex_terminal_detection::TerminalName; /// Codex CLI @@ -170,6 +172,9 @@ enum DebugSubcommand { /// Tooling: helps debug the app server. AppServer(DebugAppServerCommand), + /// Render the model-visible prompt input list as JSON. + PromptInput(DebugPromptInputCommand), + /// Internal: reset local memory state for a fresh start. #[clap(hide = true)] ClearMemories, @@ -193,6 +198,17 @@ struct DebugAppServerSendMessageV2Command { user_message: String, } +#[derive(Debug, Parser)] +struct DebugPromptInputCommand { + /// Optional user prompt to append after session context. + #[arg(value_name = "PROMPT")] + prompt: Option, + + /// Optional image(s) to attach to the user prompt. + #[arg(long = "image", short = 'i', value_name = "FILE", value_delimiter = ',', num_args = 1..)] + images: Vec, +} + #[derive(Debug, Parser)] struct ResumeCommand { /// Conversation/session id (UUID) or thread name. UUIDs take precedence if it parses. @@ -915,6 +931,20 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { )?; run_debug_app_server_command(cmd).await?; } + DebugSubcommand::PromptInput(cmd) => { + reject_remote_mode_for_subcommand( + root_remote.as_deref(), + root_remote_auth_token_env.as_deref(), + "debug prompt-input", + )?; + run_debug_prompt_input_command( + cmd, + root_config_overrides, + interactive, + arg0_paths.clone(), + ) + .await?; + } DebugSubcommand::ClearMemories => { reject_remote_mode_for_subcommand( root_remote.as_deref(), @@ -1083,6 +1113,72 @@ fn maybe_print_under_development_feature_warning( ); } +async fn run_debug_prompt_input_command( + cmd: DebugPromptInputCommand, + root_config_overrides: CliConfigOverrides, + interactive: TuiCli, + arg0_paths: Arg0DispatchPaths, +) -> anyhow::Result<()> { + let mut cli_kv_overrides = root_config_overrides + .parse_overrides() + .map_err(anyhow::Error::msg)?; + if interactive.web_search { + cli_kv_overrides.push(( + "web_search".to_string(), + toml::Value::String("live".to_string()), + )); + } + + let approval_policy = if interactive.full_auto { + Some(AskForApproval::OnRequest) + } else if interactive.dangerously_bypass_approvals_and_sandbox { + Some(AskForApproval::Never) + } else { + interactive.approval_policy.map(Into::into) + }; + let sandbox_mode = if interactive.full_auto { + Some(codex_protocol::config_types::SandboxMode::WorkspaceWrite) + } else if interactive.dangerously_bypass_approvals_and_sandbox { + Some(codex_protocol::config_types::SandboxMode::DangerFullAccess) + } else { + interactive.sandbox_mode.map(Into::into) + }; + let overrides = ConfigOverrides { + model: interactive.model, + config_profile: interactive.config_profile, + approval_policy, + sandbox_mode, + cwd: interactive.cwd, + codex_self_exe: arg0_paths.codex_self_exe, + codex_linux_sandbox_exe: arg0_paths.codex_linux_sandbox_exe, + main_execve_wrapper_exe: arg0_paths.main_execve_wrapper_exe, + show_raw_agent_reasoning: interactive.oss.then_some(true), + ephemeral: Some(true), + additional_writable_roots: interactive.add_dir, + ..Default::default() + }; + let config = + Config::load_with_cli_overrides_and_harness_overrides(cli_kv_overrides, overrides).await?; + + let mut input = interactive + .images + .into_iter() + .chain(cmd.images) + .map(|path| UserInput::LocalImage { path }) + .collect::>(); + if let Some(prompt) = cmd.prompt.or(interactive.prompt) { + input.push(UserInput::Text { + text: prompt.replace("\r\n", "\n").replace('\r', "\n"), + text_elements: Vec::new(), + }); + } + + let prompt_input = codex_core::prompt_debug::build_prompt_input(config, input).await?; + println!("{}", serde_json::to_string_pretty(&prompt_input)?); + + Ok(()) +} + async fn run_debug_clear_memories_command( root_config_overrides: &CliConfigOverrides, interactive: &TuiCli, @@ -1489,6 +1585,32 @@ mod tests { app_server } + #[test] + fn debug_prompt_input_parses_prompt_and_images() { + let cli = MultitoolCli::try_parse_from([ + "codex", + "debug", + "prompt-input", + "hello", + "--image", + "/tmp/a.png,/tmp/b.png", + ]) + .expect("parse"); + + let Some(Subcommand::Debug(DebugCommand { + subcommand: DebugSubcommand::PromptInput(cmd), + })) = cli.subcommand + else { + panic!("expected debug prompt-input subcommand"); + }; + + assert_eq!(cmd.prompt.as_deref(), Some("hello")); + assert_eq!( + cmd.images, + vec![PathBuf::from("/tmp/a.png"), PathBuf::from("/tmp/b.png")] + ); + } + fn sample_exit_info(conversation_id: Option<&str>, thread_name: Option<&str>) -> AppExitInfo { let token_usage = TokenUsage { output_tokens: 2, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index a8f3229b5..30a290cd3 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -521,6 +521,12 @@ "include_apply_patch_tool": { "type": "boolean" }, + "include_apps_instructions": { + "type": "boolean" + }, + "include_permissions_instructions": { + "type": "boolean" + }, "js_repl_node_module_dirs": { "description": "Ordered list of directories to search for Node modules in `js_repl`.", "items": { @@ -2268,6 +2274,14 @@ "default": null, "description": "Settings that govern if and what will be written to `~/.codex/history.jsonl`." }, + "include_apps_instructions": { + "description": "Whether to inject the `` developer block.", + "type": "boolean" + }, + "include_permissions_instructions": { + "description": "Whether to inject the `` developer block.", + "type": "boolean" + }, "instructions": { "description": "System instructions.", "type": "string" @@ -2627,4 +2641,4 @@ }, "title": "ConfigToml", "type": "object" -} \ No newline at end of file +} diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 241c249ce..2ef0af1dd 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -3578,22 +3578,24 @@ impl Session { { developer_sections.push(model_switch_message.into_text()); } - developer_sections.push( - DeveloperInstructions::from_policy( - turn_context.sandbox_policy.get(), - turn_context.approval_policy.value(), - turn_context.config.approvals_reviewer, - self.services.exec_policy.current().as_ref(), - &turn_context.cwd, - turn_context - .features - .enabled(Feature::ExecPermissionApprovals), - turn_context - .features - .enabled(Feature::RequestPermissionsTool), - ) - .into_text(), - ); + if turn_context.config.include_permissions_instructions { + developer_sections.push( + DeveloperInstructions::from_policy( + turn_context.sandbox_policy.get(), + turn_context.approval_policy.value(), + turn_context.config.approvals_reviewer, + self.services.exec_policy.current().as_ref(), + &turn_context.cwd, + turn_context + .features + .enabled(Feature::ExecPermissionApprovals), + turn_context + .features + .enabled(Feature::RequestPermissionsTool), + ) + .into_text(), + ); + } let separate_guardian_developer_message = crate::guardian::is_guardian_reviewer_source(&session_source); // Keep the guardian policy prompt out of the aggregated developer bundle so it @@ -3643,7 +3645,7 @@ impl Session { ); } } - if turn_context.apps_enabled() { + if turn_context.config.include_apps_instructions && turn_context.apps_enabled() { let mcp_connection_manager = self.services.mcp_connection_manager.read().await; let accessible_and_enabled_connectors = connectors::list_accessible_and_enabled_connectors_from_manager( @@ -6485,6 +6487,7 @@ pub(crate) fn build_prompt( output_schema: turn_context.final_output_json_schema.clone(), } } + #[allow(clippy::too_many_arguments)] #[instrument(level = "trace", skip_all, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 4f7e8283f..cadd666f5 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -4493,6 +4493,8 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> { base_instructions: None, developer_instructions: None, guardian_developer_instructions: None, + include_permissions_instructions: true, + include_apps_instructions: true, compact_prompt: None, commit_attribution: None, forced_chatgpt_workspace_id: None, @@ -4635,6 +4637,8 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> { base_instructions: None, developer_instructions: None, guardian_developer_instructions: None, + include_permissions_instructions: true, + include_apps_instructions: true, compact_prompt: None, commit_attribution: None, forced_chatgpt_workspace_id: None, @@ -4775,6 +4779,8 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> { base_instructions: None, developer_instructions: None, guardian_developer_instructions: None, + include_permissions_instructions: true, + include_apps_instructions: true, compact_prompt: None, commit_attribution: None, forced_chatgpt_workspace_id: None, @@ -4901,6 +4907,8 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> { base_instructions: None, developer_instructions: None, guardian_developer_instructions: None, + include_permissions_instructions: true, + include_apps_instructions: true, compact_prompt: None, commit_attribution: None, forced_chatgpt_workspace_id: None, @@ -5783,6 +5791,32 @@ async fn approvals_reviewer_defaults_to_manual_only_without_guardian_feature() - Ok(()) } +#[tokio::test] +async fn prompt_instruction_blocks_can_be_disabled_from_config_and_profiles() -> std::io::Result<()> +{ + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#"include_permissions_instructions = false +include_apps_instructions = false +profile = "chatty" + +[profiles.chatty] +include_permissions_instructions = true +"#, + )?; + + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + + assert!(config.include_permissions_instructions); + assert!(!config.include_apps_instructions); + Ok(()) +} + #[tokio::test] async fn approvals_reviewer_stays_manual_only_when_guardian_feature_is_enabled() -> std::io::Result<()> { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index d0017c7b2..bde82aadc 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -280,6 +280,12 @@ pub struct Config { /// Guardian-specific developer instructions override from requirements.toml. pub guardian_developer_instructions: Option, + /// Whether to inject the `` developer block. + pub include_permissions_instructions: bool, + + /// Whether to inject the `` developer block. + pub include_apps_instructions: bool, + /// Compact prompt override. pub compact_prompt: Option, @@ -1183,6 +1189,12 @@ pub struct ConfigToml { #[serde(default)] pub developer_instructions: Option, + /// Whether to inject the `` developer block. + pub include_permissions_instructions: Option, + + /// Whether to inject the `` developer block. + pub include_apps_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 @@ -2452,6 +2464,14 @@ impl Config { 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); + let include_permissions_instructions = config_profile + .include_permissions_instructions + .or(cfg.include_permissions_instructions) + .unwrap_or(true); + let include_apps_instructions = config_profile + .include_apps_instructions + .or(cfg.include_apps_instructions) + .unwrap_or(true); let guardian_developer_instructions = guardian_developer_instructions_from_requirements( config_layer_stack.requirements_toml(), ); @@ -2618,6 +2638,8 @@ impl Config { developer_instructions, compact_prompt, commit_attribution, + include_permissions_instructions, + include_apps_instructions, // The config.toml omits "_mode" because it's a config file. However, "_mode" // is important in code to differentiate the mode from the store implementation. cli_auth_credentials_store_mode: cfg.cli_auth_credentials_store.unwrap_or_default(), diff --git a/codex-rs/core/src/config/profile.rs b/codex-rs/core/src/config/profile.rs index 7dd8598c0..b430bcc87 100644 --- a/codex-rs/core/src/config/profile.rs +++ b/codex-rs/core/src/config/profile.rs @@ -49,6 +49,8 @@ pub struct ConfigProfile { pub experimental_instructions_file: Option, pub experimental_compact_prompt_file: Option, pub include_apply_patch_tool: Option, + pub include_permissions_instructions: Option, + pub include_apps_instructions: Option, pub experimental_use_unified_exec_tool: Option, pub experimental_use_freeform_apply_patch: Option, pub tools_view_image: Option, diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs index 34fd41c08..f20d8b401 100644 --- a/codex-rs/core/src/context_manager/updates.rs +++ b/codex-rs/core/src/context_manager/updates.rs @@ -33,6 +33,10 @@ fn build_permissions_update_item( next: &TurnContext, exec_policy: &Policy, ) -> Option { + if !next.config.include_permissions_instructions { + return None; + } + let prev = previous?; if prev.sandbox_policy == *next.sandbox_policy.get() && prev.approval_policy == next.approval_policy.value() diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 2cf34b0de..d68c2b299 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -58,6 +58,8 @@ pub mod utils; pub use utils::path_utils; pub mod personality_migration; pub mod plugins; +#[doc(hidden)] +pub mod prompt_debug; pub(crate) mod mentions { pub(crate) use crate::plugins::build_connector_slug_counts; pub(crate) use crate::plugins::build_skill_name_counts; diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs new file mode 100644 index 000000000..4a8513bb4 --- /dev/null +++ b/codex-rs/core/src/prompt_debug.rs @@ -0,0 +1,152 @@ +use std::collections::HashSet; +use std::sync::Arc; + +use codex_exec_server::EnvironmentManager; +use codex_features::Feature; +use codex_login::AuthManager; +use codex_models_manager::collaboration_mode_presets::CollaborationModesConfig; +use codex_protocol::error::Result as CodexResult; +use codex_protocol::models::ResponseInputItem; +use codex_protocol::models::ResponseItem; +use codex_protocol::protocol::SessionSource; +use codex_protocol::user_input::UserInput; +use tokio_util::sync::CancellationToken; + +use crate::codex::Session; +use crate::codex::build_prompt; +use crate::codex::built_tools; +use crate::config::Config; +use crate::thread_manager::ThreadManager; + +/// Build the model-visible `input` list for a single debug turn. +#[doc(hidden)] +pub async fn build_prompt_input( + mut config: Config, + input: Vec, +) -> CodexResult> { + config.ephemeral = true; + + let auth_manager = AuthManager::shared( + config.codex_home.clone(), + /*enable_codex_api_key_env*/ false, + config.cli_auth_credentials_store_mode, + ); + auth_manager.set_forced_chatgpt_workspace_id(config.forced_chatgpt_workspace_id.clone()); + + let thread_manager = ThreadManager::new( + &config, + Arc::clone(&auth_manager), + SessionSource::Exec, + CollaborationModesConfig { + default_mode_request_user_input: config + .features + .enabled(Feature::DefaultModeRequestUserInput), + }, + Arc::new(EnvironmentManager::from_env()), + ); + let thread = thread_manager.start_thread(config).await?; + + let output = build_prompt_input_from_session(thread.thread.codex.session.as_ref(), input).await; + let shutdown = thread.thread.shutdown_and_wait().await; + let _removed = thread_manager.remove_thread(&thread.thread_id).await; + + shutdown?; + output +} + +pub(crate) async fn build_prompt_input_from_session( + sess: &Session, + input: Vec, +) -> CodexResult> { + let turn_context = sess.new_default_turn().await; + sess.record_context_updates_and_set_reference_context_item(turn_context.as_ref()) + .await; + + if !input.is_empty() { + let input_item = ResponseInputItem::from(input); + let response_item = ResponseItem::from(input_item); + sess.record_conversation_items(turn_context.as_ref(), std::slice::from_ref(&response_item)) + .await; + } + + let prompt_input = sess + .clone_history() + .await + .for_prompt(&turn_context.model_info.input_modalities); + let router = built_tools( + sess, + turn_context.as_ref(), + &prompt_input, + &HashSet::new(), + Some(turn_context.turn_skills.outcome.as_ref()), + &CancellationToken::new(), + ) + .await?; + let base_instructions = sess.get_base_instructions().await; + let prompt = build_prompt( + prompt_input, + router.as_ref(), + turn_context.as_ref(), + base_instructions, + ); + + Ok(prompt.get_formatted_input()) +} + +#[cfg(test)] +mod tests { + use codex_protocol::models::ContentItem; + use codex_protocol::models::ResponseItem; + use codex_protocol::user_input::UserInput; + use codex_utils_absolute_path::AbsolutePathBuf; + use pretty_assertions::assert_eq; + + use crate::config::test_config; + + use super::build_prompt_input; + + #[tokio::test] + async fn build_prompt_input_includes_context_and_user_message() { + let codex_home = tempfile::tempdir().expect("create codex home"); + let cwd = tempfile::tempdir().expect("create cwd"); + let mut config = test_config(); + config.codex_home = codex_home.path().to_path_buf(); + config.cwd = AbsolutePathBuf::try_from(cwd.path().to_path_buf()).expect("absolute cwd"); + config.user_instructions = Some("Project-specific test instructions".to_string()); + + let input = build_prompt_input( + config, + vec![UserInput::Text { + text: "hello from debug prompt".to_string(), + text_elements: Vec::new(), + }], + ) + .await + .expect("build prompt input"); + + let expected_user_message = ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "hello from debug prompt".to_string(), + }], + end_turn: None, + phase: None, + }; + assert_eq!(input.last(), Some(&expected_user_message)); + assert!(input.iter().any(|item| { + let ResponseItem::Message { content, .. } = item else { + return false; + }; + + content.iter().any(|content_item| { + let (ContentItem::InputText { text } | ContentItem::OutputText { text }) = + content_item + else { + return false; + }; + text.contains("Project-specific test instructions") + }) + })); + } +} diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 95557c020..58fa098a0 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -47,6 +47,7 @@ use codex_protocol::user_input::UserInput; use core_test_support::PathBufExt; use core_test_support::apps_test_server::AppsTestServer; use core_test_support::load_default_config_for_test; +use core_test_support::responses::ResponsesRequest; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_completed_with_tokens; use core_test_support::responses::ev_message_item_added; @@ -95,6 +96,13 @@ fn message_input_texts(item: &serde_json::Value) -> Vec<&str> { .collect() } +fn message_input_text_contains(request: &ResponsesRequest, role: &str, needle: &str) -> bool { + request + .message_input_texts(role) + .iter() + .any(|text| text.contains(needle)) +} + /// Writes an `auth.json` into the provided `codex_home` with the specified parameters. /// Returns the fake JWT string written to `tokens.id_token`. #[expect(clippy::unwrap_used)] @@ -1208,47 +1216,19 @@ async fn includes_apps_guidance_as_developer_message_for_chatgpt_auth() { wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; let request = resp_mock.single_request(); - let request_body = request.body_json(); - let input = request_body["input"].as_array().expect("input array"); let apps_snippet = "Apps (Connectors) can be explicitly triggered in user messages in the format"; - let has_developer_apps_guidance = input.iter().any(|item| { - item.get("role").and_then(|value| value.as_str()) == Some("developer") - && item - .get("content") - .and_then(|value| value.as_array()) - .is_some_and(|content| { - content.iter().any(|entry| { - entry - .get("text") - .and_then(|value| value.as_str()) - .is_some_and(|text| text.contains(apps_snippet)) - }) - }) - }); assert!( - has_developer_apps_guidance, - "expected apps guidance in a developer message, got {input:#?}" + message_input_text_contains(&request, "developer", apps_snippet), + "expected apps guidance in a developer message, got {:?}", + request.body_json()["input"] ); - let has_user_apps_guidance = input.iter().any(|item| { - item.get("role").and_then(|value| value.as_str()) == Some("user") - && item - .get("content") - .and_then(|value| value.as_array()) - .is_some_and(|content| { - content.iter().any(|entry| { - entry - .get("text") - .and_then(|value| value.as_str()) - .is_some_and(|text| text.contains(apps_snippet)) - }) - }) - }); assert!( - !has_user_apps_guidance, - "did not expect apps guidance in user messages, got {input:#?}" + !message_input_text_contains(&request, "user", apps_snippet), + "did not expect apps guidance in user messages, got {:?}", + request.body_json()["input"] ); } @@ -1296,26 +1276,66 @@ async fn omits_apps_guidance_for_api_key_auth_even_when_feature_enabled() { wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; let request = resp_mock.single_request(); - let request_body = request.body_json(); - let input = request_body["input"].as_array().expect("input array"); let apps_snippet = "Apps (Connectors) can be explicitly triggered in user messages in the format"; - let has_apps_guidance = input.iter().any(|item| { - item.get("content") - .and_then(|value| value.as_array()) - .is_some_and(|content| { - content.iter().any(|entry| { - entry - .get("text") - .and_then(|value| value.as_str()) - .is_some_and(|text| text.contains(apps_snippet)) - }) - }) - }); assert!( - !has_apps_guidance, - "did not expect apps guidance for API key auth, got {input:#?}" + !message_input_text_contains(&request, "developer", apps_snippet) + && !message_input_text_contains(&request, "user", apps_snippet), + "did not expect apps guidance for API key auth, got {:?}", + request.body_json()["input"] + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn omits_apps_guidance_when_configured_off() { + skip_if_no_network!(); + let server = MockServer::start().await; + let apps_server = AppsTestServer::mount(&server) + .await + .expect("mount apps MCP mock"); + let apps_base_url = apps_server.chatgpt_base_url.clone(); + + let resp_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + + let mut builder = test_codex() + .with_auth(create_dummy_codex_auth()) + .with_config(move |config| { + config + .features + .enable(Feature::Apps) + .expect("test config should allow feature update"); + config.chatgpt_base_url = apps_base_url; + config.include_apps_instructions = false; + }); + let codex = builder + .build(&server) + .await + .expect("create new conversation") + .codex; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await + .unwrap(); + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = resp_mock.single_request(); + assert!( + !message_input_text_contains(&request, "developer", ""), + "did not expect apps instructions when include_apps_instructions = false, got {:?}", + request.body_json()["input"] ); } diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index 5945c04f8..d1738db90 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -9,6 +9,7 @@ use codex_protocol::protocol::Op; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use core_test_support::responses::ResponsesRequest; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_once; @@ -21,26 +22,11 @@ use pretty_assertions::assert_eq; use std::collections::HashSet; use tempfile::TempDir; -fn permissions_texts(input: &[serde_json::Value]) -> Vec { - input - .iter() - .filter_map(|item| { - let role = item.get("role")?.as_str()?; - if role != "developer" { - return None; - } - let text = item - .get("content")? - .as_array()? - .first()? - .get("text")? - .as_str()?; - if text.contains("") { - Some(text.to_string()) - } else { - None - } - }) +fn permissions_texts(request: &ResponsesRequest) -> Vec { + request + .message_input_texts("developer") + .into_iter() + .filter(|text| text.contains("")) .collect() } @@ -71,11 +57,7 @@ async fn permissions_message_sent_once_on_start() -> Result<()> { .await?; wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - let request = req.single_request(); - let body = request.body_json(); - let input = body["input"].as_array().expect("input array"); - let permissions = permissions_texts(input); - assert_eq!(permissions.len(), 1); + assert_eq!(permissions_texts(&req.single_request()).len(), 1); Ok(()) } @@ -139,12 +121,8 @@ async fn permissions_message_added_on_override_change() -> Result<()> { .await?; wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - let body1 = req1.single_request().body_json(); - let body2 = req2.single_request().body_json(); - let input1 = body1["input"].as_array().expect("input array"); - let input2 = body2["input"].as_array().expect("input array"); - let permissions_1 = permissions_texts(input1); - let permissions_2 = permissions_texts(input2); + let permissions_1 = permissions_texts(&req1.single_request()); + let permissions_2 = permissions_texts(&req2.single_request()); assert_eq!(permissions_1.len(), 1); assert_eq!(permissions_2.len(), 2); @@ -197,12 +175,8 @@ async fn permissions_message_not_added_when_no_change() -> Result<()> { .await?; wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - let body1 = req1.single_request().body_json(); - let body2 = req2.single_request().body_json(); - let input1 = body1["input"].as_array().expect("input array"); - let input2 = body2["input"].as_array().expect("input array"); - let permissions_1 = permissions_texts(input1); - let permissions_2 = permissions_texts(input2); + let permissions_1 = permissions_texts(&req1.single_request()); + let permissions_2 = permissions_texts(&req2.single_request()); assert_eq!(permissions_1.len(), 1); assert_eq!(permissions_2.len(), 1); @@ -211,6 +185,78 @@ async fn permissions_message_not_added_when_no_change() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn permissions_message_omitted_when_disabled() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let req1 = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let req2 = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]), + ) + .await; + + let mut builder = test_codex().with_config(move |config| { + config.include_permissions_instructions = false; + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + }); + let test = builder.build(&server).await?; + + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "hello 1".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + test.codex + .submit(Op::OverrideTurnContext { + cwd: None, + approval_policy: Some(AskForApproval::Never), + approvals_reviewer: None, + sandbox_policy: None, + windows_sandbox_level: None, + model: None, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + }) + .await?; + + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "hello 2".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + assert_eq!( + permissions_texts(&req1.single_request()), + Vec::::new() + ); + assert_eq!( + permissions_texts(&req2.single_request()), + Vec::::new() + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn resume_replays_permissions_messages() -> Result<()> { skip_if_no_network!(Ok(())); @@ -297,9 +343,7 @@ async fn resume_replays_permissions_messages() -> Result<()> { .await?; wait_for_event(&resumed.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - let body3 = req3.single_request().body_json(); - let input = body3["input"].as_array().expect("input array"); - let permissions = permissions_texts(input); + let permissions = permissions_texts(&req3.single_request()); assert_eq!(permissions.len(), 3); let unique = permissions.into_iter().collect::>(); assert_eq!(unique.len(), 2); @@ -385,9 +429,7 @@ async fn resume_and_fork_append_permissions_messages() -> Result<()> { .await?; wait_for_event(&initial.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - let body2 = req2.single_request().body_json(); - let input2 = body2["input"].as_array().expect("input array"); - let permissions_base = permissions_texts(input2); + let permissions_base = permissions_texts(&req2.single_request()); assert_eq!(permissions_base.len(), 2); builder = builder.with_config(|config| { @@ -406,9 +448,7 @@ async fn resume_and_fork_append_permissions_messages() -> Result<()> { .await?; wait_for_event(&resumed.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - let body3 = req3.single_request().body_json(); - let input3 = body3["input"].as_array().expect("input array"); - let permissions_resume = permissions_texts(input3); + let permissions_resume = permissions_texts(&req3.single_request()); assert_eq!(permissions_resume.len(), permissions_base.len() + 1); assert_eq!( &permissions_resume[..permissions_base.len()], @@ -440,9 +480,7 @@ async fn resume_and_fork_append_permissions_messages() -> Result<()> { .await?; wait_for_event(&forked.thread, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - let body4 = req4.single_request().body_json(); - let input4 = body4["input"].as_array().expect("input array"); - let permissions_fork = permissions_texts(input4); + let permissions_fork = permissions_texts(&req4.single_request()); assert_eq!(permissions_fork.len(), permissions_base.len() + 1); assert_eq!( &permissions_fork[..permissions_base.len()], @@ -494,9 +532,7 @@ async fn permissions_message_includes_writable_roots() -> Result<()> { .await?; wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - let body = req.single_request().body_json(); - let input = body["input"].as_array().expect("input array"); - let permissions = permissions_texts(input); + let permissions = permissions_texts(&req.single_request()); let expected = DeveloperInstructions::from_policy( &sandbox_policy, AskForApproval::OnRequest,