Skip to content

Commit 52061d2

Browse files
vkodithalaoz-agent
andauthored
[fix](VA) Keep in-progress playwright-cli commands in computer-use recordings (#14544)
## Description On Linux, computer-use video recordings are smart-trimmed after stopping: only committed recording action groups produce keep-windows, and everything else is hard-cut. Shell commands never enter the recording timeline, so when the agent drives the browser via `playwright-cli`, all of that on-screen work was cut from the video. This PR keeps in-progress `playwright-cli` commands in the recording: - `ShellCommandExecutor` now detects `playwright-cli` invocations (skipping leading env-var assignments and resolving the program path's file name, so `npm install playwright-cli` or `echo playwright-cli` don't match) and opens a recording action group before the command starts. - When the command finishes (any exit code), the pending group is committed; if the action is cancelled or the block is not found, it is discarded. - A `playwright-cli` command that outlives the executor's poll window returns a long-running snapshot; the group stays open and is committed when a later `ReadShellCommandOutput` poll observes the finished block. Since that poll runs in a separate executor call, a new `RecordingController::commit_action_group_now` helper commits the pending group using the recording's own elapsed clock (no-op when no recording is active or no group is pending). - If the recording is stopped/finalized while a group is still pending (e.g. a long-running `playwright-cli` session never observed finished), claiming the active recording for finalization settles the pending group at the stop point instead of dropping it, via a shared `ActiveRecording::commit_pending_group_now` helper also used by `begin_action_group`'s auto-commit. Other shell commands (npm/cargo/etc.) never open a group, so they remain trimmed exactly as before. Behavior is unchanged when no recording is active: all recording calls no-op unless a recording is Active for the conversation. ## Linked Issue None — recording-quality fix scoped and dispatched via Oz orchestration. - [ ] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [ ] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Testing - Added a unit test for the `playwright-cli` command detector (`detects_playwright_cli_commands`) covering plain, env-prefixed, and absolute-path invocations plus non-matches (`npm install playwright-cli`, `echo playwright-cli`, `cargo build`). - Added a regression test (`finalization_commits_open_pending_group`) asserting that finalizing while a group is pending commits its window instead of dropping it. - `cargo nextest run -p warp -E 'test(detects_playwright_cli_commands) or test(block_working_directory_updated_does_not_drain_finish_senders) or test(/recording_controller/)'` — 17/17 passed. - `./script/format --check` — clean. - `cargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warnings`, `cargo clippy -p warp --all-targets --tests -- -D warnings`, and `cargo clippy -p warp_completer --all-targets --tests -- -D warnings` — all green. - [ ] I have manually tested my changes locally with `./script/run` Runtime behavior only differs on Linux (macOS recordings keep everything, no smart cut), so this was verified on macOS via build + unit tests rather than a live recording. ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode CHANGELOG-BUG-FIX: In-progress playwright-cli browser automation is now preserved in computer-use video recordings instead of being trimmed out. Co-Authored-By: Oz <oz-agent@warp.dev> --------- Co-authored-by: Oz <oz-agent@warp.dev>
1 parent d84b4e3 commit 52061d2

3 files changed

Lines changed: 166 additions & 17 deletions

File tree

app/src/ai/blocklist/action_model/execute/shell_command.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use crate::ai::agent::{
2323
TransferShellCommandControlToUserResult, WriteToLongRunningShellCommandResult,
2424
};
2525
use crate::ai::blocklist::BlocklistAIPermissions;
26+
use crate::ai::blocklist::action_model::recording_controller::RecordingController;
2627
use crate::ai::blocklist::permissions::CommandExecutionPermission;
2728
use crate::ai::execution_profiles::WriteToPtyPermission;
2829
use crate::terminal::TerminalModel;
@@ -258,6 +259,14 @@ impl ShellCommandExecutor {
258259
} else {
259260
command.clone()
260261
};
262+
// Let the recording controller decide whether this command's
263+
// on-screen work should be kept in an active computer-use
264+
// recording, opening an action group before it starts if so.
265+
let conversation_id = input.conversation_id;
266+
let opened_recording_group = RecordingController::handle(ctx)
267+
.update(ctx, |controller, _| {
268+
controller.maybe_begin_action_group(conversation_id, command)
269+
});
261270
ctx.emit(ShellCommandExecutorEvent::ExecuteCommand {
262271
action_id: action_id.clone(),
263272
command: decorated_command,
@@ -278,6 +287,24 @@ impl ShellCommandExecutor {
278287
});
279288
}
280289

290+
if opened_recording_group {
291+
RecordingController::handle(ctx).update(ctx, |controller, _| {
292+
match &result {
293+
// Commit regardless of exit code: failed browser
294+
// automation is still on-screen work worth keeping.
295+
ActionResult::CommandFinished { .. } => {
296+
controller.commit_action_group_now(conversation_id);
297+
}
298+
ActionResult::Cancelled | ActionResult::BlockNotFound => {
299+
controller.discard_action_group(conversation_id);
300+
}
301+
// Still running; the group stays open until a later
302+
// poll observes the finished block.
303+
ActionResult::LongRunningCommandSnapshot { .. } => {}
304+
}
305+
});
306+
}
307+
281308
action_result_for_requested_command(command, result)
282309
},
283310
)
@@ -358,6 +385,13 @@ impl ShellCommandExecutor {
358385
let exit_code = block.exit_code();
359386
let start_ts = block.start_ts().cloned();
360387
let completed_ts = block.completed_ts().cloned();
388+
// A finished poll settles any action group left open by a
389+
// long-running `playwright-cli` command; no-op when no
390+
// group is pending.
391+
let conversation_id = input.conversation_id;
392+
RecordingController::handle(ctx).update(ctx, |controller, _| {
393+
controller.commit_action_group_now(conversation_id);
394+
});
361395
return ActionExecution::Sync(AIAgentActionResultType::ReadShellCommandOutput(
362396
ReadShellCommandOutputResult::CommandFinished {
363397
command,
@@ -372,6 +406,7 @@ impl ShellCommandExecutor {
372406
drop(model);
373407

374408
let block_selector = BlockSelector::Id(block_id.clone());
409+
let conversation_id = input.conversation_id;
375410
ActionExecution::new_async(
376411
self.action_result_future(block_selector.clone(), delay.clone()),
377412
move |result, ctx| {
@@ -383,6 +418,17 @@ impl ShellCommandExecutor {
383418
});
384419
}
385420

421+
match &result {
422+
ActionResult::CommandFinished { .. } => {
423+
RecordingController::handle(ctx).update(ctx, |controller, _| {
424+
controller.commit_action_group_now(conversation_id);
425+
});
426+
}
427+
ActionResult::LongRunningCommandSnapshot { .. }
428+
| ActionResult::Cancelled
429+
| ActionResult::BlockNotFound => {}
430+
}
431+
386432
action_result_for_read_shell_command_output(command.clone(), result)
387433
},
388434
)

app/src/ai/blocklist/action_model/recording_controller.rs

Lines changed: 82 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! Runtime-global state machine for the single per-runtime video recording.
22
33
use std::mem;
4+
use std::path::Path;
45
use std::time::Duration;
56

67
use ai::agent::action_result::StopRecordingResult;
@@ -135,6 +136,25 @@ pub(crate) struct ActiveRecording {
135136
pub(crate) pending_group: Option<PendingActionGroup>,
136137
}
137138

139+
impl ActiveRecording {
140+
/// Commits any in-flight action group using the current elapsed time as its
141+
/// finish offset (clamped to the group's start). The in-flight call's
142+
/// pointer events live in that call's own buffer and are not reachable
143+
/// here, so the entry keeps the labels but no pointer geometry. No-op when
144+
/// no group is pending.
145+
fn commit_pending_group_now(&mut self) {
146+
if let Some(pending) = self.pending_group.take() {
147+
let finish_offset = self.started_at.elapsed().max(pending.start_offset);
148+
self.actions.push(computer_use::ActionLogEntry {
149+
offset: pending.start_offset,
150+
finish_offset,
151+
labels: pending.labels,
152+
pointer_events: Vec::new(),
153+
});
154+
}
155+
}
156+
}
157+
138158
/// A pending (in-flight) `UseComputer` action group: its start offset and labels
139159
/// are captured when the call begins, and the entry is committed with its
140160
/// finish offset only when the call's action sequence returns successfully.
@@ -277,20 +297,7 @@ impl RecordingController {
277297
// with the current clock as its implicit finish offset. This can
278298
// happen when a `UseComputer` call completes and `begin_action_group`
279299
// is called for the next call before `commit_action_group` fires.
280-
if let Some(pending) = recording.pending_group.take() {
281-
let implicit_finish = recording.started_at.elapsed().max(pending.start_offset);
282-
// Defensive fallback: in the normal flow the executor commits or
283-
// discards each group in its completion callback before the next
284-
// `begin`, so this rarely fires. The prior group's pointer events
285-
// live in that call's own buffer and are not reachable here, so
286-
// this path keeps the labels but no pointer geometry.
287-
recording.actions.push(computer_use::ActionLogEntry {
288-
offset: pending.start_offset,
289-
finish_offset: implicit_finish,
290-
labels: pending.labels,
291-
pointer_events: Vec::new(),
292-
});
293-
}
300+
recording.commit_pending_group_now();
294301
let start_offset = recording.started_at.elapsed();
295302
recording.pending_group = Some(PendingActionGroup {
296303
start_offset,
@@ -305,6 +312,27 @@ impl RecordingController {
305312
None
306313
}
307314

315+
/// Opens a recording action group for a shell `command` whose on-screen
316+
/// work should survive the smart cut (currently `playwright-cli` browser
317+
/// automation). Returns whether a group was opened, so the caller can settle
318+
/// it with [`commit_action_group_now`] or [`discard_action_group`] once the
319+
/// command resolves. Returns `false` for other commands or when no recording
320+
/// is active for this conversation.
321+
///
322+
/// [`commit_action_group_now`]: Self::commit_action_group_now
323+
/// [`discard_action_group`]: Self::discard_action_group
324+
#[cfg_attr(target_family = "wasm", allow(dead_code))]
325+
pub fn maybe_begin_action_group(
326+
&mut self,
327+
conversation_id: AIConversationId,
328+
command: &str,
329+
) -> bool {
330+
is_playwright_cli_command(command)
331+
&& self
332+
.begin_action_group(conversation_id, Vec::new())
333+
.is_some()
334+
}
335+
308336
/// Commits the in-flight action group with its finish offset, derived from
309337
/// the capture start instant returned by [`begin_action_group`]. The finish
310338
/// is clamped to be no earlier than the start so the segment builder's
@@ -335,6 +363,19 @@ impl RecordingController {
335363
}
336364
}
337365

366+
/// Commits the in-flight action group using the active recording's current
367+
/// elapsed time as the finish offset, for callers that cannot thread the
368+
/// capture start instant through to completion. No-op unless a recording is
369+
/// active for this conversation with a pending group.
370+
#[cfg_attr(target_family = "wasm", allow(dead_code))]
371+
pub fn commit_action_group_now(&mut self, conversation_id: AIConversationId) {
372+
if let RecordingState::Active(recording) = &mut self.state
373+
&& recording.conversation_id == conversation_id
374+
{
375+
recording.commit_pending_group_now();
376+
}
377+
}
378+
338379
/// Discards the in-flight action group without committing it (a failed or
339380
/// cancelled `UseComputer` call). No-op if the recording is no longer active
340381
/// for this conversation.
@@ -396,9 +437,14 @@ impl RecordingController {
396437
matches: impl Fn(&str, AIConversationId) -> bool,
397438
) -> FinalizationClaim {
398439
match mem::replace(&mut self.state, RecordingState::Idle) {
399-
RecordingState::Active(recording)
440+
RecordingState::Active(mut recording)
400441
if matches(&recording.id, recording.conversation_id) =>
401442
{
443+
// A group can still be pending here (e.g. a long-running
444+
// `playwright-cli` command whose finish was never observed);
445+
// settle it so its window up to the stop point is kept rather
446+
// than dropped by the smart cut.
447+
recording.commit_pending_group_now();
402448
let (sender, receiver) = oneshot::channel();
403449
self.state = RecordingState::Finalizing {
404450
id: recording.id.clone(),
@@ -515,6 +561,27 @@ impl RecordingController {
515561
}
516562
}
517563

564+
/// Whether a requested command invokes the `playwright-cli` binary, whose
565+
/// on-screen browser automation should be kept in an active computer-use
566+
/// recording rather than trimmed away with other shell work.
567+
fn is_playwright_cli_command(command: &str) -> bool {
568+
command
569+
.split_whitespace()
570+
.find(|token| {
571+
let is_env_assignment = token
572+
.chars()
573+
.next()
574+
.is_some_and(|first| first.is_ascii_alphabetic() || first == '_')
575+
&& token.contains('=');
576+
!is_env_assignment
577+
})
578+
.is_some_and(|program| {
579+
Path::new(program)
580+
.file_name()
581+
.is_some_and(|name| name == "playwright-cli")
582+
})
583+
}
584+
518585
impl Entity for RecordingController {
519586
type Event = ();
520587
}

app/src/ai/blocklist/action_model/recording_controller_tests.rs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,14 +376,50 @@ fn commit_after_finalization_is_noop() {
376376
.is_some()
377377
);
378378
// The recording is finalized while the action is in flight; the pending
379-
// group leaves with the claimed recording.
379+
// group is settled into the claimed recording's committed actions.
380380
let FinalizationClaim::Claimed { recording, .. } =
381381
controller.claim_finalization_by_id("recording")
382382
else {
383383
panic!("active recording should be claimed");
384384
};
385+
assert_eq!(recording.actions.len(), 1);
385386
// A late commit lands on a controller that is now Finalizing, so it commits
386387
// nothing rather than recording on the wrong (finalized) recording.
387388
controller.commit_action_group(owner, Duration::from_millis(500), Vec::new());
388-
assert!(recording.actions.is_empty());
389+
assert_eq!(recording.actions.len(), 1);
390+
}
391+
392+
#[test]
393+
fn detects_playwright_cli_commands() {
394+
assert!(is_playwright_cli_command(
395+
"playwright-cli open --headed https://example.com"
396+
));
397+
assert!(is_playwright_cli_command(
398+
"PLAYWRIGHT_MCP_SANDBOX=0 playwright-cli open https://example.com"
399+
));
400+
assert!(is_playwright_cli_command(
401+
"/usr/local/bin/playwright-cli attach"
402+
));
403+
assert!(!is_playwright_cli_command("npm install playwright-cli"));
404+
assert!(!is_playwright_cli_command("echo playwright-cli"));
405+
assert!(!is_playwright_cli_command("cargo build"));
406+
}
407+
408+
#[test]
409+
fn finalization_commits_open_pending_group() {
410+
let owner = AIConversationId::new();
411+
let mut controller = active_controller("recording", owner);
412+
413+
// A long-running command's group can still be pending when the recording
414+
// is stopped; finalization must keep its window rather than drop it.
415+
controller.begin_action_group(owner, vec![]);
416+
417+
let FinalizationClaim::Claimed { recording, .. } =
418+
controller.claim_finalization_by_id("recording")
419+
else {
420+
panic!("active recording should be claimed");
421+
};
422+
assert!(recording.pending_group.is_none());
423+
assert_eq!(recording.actions.len(), 1);
424+
assert!(recording.actions[0].finish_offset >= recording.actions[0].offset);
389425
}

0 commit comments

Comments
 (0)