mirror of
https://github.com/openai/codex.git
synced 2026-09-30 17:28:17 +08:00
Save subagent and memory opt-ins through the app server (#43113)
## What changed Route the TUI's subagent and memory enable prompts through server config writes for new threads, leaving the current thread unchanged. Report success, configuration overrides, or failures after the write completes, and clarify the scope in the prompts. Write both `features.memories` and the legacy `features.memory_tool` key for compatibility with older app servers. ## Testing Extend tests to cover writes to the selected server config, preservation of current settings, legacy memory key updates, and managed configuration overrides. Update prompt snapshots and verify that confirmation emits the new save event. GitOrigin-RevId: ec331db675b38102851899a4aa878d6fe8e9606e
This commit is contained in:
@@ -1127,7 +1127,7 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
fn overridden_write_message(write_response: &ConfigWriteResponse) -> &str {
|
||||
pub(super) fn overridden_write_message(write_response: &ConfigWriteResponse) -> &str {
|
||||
write_response
|
||||
.overridden_metadata
|
||||
.as_ref()
|
||||
|
||||
@@ -2408,6 +2408,10 @@ impl App {
|
||||
AppEvent::SaveExperimentalFeatures { thread_id, updates, response_tx } => {
|
||||
self.save_experimental_features(app_server, thread_id, updates, response_tx);
|
||||
}
|
||||
AppEvent::EnableFeatureForNewThreads(feature) => {
|
||||
self.enable_feature_for_new_threads(tui, app_server, feature)
|
||||
.await;
|
||||
}
|
||||
AppEvent::UpdateFeatureFlags { updates } => {
|
||||
self.update_feature_flags(app_server, updates).await;
|
||||
}
|
||||
|
||||
@@ -1,11 +1,57 @@
|
||||
//! Menu-only feature persistence. Configured readback never replaces task state,
|
||||
//! Server-backed feature persistence. Configured readback never replaces task state,
|
||||
//! and an accepted save finishes even if its popup closes or the user navigates.
|
||||
|
||||
use super::config_persistence::overridden_write_message;
|
||||
use super::*;
|
||||
use crate::experimental_features::FeatureWriteResult;
|
||||
use tokio::sync::oneshot;
|
||||
|
||||
impl App {
|
||||
pub(super) async fn enable_feature_for_new_threads(
|
||||
&mut self,
|
||||
tui: &mut tui::Tui,
|
||||
app_server: &AppServerSession,
|
||||
feature: Feature,
|
||||
) {
|
||||
let label = match feature {
|
||||
Feature::Collab => "Subagents",
|
||||
Feature::MemoryTool => "Memories",
|
||||
_ => return,
|
||||
};
|
||||
let mut edits = vec![crate::config_update::build_feature_enabled_edit(
|
||||
feature.key(),
|
||||
/*enabled*/ true,
|
||||
)];
|
||||
if feature == Feature::MemoryTool {
|
||||
// Older app servers still use the legacy key for this feature.
|
||||
edits.push(crate::config_update::build_feature_enabled_edit(
|
||||
"memory_tool",
|
||||
/*enabled*/ true,
|
||||
));
|
||||
}
|
||||
let notice: Box<dyn HistoryCell> = match crate::config_update::write_config_batch(
|
||||
app_server.request_handle(),
|
||||
edits,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(response) if response.status == WriteStatus::Ok => {
|
||||
Box::new(history_cell::new_warning_event(format!(
|
||||
"{label} setting saved on the server for new threads. This thread is unchanged. Project or task settings may override it."
|
||||
)))
|
||||
}
|
||||
Ok(response) => Box::new(history_cell::new_error_event(format!(
|
||||
"{label} setting was saved but is overridden: {}",
|
||||
overridden_write_message(&response)
|
||||
))),
|
||||
Err(err) => Box::new(history_cell::new_error_event(format!(
|
||||
"Failed to save {label} setting: {}",
|
||||
crate::config_update::format_config_error(&err)
|
||||
))),
|
||||
};
|
||||
self.insert_history_cell(tui, notice);
|
||||
}
|
||||
|
||||
pub(super) fn fetch_experimental_features(
|
||||
&self,
|
||||
app_server: &AppServerSession,
|
||||
|
||||
@@ -8,11 +8,14 @@ async fn experimental_features_use_selected_server_profile_and_preserve_task_set
|
||||
{
|
||||
let home = tempfile::tempdir()?;
|
||||
let selected = AbsolutePathBuf::from_absolute_path(home.path().join("work.config.toml"))?;
|
||||
let managed = home.path().join("managed.toml");
|
||||
std::fs::write(home.path().join("config.toml"), "# unselected\n")?;
|
||||
std::fs::write(&selected, "[features]\nnetwork_proxy = true\n")?;
|
||||
std::fs::write(&managed, "")?;
|
||||
let loader = LoaderOverrides {
|
||||
user_config_path: Some(selected.clone()),
|
||||
user_config_profile: Some("work".parse()?),
|
||||
managed_config_path: Some(managed.clone()),
|
||||
ignore_project_config: true,
|
||||
..LoaderOverrides::without_managed_config_for_tests()
|
||||
};
|
||||
@@ -79,6 +82,49 @@ async fn experimental_features_use_selected_server_profile_and_preserve_task_set
|
||||
std::fs::read_to_string(home.path().join("config.toml"))?,
|
||||
"# unselected\n"
|
||||
);
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let notice_text = |app: &App| {
|
||||
app.transcript_cells
|
||||
.last()
|
||||
.unwrap()
|
||||
.display_lines(/*width*/ 120)
|
||||
.iter()
|
||||
.map(ToString::to_string)
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
};
|
||||
app.enable_feature_for_new_threads(&mut tui, &server, Feature::Collab)
|
||||
.await;
|
||||
insta::assert_snapshot!("subagents_enable_notice", notice_text(&app));
|
||||
std::fs::write(
|
||||
&selected,
|
||||
format!(
|
||||
"{}memory_tool = false\n",
|
||||
std::fs::read_to_string(&selected)?
|
||||
),
|
||||
)?;
|
||||
app.enable_feature_for_new_threads(&mut tui, &server, Feature::MemoryTool)
|
||||
.await;
|
||||
insta::assert_snapshot!("memories_enable_notice", notice_text(&app));
|
||||
let saved: toml::Value = toml::from_str(&std::fs::read_to_string(&selected)?)?;
|
||||
assert_eq!(saved["features"]["multi_agent"].as_bool(), Some(true));
|
||||
assert_eq!(saved["features"]["memories"].as_bool(), Some(true));
|
||||
assert_eq!(saved["features"]["memory_tool"].as_bool(), Some(true));
|
||||
assert_eq!(
|
||||
(app.config.clone(), app.chat_widget.config_ref().clone()),
|
||||
before
|
||||
);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(home.path().join("config.toml"))?,
|
||||
"# unselected\n"
|
||||
);
|
||||
std::fs::write(&managed, "[features]\nmulti_agent = false\n")?;
|
||||
app.enable_feature_for_new_threads(&mut tui, &server, Feature::Collab)
|
||||
.await;
|
||||
insta::assert_snapshot!(
|
||||
"subagents_enable_overridden",
|
||||
notice_text(&app).replace(managed.display().to_string().as_str(), "<managed config>")
|
||||
);
|
||||
// A failed save still reports to history after its popup has closed.
|
||||
let (tx, rx) = oneshot::channel();
|
||||
drop(rx);
|
||||
|
||||
@@ -139,7 +139,7 @@ impl App {
|
||||
if let Some(primary_thread_id) = self.primary_thread_id {
|
||||
self.refresh_agent_picker_threads(app_server, primary_thread_id);
|
||||
}
|
||||
self.chat_widget.open_multi_agent_enable_prompt();
|
||||
self.chat_widget.open_feature_enable_prompt(Feature::Collab);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
---
|
||||
source: tui/src/app/experimental_features_tests.rs
|
||||
expression: "app.transcript_cells.last().unwrap().display_lines(120).iter().map(ToString::to_string).collect::<Vec<_>>().join(\"\\n\")"
|
||||
---
|
||||
⚠ Memories setting saved on the server for new threads. This thread is unchanged. Project or task settings may override
|
||||
it.
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
---
|
||||
source: tui/src/app/experimental_features_tests.rs
|
||||
expression: "app.transcript_cells.last().unwrap().display_lines(120).iter().map(ToString::to_string).collect::<Vec<_>>().join(\"\\n\")"
|
||||
---
|
||||
⚠ Subagents setting saved on the server for new threads. This thread is unchanged. Project or task settings may override
|
||||
it.
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/app/experimental_features_tests.rs
|
||||
expression: "app.transcript_cells.last().unwrap().display_lines(120).iter().map(ToString::to_string).collect::<Vec<_>>().join(\"\\n\").replace(managed.display().to_string().as_str(),\n\"<managed config>\")"
|
||||
---
|
||||
■ Subagents setting was saved but is overridden: Overridden by legacy managed_config.toml: <managed config>
|
||||
@@ -2654,7 +2654,7 @@ async fn select_uncached_agent_thread_still_refreshes_liveness() -> Result<()> {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn open_agent_picker_prompts_to_enable_multi_agent_when_disabled() -> Result<()> {
|
||||
async fn open_agent_picker_prompts_when_subagents_disabled() -> Result<()> {
|
||||
let (mut app, mut app_event_rx, _op_rx) = Box::pin(make_test_app_with_channels()).await;
|
||||
let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(
|
||||
app.chat_widget.config_ref(),
|
||||
@@ -2664,24 +2664,8 @@ async fn open_agent_picker_prompts_to_enable_multi_agent_when_disabled() -> Resu
|
||||
let _ = app.config.features.disable(Feature::Collab);
|
||||
|
||||
Box::pin(app.open_agent_picker(&mut app_server)).await;
|
||||
app.chat_widget
|
||||
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
|
||||
assert_matches!(
|
||||
app_event_rx.try_recv(),
|
||||
Ok(AppEvent::UpdateFeatureFlags { updates }) if updates == vec![(Feature::Collab, true)]
|
||||
);
|
||||
let cell = match app_event_rx.try_recv() {
|
||||
Ok(AppEvent::InsertHistoryCell(cell)) => cell,
|
||||
other => panic!("expected InsertHistoryCell event, got {other:?}"),
|
||||
};
|
||||
let rendered = cell
|
||||
.display_lines(/*width*/ 120)
|
||||
.into_iter()
|
||||
.map(|line| line.to_string())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(rendered.contains("Subagents will be enabled in the next session."));
|
||||
assert!(app.chat_widget.has_active_view());
|
||||
assert!(app_event_rx.try_recv().is_err());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -1180,6 +1180,9 @@ pub(crate) enum AppEvent {
|
||||
response_tx: tokio::sync::oneshot::Sender<Result<FeatureWriteResult, String>>,
|
||||
},
|
||||
|
||||
/// Save an enable prompt on the app server without changing the current thread.
|
||||
EnableFeatureForNewThreads(Feature),
|
||||
|
||||
/// Update memory settings and persist them to config.toml.
|
||||
UpdateMemorySettings {
|
||||
use_memories: bool,
|
||||
|
||||
@@ -178,16 +178,9 @@ use tracing::debug;
|
||||
use tracing::warn;
|
||||
|
||||
const DEFAULT_MODEL_DISPLAY_NAME: &str = "loading";
|
||||
const MULTI_AGENT_ENABLE_TITLE: &str = "Enable subagents?";
|
||||
const MULTI_AGENT_ENABLE_YES: &str = "Yes, enable";
|
||||
const MULTI_AGENT_ENABLE_NO: &str = "Not now";
|
||||
const MULTI_AGENT_ENABLE_NOTICE: &str = "Subagents will be enabled in the next session.";
|
||||
const TRUSTED_ACCESS_FOR_CYBER_VERIFICATION_WARNING: &str = "Your conversations have multiple flags for possible cybersecurity risk. Responses may take longer because extra safety checks are on. To get authorized for security work, join the Trusted Access for Cyber program: https://chatgpt.com/cyber";
|
||||
const MEMORIES_DOC_URL: &str = "https://developers.openai.com/codex/memories";
|
||||
const MEMORIES_ENABLE_TITLE: &str = "Enable memories?";
|
||||
const MEMORIES_ENABLE_YES: &str = "Yes, enable";
|
||||
const MEMORIES_ENABLE_NO: &str = "Not now";
|
||||
const MEMORIES_ENABLE_NOTICE: &str = "Memories will be enabled in the next session.";
|
||||
const MEMORIES_DOC_URL: &str = "https://developers.openai.com/codex/memories";
|
||||
const PLAN_MODE_REASONING_SCOPE_TITLE: &str = "Apply reasoning change";
|
||||
const PLAN_MODE_REASONING_SCOPE_PLAN_ONLY: &str = "Apply to Plan mode override";
|
||||
const PLAN_MODE_REASONING_SCOPE_ALL_MODES: &str = "Apply to global default and Plan mode override";
|
||||
@@ -1064,44 +1057,48 @@ impl ChatWidget {
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
pub(crate) fn open_multi_agent_enable_prompt(&mut self) {
|
||||
let items = vec![
|
||||
SelectionItem {
|
||||
name: MULTI_AGENT_ENABLE_YES.to_string(),
|
||||
description: Some(
|
||||
"Save the setting now. You will need a new session to use it.".to_string(),
|
||||
),
|
||||
actions: vec![Box::new(|tx| {
|
||||
tx.send(AppEvent::UpdateFeatureFlags {
|
||||
updates: vec![(Feature::Collab, true)],
|
||||
});
|
||||
tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::new_warning_event(MULTI_AGENT_ENABLE_NOTICE.to_string()),
|
||||
)));
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
SelectionItem {
|
||||
name: MULTI_AGENT_ENABLE_NO.to_string(),
|
||||
description: Some("Keep subagents disabled.".to_string()),
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
];
|
||||
|
||||
pub(crate) fn open_feature_enable_prompt(&mut self, feature: Feature) {
|
||||
let (label, name) = match feature {
|
||||
Feature::Collab => ("Subagents", "subagents"),
|
||||
Feature::MemoryTool => ("Memories", "memories"),
|
||||
_ => return,
|
||||
};
|
||||
self.bottom_pane.show_selection_view(SelectionViewParams {
|
||||
title: Some(MULTI_AGENT_ENABLE_TITLE.to_string()),
|
||||
subtitle: Some("Subagents are currently disabled in your config.".to_string()),
|
||||
title: Some(format!("Enable {name}?")),
|
||||
subtitle: Some(format!("{label} are disabled in this TUI session.")),
|
||||
footer_note: (feature == Feature::MemoryTool).then(|| {
|
||||
Line::from(vec![
|
||||
"Learn more: ".dim(),
|
||||
MEMORIES_DOC_URL.cyan().underlined(),
|
||||
])
|
||||
}),
|
||||
footer_hint: Some(standard_popup_hint_line()),
|
||||
items,
|
||||
items: vec![
|
||||
SelectionItem {
|
||||
name: "Yes, enable".to_string(),
|
||||
description: Some(
|
||||
"Save on the server for new threads. This thread is unchanged.".to_string(),
|
||||
),
|
||||
actions: vec![Box::new(move |tx| {
|
||||
tx.send(AppEvent::EnableFeatureForNewThreads(feature));
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
SelectionItem {
|
||||
name: "Not now".to_string(),
|
||||
description: Some(format!("Keep {name} disabled.")),
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
],
|
||||
..Default::default()
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) fn open_memories_popup(&mut self) {
|
||||
if !self.config.features.enabled(Feature::MemoryTool) {
|
||||
self.open_memories_enable_prompt();
|
||||
self.open_feature_enable_prompt(Feature::MemoryTool);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1114,42 +1111,6 @@ impl ChatWidget {
|
||||
self.bottom_pane.show_view(Box::new(view));
|
||||
}
|
||||
|
||||
pub(crate) fn open_memories_enable_prompt(&mut self) {
|
||||
let items = vec![
|
||||
SelectionItem {
|
||||
name: MEMORIES_ENABLE_YES.to_string(),
|
||||
description: Some(
|
||||
"Save the setting now. You will need a new session to use it.".to_string(),
|
||||
),
|
||||
actions: vec![Box::new(|tx| {
|
||||
tx.send(AppEvent::UpdateFeatureFlags {
|
||||
updates: vec![(Feature::MemoryTool, true)],
|
||||
});
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
SelectionItem {
|
||||
name: MEMORIES_ENABLE_NO.to_string(),
|
||||
description: Some("Keep memories disabled.".to_string()),
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
},
|
||||
];
|
||||
|
||||
self.bottom_pane.show_selection_view(SelectionViewParams {
|
||||
title: Some(MEMORIES_ENABLE_TITLE.to_string()),
|
||||
subtitle: Some("Memories are currently disabled in your config.".to_string()),
|
||||
footer_note: Some(Line::from(vec![
|
||||
"Learn more: ".dim(),
|
||||
MEMORIES_DOC_URL.cyan().underlined(),
|
||||
])),
|
||||
footer_hint: Some(standard_popup_hint_line()),
|
||||
items,
|
||||
..Default::default()
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) fn set_memory_settings(&mut self, use_memories: bool, generate_memories: bool) {
|
||||
self.config.memories.use_memories = use_memories;
|
||||
self.config.memories.generate_memories = generate_memories;
|
||||
|
||||
+4
-3
@@ -1,11 +1,12 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/popups_and_settings.rs
|
||||
expression: popup
|
||||
expression: "render_bottom_popup(&chat, 80)"
|
||||
---
|
||||
Enable memories?
|
||||
Memories are currently disabled in your config.
|
||||
Memories are disabled in this TUI session.
|
||||
|
||||
› 1. Yes, enable Save the setting now. You will need a new session to use it.
|
||||
› 1. Yes, enable Save on the server for new threads. This thread is
|
||||
unchanged.
|
||||
2. Not now Keep memories disabled.
|
||||
|
||||
Learn more: https://developers.openai.com/codex/memories
|
||||
|
||||
+4
-3
@@ -1,11 +1,12 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/popups_and_settings.rs
|
||||
expression: popup
|
||||
expression: "render_bottom_popup(&chat, 80)"
|
||||
---
|
||||
Enable subagents?
|
||||
Subagents are currently disabled in your config.
|
||||
Subagents are disabled in this TUI session.
|
||||
|
||||
› 1. Yes, enable Save the setting now. You will need a new session to use it.
|
||||
› 1. Yes, enable Save on the server for new threads. This thread is
|
||||
unchanged.
|
||||
2. Not now Keep subagents disabled.
|
||||
|
||||
Press enter to confirm or esc to go back
|
||||
|
||||
@@ -3177,61 +3177,33 @@ async fn experimental_popup_loading_snapshot() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_enable_prompt_snapshot() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
chat.open_multi_agent_enable_prompt();
|
||||
|
||||
let popup = render_bottom_popup(&chat, /*width*/ 80);
|
||||
assert_chatwidget_snapshot!("multi_agent_enable_prompt", popup);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_enable_prompt_updates_feature_and_emits_notice() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
chat.open_multi_agent_enable_prompt();
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
|
||||
assert_matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::UpdateFeatureFlags { updates }) if updates == vec![(Feature::Collab, true)]
|
||||
);
|
||||
let cell = match rx.try_recv() {
|
||||
Ok(AppEvent::InsertHistoryCell(cell)) => cell,
|
||||
other => panic!("expected InsertHistoryCell event, got {other:?}"),
|
||||
};
|
||||
let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 120));
|
||||
assert!(rendered.contains("Subagents will be enabled in the next session."));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn memories_enable_prompt_snapshot() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.set_feature_enabled(Feature::MemoryTool, /*enabled*/ false);
|
||||
|
||||
chat.open_memories_popup();
|
||||
|
||||
let popup = render_bottom_popup(&chat, /*width*/ 80);
|
||||
assert_chatwidget_snapshot!("memories_enable_prompt", popup);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn memories_enable_prompt_updates_feature_without_notice() {
|
||||
async fn feature_enable_prompts_snapshot() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.set_feature_enabled(Feature::MemoryTool, /*enabled*/ false);
|
||||
|
||||
chat.open_memories_popup();
|
||||
chat.open_feature_enable_prompt(Feature::Collab);
|
||||
assert_chatwidget_snapshot!(
|
||||
"multi_agent_enable_prompt",
|
||||
render_bottom_popup(&chat, /*width*/ 80)
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
|
||||
assert_matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::UpdateFeatureFlags { updates }) if updates == vec![(Feature::MemoryTool, true)]
|
||||
Ok(AppEvent::EnableFeatureForNewThreads(Feature::Collab))
|
||||
);
|
||||
assert!(
|
||||
rx.try_recv().is_err(),
|
||||
"memory enable prompt should not emit the success notice before persistence succeeds"
|
||||
|
||||
chat.open_memories_popup();
|
||||
assert_chatwidget_snapshot!(
|
||||
"memories_enable_prompt",
|
||||
render_bottom_popup(&chat, /*width*/ 80)
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
assert_matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::EnableFeatureForNewThreads(Feature::MemoryTool))
|
||||
);
|
||||
assert!(rx.try_recv().is_err());
|
||||
assert!(!chat.has_active_view());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user