mirror of
https://github.com/openai/codex.git
synced 2026-02-01 22:47:52 +00:00
chore: deprecate old web search feature flags (#10097)
deprecate all old web search flags and aliases, including: - `[features].web_search_request` and `[features].web_search_cached` - `[tools].web_search` - `[features].web_search` slightly rework `legacy_usages` to enable pointing to non-features from deprecated features; we need to point to `web_search` (not under `[features]`) from things like `[features].web_search_cached` and `[features].web_search_request`. Added integration tests to confirm deprecation notice is shown on explicit enablement and disablement of deprecated flags.
This commit is contained in:
@@ -765,19 +765,13 @@ impl Session {
|
||||
|
||||
let mut post_session_configured_events = Vec::<Event>::new();
|
||||
|
||||
for (alias, feature) in config.features.legacy_feature_usages() {
|
||||
let canonical = feature.key();
|
||||
let summary = format!("`{alias}` is deprecated. Use `[features].{canonical}` instead.");
|
||||
let details = if alias == canonical {
|
||||
None
|
||||
} else {
|
||||
Some(format!(
|
||||
"Enable it with `--enable {canonical}` or `[features].{canonical}` in config.toml. See https://github.com/openai/codex/blob/main/docs/config.md#feature-flags for details."
|
||||
))
|
||||
};
|
||||
for usage in config.features.legacy_feature_usages() {
|
||||
post_session_configured_events.push(Event {
|
||||
id: INITIAL_SUBMIT_ID.to_owned(),
|
||||
msg: EventMsg::DeprecationNotice(DeprecationNoticeEvent { summary, details }),
|
||||
msg: EventMsg::DeprecationNotice(DeprecationNoticeEvent {
|
||||
summary: usage.summary.clone(),
|
||||
details: usage.details.clone(),
|
||||
}),
|
||||
});
|
||||
}
|
||||
if crate::config::uses_deprecated_instructions_file(&config.config_layer_stack) {
|
||||
|
||||
@@ -148,6 +148,8 @@ impl Feature {
|
||||
pub struct LegacyFeatureUsage {
|
||||
pub alias: String,
|
||||
pub feature: Feature,
|
||||
pub summary: String,
|
||||
pub details: Option<String>,
|
||||
}
|
||||
|
||||
/// Holds the effective set of enabled features.
|
||||
@@ -204,9 +206,12 @@ impl Features {
|
||||
}
|
||||
|
||||
pub fn record_legacy_usage_force(&mut self, alias: &str, feature: Feature) {
|
||||
let (summary, details) = legacy_usage_notice(alias, feature);
|
||||
self.legacy_usages.insert(LegacyFeatureUsage {
|
||||
alias: alias.to_string(),
|
||||
feature,
|
||||
summary,
|
||||
details,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -217,10 +222,8 @@ impl Features {
|
||||
self.record_legacy_usage_force(alias, feature);
|
||||
}
|
||||
|
||||
pub fn legacy_feature_usages(&self) -> impl Iterator<Item = (&str, Feature)> + '_ {
|
||||
self.legacy_usages
|
||||
.iter()
|
||||
.map(|usage| (usage.alias.as_str(), usage.feature))
|
||||
pub fn legacy_feature_usages(&self) -> impl Iterator<Item = &LegacyFeatureUsage> + '_ {
|
||||
self.legacy_usages.iter()
|
||||
}
|
||||
|
||||
pub fn emit_metrics(&self, otel: &OtelManager) {
|
||||
@@ -241,6 +244,21 @@ impl Features {
|
||||
/// Apply a table of key -> bool toggles (e.g. from TOML).
|
||||
pub fn apply_map(&mut self, m: &BTreeMap<String, bool>) {
|
||||
for (k, v) in m {
|
||||
match k.as_str() {
|
||||
"web_search_request" => {
|
||||
self.record_legacy_usage_force(
|
||||
"features.web_search_request",
|
||||
Feature::WebSearchRequest,
|
||||
);
|
||||
}
|
||||
"web_search_cached" => {
|
||||
self.record_legacy_usage_force(
|
||||
"features.web_search_cached",
|
||||
Feature::WebSearchCached,
|
||||
);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
match feature_for_key(k) {
|
||||
Some(feat) => {
|
||||
if k != feat.key() {
|
||||
@@ -301,6 +319,42 @@ impl Features {
|
||||
}
|
||||
}
|
||||
|
||||
fn legacy_usage_notice(alias: &str, feature: Feature) -> (String, Option<String>) {
|
||||
let canonical = feature.key();
|
||||
match feature {
|
||||
Feature::WebSearchRequest | Feature::WebSearchCached => {
|
||||
let label = match alias {
|
||||
"web_search" => "[features].web_search",
|
||||
"tools.web_search" => "[tools].web_search",
|
||||
"features.web_search_request" | "web_search_request" => {
|
||||
"[features].web_search_request"
|
||||
}
|
||||
"features.web_search_cached" | "web_search_cached" => {
|
||||
"[features].web_search_cached"
|
||||
}
|
||||
_ => alias,
|
||||
};
|
||||
let summary = format!("`{label}` is deprecated. Use `web_search` instead.");
|
||||
(summary, Some(web_search_details().to_string()))
|
||||
}
|
||||
_ => {
|
||||
let summary = format!("`{alias}` is deprecated. Use `[features].{canonical}` instead.");
|
||||
let details = if alias == canonical {
|
||||
None
|
||||
} else {
|
||||
Some(format!(
|
||||
"Enable it with `--enable {canonical}` or `[features].{canonical}` in config.toml. See https://github.com/openai/codex/blob/main/docs/config.md#feature-flags for details."
|
||||
))
|
||||
};
|
||||
(summary, details)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn web_search_details() -> &'static str {
|
||||
"Set `web_search` to `\"live\"`, `\"cached\"`, or `\"disabled\"` in config.toml."
|
||||
}
|
||||
|
||||
/// Keys accepted in `[features]` tables.
|
||||
fn feature_for_key(key: &str) -> Option<Feature> {
|
||||
for spec in FEATURES {
|
||||
@@ -349,13 +403,13 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
FeatureSpec {
|
||||
id: Feature::WebSearchRequest,
|
||||
key: "web_search_request",
|
||||
stage: Stage::Stable,
|
||||
stage: Stage::Deprecated,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::WebSearchCached,
|
||||
key: "web_search_cached",
|
||||
stage: Stage::UnderDevelopment,
|
||||
stage: Stage::Deprecated,
|
||||
default_enabled: false,
|
||||
},
|
||||
// Experimental program. Rendered in the `/experimental` menu for users.
|
||||
|
||||
@@ -16,6 +16,7 @@ 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 std::collections::BTreeMap;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
@@ -110,3 +111,73 @@ async fn emits_deprecation_notice_for_experimental_instructions_file() -> anyhow
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn emits_deprecation_notice_for_web_search_feature_flags() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
let mut entries = BTreeMap::new();
|
||||
entries.insert("web_search_request".to_string(), true);
|
||||
config.features.apply_map(&entries);
|
||||
});
|
||||
|
||||
let TestCodex { codex, .. } = builder.build(&server).await?;
|
||||
|
||||
let notice = wait_for_event_match(&codex, |event| match event {
|
||||
EventMsg::DeprecationNotice(ev) if ev.summary.contains("[features].web_search_request") => {
|
||||
Some(ev.clone())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let DeprecationNoticeEvent { summary, details } = notice;
|
||||
assert_eq!(
|
||||
summary,
|
||||
"`[features].web_search_request` is deprecated. Use `web_search` instead.".to_string(),
|
||||
);
|
||||
assert_eq!(
|
||||
details.as_deref(),
|
||||
Some("Set `web_search` to `\"live\"`, `\"cached\"`, or `\"disabled\"` in config.toml."),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn emits_deprecation_notice_for_disabled_web_search_feature_flag() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
let mut entries = BTreeMap::new();
|
||||
entries.insert("web_search_request".to_string(), false);
|
||||
config.features.apply_map(&entries);
|
||||
});
|
||||
|
||||
let TestCodex { codex, .. } = builder.build(&server).await?;
|
||||
|
||||
let notice = wait_for_event_match(&codex, |event| match event {
|
||||
EventMsg::DeprecationNotice(ev) if ev.summary.contains("[features].web_search_request") => {
|
||||
Some(ev.clone())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let DeprecationNoticeEvent { summary, details } = notice;
|
||||
assert_eq!(
|
||||
summary,
|
||||
"`[features].web_search_request` is deprecated. Use `web_search` instead.".to_string(),
|
||||
);
|
||||
assert_eq!(
|
||||
details.as_deref(),
|
||||
Some("Set `web_search` to `\"live\"`, `\"cached\"`, or `\"disabled\"` in config.toml."),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user