diff --git a/app/src/ai/blocklist/action_model/execute/shell_command.rs b/app/src/ai/blocklist/action_model/execute/shell_command.rs index bbe8342a646..886d262dcef 100644 --- a/app/src/ai/blocklist/action_model/execute/shell_command.rs +++ b/app/src/ai/blocklist/action_model/execute/shell_command.rs @@ -23,6 +23,7 @@ use crate::ai::agent::{ TransferShellCommandControlToUserResult, WriteToLongRunningShellCommandResult, }; use crate::ai::blocklist::BlocklistAIPermissions; +use crate::ai::blocklist::action_model::recording_controller::RecordingController; use crate::ai::blocklist::permissions::CommandExecutionPermission; use crate::ai::execution_profiles::WriteToPtyPermission; use crate::terminal::TerminalModel; @@ -258,6 +259,14 @@ impl ShellCommandExecutor { } else { command.clone() }; + // Let the recording controller decide whether this command's + // on-screen work should be kept in an active computer-use + // recording, opening an action group before it starts if so. + let conversation_id = input.conversation_id; + let opened_recording_group = RecordingController::handle(ctx) + .update(ctx, |controller, _| { + controller.maybe_begin_action_group(conversation_id, command) + }); ctx.emit(ShellCommandExecutorEvent::ExecuteCommand { action_id: action_id.clone(), command: decorated_command, @@ -278,6 +287,24 @@ impl ShellCommandExecutor { }); } + if opened_recording_group { + RecordingController::handle(ctx).update(ctx, |controller, _| { + match &result { + // Commit regardless of exit code: failed browser + // automation is still on-screen work worth keeping. + ActionResult::CommandFinished { .. } => { + controller.commit_action_group_now(conversation_id); + } + ActionResult::Cancelled | ActionResult::BlockNotFound => { + controller.discard_action_group(conversation_id); + } + // Still running; the group stays open until a later + // poll observes the finished block. + ActionResult::LongRunningCommandSnapshot { .. } => {} + } + }); + } + action_result_for_requested_command(command, result) }, ) @@ -358,6 +385,13 @@ impl ShellCommandExecutor { let exit_code = block.exit_code(); let start_ts = block.start_ts().cloned(); let completed_ts = block.completed_ts().cloned(); + // A finished poll settles any action group left open by a + // long-running `playwright-cli` command; no-op when no + // group is pending. + let conversation_id = input.conversation_id; + RecordingController::handle(ctx).update(ctx, |controller, _| { + controller.commit_action_group_now(conversation_id); + }); return ActionExecution::Sync(AIAgentActionResultType::ReadShellCommandOutput( ReadShellCommandOutputResult::CommandFinished { command, @@ -372,6 +406,7 @@ impl ShellCommandExecutor { drop(model); let block_selector = BlockSelector::Id(block_id.clone()); + let conversation_id = input.conversation_id; ActionExecution::new_async( self.action_result_future(block_selector.clone(), delay.clone()), move |result, ctx| { @@ -383,6 +418,17 @@ impl ShellCommandExecutor { }); } + match &result { + ActionResult::CommandFinished { .. } => { + RecordingController::handle(ctx).update(ctx, |controller, _| { + controller.commit_action_group_now(conversation_id); + }); + } + ActionResult::LongRunningCommandSnapshot { .. } + | ActionResult::Cancelled + | ActionResult::BlockNotFound => {} + } + action_result_for_read_shell_command_output(command.clone(), result) }, ) diff --git a/app/src/ai/blocklist/action_model/recording_controller.rs b/app/src/ai/blocklist/action_model/recording_controller.rs index 2ed3d96a35a..804bf76fc8d 100644 --- a/app/src/ai/blocklist/action_model/recording_controller.rs +++ b/app/src/ai/blocklist/action_model/recording_controller.rs @@ -1,6 +1,7 @@ //! Runtime-global state machine for the single per-runtime video recording. use std::mem; +use std::path::Path; use std::time::Duration; use ai::agent::action_result::StopRecordingResult; @@ -135,6 +136,25 @@ pub(crate) struct ActiveRecording { pub(crate) pending_group: Option, } +impl ActiveRecording { + /// Commits any in-flight action group using the current elapsed time as its + /// finish offset (clamped to the group's start). The in-flight call's + /// pointer events live in that call's own buffer and are not reachable + /// here, so the entry keeps the labels but no pointer geometry. No-op when + /// no group is pending. + fn commit_pending_group_now(&mut self) { + if let Some(pending) = self.pending_group.take() { + let finish_offset = self.started_at.elapsed().max(pending.start_offset); + self.actions.push(computer_use::ActionLogEntry { + offset: pending.start_offset, + finish_offset, + labels: pending.labels, + pointer_events: Vec::new(), + }); + } + } +} + /// A pending (in-flight) `UseComputer` action group: its start offset and labels /// are captured when the call begins, and the entry is committed with its /// finish offset only when the call's action sequence returns successfully. @@ -277,20 +297,7 @@ impl RecordingController { // with the current clock as its implicit finish offset. This can // happen when a `UseComputer` call completes and `begin_action_group` // is called for the next call before `commit_action_group` fires. - if let Some(pending) = recording.pending_group.take() { - let implicit_finish = recording.started_at.elapsed().max(pending.start_offset); - // Defensive fallback: in the normal flow the executor commits or - // discards each group in its completion callback before the next - // `begin`, so this rarely fires. The prior group's pointer events - // live in that call's own buffer and are not reachable here, so - // this path keeps the labels but no pointer geometry. - recording.actions.push(computer_use::ActionLogEntry { - offset: pending.start_offset, - finish_offset: implicit_finish, - labels: pending.labels, - pointer_events: Vec::new(), - }); - } + recording.commit_pending_group_now(); let start_offset = recording.started_at.elapsed(); recording.pending_group = Some(PendingActionGroup { start_offset, @@ -305,6 +312,27 @@ impl RecordingController { None } + /// Opens a recording action group for a shell `command` whose on-screen + /// work should survive the smart cut (currently `playwright-cli` browser + /// automation). Returns whether a group was opened, so the caller can settle + /// it with [`commit_action_group_now`] or [`discard_action_group`] once the + /// command resolves. Returns `false` for other commands or when no recording + /// is active for this conversation. + /// + /// [`commit_action_group_now`]: Self::commit_action_group_now + /// [`discard_action_group`]: Self::discard_action_group + #[cfg_attr(target_family = "wasm", allow(dead_code))] + pub fn maybe_begin_action_group( + &mut self, + conversation_id: AIConversationId, + command: &str, + ) -> bool { + is_playwright_cli_command(command) + && self + .begin_action_group(conversation_id, Vec::new()) + .is_some() + } + /// Commits the in-flight action group with its finish offset, derived from /// the capture start instant returned by [`begin_action_group`]. The finish /// is clamped to be no earlier than the start so the segment builder's @@ -335,6 +363,19 @@ impl RecordingController { } } + /// Commits the in-flight action group using the active recording's current + /// elapsed time as the finish offset, for callers that cannot thread the + /// capture start instant through to completion. No-op unless a recording is + /// active for this conversation with a pending group. + #[cfg_attr(target_family = "wasm", allow(dead_code))] + pub fn commit_action_group_now(&mut self, conversation_id: AIConversationId) { + if let RecordingState::Active(recording) = &mut self.state + && recording.conversation_id == conversation_id + { + recording.commit_pending_group_now(); + } + } + /// Discards the in-flight action group without committing it (a failed or /// cancelled `UseComputer` call). No-op if the recording is no longer active /// for this conversation. @@ -396,9 +437,14 @@ impl RecordingController { matches: impl Fn(&str, AIConversationId) -> bool, ) -> FinalizationClaim { match mem::replace(&mut self.state, RecordingState::Idle) { - RecordingState::Active(recording) + RecordingState::Active(mut recording) if matches(&recording.id, recording.conversation_id) => { + // A group can still be pending here (e.g. a long-running + // `playwright-cli` command whose finish was never observed); + // settle it so its window up to the stop point is kept rather + // than dropped by the smart cut. + recording.commit_pending_group_now(); let (sender, receiver) = oneshot::channel(); self.state = RecordingState::Finalizing { id: recording.id.clone(), @@ -515,6 +561,27 @@ impl RecordingController { } } +/// Whether a requested command invokes the `playwright-cli` binary, whose +/// on-screen browser automation should be kept in an active computer-use +/// recording rather than trimmed away with other shell work. +fn is_playwright_cli_command(command: &str) -> bool { + command + .split_whitespace() + .find(|token| { + let is_env_assignment = token + .chars() + .next() + .is_some_and(|first| first.is_ascii_alphabetic() || first == '_') + && token.contains('='); + !is_env_assignment + }) + .is_some_and(|program| { + Path::new(program) + .file_name() + .is_some_and(|name| name == "playwright-cli") + }) +} + impl Entity for RecordingController { type Event = (); } diff --git a/app/src/ai/blocklist/action_model/recording_controller_tests.rs b/app/src/ai/blocklist/action_model/recording_controller_tests.rs index 220891d1530..3e3960525ac 100644 --- a/app/src/ai/blocklist/action_model/recording_controller_tests.rs +++ b/app/src/ai/blocklist/action_model/recording_controller_tests.rs @@ -376,14 +376,50 @@ fn commit_after_finalization_is_noop() { .is_some() ); // The recording is finalized while the action is in flight; the pending - // group leaves with the claimed recording. + // group is settled into the claimed recording's committed actions. let FinalizationClaim::Claimed { recording, .. } = controller.claim_finalization_by_id("recording") else { panic!("active recording should be claimed"); }; + assert_eq!(recording.actions.len(), 1); // A late commit lands on a controller that is now Finalizing, so it commits // nothing rather than recording on the wrong (finalized) recording. controller.commit_action_group(owner, Duration::from_millis(500), Vec::new()); - assert!(recording.actions.is_empty()); + assert_eq!(recording.actions.len(), 1); +} + +#[test] +fn detects_playwright_cli_commands() { + assert!(is_playwright_cli_command( + "playwright-cli open --headed https://example.com" + )); + assert!(is_playwright_cli_command( + "PLAYWRIGHT_MCP_SANDBOX=0 playwright-cli open https://example.com" + )); + assert!(is_playwright_cli_command( + "/usr/local/bin/playwright-cli attach" + )); + assert!(!is_playwright_cli_command("npm install playwright-cli")); + assert!(!is_playwright_cli_command("echo playwright-cli")); + assert!(!is_playwright_cli_command("cargo build")); +} + +#[test] +fn finalization_commits_open_pending_group() { + let owner = AIConversationId::new(); + let mut controller = active_controller("recording", owner); + + // A long-running command's group can still be pending when the recording + // is stopped; finalization must keep its window rather than drop it. + controller.begin_action_group(owner, vec![]); + + let FinalizationClaim::Claimed { recording, .. } = + controller.claim_finalization_by_id("recording") + else { + panic!("active recording should be claimed"); + }; + assert!(recording.pending_group.is_none()); + assert_eq!(recording.actions.len(), 1); + assert!(recording.actions[0].finish_offset >= recording.actions[0].offset); }