Cleanup experimentalFeature/enablement/set (#26312)

## Why

`experimentalFeature/enablement/set` still allowed several keys that no
longer need to be managed through this API. Keeping those keys also
preserved corresponding special-case logic, including refreshing the
apps list when the `apps` key was enabled.

The endpoint also rejected an entire request when any key was invalid or
unsupported. That makes clients brittle when they send a mix of current
and stale keys, even when the valid entries can still be applied safely.

## What changed

- remove the feature keys that no longer need to be supported by
`experimentalFeature/enablement/set`
- remove the corresponding apps-list refresh path and its auth/config
plumbing
- ignore and warn on invalid or unsupported keys while still applying
valid keys from the same request
- update the app-server documentation and integration coverage for the
reduced key set and partial-acceptance behavior

## Test plan

- `just test -p codex-app-server experimental_feature_enablement_set` (6
passed)
- `just test -p codex-app-server` exercised the changed tests
successfully; unrelated sandbox-dependent and watcher/timing tests
failed locally
This commit is contained in:
Matthew Zeng
2026-06-04 13:35:31 -07:00
committed by GitHub
Unverified
parent d312a53e2a
commit 4a70e0ac1b
5 changed files with 63 additions and 263 deletions
+1 -1
View File
@@ -192,7 +192,7 @@ Example with notification opt-out:
- `modelProvider/capabilities/read` — read provider-level capabilities for the currently configured model provider.
- `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. Pass `threadId` when showing feature state for an existing loaded thread so `enabled` is computed from that thread's refreshed config, including project-local config for the thread's cwd; if omitted, the server uses its default config resolution context. For non-beta flags, `displayName`/`description`/`announcement` are `null`.
- `permissionProfile/list` — beta; list available permission profile ids with optional display `description` text, using cursor pagination. Pass `cwd` when the caller needs project-local `[permissions.<id>]` entries to be included in the current catalog view.
- `experimentalFeature/enablement/set` — patch the in-memory process-wide runtime feature enablement for currently supported feature keys. For each feature, precedence is: cloud requirements > --enable <feature_name> > config.toml > experimentalFeature/enablement/set (new) > code default.
- `experimentalFeature/enablement/set` — patch the in-memory process-wide runtime feature enablement for currently supported feature keys. For each feature, precedence is: cloud requirements > --enable <feature_name> > config.toml > experimentalFeature/enablement/set (new) > code default. Invalid keys will be ignored.
- `environment/add` — experimental; add or replace a named remote environment by `environmentId` and `execServerUrl` for later selection by `thread/start` or `turn/start`; returns `{}` and does not change the default environment.
- `collaborationMode/list` — list available collaboration mode presets (experimental, no pagination). Built-in presets do not select a model; the Plan preset selects medium reasoning effort. This response omits built-in developer instructions; clients should either pass `settings.developer_instructions: null` when setting a mode to use Codex's built-in instructions, or provide their own instructions explicitly.
- `skills/list` — list skills for one or more `cwd` values (optional `forceReload`).
+1 -2
View File
@@ -455,14 +455,13 @@ impl MessageProcessor {
.plugins_manager()
.maybe_start_plugin_startup_tasks_for_config(
&config.plugins_config_input(),
auth_manager.clone(),
auth_manager,
Some(on_effective_plugins_changed),
);
}
let config_processor = ConfigRequestProcessor::new(
outgoing.clone(),
config_manager.clone(),
auth_manager,
thread_manager.clone(),
analytics_events_client,
);
@@ -7,7 +7,6 @@ use crate::error_code::invalid_request;
use crate::outgoing_message::ConnectionRequestId;
use crate::outgoing_message::OutgoingMessageSender;
use codex_analytics::AnalyticsEventsClient;
use codex_app_server_protocol::AppListUpdatedNotification;
use codex_app_server_protocol::ClientResponsePayload;
use codex_app_server_protocol::ComputerUseRequirements;
use codex_app_server_protocol::ConfigBatchWriteParams;
@@ -29,9 +28,7 @@ use codex_app_server_protocol::NetworkDomainPermission;
use codex_app_server_protocol::NetworkRequirements;
use codex_app_server_protocol::NetworkUnixSocketPermission;
use codex_app_server_protocol::SandboxMode;
use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::WindowsSandboxSetupMode;
use codex_chatgpt::connectors;
use codex_config::ConfigRequirementsToml;
use codex_config::HookEventsToml;
use codex_config::HookHandlerConfig as CoreHookHandlerConfig;
@@ -42,7 +39,6 @@ use codex_config::SandboxModeRequirement as CoreSandboxModeRequirement;
use codex_core::ThreadManager;
use codex_features::canonical_feature_for_key;
use codex_features::feature_for_key;
use codex_login::AuthManager;
use codex_model_provider::create_model_provider;
use codex_plugin::PluginId;
use codex_protocol::config_types::WebSearchMode;
@@ -50,21 +46,18 @@ use serde_json::json;
use std::path::PathBuf;
const SUPPORTED_EXPERIMENTAL_FEATURE_ENABLEMENT: &[&str] = &[
"apps",
"auth_elicitation",
"memories",
"mentions_v2",
"plugins",
"remote_control",
"remote_plugin",
"tool_suggest",
"tool_call_mcp_elicitation",
];
#[derive(Clone)]
pub(crate) struct ConfigRequestProcessor {
outgoing: Arc<OutgoingMessageSender>,
config_manager: ConfigManager,
auth_manager: Arc<AuthManager>,
thread_manager: Arc<ThreadManager>,
analytics_events_client: AnalyticsEventsClient,
}
@@ -73,14 +66,12 @@ impl ConfigRequestProcessor {
pub(crate) fn new(
outgoing: Arc<OutgoingMessageSender>,
config_manager: ConfigManager,
auth_manager: Arc<AuthManager>,
thread_manager: Arc<ThreadManager>,
analytics_events_client: AnalyticsEventsClient,
) -> Self {
Self {
outgoing,
config_manager,
auth_manager,
thread_manager,
analytics_events_client,
}
@@ -151,7 +142,6 @@ impl ConfigRequestProcessor {
request_id: ConnectionRequestId,
params: ExperimentalFeatureEnablementSetParams,
) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
let should_refresh_apps_list = params.enablement.get("apps").copied() == Some(true);
let response = self
.handle_config_mutation_result(self.set_experimental_feature_enablement(params).await)
.await?;
@@ -161,10 +151,6 @@ impl ConfigRequestProcessor {
ClientResponsePayload::ExperimentalFeatureEnablementSet(response),
)
.await;
if should_refresh_apps_list {
self.refresh_apps_list_after_experimental_feature_enablement_set()
.await;
}
Ok(None)
}
@@ -195,71 +181,6 @@ impl ConfigRequestProcessor {
Ok(response)
}
async fn refresh_apps_list_after_experimental_feature_enablement_set(&self) {
let config = match self.load_latest_config(/*fallback_cwd*/ None).await {
Ok(config) => config,
Err(error) => {
tracing::warn!(
"failed to load config for apps list refresh after experimental feature enablement: {}",
error.message
);
return;
}
};
let auth = self.auth_manager.auth().await;
if !config.features.apps_enabled_for_auth(
auth.as_ref()
.is_some_and(codex_login::CodexAuth::uses_codex_backend),
) {
return;
}
let outgoing = Arc::clone(&self.outgoing);
let environment_manager = self.thread_manager.environment_manager();
tokio::spawn(async move {
let (all_connectors_result, accessible_connectors_result) = tokio::join!(
connectors::list_all_connectors_with_options(&config, /*force_refetch*/ true),
connectors::list_accessible_connectors_from_mcp_tools_with_environment_manager(
&config,
/*force_refetch*/ true,
Arc::clone(&environment_manager),
),
);
let all_connectors = match all_connectors_result {
Ok(connectors) => connectors,
Err(err) => {
tracing::warn!(
"failed to force-refresh directory apps after experimental feature enablement: {err:#}"
);
return;
}
};
let accessible_connectors = match accessible_connectors_result {
Ok(status) => status.connectors,
Err(err) => {
tracing::warn!(
"failed to force-refresh accessible apps after experimental feature enablement: {err:#}"
);
return;
}
};
let data = connectors::with_app_enabled_state(
connectors::merge_connectors_with_accessible(
all_connectors,
accessible_connectors,
/*all_connectors_loaded*/ true,
),
&config,
);
outgoing
.send_server_notification(ServerNotification::AppListUpdated(
AppListUpdatedNotification { data },
))
.await;
});
}
async fn load_latest_config(
&self,
fallback_cwd: Option<PathBuf>,
@@ -317,28 +238,19 @@ impl ConfigRequestProcessor {
&self,
params: ExperimentalFeatureEnablementSetParams,
) -> Result<ExperimentalFeatureEnablementSetResponse, JSONRPCErrorError> {
let ExperimentalFeatureEnablementSetParams { enablement } = params;
for key in enablement.keys() {
if canonical_feature_for_key(key).is_some() {
if SUPPORTED_EXPERIMENTAL_FEATURE_ENABLEMENT.contains(&key.as_str()) {
continue;
}
return Err(invalid_request(format!(
"unsupported feature enablement `{key}`: currently supported features are {}",
SUPPORTED_EXPERIMENTAL_FEATURE_ENABLEMENT.join(", ")
)));
let ExperimentalFeatureEnablementSetParams { mut enablement } = params;
let mut invalid_keys = Vec::new();
enablement.retain(|key, _| {
let valid = canonical_feature_for_key(key).is_some()
&& SUPPORTED_EXPERIMENTAL_FEATURE_ENABLEMENT.contains(&key.as_str());
if !valid {
invalid_keys.push(key.clone());
}
let message = if let Some(feature) = feature_for_key(key) {
format!(
"invalid feature enablement `{key}`: use canonical feature key `{}`",
feature.key()
)
} else {
format!("invalid feature enablement `{key}`")
};
return Err(invalid_request(message));
valid
});
if !invalid_keys.is_empty() {
let invalid_keys = invalid_keys.join(", ");
tracing::warn!("ignoring invalid experimental feature enablement keys: {invalid_keys}");
}
if enablement.is_empty() {
@@ -1,5 +1,4 @@
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::sync::Arc;
use std::sync::Mutex as StdMutex;
@@ -28,7 +27,6 @@ use codex_app_server_protocol::AppScreenshot;
use codex_app_server_protocol::AppsListParams;
use codex_app_server_protocol::AppsListResponse;
use codex_app_server_protocol::AuthMode;
use codex_app_server_protocol::ExperimentalFeatureEnablementSetParams;
use codex_app_server_protocol::JSONRPCError;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::RequestId;
@@ -1335,108 +1333,6 @@ async fn list_apps_force_refetch_patches_updates_from_cached_snapshots() -> Resu
Ok(())
}
#[tokio::test]
async fn experimental_feature_enablement_set_refreshes_apps_list_when_apps_turn_on() -> Result<()> {
let initial_connectors = vec![AppInfo {
id: "alpha".to_string(),
name: "Alpha".to_string(),
description: Some("Alpha v1".to_string()),
logo_url: None,
logo_url_dark: None,
distribution_channel: None,
branding: None,
app_metadata: None,
labels: None,
install_url: None,
is_accessible: false,
is_enabled: true,
plugin_display_names: Vec::new(),
}];
let (server_url, server_handle, server_control) = start_apps_server_with_delays_and_control(
initial_connectors,
Vec::new(),
Duration::ZERO,
Duration::ZERO,
)
.await?;
let codex_home = TempDir::new()?;
write_connectors_config(codex_home.path(), &server_url)?;
write_chatgpt_auth(
codex_home.path(),
ChatGptAuthFixture::new("chatgpt-token")
.account_id("account-123")
.chatgpt_user_id("user-enable-refresh")
.chatgpt_account_id("account-123"),
AuthCredentialsStoreMode::File,
)?;
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let disable_request = mcp
.send_experimental_feature_enablement_set_request(ExperimentalFeatureEnablementSetParams {
enablement: BTreeMap::from([("apps".to_string(), false)]),
})
.await?;
let _disable_response: JSONRPCResponse = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(disable_request)),
)
.await??;
server_control.set_connectors(vec![AppInfo {
id: "alpha".to_string(),
name: "Alpha".to_string(),
description: Some("Alpha v2".to_string()),
logo_url: None,
logo_url_dark: None,
distribution_channel: None,
branding: None,
app_metadata: None,
labels: None,
install_url: None,
is_accessible: false,
is_enabled: true,
plugin_display_names: Vec::new(),
}]);
server_control.set_tools(vec![connector_tool("alpha", "Alpha App")?]);
let enable_request = mcp
.send_experimental_feature_enablement_set_request(ExperimentalFeatureEnablementSetParams {
enablement: BTreeMap::from([("apps".to_string(), true)]),
})
.await?;
let _enable_response: JSONRPCResponse = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(enable_request)),
)
.await??;
let update = read_app_list_updated_notification(&mut mcp).await?;
assert_eq!(
update.data,
vec![AppInfo {
id: "alpha".to_string(),
name: "Alpha".to_string(),
description: Some("Alpha v2".to_string()),
logo_url: None,
logo_url_dark: None,
distribution_channel: None,
branding: None,
app_metadata: None,
labels: None,
install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()),
is_accessible: true,
is_enabled: true,
plugin_display_names: Vec::new(),
}]
);
server_handle.abort();
Ok(())
}
async fn read_app_list_updated_notification(
mcp: &mut TestAppServer,
) -> Result<AppListUpdatedNotification> {
@@ -267,13 +267,15 @@ async fn experimental_feature_enablement_set_applies_to_global_and_thread_config
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let actual =
set_experimental_feature_enablement(&mut mcp, BTreeMap::from([("apps".to_string(), true)]))
.await?;
let actual = set_experimental_feature_enablement(
&mut mcp,
BTreeMap::from([("auth_elicitation".to_string(), true)]),
)
.await?;
assert_eq!(
actual,
ExperimentalFeatureEnablementSetResponse {
enablement: BTreeMap::from([("apps".to_string(), true)]),
enablement: BTreeMap::from([("auth_elicitation".to_string(), true)]),
}
);
@@ -284,7 +286,7 @@ async fn experimental_feature_enablement_set_applies_to_global_and_thread_config
config
.additional
.get("features")
.and_then(|features| features.get("apps")),
.and_then(|features| features.get("auth_elicitation")),
Some(&json!(true))
);
}
@@ -333,16 +335,18 @@ async fn experimental_feature_enablement_set_only_updates_named_features() -> Re
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
set_experimental_feature_enablement(&mut mcp, BTreeMap::from([("apps".to_string(), true)]))
.await?;
set_experimental_feature_enablement(
&mut mcp,
BTreeMap::from([("mentions_v2".to_string(), true)]),
)
.await?;
let actual = set_experimental_feature_enablement(
&mut mcp,
BTreeMap::from([
("auth_elicitation".to_string(), true),
("memories".to_string(), true),
("plugins".to_string(), true),
("remote_plugin".to_string(), true),
("tool_suggest".to_string(), true),
("tool_call_mcp_elicitation".to_string(), false),
("tool_suggest".to_string(), false),
]),
)
.await?;
@@ -351,11 +355,10 @@ async fn experimental_feature_enablement_set_only_updates_named_features() -> Re
actual,
ExperimentalFeatureEnablementSetResponse {
enablement: BTreeMap::from([
("auth_elicitation".to_string(), true),
("memories".to_string(), true),
("plugins".to_string(), true),
("remote_plugin".to_string(), true),
("tool_suggest".to_string(), true),
("tool_call_mcp_elicitation".to_string(), false),
("tool_suggest".to_string(), false),
]),
}
);
@@ -366,7 +369,14 @@ async fn experimental_feature_enablement_set_only_updates_named_features() -> Re
config
.additional
.get("features")
.and_then(|features| features.get("apps")),
.and_then(|features| features.get("mentions_v2")),
Some(&json!(true))
);
assert_eq!(
config
.additional
.get("features")
.and_then(|features| features.get("auth_elicitation")),
Some(&json!(true))
);
assert_eq!(
@@ -376,13 +386,6 @@ async fn experimental_feature_enablement_set_only_updates_named_features() -> Re
.and_then(|features| features.get("memories")),
Some(&json!(true))
);
assert_eq!(
config
.additional
.get("features")
.and_then(|features| features.get("plugins")),
Some(&json!(true))
);
assert_eq!(
config
.additional
@@ -395,13 +398,6 @@ async fn experimental_feature_enablement_set_only_updates_named_features() -> Re
.additional
.get("features")
.and_then(|features| features.get("tool_suggest")),
Some(&json!(true))
);
assert_eq!(
config
.additional
.get("features")
.and_then(|features| features.get("tool_call_mcp_elicitation")),
Some(&json!(false))
);
@@ -432,8 +428,11 @@ async fn experimental_feature_enablement_set_empty_map_is_no_op() -> Result<()>
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
set_experimental_feature_enablement(&mut mcp, BTreeMap::from([("apps".to_string(), true)]))
.await?;
set_experimental_feature_enablement(
&mut mcp,
BTreeMap::from([("mentions_v2".to_string(), true)]),
)
.await?;
let actual = set_experimental_feature_enablement(&mut mcp, BTreeMap::new()).await?;
assert_eq!(
@@ -449,7 +448,7 @@ async fn experimental_feature_enablement_set_empty_map_is_no_op() -> Result<()>
config
.additional
.get("features")
.and_then(|features| features.get("apps")),
.and_then(|features| features.get("mentions_v2")),
Some(&json!(true))
);
@@ -457,36 +456,30 @@ async fn experimental_feature_enablement_set_empty_map_is_no_op() -> Result<()>
}
#[tokio::test]
async fn experimental_feature_enablement_set_rejects_non_allowlisted_feature() -> Result<()> {
async fn experimental_feature_enablement_set_ignores_invalid_features() -> Result<()> {
let codex_home = TempDir::new()?;
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let request_id = mcp
.send_experimental_feature_enablement_set_request(ExperimentalFeatureEnablementSetParams {
enablement: BTreeMap::from([("personality".to_string(), true)]),
})
.await?;
let JSONRPCError { error, .. } = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
let actual = set_experimental_feature_enablement(
&mut mcp,
BTreeMap::from([
("apps".to_string(), false),
("auth_elicitation".to_string(), true),
("connectors".to_string(), false),
("personality".to_string(), false),
("plugins".to_string(), false),
("tool_call_mcp_elicitation".to_string(), false),
("unknown_feature".to_string(), true),
]),
)
.await??;
.await?;
assert_eq!(error.code, -32600);
assert!(
error
.message
.contains("unsupported feature enablement `personality`"),
"{}",
error.message
);
assert!(
error.message.contains(
"apps, memories, mentions_v2, plugins, remote_control, remote_plugin, tool_suggest, tool_call_mcp_elicitation"
),
"{}",
error.message
assert_eq!(
actual,
ExperimentalFeatureEnablementSetResponse {
enablement: BTreeMap::from([("auth_elicitation".to_string(), true)]),
}
);
Ok(())