fix(wda): an element is not visible only when a lookup finds it absent

CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+- **WDA keeps enough idle connections for its own parallel reads.** The driver reads an element's name, rect, text and displayed at once, and a tap looks an element up four ways at once, but Go's default transport keeps two idle connections per host, so every burst closed two connections and opened two new ones. Through a forwarded port to a physical iPhone, new connections opened together fail with EOF and are sent again, which costs time on every step. The WDA client now keeps up to eight.
+- **`retry` counts retries, not attempts, as Maestro does.** `maxRetries: 1` now runs the commands twice (once, then one retry), an unset `maxRetries` means one retry, and the value is capped at 3. The runner ran exactly `maxRetries` attempts, three when unset and with no cap, so `maxRetries: 1` never retried. A value that is not an integer is logged and read as 1 instead of failing the step.
+- **WDA `launchApp` restarts a running app unless `stopApp: false`, as Maestro does.** It only activated the running app, so a relaunch left the app on the screen it was already on, and a flow checking what survives a restart restarted nothing.
+- **WDA `notVisible` passes only when a lookup finds the element absent.** `assertNotVisible` and `extendedWaitUntil: notVisible` treated any failed lookup, such as an unreadable page source or a dropped connection, as the element being gone, so they could pass without the screen being looked at. Other errors are now retried until the timeout, and then fail the step.
+
 ## [1.1.28] - 2026-09-30
 
 This release is about **running a Maestro suite and getting Maestro's answer**. Most of it came from running real Maestro suites (DuckDuckGo Android and iOS, React Native's RNTester, React Navigation) side by side with Maestro and fixing each place the two disagreed. The largest of those is selector matching: text and id selectors now match the whole value, as Maestro does, which is a behaviour change — see below. `--driver devicelab` on iOS now runs a new agent, on simulators and on real iPhones. DeviceLab becomes the default Android driver. Every flow gets `DEVICE_UDID`, `-e` stops splitting values at commas, and the Appium driver closes a long list of gaps with the native drivers.

pkg/driver/wda/client.go

@@ -34,11 +34,32 @@ func NewClient(port uint16) *Client {
 	return &Client{
 		baseURL: fmt.Sprintf("http://127.0.0.1:%d", port),
 		httpClient: &http.Client{
-			Timeout: 60 * time.Second,
+			Timeout:   60 * time.Second,
+			Transport: newWDATransport(),
 		},
 	}
 }
 
+// wdaIdleConnsPerHost is how many idle connections the client keeps open to WebDriverAgent.
+// The driver sends up to four requests at once (an element's name, rect, text and displayed;
+// a tap's four lookups), and Go keeps two idle connections per host by default, so every such
+// burst closed two connections and opened two new ones. On a real iPhone reached through a
+// forward (SSH, then iproxy), a new connection took about 300 ms, and the two opened together
+// failed at once with EOF in 297 of 996 bursts of one 44-flow run, always both of them and
+// never a kept connection. Kept for every request of a burst, no new ones are needed.
+const wdaIdleConnsPerHost = 8
+
+// newWDATransport is Go's default transport with room to keep a whole burst's connections.
+func newWDATransport() *http.Transport {
+	base, ok := http.DefaultTransport.(*http.Transport)
+	if !ok {
+		base = &http.Transport{Proxy: http.ProxyFromEnvironment}
+	}
+	t := base.Clone()
+	t.MaxIdleConnsPerHost = wdaIdleConnsPerHost
+	return t
+}
+
 // Session management
 
 // CreateSession creates a new WDA session.

pkg/driver/wda/commands.go

@@ -403,8 +403,10 @@ func (d *Driver) countVisibleMatchesOnce(sel flow.Selector) (int, error) {
 
 func (d *Driver) assertNotVisible(step *flow.AssertNotVisibleStep) *core.CommandResult {
 	// Poll with quick checks, waiting for element to disappear.
-	// Each check is a single lookup (no retries). If element is not found
-	// at any point, we pass immediately. If still visible at timeout, fail.
+	// Each check is a single lookup (no retries). If a lookup reads the
+	// screen and finds no match, we pass immediately. A lookup that could not
+	// read the screen proves nothing, so polling goes on, as it does while the
+	// element is still visible, and the step fails at timeout.
 	timeoutMs := step.TimeoutMs
 	if timeoutMs <= 0 {
 		timeoutMs = 5000
@@ -415,11 +417,14 @@ func (d *Driver) assertNotVisible(step *flow.AssertNotVisibleStep) *core.Command
 
 	for {
 		info, err := d.findElementOnce(step.Selector)
-		if err != nil || info == nil {
+		if isNotFound(err) || (err == nil && info == nil) {
 			return successResult("Element is not visible", nil)
 		}
 
 		if time.Now().After(deadline) {
+			if err != nil {
+				return errorResult(err, fmt.Sprintf("Could not check that %s is not visible: %v", selectorDesc(step.Selector), err))
+			}
 			return errorResult(fmt.Errorf("element is visible"), fmt.Sprintf("Element should not be visible: %s", selectorDesc(step.Selector)))
 		}
 
@@ -1203,9 +1208,12 @@ func (d *Driver) launchApp(step *flow.LaunchAppStep) *core.CommandResult {
 		return successResult(fmt.Sprintf("Launched app: %s", bundleID), nil)
 	}
 
-	// Terminate the app first so WDA calls launch (not activate),
-	// which is required for arguments/environment to take effect
-	if hasArgs {
+	// Terminate the app first so WDA calls launch (not activate). Arguments and
+	// environment only take effect on a real launch, and Maestro stops the app
+	// before launching it unless the flow says `stopApp: false`. Without the stop,
+	// a relaunch only brought a running app to the front, still where it was, so
+	// a flow checking what survives a restart tested nothing.
+	if hasArgs || step.StopApp == nil || *step.StopApp {
 		_ = d.client.TerminateApp(bundleID)
 	}
 
@@ -1626,6 +1634,10 @@ func (d *Driver) waitUntil(step *flow.WaitUntilStep) *core.CommandResult {
 		selector = step.NotVisible
 	}
 
+	// The last notVisible check that could not read the screen, if the last
+	// check was one.
+	var unreadable error
+
 	for {
 		select {
 		case <-ctx.Done():
@@ -1636,6 +1648,10 @@ func (d *Driver) waitUntil(step *flow.WaitUntilStep) *core.CommandResult {
 					fmt.Sprintf("Element '%s' not visible within %v", selector.Describe(), timeout),
 				)
 			}
+			if unreadable != nil {
+				return errorResult(unreadable,
+					fmt.Sprintf("Could not check that '%s' is not visible within %v: %v", selector.Describe(), timeout, unreadable))
+			}
 			return errorResult(
 				context.DeadlineExceeded,
 				fmt.Sprintf("Element '%s' still visible after %v", selector.Describe(), timeout),
@@ -1648,11 +1664,18 @@ func (d *Driver) waitUntil(step *flow.WaitUntilStep) *core.CommandResult {
 					return successResult("Element became visible", info)
 				}
 			} else {
-				// Single attempt for not visible check
+				// Single attempt for not visible check. Only a lookup that
+				// read the screen and found no match means the element is
+				// gone; one that could not read it is tried again.
 				info, err := d.findElementOnce(*step.NotVisible)
-				if err != nil || info == nil {
+				if isNotFound(err) || (err == nil && info == nil) {
 					return successResult("Element became not visible", nil)
 				}
+				unreadable = err
+				if unreadable != nil {
+					// A refused connection fails at once; do not spin on it.
+					time.Sleep(50 * time.Millisecond)
+				}
 			}
 			// HTTP round-trip (~100ms) is natural rate limit, no sleep needed
 		}

pkg/driver/wda/driver.go

@@ -1181,7 +1181,7 @@ func (d *Driver) resolveRelativeSelector(sel flow.Selector, allElements []*Parse
 	if anchorSelector != nil {
 		anchors := FilterBySelector(allElements, *anchorSelector)
 		if len(anchors) == 0 {
-			return nil, fmt.Errorf("anchor element not found")
+			return nil, notFound("anchor element not found")
 		}
 
 		var matchingCandidates []*ParsedElement
@@ -1203,7 +1203,7 @@ func (d *Driver) resolveRelativeSelector(sel flow.Selector, allElements []*Parse
 	// Bounds-based visibility (FilterOutOfBounds already applied above);
 	// XCUITest's `visible="false"` is unreliable on RN testID wrappers.
 	if len(candidates) == 0 {
-		return nil, fmt.Errorf("no elements match selector")
+		return nil, notFound("no elements match selector")
 	}
 
 	// Prioritize clickable/interactive elements
@@ -1262,10 +1262,10 @@ func (d *Driver) findElementByPageSourceOnce(sel flow.Selector) (*core.ElementIn
 	if len(candidates) == 0 {
 		if sel.Text != "" {
 			if closest := ClosestTexts(allElements, sel.Text, 3); len(closest) > 0 {
-				return nil, fmt.Errorf("no elements match selector; closest on-screen texts: %s", strings.Join(closest, ", "))
+				return nil, notFound("no elements match selector; closest on-screen texts: %s", strings.Join(closest, ", "))
 			}
 		}
-		return nil, fmt.Errorf("no elements match selector")
+		return nil, notFound("no elements match selector")
 	}
 
 	// Prioritize clickable/interactive elements

pkg/driver/wda/driver_test.go

@@ -4864,7 +4864,14 @@ func TestInputTextTapError(t *testing.T) {
 func TestAssertNotVisibleWithTimeout(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
-		// Return error to indicate element not found
+		// The screen reads, and holds nothing matching. (An error from
+		// /source would not mean "not found"; see notvisible_test.go.)
+		if strings.HasSuffix(r.URL.Path, "/source") {
+			jsonResponse(w, map[string]interface{}{
+				"value": `<AppiumAUT><XCUIElementTypeApplication type="XCUIElementTypeApplication" x="0" y="0" width="390" height="844"/></AppiumAUT>`,
+			})
+			return
+		}
 		jsonResponse(w, map[string]interface{}{
 			"value": map[string]interface{}{"error": "not found"},
 		})

pkg/driver/wda/kept_connections_test.go

@@ -0,0 +1,115 @@
+package wda
+
+import (
+	"fmt"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"strings"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/devicelab-dev/maestro-runner/pkg/core"
+)
+
+// keptConnServer answers an element's four property reads, each after a short delay so the
+// four of a burst are in flight together, as they are on a phone. It counts the connections
+// it accepted, and with dropNewAfter > 0 closes, without an answer, every new connection past
+// that many: a forward whose new connections fail, while kept ones work.
+type keptConnServer struct {
+	server       *httptest.Server
+	accepted     int32
+	dropped      int32
+	dropNewAfter int32
+}
+
+func newKeptConnServer(t *testing.T, dropNewAfter int32) *keptConnServer {
+	t.Helper()
+	es := &keptConnServer{dropNewAfter: dropNewAfter}
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(20 * time.Millisecond)
+		switch {
+		case strings.HasSuffix(r.URL.Path, "/rect"):
+			jsonResponse(w, map[string]interface{}{"value": map[string]interface{}{"x": 10, "y": 20, "width": 100, "height": 40}})
+		case strings.HasSuffix(r.URL.Path, "/displayed"):
+			jsonResponse(w, map[string]interface{}{"value": true})
+		default:
+			jsonResponse(w, map[string]interface{}{"value": "Continue"})
+		}
+	})
+	es.server = httptest.NewUnstartedServer(handler)
+	es.server.Config.ConnState = func(c net.Conn, state http.ConnState) {
+		if state != http.StateNew {
+			return
+		}
+		n := atomic.AddInt32(&es.accepted, 1)
+		if es.dropNewAfter > 0 && n > es.dropNewAfter {
+			atomic.AddInt32(&es.dropped, 1)
+			_ = c.Close()
+		}
+	}
+	es.server.Start()
+	t.Cleanup(es.server.Close)
+	return es
+}
+
+// driverFor is a driver whose client is built the way the runner builds it.
+func (es *keptConnServer) driverFor(t *testing.T) *Driver {
+	t.Helper()
+	_, portText, err := net.SplitHostPort(strings.TrimPrefix(es.server.URL, "http://"))
+	if err != nil {
+		t.Fatalf("server address %q: %v", es.server.URL, err)
+	}
+	port, _ := strconv.Atoi(portText)
+	client := NewClient(uint16(port))
+	client.baseURL = es.server.URL // the listener is on 127.0.0.1, not localhost's first address
+	client.sessionID = "s1"
+	return &Driver{client: client, info: &core.PlatformInfo{Platform: "ios", ScreenWidth: 390, ScreenHeight: 844}}
+}
+
+// Fifty elements' reads, four at a time, open four connections and then keep them, instead of
+// closing two and opening two for every element.
+func TestElementReadsKeepTheirConnections(t *testing.T) {
+	es := newKeptConnServer(t, 0)
+	d := es.driverFor(t)
+	for i := 0; i < 50; i++ {
+		if _, err := d.getElementInfo(fmt.Sprintf("E%d", i)); err != nil {
+			t.Fatalf("element %d: %v", i, err)
+		}
+	}
+	if n := atomic.LoadInt32(&es.accepted); n > 4 {
+		t.Errorf("50 bursts of four reads opened %d connections, want at most 4", n)
+	}
+}
+
+// Where a new connection fails (the phone's forward), only the first burst opens any, so the
+// reads after it never meet a failing one.
+func TestElementReadsSurviveAForwardWhoseNewConnectionsFail(t *testing.T) {
+	es := newKeptConnServer(t, 4)
+	d := es.driverFor(t)
+	var mu sync.Mutex
+	failed := 0
+	for i := 0; i < 50; i++ {
+		if _, err := d.getElementInfo(fmt.Sprintf("E%d", i)); err != nil {
+			mu.Lock()
+			failed++
+			mu.Unlock()
+		}
+	}
+	if dropped := atomic.LoadInt32(&es.dropped); dropped != 0 || failed != 0 {
+		t.Errorf("after the first burst, %d new connections were needed and dropped, and %d reads failed; want none", dropped, failed)
+	}
+}
+
+func TestTheWDATransportKeepsABurstsConnections(t *testing.T) {
+	tr := newWDATransport()
+	if tr.MaxIdleConnsPerHost < 4 {
+		t.Errorf("MaxIdleConnsPerHost = %d, fewer than the driver's four requests at once", tr.MaxIdleConnsPerHost)
+	}
+	if tr.Proxy == nil || tr.DialContext == nil {
+		t.Error("the WDA transport lost the default transport's proxy or dialer")
+	}
+}

pkg/driver/wda/launch_stop_test.go

@@ -0,0 +1,81 @@
+package wda
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/devicelab-dev/maestro-runner/pkg/core"
+	"github.com/devicelab-dev/maestro-runner/pkg/flow"
+)
+
+// launchWithSession runs launchApp against a WDA that already has a session, the
+// usual case for every launchApp after a flow's first, and returns the app
+// lifecycle calls it made, in order.
+func launchWithSession(t *testing.T, step *flow.LaunchAppStep) []string {
+	t.Helper()
+	var mu sync.Mutex
+	var calls []string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		mu.Lock()
+		defer mu.Unlock()
+		switch {
+		case strings.HasSuffix(r.URL.Path, "/wda/apps/terminate"):
+			calls = append(calls, "terminate")
+		case strings.HasSuffix(r.URL.Path, "/wda/apps/launch"):
+			calls = append(calls, "launch")
+		case r.URL.Path == "/session" && r.Method == http.MethodPost:
+			calls = append(calls, "new-session")
+		}
+		jsonResponse(w, map[string]interface{}{"value": nil})
+	}))
+	defer server.Close()
+
+	client := &Client{baseURL: server.URL, httpClient: http.DefaultClient, sessionID: "test-session"}
+	driver := &Driver{client: client, info: &core.PlatformInfo{Platform: "ios"}}
+	if result := driver.launchApp(step); !result.Success {
+		t.Fatalf("launchApp failed: %s", result.Message)
+	}
+	mu.Lock()
+	defer mu.Unlock()
+	return append([]string(nil), calls...)
+}
+
+// Maestro stops the app before launching it by default. A relaunch that only
+// activated the running app left it where it was, so a flow asserting what a
+// restart preserves passed or failed on the pre-restart screen.
+func TestLaunchAppStopsARunningAppFirstByDefault(t *testing.T) {
+	got := strings.Join(launchWithSession(t, &flow.LaunchAppStep{AppID: "com.test.app"}), ",")
+	if got != "terminate,launch" {
+		t.Fatalf("lifecycle calls %q, want terminate,launch", got)
+	}
+}
+
+func TestLaunchAppStopAppTrueStopsFirst(t *testing.T) {
+	stop := true
+	got := strings.Join(launchWithSession(t, &flow.LaunchAppStep{AppID: "com.test.app", StopApp: &stop}), ",")
+	if got != "terminate,launch" {
+		t.Fatalf("lifecycle calls %q, want terminate,launch", got)
+	}
+}
+
+func TestLaunchAppStopAppFalseOnlyBringsTheAppForward(t *testing.T) {
+	stop := false
+	got := strings.Join(launchWithSession(t, &flow.LaunchAppStep{AppID: "com.test.app", StopApp: &stop}), ",")
+	if got != "launch" {
+		t.Fatalf("lifecycle calls %q, want launch alone", got)
+	}
+}
+
+// Arguments need a real launch, so they still stop the app even with stopApp: false.
+func TestLaunchAppArgumentsStillStopTheApp(t *testing.T) {
+	stop := false
+	step := &flow.LaunchAppStep{AppID: "com.test.app", StopApp: &stop, Arguments: map[string]any{"e2e": true}}
+	got := strings.Join(launchWithSession(t, step), ",")
+	if got != "terminate,launch" {
+		t.Fatalf("lifecycle calls %q, want terminate,launch", got)
+	}
+}