instrument current event loop to conditionally use managed input

Author: dingfeli

crates/chat-cli-ui/src/conduit.rs

@@ -61,7 +61,7 @@ impl ViewEnd {
     /// This blocks the current thread and consumes the [ViewEnd]
     pub fn into_legacy_mode(
         mut self,
-        handle_input: bool,
+        managed_input: bool,
         theme_source: impl ThemeSource,
         mut stderr: std::io::Stderr,
         mut stdout: std::io::Stdout,
@@ -254,7 +254,7 @@ impl ViewEnd {
             Ok::<(), ConduitError>(())
         }
 
-        if handle_input {
+        if managed_input {
             let (incoming_events_tx, mut incoming_events_rx) = tokio::sync::mpsc::unbounded_channel::<IncomingEvent>();
             let (prompt_signal_tx, prompt_signal_rx) = std::sync::mpsc::channel::<PromptSignal>();
 

crates/chat-cli/src/cli/chat/mod.rs

@@ -4,7 +4,10 @@ use spinners::{
 };
 
 use crate::theme::StyledText;
-use crate::util::ui::should_send_structured_message;
+use crate::util::ui::{
+    should_send_structured_message,
+    should_use_ui_managed_input,
+};
 pub mod cli;
 mod consts;
 pub mod context;
@@ -429,13 +432,18 @@ impl ChatArgs {
             .build(os, Box::new(std::io::stderr()), !self.no_interactive)
             .await?;
         let tool_config = tool_manager.load_tools(os, &mut stderr).await?;
+        let input_source = if should_use_ui_managed_input() {
+            None
+        } else {
+            Some(InputSource::new(os, prompt_request_sender, prompt_response_receiver)?)
+        };
 
         ChatSession::new(
             os,
             &conversation_id,
             agents,
             input,
-            InputSource::new(os, prompt_request_sender, prompt_response_receiver)?,
+            input_source,
             self.resume,
             || terminal::window_size().map(|s| s.columns.into()).ok(),
             tool_manager,
@@ -578,7 +586,7 @@ pub struct ChatSession {
     initial_input: Option<String>,
     /// Whether we're starting a new conversation or continuing an old one.
     existing_conversation: bool,
-    input_source: InputSource,
+    input_source: Option<InputSource>,
     /// Width of the terminal, required for [ParseState].
     terminal_width_provider: fn() -> Option<usize>,
     spinner: Option<Spinner>,
@@ -617,7 +625,7 @@ impl ChatSession {
         conversation_id: &str,
         mut agents: Agents,
         mut input: Option<String>,
-        input_source: InputSource,
+        input_source: Option<InputSource>,
         resume_conversation: bool,
         terminal_width_provider: fn() -> Option<usize>,
         tool_manager: ToolManager,
@@ -630,13 +638,14 @@ impl ChatSession {
         // Only load prior conversation if we need to resume
         let mut existing_conversation = false;
 
+        let should_use_ui_managed_input = input_source.is_none();
         let should_send_structured_msg = should_send_structured_message(os);
         let (view_end, managed_input, mut control_end_stderr, control_end_stdout) =
             get_legacy_conduits(should_send_structured_msg);
 
         let stderr = std::io::stderr();
         let stdout = std::io::stdout();
-        if let Err(e) = view_end.into_legacy_mode(true, StyledText, stderr, stdout) {
+        if let Err(e) = view_end.into_legacy_mode(should_use_ui_managed_input, StyledText, stderr, stdout) {
             error!("Conduit view end legacy mode exited: {:?}", e);
         }
 
@@ -740,7 +749,11 @@ impl ChatSession {
             inner: Some(ChatState::default()),
             ctrlc_rx,
             wrap,
-            managed_input: Some(managed_input),
+            managed_input: if should_use_ui_managed_input {
+                Some(managed_input)
+            } else {
+                None
+            },
         })
     }
 
@@ -1938,8 +1951,9 @@ impl ChatSession {
                 .filter(|name| *name != DUMMY_TOOL_NAME)
                 .cloned()
                 .collect::<Vec<_>>();
-            self.input_source
-                .put_skim_command_selector(os, Arc::new(context_manager.clone()), tool_names);
+            if let Some(input_source) = &mut self.input_source {
+                input_source.put_skim_command_selector(os, Arc::new(context_manager.clone()), tool_names);
+            }
         }
 
         execute!(self.stderr, StyledText::reset(), StyledText::reset_attributes())?;
@@ -3438,9 +3452,14 @@ impl ChatSession {
 
     /// Helper function to read user input with a prompt and Ctrl+C handling
     fn read_user_input(&mut self, prompt: &str, exit_on_single_ctrl_c: bool) -> Option<String> {
+        // If this function is called at all, input_source should not be None
+        debug_assert!(self.input_source.is_some());
+
         let mut ctrl_c = false;
+        let input_source = self.input_source.as_mut()?;
+
         loop {
-            match (self.input_source.read_line(Some(prompt)), ctrl_c) {
+            match (input_source.read_line(Some(prompt)), ctrl_c) {
                 (Ok(Some(line)), _) => {
                     if line.trim().is_empty() {
                         continue; // Reprompt if the input is empty
@@ -3912,11 +3931,11 @@ mod tests {
             "fake_conv_id",
             agents,
             None,
-            InputSource::new_mock(vec![
+            Some(InputSource::new_mock(vec![
                 "create a new file".to_string(),
                 "y".to_string(),
                 "exit".to_string(),
-            ]),
+            ])),
             false,
             || Some(80),
             tool_manager,
@@ -4040,7 +4059,7 @@ mod tests {
             "fake_conv_id",
             agents,
             None,
-            InputSource::new_mock(vec![
+            Some(InputSource::new_mock(vec![
                 "/tools".to_string(),
                 "/tools help".to_string(),
                 "create a new file".to_string(),
@@ -4057,7 +4076,7 @@ mod tests {
                 "create a file".to_string(), // prompt again due to reset
                 "n".to_string(),             // cancel
                 "exit".to_string(),
-            ]),
+            ])),
             false,
             || Some(80),
             tool_manager,
@@ -4145,15 +4164,15 @@ mod tests {
             "fake_conv_id",
             agents,
             None,
-            InputSource::new_mock(vec![
+            Some(InputSource::new_mock(vec![
                 "create 2 new files parallel".to_string(),
                 "t".to_string(),
                 "/tools reset".to_string(),
                 "create 2 new files parallel".to_string(),
                 "y".to_string(),
                 "y".to_string(),
                 "exit".to_string(),
-            ]),
+            ])),
             false,
             || Some(80),
             tool_manager,
@@ -4221,13 +4240,13 @@ mod tests {
             "fake_conv_id",
             agents,
             None,
-            InputSource::new_mock(vec![
+            Some(InputSource::new_mock(vec![
                 "/tools trust-all".to_string(),
                 "create a new file".to_string(),
                 "/tools reset".to_string(),
                 "create a new file".to_string(),
                 "exit".to_string(),
-            ]),
+            ])),
             false,
             || Some(80),
             tool_manager,
@@ -4277,7 +4296,11 @@ mod tests {
             "fake_conv_id",
             agents,
             None,
-            InputSource::new_mock(vec!["/subscribe".to_string(), "y".to_string(), "/quit".to_string()]),
+            Some(InputSource::new_mock(vec![
+                "/subscribe".to_string(),
+                "y".to_string(),
+                "/quit".to_string(),
+            ])),
             false,
             || Some(80),
             tool_manager,
@@ -4380,11 +4403,11 @@ mod tests {
             "fake_conv_id",
             agents,
             None, // No initial input
-            InputSource::new_mock(vec![
+            Some(InputSource::new_mock(vec![
                 "read /test.txt".to_string(),
                 "y".to_string(), // Accept tool execution
                 "exit".to_string(),
-            ]),
+            ])),
             false,
             || Some(80),
             tool_manager,
@@ -4514,7 +4537,10 @@ mod tests {
             "test_conv_id",
             agents,
             None,
-            InputSource::new_mock(vec!["read /sensitive.txt".to_string(), "exit".to_string()]),
+            Some(InputSource::new_mock(vec![
+                "read /sensitive.txt".to_string(),
+                "exit".to_string(),
+            ])),
             false,
             || Some(80),
             tool_manager,

crates/chat-cli/src/util/ui.rs

@@ -6,10 +6,6 @@ use crossterm::style::{
     Attribute,
 };
 use eyre::Result;
-use serde::{
-    Deserialize,
-    Serialize,
-};
 
 use crate::cli::feed::Feed;
 use crate::constants::ui_text;
@@ -158,17 +154,37 @@ fn print_with_bold(output: &mut impl Write, segments: &[(String, bool)]) -> Resu
     Ok(())
 }
 
-#[derive(Default, Debug, Serialize, Deserialize)]
-#[serde(rename_all = "camelCase")]
-pub enum UiMode {
-    #[default]
-    Structured,
-    Passthrough,
-    New,
-}
-
+/// This dictates the event loop's egress behavior. It controls what gets emitted to the UI from the
+/// event loop.
+/// There are three possible potent states:
+/// - structured: This makes the event loop send structured messages where applicable (in addition
+///   to logging ANSI bytes directly where it has not been instrumented)
+/// - new: This spawns the new UI to be used on top of the current event loop (if we end up enabling
+///   this)
+/// - unset: This is the default behavior where everything is unstructured (i.e. ANSI bytes straight
+///   to stderr or stdout)
+///
+/// The reason why this is a setting as opposed to managed input, which is controlled via an env
+/// var, is because the choice of UI is a user concern. Whereas managed input is purely a
+/// development concern.
 pub fn should_send_structured_message(os: &Os) -> bool {
     let ui_mode = os.database.settings.get_string(Setting::UiMode);
 
-    ui_mode.as_deref().is_some_and(|mode| mode == "structured")
+    ui_mode
+        .as_deref()
+        .is_some_and(|mode| mode == "structured" || mode == "new")
+}
+
+/// NOTE: unless you are doing testing work for the new UI, you likely would not need to worry
+/// about setting this environment variable.
+/// This dictates the event loop's ingress behavior. It controls how the event loop receives input
+/// from the user.
+/// A normal input refers to the use of [crate::cli::chat::InputSource], which is owned by
+/// the [crate::cli::chat::ChatSession]. It is not managed by the UI layer (nor is the UI even
+/// aware of its existence).
+/// Conversely, an "ui managed" input is one where stdin is managed by the UI layer. For the event
+/// loop, this effectively means forgoing the ownership of [crate::cli::chat::InputSource] (it is
+/// replaced by a None) and instead delegating the reading of user input to the UI layer.
+pub fn should_use_ui_managed_input() -> bool {
+    std::env::var("Q_UI_MANAGED_INPUT").is_ok()
 }