fix(mcp): accept bounded signed pointer coordinates

[steipete]

Signed global coordinates were rejected by the MCP move and foreground drag handlers, even when they fit the existing coordinate magnitude limit. Accept signed coordinates for both paths while preserving the bounds and target/foreground guards, with exact recorded-delivery regression tests. This finishes the verified claim from #963 and retains contributor credit.

The original consolidation also covered #960, #970, #973 and #989. Those fixes have since landed separately on main; this branch has been reconciled and no longer duplicates their implementation. It keeps stronger explicit test-context isolation, complete-detection replacement coverage, and documentation of the now-landed motion and snapshot behavior.

Verification: the actual handlers reproduced signed-target refusals on main. The integrated candidate passed 69 native tests across nine Core/SDK suites at low priority with two build jobs. All 17 originally tested source/test/docs files match the reconciled branch byte-for-byte. Fourteen changed Swift files passed strict SwiftLint and nonmutating SwiftFormat; both the complete and narrowed diffs have clean independent P0–P2 reviews.

Native pointer delivery is recorded by test services; physical secondary-display input is not claimed. Snapshot tests use real SDK filesystem storage with synthetic data. No UI input was sent by these tests.

Rebased after #992 landed, with its serialized Unreleased entry added. The nine code/test/docs files in the current delta remain byte-identical to the previously tested candidate. Renewed independent review is clean through P2; final exact-head hosted CI is required before merge. No project release version changes.

Co-authored-by: Rudy Mizrahi Celekli 47457359+rudycelekli@users.noreply.github.com

[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.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed October 7, 2026, 7:11 AM ET / 11:11 UTC (Revision 3).

ClawSweeper review

What this changes

The PR accepts bounded negative coordinates for MCP cursor moves and foreground drags, adds isolated regression coverage, and documents pointer and snapshot behavior.

Merge readiness

✅ Ready for maintainer review

The fix remains necessary on current main, and no actionable introduced defect was found. The prior changelog blocker is resolved; this collaborator-authored PR has no additional review action beyond ordinary landing checks.

Priority: P2
Reviewed head: 7f2037a25ce9455aab580cd70d8060406d128686

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused parser repair with useful handler regressions and no identified blocking defect.
Proof confidence	🌊 off-meta tidepool	Not applicable: The collaborator exemption applies: reported native MCP handler runs record signed requests through test services, without claiming physical input. Related renderer screenshots do not prove pointer behavior. No stored-data contract changes are introduced.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: The collaborator exemption applies: reported native MCP handler runs record signed requests through test services, without claiming physical input. Related renderer screenshots do not prove pointer behavior. No stored-data contract changes are introduced.
Evidence reviewed	7 items	Pinned introduced change: The complete introduced diff changes only two production coordinate parsers; their symmetric inclusive bounds retain rejection of nonfinite and out-of-range inputs. The remaining changes are tests, documentation, and one changelog entry. Current main still rejects signed move coordinates: The pinned current-main parser explicitly requires nonnegative coordinates. The PR therefore repairs a remaining validation restriction rather than duplicating the separately merged pointer-direction or duration fixes. Dispatch boundary preserved: Move still requires foreground=true, and drag retains its separate exact-window background path. The changed drag bound applies only to foreground coordinates; final service dispatch and outcome composition are unchanged. This numerical validation repair does not materially change authority.
Findings	None	None.
Security	None	None.

How this fits together

Peekaboo’s MCP pointer tools turn agent requests into macOS cursor movements and drags. They validate coordinates and foreground consent before passing requests to automation services.

flowchart TD
 A[Agent pointer request] --> B[MCP move or drag]
 B --> C[Coordinate validation]
 C --> D[Foreground consent or exact window guards]
 D --> E[Automation service]
 E --> F[Pointer delivery and response]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	production +6/-14; tests +88/-4	Production shrinks through a justified parser repair while regression coverage records accepted targets and refusals.

Root-cause cluster

Relationship: canonical
Canonical: #994
Summary: This PR is the explicit successor for the signed-coordinate repair; the other merged fixes cover separate behavior.

Members:

• superseded: fix(mcp): accept signed coordinates on secondary displays #963 - The closed-unmerged predecessor proposed the same signed-coordinate repair, which this branch retains with contributor credit.
• adjacent_distinct: fix(mcp): validate smooth overrides for human pointer movement #970 - The merged repair validates human-movement overrides; this branch changes only its test context and adds documentation.
• partial_overlap: fix(snapshots): retain persisted detection classification #989 - The persistence implementation is already merged; this branch adds replacement coverage and documentation.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Use symmetric coordinate bounds in the existing MCP parsers while retaining foreground consent, exact-window isolation, and refusal before dispatch.

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

Yes, from source: current-main parsers reject negative raw coordinates before dispatch, and the added handler tests cover those requests. This read-only review did not execute native tests.

Is this the best way to solve the issue?

Yes. Adjusting the existing numerical bounds is a focused repair, and the branch preserves the surrounding consent and targeting contracts.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

No label changes.

Label justifications:

• P2: This is a bounded repair for MCP pointer targeting on displays with negative global coordinates.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The collaborator exemption applies: reported native MCP handler runs record signed requests through test services, without claiming physical input. Related renderer screenshots do not prove pointer behavior. No stored-data contract changes are introduced.

Evidence

What I checked:

• Pinned introduced change: The complete introduced diff changes only two production coordinate parsers; their symmetric inclusive bounds retain rejection of nonfinite and out-of-range inputs. The remaining changes are tests, documentation, and one changelog entry. (Core/PeekabooCore/Sources/PeekabooAgentRuntime/MCP/Tools/MoveTool+Parsing.swift:18, 7f2037a25ce9)
• Current main still rejects signed move coordinates: The pinned current-main parser explicitly requires nonnegative coordinates. The PR therefore repairs a remaining validation restriction rather than duplicating the separately merged pointer-direction or duration fixes. (Core/PeekabooCore/Sources/PeekabooAgentRuntime/MCP/Tools/MoveTool+Parsing.swift:18, a12017c720ac)
• Dispatch boundary preserved: Move still requires foreground=true, and drag retains its separate exact-window background path. The changed drag bound applies only to foreground coordinates; final service dispatch and outcome composition are unchanged. This numerical validation repair does not materially change authority. (Core/PeekabooCore/Sources/PeekabooAgentRuntime/MCP/Tools/DragTool.swift:138, 7f2037a25ce9)
• Regression coverage and proof scope: The new tests exercise actual MCP handlers with recorded automation-service requests: three signed move cases, one signed drag case, and five malformed/extreme cases across both handlers. The captured PR body reports 69 native tests and explicitly disclaims physical secondary-display input. Collaborator authorship exempts this PR from the external-contributor live-proof gate. (Core/PeekabooCore/Tests/PeekabooTests/MCP/MCPSecondaryDisplayPointerTests.swift:12, 7f2037a25ce9)
• Prior blocker resolved: The current diff includes the serialized Unreleased entry after the merged docs work at fix(docs): preserve rendered source text and navigation #992. The previous review retained no code findings. Its earlier reviewed commit is unavailable locally, so byte-identical reconciliation is treated as the author's claim rather than independently verified. (CHANGELOG.md:5, 7f2037a25ce9)
• Feature-history routing: Main history connects Peter Steinberger to exact-window drag and earlier move helper refactors, and Rudy Mizrahi Celekli to the merged human-movement validation repair. Historical blob inspection encountered a promisor fetch HTTP 403, so these are routing signals, not verified source-line introduction claims. (Core/PeekabooCore/Sources/PeekabooAgentRuntime/MCP/Tools/DragTool+Resolution.swift, 874835f57fa6)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• rudycelekli: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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.

History

Review history (2 earlier review cycles)

• reviewed 2026-10-07T04:07:17.035Z sha ff25979 :: blocked before merge. :: none
• reviewed 2026-10-07T04:14:27.569Z sha 7a6713b :: needs changes before merge. :: none

[cursor]

PR Summary

Medium Risk
Changes foreground pointer coordinate validation for MCP move/drag, which affects real cursor delivery; bounds and tests limit blast radius but multi-display targeting behavior shifts for previously rejected inputs.

Overview
Foreground MCP move and drag now accept signed global coordinates in the existing ±20000 bound, so targets on displays left of or above the primary display are valid. Parsing in MoveTool+Parsing and DragTool+Resolution drops the old non-negative-only checks and uses a single symmetric range; background drag behavior and other guards are unchanged.

Regression coverage adds MCPSecondaryDisplayPointerTests (signed targets, drag endpoints, refusal of out-of-range/NaN/inf), records dragRequests on the MCP test automation mock, and switches several MCP move tests to makeContext(..., executionPolicy: .foregroundAllowed) instead of the legacy helper.

Snapshot disk readback is documented and tested so a later complete detection overwrites prior dialog/truncation metadata (new test in SnapshotDetectionMetadataTests). see and human-mouse-move docs note persisted classification behavior and MCP coordinate/direction conventions.

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