Fix codex exec --profile handling (#14524)
PR #14005 introduced a regression whereby `codex exec --profile` overrides were dropped when starting or resuming a thread. That causes the thread to miss profile-scoped settings like `model_instructions_file`. This PR preserve the active profile in the thread start/resume config overrides so the app-server rebuild sees the same profile that exec resolved. Fixes #14515
This commit is contained in:
parent
53d5972226
commit
d32820ab07
2 changed files with 79 additions and 0 deletions
|
|
@ -160,6 +160,75 @@ async fn exec_cli_applies_model_instructions_file() {
|
|||
);
|
||||
}
|
||||
|
||||
/// Verify that `codex exec --profile ...` preserves the active profile when it
|
||||
/// starts the in-process app-server thread, so profile-scoped
|
||||
/// `model_instructions_file` is applied to the outbound request.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_cli_profile_applies_model_instructions_file() {
|
||||
skip_if_no_network!();
|
||||
|
||||
let server = MockServer::start().await;
|
||||
let sse = concat!(
|
||||
"data: {\"type\":\"response.created\",\"response\":{}}\n\n",
|
||||
"data: {\"type\":\"response.completed\",\"response\":{\"id\":\"r1\"}}\n\n"
|
||||
);
|
||||
let resp_mock = core_test_support::responses::mount_sse_once(&server, sse.to_string()).await;
|
||||
|
||||
let custom = TempDir::new().unwrap();
|
||||
let marker = "cli-profile-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('\\', "/");
|
||||
|
||||
let provider_override = format!(
|
||||
"model_providers.mock={{ name = \"mock\", base_url = \"{}/v1\", env_key = \"PATH\", wire_api = \"responses\" }}",
|
||||
server.uri()
|
||||
);
|
||||
|
||||
let home = TempDir::new().unwrap();
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
format!("[profiles.default]\nmodel_instructions_file = \"{custom_path_str}\"\n",),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let repo_root = repo_root();
|
||||
let bin = codex_utils_cargo_bin::cargo_bin("codex").unwrap();
|
||||
let mut cmd = AssertCommand::new(bin);
|
||||
cmd.arg("exec")
|
||||
.arg("--skip-git-repo-check")
|
||||
.arg("--profile")
|
||||
.arg("default")
|
||||
.arg("-c")
|
||||
.arg(&provider_override)
|
||||
.arg("-c")
|
||||
.arg("model_provider=\"mock\"")
|
||||
.arg("-C")
|
||||
.arg(&repo_root)
|
||||
.arg("hello?\n");
|
||||
cmd.env("CODEX_HOME", home.path())
|
||||
.env("OPENAI_API_KEY", "dummy")
|
||||
.env("OPENAI_BASE_URL", format!("{}/v1", server.uri()));
|
||||
|
||||
let output = cmd.output().unwrap();
|
||||
println!("Status: {}", output.status);
|
||||
println!("Stdout:\n{}", String::from_utf8_lossy(&output.stdout));
|
||||
println!("Stderr:\n{}", String::from_utf8_lossy(&output.stderr));
|
||||
assert!(output.status.success());
|
||||
|
||||
let request = resp_mock.single_request();
|
||||
let body = request.body_json();
|
||||
let instructions = body
|
||||
.get("instructions")
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or_default()
|
||||
.to_string();
|
||||
assert!(
|
||||
instructions.contains(marker),
|
||||
"instructions did not contain profile marker; got: {instructions}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Tests streaming responses through the CLI using a local SSE fixture file.
|
||||
/// This test:
|
||||
/// 1. Uses a pre-recorded SSE response fixture instead of a live server
|
||||
|
|
|
|||
|
|
@ -77,6 +77,7 @@ use codex_utils_oss::get_default_model_for_oss_provider;
|
|||
use event_processor_with_human_output::EventProcessorWithHumanOutput;
|
||||
use event_processor_with_jsonl_output::EventProcessorWithJsonOutput;
|
||||
use serde_json::Value;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::collections::VecDeque;
|
||||
use std::io::IsTerminal;
|
||||
|
|
@ -914,6 +915,7 @@ fn thread_start_params_from_config(config: &Config) -> ThreadStartParams {
|
|||
cwd: Some(config.cwd.to_string_lossy().to_string()),
|
||||
approval_policy: Some(config.permissions.approval_policy.value().into()),
|
||||
sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get()),
|
||||
config: config_request_overrides_from_config(config),
|
||||
ephemeral: Some(config.ephemeral),
|
||||
..ThreadStartParams::default()
|
||||
}
|
||||
|
|
@ -928,10 +930,18 @@ fn thread_resume_params_from_config(config: &Config, path: Option<PathBuf>) -> T
|
|||
cwd: Some(config.cwd.to_string_lossy().to_string()),
|
||||
approval_policy: Some(config.permissions.approval_policy.value().into()),
|
||||
sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get()),
|
||||
config: config_request_overrides_from_config(config),
|
||||
..ThreadResumeParams::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn config_request_overrides_from_config(config: &Config) -> Option<HashMap<String, Value>> {
|
||||
config
|
||||
.active_profile
|
||||
.as_ref()
|
||||
.map(|profile| HashMap::from([("profile".to_string(), Value::String(profile.clone()))]))
|
||||
}
|
||||
|
||||
async fn send_request_with_response<T>(
|
||||
client: &InProcessAppServerClient,
|
||||
request: ClientRequest,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue