From 41a0c63c2317a97fd0acbb00c6ad6c9ccad38050 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Mon, 2 Mar 2026 19:00:12 +0100 Subject: [PATCH] agent: Add linked action log support for subagent threads (#50500) 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 --- crates/action_log/src/action_log.rs | 364 +++++++++++++++++++++++++++- crates/agent/src/thread.rs | 27 ++- 2 files changed, 378 insertions(+), 13 deletions(-) diff --git a/crates/action_log/src/action_log.rs b/crates/action_log/src/action_log.rs index 1157d8d6f881ecb33df8104dd4be04bd9d846b5e..5f8a639c0559c10546fc5640dc240aeba9dde487 100644 --- a/crates/action_log/src/action_log.rs +++ b/crates/action_log/src/action_log.rs @@ -48,6 +48,10 @@ pub struct ActionLog { tracked_buffers: BTreeMap, TrackedBuffer>, /// The project this action log is associated with project: Entity, + /// 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>, /// Stores undo information for the most recent reject operation last_reject_undo: Option, } @@ -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) -> Self { + self.linked_action_log = Some(linked_action_log); + self + } + pub fn project(&self) -> &Entity { &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, cx: &mut Context) { + 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, cx: &mut Context) { + 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, cx: &mut Context) { + 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, cx: &mut Context) { + 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, &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> { 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::>() + }); + let parent_stale = cx.read(|cx| { + parent_log + .read(cx) + .stale_buffers(cx) + .cloned() + .collect::>() + }); + 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::>() + }); + let parent_stale = cx.read(|cx| { + parent_log + .read(cx) + .stale_buffers(cx) + .cloned() + .collect::>() + }); + 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, diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 4560671cc8ad84fb43f07ee711aa72f053e4a2a9..2a9e8d3270cc6ae6b95e28dbb2c06370980bf028 100644 --- a/crates/agent/src/thread.rs +++ b/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, model: Option>, cx: &mut Context, + ) -> 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_context: Entity, + context_server_registry: Entity, + templates: Arc, + model: Option>, + action_log: Entity, + cx: &mut Context, ) -> 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 {