Skip to content

Commit 94e69e7

Browse files
rgarciacursoragent
andauthored
fix: replace time.Sleep with context-aware sleeps in doPressKey and doDragMouse (#146)
## Summary - `doPressKey` used `time.Sleep` for the key hold duration, blocking without respecting context cancellation while holding `inputMu` - `doDragMouse` used `time.Sleep` for the optional pre-drag delay, same problem - Both are especially problematic in the batch flow where the mutex is held for the entire batch ## Changes - Extract a `sleepWithContext` helper that uses `select` with `time.NewTimer` and `ctx.Done()` - Refactor `doSleep` to use the shared helper - Replace `time.Sleep` in `doPressKey` with `sleepWithContext`; on cancellation, best-effort release held keys using a background context - Replace `time.Sleep` in `doDragMouse` with `sleepWithContext`; on cancellation, best-effort release mouse button and modifier keys - No remaining `time.Sleep` calls in `computer.go` ## Test plan - [x] `go build ./...` and `go vet ./...` pass - [x] `go test ./cmd/api/api/...` passes - [ ] Verify key hold and drag delay are interruptible by cancelling a request mid-sleep Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches low-level input simulation and cleanup behavior; cancellation paths now run `xdotool` releases via `context.Background()`, which could change how interrupted requests leave system input state. > > **Overview** > Makes input-related delays context-aware to avoid blocking while `inputMu` is held (notably during `BatchComputerAction`). > > Replaces `time.Sleep` in `doPressKey` key-hold and `doDragMouse` pre-drag delay with a shared `sleepWithContext` helper, and refactors `doSleep` to use it as well. On cancellation, both key-hold and drag delay now perform **best-effort cleanup** (releasing keys and/or mouse button) via `context.Background()` before returning an error. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 90ff857. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent a90f3b3 commit 94e69e7

File tree

1 file changed

+52
-20
lines changed

1 file changed

+52
-20
lines changed

server/cmd/api/api/computer.go

Lines changed: 52 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -557,24 +557,33 @@ func (s *ApiService) doPressKey(ctx context.Context, body oapi.PressKeyRequest)
557557
}
558558

559559
d := time.Duration(*body.Duration) * time.Millisecond
560-
time.Sleep(d)
561560

562-
argsUp := []string{}
563-
for _, key := range body.Keys {
564-
argsUp = append(argsUp, "keyup", key)
565-
}
566-
if body.HoldKeys != nil {
567-
for _, key := range *body.HoldKeys {
561+
// Best-effort release helper: always attempt to release keys even if sleep was interrupted.
562+
releaseKeys := func() error {
563+
argsUp := []string{}
564+
for _, key := range body.Keys {
568565
argsUp = append(argsUp, "keyup", key)
569566
}
567+
if body.HoldKeys != nil {
568+
for _, key := range *body.HoldKeys {
569+
argsUp = append(argsUp, "keyup", key)
570+
}
571+
}
572+
// Use background context for cleanup so keys are released even on cancellation.
573+
if output, err := defaultXdoTool.Run(context.Background(), argsUp...); err != nil {
574+
log.Error("xdotool keyup failed", "err", err, "output", string(output))
575+
return &executionError{msg: fmt.Sprintf("failed to release keys. out=%s", string(output))}
576+
}
577+
return nil
570578
}
571579

572-
if output, err := defaultXdoTool.Run(ctx, argsUp...); err != nil {
573-
log.Error("xdotool keyup failed", "err", err, "output", string(output))
574-
return &executionError{msg: fmt.Sprintf("failed to release keys. out=%s", string(output))}
580+
if err := sleepWithContext(ctx, d); err != nil {
581+
// Context cancelled while holding keys down; release them before returning.
582+
_ = releaseKeys()
583+
return &executionError{msg: fmt.Sprintf("key hold interrupted: %s", err)}
575584
}
576585

577-
return nil
586+
return releaseKeys()
578587
}
579588

580589
// Tap behavior: hold modifiers (if any), tap each key, then release modifiers.
@@ -770,7 +779,17 @@ func (s *ApiService) doDragMouse(ctx context.Context, body oapi.DragMouseRequest
770779

771780
// Optional delay between mousedown and movement
772781
if body.Delay != nil && *body.Delay > 0 {
773-
time.Sleep(time.Duration(*body.Delay) * time.Millisecond)
782+
if err := sleepWithContext(ctx, time.Duration(*body.Delay)*time.Millisecond); err != nil {
783+
// Best-effort release: mouseup + modifier keyup
784+
cleanupArgs := []string{"mouseup", btn}
785+
if body.HoldKeys != nil {
786+
for _, key := range *body.HoldKeys {
787+
cleanupArgs = append(cleanupArgs, "keyup", key)
788+
}
789+
}
790+
_, _ = defaultXdoTool.Run(context.Background(), cleanupArgs...)
791+
return &executionError{msg: fmt.Sprintf("drag delay interrupted: %s", err)}
792+
}
774793
}
775794

776795
// Phase 2: move along path (excluding first point) using fixed-count relative steps
@@ -869,6 +888,24 @@ func (s *ApiService) DragMouse(ctx context.Context, request oapi.DragMouseReques
869888

870889
const maxSleepDurationMs = 30_000
871890

891+
// sleepWithContext pauses for the given duration, returning early if the context is cancelled.
892+
// This should be used instead of time.Sleep when holding the inputMu mutex, so that context
893+
// cancellation can promptly release the lock.
894+
func sleepWithContext(ctx context.Context, d time.Duration) error {
895+
if d <= 0 {
896+
return nil
897+
}
898+
timer := time.NewTimer(d)
899+
defer timer.Stop()
900+
901+
select {
902+
case <-timer.C:
903+
return nil
904+
case <-ctx.Done():
905+
return ctx.Err()
906+
}
907+
}
908+
872909
func (s *ApiService) doSleep(ctx context.Context, body oapi.SleepAction) error {
873910
if body.DurationMs < 0 {
874911
return &validationError{msg: "duration_ms must be >= 0"}
@@ -877,15 +914,10 @@ func (s *ApiService) doSleep(ctx context.Context, body oapi.SleepAction) error {
877914
return &validationError{msg: fmt.Sprintf("duration_ms must be <= %d", maxSleepDurationMs)}
878915
}
879916

880-
timer := time.NewTimer(time.Duration(body.DurationMs) * time.Millisecond)
881-
defer timer.Stop()
882-
883-
select {
884-
case <-timer.C:
885-
return nil
886-
case <-ctx.Done():
887-
return &executionError{msg: fmt.Sprintf("sleep interrupted: %s", ctx.Err())}
917+
if err := sleepWithContext(ctx, time.Duration(body.DurationMs)*time.Millisecond); err != nil {
918+
return &executionError{msg: fmt.Sprintf("sleep interrupted: %s", err)}
888919
}
920+
return nil
889921
}
890922

891923
func (s *ApiService) BatchComputerAction(ctx context.Context, request oapi.BatchComputerActionRequestObject) (oapi.BatchComputerActionResponseObject, error) {

0 commit comments

Comments
 (0)