diff --git a/agent/agentdesktop/api.go b/agent/agentdesktop/api.go index e69c813055..536f889bdb 100644 --- a/agent/agentdesktop/api.go +++ b/agent/agentdesktop/api.go @@ -2,7 +2,6 @@ package agentdesktop import ( "encoding/json" - "math" "net/http" "strconv" "time" @@ -13,6 +12,7 @@ import ( "github.com/coder/coder/v2/agent/agentssh" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/codersdk/workspacesdk" "github.com/coder/quartz" "github.com/coder/websocket" ) @@ -26,9 +26,9 @@ type DesktopAction struct { Duration *int `json:"duration,omitempty"` ScrollAmount *int `json:"scroll_amount,omitempty"` ScrollDirection *string `json:"scroll_direction,omitempty"` - // ScaledWidth and ScaledHeight are the coordinate space the - // model is using. When provided, coordinates are linearly - // mapped from scaled → native before dispatching. + // ScaledWidth and ScaledHeight describe the declared model-facing desktop + // geometry. When provided, input coordinates are mapped from declared space + // to native desktop pixels before dispatching. ScaledWidth *int `json:"scaled_width,omitempty"` ScaledHeight *int `json:"scaled_height,omitempty"` } @@ -144,17 +144,8 @@ func (a *API) handleAction(rw http.ResponseWriter, r *http.Request) { slog.F("elapsed_ms", a.clock.Since(handlerStart).Milliseconds()), ) - // Helper to scale a coordinate pair from the model's space to - // native display pixels. - scaleXY := func(x, y int) (int, int) { - if action.ScaledWidth != nil && *action.ScaledWidth > 0 { - x = scaleCoordinate(x, *action.ScaledWidth, cfg.Width) - } - if action.ScaledHeight != nil && *action.ScaledHeight > 0 { - y = scaleCoordinate(y, *action.ScaledHeight, cfg.Height) - } - return x, y - } + geometry := desktopGeometryForAction(cfg, action) + scaleXY := geometry.DeclaredPointToNative var resp DesktopActionResponse @@ -192,7 +183,7 @@ func (a *API) handleAction(rw http.ResponseWriter, r *http.Request) { resp.Output = "type action performed" case "cursor_position": - x, y, err := a.desktop.CursorPosition(ctx) + nativeX, nativeY, err := a.desktop.CursorPosition(ctx) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Cursor position failed.", @@ -200,6 +191,7 @@ func (a *API) handleAction(rw http.ResponseWriter, r *http.Request) { }) return } + x, y := geometry.NativePointToDeclared(nativeX, nativeY) resp.Output = "x=" + strconv.Itoa(x) + ",y=" + strconv.Itoa(y) case "mouse_move": @@ -447,14 +439,10 @@ func (a *API) handleAction(rw http.ResponseWriter, r *http.Request) { resp.Output = "hold_key action performed" case "screenshot": - var opts ScreenshotOptions - if action.ScaledWidth != nil && *action.ScaledWidth > 0 { - opts.TargetWidth = *action.ScaledWidth - } - if action.ScaledHeight != nil && *action.ScaledHeight > 0 { - opts.TargetHeight = *action.ScaledHeight - } - result, err := a.desktop.Screenshot(ctx, opts) + result, err := a.desktop.Screenshot(ctx, ScreenshotOptions{ + TargetWidth: geometry.DeclaredWidth, + TargetHeight: geometry.DeclaredHeight, + }) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Screenshot failed.", @@ -464,16 +452,8 @@ func (a *API) handleAction(rw http.ResponseWriter, r *http.Request) { } resp.Output = "screenshot" resp.ScreenshotData = result.Data - if action.ScaledWidth != nil && *action.ScaledWidth > 0 && *action.ScaledWidth != cfg.Width { - resp.ScreenshotWidth = *action.ScaledWidth - } else { - resp.ScreenshotWidth = cfg.Width - } - if action.ScaledHeight != nil && *action.ScaledHeight > 0 && *action.ScaledHeight != cfg.Height { - resp.ScreenshotHeight = *action.ScaledHeight - } else { - resp.ScreenshotHeight = cfg.Height - } + resp.ScreenshotWidth = geometry.DeclaredWidth + resp.ScreenshotHeight = geometry.DeclaredHeight default: httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -512,6 +492,23 @@ func coordFromAction(action DesktopAction) (x, y int, err error) { return action.Coordinate[0], action.Coordinate[1], nil } +func desktopGeometryForAction(cfg DisplayConfig, action DesktopAction) workspacesdk.DesktopGeometry { + declaredWidth := cfg.Width + declaredHeight := cfg.Height + if action.ScaledWidth != nil && *action.ScaledWidth > 0 { + declaredWidth = *action.ScaledWidth + } + if action.ScaledHeight != nil && *action.ScaledHeight > 0 { + declaredHeight = *action.ScaledHeight + } + return workspacesdk.NewDesktopGeometryWithDeclared( + cfg.Width, + cfg.Height, + declaredWidth, + declaredHeight, + ) +} + // missingFieldError is returned when a required field is absent from // a DesktopAction. type missingFieldError struct { @@ -522,15 +519,3 @@ type missingFieldError struct { func (e *missingFieldError) Error() string { return "Missing \"" + e.field + "\" for " + e.action + " action." } - -// scaleCoordinate maps a coordinate from scaled → native space. -func scaleCoordinate(scaled, scaledDim, nativeDim int) int { - if scaledDim == 0 || scaledDim == nativeDim { - return scaled - } - native := (float64(scaled)+0.5)*float64(nativeDim)/float64(scaledDim) - 0.5 - // Clamp to valid range. - native = math.Max(native, 0) - native = math.Min(native, float64(nativeDim-1)) - return int(native) -} diff --git a/agent/agentdesktop/api_test.go b/agent/agentdesktop/api_test.go index 663f177c81..a4a91d835e 100644 --- a/agent/agentdesktop/api_test.go +++ b/agent/agentdesktop/api_test.go @@ -27,10 +27,12 @@ var _ agentdesktop.Desktop = (*fakeDesktop)(nil) // fakeDesktop is a minimal Desktop implementation for unit tests. type fakeDesktop struct { startErr error + cursorPos [2]int startCfg agentdesktop.DisplayConfig vncConnErr error screenshotErr error screenshotRes agentdesktop.ScreenshotResult + lastShotOpts agentdesktop.ScreenshotOptions closed bool // Track calls for assertions. @@ -51,7 +53,8 @@ func (f *fakeDesktop) VNCConn(context.Context) (net.Conn, error) { return nil, f.vncConnErr } -func (f *fakeDesktop) Screenshot(_ context.Context, _ agentdesktop.ScreenshotOptions) (agentdesktop.ScreenshotResult, error) { +func (f *fakeDesktop) Screenshot(_ context.Context, opts agentdesktop.ScreenshotOptions) (agentdesktop.ScreenshotResult, error) { + f.lastShotOpts = opts return f.screenshotRes, f.screenshotErr } @@ -100,8 +103,8 @@ func (f *fakeDesktop) Type(_ context.Context, text string) error { return nil } -func (*fakeDesktop) CursorPosition(context.Context) (x int, y int, err error) { - return 10, 20, nil +func (f *fakeDesktop) CursorPosition(context.Context) (x int, y int, err error) { + return f.cursorPos[0], f.cursorPos[1], nil } func (f *fakeDesktop) Close() error { @@ -135,8 +138,12 @@ func TestHandleAction_Screenshot(t *testing.T) { t.Parallel() logger := slogtest.Make(t, nil) + geometry := workspacesdk.DefaultDesktopGeometry() fake := &fakeDesktop{ - startCfg: agentdesktop.DisplayConfig{Width: workspacesdk.DesktopDisplayWidth, Height: workspacesdk.DesktopDisplayHeight}, + startCfg: agentdesktop.DisplayConfig{ + Width: geometry.NativeWidth, + Height: geometry.NativeHeight, + }, screenshotRes: agentdesktop.ScreenshotResult{Data: "base64data"}, } api := agentdesktop.NewAPI(logger, fake, nil) @@ -158,11 +165,52 @@ func TestHandleAction_Screenshot(t *testing.T) { var result agentdesktop.DesktopActionResponse err = json.NewDecoder(rr.Body).Decode(&result) require.NoError(t, err) - // Dimensions come from DisplayConfig, not the screenshot CLI. assert.Equal(t, "screenshot", result.Output) assert.Equal(t, "base64data", result.ScreenshotData) - assert.Equal(t, workspacesdk.DesktopDisplayWidth, result.ScreenshotWidth) - assert.Equal(t, workspacesdk.DesktopDisplayHeight, result.ScreenshotHeight) + assert.Equal(t, geometry.NativeWidth, result.ScreenshotWidth) + assert.Equal(t, geometry.NativeHeight, result.ScreenshotHeight) + assert.Equal(t, agentdesktop.ScreenshotOptions{ + TargetWidth: geometry.NativeWidth, + TargetHeight: geometry.NativeHeight, + }, fake.lastShotOpts) +} + +func TestHandleAction_ScreenshotUsesDeclaredDimensionsFromRequest(t *testing.T) { + t.Parallel() + + logger := slogtest.Make(t, nil) + fake := &fakeDesktop{ + startCfg: agentdesktop.DisplayConfig{Width: 1920, Height: 1080}, + screenshotRes: agentdesktop.ScreenshotResult{Data: "base64data"}, + } + api := agentdesktop.NewAPI(logger, fake, nil) + defer api.Close() + + sw := 1280 + sh := 720 + body := agentdesktop.DesktopAction{ + Action: "screenshot", + ScaledWidth: &sw, + ScaledHeight: &sh, + } + b, err := json.Marshal(body) + require.NoError(t, err) + + rr := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/action", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + + handler := api.Routes() + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, agentdesktop.ScreenshotOptions{TargetWidth: 1280, TargetHeight: 720}, fake.lastShotOpts) + + var result agentdesktop.DesktopActionResponse + err = json.NewDecoder(rr.Body).Decode(&result) + require.NoError(t, err) + assert.Equal(t, 1280, result.ScreenshotWidth) + assert.Equal(t, 720, result.ScreenshotHeight) } func TestHandleAction_LeftClick(t *testing.T) { @@ -315,7 +363,6 @@ func TestHandleAction_HoldKey(t *testing.T) { handler.ServeHTTP(rr, req) }() - // Wait for the timer to be created, then advance past it. trap.MustWait(req.Context()).MustRelease(req.Context()) mClk.Advance(time.Duration(dur) * time.Millisecond).MustWait(req.Context()) @@ -389,7 +436,6 @@ func TestHandleAction_ScrollDown(t *testing.T) { handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusOK, rr.Code) - // dy should be positive 5 for "down". assert.Equal(t, [4]int{500, 400, 0, 5}, fake.lastScroll) } @@ -398,13 +444,11 @@ func TestHandleAction_CoordinateScaling(t *testing.T) { logger := slogtest.Make(t, nil) fake := &fakeDesktop{ - // Native display is 1920x1080. startCfg: agentdesktop.DisplayConfig{Width: 1920, Height: 1080}, } api := agentdesktop.NewAPI(logger, fake, nil) defer api.Close() - // Model is working in a 1280x720 coordinate space. sw := 1280 sh := 720 body := agentdesktop.DesktopAction{ @@ -424,12 +468,43 @@ func TestHandleAction_CoordinateScaling(t *testing.T) { handler.ServeHTTP(rr, req) assert.Equal(t, http.StatusOK, rr.Code) - // 640 in 1280-space → 960 in 1920-space (midpoint maps to - // midpoint). assert.Equal(t, 960, fake.lastMove[0]) assert.Equal(t, 540, fake.lastMove[1]) } +func TestHandleAction_CoordinateScalingClampsToLastPixel(t *testing.T) { + t.Parallel() + + logger := slogtest.Make(t, nil) + fake := &fakeDesktop{ + startCfg: agentdesktop.DisplayConfig{Width: 1920, Height: 1080}, + } + api := agentdesktop.NewAPI(logger, fake, nil) + defer api.Close() + + sw := 1366 + sh := 768 + body := agentdesktop.DesktopAction{ + Action: "mouse_move", + Coordinate: &[2]int{1365, 767}, + ScaledWidth: &sw, + ScaledHeight: &sh, + } + b, err := json.Marshal(body) + require.NoError(t, err) + + rr := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/action", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + + handler := api.Routes() + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + assert.Equal(t, 1919, fake.lastMove[0]) + assert.Equal(t, 1079, fake.lastMove[1]) +} + func TestClose_DelegatesToDesktop(t *testing.T) { t.Parallel() @@ -446,15 +521,12 @@ func TestClose_PreventsNewSessions(t *testing.T) { t.Parallel() logger := slogtest.Make(t, nil) - // After Close(), Start() will return an error because the - // underlying Desktop is closed. fake := &fakeDesktop{} api := agentdesktop.NewAPI(logger, fake, nil) err := api.Close() require.NoError(t, err) - // Simulate the closed desktop returning an error on Start(). fake.startErr = xerrors.New("desktop is closed") rr := httptest.NewRecorder() @@ -465,3 +537,40 @@ func TestClose_PreventsNewSessions(t *testing.T) { assert.Equal(t, http.StatusInternalServerError, rr.Code) } + +func TestHandleAction_CursorPositionReturnsDeclaredCoordinates(t *testing.T) { + t.Parallel() + + logger := slogtest.Make(t, nil) + fake := &fakeDesktop{ + startCfg: agentdesktop.DisplayConfig{Width: 1920, Height: 1080}, + cursorPos: [2]int{960, 540}, + } + api := agentdesktop.NewAPI(logger, fake, nil) + defer api.Close() + + sw := 1280 + sh := 720 + body := agentdesktop.DesktopAction{ + Action: "cursor_position", + ScaledWidth: &sw, + ScaledHeight: &sh, + } + b, err := json.Marshal(body) + require.NoError(t, err) + + rr := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/action", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + + handler := api.Routes() + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusOK, rr.Code) + + var resp agentdesktop.DesktopActionResponse + err = json.NewDecoder(rr.Body).Decode(&resp) + require.NoError(t, err) + // Native (960,540) in 1920x1080 should map to declared space in 1280x720. + assert.Equal(t, "x=640,y=360", resp.Output) +} diff --git a/agent/agentdesktop/portabledesktop.go b/agent/agentdesktop/portabledesktop.go index 36e50b15ab..4dfb0c4ed7 100644 --- a/agent/agentdesktop/portabledesktop.go +++ b/agent/agentdesktop/portabledesktop.go @@ -111,7 +111,7 @@ func (p *portableDesktop) Start(ctx context.Context) (DisplayConfig, error) { //nolint:gosec // portabledesktop is a trusted binary resolved via ensureBinary. cmd := p.execer.CommandContext(sessionCtx, p.binPath, "up", "--json", - "--geometry", fmt.Sprintf("%dx%d", workspacesdk.DesktopDisplayWidth, workspacesdk.DesktopDisplayHeight)) + "--geometry", fmt.Sprintf("%dx%d", workspacesdk.DesktopNativeWidth, workspacesdk.DesktopNativeHeight)) stdout, err := cmd.StdoutPipe() if err != nil { sessionCancel() diff --git a/coderd/chatd/chatd.go b/coderd/chatd/chatd.go index 18cbacdd8e..44b0790850 100644 --- a/coderd/chatd/chatd.go +++ b/coderd/chatd/chatd.go @@ -3272,14 +3272,17 @@ func (p *Server) runChat( } if isComputerUse { + desktopGeometry := workspacesdk.DefaultDesktopGeometry() providerTools = append(providerTools, chatloop.ProviderTool{ Definition: chattool.ComputerUseProviderTool( - workspacesdk.DesktopDisplayWidth, - workspacesdk.DesktopDisplayHeight), + desktopGeometry.DeclaredWidth, + desktopGeometry.DeclaredHeight, + ), Runner: chattool.NewComputerUseTool( - workspacesdk.DesktopDisplayWidth, - workspacesdk.DesktopDisplayHeight, - workspaceCtx.getWorkspaceConn, quartz.NewReal(), + desktopGeometry.DeclaredWidth, + desktopGeometry.DeclaredHeight, + workspaceCtx.getWorkspaceConn, + quartz.NewReal(), ), }) } diff --git a/coderd/chatd/chattool/computeruse.go b/coderd/chatd/chattool/computeruse.go index c5c2e8e303..4b3a112b7f 100644 --- a/coderd/chatd/chattool/computeruse.go +++ b/coderd/chatd/chattool/computeruse.go @@ -3,7 +3,6 @@ package chattool import ( "context" "fmt" - "math" "time" "charm.land/fantasy" @@ -25,23 +24,25 @@ const ( // computerUseTool implements fantasy.AgentTool and // chatloop.ToolDefiner for Anthropic computer use. type computerUseTool struct { - displayWidth int - displayHeight int + declaredWidth int + declaredHeight int getWorkspaceConn func(ctx context.Context) (workspacesdk.AgentConn, error) providerOptions fantasy.ProviderOptions clock quartz.Clock } -// NewComputerUseTool creates a computer use AgentTool that -// delegates to the agent's desktop endpoints. +// NewComputerUseTool creates a computer use AgentTool that delegates to the +// agent's desktop endpoints. declaredWidth and declaredHeight are the +// model-facing desktop dimensions advertised to Anthropic and requested for +// screenshots. func NewComputerUseTool( - displayWidth, displayHeight int, + declaredWidth, declaredHeight int, getWorkspaceConn func(ctx context.Context) (workspacesdk.AgentConn, error), clock quartz.Clock, ) fantasy.AgentTool { return &computerUseTool{ - displayWidth: displayWidth, - displayHeight: displayHeight, + declaredWidth: declaredWidth, + declaredHeight: declaredHeight, getWorkspaceConn: getWorkspaceConn, clock: clock, } @@ -56,14 +57,13 @@ func (*computerUseTool) Info() fantasy.ToolInfo { } } -// ComputerUseProviderTool creates the provider-defined tool -// definition for Anthropic computer use. This is passed via -// ProviderTools so the API receives the correct wire format. -func ComputerUseProviderTool(displayWidth, displayHeight int) fantasy.Tool { +// ComputerUseProviderTool creates the provider-defined Anthropic computer-use +// tool definition using the declared model-facing desktop geometry. +func ComputerUseProviderTool(declaredWidth, declaredHeight int) fantasy.Tool { return fantasyanthropic.NewComputerUseTool( fantasyanthropic.ComputerUseToolOptions{ - DisplayWidthPx: int64(displayWidth), - DisplayHeightPx: int64(displayHeight), + DisplayWidthPx: int64(declaredWidth), + DisplayHeightPx: int64(declaredHeight), ToolVersion: fantasyanthropic.ComputerUse20251124, }, ) @@ -92,10 +92,7 @@ func (t *computerUseTool) Run(ctx context.Context, call fantasy.ToolCall) (fanta ), nil } - // Compute scaled screenshot size for Anthropic constraints. - scaledW, scaledH := computeScaledScreenshotSize( - t.displayWidth, t.displayHeight, - ) + declaredWidth, declaredHeight := t.declaredActionDimensions() // For wait actions, sleep then return a screenshot. if input.Action == fantasyanthropic.ActionWait { @@ -111,8 +108,8 @@ func (t *computerUseTool) Run(ctx context.Context, call fantasy.ToolCall) (fanta } screenshotAction := workspacesdk.DesktopAction{ Action: "screenshot", - ScaledWidth: &scaledW, - ScaledHeight: &scaledH, + ScaledWidth: &declaredWidth, + ScaledHeight: &declaredHeight, } screenResp, sErr := conn.ExecuteDesktopAction(ctx, screenshotAction) if sErr != nil { @@ -129,8 +126,8 @@ func (t *computerUseTool) Run(ctx context.Context, call fantasy.ToolCall) (fanta if input.Action == fantasyanthropic.ActionScreenshot { screenshotAction := workspacesdk.DesktopAction{ Action: "screenshot", - ScaledWidth: &scaledW, - ScaledHeight: &scaledH, + ScaledWidth: &declaredWidth, + ScaledHeight: &declaredHeight, } screenResp, sErr := conn.ExecuteDesktopAction(ctx, screenshotAction) if sErr != nil { @@ -146,8 +143,8 @@ func (t *computerUseTool) Run(ctx context.Context, call fantasy.ToolCall) (fanta // Build the action request. action := workspacesdk.DesktopAction{ Action: string(input.Action), - ScaledWidth: &scaledW, - ScaledHeight: &scaledH, + ScaledWidth: &declaredWidth, + ScaledHeight: &declaredHeight, } if input.Coordinate != ([2]int64{}) { coord := [2]int{int(input.Coordinate[0]), int(input.Coordinate[1])} @@ -183,8 +180,8 @@ func (t *computerUseTool) Run(ctx context.Context, call fantasy.ToolCall) (fanta // Take a screenshot after every action (Anthropic pattern). screenshotAction := workspacesdk.DesktopAction{ Action: "screenshot", - ScaledWidth: &scaledW, - ScaledHeight: &scaledH, + ScaledWidth: &declaredWidth, + ScaledHeight: &declaredHeight, } screenResp, sErr := conn.ExecuteDesktopAction(ctx, screenshotAction) if sErr != nil { @@ -198,23 +195,17 @@ func (t *computerUseTool) Run(ctx context.Context, call fantasy.ToolCall) (fanta ), nil } -// computeScaledScreenshotSize computes the target screenshot -// dimensions to fit within Anthropic's constraints. -func computeScaledScreenshotSize(width, height int) (scaledWidth int, scaledHeight int) { - const maxLongEdge = 1568 - const maxTotalPixels = 1_150_000 - - longEdge := max(width, height) - totalPixels := width * height - longEdgeScale := float64(maxLongEdge) / float64(longEdge) - totalPixelsScale := math.Sqrt( - float64(maxTotalPixels) / float64(totalPixels), - ) - scale := min(1.0, longEdgeScale, totalPixelsScale) - - if scale >= 1.0 { - return width, height +func (t *computerUseTool) declaredActionDimensions() (declaredWidth, declaredHeight int) { + if t.declaredWidth <= 0 || t.declaredHeight <= 0 { + geometry := workspacesdk.DefaultDesktopGeometry() + return geometry.DeclaredWidth, geometry.DeclaredHeight } - return max(1, int(float64(width)*scale)), - max(1, int(float64(height)*scale)) + return t.declaredWidth, t.declaredHeight +} + +// computeScaledScreenshotSize preserves the historical helper name while using +// the shared declared-geometry selection logic. +func computeScaledScreenshotSize(width, height int) (scaledWidth int, scaledHeight int) { + geometry := workspacesdk.NewDesktopGeometry(width, height) + return geometry.DeclaredWidth, geometry.DeclaredHeight } diff --git a/coderd/chatd/chattool/computeruse_internal_test.go b/coderd/chatd/chattool/computeruse_internal_test.go index 13820a519e..f773306fd5 100644 --- a/coderd/chatd/chattool/computeruse_internal_test.go +++ b/coderd/chatd/chattool/computeruse_internal_test.go @@ -15,11 +15,11 @@ func TestComputeScaledScreenshotSize(t *testing.T) { wantW, wantH int }{ { - name: "1920x1080_scales_down", + name: "1920x1080_prefers_standard_1280x720", width: 1920, height: 1080, - wantW: 1429, - wantH: 804, + wantW: 1280, + wantH: 720, }, { name: "1280x800_no_scaling", @@ -29,18 +29,18 @@ func TestComputeScaledScreenshotSize(t *testing.T) { wantH: 800, }, { - name: "3840x2160_large_display", + name: "3840x2160_prefers_standard_1280x720", width: 3840, height: 2160, - wantW: 1429, - wantH: 804, + wantW: 1280, + wantH: 720, }, { - name: "1568x1000_pixel_cap_applies", + name: "1568x1000_prefers_standard_1280x816", width: 1568, height: 1000, - wantW: 1342, - wantH: 856, + wantW: 1280, + wantH: 816, }, { name: "100x100_small_display", @@ -50,14 +50,18 @@ func TestComputeScaledScreenshotSize(t *testing.T) { wantH: 100, }, { - name: "4000x3000_stays_within_limits", - width: 4000, - // Both constraints apply. The function should keep - // the result within maxLongEdge=1568 and - // totalPixels<=1,150,000. + name: "4000x3000_prefers_standard_1024x768", + width: 4000, height: 3000, - wantW: 1238, - wantH: 928, + wantW: 1024, + wantH: 768, + }, + { + name: "1920x1200_prefers_standard_1280x800", + width: 1920, + height: 1200, + wantW: 1280, + wantH: 800, }, } diff --git a/coderd/chatd/chattool/computeruse_test.go b/coderd/chatd/chattool/computeruse_test.go index f8740cda6d..a59d0141b1 100644 --- a/coderd/chatd/chattool/computeruse_test.go +++ b/coderd/chatd/chattool/computeruse_test.go @@ -19,7 +19,8 @@ import ( func TestComputerUseTool_Info(t *testing.T) { t.Parallel() - tool := chattool.NewComputerUseTool(workspacesdk.DesktopDisplayWidth, workspacesdk.DesktopDisplayHeight, nil, quartz.NewReal()) + geometry := workspacesdk.DefaultDesktopGeometry() + tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, nil, quartz.NewReal()) info := tool.Info() assert.Equal(t, "computer", info.Name) assert.NotEmpty(t, info.Description) @@ -28,14 +29,25 @@ func TestComputerUseTool_Info(t *testing.T) { func TestComputerUseProviderTool(t *testing.T) { t.Parallel() - def := chattool.ComputerUseProviderTool(workspacesdk.DesktopDisplayWidth, workspacesdk.DesktopDisplayHeight) + geometry := workspacesdk.DefaultDesktopGeometry() + def := chattool.ComputerUseProviderTool(geometry.DeclaredWidth, geometry.DeclaredHeight) pdt, ok := def.(fantasy.ProviderDefinedTool) require.True(t, ok, "ComputerUseProviderTool should return a ProviderDefinedTool") assert.Contains(t, pdt.ID, "computer") assert.Equal(t, "computer", pdt.Name) - // Verify display dimensions are passed through. - assert.Equal(t, int64(workspacesdk.DesktopDisplayWidth), pdt.Args["display_width_px"]) - assert.Equal(t, int64(workspacesdk.DesktopDisplayHeight), pdt.Args["display_height_px"]) + assert.Equal(t, int64(geometry.DeclaredWidth), pdt.Args["display_width_px"]) + assert.Equal(t, int64(geometry.DeclaredHeight), pdt.Args["display_height_px"]) +} + +func TestComputerUseProviderTool_PrefersDeclaredGeometry(t *testing.T) { + t.Parallel() + + geometry := workspacesdk.NewDesktopGeometry(1920, 1080) + def := chattool.ComputerUseProviderTool(geometry.DeclaredWidth, geometry.DeclaredHeight) + pdt, ok := def.(fantasy.ProviderDefinedTool) + require.True(t, ok, "ComputerUseProviderTool should return a ProviderDefinedTool") + assert.Equal(t, int64(1280), pdt.Args["display_width_px"]) + assert.Equal(t, int64(720), pdt.Args["display_height_px"]) } func TestComputerUseTool_Run_Screenshot(t *testing.T) { @@ -43,18 +55,25 @@ func TestComputerUseTool_Run_Screenshot(t *testing.T) { ctrl := gomock.NewController(t) mockConn := agentconnmock.NewMockAgentConn(ctrl) + geometry := workspacesdk.DefaultDesktopGeometry() mockConn.EXPECT().ExecuteDesktopAction( gomock.Any(), - gomock.Any(), - ).Return(workspacesdk.DesktopActionResponse{ - Output: "screenshot", - ScreenshotData: "base64png", - ScreenshotWidth: 1024, - ScreenshotHeight: 768, - }, nil) + gomock.AssignableToTypeOf(workspacesdk.DesktopAction{}), + ).DoAndReturn(func(_ context.Context, action workspacesdk.DesktopAction) (workspacesdk.DesktopActionResponse, error) { + require.NotNil(t, action.ScaledWidth) + require.NotNil(t, action.ScaledHeight) + assert.Equal(t, geometry.DeclaredWidth, *action.ScaledWidth) + assert.Equal(t, geometry.DeclaredHeight, *action.ScaledHeight) + return workspacesdk.DesktopActionResponse{ + Output: "screenshot", + ScreenshotData: "base64png", + ScreenshotWidth: geometry.DeclaredWidth, + ScreenshotHeight: geometry.DeclaredHeight, + }, nil + }) - tool := chattool.NewComputerUseTool(workspacesdk.DesktopDisplayWidth, workspacesdk.DesktopDisplayHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { + tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { return mockConn, nil }, quartz.NewReal()) @@ -77,27 +96,39 @@ func TestComputerUseTool_Run_LeftClick(t *testing.T) { ctrl := gomock.NewController(t) mockConn := agentconnmock.NewMockAgentConn(ctrl) + geometry := workspacesdk.DefaultDesktopGeometry() - // Expect the action call first. mockConn.EXPECT().ExecuteDesktopAction( gomock.Any(), - gomock.Any(), - ).Return(workspacesdk.DesktopActionResponse{ - Output: "left_click performed", - }, nil) + gomock.AssignableToTypeOf(workspacesdk.DesktopAction{}), + ).DoAndReturn(func(_ context.Context, action workspacesdk.DesktopAction) (workspacesdk.DesktopActionResponse, error) { + require.NotNil(t, action.Coordinate) + assert.Equal(t, [2]int{100, 200}, *action.Coordinate) + require.NotNil(t, action.ScaledWidth) + require.NotNil(t, action.ScaledHeight) + assert.Equal(t, geometry.DeclaredWidth, *action.ScaledWidth) + assert.Equal(t, geometry.DeclaredHeight, *action.ScaledHeight) + return workspacesdk.DesktopActionResponse{Output: "left_click performed"}, nil + }) - // Then expect a screenshot (auto-screenshot after action). mockConn.EXPECT().ExecuteDesktopAction( gomock.Any(), - gomock.Any(), - ).Return(workspacesdk.DesktopActionResponse{ - Output: "screenshot", - ScreenshotData: "after-click", - ScreenshotWidth: 1024, - ScreenshotHeight: 768, - }, nil) + gomock.AssignableToTypeOf(workspacesdk.DesktopAction{}), + ).DoAndReturn(func(_ context.Context, action workspacesdk.DesktopAction) (workspacesdk.DesktopActionResponse, error) { + assert.Equal(t, "screenshot", action.Action) + require.NotNil(t, action.ScaledWidth) + require.NotNil(t, action.ScaledHeight) + assert.Equal(t, geometry.DeclaredWidth, *action.ScaledWidth) + assert.Equal(t, geometry.DeclaredHeight, *action.ScaledHeight) + return workspacesdk.DesktopActionResponse{ + Output: "screenshot", + ScreenshotData: "after-click", + ScreenshotWidth: geometry.DeclaredWidth, + ScreenshotHeight: geometry.DeclaredHeight, + }, nil + }) - tool := chattool.NewComputerUseTool(workspacesdk.DesktopDisplayWidth, workspacesdk.DesktopDisplayHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { + tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { return mockConn, nil }, quartz.NewReal()) @@ -118,18 +149,25 @@ func TestComputerUseTool_Run_Wait(t *testing.T) { ctrl := gomock.NewController(t) mockConn := agentconnmock.NewMockAgentConn(ctrl) - // Expect a screenshot after the wait completes. + geometry := workspacesdk.DefaultDesktopGeometry() + mockConn.EXPECT().ExecuteDesktopAction( gomock.Any(), - gomock.Any(), - ).Return(workspacesdk.DesktopActionResponse{ - Output: "screenshot", - ScreenshotData: "after-wait", - ScreenshotWidth: 1024, - ScreenshotHeight: 768, - }, nil) + gomock.AssignableToTypeOf(workspacesdk.DesktopAction{}), + ).DoAndReturn(func(_ context.Context, action workspacesdk.DesktopAction) (workspacesdk.DesktopActionResponse, error) { + require.NotNil(t, action.ScaledWidth) + require.NotNil(t, action.ScaledHeight) + assert.Equal(t, geometry.DeclaredWidth, *action.ScaledWidth) + assert.Equal(t, geometry.DeclaredHeight, *action.ScaledHeight) + return workspacesdk.DesktopActionResponse{ + Output: "screenshot", + ScreenshotData: "after-wait", + ScreenshotWidth: geometry.DeclaredWidth, + ScreenshotHeight: geometry.DeclaredHeight, + }, nil + }) - tool := chattool.NewComputerUseTool(workspacesdk.DesktopDisplayWidth, workspacesdk.DesktopDisplayHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { + tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { return mockConn, nil }, quartz.NewReal()) @@ -150,7 +188,8 @@ func TestComputerUseTool_Run_Wait(t *testing.T) { func TestComputerUseTool_Run_ConnError(t *testing.T) { t.Parallel() - tool := chattool.NewComputerUseTool(workspacesdk.DesktopDisplayWidth, workspacesdk.DesktopDisplayHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { + geometry := workspacesdk.DefaultDesktopGeometry() + tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { return nil, xerrors.New("workspace not available") }, quartz.NewReal()) @@ -169,7 +208,8 @@ func TestComputerUseTool_Run_ConnError(t *testing.T) { func TestComputerUseTool_Run_InvalidInput(t *testing.T) { t.Parallel() - tool := chattool.NewComputerUseTool(workspacesdk.DesktopDisplayWidth, workspacesdk.DesktopDisplayHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { + geometry := workspacesdk.DefaultDesktopGeometry() + tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) { return nil, xerrors.New("should not be called") }, quartz.NewReal()) diff --git a/codersdk/workspacesdk/agentconn.go b/codersdk/workspacesdk/agentconn.go index 1cf8ca7a99..acb22e1d29 100644 --- a/codersdk/workspacesdk/agentconn.go +++ b/codersdk/workspacesdk/agentconn.go @@ -578,8 +578,10 @@ type DesktopAction struct { Duration *int `json:"duration,omitempty"` ScrollAmount *int `json:"scroll_amount,omitempty"` ScrollDirection *string `json:"scroll_direction,omitempty"` - ScaledWidth *int `json:"scaled_width,omitempty"` - ScaledHeight *int `json:"scaled_height,omitempty"` + // ScaledWidth and ScaledHeight carry the declared model-facing desktop + // geometry used for screenshot sizing and coordinate mapping. + ScaledWidth *int `json:"scaled_width,omitempty"` + ScaledHeight *int `json:"scaled_height,omitempty"` } // DesktopActionResponse is the response from the desktop action diff --git a/codersdk/workspacesdk/display.go b/codersdk/workspacesdk/display.go index b2f77bd0d1..a3e4e4d040 100644 --- a/codersdk/workspacesdk/display.go +++ b/codersdk/workspacesdk/display.go @@ -1,10 +1,167 @@ package workspacesdk +import "math" + const ( - // DesktopDisplayWidth is the default display width in pixels - // used for computer-use desktop sessions. - DesktopDisplayWidth = 1366 - // DesktopDisplayHeight is the default display height in pixels - // used for computer-use desktop sessions. - DesktopDisplayHeight = 768 + // DesktopNativeWidth is the default native desktop width in pixels used for + // computer-use desktop sessions. + DesktopNativeWidth = 1920 + // DesktopNativeHeight is the default native desktop height in pixels used for + // computer-use desktop sessions. + DesktopNativeHeight = 1080 + + desktopDeclaredMaxLongEdge = 1568 + desktopDeclaredMaxTotalPixels = 1_150_000 ) + +var preferredDeclaredDesktopWidths = []int{1280, 1024} + +// DesktopGeometry describes the native workspace desktop and the declared +// model-facing geometry used for screenshots and coordinates. +type DesktopGeometry struct { + NativeWidth int + NativeHeight int + DeclaredWidth int + DeclaredHeight int +} + +// DefaultDesktopGeometry returns the default native desktop geometry together +// with the declared model-facing geometry derived from it. +func DefaultDesktopGeometry() DesktopGeometry { + return NewDesktopGeometry(DesktopNativeWidth, DesktopNativeHeight) +} + +// NewDesktopGeometry derives a declared model-facing geometry from the native +// desktop size. +func NewDesktopGeometry(nativeWidth, nativeHeight int) DesktopGeometry { + nativeWidth = sanitizeDesktopDimension(nativeWidth) + nativeHeight = sanitizeDesktopDimension(nativeHeight) + + declaredWidth, declaredHeight := computeDeclaredDesktopSize( + nativeWidth, + nativeHeight, + ) + + return DesktopGeometry{ + NativeWidth: nativeWidth, + NativeHeight: nativeHeight, + DeclaredWidth: declaredWidth, + DeclaredHeight: declaredHeight, + } +} + +// NewDesktopGeometryWithDeclared returns a geometry that preserves the native +// desktop size while using the provided declared model-facing dimensions. +func NewDesktopGeometryWithDeclared( + nativeWidth, + nativeHeight, + declaredWidth, + declaredHeight int, +) DesktopGeometry { + nativeWidth = sanitizeDesktopDimension(nativeWidth) + nativeHeight = sanitizeDesktopDimension(nativeHeight) + if declaredWidth <= 0 { + declaredWidth = nativeWidth + } + if declaredHeight <= 0 { + declaredHeight = nativeHeight + } + + return DesktopGeometry{ + NativeWidth: nativeWidth, + NativeHeight: nativeHeight, + DeclaredWidth: sanitizeDesktopDimension(declaredWidth), + DeclaredHeight: sanitizeDesktopDimension(declaredHeight), + } +} + +// DeclaredPointToNative maps a point from declared model-facing coordinates to +// native desktop coordinates using the existing pixel-center truncation rule. +func (g DesktopGeometry) DeclaredPointToNative(x, y int) (nativeX, nativeY int) { + return scaleDesktopCoordinate(x, g.DeclaredWidth, g.NativeWidth), + scaleDesktopCoordinate(y, g.DeclaredHeight, g.NativeHeight) +} + +// NativePointToDeclared maps a point from native desktop coordinates to the +// declared model-facing coordinate space using the same truncating transform. +func (g DesktopGeometry) NativePointToDeclared(x, y int) (declaredX, declaredY int) { + return scaleDesktopCoordinate(x, g.NativeWidth, g.DeclaredWidth), + scaleDesktopCoordinate(y, g.NativeHeight, g.DeclaredHeight) +} + +func computeDeclaredDesktopSize(nativeWidth, nativeHeight int) (declaredWidth, declaredHeight int) { + if desktopSizeFitsDeclaredLimits(nativeWidth, nativeHeight) { + return nativeWidth, nativeHeight + } + + if nativeWidth >= nativeHeight { + for _, declaredWidth := range preferredDeclaredDesktopWidths { + if declaredWidth > nativeWidth { + continue + } + + declaredHeight := max(1, declaredWidth*nativeHeight/nativeWidth) + if desktopSizeFitsDeclaredLimits(declaredWidth, declaredHeight) { + return declaredWidth, declaredHeight + } + } + } + + return computeGenericDeclaredDesktopSize(nativeWidth, nativeHeight) +} + +func desktopSizeFitsDeclaredLimits(width, height int) bool { + return max(width, height) <= desktopDeclaredMaxLongEdge && + width*height <= desktopDeclaredMaxTotalPixels +} + +func computeGenericDeclaredDesktopSize(width, height int) (scaledWidth, scaledHeight int) { + longEdge := max(width, height) + totalPixels := width * height + longEdgeScale := float64(desktopDeclaredMaxLongEdge) / float64(longEdge) + totalPixelsScale := math.Sqrt( + float64(desktopDeclaredMaxTotalPixels) / float64(totalPixels), + ) + scale := min(1.0, longEdgeScale, totalPixelsScale) + + if scale >= 1.0 { + return width, height + } + + return max(1, int(float64(width)*scale)), + max(1, int(float64(height)*scale)) +} + +func scaleDesktopCoordinate(coord, fromDim, toDim int) int { + if toDim <= 0 { + return 0 + } + if fromDim <= 0 || fromDim == toDim { + return clampDesktopCoordinate(coord, toDim) + } + + scaled := (float64(coord)+0.5)*float64(toDim)/float64(fromDim) - 0.5 + scaled = math.Max(scaled, 0) + scaled = math.Min(scaled, float64(toDim-1)) + return int(math.Round(scaled)) +} + +func clampDesktopCoordinate(coord, dim int) int { + if dim <= 0 { + return 0 + } + if coord < 0 { + return 0 + } + if coord >= dim { + return dim - 1 + } + return coord +} + +func sanitizeDesktopDimension(dim int) int { + if dim <= 0 { + return 1 + } + return dim +} diff --git a/codersdk/workspacesdk/display_test.go b/codersdk/workspacesdk/display_test.go new file mode 100644 index 0000000000..6ff5606c86 --- /dev/null +++ b/codersdk/workspacesdk/display_test.go @@ -0,0 +1,213 @@ +package workspacesdk_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/coder/coder/v2/codersdk/workspacesdk" +) + +func TestNewDesktopGeometry(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + nativeWidth int + nativeHeight int + declaredWidth int + declaredHeight int + }{ + { + name: "1366x768_keeps_native_geometry", + nativeWidth: 1366, + nativeHeight: 768, + declaredWidth: 1366, + declaredHeight: 768, + }, + { + name: "1920x1080_prefers_1280x720", + nativeWidth: 1920, + nativeHeight: 1080, + declaredWidth: 1280, + declaredHeight: 720, + }, + { + name: "1920x1200_prefers_1280x800", + nativeWidth: 1920, + nativeHeight: 1200, + declaredWidth: 1280, + declaredHeight: 800, + }, + { + name: "2048x1536_prefers_1024x768", + nativeWidth: 2048, + nativeHeight: 1536, + declaredWidth: 1024, + declaredHeight: 768, + }, + { + name: "3840x2160_prefers_1280x720", + nativeWidth: 3840, + nativeHeight: 2160, + declaredWidth: 1280, + declaredHeight: 720, + }, + { + name: "1568x1000_prefers_1280x816", + nativeWidth: 1568, + nativeHeight: 1000, + declaredWidth: 1280, + declaredHeight: 816, + }, + { + name: "portrait_falls_back_to_generic_scaling", + nativeWidth: 1000, + nativeHeight: 2000, + declaredWidth: 758, + declaredHeight: 1516, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + geometry := workspacesdk.NewDesktopGeometry( + tt.nativeWidth, + tt.nativeHeight, + ) + + assert.Equal(t, tt.nativeWidth, geometry.NativeWidth) + assert.Equal(t, tt.nativeHeight, geometry.NativeHeight) + assert.Equal(t, tt.declaredWidth, geometry.DeclaredWidth) + assert.Equal(t, tt.declaredHeight, geometry.DeclaredHeight) + assert.LessOrEqual(t, max(geometry.DeclaredWidth, geometry.DeclaredHeight), 1568) + assert.LessOrEqual(t, geometry.DeclaredWidth*geometry.DeclaredHeight, 1_150_000) + }) + } +} + +func TestDefaultDesktopGeometry(t *testing.T) { + t.Parallel() + + geometry := workspacesdk.DefaultDesktopGeometry() + + assert.Equal(t, workspacesdk.DesktopNativeWidth, geometry.NativeWidth) + assert.Equal(t, workspacesdk.DesktopNativeHeight, geometry.NativeHeight) + assert.Equal(t, 1280, geometry.DeclaredWidth) + assert.Equal(t, 720, geometry.DeclaredHeight) +} + +func TestDesktopGeometryDeclaredPointToNative(t *testing.T) { + t.Parallel() + + geometry := workspacesdk.NewDesktopGeometryWithDeclared(1920, 1080, 1280, 720) + + tests := []struct { + name string + x int + y int + wantX int + wantY int + }{ + { + name: "origin", + x: 0, + y: 0, + wantX: 0, + wantY: 0, + }, + { + name: "center", + x: 640, + y: 360, + wantX: 960, + wantY: 540, + }, + { + name: "max_coordinate_maps_to_last_native_pixel", + x: 1279, + y: 719, + wantX: 1919, + wantY: 1079, + }, + { + name: "out_of_bounds_values_are_clamped", + x: 5000, + y: -5, + wantX: 1919, + wantY: 0, + }, + { + name: "rounding_applies", + x: 853, + y: 402, + wantX: 1280, + wantY: 603, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gotX, gotY := geometry.DeclaredPointToNative(tt.x, tt.y) + assert.Equal(t, tt.wantX, gotX) + assert.Equal(t, tt.wantY, gotY) + }) + } +} + +func TestDesktopGeometryNativePointToDeclared(t *testing.T) { + t.Parallel() + + geometry := workspacesdk.NewDesktopGeometryWithDeclared(1920, 1080, 1366, 768) + + tests := []struct { + name string + x int + y int + wantX int + wantY int + }{ + { + name: "origin", + x: 0, + y: 0, + wantX: 0, + wantY: 0, + }, + { + name: "center", + x: 960, + y: 540, + wantX: 683, + wantY: 384, + }, + { + name: "bottom_right_maps_to_last_pixel", + x: 1919, + y: 1079, + wantX: 1365, + wantY: 767, + }, + { + name: "out_of_bounds_values_are_clamped", + x: -10, + y: 5000, + wantX: 0, + wantY: 767, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gotX, gotY := geometry.NativePointToDeclared(tt.x, tt.y) + assert.Equal(t, tt.wantX, gotX) + assert.Equal(t, tt.wantY, gotY) + }) + } +}