Skip to content

Commit 9646bf0

Browse files
committed
feat(acp): Add /agent command for ACP agent switching with pending selection
Implements agent/model switching for ACP mode: - Add /agent slash command to open agent picker popup - Add agent_picker component in nori/ with SelectionViewParams - Track pending agent selection - switch happens on next prompt submission - Modify /model to show disabled options in ACP mode with redirect to /agent - Add list_available_agents() and AcpAgentInfo to ACP registry - Add AppEvent variants for SetPendingAgent, ClearPendingAgent, SubmitWithAgentSwitch - Add ACP file tracing initialization to TUI - Add E2E tests for agent switching behavior
1 parent f487fce commit 9646bf0

11 files changed

Lines changed: 721 additions & 1 deletion

File tree

codex-rs/acp/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ pub use backend::AcpBackendConfig;
1414
pub use connection::AcpConnection;
1515
pub use connection::ApprovalRequest;
1616
pub use registry::AcpAgentConfig;
17+
pub use registry::AcpAgentInfo;
1718
pub use registry::AcpProviderInfo;
1819
pub use registry::get_agent_config;
20+
pub use registry::list_available_agents;
1921
pub use tracing_setup::init_file_tracing;
2022
pub use translator::TranslatedEvent;
2123
pub use translator::translate_session_update;

codex-rs/acp/src/registry.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,49 @@
66
use anyhow::Result;
77
use std::time::Duration;
88

9+
/// Information about an available ACP agent for display in the picker
10+
#[derive(Debug, Clone, PartialEq, Eq)]
11+
pub struct AcpAgentInfo {
12+
/// Model name used to select this agent (e.g., "mock-model", "gemini-2.5-flash")
13+
pub model_name: String,
14+
/// Display name shown in the picker
15+
pub display_name: String,
16+
/// Description of the agent
17+
pub description: String,
18+
/// Provider slug for this agent
19+
pub provider_slug: String,
20+
}
21+
22+
/// Get list of all available ACP agents for the agent picker
23+
pub fn list_available_agents() -> Vec<AcpAgentInfo> {
24+
vec![
25+
AcpAgentInfo {
26+
model_name: "mock-model".to_string(),
27+
display_name: "Mock ACP".to_string(),
28+
description: "Mock agent for testing".to_string(),
29+
provider_slug: "mock-acp".to_string(),
30+
},
31+
AcpAgentInfo {
32+
model_name: "mock-model-alt".to_string(),
33+
display_name: "Mock ACP Alt".to_string(),
34+
description: "Alternate mock agent for testing".to_string(),
35+
provider_slug: "mock-acp-alt".to_string(),
36+
},
37+
AcpAgentInfo {
38+
model_name: "gemini-2.5-flash".to_string(),
39+
display_name: "Gemini 2.5 Flash".to_string(),
40+
description: "Google Gemini via ACP".to_string(),
41+
provider_slug: "gemini-acp".to_string(),
42+
},
43+
AcpAgentInfo {
44+
model_name: "claude-4.5".to_string(),
45+
display_name: "Claude 4.5".to_string(),
46+
description: "Anthropic Claude via ACP".to_string(),
47+
provider_slug: "claude-acp".to_string(),
48+
},
49+
]
50+
}
51+
952
/// Default idle timeout for ACP streaming (5 minutes)
1053
const DEFAULT_STREAM_IDLE_TIMEOUT: Duration = Duration::from_secs(300);
1154

codex-rs/tui-pty-e2e/tests/agent_switching.rs

Lines changed: 333 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,3 +374,336 @@ fn test_acp_agent_switch_via_model_picker() {
374374
// If no new PID, the model picker might not trigger subprocess restart
375375
// This is acceptable behavior - document it
376376
}
377+
378+
// ============================================================================
379+
// Test: /agent Slash Command - Shows Available Agents
380+
// ============================================================================
381+
382+
/// Test that /agent command shows available ACP agents from the registry
383+
#[test]
384+
#[cfg(target_os = "linux")]
385+
fn test_agent_command_shows_available_agents() {
386+
let config = SessionConfig::new().with_model("mock-model".to_string());
387+
388+
let mut session = TuiSession::spawn_with_config(24, 80, config).expect("Failed to spawn TUI");
389+
390+
session
391+
.wait_for_text("›", TIMEOUT)
392+
.expect("TUI should start");
393+
std::thread::sleep(TIMEOUT_INPUT);
394+
395+
// Open agent picker with /agent command
396+
session.send_str("/agent").unwrap();
397+
std::thread::sleep(TIMEOUT_INPUT);
398+
session.send_key(Key::Enter).unwrap();
399+
std::thread::sleep(TIMEOUT_INPUT);
400+
401+
// Wait for agent picker to appear - it should show available agents
402+
session
403+
.wait_for(
404+
|screen| {
405+
// Should show available agents from the ACP registry
406+
screen.contains("Select Agent") || screen.contains("mock-model")
407+
},
408+
Duration::from_secs(3),
409+
)
410+
.expect("Agent picker should appear");
411+
412+
// Verify both mock agents are visible
413+
let screen = session.screen_contents();
414+
assert!(
415+
screen.contains("mock-model") || screen.contains("Mock"),
416+
"Agent picker should show mock-model agent, got: {}",
417+
screen
418+
);
419+
}
420+
421+
// ============================================================================
422+
// Test: /agent Slash Command - Pending Selection
423+
// ============================================================================
424+
425+
/// Test that selecting an agent in /agent tracks it as pending and doesn't
426+
/// switch immediately
427+
#[test]
428+
#[cfg(target_os = "linux")]
429+
fn test_agent_command_pending_selection() {
430+
let config = SessionConfig::new().with_model("mock-model".to_string());
431+
432+
let mut session = TuiSession::spawn_with_config(24, 80, config).expect("Failed to spawn TUI");
433+
434+
session
435+
.wait_for_text("›", TIMEOUT)
436+
.expect("TUI should start");
437+
std::thread::sleep(TIMEOUT_INPUT);
438+
439+
let log_path = session.acp_log_path().expect("Should have log path");
440+
let initial_pids = extract_mock_agent_pids_from_log(&log_path);
441+
assert!(!initial_pids.is_empty(), "Should have initial PID");
442+
let initial_pid = initial_pids[0];
443+
444+
// Open agent picker with /agent command
445+
session.send_str("/agent").unwrap();
446+
std::thread::sleep(TIMEOUT_INPUT);
447+
session.send_key(Key::Enter).unwrap();
448+
std::thread::sleep(TIMEOUT_INPUT);
449+
450+
// Wait for agent picker to appear
451+
session
452+
.wait_for(
453+
|screen| screen.contains("Select Agent") || screen.contains("mock-model"),
454+
Duration::from_secs(3),
455+
)
456+
.expect("Agent picker should appear");
457+
458+
// Select a different agent (mock-model-alt)
459+
session.send_key(Key::Down).unwrap();
460+
std::thread::sleep(TIMEOUT_INPUT);
461+
session.send_key(Key::Enter).unwrap();
462+
std::thread::sleep(Duration::from_millis(500));
463+
464+
// After selecting, the OLD agent should still be running (pending selection)
465+
let pids_after_selection = extract_mock_agent_pids_from_log(&log_path);
466+
assert_eq!(
467+
pids_after_selection.len(),
468+
initial_pids.len(),
469+
"No new subprocess should be spawned yet - selection is pending until next prompt"
470+
);
471+
472+
// The original process should still be alive
473+
assert!(
474+
process_exists_and_not_zombie(initial_pid),
475+
"Original agent should still be running after pending selection"
476+
);
477+
}
478+
479+
// ============================================================================
480+
// Test: /agent Slash Command - Switch on Prompt Submission
481+
// ============================================================================
482+
483+
/// Test that agent switch happens on next prompt submission
484+
#[test]
485+
#[cfg(target_os = "linux")]
486+
fn test_agent_switch_on_prompt_submission() {
487+
let config = SessionConfig::new().with_model("mock-model".to_string());
488+
489+
let mut session = TuiSession::spawn_with_config(24, 80, config).expect("Failed to spawn TUI");
490+
491+
session
492+
.wait_for_text("›", TIMEOUT)
493+
.expect("TUI should start");
494+
std::thread::sleep(TIMEOUT_INPUT);
495+
496+
let log_path = session.acp_log_path().expect("Should have log path");
497+
let initial_pids = extract_mock_agent_pids_from_log(&log_path);
498+
assert!(!initial_pids.is_empty(), "Should have initial PID");
499+
let initial_pid = initial_pids[0];
500+
501+
// Open agent picker with /agent command
502+
session.send_str("/agent").unwrap();
503+
std::thread::sleep(TIMEOUT_INPUT);
504+
session.send_key(Key::Enter).unwrap();
505+
std::thread::sleep(TIMEOUT_INPUT);
506+
507+
// Wait for agent picker to appear
508+
session
509+
.wait_for(
510+
|screen| screen.contains("Select Agent") || screen.contains("mock-model"),
511+
Duration::from_secs(3),
512+
)
513+
.expect("Agent picker should appear");
514+
515+
// Select a different agent (mock-model-alt)
516+
session.send_key(Key::Down).unwrap();
517+
std::thread::sleep(TIMEOUT_INPUT);
518+
session.send_key(Key::Enter).unwrap();
519+
std::thread::sleep(Duration::from_millis(500));
520+
521+
// Now submit a prompt - this should trigger the agent switch
522+
session.send_str("hello").unwrap();
523+
std::thread::sleep(TIMEOUT_INPUT);
524+
session.send_key(Key::Enter).unwrap();
525+
526+
// Wait for the response to start
527+
session
528+
.wait_for_text("Working", Duration::from_secs(5))
529+
.ok(); // May or may not see this depending on response speed
530+
std::thread::sleep(Duration::from_millis(1000));
531+
532+
// Check that a new agent was spawned
533+
let post_prompt_pids = extract_mock_agent_pids_from_log(&log_path);
534+
assert!(
535+
post_prompt_pids.len() > initial_pids.len(),
536+
"New subprocess should be spawned after prompt submission with pending agent: initial={:?}, after={:?}",
537+
initial_pids,
538+
post_prompt_pids
539+
);
540+
541+
let new_pid = *post_prompt_pids.last().unwrap();
542+
assert_ne!(
543+
initial_pid, new_pid,
544+
"New agent should have different PID after prompt submission"
545+
);
546+
}
547+
548+
// ============================================================================
549+
// Test: /agent - No Switch During Active Prompt Turn
550+
// ============================================================================
551+
552+
/// Test that navigating /agent picker during streaming doesn't kill the agent
553+
#[test]
554+
#[cfg(target_os = "linux")]
555+
fn test_agent_picker_no_switch_during_streaming() {
556+
let config = SessionConfig::new()
557+
.with_model("mock-model".to_string())
558+
.with_stream_until_cancel(); // Agent streams until cancelled
559+
560+
let mut session = TuiSession::spawn_with_config(24, 80, config).expect("Failed to spawn TUI");
561+
562+
session
563+
.wait_for_text("›", TIMEOUT)
564+
.expect("TUI should start");
565+
std::thread::sleep(TIMEOUT_INPUT);
566+
567+
let log_path = session.acp_log_path().expect("Should have log path");
568+
let initial_pids = extract_mock_agent_pids_from_log(&log_path);
569+
assert!(!initial_pids.is_empty(), "Should have initial PID");
570+
let initial_pid = initial_pids[0];
571+
572+
// Start a streaming prompt
573+
session.send_str("Start streaming").unwrap();
574+
std::thread::sleep(TIMEOUT_INPUT);
575+
session.send_key(Key::Enter).unwrap();
576+
577+
// Wait for streaming to start
578+
session
579+
.wait_for_text("Working", Duration::from_secs(5))
580+
.expect("Streaming should start");
581+
582+
// While streaming, the agent should still be running
583+
assert!(
584+
process_exists_and_not_zombie(initial_pid),
585+
"Agent should be running during streaming"
586+
);
587+
588+
// Cancel streaming first so we can access the UI
589+
session.send_key(Key::Escape).unwrap();
590+
std::thread::sleep(Duration::from_millis(500));
591+
592+
// The agent should still be the same
593+
let pids_after = extract_mock_agent_pids_from_log(&log_path);
594+
assert_eq!(
595+
pids_after.len(),
596+
initial_pids.len(),
597+
"No new subprocess should be spawned during/after streaming cancel"
598+
);
599+
assert!(
600+
process_exists_and_not_zombie(initial_pid),
601+
"Original agent should still be running after cancel"
602+
);
603+
}
604+
605+
// ============================================================================
606+
// Test: /model Slash Command - Shows Disabled in ACP Mode
607+
// ============================================================================
608+
609+
/// Test that /model command shows disabled options in ACP mode
610+
#[test]
611+
#[cfg(target_os = "linux")]
612+
fn test_model_command_shows_disabled_in_acp_mode() {
613+
let config = SessionConfig::new().with_model("mock-model".to_string());
614+
615+
let mut session = TuiSession::spawn_with_config(24, 80, config).expect("Failed to spawn TUI");
616+
617+
session
618+
.wait_for_text("›", TIMEOUT)
619+
.expect("TUI should start");
620+
std::thread::sleep(TIMEOUT_INPUT);
621+
622+
// Open model picker with /model command
623+
session.send_str("/model").unwrap();
624+
std::thread::sleep(TIMEOUT_INPUT);
625+
session.send_key(Key::Enter).unwrap();
626+
std::thread::sleep(TIMEOUT_INPUT);
627+
628+
// Wait for model picker to appear
629+
session
630+
.wait_for(
631+
|screen| screen.contains("Select Model") || screen.contains("Model"),
632+
Duration::from_secs(3),
633+
)
634+
.expect("Model picker should appear");
635+
636+
// In ACP mode, model options should show as disabled or indicate
637+
// they're not available
638+
let screen = session.screen_contents();
639+
assert!(
640+
screen.contains("disabled")
641+
|| screen.contains("Not available")
642+
|| screen.contains("ACP")
643+
|| screen.contains("Use /agent"),
644+
"Model picker should indicate options are disabled in ACP mode, got: {}",
645+
screen
646+
);
647+
}
648+
649+
// ============================================================================
650+
// Test: /agent Slash Command - Cleanup After Switch
651+
// ============================================================================
652+
653+
/// Test that old agent subprocess is cleaned up after switch on prompt
654+
#[test]
655+
#[cfg(target_os = "linux")]
656+
fn test_agent_cleanup_after_switch_on_prompt() {
657+
let config = SessionConfig::new().with_model("mock-model".to_string());
658+
659+
let mut session = TuiSession::spawn_with_config(24, 80, config).expect("Failed to spawn TUI");
660+
661+
session
662+
.wait_for_text("›", TIMEOUT)
663+
.expect("TUI should start");
664+
std::thread::sleep(TIMEOUT_INPUT);
665+
666+
let log_path = session.acp_log_path().expect("Should have log path");
667+
let initial_pids = extract_mock_agent_pids_from_log(&log_path);
668+
assert!(!initial_pids.is_empty(), "Should have initial PID");
669+
let initial_pid = initial_pids[0];
670+
671+
// Verify initial process exists
672+
assert!(
673+
process_exists_and_not_zombie(initial_pid),
674+
"Initial agent should exist"
675+
);
676+
677+
// Open agent picker and select a different agent
678+
session.send_str("/agent").unwrap();
679+
std::thread::sleep(TIMEOUT_INPUT);
680+
session.send_key(Key::Enter).unwrap();
681+
std::thread::sleep(TIMEOUT_INPUT);
682+
683+
session
684+
.wait_for(
685+
|screen| screen.contains("Select Agent") || screen.contains("mock-model"),
686+
Duration::from_secs(3),
687+
)
688+
.expect("Agent picker should appear");
689+
690+
session.send_key(Key::Down).unwrap();
691+
std::thread::sleep(TIMEOUT_INPUT);
692+
session.send_key(Key::Enter).unwrap();
693+
std::thread::sleep(Duration::from_millis(500));
694+
695+
// Submit prompt to trigger switch
696+
session.send_str("trigger switch").unwrap();
697+
std::thread::sleep(TIMEOUT_INPUT);
698+
session.send_key(Key::Enter).unwrap();
699+
700+
// Wait for response
701+
std::thread::sleep(Duration::from_millis(2000));
702+
703+
// Old process should be cleaned up
704+
assert!(
705+
!process_exists(initial_pid) || !process_exists_and_not_zombie(initial_pid),
706+
"Old agent subprocess {} should be cleaned up after switch",
707+
initial_pid
708+
);
709+
}

0 commit comments

Comments
 (0)