feat: deprecation warning (#5825)
<img width="955" height="311" alt="Screenshot 2025-10-28 at 14 26 25" src="https://github.com/user-attachments/assets/99729b3d-3bc9-4503-aab3-8dc919220ab4" />
This commit is contained in:
@@ -78,6 +78,7 @@ use crate::protocol::AgentReasoningSectionBreakEvent;
|
||||
use crate::protocol::ApplyPatchApprovalRequestEvent;
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::BackgroundEventEvent;
|
||||
use crate::protocol::DeprecationNoticeEvent;
|
||||
use crate::protocol::ErrorEvent;
|
||||
use crate::protocol::Event;
|
||||
use crate::protocol::EventMsg;
|
||||
@@ -454,7 +455,7 @@ impl Session {
|
||||
};
|
||||
|
||||
// Error messages to dispatch after SessionConfigured is sent.
|
||||
let mut post_session_configured_error_events = Vec::<Event>::new();
|
||||
let mut post_session_configured_events = Vec::<Event>::new();
|
||||
|
||||
// Kick off independent async setup tasks in parallel to reduce startup latency.
|
||||
//
|
||||
@@ -502,7 +503,7 @@ impl Session {
|
||||
Err(e) => {
|
||||
let message = format!("Failed to create MCP connection manager: {e:#}");
|
||||
error!("{message}");
|
||||
post_session_configured_error_events.push(Event {
|
||||
post_session_configured_events.push(Event {
|
||||
id: INITIAL_SUBMIT_ID.to_owned(),
|
||||
msg: EventMsg::Error(ErrorEvent { message }),
|
||||
});
|
||||
@@ -516,7 +517,7 @@ impl Session {
|
||||
let auth_entry = auth_statuses.get(&server_name);
|
||||
let display_message = mcp_init_error_display(&server_name, auth_entry, &err);
|
||||
warn!("MCP client for `{server_name}` failed to start: {err:#}");
|
||||
post_session_configured_error_events.push(Event {
|
||||
post_session_configured_events.push(Event {
|
||||
id: INITIAL_SUBMIT_ID.to_owned(),
|
||||
msg: EventMsg::Error(ErrorEvent {
|
||||
message: display_message,
|
||||
@@ -525,6 +526,22 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
for (alias, feature) in session_configuration.features.legacy_feature_usages() {
|
||||
let canonical = feature.key();
|
||||
let summary = format!("`{alias}` is deprecated. Use `{canonical}` instead.");
|
||||
let details = if alias == canonical {
|
||||
None
|
||||
} else {
|
||||
Some(format!(
|
||||
"You can either enable it using the CLI with `--enable {canonical}` or through the config.toml file with `[features].{canonical}`"
|
||||
))
|
||||
};
|
||||
post_session_configured_events.push(Event {
|
||||
id: INITIAL_SUBMIT_ID.to_owned(),
|
||||
msg: EventMsg::DeprecationNotice(DeprecationNoticeEvent { summary, details }),
|
||||
});
|
||||
}
|
||||
|
||||
let otel_event_manager = OtelEventManager::new(
|
||||
conversation_id,
|
||||
config.model.as_str(),
|
||||
@@ -589,7 +606,7 @@ impl Session {
|
||||
rollout_path,
|
||||
}),
|
||||
})
|
||||
.chain(post_session_configured_error_events.into_iter());
|
||||
.chain(post_session_configured_events.into_iter());
|
||||
for event in events {
|
||||
sess.send_event_raw(event).await;
|
||||
}
|
||||
@@ -1076,9 +1093,6 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
/// Helper that emits a BackgroundEvent with the given message. This keeps
|
||||
/// the call‑sites terse so adding more diagnostics does not clutter the
|
||||
/// core agent logic.
|
||||
pub(crate) async fn notify_background_event(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
|
||||
@@ -66,10 +66,17 @@ impl Feature {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
|
||||
pub struct LegacyFeatureUsage {
|
||||
pub alias: String,
|
||||
pub feature: Feature,
|
||||
}
|
||||
|
||||
/// Holds the effective set of enabled features.
|
||||
#[derive(Debug, Clone, Default, PartialEq)]
|
||||
pub struct Features {
|
||||
enabled: BTreeSet<Feature>,
|
||||
legacy_usages: BTreeSet<LegacyFeatureUsage>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
@@ -101,7 +108,10 @@ impl Features {
|
||||
set.insert(spec.id);
|
||||
}
|
||||
}
|
||||
Self { enabled: set }
|
||||
Self {
|
||||
enabled: set,
|
||||
legacy_usages: BTreeSet::new(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn enabled(&self, f: Feature) -> bool {
|
||||
@@ -116,11 +126,34 @@ impl Features {
|
||||
self.enabled.remove(&f);
|
||||
}
|
||||
|
||||
pub fn record_legacy_usage_force(&mut self, alias: &str, feature: Feature) {
|
||||
self.legacy_usages.insert(LegacyFeatureUsage {
|
||||
alias: alias.to_string(),
|
||||
feature,
|
||||
});
|
||||
}
|
||||
|
||||
pub fn record_legacy_usage(&mut self, alias: &str, feature: Feature) {
|
||||
if alias == feature.key() {
|
||||
return;
|
||||
}
|
||||
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))
|
||||
}
|
||||
|
||||
/// 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 feature_for_key(k) {
|
||||
Some(feat) => {
|
||||
if k != feat.key() {
|
||||
self.record_legacy_usage(k.as_str(), feat);
|
||||
}
|
||||
if *v {
|
||||
self.enable(feat);
|
||||
} else {
|
||||
|
||||
@@ -134,6 +134,7 @@ fn set_if_some(
|
||||
if let Some(enabled) = maybe_value {
|
||||
set_feature(features, feature, enabled);
|
||||
log_alias(alias_key, feature);
|
||||
features.record_legacy_usage_force(alias_key, feature);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -75,6 +75,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool {
|
||||
| EventMsg::PlanUpdate(_)
|
||||
| EventMsg::ShutdownComplete
|
||||
| EventMsg::ViewImageToolCall(_)
|
||||
| EventMsg::DeprecationNotice(_)
|
||||
| EventMsg::ItemStarted(_)
|
||||
| EventMsg::ItemCompleted(_) => false,
|
||||
}
|
||||
|
||||
@@ -97,11 +97,9 @@ impl TestCodexBuilder {
|
||||
let mut config = load_default_config_for_test(home);
|
||||
config.cwd = cwd.path().to_path_buf();
|
||||
config.model_provider = model_provider;
|
||||
config.codex_linux_sandbox_exe = Some(PathBuf::from(
|
||||
assert_cmd::Command::cargo_bin("codex")?
|
||||
.get_program()
|
||||
.to_os_string(),
|
||||
));
|
||||
if let Ok(cmd) = assert_cmd::Command::cargo_bin("codex") {
|
||||
config.codex_linux_sandbox_exe = Some(PathBuf::from(cmd.get_program().to_os_string()));
|
||||
}
|
||||
|
||||
let mut mutators = vec![];
|
||||
swap(&mut self.config_mutators, &mut mutators);
|
||||
|
||||
51
codex-rs/core/tests/suite/deprecation_notice.rs
Normal file
51
codex-rs/core/tests/suite/deprecation_notice.rs
Normal file
@@ -0,0 +1,51 @@
|
||||
#![cfg(not(target_os = "windows"))]
|
||||
|
||||
use anyhow::Ok;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::protocol::DeprecationNoticeEvent;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
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;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn emits_deprecation_notice_for_legacy_feature_flag() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
let mut builder = test_codex().with_config(|config| {
|
||||
config.features.enable(Feature::StreamableShell);
|
||||
config.features.record_legacy_usage_force(
|
||||
"experimental_use_exec_command_tool",
|
||||
Feature::StreamableShell,
|
||||
);
|
||||
config.use_experimental_streamable_shell_tool = true;
|
||||
});
|
||||
|
||||
let TestCodex { codex, .. } = builder.build(&server).await?;
|
||||
|
||||
let notice = wait_for_event_match(&codex, |event| match event {
|
||||
EventMsg::DeprecationNotice(ev) => Some(ev.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let DeprecationNoticeEvent { summary, details } = notice;
|
||||
assert_eq!(
|
||||
summary,
|
||||
"`experimental_use_exec_command_tool` is deprecated. Use `streamable_shell` instead."
|
||||
.to_string(),
|
||||
);
|
||||
assert_eq!(
|
||||
details.as_deref(),
|
||||
Some(
|
||||
"You can either enable it using the CLI with `--enable streamable_shell` or through the config.toml file with `[features].streamable_shell`"
|
||||
),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -10,6 +10,7 @@ mod cli_stream;
|
||||
mod client;
|
||||
mod compact;
|
||||
mod compact_resume_fork;
|
||||
mod deprecation_notice;
|
||||
mod exec;
|
||||
mod fork_conversation;
|
||||
mod grep_files;
|
||||
|
||||
@@ -4,6 +4,7 @@ use codex_core::config::Config;
|
||||
use codex_core::protocol::AgentMessageEvent;
|
||||
use codex_core::protocol::AgentReasoningRawContentEvent;
|
||||
use codex_core::protocol::BackgroundEventEvent;
|
||||
use codex_core::protocol::DeprecationNoticeEvent;
|
||||
use codex_core::protocol::ErrorEvent;
|
||||
use codex_core::protocol::Event;
|
||||
use codex_core::protocol::EventMsg;
|
||||
@@ -160,6 +161,16 @@ impl EventProcessor for EventProcessorWithHumanOutput {
|
||||
let prefix = "ERROR:".style(self.red);
|
||||
ts_msg!(self, "{prefix} {message}");
|
||||
}
|
||||
EventMsg::DeprecationNotice(DeprecationNoticeEvent { summary, details }) => {
|
||||
ts_msg!(
|
||||
self,
|
||||
"{} {summary}",
|
||||
"deprecated:".style(self.magenta).style(self.bold)
|
||||
);
|
||||
if let Some(details) = details {
|
||||
ts_msg!(self, " {}", details.style(self.dimmed));
|
||||
}
|
||||
}
|
||||
EventMsg::BackgroundEvent(BackgroundEventEvent { message }) => {
|
||||
ts_msg!(self, "{}", message.style(self.dimmed));
|
||||
}
|
||||
|
||||
@@ -290,7 +290,8 @@ async fn run_codex_tool_session_inner(
|
||||
| EventMsg::ItemCompleted(_)
|
||||
| EventMsg::UndoStarted(_)
|
||||
| EventMsg::UndoCompleted(_)
|
||||
| EventMsg::ExitedReviewMode(_) => {
|
||||
| EventMsg::ExitedReviewMode(_)
|
||||
| EventMsg::DeprecationNotice(_) => {
|
||||
// For now, we do not do anything extra for these
|
||||
// events. Note that
|
||||
// send(codex_event_to_notification(&event)) above has
|
||||
|
||||
@@ -497,6 +497,10 @@ pub enum EventMsg {
|
||||
|
||||
ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent),
|
||||
|
||||
/// Notification advising the user that something they are using has been
|
||||
/// deprecated and should be phased out.
|
||||
DeprecationNotice(DeprecationNoticeEvent),
|
||||
|
||||
BackgroundEvent(BackgroundEventEvent),
|
||||
|
||||
UndoStarted(UndoStartedEvent),
|
||||
@@ -1156,6 +1160,15 @@ pub struct BackgroundEventEvent {
|
||||
pub message: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct DeprecationNoticeEvent {
|
||||
/// Concise summary of what is deprecated.
|
||||
pub summary: String,
|
||||
/// Optional extra guidance, such as migration steps or rationale.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub details: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct UndoStartedEvent {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
||||
@@ -16,6 +16,7 @@ use codex_core::protocol::AgentReasoningRawContentDeltaEvent;
|
||||
use codex_core::protocol::AgentReasoningRawContentEvent;
|
||||
use codex_core::protocol::ApplyPatchApprovalRequestEvent;
|
||||
use codex_core::protocol::BackgroundEventEvent;
|
||||
use codex_core::protocol::DeprecationNoticeEvent;
|
||||
use codex_core::protocol::ErrorEvent;
|
||||
use codex_core::protocol::Event;
|
||||
use codex_core::protocol::EventMsg;
|
||||
@@ -663,6 +664,12 @@ impl ChatWidget {
|
||||
debug!("TurnDiffEvent: {unified_diff}");
|
||||
}
|
||||
|
||||
fn on_deprecation_notice(&mut self, event: DeprecationNoticeEvent) {
|
||||
let DeprecationNoticeEvent { summary, details } = event;
|
||||
self.add_to_history(history_cell::new_deprecation_notice(summary, details));
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
fn on_background_event(&mut self, message: String) {
|
||||
debug!("BackgroundEvent: {message}");
|
||||
}
|
||||
@@ -1496,6 +1503,7 @@ impl ChatWidget {
|
||||
EventMsg::ListCustomPromptsResponse(ev) => self.on_list_custom_prompts(ev),
|
||||
EventMsg::ShutdownComplete => self.on_shutdown_complete(),
|
||||
EventMsg::TurnDiff(TurnDiffEvent { unified_diff }) => self.on_turn_diff(unified_diff),
|
||||
EventMsg::DeprecationNotice(ev) => self.on_deprecation_notice(ev),
|
||||
EventMsg::BackgroundEvent(BackgroundEventEvent { message }) => {
|
||||
self.on_background_event(message)
|
||||
}
|
||||
|
||||
@@ -1005,6 +1005,38 @@ pub(crate) fn new_warning_event(message: String) -> PlainHistoryCell {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct DeprecationNoticeCell {
|
||||
summary: String,
|
||||
details: Option<String>,
|
||||
}
|
||||
|
||||
pub(crate) fn new_deprecation_notice(
|
||||
summary: String,
|
||||
details: Option<String>,
|
||||
) -> DeprecationNoticeCell {
|
||||
DeprecationNoticeCell { summary, details }
|
||||
}
|
||||
|
||||
impl HistoryCell for DeprecationNoticeCell {
|
||||
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
let mut lines: Vec<Line<'static>> = Vec::new();
|
||||
lines.push(vec!["⚠ ".red().bold(), self.summary.clone().red()].into());
|
||||
|
||||
let wrap_width = width.saturating_sub(4).max(1) as usize;
|
||||
|
||||
if let Some(details) = &self.details {
|
||||
let line = textwrap::wrap(details, wrap_width)
|
||||
.into_iter()
|
||||
.map(|s| s.to_string().dim().into())
|
||||
.collect::<Vec<_>>();
|
||||
lines.extend(line);
|
||||
}
|
||||
|
||||
lines
|
||||
}
|
||||
}
|
||||
|
||||
/// Render a summary of configured MCP servers from the current `Config`.
|
||||
pub(crate) fn empty_mcp_output() -> PlainHistoryCell {
|
||||
let lines: Vec<Line<'static>> = vec![
|
||||
@@ -2259,4 +2291,21 @@ mod tests {
|
||||
let rendered_transcript = render_transcript(cell.as_ref());
|
||||
assert_eq!(rendered_transcript, vec!["• We should fix the bug next."]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deprecation_notice_renders_summary_with_details() {
|
||||
let cell = new_deprecation_notice(
|
||||
"Feature flag `foo`".to_string(),
|
||||
Some("Use flag `bar` instead.".to_string()),
|
||||
);
|
||||
let lines = cell.display_lines(80);
|
||||
let rendered = render_lines(&lines);
|
||||
assert_eq!(
|
||||
rendered,
|
||||
vec![
|
||||
"⚠ Feature flag `foo`".to_string(),
|
||||
"Use flag `bar` instead.".to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user