agent: Add linked action log support for subagent threads (#50500)

Ben Brandt and Bennet Bo Fenner created

Subagents now forward buffer reads/writes/edits to a parent action log,
allowing the parent's review experience to track all file changes made
by subagents alongside its own.

Release Notes:

- N/A

---------

Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>

Change summary

crates/action_log/src/action_log.rs | 364 ++++++++++++++++++++++++++++++
crates/agent/src/thread.rs          |  27 ++
2 files changed, 378 insertions(+), 13 deletions(-)

Detailed changes

crates/action_log/src/action_log.rs 🔗

@@ -48,6 +48,10 @@ pub struct ActionLog {
     tracked_buffers: BTreeMap<Entity<Buffer>, TrackedBuffer>,
     /// The project this action log is associated with
     project: Entity<Project>,
+    /// An action log to forward all public methods to
+    /// Useful in cases like subagents, where we want to track individual diffs for this subagent,
+    /// but also want to associate the reads/writes with a parent review experience
+    linked_action_log: Option<Entity<ActionLog>>,
     /// Stores undo information for the most recent reject operation
     last_reject_undo: Option<LastRejectUndo>,
 }
@@ -58,10 +62,16 @@ impl ActionLog {
         Self {
             tracked_buffers: BTreeMap::default(),
             project,
+            linked_action_log: None,
             last_reject_undo: None,
         }
     }
 
+    pub fn with_linked_action_log(mut self, linked_action_log: Entity<ActionLog>) -> Self {
+        self.linked_action_log = Some(linked_action_log);
+        self
+    }
+
     pub fn project(&self) -> &Entity<Project> {
         &self.project
     }
@@ -496,16 +506,25 @@ impl ActionLog {
 
     /// Track a buffer as read by agent, so we can notify the model about user edits.
     pub fn buffer_read(&mut self, buffer: Entity<Buffer>, cx: &mut Context<Self>) {
+        if let Some(linked_action_log) = &mut self.linked_action_log {
+            linked_action_log.update(cx, |log, cx| log.buffer_read(buffer.clone(), cx));
+        }
         self.track_buffer_internal(buffer, false, cx);
     }
 
     /// Mark a buffer as created by agent, so we can refresh it in the context
     pub fn buffer_created(&mut self, buffer: Entity<Buffer>, cx: &mut Context<Self>) {
+        if let Some(linked_action_log) = &mut self.linked_action_log {
+            linked_action_log.update(cx, |log, cx| log.buffer_created(buffer.clone(), cx));
+        }
         self.track_buffer_internal(buffer, true, cx);
     }
 
     /// Mark a buffer as edited by agent, so we can refresh it in the context
     pub fn buffer_edited(&mut self, buffer: Entity<Buffer>, cx: &mut Context<Self>) {
+        if let Some(linked_action_log) = &mut self.linked_action_log {
+            linked_action_log.update(cx, |log, cx| log.buffer_edited(buffer.clone(), cx));
+        }
         let new_version = buffer.read(cx).version();
         let tracked_buffer = self.track_buffer_internal(buffer, false, cx);
         if let TrackedBufferStatus::Deleted = tracked_buffer.status {
@@ -517,6 +536,7 @@ impl ActionLog {
     }
 
     pub fn will_delete_buffer(&mut self, buffer: Entity<Buffer>, cx: &mut Context<Self>) {
+        let has_linked_action_log = self.linked_action_log.is_some();
         let tracked_buffer = self.track_buffer_internal(buffer.clone(), false, cx);
         match tracked_buffer.status {
             TrackedBufferStatus::Created { .. } => {
@@ -524,12 +544,24 @@ impl ActionLog {
                 cx.notify();
             }
             TrackedBufferStatus::Modified => {
-                buffer.update(cx, |buffer, cx| buffer.set_text("", cx));
                 tracked_buffer.status = TrackedBufferStatus::Deleted;
-                tracked_buffer.schedule_diff_update(ChangeAuthor::Agent, cx);
+                if !has_linked_action_log {
+                    buffer.update(cx, |buffer, cx| buffer.set_text("", cx));
+                    tracked_buffer.schedule_diff_update(ChangeAuthor::Agent, cx);
+                }
             }
+
             TrackedBufferStatus::Deleted => {}
         }
+
+        if let Some(linked_action_log) = &mut self.linked_action_log {
+            linked_action_log.update(cx, |log, cx| log.will_delete_buffer(buffer.clone(), cx));
+        }
+
+        if has_linked_action_log && let Some(tracked_buffer) = self.tracked_buffers.get(&buffer) {
+            tracked_buffer.schedule_diff_update(ChangeAuthor::Agent, cx);
+        }
+
         cx.notify();
     }
 
@@ -914,15 +946,6 @@ impl ActionLog {
             .collect()
     }
 
-    /// Returns all tracked buffers for debugging purposes
-    #[cfg(any(test, feature = "test-support"))]
-    pub fn tracked_buffers_for_debug(
-        &self,
-        _cx: &App,
-    ) -> impl Iterator<Item = (&Entity<Buffer>, &TrackedBuffer)> {
-        self.tracked_buffers.iter()
-    }
-
     /// Iterate over buffers changed since last read or edited by the model
     pub fn stale_buffers<'a>(&'a self, cx: &'a App) -> impl Iterator<Item = &'a Entity<Buffer>> {
         self.tracked_buffers
@@ -2634,6 +2657,325 @@ mod tests {
         assert!(!action_log.read_with(cx, |log, _| log.has_pending_undo()));
     }
 
+    #[gpui::test]
+    async fn test_linked_action_log_buffer_read(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(path!("/dir"), json!({"file": "hello world"}))
+            .await;
+        let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await;
+        let parent_log = cx.new(|_| ActionLog::new(project.clone()));
+        let child_log =
+            cx.new(|_| ActionLog::new(project.clone()).with_linked_action_log(parent_log.clone()));
+
+        let file_path = project
+            .read_with(cx, |project, cx| project.find_project_path("dir/file", cx))
+            .unwrap();
+        let buffer = project
+            .update(cx, |project, cx| project.open_buffer(file_path, cx))
+            .await
+            .unwrap();
+
+        cx.update(|cx| {
+            child_log.update(cx, |log, cx| log.buffer_read(buffer.clone(), cx));
+        });
+
+        // Neither log considers the buffer stale immediately after reading it.
+        let child_stale = cx.read(|cx| {
+            child_log
+                .read(cx)
+                .stale_buffers(cx)
+                .cloned()
+                .collect::<Vec<_>>()
+        });
+        let parent_stale = cx.read(|cx| {
+            parent_log
+                .read(cx)
+                .stale_buffers(cx)
+                .cloned()
+                .collect::<Vec<_>>()
+        });
+        assert!(child_stale.is_empty());
+        assert!(parent_stale.is_empty());
+
+        // Simulate a user edit after the agent read the file.
+        cx.update(|cx| {
+            buffer.update(cx, |buffer, cx| {
+                buffer.edit([(0..5, "goodbye")], None, cx).unwrap();
+            });
+        });
+        cx.run_until_parked();
+
+        // Both child and parent should see the buffer as stale because both tracked
+        // it at the pre-edit version via buffer_read forwarding.
+        let child_stale = cx.read(|cx| {
+            child_log
+                .read(cx)
+                .stale_buffers(cx)
+                .cloned()
+                .collect::<Vec<_>>()
+        });
+        let parent_stale = cx.read(|cx| {
+            parent_log
+                .read(cx)
+                .stale_buffers(cx)
+                .cloned()
+                .collect::<Vec<_>>()
+        });
+        assert_eq!(child_stale, vec![buffer.clone()]);
+        assert_eq!(parent_stale, vec![buffer]);
+    }
+
+    #[gpui::test]
+    async fn test_linked_action_log_buffer_edited(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(path!("/dir"), json!({"file": "abc\ndef\nghi"}))
+            .await;
+        let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await;
+        let parent_log = cx.new(|_| ActionLog::new(project.clone()));
+        let child_log =
+            cx.new(|_| ActionLog::new(project.clone()).with_linked_action_log(parent_log.clone()));
+
+        let file_path = project
+            .read_with(cx, |project, cx| project.find_project_path("dir/file", cx))
+            .unwrap();
+        let buffer = project
+            .update(cx, |project, cx| project.open_buffer(file_path, cx))
+            .await
+            .unwrap();
+
+        cx.update(|cx| {
+            child_log.update(cx, |log, cx| log.buffer_read(buffer.clone(), cx));
+            buffer.update(cx, |buffer, cx| {
+                buffer
+                    .edit([(Point::new(1, 0)..Point::new(1, 3), "DEF")], None, cx)
+                    .unwrap();
+            });
+            child_log.update(cx, |log, cx| log.buffer_edited(buffer.clone(), cx));
+        });
+        cx.run_until_parked();
+
+        let expected_hunks = vec![(
+            buffer,
+            vec![HunkStatus {
+                range: Point::new(1, 0)..Point::new(2, 0),
+                diff_status: DiffHunkStatusKind::Modified,
+                old_text: "def\n".into(),
+            }],
+        )];
+        assert_eq!(
+            unreviewed_hunks(&child_log, cx),
+            expected_hunks,
+            "child should track the agent edit"
+        );
+        assert_eq!(
+            unreviewed_hunks(&parent_log, cx),
+            expected_hunks,
+            "parent should also track the agent edit via linked log forwarding"
+        );
+    }
+
+    #[gpui::test]
+    async fn test_linked_action_log_buffer_created(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(path!("/dir"), json!({})).await;
+        let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await;
+        let parent_log = cx.new(|_| ActionLog::new(project.clone()));
+        let child_log =
+            cx.new(|_| ActionLog::new(project.clone()).with_linked_action_log(parent_log.clone()));
+
+        let file_path = project
+            .read_with(cx, |project, cx| {
+                project.find_project_path("dir/new_file", cx)
+            })
+            .unwrap();
+        let buffer = project
+            .update(cx, |project, cx| project.open_buffer(file_path, cx))
+            .await
+            .unwrap();
+
+        cx.update(|cx| {
+            child_log.update(cx, |log, cx| log.buffer_created(buffer.clone(), cx));
+            buffer.update(cx, |buffer, cx| buffer.set_text("hello", cx));
+            child_log.update(cx, |log, cx| log.buffer_edited(buffer.clone(), cx));
+        });
+        project
+            .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx))
+            .await
+            .unwrap();
+        cx.run_until_parked();
+
+        let expected_hunks = vec![(
+            buffer.clone(),
+            vec![HunkStatus {
+                range: Point::new(0, 0)..Point::new(0, 5),
+                diff_status: DiffHunkStatusKind::Added,
+                old_text: "".into(),
+            }],
+        )];
+        assert_eq!(
+            unreviewed_hunks(&child_log, cx),
+            expected_hunks,
+            "child should track the created file"
+        );
+        assert_eq!(
+            unreviewed_hunks(&parent_log, cx),
+            expected_hunks,
+            "parent should also track the created file via linked log forwarding"
+        );
+    }
+
+    #[gpui::test]
+    async fn test_linked_action_log_will_delete_buffer(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(path!("/dir"), json!({"file": "hello\n"}))
+            .await;
+        let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await;
+        let parent_log = cx.new(|_| ActionLog::new(project.clone()));
+        let child_log =
+            cx.new(|_| ActionLog::new(project.clone()).with_linked_action_log(parent_log.clone()));
+
+        let file_path = project
+            .read_with(cx, |project, cx| project.find_project_path("dir/file", cx))
+            .unwrap();
+        let buffer = project
+            .update(cx, |project, cx| project.open_buffer(file_path.clone(), cx))
+            .await
+            .unwrap();
+
+        cx.update(|cx| {
+            child_log.update(cx, |log, cx| log.will_delete_buffer(buffer.clone(), cx));
+        });
+        project
+            .update(cx, |project, cx| project.delete_file(file_path, false, cx))
+            .unwrap()
+            .await
+            .unwrap();
+        cx.run_until_parked();
+
+        let expected_hunks = vec![(
+            buffer.clone(),
+            vec![HunkStatus {
+                range: Point::new(0, 0)..Point::new(0, 0),
+                diff_status: DiffHunkStatusKind::Deleted,
+                old_text: "hello\n".into(),
+            }],
+        )];
+        assert_eq!(
+            unreviewed_hunks(&child_log, cx),
+            expected_hunks,
+            "child should track the deleted file"
+        );
+        assert_eq!(
+            unreviewed_hunks(&parent_log, cx),
+            expected_hunks,
+            "parent should also track the deleted file via linked log forwarding"
+        );
+    }
+
+    /// Simulates the subagent scenario: two child logs linked to the same parent, each
+    /// editing a different file. The parent accumulates all edits while each child
+    /// only sees its own.
+    #[gpui::test]
+    async fn test_linked_action_log_independent_tracking(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(
+            path!("/dir"),
+            json!({
+                "file_a": "content of a",
+                "file_b": "content of b",
+            }),
+        )
+        .await;
+        let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await;
+        let parent_log = cx.new(|_| ActionLog::new(project.clone()));
+        let child_log_1 =
+            cx.new(|_| ActionLog::new(project.clone()).with_linked_action_log(parent_log.clone()));
+        let child_log_2 =
+            cx.new(|_| ActionLog::new(project.clone()).with_linked_action_log(parent_log.clone()));
+
+        let file_a_path = project
+            .read_with(cx, |project, cx| {
+                project.find_project_path("dir/file_a", cx)
+            })
+            .unwrap();
+        let file_b_path = project
+            .read_with(cx, |project, cx| {
+                project.find_project_path("dir/file_b", cx)
+            })
+            .unwrap();
+        let buffer_a = project
+            .update(cx, |project, cx| project.open_buffer(file_a_path, cx))
+            .await
+            .unwrap();
+        let buffer_b = project
+            .update(cx, |project, cx| project.open_buffer(file_b_path, cx))
+            .await
+            .unwrap();
+
+        cx.update(|cx| {
+            child_log_1.update(cx, |log, cx| log.buffer_read(buffer_a.clone(), cx));
+            buffer_a.update(cx, |buffer, cx| {
+                buffer.edit([(0..0, "MODIFIED: ")], None, cx).unwrap();
+            });
+            child_log_1.update(cx, |log, cx| log.buffer_edited(buffer_a.clone(), cx));
+
+            child_log_2.update(cx, |log, cx| log.buffer_read(buffer_b.clone(), cx));
+            buffer_b.update(cx, |buffer, cx| {
+                buffer.edit([(0..0, "MODIFIED: ")], None, cx).unwrap();
+            });
+            child_log_2.update(cx, |log, cx| log.buffer_edited(buffer_b.clone(), cx));
+        });
+        cx.run_until_parked();
+
+        let child_1_changed: Vec<_> = cx.read(|cx| {
+            child_log_1
+                .read(cx)
+                .changed_buffers(cx)
+                .into_keys()
+                .collect()
+        });
+        let child_2_changed: Vec<_> = cx.read(|cx| {
+            child_log_2
+                .read(cx)
+                .changed_buffers(cx)
+                .into_keys()
+                .collect()
+        });
+        let parent_changed: Vec<_> = cx.read(|cx| {
+            parent_log
+                .read(cx)
+                .changed_buffers(cx)
+                .into_keys()
+                .collect()
+        });
+
+        assert_eq!(
+            child_1_changed,
+            vec![buffer_a.clone()],
+            "child 1 should only track file_a"
+        );
+        assert_eq!(
+            child_2_changed,
+            vec![buffer_b.clone()],
+            "child 2 should only track file_b"
+        );
+        assert_eq!(parent_changed.len(), 2, "parent should track both files");
+        assert!(
+            parent_changed.contains(&buffer_a) && parent_changed.contains(&buffer_b),
+            "parent should contain both buffer_a and buffer_b"
+        );
+    }
+
     #[derive(Debug, PartialEq)]
     struct HunkStatus {
         range: Range<Point>,

crates/agent/src/thread.rs 🔗

@@ -917,12 +917,16 @@ impl Thread {
         let context_server_registry = parent_thread.read(cx).context_server_registry.clone();
         let templates = parent_thread.read(cx).templates.clone();
         let model = parent_thread.read(cx).model().cloned();
-        let mut thread = Self::new(
+        let parent_action_log = parent_thread.read(cx).action_log().clone();
+        let action_log =
+            cx.new(|_cx| ActionLog::new(project.clone()).with_linked_action_log(parent_action_log));
+        let mut thread = Self::new_internal(
             project,
             project_context,
             context_server_registry,
             templates,
             model,
+            action_log,
             cx,
         );
         thread.subagent_context = Some(SubagentContext {
@@ -939,6 +943,26 @@ impl Thread {
         templates: Arc<Templates>,
         model: Option<Arc<dyn LanguageModel>>,
         cx: &mut Context<Self>,
+    ) -> Self {
+        Self::new_internal(
+            project.clone(),
+            project_context,
+            context_server_registry,
+            templates,
+            model,
+            cx.new(|_cx| ActionLog::new(project)),
+            cx,
+        )
+    }
+
+    fn new_internal(
+        project: Entity<Project>,
+        project_context: Entity<ProjectContext>,
+        context_server_registry: Entity<ContextServerRegistry>,
+        templates: Arc<Templates>,
+        model: Option<Arc<dyn LanguageModel>>,
+        action_log: Entity<ActionLog>,
+        cx: &mut Context<Self>,
     ) -> Self {
         let settings = AgentSettings::get_global(cx);
         let profile_id = settings.default_profile.clone();
@@ -950,7 +974,6 @@ impl Thread {
             .default_model
             .as_ref()
             .and_then(|model| model.effort.clone());
-        let action_log = cx.new(|_cx| ActionLog::new(project.clone()));
         let (prompt_capabilities_tx, prompt_capabilities_rx) =
             watch::channel(Self::prompt_capabilities(model.as_deref()));
         Self {