agent: Make directory context display update on rename (#29189)

Michael Sloan created

Release Notes:

- N/A

Change summary

crates/agent/src/active_thread.rs         |  8 +---
crates/agent/src/context.rs               | 24 ++++++++++-----
crates/agent/src/context_store.rs         | 40 +++++++++++++++++++-----
crates/agent/src/ui/context_pill.rs       | 12 +++++--
crates/project_panel/src/project_panel.rs | 21 ++++++++++---
5 files changed, 74 insertions(+), 31 deletions(-)

Detailed changes

crates/agent/src/active_thread.rs 🔗

@@ -3282,12 +3282,10 @@ pub(crate) fn open_context(
             }
         }
         AssistantContext::Directory(directory_context) => {
-            let project_path = directory_context.project_path(cx);
+            let entry_id = directory_context.entry_id;
             workspace.update(cx, |workspace, cx| {
-                workspace.project().update(cx, |project, cx| {
-                    if let Some(entry) = project.entry_for_path(&project_path, cx) {
-                        cx.emit(project::Event::RevealInProjectPanel(entry.id));
-                    }
+                workspace.project().update(cx, |_project, cx| {
+                    cx.emit(project::Event::RevealInProjectPanel(entry_id));
                 })
             })
         }

crates/agent/src/context.rs 🔗

@@ -3,7 +3,7 @@ use std::{ops::Range, path::Path, sync::Arc};
 use gpui::{App, Entity, SharedString};
 use language::{Buffer, File};
 use language_model::LanguageModelRequestMessage;
-use project::{ProjectPath, Worktree};
+use project::{ProjectEntryId, ProjectPath, Worktree};
 use prompt_store::UserPromptId;
 use rope::Point;
 use serde::{Deserialize, Serialize};
@@ -83,17 +83,25 @@ pub struct FileContext {
 pub struct DirectoryContext {
     pub id: ContextId,
     pub worktree: Entity<Worktree>,
-    pub path: Arc<Path>,
+    pub entry_id: ProjectEntryId,
+    pub last_path: Arc<Path>,
     /// Buffers of the files within the directory.
     pub context_buffers: Vec<ContextBuffer>,
 }
 
 impl DirectoryContext {
-    pub fn project_path(&self, cx: &App) -> ProjectPath {
-        ProjectPath {
-            worktree_id: self.worktree.read(cx).id(),
-            path: self.path.clone(),
-        }
+    pub fn entry<'a>(&self, cx: &'a App) -> Option<&'a project::Entry> {
+        self.worktree.read(cx).entry_for_id(self.entry_id)
+    }
+
+    pub fn project_path(&self, cx: &App) -> Option<ProjectPath> {
+        let worktree = self.worktree.read(cx);
+        worktree
+            .entry_for_id(self.entry_id)
+            .map(|entry| ProjectPath {
+                worktree_id: worktree.id(),
+                path: entry.path.clone(),
+            })
     }
 }
 
@@ -131,7 +139,7 @@ impl ThreadContext {
 #[derive(Clone)]
 pub struct ContextBuffer {
     pub id: BufferId,
-    // TODO: Entity<Buffer> holds onto the thread even if the thread is deleted. Should probably be
+    // TODO: Entity<Buffer> holds onto the buffer even if the buffer is deleted. Should probably be
     // a WeakEntity and handle removal from the UI when it has dropped.
     pub buffer: Entity<Buffer>,
     pub file: Arc<dyn File>,

crates/agent/src/context_store.rs 🔗

@@ -8,7 +8,7 @@ use futures::future::join_all;
 use futures::{self, Future, FutureExt, future};
 use gpui::{App, AppContext as _, Context, Entity, SharedString, Task, WeakEntity};
 use language::{Buffer, File};
-use project::{Project, ProjectItem, ProjectPath, Worktree};
+use project::{Project, ProjectEntryId, ProjectItem, ProjectPath, Worktree};
 use prompt_store::UserPromptId;
 use rope::{Point, Rope};
 use text::{Anchor, BufferId, OffsetRangeExt};
@@ -162,6 +162,14 @@ impl ContextStore {
             return Task::ready(Err(anyhow!("failed to read project")));
         };
 
+        let Some(entry_id) = project
+            .read(cx)
+            .entry_for_path(&project_path, cx)
+            .map(|entry| entry.id)
+        else {
+            return Task::ready(Err(anyhow!("no entry found for directory context")));
+        };
+
         let already_included = match self.includes_directory(&project_path) {
             Some(FileInclusion::Direct(context_id)) => {
                 if remove_if_exists {
@@ -231,7 +239,7 @@ impl ContextStore {
             }
 
             this.update(cx, |this, cx| {
-                this.insert_directory(worktree, project_path, context_buffers, cx);
+                this.insert_directory(worktree, entry_id, project_path, context_buffers, cx);
             })?;
 
             anyhow::Ok(())
@@ -241,19 +249,21 @@ impl ContextStore {
     fn insert_directory(
         &mut self,
         worktree: Entity<Worktree>,
+        entry_id: ProjectEntryId,
         project_path: ProjectPath,
         context_buffers: Vec<ContextBuffer>,
         cx: &mut Context<Self>,
     ) {
         let id = self.next_context_id.post_inc();
-        let path = project_path.path.clone();
+        let last_path = project_path.path.clone();
         self.directories.insert(project_path, id);
 
         self.context
             .push(AssistantContext::Directory(DirectoryContext {
                 id,
                 worktree,
-                path,
+                entry_id,
+                last_path,
                 context_buffers,
             }));
         cx.notify();
@@ -875,6 +885,7 @@ pub fn refresh_context_store_text(
         let task = maybe!({
             match context {
                 AssistantContext::File(file_context) => {
+                    // TODO: Should refresh if the path has changed, as it's in the text.
                     if changed_buffers.is_empty()
                         || changed_buffers.contains(&file_context.context_buffer.buffer)
                     {
@@ -883,8 +894,9 @@ pub fn refresh_context_store_text(
                     }
                 }
                 AssistantContext::Directory(directory_context) => {
-                    let directory_path = directory_context.project_path(cx);
-                    let should_refresh = changed_buffers.is_empty()
+                    let directory_path = directory_context.project_path(cx)?;
+                    let should_refresh = directory_path.path != directory_context.last_path
+                        || changed_buffers.is_empty()
                         || changed_buffers.iter().any(|buffer| {
                             let Some(buffer_path) = buffer.read(cx).project_path(cx) else {
                                 return false;
@@ -894,10 +906,16 @@ pub fn refresh_context_store_text(
 
                     if should_refresh {
                         let context_store = context_store.clone();
-                        return refresh_directory_text(context_store, directory_context, cx);
+                        return refresh_directory_text(
+                            context_store,
+                            directory_context,
+                            directory_path,
+                            cx,
+                        );
                     }
                 }
                 AssistantContext::Symbol(symbol_context) => {
+                    // TODO: Should refresh if the path has changed, as it's in the text.
                     if changed_buffers.is_empty()
                         || changed_buffers.contains(&symbol_context.context_symbol.buffer)
                     {
@@ -906,6 +924,7 @@ pub fn refresh_context_store_text(
                     }
                 }
                 AssistantContext::Excerpt(excerpt_context) => {
+                    // TODO: Should refresh if the path has changed, as it's in the text.
                     if changed_buffers.is_empty()
                         || changed_buffers.contains(&excerpt_context.context_buffer.buffer)
                     {
@@ -965,6 +984,7 @@ fn refresh_file_text(
 fn refresh_directory_text(
     context_store: Entity<ContextStore>,
     directory_context: &DirectoryContext,
+    directory_path: ProjectPath,
     cx: &App,
 ) -> Option<Task<()>> {
     let mut stale = false;
@@ -989,7 +1009,8 @@ fn refresh_directory_text(
 
     let id = directory_context.id;
     let worktree = directory_context.worktree.clone();
-    let path = directory_context.path.clone();
+    let entry_id = directory_context.entry_id;
+    let last_path = directory_path.path;
     Some(cx.spawn(async move |cx| {
         let context_buffers = context_buffers.await;
         context_store
@@ -997,7 +1018,8 @@ fn refresh_directory_text(
                 let new_directory_context = DirectoryContext {
                     id,
                     worktree,
-                    path,
+                    entry_id,
+                    last_path,
                     context_buffers,
                 };
                 context_store.replace_context(AssistantContext::Directory(new_directory_context));

crates/agent/src/ui/context_pill.rs 🔗

@@ -264,10 +264,14 @@ impl AddedContext {
             }
 
             AssistantContext::Directory(directory_context) => {
-                let full_path = directory_context
-                    .worktree
-                    .read(cx)
-                    .full_path(&directory_context.path);
+                let worktree = directory_context.worktree.read(cx);
+                // If the directory no longer exists, use its last known path.
+                let full_path = worktree
+                    .entry_for_id(directory_context.entry_id)
+                    .map_or_else(
+                        || directory_context.last_path.clone(),
+                        |entry| worktree.full_path(&entry.path).into(),
+                    );
                 let full_path_string: SharedString =
                     full_path.to_string_lossy().into_owned().into();
                 let name = full_path

crates/project_panel/src/project_panel.rs 🔗

@@ -331,15 +331,19 @@ impl ProjectPanel {
             cx.subscribe(&project, |this, project, event, cx| match event {
                 project::Event::ActiveEntryChanged(Some(entry_id)) => {
                     if ProjectPanelSettings::get_global(cx).auto_reveal_entries {
-                        this.reveal_entry(project.clone(), *entry_id, true, cx);
+                        this.reveal_entry(project.clone(), *entry_id, true, cx).ok();
                     }
                 }
                 project::Event::ActiveEntryChanged(None) => {
                     this.marked_entries.clear();
                 }
                 project::Event::RevealInProjectPanel(entry_id) => {
-                    this.reveal_entry(project.clone(), *entry_id, false, cx);
-                    cx.emit(PanelEvent::Activate);
+                    if let Some(()) = this
+                        .reveal_entry(project.clone(), *entry_id, false, cx)
+                        .log_err()
+                    {
+                        cx.emit(PanelEvent::Activate);
+                    }
                 }
                 project::Event::ActivateProjectPanel => {
                     cx.emit(PanelEvent::Activate);
@@ -4422,7 +4426,7 @@ impl ProjectPanel {
         entry_id: ProjectEntryId,
         skip_ignored: bool,
         cx: &mut Context<Self>,
-    ) {
+    ) -> Result<()> {
         if let Some(worktree) = project.read(cx).worktree_for_entry(entry_id, cx) {
             let worktree = worktree.read(cx);
             if skip_ignored
@@ -4430,7 +4434,9 @@ impl ProjectPanel {
                     .entry_for_id(entry_id)
                     .map_or(true, |entry| entry.is_ignored && !entry.is_always_included)
             {
-                return;
+                return Err(anyhow!(
+                    "can't reveal an ignored entry in the project panel"
+                ));
             }
 
             let worktree_id = worktree.id();
@@ -4443,6 +4449,11 @@ impl ProjectPanel {
             });
             self.autoscroll(cx);
             cx.notify();
+            Ok(())
+        } else {
+            Err(anyhow!(
+                "can't reveal a non-existent entry in the project panel"
+            ))
         }
     }