You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
extend the existing Move tool with optional from_loc and duration parameters
preserve the current-cursor drag path when the new parameters are omitted
validate explicit drag coordinates and duration before mouse input
guarantee mouse-button release when press or movement raises
mark Move as destructive and non-idempotent because it can perform drag-and-drop
Why
An agent that knows both drag endpoints currently needs separate move and drag calls. The cursor can change between those calls, so the actual source is not deterministic. An explicit source makes the operation atomic while retaining the existing API as the default.
from_loc and duration are additive and require drag=True. Duration is bounded to 0–10 seconds. Omitting them preserves the existing behavior.
Scope
This PR contains only deterministic drag behavior, its tests, and the corresponding README update. It intentionally does not include foreground-window guards or other automation capabilities.
Testing
ruff check on all changed Python files: passed
ruff format --check on all changed Python files: passed
focused drag tests: 30 passed
full suite: 348 passed, 3 failed
the same three tests/test_iserror_compliance.py import failures reproduce on unmodified upstream/main
manually smoke-tested on Windows 10 at real 100% and 150% display scaling
Dependencies
None. This branch is based directly on current main.
Add deterministic explicit-start drag support to Move
✨ Enhancement🧪 Tests📝 Documentation🕐 40+ Minutes
AI Description
• Extend Move with optional from_loc/duration for atomic, deterministic drag-and-drop.
• Validate drag inputs early and guarantee mouse-button release on drag failures.
• Add focused unit tests and update README tool docs/annotations for new behavior.
Diagram
graph TD
A["MCP Client"] --> B["Move tool (input.py)"] --> C["Desktop.drag (service.py)"] --> D["UIA DragDrop (core.py)"]
D -->|"duration=None"| E["MoveTo"]
D -->|"duration set"| F["MoveToDuration"]
E --> G["Cursor / mouse events"]
F --> G
H["Deterministic drag tests"] --> B --> C --> D
Loading
High-Level Assessment
The following are alternative approaches to this PR:
1. Introduce a dedicated `Drag` tool (keep `Move` pure)
➕ Keeps Move idempotent/non-destructive for tool-selection heuristics
➕ Avoids expanding Move’s parameter surface area over time
➖ Adds a new public tool/API surface for clients to adopt and document
2. Represent drag options as a single structured parameter (e.g., `drag_options`)
➕ Reduces long parameter lists as drag behavior grows (e.g., easing, button, modifiers)
➕ Makes it clearer which options only apply when dragging
➖ More verbose for common use
➖ Potentially more breaking/complex for existing clients than additive top-level params
Recommendation: The PR’s additive-parameter approach is a good fit: it preserves current behavior by default, enables deterministic atomic drags when needed, and adds robust validation plus guaranteed release semantics at the lowest level. The main alternative worth considering is a separate Drag tool if downstream consumers heavily rely on idempotent/non-destructive tool hints, but since Move already supported drag=True, updating the annotations here is accurate and keeps the API simple.
Files changed (7) +468 / -14
Enhancement (2) +95 / -11
service.pyAdd explicit-start and duration support to Desktop.drag+38/-4
Add explicit-start and duration support to Desktop.drag
• Extends 'Desktop.drag' to accept optional 'from_loc' and 'duration', including strict duration normalization (finite, 0–10s) and returning structured drag metadata (start/end/duration). Keeps legacy behavior when 'from_loc' is omitted (uses current cursor).
input.pyExtend 'Move' tool API, validation, and annotations+57/-7
Extend 'Move' tool API, validation, and annotations
• Adds 'from_loc' and 'duration' parameters to the 'Move' tool with early validation and normalization (including integer-only points). Enforces that drag-only options require 'drag=True' and updates tool annotations to destructive/non-idempotent to reflect drag side effects.
core.pyAdd duration-based motion and exception-safe DragDrop release+36/-2
Add duration-based motion and exception-safe DragDrop release
• Introduces 'MoveToDuration' to move the cursor over a bounded duration with intermediate steps. Updates 'DragDrop' to accept an optional duration, select the appropriate motion path, and ensure 'ReleaseMouse' is attempted even when press/move raises.
test_deterministic_drag_core.pyTest DragDrop release safety and duration path+97/-0
Test DragDrop release safety and duration path
• Adds unit tests ensuring 'DragDrop' releases the mouse after press/move failures and uses 'MoveToDuration' when duration is provided. Also validates intermediate/final cursor positioning behavior in 'MoveToDuration'.
test_deterministic_drag_desktop.pyTest Desktop.drag explicit start and duration validation+84/-0
Test Desktop.drag explicit start and duration validation
• Adds tests for explicit-start drag forwarding (including string-to-float duration normalization), legacy cursor-start behavior, and early rejection of non-finite/boolean durations before any input is attempted.
test_deterministic_drag_input.pyTest Move tool parsing/validation for deterministic drag options+152/-0
Test Move tool parsing/validation for deterministic drag options
• Adds tests covering backward-compatible move behavior, updated annotations, acceptance of explicit start points (list and JSON string forms), normalization of string coordinates, and rejection of invalid drag points or drag-only options without 'drag=True'.
• Expands the 'Move' tool documentation to describe 'from_loc' for explicit-start deterministic drags and optional 'duration' for intermediate movement.
1. Short durations skip motion✓ Resolved🐞 Bug≡ Correctness
Description
MoveToDuration computes stepCount with int(duration/0.01) and loops range(1, stepCount), so any
0<duration<0.01 yields stepCount=1 with no intermediate cursor updates and effectively no
duration-based movement. Because Desktop.drag allows any finite duration in [0,10], callers can pass
small positives and get behavior that contradicts the new duration semantics.
+ stepCount = max(1, min(200, int(duration / 0.01)))+ interval = duration / stepCount+ for i in range(1, stepCount):+ ratio = i / stepCount+ cx = curX + round((x - curX) * ratio)+ cy = curY + round((y - curY) * ratio)+ SetCursorPos(cx, cy)+ time.sleep(interval)+ SetCursorPos(x, y)+ time.sleep(waitTime)
Relevance
⭐⭐ Medium
No historical evidence found about enforcing minimum duration semantics / step granularity in cursor
movement helpers.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The code truncates duration to steps with int() and iterates a loop that is empty when stepCount==1,
which occurs for any 0<duration<0.01. DragDrop routes to MoveToDuration whenever duration is
provided, and Desktop.drag accepts small positive durations, making this reachable from the new API.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`MoveToDuration()` truncates `duration/0.01` with `int()`, so small positive durations can result in `stepCount == 1`, producing no intermediate positions and essentially ignoring the requested duration.
## Issue Context
This impacts the new `duration`-based drag path (via `DragDrop(..., duration=...)`). The current implementation also under-sleeps by one interval because it doesn’t sleep after the final position is set.
## Fix Focus Areas
- src/windows_mcp/uia/core.py[481-501]
- src/windows_mcp/uia/core.py[503-535]
## Suggested fix direction
- Use `math.ceil()` instead of `int()` to avoid truncation.
- Ensure at least one intermediate step for any `duration > 0` (e.g., force `stepCount >= 2` when duration is positive).
- Optionally ensure total sleep time matches the requested duration (e.g., sleep after the final set, or distribute sleeps across `stepCount` updates).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Desktop.drag indexes loc/from_loc lists at [0]/[1] without validating length or numeric types, so
malformed inputs can raise IndexError after the initial sleep or pass invalid coordinates into
uia.DragDrop. Validation is currently enforced only by the Move tool path, but Desktop.drag is
directly invoked in tests and is a public service method.
if isinstance(loc, list):
x, y = loc[0], loc[1]
else:
x, y = loc
+ effective_duration = self._normalize_drag_duration(duration)
sleep(0.5)
- cx, cy = uia.GetCursorPos()- uia.DragDrop(cx, cy, x, y, moveSpeed=1)+ if from_loc is None:+ cx, cy = uia.GetCursorPos()+ elif isinstance(from_loc, list):+ cx, cy = from_loc[0], from_loc[1]+ else:+ cx, cy = from_loc+ uia.DragDrop(cx, cy, x, y, moveSpeed=1, duration=effective_duration)
Relevance
⭐⭐ Medium
No historical evidence found for validating drag coordinate lists in Desktop service methods.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
Desktop.drag currently trusts list indexing for both loc and from_loc, while the tool layer is the
only place that enforces integer drag points. Tests call Desktop.drag directly, demonstrating it’s a
callable surface beyond the tool boundary.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`Desktop.drag()` accepts `loc`/`from_loc` and immediately indexes into them without validating shape/type. Direct callers can trigger `IndexError` or send non-integer coordinates downstream.
## Issue Context
The Move tool validates drag points via `_as_point()`, but `Desktop.drag()` can be called directly (tests already do). Validation should occur before the initial `sleep(0.5)` and before calling `uia.DragDrop()`.
## Fix Focus Areas
- src/windows_mcp/desktop/service.py[789-812]
## Suggested fix direction
- Validate `loc` and `from_loc` early:
- Must be a tuple/list of length 2.
- Elements must be ints (reject bools).
- Raise `ValueError` with clear messages.
- Do this validation before `sleep(0.5)` and before reading `from_loc` / calling `uia.DragDrop()`. (Optionally centralize in a small private helper similar to `_normalize_drag_duration()`.)
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. move_tool missing Google docstring 📘 Rule violation✧ Quality
Description
The modified MCP tool function move_tool has no Google-style docstring describing
args/returns/raises. This reduces API clarity for a public tool entry point.
loc: list[int] | str | None = None,
label: int | None = None,
drag: bool | str = False,
+ from_loc: list[int] | str | None = None,+ duration: float | int | str | None = None,
ctx: Context = None,
) -> str:
desktop = get_desktop()
loc = _as_loc(loc)
- drag = drag is True or (isinstance(drag, str) and drag.lower() == "true")+ from_loc = _as_loc(from_loc)+ drag = _as_bool(drag)
if loc is None and label is None:
raise ValueError("Either loc or label must be provided.")
if label is not None:
loc = _resolve_label(desktop, label)
- if len(loc) != 2:+ if not isinstance(loc, list) or len(loc) != 2:
raise ValueError("loc must be a list of exactly 2 integers [x, y]")
+ if from_loc is not None and (not isinstance(from_loc, list) or len(from_loc) != 2):+ raise ValueError("from_loc must be a list of exactly 2 integers [x, y]")+ has_drag_only_options = any(+ value is not None+ for value in (+ from_loc,+ duration,+ )+ )+ if has_drag_only_options and not drag:+ raise ValueError("from_loc and duration require drag=True")+ if drag:+ loc = _as_point(loc, "loc")+ if from_loc is not None:+ from_loc = _as_point(from_loc, "from_loc")
x, y = loc[0], loc[1]
if drag:
- desktop.drag(loc)- return f"Dragged to ({x},{y})."+ result = desktop.drag(+ loc,+ from_loc=from_loc,+ duration=duration,+ )+ start_x, start_y = result["start"]+ effective_duration = result["duration"]+ if effective_duration is None:+ return f"Dragged from ({start_x},{start_y}) to ({x},{y})."+ return (+ f"Dragged from ({start_x},{start_y}) to ({x},{y}) "+ f"over {effective_duration:.3f} seconds."+ )
else:
desktop.move(loc)
return f"Moved the mouse pointer to ({x},{y})."
Relevance
⭐ Low
Team previously rejected adding Google-style Args/Returns sections to public APIs (PR #232, #202).
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
PR Compliance ID 222802 requires Google-style docstrings on public functions/classes in changed
files. The PR modifies move_tool but the function body starts immediately without a docstring.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The `move_tool` MCP tool function is missing a Google-style docstring.
## Issue Context
This PR changes `move_tool`'s signature/behavior by adding `from_loc` and `duration`, so the docstring should document these arguments and validation errors.
## Fix Focus Areas
- src/windows_mcp/tools/input.py[336-388]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
4. Desktop.drag missing Google docstring 📘 Rule violation✧ Quality
Description
The modified Desktop.drag method lacks a Google-style docstring documenting from_loc,
duration, return shape, and possible ValueErrors. This makes the updated API harder to use and
audit.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
PR Compliance ID 222802 requires Google-style docstrings on public functions/classes in changed
files. Desktop.drag is modified in this PR and has no docstring preceding its implementation.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`Desktop.drag` was updated to accept `from_loc` and `duration` and return a dict, but it has no Google-style docstring.
## Issue Context
The method validates duration and returns structured data (`start`, `end`, `duration`), which should be documented.
## Fix Focus Areas
- src/windows_mcp/desktop/service.py[774-812]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
5. Core mouse docstrings not Google 📘 Rule violation✧ Quality
Description
The newly added/modified MoveToDuration and DragDrop functions use non–Google-style docstrings
and omit structured Args:/Returns:/Raises: sections. This violates the documentation standard
for public functions changed in this PR.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
PR Compliance ID 222802 requires Google-style docstrings on public functions/classes in changed
files. The PR adds MoveToDuration with a minimal docstring and modifies DragDrop/its docstring,
but neither uses the required Google-style sections.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`MoveToDuration` and `DragDrop` are public functions touched by this PR but their docstrings are not in Google style (missing `Args:`, etc.).
## Issue Context
These functions now support bounded-duration movement and error-safe drag cleanup; their parameters and behavior should be documented in the required format.
## Fix Focus Areas
- src/windows_mcp/uia/core.py[481-534]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The newly added MoveToDuration function and several local identifiers (curX, curY,
stepCount, waitTime) are not snake_case. This violates the naming convention and makes new
code inconsistent with the required identifier style.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
PR Compliance ID 222797 requires snake_case for function and variable identifiers. The PR adds
MoveToDuration and local variables with camelCase names in src/windows_mcp/uia/core.py.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Newly added identifiers in `src/windows_mcp/uia/core.py` are not `snake_case` (e.g., `MoveToDuration`, `curX`, `stepCount`, `waitTime`).
## Issue Context
Compliance requires snake_case for function and variable identifiers added/modified in this PR.
## Fix Focus Areas
- src/windows_mcp/uia/core.py[481-500]
- src/windows_mcp/uia/core.py[503-534]
- tests/test_deterministic_drag_core.py[1-97]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Movetool with optionalfrom_locanddurationparametersMoveas destructive and non-idempotent because it can perform drag-and-dropWhy
An agent that knows both drag endpoints currently needs separate move and drag calls. The cursor can change between those calls, so the actual source is not deterministic. An explicit source makes the operation atomic while retaining the existing API as the default.
API
from_locanddurationare additive and requiredrag=True. Duration is bounded to 0–10 seconds. Omitting them preserves the existing behavior.Scope
This PR contains only deterministic drag behavior, its tests, and the corresponding README update. It intentionally does not include foreground-window guards or other automation capabilities.
Testing
ruff checkon all changed Python files: passedruff format --checkon all changed Python files: passed30 passed348 passed, 3 failedtests/test_iserror_compliance.pyimport failures reproduce on unmodifiedupstream/mainDependencies
None. This branch is based directly on current
main.Closes #326