mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] allow disabling prompt instruction blocks (#16735)
This PR adds root and profile config switches to omit the generated `<permissions instructions>` and `<apps_instructions>` 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.
This commit is contained in:
committed by
GitHub
Unverified
parent
f263607c60
commit
8d19646861
@@ -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<String>,
|
||||
|
||||
/// Optional image(s) to attach to the user prompt.
|
||||
#[arg(long = "image", short = 'i', value_name = "FILE", value_delimiter = ',', num_args = 1..)]
|
||||
images: Vec<PathBuf>,
|
||||
}
|
||||
|
||||
#[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::<Vec<_>>();
|
||||
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,
|
||||
|
||||
@@ -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 `<apps_instructions>` developer block.",
|
||||
"type": "boolean"
|
||||
},
|
||||
"include_permissions_instructions": {
|
||||
"description": "Whether to inject the `<permissions instructions>` developer block.",
|
||||
"type": "boolean"
|
||||
},
|
||||
"instructions": {
|
||||
"description": "System instructions.",
|
||||
"type": "string"
|
||||
@@ -2627,4 +2641,4 @@
|
||||
},
|
||||
"title": "ConfigToml",
|
||||
"type": "object"
|
||||
}
|
||||
}
|
||||
|
||||
+20
-17
@@ -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,
|
||||
|
||||
@@ -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<()> {
|
||||
|
||||
@@ -280,6 +280,12 @@ pub struct Config {
|
||||
/// Guardian-specific developer instructions override from requirements.toml.
|
||||
pub guardian_developer_instructions: Option<String>,
|
||||
|
||||
/// Whether to inject the `<permissions instructions>` developer block.
|
||||
pub include_permissions_instructions: bool,
|
||||
|
||||
/// Whether to inject the `<apps_instructions>` developer block.
|
||||
pub include_apps_instructions: bool,
|
||||
|
||||
/// Compact prompt override.
|
||||
pub compact_prompt: Option<String>,
|
||||
|
||||
@@ -1183,6 +1189,12 @@ pub struct ConfigToml {
|
||||
#[serde(default)]
|
||||
pub developer_instructions: Option<String>,
|
||||
|
||||
/// Whether to inject the `<permissions instructions>` developer block.
|
||||
pub include_permissions_instructions: Option<bool>,
|
||||
|
||||
/// Whether to inject the `<apps_instructions>` developer block.
|
||||
pub include_apps_instructions: Option<bool>,
|
||||
|
||||
/// 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(),
|
||||
|
||||
@@ -49,6 +49,8 @@ pub struct ConfigProfile {
|
||||
pub experimental_instructions_file: Option<AbsolutePathBuf>,
|
||||
pub experimental_compact_prompt_file: Option<AbsolutePathBuf>,
|
||||
pub include_apply_patch_tool: Option<bool>,
|
||||
pub include_permissions_instructions: Option<bool>,
|
||||
pub include_apps_instructions: Option<bool>,
|
||||
pub experimental_use_unified_exec_tool: Option<bool>,
|
||||
pub experimental_use_freeform_apply_patch: Option<bool>,
|
||||
pub tools_view_image: Option<bool>,
|
||||
|
||||
@@ -33,6 +33,10 @@ fn build_permissions_update_item(
|
||||
next: &TurnContext,
|
||||
exec_policy: &Policy,
|
||||
) -> Option<DeveloperInstructions> {
|
||||
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()
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<UserInput>,
|
||||
) -> CodexResult<Vec<ResponseItem>> {
|
||||
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<UserInput>,
|
||||
) -> CodexResult<Vec<ResponseItem>> {
|
||||
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")
|
||||
})
|
||||
}));
|
||||
}
|
||||
}
|
||||
@@ -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", "<apps_instructions>"),
|
||||
"did not expect apps instructions when include_apps_instructions = false, got {:?}",
|
||||
request.body_json()["input"]
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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<String> {
|
||||
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("<permissions instructions>") {
|
||||
Some(text.to_string())
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
fn permissions_texts(request: &ResponsesRequest) -> Vec<String> {
|
||||
request
|
||||
.message_input_texts("developer")
|
||||
.into_iter()
|
||||
.filter(|text| text.contains("<permissions instructions>"))
|
||||
.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::<String>::new()
|
||||
);
|
||||
assert_eq!(
|
||||
permissions_texts(&req2.single_request()),
|
||||
Vec::<String>::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::<HashSet<String>>();
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user