mirror of
https://github.com/openai/codex.git
synced 2026-04-30 03:12:20 +03:00
chore: rm remote models fflag (#11699)
rm `remote_models` feature flag. We see issues like #11527 when a user has `remote_models` disabled, as we always use the default fallback `ModelInfo`. This causes issues with model performance. Builds on #11690, which helps by warning the user when they are using the default fallback. This PR will make that happen much less frequently as an accidental consequence of disabling `remote_models`.
This commit is contained in:
@@ -81,7 +81,6 @@ async fn user_turn_personality_none_does_not_add_update_message() -> anyhow::Res
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5.2-codex")
|
||||
.with_config(|config| {
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
config.features.enable(Feature::Personality);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
@@ -127,7 +126,6 @@ async fn config_personality_some_sets_instructions_template() -> anyhow::Result<
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5.2-codex")
|
||||
.with_config(|config| {
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
config.features.enable(Feature::Personality);
|
||||
config.personality = Some(Personality::Friendly);
|
||||
});
|
||||
@@ -181,7 +179,6 @@ async fn config_personality_none_sends_no_personality() -> anyhow::Result<()> {
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5.2-codex")
|
||||
.with_config(|config| {
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
config.features.enable(Feature::Personality);
|
||||
config.personality = Some(Personality::None);
|
||||
});
|
||||
@@ -242,7 +239,6 @@ async fn default_personality_is_pragmatic_without_config_toml() -> anyhow::Resul
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5.2-codex")
|
||||
.with_config(|config| {
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
config.features.enable(Feature::Personality);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
@@ -290,7 +286,6 @@ async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()>
|
||||
let mut builder = test_codex()
|
||||
.with_model("exp-codex-personality")
|
||||
.with_config(|config| {
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
config.features.enable(Feature::Personality);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
@@ -386,7 +381,6 @@ async fn user_turn_personality_same_value_does_not_add_update_message() -> anyho
|
||||
let mut builder = test_codex()
|
||||
.with_model("exp-codex-personality")
|
||||
.with_config(|config| {
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
config.features.enable(Feature::Personality);
|
||||
config.personality = Some(Personality::Pragmatic);
|
||||
});
|
||||
@@ -494,7 +488,6 @@ async fn user_turn_personality_skips_if_feature_disabled() -> anyhow::Result<()>
|
||||
let mut builder = test_codex()
|
||||
.with_model("exp-codex-personality")
|
||||
.with_config(|config| {
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
config.features.disable(Feature::Personality);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
@@ -570,123 +563,6 @@ async fn user_turn_personality_skips_if_feature_disabled() -> anyhow::Result<()>
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn ignores_remote_personality_if_remote_models_disabled() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = MockServer::builder()
|
||||
.body_print_limit(BodyPrintLimit::Limited(80_000))
|
||||
.start()
|
||||
.await;
|
||||
|
||||
let remote_slug = "gpt-5.2-codex";
|
||||
let remote_personality_message = "Friendly from remote template";
|
||||
let remote_model = ModelInfo {
|
||||
slug: remote_slug.to_string(),
|
||||
display_name: "Remote personality test".to_string(),
|
||||
description: Some("Remote model with personality template".to_string()),
|
||||
default_reasoning_level: Some(ReasoningEffort::Medium),
|
||||
supported_reasoning_levels: vec![ReasoningEffortPreset {
|
||||
effort: ReasoningEffort::Medium,
|
||||
description: ReasoningEffort::Medium.to_string(),
|
||||
}],
|
||||
shell_type: ConfigShellToolType::UnifiedExec,
|
||||
visibility: ModelVisibility::List,
|
||||
supported_in_api: true,
|
||||
priority: 1,
|
||||
upgrade: None,
|
||||
base_instructions: "base instructions".to_string(),
|
||||
model_messages: Some(ModelMessages {
|
||||
instructions_template: Some("Base instructions\n{{ personality }}\n".to_string()),
|
||||
instructions_variables: Some(ModelInstructionsVariables {
|
||||
personality_default: None,
|
||||
personality_friendly: Some(remote_personality_message.to_string()),
|
||||
personality_pragmatic: None,
|
||||
}),
|
||||
}),
|
||||
supports_reasoning_summaries: false,
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
apply_patch_tool_type: None,
|
||||
truncation_policy: TruncationPolicyConfig::bytes(10_000),
|
||||
supports_parallel_tool_calls: false,
|
||||
context_window: Some(128_000),
|
||||
auto_compact_token_limit: None,
|
||||
effective_context_window_percent: 95,
|
||||
experimental_supported_tools: Vec::new(),
|
||||
input_modalities: default_input_modalities(),
|
||||
prefer_websockets: false,
|
||||
used_fallback_model_metadata: false,
|
||||
};
|
||||
|
||||
let _models_mock = mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![remote_model],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let resp_mock = mount_sse_once(&server, sse_completed("resp-1")).await;
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_auth(codex_core::CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(|config| {
|
||||
config.features.disable(Feature::RemoteModels);
|
||||
config.features.enable(Feature::Personality);
|
||||
config.model = Some(remote_slug.to_string());
|
||||
config.personality = Some(Personality::Friendly);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
wait_for_model_available(
|
||||
&test.thread_manager.get_models_manager(),
|
||||
remote_slug,
|
||||
&test.config,
|
||||
)
|
||||
.await;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: test.cwd_path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
model: remote_slug.to_string(),
|
||||
effort: test.config.model_reasoning_effort,
|
||||
summary: ReasoningSummary::Auto,
|
||||
collaboration_mode: None,
|
||||
personality: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
|
||||
|
||||
let request = resp_mock.single_request();
|
||||
let instructions_text = request.instructions_text();
|
||||
|
||||
assert!(
|
||||
instructions_text.contains("You are Codex, a coding agent based on GPT-5. You and the user share the same workspace and collaborate to achieve the user's goals."),
|
||||
"expected instructions to use the template instructions, got: {instructions_text:?}"
|
||||
);
|
||||
assert!(
|
||||
instructions_text.contains(
|
||||
"You optimize for team morale and being a supportive teammate as much as code quality."
|
||||
),
|
||||
"expected instructions to include the local friendly personality template, got: {instructions_text:?}"
|
||||
);
|
||||
assert!(
|
||||
!instructions_text.contains("{{ personality }}"),
|
||||
"expected legacy personality placeholder to be replaced, got: {instructions_text:?}"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -750,19 +626,13 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
||||
let mut builder = test_codex()
|
||||
.with_auth(codex_core::CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
config.features.enable(Feature::Personality);
|
||||
config.model = Some(remote_slug.to_string());
|
||||
config.personality = Some(Personality::Friendly);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
wait_for_model_available(
|
||||
&test.thread_manager.get_models_manager(),
|
||||
remote_slug,
|
||||
&test.config,
|
||||
)
|
||||
.await;
|
||||
wait_for_model_available(&test.thread_manager.get_models_manager(), remote_slug).await;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
@@ -867,18 +737,12 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
let mut builder = test_codex()
|
||||
.with_auth(codex_core::CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::RemoteModels);
|
||||
config.features.enable(Feature::Personality);
|
||||
config.model = Some("gpt-5.2-codex".to_string());
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
wait_for_model_available(
|
||||
&test.thread_manager.get_models_manager(),
|
||||
remote_slug,
|
||||
&test.config,
|
||||
)
|
||||
.await;
|
||||
wait_for_model_available(&test.thread_manager.get_models_manager(), remote_slug).await;
|
||||
|
||||
test.codex
|
||||
.submit(Op::UserTurn {
|
||||
@@ -957,16 +821,10 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn wait_for_model_available(
|
||||
manager: &Arc<ModelsManager>,
|
||||
slug: &str,
|
||||
config: &codex_core::config::Config,
|
||||
) {
|
||||
async fn wait_for_model_available(manager: &Arc<ModelsManager>, slug: &str) {
|
||||
let deadline = Instant::now() + Duration::from_secs(2);
|
||||
loop {
|
||||
let models = manager
|
||||
.list_models(config, RefreshStrategy::OnlineIfUncached)
|
||||
.await;
|
||||
let models = manager.list_models(RefreshStrategy::OnlineIfUncached).await;
|
||||
if models.iter().any(|model| model.model == slug) {
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user