feat: return an error if unknown enabled/disabled feature (#5817)
This commit is contained in:
@@ -29,6 +29,7 @@ mod mcp_cmd;
|
|||||||
use crate::mcp_cmd::McpCli;
|
use crate::mcp_cmd::McpCli;
|
||||||
use codex_core::config::Config;
|
use codex_core::config::Config;
|
||||||
use codex_core::config::ConfigOverrides;
|
use codex_core::config::ConfigOverrides;
|
||||||
|
use codex_core::features::is_known_feature_key;
|
||||||
|
|
||||||
/// Codex CLI
|
/// Codex CLI
|
||||||
///
|
///
|
||||||
@@ -286,15 +287,25 @@ struct FeatureToggles {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl FeatureToggles {
|
impl FeatureToggles {
|
||||||
fn to_overrides(&self) -> Vec<String> {
|
fn to_overrides(&self) -> anyhow::Result<Vec<String>> {
|
||||||
let mut v = Vec::new();
|
let mut v = Vec::new();
|
||||||
for k in &self.enable {
|
for feature in &self.enable {
|
||||||
v.push(format!("features.{k}=true"));
|
Self::validate_feature(feature)?;
|
||||||
|
v.push(format!("features.{feature}=true"));
|
||||||
}
|
}
|
||||||
for k in &self.disable {
|
for feature in &self.disable {
|
||||||
v.push(format!("features.{k}=false"));
|
Self::validate_feature(feature)?;
|
||||||
|
v.push(format!("features.{feature}=false"));
|
||||||
|
}
|
||||||
|
Ok(v)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn validate_feature(feature: &str) -> anyhow::Result<()> {
|
||||||
|
if is_known_feature_key(feature) {
|
||||||
|
Ok(())
|
||||||
|
} else {
|
||||||
|
anyhow::bail!("Unknown feature flag: {feature}")
|
||||||
}
|
}
|
||||||
v
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -345,9 +356,8 @@ async fn cli_main(codex_linux_sandbox_exe: Option<PathBuf>) -> anyhow::Result<()
|
|||||||
} = MultitoolCli::parse();
|
} = MultitoolCli::parse();
|
||||||
|
|
||||||
// Fold --enable/--disable into config overrides so they flow to all subcommands.
|
// Fold --enable/--disable into config overrides so they flow to all subcommands.
|
||||||
root_config_overrides
|
let toggle_overrides = feature_toggles.to_overrides()?;
|
||||||
.raw_overrides
|
root_config_overrides.raw_overrides.extend(toggle_overrides);
|
||||||
.extend(feature_toggles.to_overrides());
|
|
||||||
|
|
||||||
match subcommand {
|
match subcommand {
|
||||||
None => {
|
None => {
|
||||||
@@ -605,6 +615,7 @@ mod tests {
|
|||||||
use assert_matches::assert_matches;
|
use assert_matches::assert_matches;
|
||||||
use codex_core::protocol::TokenUsage;
|
use codex_core::protocol::TokenUsage;
|
||||||
use codex_protocol::ConversationId;
|
use codex_protocol::ConversationId;
|
||||||
|
use pretty_assertions::assert_eq;
|
||||||
|
|
||||||
fn finalize_from_args(args: &[&str]) -> TuiCli {
|
fn finalize_from_args(args: &[&str]) -> TuiCli {
|
||||||
let cli = MultitoolCli::try_parse_from(args).expect("parse");
|
let cli = MultitoolCli::try_parse_from(args).expect("parse");
|
||||||
@@ -781,4 +792,32 @@ mod tests {
|
|||||||
assert!(!interactive.resume_last);
|
assert!(!interactive.resume_last);
|
||||||
assert_eq!(interactive.resume_session_id, None);
|
assert_eq!(interactive.resume_session_id, None);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn feature_toggles_known_features_generate_overrides() {
|
||||||
|
let toggles = FeatureToggles {
|
||||||
|
enable: vec!["web_search_request".to_string()],
|
||||||
|
disable: vec!["unified_exec".to_string()],
|
||||||
|
};
|
||||||
|
let overrides = toggles.to_overrides().expect("valid features");
|
||||||
|
assert_eq!(
|
||||||
|
overrides,
|
||||||
|
vec![
|
||||||
|
"features.web_search_request=true".to_string(),
|
||||||
|
"features.unified_exec=false".to_string(),
|
||||||
|
]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn feature_toggles_unknown_feature_errors() {
|
||||||
|
let toggles = FeatureToggles {
|
||||||
|
enable: vec!["does_not_exist".to_string()],
|
||||||
|
disable: Vec::new(),
|
||||||
|
};
|
||||||
|
let err = toggles
|
||||||
|
.to_overrides()
|
||||||
|
.expect_err("feature should be rejected");
|
||||||
|
assert_eq!(err.to_string(), "Unknown feature flag: does_not_exist");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -191,6 +191,11 @@ fn feature_for_key(key: &str) -> Option<Feature> {
|
|||||||
legacy::feature_for_key(key)
|
legacy::feature_for_key(key)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns `true` if the provided string matches a known feature toggle key.
|
||||||
|
pub fn is_known_feature_key(key: &str) -> bool {
|
||||||
|
feature_for_key(key).is_some()
|
||||||
|
}
|
||||||
|
|
||||||
/// Deserializable features table for TOML.
|
/// Deserializable features table for TOML.
|
||||||
#[derive(Deserialize, Debug, Clone, Default, PartialEq)]
|
#[derive(Deserialize, Debug, Clone, Default, PartialEq)]
|
||||||
pub struct FeaturesToml {
|
pub struct FeaturesToml {
|
||||||
|
|||||||
Reference in New Issue
Block a user