Show "Restore checkpoint" more aggressively

This commit is contained in:
Conrad Irwin 2026-04-24 12:41:51 -06:00
parent 8334374398
commit 996918d4e1
14 changed files with 275 additions and 56 deletions

View file

@ -52,6 +52,8 @@ impl std::error::Error for MaxOutputTokensError {}
/// Key used in ACP ToolCall meta to store the tool's programmatic name.
/// This is a workaround since ACP's ToolCall doesn't have a dedicated name field.
pub const TOOL_NAME_META_KEY: &str = "tool_name";
/// Key used in ACP ToolCall meta to indicate whether the tool may modify project state.
pub const MAY_MODIFY_PROJECT_STATE_META_KEY: &str = "may_modify_project_state";
/// Helper to extract tool name from ACP meta
pub fn tool_name_from_meta(meta: &Option<acp::Meta>) -> Option<SharedString> {
@ -61,9 +63,23 @@ pub fn tool_name_from_meta(meta: &Option<acp::Meta>) -> Option<SharedString> {
.map(|s| SharedString::from(s.to_owned()))
}
/// Helper to create meta with tool name
pub fn meta_with_tool_name(tool_name: &str) -> acp::Meta {
acp::Meta::from_iter([(TOOL_NAME_META_KEY.into(), tool_name.into())])
/// Helper to extract whether a tool may modify project state from ACP meta.
pub fn may_modify_project_state_from_meta(meta: &Option<acp::Meta>) -> bool {
meta.as_ref()
.and_then(|m| m.get(MAY_MODIFY_PROJECT_STATE_META_KEY))
.and_then(|v| v.as_bool())
.unwrap_or(false)
}
/// Helper to create meta with tool information.
pub fn meta_with_tool_info(tool_name: &str, may_modify_project_state: bool) -> acp::Meta {
acp::Meta::from_iter([
(TOOL_NAME_META_KEY.into(), tool_name.into()),
(
MAY_MODIFY_PROJECT_STATE_META_KEY.into(),
may_modify_project_state.into(),
),
])
}
/// Key used in ACP ToolCall meta to store the session id and message indexes
@ -100,6 +116,7 @@ pub struct UserMessage {
pub struct Checkpoint {
git_checkpoint: GitStoreCheckpoint,
pub show: bool,
pub show_pending: bool,
}
impl UserMessage {
@ -256,6 +273,7 @@ pub struct ToolCall {
pub raw_input_markdown: Option<Entity<Markdown>>,
pub raw_output: Option<serde_json::Value>,
pub tool_name: Option<SharedString>,
pub may_modify_project_state: bool,
pub subagent_session_info: Option<SubagentSessionInfo>,
}
@ -296,6 +314,7 @@ impl ToolCall {
.and_then(|input| markdown_for_raw_output(input, &language_registry, cx));
let tool_name = tool_name_from_meta(&tool_call.meta);
let may_modify_project_state = may_modify_project_state_from_meta(&tool_call.meta);
let subagent_session_info = subagent_session_info_from_meta(&tool_call.meta);
@ -312,6 +331,7 @@ impl ToolCall {
raw_input_markdown,
raw_output: tool_call.raw_output,
tool_name,
may_modify_project_state,
subagent_session_info,
};
Ok(result)
@ -345,6 +365,14 @@ impl ToolCall {
self.status = status.into();
}
if let Some(tool_name) = tool_name_from_meta(&meta) {
self.tool_name = Some(tool_name);
}
if may_modify_project_state_from_meta(&meta) {
self.may_modify_project_state = true;
}
if let Some(subagent_session_info) = subagent_session_info_from_meta(&meta) {
self.subagent_session_info = Some(subagent_session_info);
}
@ -1392,6 +1420,24 @@ impl AcpThread {
false
}
pub fn has_project_mutating_tool_calls_since_last_user_message(&self) -> bool {
for entry in self.entries.iter().rev() {
match entry {
AgentThreadEntry::UserMessage(_) => return false,
AgentThreadEntry::ToolCall(call)
if call.may_modify_project_state || call.diffs().next().is_some() =>
{
return true;
}
AgentThreadEntry::ToolCall(_)
| AgentThreadEntry::AssistantMessage(_)
| AgentThreadEntry::CompletedPlan(_) => {}
}
}
false
}
pub fn has_in_progress_tool_calls(&self) -> bool {
for entry in self.entries.iter().rev() {
match entry {
@ -1835,40 +1881,53 @@ impl AcpThread {
raw_input_markdown: None,
raw_output: None,
tool_name: None,
may_modify_project_state: false,
subagent_session_info: None,
};
self.push_entry(AgentThreadEntry::ToolCall(failed_tool_call), cx);
return Ok(());
}
};
let AgentThreadEntry::ToolCall(call) = &mut self.entries[ix] else {
unreachable!()
};
let mut location_update_tool_call_id = None;
let show_checkpoint_pending = {
let AgentThreadEntry::ToolCall(call) = &mut self.entries[ix] else {
unreachable!()
};
match update {
ToolCallUpdate::UpdateFields(update) => {
let location_updated = update.fields.locations.is_some();
call.update_fields(
update.fields,
update.meta,
languages,
path_style,
&self.terminals,
cx,
)?;
if location_updated {
self.resolve_locations(update.tool_call_id, cx);
match update {
ToolCallUpdate::UpdateFields(update) => {
if update.fields.locations.is_some() {
location_update_tool_call_id = Some(update.tool_call_id.clone());
}
call.update_fields(
update.fields,
update.meta,
languages,
path_style,
&self.terminals,
cx,
)?;
}
ToolCallUpdate::UpdateDiff(update) => {
call.content.clear();
call.content.push(ToolCallContent::Diff(update.diff));
}
ToolCallUpdate::UpdateTerminal(update) => {
call.content.clear();
call.content
.push(ToolCallContent::Terminal(update.terminal));
}
}
ToolCallUpdate::UpdateDiff(update) => {
call.content.clear();
call.content.push(ToolCallContent::Diff(update.diff));
}
ToolCallUpdate::UpdateTerminal(update) => {
call.content.clear();
call.content
.push(ToolCallContent::Terminal(update.terminal));
}
call.may_modify_project_state || call.diffs().next().is_some()
};
if let Some(tool_call_id) = location_update_tool_call_id {
self.resolve_locations(tool_call_id, cx);
}
if show_checkpoint_pending {
self.show_last_checkpoint_pending(cx);
}
cx.emit(AcpThreadEvent::EntryUpdated(ix));
@ -1916,19 +1975,27 @@ impl AcpThread {
}
if let Some(ix) = self.index_for_tool_call(&id) {
let AgentThreadEntry::ToolCall(call) = &mut self.entries[ix] else {
unreachable!()
let show_checkpoint_pending = {
let AgentThreadEntry::ToolCall(call) = &mut self.entries[ix] else {
unreachable!()
};
call.update_fields(
update.fields,
update.meta,
language_registry,
path_style,
&self.terminals,
cx,
)?;
call.status = status;
call.may_modify_project_state || call.diffs().next().is_some()
};
call.update_fields(
update.fields,
update.meta,
language_registry,
path_style,
&self.terminals,
cx,
)?;
call.status = status;
if show_checkpoint_pending {
self.show_last_checkpoint_pending(cx);
}
cx.emit(AcpThreadEvent::EntryUpdated(ix));
} else {
@ -1940,7 +2007,12 @@ impl AcpThread {
&self.terminals,
cx,
)?;
let show_checkpoint_pending =
call.may_modify_project_state || call.diffs().next().is_some();
self.push_entry(AgentThreadEntry::ToolCall(call), cx);
if show_checkpoint_pending {
self.show_last_checkpoint_pending(cx);
}
};
self.resolve_locations(id, cx);
@ -2229,6 +2301,7 @@ impl AcpThread {
message.checkpoint = old_checkpoint.map(|git_checkpoint| Checkpoint {
git_checkpoint,
show: false,
show_pending: false,
});
}
this.connection.prompt(message_id, request, cx)
@ -2543,6 +2616,7 @@ impl AcpThread {
if let Some((ix, message)) = this.user_message_mut(&user_message_id) {
if let Some(checkpoint) = message.checkpoint.as_mut() {
checkpoint.show = !equal;
checkpoint.show_pending = false;
cx.emit(AcpThreadEvent::EntryUpdated(ix));
}
}
@ -2552,6 +2626,16 @@ impl AcpThread {
})
}
fn show_last_checkpoint_pending(&mut self, cx: &mut Context<Self>) {
if let Some((ix, message)) = self.last_user_message()
&& let Some(checkpoint) = message.checkpoint.as_mut()
&& !checkpoint.show_pending
{
checkpoint.show_pending = true;
cx.emit(AcpThreadEvent::EntryUpdated(ix));
}
}
fn last_user_message(&mut self) -> Option<(usize, &mut UserMessage)> {
self.entries
.iter_mut()
@ -3955,6 +4039,60 @@ mod tests {
.unwrap();
assert!(cx.read(|cx| !thread.read(cx).has_pending_edit_tool_calls()));
assert!(cx.read(|cx| {
thread
.read(cx)
.has_project_mutating_tool_calls_since_last_user_message()
}));
}
#[gpui::test]
async fn test_mutating_tool_calls_since_last_user_message_uses_metadata(
cx: &mut TestAppContext,
) {
init_test(cx);
let fs = FakeFs::new(cx.background_executor.clone());
fs.insert_tree(path!("/test"), json!({})).await;
let project = Project::test(fs, [path!("/test").as_ref()], cx).await;
let connection = Rc::new(FakeAgentConnection::new().on_user_message({
move |_, thread, mut cx| {
async move {
thread
.update(&mut cx, |thread, cx| {
thread.handle_session_update(
acp::SessionUpdate::ToolCall(
acp::ToolCall::new("test", "Create directory")
.kind(acp::ToolKind::Read)
.meta(meta_with_tool_info("create_directory", true)),
),
cx,
)
})
.unwrap()
.unwrap();
Ok(acp::PromptResponse::new(acp::StopReason::EndTurn))
}
.boxed_local()
}
}));
let thread = cx
.update(|cx| {
connection.new_session(project, PathList::new(&[Path::new(path!("/test"))]), cx)
})
.await
.unwrap();
cx.update(|cx| thread.update(cx, |thread, cx| thread.send(vec!["Hi".into()], cx)))
.await
.unwrap();
assert!(cx.read(|cx| {
thread
.read(cx)
.has_project_mutating_tool_calls_since_last_user_message()
}));
}
#[gpui::test(iterations = 10)]

View file

@ -3470,7 +3470,10 @@ async fn test_tool_updates_to_completion(cx: &mut TestAppContext) {
tool_call,
acp::ToolCall::new("1", "Echo")
.raw_input(json!({}))
.meta(acp::Meta::from_iter([("tool_name".into(), "echo".into())]))
.meta(acp::Meta::from_iter([
("tool_name".into(), "echo".into()),
("may_modify_project_state".into(), false.into()),
]))
);
let update = expect_tool_call_update_fields(&mut events).await;
assert_eq!(
@ -3566,10 +3569,10 @@ async fn test_update_plan_tool_updates_thread_events(cx: &mut TestAppContext) {
}
]
}))
.meta(acp::Meta::from_iter([(
"tool_name".into(),
"update_plan".into()
)]))
.meta(acp::Meta::from_iter([
("tool_name".into(), "update_plan".into()),
("may_modify_project_state".into(), false.into()),
]))
);
let update = expect_tool_call_update_fields(&mut events).await;

View file

@ -1226,6 +1226,7 @@ impl Thread {
&tool_use.name,
title,
kind,
tool.may_modify_project_state(),
tool_use.input.clone(),
);
@ -2322,12 +2323,20 @@ impl Thread {
let tool = self.tool(tool_use.name.as_ref());
let mut title = SharedString::from(&tool_use.name);
let mut kind = acp::ToolKind::Other;
let mut may_modify_project_state = false;
if let Some(tool) = tool.as_ref() {
title = tool.initial_title(tool_use.input.clone(), cx);
kind = tool.kind();
may_modify_project_state = tool.may_modify_project_state();
}
self.send_or_update_tool_use(&tool_use, title, kind, event_stream);
self.send_or_update_tool_use(
&tool_use,
title,
kind,
may_modify_project_state,
event_stream,
);
let Some(tool) = tool else {
let content = format!("No tool named {} exists", tool_use.name);
@ -2464,6 +2473,7 @@ impl Thread {
&tool_use,
SharedString::from(&tool_use.name),
acp::ToolKind::Other,
false,
event_stream,
);
@ -2511,6 +2521,7 @@ impl Thread {
tool_use: &LanguageModelToolUse,
title: SharedString,
kind: acp::ToolKind,
may_modify_project_state: bool,
event_stream: &ThreadEventStream,
) {
// Ensure the last message ends in the current tool use
@ -2532,6 +2543,7 @@ impl Thread {
&tool_use.name,
title,
kind,
may_modify_project_state,
tool_use.input.clone(),
);
last_message
@ -3336,6 +3348,11 @@ where
fn kind() -> acp::ToolKind;
/// Returns whether the tool may modify project state.
fn may_modify_project_state() -> bool {
false
}
/// The initial tool title to display. Can be updated during the tool run.
fn initial_title(
&self,
@ -3411,6 +3428,9 @@ pub trait AnyAgentTool {
fn name(&self) -> SharedString;
fn description(&self) -> SharedString;
fn kind(&self) -> acp::ToolKind;
fn may_modify_project_state(&self) -> bool {
false
}
fn initial_title(&self, input: serde_json::Value, _cx: &mut App) -> SharedString;
fn input_schema(&self, format: LanguageModelToolSchemaFormat) -> Result<serde_json::Value>;
fn supports_input_streaming(&self) -> bool {
@ -3451,6 +3471,10 @@ where
T::kind()
}
fn may_modify_project_state(&self) -> bool {
T::may_modify_project_state()
}
fn supports_input_streaming(&self) -> bool {
T::supports_input_streaming()
}
@ -3542,6 +3566,7 @@ impl ThreadEventStream {
tool_name: &str,
title: SharedString,
kind: acp::ToolKind,
may_modify_project_state: bool,
input: serde_json::Value,
) {
self.0
@ -3550,6 +3575,7 @@ impl ThreadEventStream {
tool_name,
title.to_string(),
kind,
may_modify_project_state,
input,
))))
.ok();
@ -3560,12 +3586,16 @@ impl ThreadEventStream {
tool_name: &str,
title: String,
kind: acp::ToolKind,
may_modify_project_state: bool,
input: serde_json::Value,
) -> acp::ToolCall {
acp::ToolCall::new(id.to_string(), title)
.kind(kind)
.raw_input(input)
.meta(acp_thread::meta_with_tool_name(tool_name))
.meta(acp_thread::meta_with_tool_info(
tool_name,
may_modify_project_state,
))
}
fn update_tool_call_fields(

View file

@ -66,6 +66,10 @@ impl AgentTool for CopyPathTool {
acp::ToolKind::Move
}
fn may_modify_project_state() -> bool {
true
}
fn initial_title(
&self,
input: Result<Self::Input, serde_json::Value>,

View file

@ -57,6 +57,10 @@ impl AgentTool for CreateDirectoryTool {
acp::ToolKind::Read
}
fn may_modify_project_state() -> bool {
true
}
fn initial_title(
&self,
input: Result<Self::Input, serde_json::Value>,

View file

@ -60,6 +60,10 @@ impl AgentTool for DeletePathTool {
acp::ToolKind::Delete
}
fn may_modify_project_state() -> bool {
true
}
fn initial_title(
&self,
input: Result<Self::Input, serde_json::Value>,

View file

@ -185,6 +185,10 @@ impl AgentTool for EditFileTool {
acp::ToolKind::Edit
}
fn may_modify_project_state() -> bool {
true
}
fn initial_title(
&self,
input: Result<Self::Input, serde_json::Value>,

View file

@ -67,6 +67,10 @@ impl AgentTool for MovePathTool {
acp::ToolKind::Move
}
fn may_modify_project_state() -> bool {
true
}
fn initial_title(
&self,
input: Result<Self::Input, serde_json::Value>,

View file

@ -54,6 +54,10 @@ impl AgentTool for RestoreFileFromDiskTool {
acp::ToolKind::Other
}
fn may_modify_project_state() -> bool {
true
}
fn initial_title(
&self,
input: Result<Self::Input, serde_json::Value>,

View file

@ -51,6 +51,10 @@ impl AgentTool for SaveFileTool {
acp::ToolKind::Other
}
fn may_modify_project_state() -> bool {
true
}
fn initial_title(
&self,
input: Result<Self::Input, serde_json::Value>,

View file

@ -476,6 +476,10 @@ impl AgentTool for StreamingEditFileTool {
acp::ToolKind::Edit
}
fn may_modify_project_state() -> bool {
true
}
fn initial_title(
&self,
input: Result<Self::Input, serde_json::Value>,

View file

@ -70,6 +70,10 @@ impl AgentTool for TerminalTool {
acp::ToolKind::Execute
}
fn may_modify_project_state() -> bool {
true
}
fn initial_title(
&self,
input: Result<Self::Input, serde_json::Value>,

View file

@ -4534,13 +4534,25 @@ impl ThreadView {
let editor_focus = editor.focus_handle(cx).is_focused(window);
let focus_border = cx.theme().colors().border_focused;
let has_checkpoint_button = message
let thread = self.thread.read(cx);
let has_final_checkpoint_button = message
.checkpoint
.as_ref()
.is_some_and(|checkpoint| checkpoint.show);
let has_pending_checkpoint_button = message
.checkpoint
.as_ref()
.is_some_and(|checkpoint| checkpoint.show_pending);
let is_last_user_message = !thread
.entries()
.iter()
.skip(entry_ix + 1)
.any(|entry| matches!(entry, AgentThreadEntry::UserMessage(_)));
let has_checkpoint_button = has_final_checkpoint_button
|| (is_last_user_message && has_pending_checkpoint_button);
let is_subagent = self.is_subagent();
let can_rewind = self.thread.read(cx).supports_truncate(cx);
let can_rewind = thread.supports_truncate(cx);
let is_editable = can_rewind && message.id.is_some() && !is_subagent;
let agent_name = if is_subagent {
"subagents".into()

View file

@ -236,13 +236,13 @@ function sign_app_binaries() {
open "$app_path"
fi
if [[ "$target_dir" = "debug" ]]; then
echo "Debug build detected - skipping DMG creation and signing"
if [ "$local_install" = false ]; then
echo "Created application bundle:"
echo "$app_path"
fi
else
# if [[ "$target_dir" = "debug" ]]; then
# echo "Debug build detected - skipping DMG creation and signing"
# if [ "$local_install" = false ]; then
# echo "Created application bundle:"
# echo "$app_path"
# fi
# else
dmg_target_directory="target/${target_triple}/${target_dir}"
dmg_source_directory="${dmg_target_directory}/dmg"
dmg_file_path="${dmg_target_directory}/Zed-${arch_suffix}.dmg"
@ -280,7 +280,7 @@ function sign_app_binaries() {
if [ "$open_result" = true ]; then
open $dmg_target_directory
fi
fi
# fi
}
function sign_binary() {