fix(agent): run coord-click loop detection on converted CSS pixels

[alectimison-maker]

Summary

• Coordinate-click loop detection now buckets the converted CSS-pixel point a click actually dispatches at, instead of the raw model-authored image-pixel coordinates.
• New _resolveClickCoordsForLoop helper resolves the same point executeTool dispatches (_screenshotClickCoords conversion when clicking from a downscaled screenshot), and _executeToolBatch feeds that to _checkCoordClickLoop.

Motivation

When the model clicks from a downscaled screenshot, executeTool multiplies the image pixels by the stored screenshotClickScale before dispatch (agent.js _screenshotClickCoords). The loop detector, however, received the pre-conversion coordinates. The classic failure mode this detector exists for — "click missed, try a slightly different spot" — sees the model nudge image pixels by 5-20px that all collapse to the same CSS pixel on dispatch, but land in different 5px buckets in the detector. The dedicated loop never accumulates and the run burns steps until the generic detector or the step limit catches it.

Design

A click dispatches at exactly (mapped.x, mapped.y) whenever _screenshotClickCoords returns a value (pass-through when no scale is stored or from_screenshot is absent). The helper reuses that single conversion so the detector measures the same point as the dispatch — no duplicated conversion logic. loop-detector.js and _clickProgressIdent are untouched.

Testing

• node test/run.js — 1765 passed, 0 failed (2 new tests, both Chrome and Firefox agents)
• npm run test:security — 60/60 passed
• npm run test:toolbar-guard — 33 passed

New tests: helper converts from_screenshot clicks by the stored scale, passes through without a scale or flag, and a batch-flow wiring test proves that distinct image-pixel clicks collapsing to the same CSS 5px bucket now register as one repeated point in _checkCoordClickLoop.

Compatibility and risks

• Behavior change is limited to loop-detection bookkeeping; dispatch coordinates are unchanged.
• Firefox mirror updated identically.

Scope

• _clickProgressIdent (progress-ledger dedup) still hashes raw args — recorded as a potential follow-up, not addressed here.

[vercel]

@alectimison-maker is attempting to deploy a commit to the esokullu's projects Team on Vercel.

A member of the Team first needs to authorize it.

[esokullu]

Reviewed this alongside #2808, #2809, #2814 and #2815.

I think this change moves the detector in the opposite direction from the one intended.

The JSDoc on _resolveClickCoordsForLoop (src/chrome/src/agent/agent.js:8746-8754) says that "under downscaled screenshots the model's image-pixel nudges collapse to the same CSS pixel and the detector must see that". Under a downscaled screenshot the conversion goes the other way. _setScreenshotClickScale is only ever called with cssW / shrunk.width (agent.js:19414 and :19522), and shrunk.width < cssW whenever a resize happened, so the stored scale is always ≥ 1 and _screenshotClickCoords multiplies. Image-pixel nudges get magnified, not collapsed.

Since _checkCoordClickLoop buckets at a fixed 5 px (loop-detector.js:487-489), converting first shrinks the effective tolerance to roughly 5 / scale image pixels. Concretely, on a 3840×2160 viewport downscaled to 1568×882 (scale ≈ 2.449), a model clicking a dead button at image (500,300), (502,301), (504,299), (501,302)… used to bucket to 500/500/505/500 and trip the nudge at 5 repeats and the stop at 8. After this change those become CSS 1225/1230/1235/1227 → buckets 1225/1230/1235/1225, the count never reaches 5, and the run burns its whole step budget on a button that does nothing.

The new test doesn't catch it because [[200,400],[202,402],[50,60]] is a 1-px nudge, and round(100/5)*5 === round(101/5)*5 already held before the conversion. The comment at test/run.js:11800-11803 claims those coordinates "collapse to the same CSS bucket", but they collapsed to the same bucket in image space too, so the assertion passes either way. A test that actually distinguishes the two needs a scale above 1 and a nudge of 2-4 image pixels.

If the goal is to catch nudges that survive bucketing, dividing the bucket size by the scale (or bucketing in image space, which is what the model is actually reasoning in) gets you there. Converting to CSS pixels first makes the loop harder to detect exactly on the high-DPI screens where downscaling happens.

Two smaller things:

• if (!mapped) return { x: Number(args.x), y: Number(args.y) }; at :8756 is only reachable when Number(args.x) is non-finite, since _screenshotClickCoords returns null only in that case. So the fallback hands NaN to the detector, which keys it as "NaN,NaN". Same behavior as before this PR, so it isn't a regression, but the fallback reads as though it does something useful. const point = this._screenshotClickCoords(tabId, fnArgs); if (point) coordCheck = … is clearer and skips the bogus key.
• _screenshotClickCoords already returns { x, y, converted: false } for the unconverted case, so the new wrapper mostly re-expresses a return value that exists. Calling it directly at the one call site would save a method, its JSDoc, and two tests per tree.

[alectimison-maker]

You're right on all three points, and the premise of the original finding was inverted. _setScreenshotClickScale is only ever called with cssW / shrunk.width where shrunk.width < cssW, so the stored scale is always ≥ 1 and the conversion magnifies image-pixel nudges out of the fixed 5px bucket — the detector catches loops better in image space (which is exactly where the model reasons). Converting first actively degrades detection on the high-DPI screens where downscaling happens.

Closing this PR. I'm opening a small follow-up that pins the intended image-space bucketing with a scale-aware test, so a future change cannot "fix" this the wrong way again.

[alectimison-maker]

Closed per review: conversion direction was inverted (scale is always ≥ 1), which would degrade loop detection.