mirror of
https://github.com/openai/codex.git
synced 2026-10-30 19:18:49 +00:00
Harden TUI subagent navigation (#39088)
## What changed - Use `/subagents` consistently for the subagent picker and status UI, removing the `/agent` alias. - Rejoin already-loaded subagent threads without overriding their existing settings. - Route notifications and approval requests only to the active thread or its known subagents, discard unrelated startup events, and avoid replaying resolved approvals. ## Testing - Add coverage for filtering buffered startup events, dropping resolved approvals, preserving owned subagent approvals, and resuming threads without setting overrides. GitOrigin-RevId: b4e597f7cd4756faccf31d8b964e7a172f5ab0eb
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
//! Multi-agent picker navigation and labeling state for the TUI app.
|
||||
//!
|
||||
//! This module exists to keep the pure parts of multi-agent navigation out of [`crate::app::App`].
|
||||
//! It owns the stable spawn-order cache used by the `/agent` picker, keyboard next/previous
|
||||
//! It owns the stable spawn-order cache used by the `/subagents` picker, keyboard next/previous
|
||||
//! navigation, and the contextual footer label for the thread currently being watched.
|
||||
//!
|
||||
//! Responsibilities here are intentionally narrow:
|
||||
@@ -230,7 +230,7 @@ impl AgentNavigationState {
|
||||
///
|
||||
/// This is reserved for entries that were only discovered opportunistically and never became
|
||||
/// replayable local threads. Keeping those around after the backend confirms they are gone
|
||||
/// would leave ghost rows in `/agent`.
|
||||
/// would leave ghost rows in `/subagents`.
|
||||
pub(crate) fn remove(&mut self, thread_id: ThreadId) {
|
||||
self.threads.remove(&thread_id);
|
||||
self.order.retain(|candidate| *candidate != thread_id);
|
||||
@@ -361,7 +361,7 @@ impl AgentNavigationState {
|
||||
)
|
||||
}
|
||||
|
||||
/// Builds the `/agent` picker subtitle from the same canonical bindings used by key handling.
|
||||
/// Builds the `/subagents` picker subtitle from the same canonical bindings used by key handling.
|
||||
///
|
||||
/// Keeping this text derived from the actual shortcut helpers prevents the picker copy from
|
||||
/// drifting if the bindings ever change on one platform.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Bounded, best-effort previews for the v2 `/agent` status output.
|
||||
//! Bounded, best-effort previews for the v2 `/subagents` status output.
|
||||
|
||||
use super::ThreadBufferedEvent;
|
||||
use super::ThreadEventStore;
|
||||
@@ -32,7 +32,7 @@ impl AgentStatusHistoryCell {
|
||||
impl HistoryCell for AgentStatusHistoryCell {
|
||||
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
let mut lines: Vec<Line<'static>> = vec![
|
||||
"/agent".magenta().into(),
|
||||
"/subagents".magenta().into(),
|
||||
"Sub-agents running".bold().into(),
|
||||
"".into(),
|
||||
];
|
||||
|
||||
@@ -54,7 +54,7 @@ fn agent_status_uses_bounded_buffered_activity() {
|
||||
.join("\n");
|
||||
|
||||
insta::assert_snapshot!(rendered, @r###"
|
||||
/agent
|
||||
/subagents
|
||||
Sub-agents running
|
||||
|
||||
• `/root/reviewer`
|
||||
@@ -102,7 +102,7 @@ fn agent_status_uses_reasoning_summaries_only() {
|
||||
.join("\n");
|
||||
|
||||
insta::assert_snapshot!(rendered, @r###"
|
||||
/agent
|
||||
/subagents
|
||||
Sub-agents running
|
||||
|
||||
• `/root/reviewer`
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
//! App-server event stream handling for the TUI app.
|
||||
|
||||
use super::App;
|
||||
use super::ThreadBufferedEvent;
|
||||
use super::app_server_event_targets::ServerNotificationThreadTarget;
|
||||
use super::app_server_event_targets::server_notification_thread_target;
|
||||
use super::app_server_event_targets::server_request_thread_id;
|
||||
@@ -11,9 +12,15 @@ use crate::app_server_session::AppServerSession;
|
||||
use crate::app_server_session::status_account_display_from_auth_mode;
|
||||
use codex_app_server_client::AppServerEvent;
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::RateLimitReachedType;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::SessionSource;
|
||||
use codex_app_server_protocol::ThreadReadParams;
|
||||
use codex_app_server_protocol::ThreadReadResponse;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
|
||||
impl App {
|
||||
pub(super) fn refresh_mcp_startup_expected_servers_from_config(&mut self) {
|
||||
@@ -81,6 +88,10 @@ impl App {
|
||||
) {
|
||||
match ¬ification {
|
||||
ServerNotification::ServerRequestResolved(notification) => {
|
||||
self.pending_primary_events.retain(|event| {
|
||||
!matches!(event, ThreadBufferedEvent::Request(request)
|
||||
if request.id() == ¬ification.request_id)
|
||||
});
|
||||
if let Some(request) = self
|
||||
.pending_app_server_requests
|
||||
.resolve_notification(¬ification.request_id)
|
||||
@@ -179,6 +190,31 @@ impl App {
|
||||
|
||||
match server_notification_thread_target(¬ification) {
|
||||
ServerNotificationThreadTarget::Thread(thread_id) => {
|
||||
if self.primary_thread_id.is_none() && !self.pending_startup_thread_start {
|
||||
return;
|
||||
}
|
||||
if self.primary_thread_id.is_some()
|
||||
&& self.primary_thread_id != Some(thread_id)
|
||||
&& !self.thread_event_channels.contains_key(&thread_id)
|
||||
&& self.agent_navigation.get(&thread_id).is_none()
|
||||
&& !self.side_threads.contains_key(&thread_id)
|
||||
&& !matches!(¬ification, ServerNotification::McpServerStatusUpdated(_))
|
||||
&& !matches!(
|
||||
¬ification,
|
||||
ServerNotification::ThreadStarted(started)
|
||||
if matches!(
|
||||
&started.thread.source,
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
..
|
||||
}) if self.primary_thread_id == Some(*parent_thread_id)
|
||||
|| self.thread_event_channels.contains_key(parent_thread_id)
|
||||
|| self.agent_navigation.get(parent_thread_id).is_some()
|
||||
)
|
||||
)
|
||||
{
|
||||
return;
|
||||
}
|
||||
let result = if self.primary_thread_id == Some(thread_id)
|
||||
|| self.primary_thread_id.is_none()
|
||||
{
|
||||
@@ -232,6 +268,52 @@ impl App {
|
||||
}
|
||||
return;
|
||||
}
|
||||
if thread_id.is_some()
|
||||
&& self.primary_thread_id.is_none()
|
||||
&& self.pending_startup_thread_start
|
||||
{
|
||||
self.pending_primary_events
|
||||
.push_back(ThreadBufferedEvent::Request(Box::new(request)));
|
||||
return;
|
||||
}
|
||||
if thread_id.is_some()
|
||||
&& self.primary_thread_id.is_none()
|
||||
&& !self.pending_startup_thread_start
|
||||
{
|
||||
return;
|
||||
}
|
||||
if let Some(thread_id) = thread_id
|
||||
&& self.primary_thread_id.is_some()
|
||||
&& self.primary_thread_id != Some(thread_id)
|
||||
&& !self.thread_event_channels.contains_key(&thread_id)
|
||||
&& self.agent_navigation.get(&thread_id).is_none()
|
||||
&& !self.side_threads.contains_key(&thread_id)
|
||||
{
|
||||
let thread = app_server_client
|
||||
.request_handle()
|
||||
.request_typed::<ThreadReadResponse>(ClientRequest::ThreadRead {
|
||||
request_id: RequestId::String(format!("subagent-approval-{thread_id}")),
|
||||
params: ThreadReadParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
include_turns: false,
|
||||
},
|
||||
})
|
||||
.await;
|
||||
if !matches!(
|
||||
thread,
|
||||
Ok(ThreadReadResponse { thread })
|
||||
if matches!(thread.source,
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
..
|
||||
}) if self.primary_thread_id == Some(parent_thread_id)
|
||||
|| self.thread_event_channels.contains_key(&parent_thread_id)
|
||||
|| self.agent_navigation.get(&parent_thread_id).is_some()
|
||||
)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(unsupported) = self
|
||||
.pending_app_server_requests
|
||||
|
||||
@@ -7,6 +7,9 @@
|
||||
use std::io;
|
||||
|
||||
use super::agent_picker::AGENT_PICKER_VIEW_ID;
|
||||
use super::app_server_event_targets::ServerNotificationThreadTarget;
|
||||
use super::app_server_event_targets::server_notification_thread_target;
|
||||
use super::app_server_event_targets::server_request_thread_id;
|
||||
use super::*;
|
||||
use crate::app_server_session::source_agent_path;
|
||||
use crate::app_server_session::thread_blocks_direct_input;
|
||||
@@ -357,8 +360,8 @@ impl App {
|
||||
/// TUI has not cached a local event channel yet.
|
||||
///
|
||||
/// Resume-time backfill intentionally avoids creating empty placeholder channels, because those
|
||||
/// placeholders make stale `/agent` entries open blank transcripts. When a user later selects a
|
||||
/// still-live discovered thread, attach it on demand with a real resumed snapshot.
|
||||
/// placeholders make stale `/subagents` entries open blank transcripts. When a user later
|
||||
/// selects a still-live discovered thread, attach it on demand with a real resumed snapshot.
|
||||
pub(super) async fn attach_live_thread_for_selection(
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
@@ -369,7 +372,11 @@ impl App {
|
||||
}
|
||||
|
||||
let (session, turns, live_attached) = match app_server
|
||||
.resume_thread(self.config.clone(), thread_id, self.resume_model_settings())
|
||||
.resume_thread(
|
||||
self.config.clone(),
|
||||
thread_id,
|
||||
crate::app_server_session::ResumeModelSettings::PreserveExistingThread,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(started) => {
|
||||
@@ -628,8 +635,48 @@ impl App {
|
||||
.set_queue_submissions_until_session_configured(/*queue*/ false);
|
||||
match result {
|
||||
Ok(started) => {
|
||||
let thread_id = started.session.thread_id;
|
||||
self.pending_primary_events.retain(|event| match event {
|
||||
ThreadBufferedEvent::Notification(notification) => matches!(
|
||||
server_notification_thread_target(notification),
|
||||
ServerNotificationThreadTarget::Thread(event_thread_id)
|
||||
if event_thread_id == thread_id
|
||||
),
|
||||
ThreadBufferedEvent::Request(request) => {
|
||||
server_request_thread_id(request) == Some(thread_id)
|
||||
}
|
||||
ThreadBufferedEvent::HistoryEntryResponse(_)
|
||||
| ThreadBufferedEvent::FeedbackSubmission(_) => true,
|
||||
});
|
||||
self.pending_app_server_requests.clear();
|
||||
let mut unsupported_requests = Vec::new();
|
||||
for event in &self.pending_primary_events {
|
||||
if let ThreadBufferedEvent::Request(request) = event
|
||||
&& let Some(unsupported) = self
|
||||
.pending_app_server_requests
|
||||
.note_server_request(request)
|
||||
{
|
||||
unsupported_requests.push(unsupported);
|
||||
}
|
||||
}
|
||||
for unsupported in unsupported_requests {
|
||||
self.pending_primary_events.retain(|event| {
|
||||
!matches!(event, ThreadBufferedEvent::Request(request)
|
||||
if request.id() == &unsupported.request_id)
|
||||
});
|
||||
if let Err(error) = self
|
||||
.reject_app_server_request(
|
||||
app_server,
|
||||
unsupported.request_id,
|
||||
unsupported.message,
|
||||
)
|
||||
.await
|
||||
{
|
||||
tracing::warn!("{error}");
|
||||
}
|
||||
}
|
||||
if started.blocks_direct_input {
|
||||
self.mark_primary_thread_parent_owned(started.session.thread_id);
|
||||
self.mark_primary_thread_parent_owned(thread_id);
|
||||
}
|
||||
self.enqueue_primary_thread_session(started.session, started.turns)
|
||||
.await?;
|
||||
@@ -773,7 +820,7 @@ impl App {
|
||||
/// Fetches all loaded threads from the app server and registers descendants of the primary
|
||||
/// thread in the navigation cache and chat widget metadata.
|
||||
///
|
||||
/// Called when opening the `/agent` picker and after resuming a thread so that the picker and
|
||||
/// Called when opening the `/subagents` picker and after resuming a thread so that the picker and
|
||||
/// keyboard navigation are pre-populated even if the TUI did not witness the original spawn
|
||||
/// events. Fresh and forked threads cannot have pre-existing descendants.
|
||||
///
|
||||
|
||||
@@ -1723,13 +1723,13 @@ async fn open_agent_picker_preserves_running_hints_until_observed_completion() -
|
||||
let event = app_event_rx.try_recv().expect("agent status history cell");
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 80));
|
||||
if rendered.contains("/agent") {
|
||||
if rendered.contains("/subagents") {
|
||||
break rendered;
|
||||
}
|
||||
}
|
||||
};
|
||||
assert_snapshot!(status, @r###"
|
||||
/agent
|
||||
/subagents
|
||||
Sub-agents running
|
||||
|
||||
• `/root/child`
|
||||
|
||||
@@ -3,6 +3,7 @@ use crate::app_event_sender::AppEventSender;
|
||||
use crate::bottom_pane::BottomPane;
|
||||
use crate::bottom_pane::BottomPaneParams;
|
||||
use crate::tui::FrameRequester;
|
||||
use app_test_support::create_fake_parented_rollout_with_source;
|
||||
use codex_app_server_protocol::ToolRequestUserInputOption;
|
||||
use codex_app_server_protocol::ToolRequestUserInputQuestion;
|
||||
use crossterm::event::KeyCode;
|
||||
@@ -844,6 +845,195 @@ async fn startup_thread_started_submits_queued_startup_input() {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_thread_started_discards_another_threads_buffered_events() {
|
||||
let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
app.pending_startup_thread_start = true;
|
||||
let other_thread_id = ThreadId::new();
|
||||
app.enqueue_primary_thread_notification(ServerNotification::McpServerStatusUpdated(
|
||||
McpServerStatusUpdatedNotification {
|
||||
thread_id: Some(other_thread_id.to_string()),
|
||||
name: "other".to_string(),
|
||||
status: McpServerStartupState::Starting,
|
||||
error: None,
|
||||
failure_reason: None,
|
||||
},
|
||||
))
|
||||
.await
|
||||
.expect("foreign notification should be buffered");
|
||||
|
||||
let request = ServerRequest::CommandExecutionRequestApproval {
|
||||
request_id: AppServerRequestId::Integer(1),
|
||||
params: CommandExecutionRequestApprovalParams {
|
||||
thread_id: other_thread_id.to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
item_id: "item-1".to_string(),
|
||||
started_at_ms: 0,
|
||||
approval_id: None,
|
||||
environment_id: None,
|
||||
reason: None,
|
||||
network_approval_context: None,
|
||||
command: None,
|
||||
cwd: None,
|
||||
command_actions: None,
|
||||
additional_permissions: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
proposed_network_policy_amendments: None,
|
||||
available_decisions: None,
|
||||
},
|
||||
};
|
||||
app.pending_app_server_requests
|
||||
.note_server_request(&request);
|
||||
app.enqueue_primary_thread_request(request)
|
||||
.await
|
||||
.expect("foreign request should be buffered");
|
||||
|
||||
let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(
|
||||
app.chat_widget.config_ref(),
|
||||
))
|
||||
.await
|
||||
.expect("embedded app server");
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
codex_app_server_client::AppServerEvent::ServerRequest(Box::new(
|
||||
ServerRequest::CurrentTimeRead {
|
||||
request_id: AppServerRequestId::Integer(2),
|
||||
params: codex_app_server_protocol::CurrentTimeReadParams {
|
||||
thread_id: other_thread_id.to_string(),
|
||||
},
|
||||
},
|
||||
)),
|
||||
)
|
||||
.await;
|
||||
assert!(app.pending_primary_events.iter().any(|event| {
|
||||
matches!(event, ThreadBufferedEvent::Request(request)
|
||||
if matches!(request.as_ref(), ServerRequest::CurrentTimeRead { .. }))
|
||||
}));
|
||||
let thread_id = ThreadId::new();
|
||||
app.handle_startup_thread_started(
|
||||
&mut app_server,
|
||||
Ok(AppServerStartedThread {
|
||||
session: test_thread_session(thread_id, test_path_buf("/tmp/project")),
|
||||
turns: Vec::new(),
|
||||
blocks_direct_input: false,
|
||||
}),
|
||||
)
|
||||
.await
|
||||
.expect("startup thread should attach");
|
||||
|
||||
assert!(app.pending_primary_events.is_empty());
|
||||
assert!(
|
||||
app.active_thread_rx
|
||||
.as_mut()
|
||||
.is_some_and(|rx| rx.try_recv().is_err())
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_thread_started_does_not_replay_resolved_approval() -> Result<()> {
|
||||
let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
app.pending_startup_thread_start = true;
|
||||
let app_server =
|
||||
crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()).await?;
|
||||
let thread_id = ThreadId::new();
|
||||
let request = exec_approval_request(thread_id, "turn-1", "item-1", /*approval_id*/ None);
|
||||
let request_id = request.id().clone();
|
||||
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
codex_app_server_client::AppServerEvent::ServerRequest(Box::new(request.clone())),
|
||||
)
|
||||
.await;
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
codex_app_server_client::AppServerEvent::ServerNotification(Box::new(
|
||||
ServerNotification::ServerRequestResolved(
|
||||
codex_app_server_protocol::ServerRequestResolvedNotification {
|
||||
thread_id: thread_id.to_string(),
|
||||
request_id,
|
||||
},
|
||||
),
|
||||
)),
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut app_server = app_server;
|
||||
app.handle_startup_thread_started(
|
||||
&mut app_server,
|
||||
Ok(AppServerStartedThread {
|
||||
session: test_thread_session(thread_id, test_path_buf("/tmp/project")),
|
||||
turns: Vec::new(),
|
||||
blocks_direct_input: false,
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
!app.pending_app_server_requests
|
||||
.contains_server_request(&request)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn owned_subagent_approval_before_thread_started_is_preserved() -> Result<()> {
|
||||
let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
let codex_home = tempdir()?;
|
||||
app.config.codex_home = codex_home.path().to_path_buf().abs();
|
||||
app.config.sqlite = codex_state::SqliteConfig::new_for_testing(codex_home.path().abs());
|
||||
let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?;
|
||||
let parent = app_server.start_thread(&app.config).await?;
|
||||
let parent_thread_id = parent.session.thread_id;
|
||||
app.enqueue_primary_thread_session(parent.session, parent.turns)
|
||||
.await?;
|
||||
let child_thread_id = ThreadId::from_string(
|
||||
&create_fake_parented_rollout_with_source(
|
||||
codex_home.path(),
|
||||
"2026-01-01T00-00-01",
|
||||
"2026-01-01T00:00:01Z",
|
||||
"child task",
|
||||
Some(app.config.model_provider_id.as_str()),
|
||||
/*git_info*/ None,
|
||||
RolloutSessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth: 1,
|
||||
agent_path: None,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
}),
|
||||
parent_thread_id.into(),
|
||||
parent_thread_id,
|
||||
)
|
||||
.expect("create child rollout"),
|
||||
)?;
|
||||
app_server
|
||||
.resume_thread(
|
||||
app.config.clone(),
|
||||
child_thread_id,
|
||||
crate::app_server_session::ResumeModelSettings::RestoreFromThread,
|
||||
)
|
||||
.await?;
|
||||
let request = exec_approval_request(
|
||||
child_thread_id,
|
||||
"turn-1",
|
||||
"item-1",
|
||||
/*approval_id*/ None,
|
||||
);
|
||||
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
codex_app_server_client::AppServerEvent::ServerRequest(Box::new(request.clone())),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
app.pending_app_server_requests
|
||||
.contains_server_request(&request)
|
||||
);
|
||||
assert!(app.thread_event_channels.contains_key(&child_thread_id));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_thread_start_failure_returns_error() {
|
||||
let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
|
||||
@@ -1355,7 +1355,11 @@ impl App {
|
||||
}
|
||||
|
||||
match app_server
|
||||
.resume_thread(self.config.clone(), thread_id, self.resume_model_settings())
|
||||
.resume_thread(
|
||||
self.config.clone(),
|
||||
thread_id,
|
||||
crate::app_server_session::ResumeModelSettings::PreserveExistingThread,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(started) => {
|
||||
@@ -1407,7 +1411,7 @@ impl App {
|
||||
.retain(ThreadEventStore::event_survives_session_refresh);
|
||||
}
|
||||
|
||||
/// Opens the `/agent` picker after refreshing cached labels for known threads.
|
||||
/// Opens the `/subagents` picker after refreshing cached labels for known threads.
|
||||
///
|
||||
/// The picker state is derived from long-lived thread channels plus best-effort metadata
|
||||
/// refreshes from the backend. Refresh failures are treated as "thread is only inspectable by
|
||||
|
||||
@@ -288,6 +288,8 @@ pub(crate) enum ResumeModelSettings {
|
||||
OverrideFromCurrentConfig,
|
||||
/// Omits those overrides so app-server restores the settings saved with the thread.
|
||||
RestoreFromThread,
|
||||
/// Rejoins a loaded thread without changing any of its existing settings.
|
||||
PreserveExistingThread,
|
||||
}
|
||||
|
||||
impl ThreadParamsMode {
|
||||
@@ -1690,6 +1692,12 @@ fn thread_resume_params_from_config(
|
||||
remote_cwd_override: Option<&std::path::Path>,
|
||||
model_settings: ResumeModelSettings,
|
||||
) -> ThreadResumeParams {
|
||||
if model_settings == ResumeModelSettings::PreserveExistingThread {
|
||||
return ThreadResumeParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
..ThreadResumeParams::default()
|
||||
};
|
||||
}
|
||||
let permissions = permissions_selection_from_config(&config, thread_params_mode);
|
||||
let sandbox = permissions
|
||||
.is_none()
|
||||
@@ -1714,7 +1722,9 @@ fn thread_resume_params_from_config(
|
||||
config.model.clone(),
|
||||
thread_params_mode.model_provider_from_config(&config),
|
||||
),
|
||||
ResumeModelSettings::RestoreFromThread => (None, None),
|
||||
ResumeModelSettings::RestoreFromThread | ResumeModelSettings::PreserveExistingThread => {
|
||||
(None, None)
|
||||
}
|
||||
};
|
||||
ThreadResumeParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
@@ -2769,6 +2779,29 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_resume_params_can_rejoin_without_overriding_existing_settings() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
let config = build_config(&temp_dir).await;
|
||||
let thread_id = ThreadId::new();
|
||||
|
||||
let params = thread_resume_params_from_config(
|
||||
config,
|
||||
thread_id,
|
||||
ThreadParamsMode::Embedded,
|
||||
/*remote_cwd_override*/ None,
|
||||
ResumeModelSettings::PreserveExistingThread,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
params,
|
||||
ThreadResumeParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
..ThreadResumeParams::default()
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn persisted_resume_does_not_forward_implicit_service_tier() -> Result<()> {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
@@ -52,7 +52,10 @@ impl AppServerSession {
|
||||
thread_id: ThreadId,
|
||||
model_settings: ResumeModelSettings,
|
||||
) -> Result<AppServerStartedThread> {
|
||||
let session_config = if model_settings == ResumeModelSettings::RestoreFromThread {
|
||||
let session_config = if matches!(
|
||||
model_settings,
|
||||
ResumeModelSettings::RestoreFromThread | ResumeModelSettings::PreserveExistingThread
|
||||
) {
|
||||
config.clone()
|
||||
} else {
|
||||
self.session_config_with_effective_service_tier(&config)
|
||||
|
||||
@@ -384,7 +384,6 @@ fn parent_owned_command_is_allowed(command: SlashCommand, args: &str) -> bool {
|
||||
| SlashCommand::App
|
||||
| SlashCommand::Side
|
||||
| SlashCommand::Btw
|
||||
| SlashCommand::Agent
|
||||
| SlashCommand::MultiAgents
|
||||
| SlashCommand::Vim
|
||||
| SlashCommand::Keymap
|
||||
@@ -4977,7 +4976,7 @@ mod tests {
|
||||
#[test]
|
||||
fn parent_owned_thread_allows_bare_navigation_commands() {
|
||||
for (command, expected) in [
|
||||
("/agent", SlashCommand::Agent),
|
||||
("/subagents", SlashCommand::MultiAgents),
|
||||
("/side", SlashCommand::Side),
|
||||
("/btw", SlashCommand::Btw),
|
||||
("/diff ", SlashCommand::Diff),
|
||||
@@ -4997,13 +4996,13 @@ mod tests {
|
||||
fn parent_owned_thread_allows_safe_command_selected_from_prefix() {
|
||||
let (mut composer, _rx) = new_test_composer();
|
||||
composer.set_parent_owned_thread();
|
||||
type_chars_humanlike(&mut composer, &['/', 'a', 'g']);
|
||||
type_chars_humanlike(&mut composer, &['/', 's', 'u', 'b']);
|
||||
|
||||
let result = composer
|
||||
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE))
|
||||
.0;
|
||||
|
||||
assert_eq!(result, InputResult::Command(SlashCommand::Agent));
|
||||
assert_eq!(result, InputResult::Command(SlashCommand::MultiAgents));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1383,7 +1383,7 @@ impl BottomPane {
|
||||
.lines()
|
||||
.next()
|
||||
.and_then(parse_slash_name)
|
||||
.is_some_and(|(name, _, _)| name == "agent");
|
||||
.is_some_and(|(name, _, _)| name == "subagents");
|
||||
|
||||
self.keymap.chat.interrupt_turn.is_pressed(key_event)
|
||||
&& self.is_task_running
|
||||
@@ -2783,12 +2783,12 @@ mod tests {
|
||||
|
||||
pane.set_task_running(/*running*/ true);
|
||||
|
||||
// Repro: `/agent ` hides the popup (cursor past command name). Esc should
|
||||
// Repro: `/subagents ` hides the popup (cursor past command name). Esc should
|
||||
// keep editing command text instead of interrupting the running task.
|
||||
pane.insert_str("/agent ");
|
||||
pane.insert_str("/subagents ");
|
||||
assert!(
|
||||
!pane.composer.popup_active(),
|
||||
"expected command popup to be hidden after entering `/agent `"
|
||||
"expected command popup to be hidden after entering `/subagents `"
|
||||
);
|
||||
|
||||
pane.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
@@ -2796,10 +2796,10 @@ mod tests {
|
||||
while let Ok(ev) = rx.try_recv() {
|
||||
assert!(
|
||||
!matches!(ev, AppEvent::CodexOp(Op::Interrupt)),
|
||||
"expected Esc to not send Op::Interrupt while typing `/agent`"
|
||||
"expected Esc to not send Op::Interrupt while typing `/subagents`"
|
||||
);
|
||||
}
|
||||
assert_eq!(pane.composer_text(), "/agent ");
|
||||
assert_eq!(pane.composer_text(), "/subagents ");
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -2884,7 +2884,7 @@ mod tests {
|
||||
keymap.chat.interrupt_turn = vec![crate::key_hint::plain(KeyCode::F(12))];
|
||||
pane.set_keymap_bindings(&keymap);
|
||||
pane.set_task_running(/*running*/ true);
|
||||
pane.insert_str("/agent ");
|
||||
pane.insert_str("/subagents ");
|
||||
|
||||
pane.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
assert!(
|
||||
@@ -2895,7 +2895,7 @@ mod tests {
|
||||
pane.handle_key_event(KeyEvent::new(KeyCode::F(12), KeyModifiers::NONE));
|
||||
assert!(
|
||||
matches!(rx.try_recv(), Ok(AppEvent::CodexOp(Op::Interrupt))),
|
||||
"expected configured key to interrupt while `/agent` is being edited"
|
||||
"expected configured key to interrupt while `/subagents` is being edited"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -60,7 +60,7 @@ impl PendingThreadApprovals {
|
||||
lines.push(
|
||||
Line::from(vec![
|
||||
" ".into(),
|
||||
"/agent".cyan().bold(),
|
||||
"/subagents".cyan().bold(),
|
||||
" to switch threads".dim(),
|
||||
])
|
||||
.dim(),
|
||||
@@ -120,7 +120,7 @@ mod tests {
|
||||
snapshot_rows(&widget, /*width*/ 40).replace(' ', "."),
|
||||
@r"
|
||||
..!.Approval.needed.in.Robie.[explorer].
|
||||
..../agent.to.switch.threads............
|
||||
..../subagents.to.switch.threads........
|
||||
"
|
||||
);
|
||||
}
|
||||
@@ -142,7 +142,7 @@ mod tests {
|
||||
..!.Approval.needed.in.Robie.[explorer].....
|
||||
..!.Approval.needed.in.Inspector............
|
||||
............................................
|
||||
..../agent.to.switch.threads................
|
||||
..../subagents.to.switch.threads............
|
||||
"
|
||||
);
|
||||
}
|
||||
|
||||
+1
-2
@@ -23,7 +23,6 @@ expression: commands
|
||||
/app - continue this session in the Desktop app
|
||||
/init - create an AGENTS.md file with instructions for Codex
|
||||
/compact - summarize conversation to prevent hitting the context limit
|
||||
/agent - switch the active agent thread
|
||||
/side - start a side conversation in an ephemeral fork
|
||||
/copy - copy last response as markdown
|
||||
/export - export the conversation as markdown
|
||||
@@ -46,4 +45,4 @@ expression: commands
|
||||
/stop - stop all background terminals
|
||||
/clear - clear the terminal and start a new chat
|
||||
/test-approval - test approval request
|
||||
/subagents - switch the active agent thread
|
||||
/subagents - switch between this session's subagents
|
||||
|
||||
@@ -313,7 +313,7 @@ impl ChatWidget {
|
||||
SlashCommand::Side | SlashCommand::Btw => {
|
||||
self.request_empty_side_conversation(cmd);
|
||||
}
|
||||
SlashCommand::Agent | SlashCommand::MultiAgents => {
|
||||
SlashCommand::MultiAgents => {
|
||||
self.app_event_tx.send(AppEvent::OpenAgentPicker);
|
||||
}
|
||||
SlashCommand::Permissions => {
|
||||
@@ -1139,7 +1139,6 @@ impl ChatWidget {
|
||||
| SlashCommand::Side
|
||||
| SlashCommand::Btw
|
||||
| SlashCommand::Keymap
|
||||
| SlashCommand::Agent
|
||||
| SlashCommand::MultiAgents
|
||||
| SlashCommand::Permissions
|
||||
| SlashCommand::ElevateSandbox
|
||||
|
||||
@@ -49,7 +49,7 @@ async fn parent_owned_thread_blocks_all_direct_input_entry_points() {
|
||||
"/side inspect this",
|
||||
"/archive",
|
||||
"/rename",
|
||||
"/agent parent",
|
||||
"/subagents parent",
|
||||
"/diff now",
|
||||
"!echo blocked",
|
||||
" !echo blocked",
|
||||
|
||||
@@ -891,11 +891,11 @@ async fn esc_with_pending_steers_overrides_agent_command_interrupt_behavior() {
|
||||
}
|
||||
|
||||
chat.bottom_pane
|
||||
.set_composer_text("/agent ".to_string(), Vec::new(), Vec::new());
|
||||
.set_composer_text("/subagents ".to_string(), Vec::new(), Vec::new());
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
|
||||
next_interrupt_op(&mut op_rx);
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "/agent ");
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "/subagents ");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
//! Helpers for rendering and navigating multi-agent state in the TUI.
|
||||
//!
|
||||
//! This module owns the shared presentation contracts for multi-agent history rows, `/agent` picker
|
||||
//! entries, and the fast-switch keyboard shortcuts. Higher-level coordination, such as deciding
|
||||
//! which thread becomes active or when a thread closes, stays in [`crate::app::App`].
|
||||
//! This module owns the shared presentation contracts for multi-agent history rows, `/subagents`
|
||||
//! picker entries, and the fast-switch keyboard shortcuts. Higher-level coordination, such as
|
||||
//! deciding which thread becomes active or when a thread closes, stays in [`crate::app::App`].
|
||||
|
||||
use crate::history_cell::PlainHistoryCell;
|
||||
use crate::render::line_utils::prefix_lines;
|
||||
|
||||
@@ -40,7 +40,6 @@ pub enum SlashCommand {
|
||||
Compact,
|
||||
Plan,
|
||||
Goal,
|
||||
Agent,
|
||||
Side,
|
||||
Btw,
|
||||
Copy,
|
||||
@@ -127,7 +126,7 @@ impl SlashCommand {
|
||||
SlashCommand::Personality => "choose a communication style for Codex",
|
||||
SlashCommand::Plan => "switch to Plan mode",
|
||||
SlashCommand::Goal => "set or view the goal for a long-running task",
|
||||
SlashCommand::Agent | SlashCommand::MultiAgents => "switch the active agent thread",
|
||||
SlashCommand::MultiAgents => "switch between this session's subagents",
|
||||
SlashCommand::Side | SlashCommand::Btw => {
|
||||
"start a side conversation in an ephemeral fork"
|
||||
}
|
||||
@@ -256,7 +255,7 @@ impl SlashCommand {
|
||||
| SlashCommand::Btw => true,
|
||||
SlashCommand::Rollout => true,
|
||||
SlashCommand::TestApproval => true,
|
||||
SlashCommand::Agent | SlashCommand::MultiAgents => true,
|
||||
SlashCommand::MultiAgents => true,
|
||||
SlashCommand::Theme | SlashCommand::Pets => false,
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user