fix(mcp): accept signed coordinates on secondary displays

[rudycelekli]

Change

Secondary displays left of or above the primary display use negative global coordinates. Move and foreground Drag incorrectly reject these valid coordinates even though background Drag and global coordinate geometry support them. Admit signed coordinates symmetrically within the existing magnitude bound.

Observed behavior and regression proof

Native actual MoveTool.execute and DragTool.execute tests with injected owned services reproduce the failure: three signed Move targets and one signed foreground Drag are refused before. After the tool handlers succeed and retain the requested target. NaN, infinity and coordinates outside ±20000 remain refused. Background exact-window admission is unchanged.

Verification

• Native Swift 6.3 on macOS 26.
• Full Core package graph compiled, with 21 focused tests in 5 suites passing across the independent source patches. The Core suite covers the pointer, formatter, shortcut and signed-coordinate handler paths; terminal title has its own actual-source native package and captured production consumer.
• Changed files pass SwiftFormat and SwiftLint with zero findings; git diff --check passes.
• The integrated MCP handler tests use injected fixture services and do not drive an operator desktop. Terminal verification captures byte output and does not execute injected OSC commands.
• Hosted CI remains unverified until publication. Full CLI test:safe and live desktop automation are separate gates; these focused results do not claim those pass.

AI assistance was used for investigation, implementation and regression tests.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[cursor]

PR Summary

Low Risk
Narrow input-validation change in MCP pointer tools with regression tests; no auth, data, or automation execution path redesign.

Overview
Move and foreground Drag MCP tools no longer require non-negative x,y values. Coordinate parsing now accepts any finite pair within ±20000, matching macOS global desktop space where monitors left of or above the primary use negative coordinates.

Validation is consolidated into a single symmetric range check (replacing separate non-negative and upper-bound guards). Drag still skips this bound when background is true, preserving prior background behavior.

Adds MCPSecondaryDisplayPointerTests covering signed move/drag success and refusal of out-of-range, NaN, and infinity inputs.

^{Reviewed by Cursor Bugbot for commit 58d7dc5. Bugbot is set up for automated code reviews on this repo. Configure here.}

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed October 5, 2026, 9:59 AM ET / 13:59 UTC.

ClawSweeper review

What this changes

The PR allows MCP mouse moves and foreground drags to use negative secondary-display coordinates while retaining the ±20000 coordinate limit.

Merge readiness

⛔ Blocked before merge - 2 items remain

This remains a useful, focused fix: current main and v4.8.0 still reject valid signed coordinates. No introduced correctness defect was found, but real desktop behavior proof is required before merge.

Priority: P2
Reviewed head: 58d7dc5f9af0c2d033d6ce5239968cde5c93a137

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The implementation is focused and source-consistent, while mock-only evidence leaves the external-contributor runtime gate unmet.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: Move and Drag parsing feed native pointer delivery, but the reported macOS tests replace that delivery with MockAutomationService. They establish handler acceptance, not observed cursor movement or dragging on a real secondary display; no stored-data contract changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: Move and Drag parsing feed native pointer delivery, but the reported macOS tests replace that delivery with MockAutomationService. They establish handler acceptance, not observed cursor movement or dragging on a real secondary display; no stored-data contract changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	7 items	Pinned introduced change: The exact base-to-head delta changes two coordinate guards and adds one regression test file; production changes total +6/-14 lines and tests add 49 lines. Current main still rejects signed coordinates: Main's Move parser requires x and y to be non-negative. Drag applies the same restriction to foreground requests, while background requests bypass that magnitude guard. Latest release retains the old validation: GitHub's v4.8.0 contents endpoint returned Move blob a6b3abd5a80bef50071ccec11aa821220a5ef5eb; the Drag endpoint returned a8a6dc41b60a94988a2ecb4021f46b4ac32248fd. Both match the inspected main blobs, so the latest release does not contain this fix.
Findings	None	None.
Security	None	None.

How this fits together

Peekaboo's MCP pointer tools translate agent requests into desktop mouse movement and drag operations. Coordinate validation runs before requests reach the native automation services.

flowchart TD
 A[Agent pointer request] --> B[MCP move or drag tool]
 B --> C[Coordinate validation]
 C --> D[Foreground consent or background window checks]
 D --> E[Desktop automation service]
 E --> F[Native pointer events]
 F --> G[Action response]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: Move and Drag parsing feed native pointer delivery, but the reported macOS tests replace that delivery with MockAutomationService. They establish handler acceptance, not observed cursor movement or dragging on a real secondary display; no stored-data contract changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Complete next step (P2) - Provide after-fix real MCP move and foreground drag proof on a secondary display with negative coordinates. Screenshots or recordings are preferred when they show the result; terminal output or logs with observable target results also count. Redact private information before posting. Updating the PR body should trigger a fresh review; otherwise ask a maintainer to comment @clawsweeper re-review.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test LOC	production +6/-14; tests +49/-0	The patch reduces validation code and adds focused coverage for the stated secondary-display defect.

Technical review

Best possible solution:

Keep the symmetric coordinate validation and verify that real MCP move and foreground drag operations reach their intended secondary-display targets.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: main rejects an MCP move to '-100,200' or a foreground drag with negative endpoints before dispatch. This read-only review did not execute native desktop automation.

Is this the best way to solve the issue?

Yes. Widening the existing guards symmetrically is a focused repair of the documented global-coordinate contract and preserves the existing delivery checks.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 155be0832821.

Labels

Label changes:

• add P2: This repairs a bounded MCP pointer failure for users with displays left of or above the primary display.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Move and Drag parsing feed native pointer delivery, but the reported macOS tests replace that delivery with MockAutomationService. They establish handler acceptance, not observed cursor movement or dragging on a real secondary display; no stored-data contract changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

• P2: This repairs a bounded MCP pointer failure for users with displays left of or above the primary display.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Move and Drag parsing feed native pointer delivery, but the reported macOS tests replace that delivery with MockAutomationService. They establish handler acceptance, not observed cursor movement or dragging on a real secondary display; no stored-data contract changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Pinned introduced change: The exact base-to-head delta changes two coordinate guards and adds one regression test file; production changes total +6/-14 lines and tests add 49 lines. (Core/PeekabooCore/Sources/PeekabooAgentRuntime/MCP/Tools/MoveTool+Parsing.swift:18, 58d7dc5f9af0)
• Current main still rejects signed coordinates: Main's Move parser requires x and y to be non-negative. Drag applies the same restriction to foreground requests, while background requests bypass that magnitude guard. (Core/PeekabooCore/Sources/PeekabooAgentRuntime/MCP/Tools/MoveTool+Parsing.swift:18, 155be0832821)
• Latest release retains the old validation: GitHub's v4.8.0 contents endpoint returned Move blob a6b3abd5a80bef50071ccec11aa821220a5ef5eb; the Drag endpoint returned a8a6dc41b60a94988a2ecb4021f46b4ac32248fd. Both match the inspected main blobs, so the latest release does not contain this fix. (Core/PeekabooCore/Sources/PeekabooAgentRuntime/MCP/Tools/MoveTool+Parsing.swift:18, 4d43dc9d80cd)
• Existing coordinate contract supports negative points: Global geometry tests explicitly cover negative y for an above-primary display and negative x for a left-side display. Native gesture delivery passes these points into pointer events without the MCP parser's non-negative restriction. (Core/PeekabooFoundation/Tests/PeekabooFoundationTests/GlobalScreenCoordinateGeometryTests.swift:15, 58d7dc5f9af0)
• Supplied proof stops at mock automation: The complete captured PR body explicitly says the integrated tests use fixture services and do not drive an operator desktop. Added tests instantiate MockAutomationService; its drag method is a no-op and moveMouse only records the target. The live body matched the captured text, and the scratch directory contained no media artifacts. (Core/PeekabooCore/Tests/PeekabooTests/MCP/MCPToolExecutionTests.swift:1310, 58d7dc5f9af0)
• Relevant merged area history: Path history includes numeric-validation work and the recent exact-window drag implementation. GitHub verifies steipete as the author account for commit 874835f and supplies its Drag resolution patch. Local blame and deeper content-history searches failed because required historical blobs could not be fetched; no introduction claim rests on those failed commands. (Core/PeekabooCore/Sources/PeekabooAgentRuntime/MCP/Tools/DragTool+Resolution.swift:70, 874835f57fa6)

Likely related people:

• unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Add an after-fix real MCP move and foreground drag demonstration on a display with negative global coordinates, showing the resulting pointer or drop target.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[rudycelekli]

I checked the concrete environment prerequisite for the requested physical secondary-display proof. The native macOS readiness probe reports Screen Recording=true and Accessibility=true, but exactly one display with frame (0,0,1728,1117); no secondary display with a negative desktop origin exists here. The current handler tests establish parameter acceptance and recorded dispatch only, and I am not treating them as observed pointer movement or foreground dragging. The remaining proof requires an actual secondary display positioned left/below the primary and a current-head MCP move plus foreground drag with observable target diagnostics. That physical proof remains pending; no code or proof override is requested.

[steipete]

Thanks for reporting rejected signed coordinates. Both the actual move and drag handlers reproduce the refusal on main. Owner PR #994 accepts signed coordinates within the existing magnitude limit, with exact recorded-delivery regression tests and updated docs. Closing this duplicate in favor of that pending successor; physical secondary-display input is not claimed as tested.