diff --git a/AGENTS.md b/AGENTS.md index 07f34964..e8bc4128 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -136,6 +136,7 @@ Key invariants: - Direct-provider remote telemetry records requested, started, retry-scheduled, and finished model-turn phases with explicit `llm_finished` state; root `provider_run_finished` records distinguish clean completion from failure or cancellation and mark the response stream terminal - `use_rig` and provider selection may choose request/transport details but must never choose lifecycle ownership - Direct-provider output may be projected through `ResponseStream`, but provider progress must not depend on response-stream result draining or `AfterStreamFinished` +- Direct-provider `RequestFileEdits` views must register from streaming output before provider-run completion; preprocessing results must survive delayed view registration, and `NotReady` retries must remain automatic rather than emitting a synthetic user permission decision - A clean direct-provider `ProviderRunOutcome::Completed` explicitly finalizes the conversation as `Success` after terminal output projection, even if earlier turns added tool actions; child-completion waits rely on that status - Provider actions and results must correlate by `(conversation_id, run_id, epoch, call_id)`; stale or duplicate callbacks must not advance a run - Active provider runs must checkpoint before external work, persist without credentials, normalize unsafe restored states, and reconcile command state before continuing diff --git a/app/src/ai/blocklist/action_model.rs b/app/src/ai/blocklist/action_model.rs index b4f1d7fd..445c6009 100644 --- a/app/src/ai/blocklist/action_model.rs +++ b/app/src/ai/blocklist/action_model.rs @@ -190,6 +190,54 @@ type ProviderActionCorrelation = ( ProviderToolExecutionRef, ); +type ActionExecutionKey = (AIConversationId, AIAgentActionId); + +#[derive(Default)] +struct NotReadyActionTracker { + actions: HashSet, +} + +impl NotReadyActionTracker { + fn should_retry(&self, conversation_id: AIConversationId, action_id: &AIAgentActionId) -> bool { + self.actions.contains(&(conversation_id, action_id.clone())) + } + + fn update_after_attempt( + &mut self, + conversation_id: AIConversationId, + action_id: AIAgentActionId, + reason: NotExecutedReason, + initiator: ActionExecutionInitiator, + ) { + let key = (conversation_id, action_id); + match (reason, initiator) { + (NotExecutedReason::NotReady, ActionExecutionInitiator::Automatic) => { + self.actions.insert(key); + } + (NotExecutedReason::NotReady, ActionExecutionInitiator::User) + | (NotExecutedReason::NeedsConfirmation | NotExecutedReason::WaitingOnSharer, _) => { + self.actions.remove(&key); + } + } + } + + fn clear(&mut self, conversation_id: AIConversationId, action_id: &AIAgentActionId) { + self.actions.remove(&(conversation_id, action_id.clone())); + } +} + +#[derive(Clone, Copy)] +enum ActionExecutionInitiator { + Automatic, + User, +} + +impl ActionExecutionInitiator { + fn is_user_initiated(self) -> bool { + matches!(self, Self::User) + } +} + impl RunningActions { fn new(phase: RunningActionPhase, action_id: AIAgentActionId) -> Self { Self { @@ -678,6 +726,9 @@ pub struct BlocklistAIActionModel { /// Permission-card rejections whose cancelled action result must not emit a second provider event. denied_permissions: HashSet<(AIConversationId, AIAgentActionId)>, + /// Actions parked because their executor-specific UI or state was not ready yet. + not_ready_actions: NotReadyActionTracker, + /// Durable provider work identity for actions owned by an active provider run. provider_tool_executions: HashMap<(AIConversationId, AIAgentActionId), ProviderToolExecutionRef>, @@ -771,6 +822,7 @@ impl BlocklistAIActionModel { running_actions: Default::default(), action_order: Default::default(), denied_permissions: Default::default(), + not_ready_actions: Default::default(), provider_tool_executions: Default::default(), terminal_view_id, pending_preprocessed_actions: Default::default(), @@ -975,9 +1027,12 @@ impl BlocklistAIActionModel { } } - let Some(result) = - self.start_pending_action_by_id(&front_action.id, conversation_id, false, ctx) - else { + let Some(result) = self.start_pending_action_by_id( + &front_action.id, + conversation_id, + ActionExecutionInitiator::Automatic, + ctx, + ) else { log::info!("[tool-debug] try_to_execute_available_actions: start_pending_action_by_id returned None (blocked)"); return; }; @@ -1290,7 +1345,12 @@ impl BlocklistAIActionModel { }; if self - .start_pending_action_by_id(&pending_action_id, conversation_id, true, ctx) + .start_pending_action_by_id( + &pending_action_id, + conversation_id, + ActionExecutionInitiator::User, + ctx, + ) .is_some_and(|result| matches!(result, StartedAction::Sync)) { self.try_to_execute_available_actions(conversation_id, ctx); @@ -1305,7 +1365,39 @@ impl BlocklistAIActionModel { ctx: &mut ModelContext, ) { if self - .start_pending_action_by_id(action_id, conversation_id, true, ctx) + .start_pending_action_by_id( + action_id, + conversation_id, + ActionExecutionInitiator::User, + ctx, + ) + .is_some_and(|result| matches!(result, StartedAction::Sync)) + { + self.try_to_execute_available_actions(conversation_id, ctx); + } + } + + /// Retries an action only when a prior automatic attempt found its executor not ready. + pub(super) fn retry_not_ready_action( + &mut self, + action_id: &AIAgentActionId, + conversation_id: AIConversationId, + ctx: &mut ModelContext, + ) { + if !self + .not_ready_actions + .should_retry(conversation_id, action_id) + { + return; + } + + if self + .start_pending_action_by_id( + action_id, + conversation_id, + ActionExecutionInitiator::Automatic, + ctx, + ) .is_some_and(|result| matches!(result, StartedAction::Sync)) { self.try_to_execute_available_actions(conversation_id, ctx); @@ -1339,50 +1431,69 @@ impl BlocklistAIActionModel { conversation_id: AIConversationId, ctx: &mut ModelContext, ) { - if reason.needs_confirmation() { - #[cfg(not(target_family = "wasm"))] - log_tool_event( - ctx, - RemoteLogLevel::Info, - "Tool permission requested", - serde_json::json!({ - "event": "tool_permission_requested", - "conversation_id": conversation_id.to_string(), - "action_id": action.id.to_string(), - "task_id": action.task_id.to_string(), - "tool_name": action_tool_name(action), - "permission_kind": format!("{:?}", permission_kind_for_action(&action.action)), - "reason": format!("{reason:?}"), - }), - ); - let execution_ref = self.provider_tool_execution_ref(conversation_id, &action.id); - ctx.emit(BlocklistAIActionEvent::ActionBlockedOnUserConfirmation { - action_id: action.id.clone(), - execution_ref: execution_ref.clone(), - }); - ctx.emit(BlocklistAIActionEvent::ToolLifecycle { - action_id: action.id.clone(), - execution_ref, - event: ToolEvent::PermissionRequested { - request: PermissionRequest { - id: permission_request_id(&action.id), - call_id: action.id.to_string(), - kind: permission_kind_for_action(&action.action), - reason: Some(action.action.user_friendly_name()), - }, - }, - }); - BlocklistAIHistoryModel::handle(ctx).update(ctx, |history_model, ctx| { - let blocked_action_user_friendly_str = action.action.user_friendly_name(); - history_model.update_conversation_status( - self.terminal_view_id, - conversation_id, - ConversationStatus::Blocked { - blocked_action: format!("{blocked_action_user_friendly_str:?}"), - }, + match reason { + NotExecutedReason::NeedsConfirmation => { + #[cfg(not(target_family = "wasm"))] + log_tool_event( ctx, + RemoteLogLevel::Info, + "Tool permission requested", + serde_json::json!({ + "event": "tool_permission_requested", + "conversation_id": conversation_id.to_string(), + "action_id": action.id.to_string(), + "task_id": action.task_id.to_string(), + "tool_name": action_tool_name(action), + "permission_kind": format!("{:?}", permission_kind_for_action(&action.action)), + "reason": format!("{reason:?}"), + }), ); - }); + let execution_ref = self.provider_tool_execution_ref(conversation_id, &action.id); + ctx.emit(BlocklistAIActionEvent::ActionBlockedOnUserConfirmation { + action_id: action.id.clone(), + execution_ref: execution_ref.clone(), + }); + ctx.emit(BlocklistAIActionEvent::ToolLifecycle { + action_id: action.id.clone(), + execution_ref, + event: ToolEvent::PermissionRequested { + request: PermissionRequest { + id: permission_request_id(&action.id), + call_id: action.id.to_string(), + kind: permission_kind_for_action(&action.action), + reason: Some(action.action.user_friendly_name()), + }, + }, + }); + BlocklistAIHistoryModel::handle(ctx).update(ctx, |history_model, ctx| { + let blocked_action_user_friendly_str = action.action.user_friendly_name(); + history_model.update_conversation_status( + self.terminal_view_id, + conversation_id, + ConversationStatus::Blocked { + blocked_action: format!("{blocked_action_user_friendly_str:?}"), + }, + ctx, + ); + }); + } + NotExecutedReason::NotReady => { + #[cfg(not(target_family = "wasm"))] + log_tool_event( + ctx, + RemoteLogLevel::Warn, + "Tool execution deferred", + serde_json::json!({ + "event": "tool_execution_not_ready", + "conversation_id": conversation_id.to_string(), + "action_id": action.id.to_string(), + "task_id": action.task_id.to_string(), + "tool_name": action_tool_name(action), + "permission_kind": format!("{:?}", permission_kind_for_action(&action.action)), + }), + ); + } + NotExecutedReason::WaitingOnSharer => {} } } @@ -1415,9 +1526,10 @@ impl BlocklistAIActionModel { &mut self, action_id: &AIAgentActionId, conversation_id: AIConversationId, - is_user_initiated: bool, + initiator: ActionExecutionInitiator, ctx: &mut ModelContext, ) -> Option { + let is_user_initiated = initiator.is_user_initiated(); if is_user_initiated && self.running_actions.contains_key(&conversation_id) { // User-driven approvals still execute one action at a time so that interactive // confirmations do not overlap in the UI. @@ -1476,6 +1588,7 @@ impl BlocklistAIActionModel { match execute_result { TryExecuteResult::ExecutedAsync => { + self.not_ready_actions.clear(conversation_id, &action_id); #[cfg(not(target_family = "wasm"))] log_tool_event( ctx, @@ -1494,6 +1607,7 @@ impl BlocklistAIActionModel { Some(StartedAction::Async { phase }) } TryExecuteResult::ExecutedSync => { + self.not_ready_actions.clear(conversation_id, &action_id); #[cfg(not(target_family = "wasm"))] log_tool_event( ctx, @@ -1511,6 +1625,12 @@ impl BlocklistAIActionModel { Some(StartedAction::Sync) } TryExecuteResult::NotExecuted { reason, action } => { + self.not_ready_actions.update_after_attempt( + conversation_id, + action_id, + reason, + initiator, + ); self.pending_actions .entry(conversation_id) .or_default() @@ -2047,6 +2167,8 @@ impl BlocklistAIActionModel { std::mem::discriminant(&action_result.result), cancellation_reason ); + self.not_ready_actions + .clear(conversation_id, &action_result.id); let should_remove_entry = self.running_actions .get_mut(&conversation_id) diff --git a/app/src/ai/blocklist/action_model/execute.rs b/app/src/ai/blocklist/action_model/execute.rs index 76b0c8e1..c0050ee8 100644 --- a/app/src/ai/blocklist/action_model/execute.rs +++ b/app/src/ai/blocklist/action_model/execute.rs @@ -231,12 +231,6 @@ pub enum NotExecutedReason { WaitingOnSharer, } -impl NotExecutedReason { - pub fn needs_confirmation(&self) -> bool { - matches!(self, Self::NeedsConfirmation) - } -} - /// Result type for `BlocklistAIActionExecutor::try_to_execute_action`. #[derive(Debug)] pub(super) enum TryExecuteResult { diff --git a/app/src/ai/blocklist/action_model/execute/request_file_edits.rs b/app/src/ai/blocklist/action_model/execute/request_file_edits.rs index c7705f69..e40c101a 100644 --- a/app/src/ai/blocklist/action_model/execute/request_file_edits.rs +++ b/app/src/ai/blocklist/action_model/execute/request_file_edits.rs @@ -42,10 +42,34 @@ use crate::terminal::model::session::SessionType; use crate::{safe_warn, BlocklistAIHistoryModel}; const APPLY_DIFF_RESULT_CONTEXT_LINES: usize = 10; +type AppliedDiffs = (Vec, DiffSessionType); + +#[derive(Default)] +struct PendingAppliedDiffs { + by_action: HashMap, +} + +impl PendingAppliedDiffs { + fn buffer( + &mut self, + action_id: AIAgentActionId, + diffs: Vec, + diff_session_type: DiffSessionType, + ) { + self.by_action.insert(action_id, (diffs, diff_session_type)); + } + + fn take(&mut self, action_id: &AIAgentActionId) -> Option { + self.by_action.remove(action_id) + } +} + pub struct RequestFileEditsExecutor { active_session: ModelHandle, apply_diff_model: ModelHandle, diff_views: HashMap>, + /// Successfully applied diffs that completed before their view was registered. + pending_applied_diffs: PendingAppliedDiffs, /// Set of action IDs where diff application failed. diff_application_failures: HashMap>, terminal_view_id: EntityId, @@ -62,6 +86,7 @@ impl RequestFileEditsExecutor { active_session, apply_diff_model, diff_views: HashMap::new(), + pending_applied_diffs: PendingAppliedDiffs::default(), diff_application_failures: HashMap::new(), terminal_view_id, } @@ -117,15 +142,18 @@ impl RequestFileEditsExecutor { .is_allowed() } - /// Registers a diff view to handle a RequestFileEdits action. - /// Note this MUST be called before `execute` or `preprocess_action` is invoked in - /// order for the necessary state to be set to handle the action. + /// Registers a diff view to handle a RequestFileEdits action and applies any diffs that + /// finished preprocessing before the UI observed the action. pub fn register_requested_edits( &mut self, action_id: &AIAgentActionId, view: &ViewHandle, + ctx: &mut ModelContext, ) { self.diff_views.insert(action_id.clone(), view.clone()); + if let Some((diffs, diff_session_type)) = self.pending_applied_diffs.take(action_id) { + Self::apply_diffs_to_view(view, diffs, diff_session_type, ctx); + } } pub(super) fn execute( @@ -322,23 +350,43 @@ impl RequestFileEditsExecutor { tx: oneshot::Sender<()>, ctx: &mut ModelContext, ) { - tx.send(()).ok(); + match applied_diffs { + Ok(applied_diffs) if !applied_diffs.is_empty() => { + let current_working_directory = self + .active_session + .as_ref(ctx) + .current_working_directory() + .cloned(); + let shell_launch_data = self.active_session.as_ref(ctx).shell_launch_data(ctx); + let diffs = applied_diffs + .into_iter() + .map(|diff| { + let path = host_native_absolute_path( + diff.file_name.as_str(), + &shell_launch_data, + ¤t_working_directory, + ); + FileDiff::new(diff.original_content, path, diff.diff_type) + }) + .collect(); + let diff_session_type = match self.active_session.as_ref(ctx).session_type(ctx) { + Some(SessionType::WarpifiedRemote { + host_id: Some(host_id), + }) => DiffSessionType::Remote(host_id.clone()), + _ => DiffSessionType::Local, + }; - let Some(diff_view) = self.diff_views.get(&id) else { - log::warn!( - "Tried to apply diffs for a RequestFileEdits action without a corresponding diff view" - ); - return; - }; - - let applied_diffs = match applied_diffs { - Ok(diffs) if !diffs.is_empty() => diffs, + if let Some(diff_view) = self.diff_views.get(&id).cloned() { + Self::apply_diffs_to_view(&diff_view, diffs, diff_session_type, ctx); + } else { + self.pending_applied_diffs + .buffer(id, diffs, diff_session_type); + } + } Ok(_) => { - // We didn't generate any diffs--consider this a failure. log::warn!("No diffs generated"); self.diff_application_failures .insert(id, vec1![DiffApplicationError::EmptyDiff]); - return; } Err(err) => { safe_warn!( @@ -346,38 +394,18 @@ impl RequestFileEditsExecutor { full: ("Failed to generate diffs {err:?}") ); self.diff_application_failures.insert(id, err); - return; } - }; - - let current_working_directory = self - .active_session - .as_ref(ctx) - .current_working_directory() - .cloned(); - - let shell_launch_data = self.active_session.as_ref(ctx).shell_launch_data(ctx); - - let mut diffs = Vec::with_capacity(applied_diffs.len()); - for diff in applied_diffs { - let path = host_native_absolute_path( - diff.file_name.as_str(), - &shell_launch_data, - ¤t_working_directory, - ); - let file_diff = FileDiff::new(diff.original_content, path, diff.diff_type); - diffs.push(file_diff); } - // Set the session type on the diff view so save/delete/create routes - // through the correct FileModel backend. - let diff_session_type = match self.active_session.as_ref(ctx).session_type(ctx) { - Some(SessionType::WarpifiedRemote { - host_id: Some(host_id), - }) => DiffSessionType::Remote(host_id.clone()), - _ => DiffSessionType::Local, - }; + tx.send(()).ok(); + } + fn apply_diffs_to_view( + diff_view: &ViewHandle, + diffs: Vec, + diff_session_type: DiffSessionType, + ctx: &mut ModelContext, + ) { diff_view.update(ctx, |diff_view, ctx| { diff_view.set_diff_session_type(diff_session_type); diff_view.set_candidate_diffs(diffs, ctx); diff --git a/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs b/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs index 3ea2a50e..2368512c 100644 --- a/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs +++ b/app/src/ai/blocklist/action_model/execute/request_file_edits_tests.rs @@ -2,8 +2,36 @@ use std::collections::HashMap; use ai::agent::action_result::AnyFileContent; use ai::agent::FileLocations; +use ai::diff_validation::DiffType; -use super::updated_file_contexts_from_editor_buffers; +use super::{ + updated_file_contexts_from_editor_buffers, AIAgentActionId, DiffSessionType, FileDiff, + PendingAppliedDiffs, +}; + +#[test] +fn applied_diffs_survive_until_delayed_view_registration() { + let action_id = AIAgentActionId::from("file-edit".to_string()); + let mut pending = PendingAppliedDiffs::default(); + pending.buffer( + action_id.clone(), + vec![FileDiff::new( + "before".to_string(), + "/workspace/src/main.rs".to_string(), + DiffType::update(vec![], None), + )], + DiffSessionType::Local, + ); + + let (diffs, session_type) = pending + .take(&action_id) + .expect("buffered diffs should remain available for registration"); + assert_eq!(diffs.len(), 1); + assert_eq!(diffs[0].base.content, "before"); + assert_eq!(diffs[0].base.file_path, "/workspace/src/main.rs"); + assert!(matches!(session_type, DiffSessionType::Local)); + assert!(pending.take(&action_id).is_none()); +} #[test] fn updated_file_contexts_from_editor_buffers_returns_changed_lines_with_context() { diff --git a/app/src/ai/blocklist/action_model_tests.rs b/app/src/ai/blocklist/action_model_tests.rs index 6e2f286b..c36fb85d 100644 --- a/app/src/ai/blocklist/action_model_tests.rs +++ b/app/src/ai/blocklist/action_model_tests.rs @@ -140,6 +140,47 @@ fn phased_scheduling_stops_at_serial_barrier_and_resumes_afterward() { assert_eq!(count_startable_actions_for_pass(&actions[3..]), 2); } +#[test] +fn automatic_retries_only_target_actions_deferred_as_not_ready() { + let conversation_id = AIConversationId::new(); + let action_id = AIAgentActionId::from("file-edit".to_string()); + let mut tracker = NotReadyActionTracker::default(); + + tracker.update_after_attempt( + conversation_id, + action_id.clone(), + NotExecutedReason::NotReady, + ActionExecutionInitiator::Automatic, + ); + assert!(tracker.should_retry(conversation_id, &action_id)); + assert!(!ActionExecutionInitiator::Automatic.is_user_initiated()); + + tracker.update_after_attempt( + conversation_id, + action_id.clone(), + NotExecutedReason::NotReady, + ActionExecutionInitiator::User, + ); + assert!(!tracker.should_retry(conversation_id, &action_id)); + + tracker.update_after_attempt( + conversation_id, + action_id.clone(), + NotExecutedReason::NeedsConfirmation, + ActionExecutionInitiator::Automatic, + ); + assert!(!tracker.should_retry(conversation_id, &action_id)); + + tracker.update_after_attempt( + conversation_id, + action_id.clone(), + NotExecutedReason::WaitingOnSharer, + ActionExecutionInitiator::Automatic, + ); + assert!(!tracker.should_retry(conversation_id, &action_id)); + assert!(ActionExecutionInitiator::User.is_user_initiated()); +} + #[test] fn finished_results_stay_in_original_action_order() { let action_order = HashMap::from([ diff --git a/app/src/ai/blocklist/block.rs b/app/src/ai/blocklist/block.rs index aacfdaef..ba84d860 100644 --- a/app/src/ai/blocklist/block.rs +++ b/app/src/ai/blocklist/block.rs @@ -2196,6 +2196,19 @@ impl AIBlock { } match action { + AIAgentAction { + id: action_id, + action: AIAgentActionType::RequestFileEdits { title, file_edits }, + .. + } => { + self.ensure_requested_edit_view( + action_id, + title, + file_edits.clone(), + output.server_output_id.clone(), + ctx, + ); + } AIAgentAction { id: action_id, action: @@ -2761,7 +2774,7 @@ impl AIBlock { }, .. } => { - self.handle_requested_edit_complete( + self.ensure_requested_edit_view( id, title, file_edits.clone(), @@ -3281,7 +3294,7 @@ impl AIBlock { }); } - fn handle_requested_edit_complete( + fn ensure_requested_edit_view( &mut self, action_id: &AIAgentActionId, title: &Option, @@ -3289,6 +3302,10 @@ impl AIBlock { server_output_id: Option, ctx: &mut ViewContext, ) { + if self.requested_edits.contains_key(action_id) { + return; + } + let identifiers = AIIdentifiers { client_conversation_id: Some(self.client_ids.conversation_id), client_exchange_id: Some(self.client_ids.client_exchange_id), @@ -3344,14 +3361,6 @@ impl AIBlock { ctx, ) }); - let executor = self - .action_model - .as_ref(ctx) - .request_file_edits_executor(ctx); - executor.update(ctx, |executor, _| { - executor.register_requested_edits(action_id, &view); - }); - // If the diff is being viewed in a shared session (read-only mode), populate diffs from the payload. if self.action_model.as_ref(ctx).is_view_only() { let active_session = self.active_session.as_ref(ctx); @@ -3508,7 +3517,17 @@ impl AIBlock { }); self.requested_edits - .insert(action_id.clone(), RequestedEdit::new(view)); + .insert(action_id.clone(), RequestedEdit::new(view.clone())); + let executor = self + .action_model + .as_ref(ctx) + .request_file_edits_executor(ctx); + executor.update(ctx, |executor, ctx| { + executor.register_requested_edits(action_id, &view, ctx); + }); + self.action_model.update(ctx, |action_model, ctx| { + action_model.retry_not_ready_action(action_id, self.client_ids.conversation_id, ctx); + }); if self.model.request_type(ctx).is_passive() { BlocklistAIHistoryModel::handle(ctx).update(ctx, |history, ctx| { diff --git a/app/src/ai/blocklist/block/view_impl/output.rs b/app/src/ai/blocklist/block/view_impl/output.rs index 94d9aa16..18ceaadf 100644 --- a/app/src/ai/blocklist/block/view_impl/output.rs +++ b/app/src/ai/blocklist/block/view_impl/output.rs @@ -136,6 +136,10 @@ use crate::{AIAgentTodoList, FeatureFlag}; const BLOCKED_ACTION_MESSAGE_FOR_UPLOADING_ARTIFACT: &str = "Grant access to upload this artifact?"; +fn should_render_requested_edit(action_status: Option<&AIActionStatus>) -> bool { + !action_status.is_some_and(AIActionStatus::is_preprocessing) +} + /// Data required to render the AI block output component. #[derive(Copy, Clone)] pub(crate) struct Props<'a> { @@ -564,10 +568,7 @@ pub(super) fn render(props: Props, app: &AppContext) -> Box { let action_status = props.action_model.as_ref(app).get_action_status(id); - let is_preprocessing = action_status - .clone() - .is_some_and(|status| status.is_preprocessing()); - if !is_preprocessing && !status.is_streaming() { + if should_render_requested_edit(action_status.as_ref()) { if let Some(requested_edit) = props.requested_edits.get(id) { // Don't render the requested edit if the diffs are empty for passive code diffs. if request_type.is_passive_code_diff() diff --git a/app/src/ai/blocklist/block/view_impl/output_tests.rs b/app/src/ai/blocklist/block/view_impl/output_tests.rs index ad75f962..578e2079 100644 --- a/app/src/ai/blocklist/block/view_impl/output_tests.rs +++ b/app/src/ai/blocklist/block/view_impl/output_tests.rs @@ -11,12 +11,23 @@ use watcher::HomeDirectoryWatcher; use super::{ format_upload_artifact_text, parsed_skill_for_common_locations, read_skill_display_text, + should_render_requested_edit, }; use crate::ai::agent::UploadArtifactResult; +use crate::ai::blocklist::action_model::AIActionStatus; use crate::ai::skills::SkillManager; use crate::settings::AISettings; use crate::warp_managed_paths_watcher::WarpManagedPathsWatcher; +#[test] +fn requested_edits_render_as_soon_as_preprocessing_finishes() { + assert!(!should_render_requested_edit(Some( + &AIActionStatus::Preprocessing + ))); + assert!(should_render_requested_edit(Some(&AIActionStatus::Blocked))); + assert!(should_render_requested_edit(None)); +} + #[test] fn format_upload_artifact_text_includes_request_details() { let request = UploadArtifactRequest { diff --git a/app/src/ai/blocklist/inline_action/code_diff_view.rs b/app/src/ai/blocklist/inline_action/code_diff_view.rs index 651f1176..438d7a08 100644 --- a/app/src/ai/blocklist/inline_action/code_diff_view.rs +++ b/app/src/ai/blocklist/inline_action/code_diff_view.rs @@ -366,7 +366,7 @@ pub enum CodeDiffState { /// The diff is received, but is queued for interaction behind another action. Queued, /// The user is reviewing (and possibly editing) the code diff. - /// Unlike requested commands, a [`CodeDiffView`] is only created upon stream completion. + /// The view is created as soon as the requested edit is present in streaming output. WaitingForUser, /// If the payload is some, the code diff was accepted but the individual file changes have not /// been fully computed and saved yet. We cache the accepted diff state to collect unified diffs diff --git a/app/src/ai/runtime/event_translator_tests.rs b/app/src/ai/runtime/event_translator_tests.rs index 019cf9f4..82d7ee04 100644 --- a/app/src/ai/runtime/event_translator_tests.rs +++ b/app/src/ai/runtime/event_translator_tests.rs @@ -82,6 +82,86 @@ fn restored_provider_projection_skips_stream_initialization() { )); } +#[test] +fn provider_followup_turn_starts_a_distinct_text_message() { + let mut projector = ProviderRunResponseProjector::new(RuntimeResponseConfig { + task_id: "task".to_owned(), + conversation_id: "conversation".to_owned(), + needs_create_task: false, + user_query: None, + model_id: "model".to_owned(), + max_context_tokens: Some(1_000), + capabilities: RuntimeCapabilities::provider(), + empty_output_message: None, + }); + let first_work_id = galaxy_agent_core::ExternalWorkId { + run_id: galaxy_agent_core::ProviderRunId::new("run"), + epoch: galaxy_agent_core::RunEpoch::new(1), + }; + projector + .project(ProviderRunProjection::ModelTurnStarted { + work_id: first_work_id.clone(), + profile: galaxy_agent_core::ProviderRequestProfile::new("base"), + runtime_id: "runtime".to_owned(), + model_id: "model".to_owned(), + runtime_request_id: "request-1".to_owned(), + retry_attempt: 0, + elapsed_ms: 1, + }) + .unwrap(); + let first_text = projector + .project(ProviderRunProjection::ModelEvent { + work_id: first_work_id, + event: AgentEvent::TextDelta { + text: "before tool".to_owned(), + }, + }) + .unwrap(); + let Some(response_event::Type::ClientActions(first_actions)) = &first_text[0].r#type else { + panic!("expected first text action"); + }; + let Some(client_action::Action::AddMessagesToTask(first_add)) = + &first_actions.actions[0].action + else { + panic!("first turn should add a text message"); + }; + let first_message_id = first_add.messages[0].id.clone(); + + let second_work_id = galaxy_agent_core::ExternalWorkId { + run_id: galaxy_agent_core::ProviderRunId::new("run"), + epoch: galaxy_agent_core::RunEpoch::new(2), + }; + projector + .project(ProviderRunProjection::ModelTurnStarted { + work_id: second_work_id.clone(), + profile: galaxy_agent_core::ProviderRequestProfile::new("base"), + runtime_id: "runtime".to_owned(), + model_id: "model".to_owned(), + runtime_request_id: "request-2".to_owned(), + retry_attempt: 0, + elapsed_ms: 1, + }) + .unwrap(); + let second_text = projector + .project(ProviderRunProjection::ModelEvent { + work_id: second_work_id, + event: AgentEvent::TextDelta { + text: "after tool".to_owned(), + }, + }) + .unwrap(); + let Some(response_event::Type::ClientActions(second_actions)) = &second_text[0].r#type else { + panic!("expected follow-up text action"); + }; + let Some(client_action::Action::AddMessagesToTask(second_add)) = + &second_actions.actions[0].action + else { + panic!("follow-up turn should add a text message"); + }; + + assert_ne!(first_message_id, second_add.messages[0].id); +} + #[test] fn provider_and_session_runtimes_share_text_translation() { for mut translator in [provider_translator(), session_translator()] {