From 0c744c1fabc31eae55149fe02ff6f7e1b9f551f4 Mon Sep 17 00:00:00 2001 From: Hasan Amin Date: Sat, 12 Sep 2026 15:48:52 -0400 Subject: [PATCH] reject stale wda element bounds --- CHANGELOG.md | 3 + pkg/driver/wda/driver.go | 14 ++-- pkg/driver/wda/stale_element_test.go | 110 +++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 pkg/driver/wda/stale_element_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 37c28ae7..551670ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- WDA rejects missing or invalid element bounds and retries selector lookup, preventing stale elements from producing successful taps at `(0, 0)`. + ## [1.1.27] - 2026-09-11 Mostly about **flows that already work on Maestro working the same way here**. One of 1.1.26's selector fixes woke up a parsing bug that had been dormant for months: any `inputText` that named an `id:` stopped finding its field, and because that step usually lives in a shared login sub-flow, whole suites went red on their first command. That is fixed, and so are three places where a Maestro flow was read differently here without a word — `speed:` on scrolls, the `value:` spelling of airplane and dark mode, and an element-relative `point:` on swipes. `addMedia` now takes documents, so a flow can seed a PDF and pick it in the system file picker, and a password passed through a variable no longer ends up in the report. There are no behaviour changes: nothing that passes on 1.1.26 should start failing. diff --git a/pkg/driver/wda/driver.go b/pkg/driver/wda/driver.go index dd0c7d6c..9118ee2d 100644 --- a/pkg/driver/wda/driver.go +++ b/pkg/driver/wda/driver.go @@ -909,12 +909,17 @@ func (d *Driver) getElementInfo(elemID string) (*core.ElementInfo, error) { }() wg.Wait() + if rectErr != nil { + return nil, fmt.Errorf("element bounds unavailable: %w", rectErr) + } + if w <= 0 || h <= 0 { + return nil, fmt.Errorf("element has invalid bounds: %dx%d", w, h) + } + info.Bounds = core.Bounds{X: x, Y: y, Width: w, Height: h} + if textErr == nil { info.Text = text } - if rectErr == nil { - info.Bounds = core.Bounds{X: x, Y: y, Width: w, Height: h} - } if nameErr == nil { info.Class = elemName } @@ -924,9 +929,6 @@ func (d *Driver) getElementInfo(elemID string) (*core.ElementInfo, error) { // XCUITest says off-screen. Check bounds geometrically — if they're // inside the viewport, override XCUITest and accept the element with // a MatchNote so the report records the override. - if rectErr != nil { - return nil, fmt.Errorf("element exists but is not visible on screen (no bounds)") - } screenW, screenH, sErr := d.screenSize() if sErr != nil { return nil, fmt.Errorf("element exists but is not visible on screen (screen size unavailable)") diff --git a/pkg/driver/wda/stale_element_test.go b/pkg/driver/wda/stale_element_test.go new file mode 100644 index 00000000..1695ac49 --- /dev/null +++ b/pkg/driver/wda/stale_element_test.go @@ -0,0 +1,110 @@ +package wda + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "github.com/devicelab-dev/maestro-runner/pkg/flow" +) + +func TestTapOnStaleElementRecovery(t *testing.T) { + for _, present := range []bool{true, false} { + name := "element disappeared" + if present { + name = "element remains in page source" + } + t.Run(name, func(t *testing.T) { + var reads atomic.Int32 + taps := make(chan [2]float64, 8) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasSuffix(r.URL.Path, "/element"): + jsonResponse(w, map[string]any{"value": map[string]string{"ELEMENT": "stale"}}) + case strings.Contains(r.URL.Path, "/element/stale/"): + w.WriteHeader(http.StatusNotFound) + jsonResponse(w, map[string]any{"value": map[string]string{ + "error": "stale element reference", "message": "element is no longer attached", + }}) + case strings.HasSuffix(r.URL.Path, "/source"): + reads.Add(1) + source := `` + if present { + source = `` + } + jsonResponse(w, map[string]any{"value": source}) + case strings.HasSuffix(r.URL.Path, "/wda/tap"): + var point struct{ X, Y float64 } + if err := json.NewDecoder(r.Body).Decode(&point); err != nil { + t.Error(err) + } + taps <- [2]float64{point.X, point.Y} + jsonResponse(w, map[string]any{"value": nil}) + case strings.HasSuffix(r.URL.Path, "/window/size"): + jsonResponse(w, map[string]any{"value": map[string]int{"width": 390, "height": 844}}) + default: + t.Errorf("unexpected request: %s", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + result := createTestDriver(server).tapOn(&flow.TapOnStep{ + Selector: flow.Selector{Text: "Cancel"}, + BaseStep: flow.BaseStep{TimeoutMs: 100}, + }) + if result.Success != present { + t.Fatalf("success = %v, want %v: %s", result.Success, present, result.Message) + } + if reads.Load() == 0 { + t.Error("stale element must be resolved from a fresh page source") + } + select { + case point := <-taps: + if !present || point != [2]float64{42, 82} { + t.Errorf("unexpected tap at %v", point) + } + default: + if present { + t.Error("expected a tap on the recovered element") + } + } + }) + } +} + +func TestGetElementInfoRejectsInvalidBounds(t *testing.T) { + for _, bounds := range []struct { + name string + width, height int + }{ + {"zero width", 0, 44}, + {"zero height", 44, 0}, + {"negative width", -1, 44}, + {"negative height", 44, -1}, + } { + t.Run(bounds.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasSuffix(r.URL.Path, "/rect"): + jsonResponse(w, map[string]any{"value": map[string]int{ + "x": 20, "y": 60, "width": bounds.width, "height": bounds.height, + }}) + case strings.HasSuffix(r.URL.Path, "/displayed"): + jsonResponse(w, map[string]any{"value": true}) + default: + jsonResponse(w, map[string]any{"value": "Cancel"}) + } + })) + defer server.Close() + if _, err := createTestDriver(server).getElementInfo("cancel"); err == nil { + t.Error("expected invalid element bounds to be rejected") + } + }) + } +}