QUALITY-544: Lock NLD for block/image/file-attached agent-view entry

.gitignore

@@ -55,3 +55,6 @@ desired_behavior.md
 
 # Don't include the python cache for bundled skills.
 __pycache__/
+
+# Project notes
+.note/

app/src/ai/blocklist/context_model.rs

@@ -318,6 +318,34 @@ impl BlocklistAIContextModel {
         }
     }
 
+    /// Test-only constructor that skips every subscription and singleton lookup performed by
+    /// [`Self::new`], so unit tests can build a [`BlocklistAIContextModel`] without registering
+    /// `BlocklistAIHistoryModel`, `LLMPreferences`, `ModelEventDispatcher`, `Sessions`, or
+    /// `AppExecutionMode`. Callers still pass real [`TerminalModel`] and [`AgentViewController`]
+    /// handles to populate the struct fields, but neither needs to be functional for the
+    /// methods exercised by these tests.
+    #[cfg(test)]
+    pub(crate) fn new_for_test(
+        terminal_model: Arc<FairMutex<TerminalModel>>,
+        terminal_view_id: EntityId,
+        agent_view_controller: ModelHandle<AgentViewController>,
+    ) -> Self {
+        Self {
+            terminal_model,
+            directory_context: Default::default(),
+            pending_context_block_ids: HashSet::new(),
+            pending_context_selected_text: None,
+            pending_attachments: Default::default(),
+            pending_query_state: PendingQueryState::default(),
+            terminal_view_id,
+            agent_view_controller,
+            pending_inline_diff_hunk_attachments: Default::default(),
+            pending_document_id: None,
+            auto_attached_agent_view_user_block_ids: Vec::new(),
+            queue_next_prompt_enabled: false,
+        }
+    }
+
     /// Resets the set of blocks to be included as context to an empty list.
     /// Also removes any selected text that was to be included as context.
     pub fn reset_context_to_default(&mut self, ctx: &mut ModelContext<Self>) {
@@ -329,6 +357,12 @@ impl BlocklistAIContextModel {
         self.auto_attached_agent_view_user_block_ids.clear();
     }
 
+    /// Returns `true` if the next AI query has any context that should force the input to be
+    /// locked in AI mode (skipping NLD): a pending image or file attachment, or a pending block.
+    pub fn has_locking_attachment(&self) -> bool {
+        !self.pending_context_block_ids.is_empty() || !self.pending_attachments.is_empty()
+    }
+
     /// Returns the set `BlockId`s corresponding to blocks to be included as context with the next
     /// query.
     pub fn pending_context_block_ids(&self) -> &HashSet<BlockId> {
@@ -991,3 +1025,7 @@ pub enum BlocklistAIContextEvent {
 impl Entity for BlocklistAIContextModel {
     type Event = BlocklistAIContextEvent;
 }
+
+#[cfg(test)]
+#[path = "context_model_test.rs"]
+mod tests;

app/src/ai/blocklist/context_model_test.rs

@@ -0,0 +1,176 @@
+//! Unit tests for [`BlocklistAIContextModel::has_locking_attachment`].
+//!
+//! These tests deliberately bypass the production [`BlocklistAIContextModel::new`] constructor
+//! (which subscribes to several singletons) and instead use [`BlocklistAIContextModel::new_for_test`]
+//! together with [`super::agent_view::AgentViewController::new`]. That keeps the fixture small
+//! enough to focus on the lock logic without standing up `BlocklistAIHistoryModel`,
+//! `LLMPreferences`, `CloudModel`, `UpdateManager`, or `AppExecutionMode`.
+
+use std::sync::Arc;
+
+use parking_lot::FairMutex;
+use warpui::r#async::executor::Background;
+use warpui::{App, EntityId, ModelHandle};
+
+use super::{BlocklistAIContextModel, PendingAttachment, PendingFile};
+use crate::ai::agent::ImageContext;
+use crate::ai::blocklist::agent_view::{AgentViewController, EphemeralMessageModel};
+use crate::terminal::color::{self, Colors};
+use crate::terminal::event_listener::ChannelEventListener;
+use crate::terminal::model::test_utils::block_size;
+use crate::terminal::model::{BlockId, TerminalModel};
+
+impl BlocklistAIContextModel {
+    pub(crate) fn append_pending_attachments_for_test(
+        &mut self,
+        attachments: Vec<PendingAttachment>,
+    ) {
+        self.pending_attachments.extend(attachments);
+    }
+
+    pub(crate) fn insert_pending_block_id_for_test(&mut self, block_id: BlockId) {
+        self.pending_context_block_ids.insert(block_id);
+    }
+
+    pub(crate) fn set_pending_selected_text_for_test(&mut self, text: Option<String>) {
+        self.pending_context_selected_text = text;
+    }
+}
+
+/// Builds a [`BlocklistAIContextModel`] with stub dependencies. None of the dependencies are
+/// exercised by the methods under test; they only need to satisfy the struct's field types.
+fn build_test_context_model(app: &mut App) -> ModelHandle<BlocklistAIContextModel> {
+    let terminal_model = Arc::new(FairMutex::new(TerminalModel::new_for_test(
+        block_size(),
+        color::List::from(&Colors::default()),
+        ChannelEventListener::new_for_test(),
+        Arc::new(Background::default()),
+        false, /* should_show_bootstrap_block */
+        None,  /* restored_blocks */
+        false, /* honor_ps1 */
+        false, /* is_inverted */
+        None,  /* session_startup_path */
+    )));
+    let terminal_view_id = EntityId::new();
+
+    let ephemeral_message_model = app.add_model(|_| EphemeralMessageModel::new());
+    let agent_view_controller = app.add_model(|_| {
+        AgentViewController::new(
+            terminal_model.clone(),
+            terminal_view_id,
+            ephemeral_message_model,
+        )
+    });
+
+    app.add_model(|_| {
+        BlocklistAIContextModel::new_for_test(
+            terminal_model,
+            terminal_view_id,
+            agent_view_controller,
+        )
+    })
+}
+
+fn make_image_attachment(file_name: &str) -> PendingAttachment {
+    PendingAttachment::Image(ImageContext {
+        data: String::new(),
+        mime_type: "image/png".to_owned(),
+        file_name: file_name.to_owned(),
+        is_figma: false,
+    })
+}
+
+fn make_file_attachment(file_name: &str) -> PendingAttachment {
+    PendingAttachment::File(PendingFile {
+        file_name: file_name.to_owned(),
+        file_path: file_name.into(),
+        mime_type: "text/plain".to_owned(),
+    })
+}
+
+#[test]
+fn has_locking_attachment_is_false_for_default_state() {
+    App::test((), |mut app| async move {
+        let model = build_test_context_model(&mut app);
+
+        model.read(&app, |m, _| {
+            assert!(!m.has_locking_attachment());
+        });
+    });
+}
+
+#[test]
+fn has_locking_attachment_is_true_with_pending_block_id() {
+    App::test((), |mut app| async move {
+        let model = build_test_context_model(&mut app);
+
+        model.update(&mut app, |m, _| {
+            m.insert_pending_block_id_for_test(BlockId::new());
+        });
+
+        model.read(&app, |m, _| assert!(m.has_locking_attachment()));
+    });
+}
+
+#[test]
+fn has_locking_attachment_is_false_with_only_pending_selected_text() {
+    // Selected text alone is *not* a locking attachment: the user could be selecting shell
+    // command text (e.g. to copy a previously-run command), and forcing the input into AI
+    // mode in that case would be wrong. Only images, files, or blocks should force the lock.
+    App::test((), |mut app| async move {
+        let model = build_test_context_model(&mut app);
+
+        model.update(&mut app, |m, _| {
+            m.set_pending_selected_text_for_test(Some("hello".to_owned()));
+        });
+
+        model.read(&app, |m, _| assert!(!m.has_locking_attachment()));
+    });
+}
+
+#[test]
+fn has_locking_attachment_is_true_with_pending_image_attachment() {
+    App::test((), |mut app| async move {
+        let model = build_test_context_model(&mut app);
+
+        model.update(&mut app, |m, _| {
+            m.append_pending_attachments_for_test(vec![make_image_attachment("a.png")]);
+        });
+
+        model.read(&app, |m, _| assert!(m.has_locking_attachment()));
+    });
+}
+
+#[test]
+fn has_locking_attachment_is_true_with_only_file_attachments() {
+    // File attachments are locking attachments — the user has explicitly attached a file as
+    // context, which is unambiguously a signal that the next query is intended for the agent.
+    App::test((), |mut app| async move {
+        let model = build_test_context_model(&mut app);
+
+        model.update(&mut app, |m, _| {
+            m.append_pending_attachments_for_test(vec![
+                make_file_attachment("notes.txt"),
+                make_file_attachment("readme.md"),
+            ]);
+        });
+
+        model.read(&app, |m, _| assert!(m.has_locking_attachment()));
+    });
+}
+
+#[test]
+fn has_locking_attachment_is_true_with_mixed_image_and_file_attachments() {
+    App::test((), |mut app| async move {
+        let model = build_test_context_model(&mut app);
+
+        model.update(&mut app, |m, _| {
+            m.append_pending_attachments_for_test(vec![
+                make_file_attachment("notes.txt"),
+                make_image_attachment("a.png"),
+            ]);
+        });
+
+        model.read(&app, |m, _| assert!(m.has_locking_attachment()));
+    });
+}

app/src/ai/blocklist/input_model.rs

@@ -20,6 +20,7 @@ use warpui::{AppContext, Entity, EntityId, ModelContext, ModelHandle, SingletonE
 pub use input_classifier::InputType;
 
 use super::agent_view::{AgentViewController, AgentViewControllerEvent, AgentViewEntryOrigin};
+use super::context_model::BlocklistAIContextModel;
 use crate::terminal::cli_agent_sessions::{
     CLIAgentInputState, CLIAgentSessionsModel, CLIAgentSessionsModelEvent,
 };
@@ -148,6 +149,11 @@ pub struct BlocklistAIInputModel {
 
     agent_view_controller: ModelHandle<AgentViewController>,
 
+    /// Handle to the per-pane context model. Used to read pending attachments / blocks when
+    /// deciding whether to force-lock the input to AI mode (see
+    /// [`BlocklistAIContextModel::has_locking_attachment`]).
+    ai_context_model: ModelHandle<BlocklistAIContextModel>,
+
     terminal_view_id: EntityId,
 
     autodetect_abort_handle: Option<AbortHandle>,
@@ -158,6 +164,7 @@ impl BlocklistAIInputModel {
     pub fn new(
         model: Arc<FairMutex<TerminalModel>>,
         agent_view_controller: ModelHandle<AgentViewController>,
+        ai_context_model: ModelHandle<BlocklistAIContextModel>,
         terminal_view_id: EntityId,
         ctx: &mut ModelContext<Self>,
     ) -> Self {
@@ -267,6 +274,19 @@ impl BlocklistAIInputModel {
                             },
                             ctx,
                         );
+                    } else if me.has_locking_attachment(ctx) {
+                        // Interaction patterns that should fully bypass NLD on
+                        // entry: image / file attachment in progress / attached, or block
+                        // already in pending context. Force-lock to AI regardless of the
+                        // user's NLD setting so the classifier never gets a chance to drop
+                        // the buffer back to shell.
+                        me.set_input_config_internal(
+                            InputConfig {
+                                input_type: InputType::AI,
+                                is_locked: true,
+                            },
+                            ctx,
+                        );
                     } else {
                         let is_autodetection_enabled =
                             AISettings::as_ref(ctx).is_ai_autodetection_enabled(ctx);
@@ -320,6 +340,7 @@ impl BlocklistAIInputModel {
                 is_locked: !is_autodetection_enabled,
             },
             agent_view_controller,
+            ai_context_model,
             terminal_view_id,
             last_ai_autodetection_ts: None,
             last_explicit_input_type_set_at: None,
@@ -329,6 +350,11 @@ impl BlocklistAIInputModel {
         }
     }
 
+    /// Convenience wrapper around `BlocklistAIContextModel::has_locking_attachment`.
+    fn has_locking_attachment(&self, app: &AppContext) -> bool {
+        self.ai_context_model.as_ref(app).has_locking_attachment()
+    }
+
     /// Returns the InputType enum which specifies how we will handle the terminal input.
     pub fn input_type(&self) -> InputType {
         self.input_config.input_type
@@ -488,6 +514,14 @@ impl BlocklistAIInputModel {
             return false;
         }
 
+        // Defense in depth: while there is a pending attachment (image / file) or block,
+        // the classifier must never have a chance to flip the input back to shell mode, even
+        // per-keystroke. The `EnteredAgentView` subscriber and `set_input_mode_agent` already
+        // lock at entry; this guard protects the window if any future caller forgets.
+        if self.has_locking_attachment(app) {
+            return false;
+        }
+
         let ai_settings = AISettings::as_ref(app);
         if FeatureFlag::AgentView.is_enabled() {
             if self.agent_view_controller.as_ref(app).is_fullscreen() {

app/src/terminal/input.rs

@@ -13015,10 +13015,16 @@ impl Input {
 
         // When AgentView is enabled, reverting to AI mode in an active agent view with an empty
         // buffer should unlock (re-enable autodetection) - semantically like clearing the "!".
+        //
+        // If there is a pending image / file attachment or block, do NOT unlock. The user's
+        // intent is unambiguously "talk to the agent"; letting the classifier flip the input
+        // back to shell mode would be a bug.
+        let has_locking_attachment = self.ai_context_model.as_ref(ctx).has_locking_attachment();
         let should_unlock = FeatureFlag::AgentView.is_enabled()
             && self.agent_view_controller.as_ref(ctx).is_fullscreen()
             && is_input_buffer_empty
-            && AISettings::as_ref(ctx).is_ai_autodetection_enabled(ctx);
+            && AISettings::as_ref(ctx).is_ai_autodetection_enabled(ctx)
+            && !has_locking_attachment;
 
         if should_unlock {
             self.ai_input_model.update(ctx, |ai_input_model, ctx| {

app/src/terminal/view.rs

@@ -3293,10 +3293,21 @@ impl TerminalView {
             ctx.notify();
         });
 
+        let ai_context_model = ctx.add_model(|ctx| {
+            BlocklistAIContextModel::new(
+                sessions.clone(),
+                &model_events_handle,
+                model.clone(),
+                terminal_view_id,
+                agent_view_controller.clone(),
+                ctx,
+            )
+        });
         let ai_input_model = ctx.add_model(|ctx| {
             let mut model = BlocklistAIInputModel::new(
                 model.clone(),
                 agent_view_controller.clone(),
+                ai_context_model.clone(),
                 terminal_view_id,
                 ctx,
             );
@@ -3322,16 +3333,6 @@ impl TerminalView {
                 ctx,
             )
         });
-        let ai_context_model = ctx.add_model(|ctx| {
-            BlocklistAIContextModel::new(
-                sessions.clone(),
-                &model_events_handle,
-                model.clone(),
-                terminal_view_id,
-                agent_view_controller.clone(),
-                ctx,
-            )
-        });
         let ai_controller = ctx.add_model(|ctx| {
             BlocklistAIController::new(
                 ai_input_model.clone(),