Merge branch 'sidecar-adb-and-ime' into trial-merge

.github/workflows/ci.yml

@@ -95,6 +95,19 @@ jobs:
       - name: Run tests
         run: make test
 
+      # folio is its own gradle build, and the metro plugin it compiles with
+      # needs a 21 runtime where the sidecar toolchain pins 17. Switching
+      # JAVA_HOME after `make test` rather than installing both up front
+      # leaves every step above this one on exactly the JDK it ran on before.
+      - name: Set up JDK 21 for folio
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          java-version: "21"
+
+      - name: Run folio's unit tests
+        run: make test-folio
+
   browser:
     runs-on: ubuntu-latest
     steps:

Makefile

@@ -29,7 +29,7 @@ WEB_DIST := replay-ui/dist
 
 GOLINES := $(shell $(GO) env GOPATH)/bin/golines
 
-.PHONY: bootstrap proto sidecar sidecar-embed sanderling sanderling-web sanderling-android sanderling-ios install test test-go test-browser test-companion test-kotlin test-spec-api test-ci-scripts spec-typecheck web-test web-typecheck web-build web-dev replay-dev docs clean release-cli release-npm-dry fmt fmt-go fmt-kotlin fmt-ts fmt-swift
+.PHONY: bootstrap proto sidecar sidecar-embed sanderling sanderling-web sanderling-android sanderling-ios install test test-go test-browser test-companion test-kotlin test-folio test-spec-api test-ci-scripts spec-typecheck web-test web-typecheck web-build web-dev replay-dev docs clean release-cli release-npm-dry fmt fmt-go fmt-kotlin fmt-ts fmt-swift
 
 bootstrap:
 	$(GO) mod download
@@ -145,6 +145,14 @@ test-companion: $(COMPANION_EMBED) $(RUNNER_EMBED)
 test-kotlin:
 	ANDROID_HOME=$(ANDROID_HOME) $(GRADLE) :sidecar:test
 
+# folio is its own gradle build, so nothing in the root build runs its tests.
+# Kept out of `test` because the metro plugin folio compiles with needs a 21+
+# runtime, where the sidecar toolchain pins 17: folding this in would raise the
+# JDK floor of the target everyone runs constantly. CI runs it as its own step.
+test-folio:
+	cd examples/folio && ANDROID_HOME=$(ANDROID_HOME) $(GRADLE) \
+		:core:testDebugUnitTest :app:shared:testDebugUnitTest
+
 # The CI scripts that read a trace and decide whether a green leg is
 # evidence. bash and python3 only, which is all a runner has.
 test-ci-scripts:

examples/folio/justfile

@@ -78,6 +78,13 @@ _ensure-device:
     echo "emulator did not finish booting in time (see /tmp/folio-emulator.log)" >&2
     exit 1
 
+# Run folio's own unit tests. Named test-unit because `test` is the fuzz run.
+test-unit:
+    #!/usr/bin/env bash
+    set -euo pipefail
+    export ANDROID_HOME="$(just _android-home)"
+    ./gradlew :core:testDebugUnitTest :app:shared:testDebugUnitTest
+
 # Build the folio debug APK without installing it.
 build:
     #!/usr/bin/env bash

sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt

@@ -329,11 +329,12 @@ internal fun readLogcat(
         arguments.add(since)
     }
     return try {
-        val process = ProcessBuilder(
-            adbCmd(serial) + arguments,
-        ).redirectErrorStream(false).start()
-        val output = process.inputStream.bufferedReader().readText()
-        process.waitFor()
+        val command = adbCmd(serial) + arguments
+        val output = readProcessOutput(
+            ProcessBuilder(command).redirectErrorStream(false).start(),
+            ADB_OUTPUT_TIMEOUT_MILLIS,
+            describe = { command.joinToString(" ") },
+        )
         StubDriverBackend.parseLogcatOutput(output)
     } catch (cause: Exception) {
         println("adb logcat failed: $cause")
@@ -359,13 +360,76 @@ internal fun readProcMetrics(serial: String?, bundleId: String): MetricsSample {
 private fun adbCmd(serial: String?): List<String> =
     if (serial == null) listOf("adb") else listOf("adb", "-s", serial)
 
-private fun adbOutput(serial: String?, arguments: List<String>): String = try {
-    val process = ProcessBuilder(
-        adbCmd(serial) + arguments,
-    ).redirectErrorStream(false).start()
-    val output = process.inputStream.bufferedReader().readText()
-    process.waitFor()
+// ADB_OUTPUT_TIMEOUT_MILLIS bounds the diagnostic adb reads: dumpsys, logcat,
+// `settings get`, /proc stats. None of them is the driver's data path, so the
+// bound wants to be generous enough that it cannot fire on a link that works,
+// and it is: a hierarchy fetch, far heavier than any of these, measures at a
+// 76ms median and a 168ms p90 over the same remote adb link. What it caps is
+// the other end, where a wedged adb once held a step ~100s.
+internal const val ADB_OUTPUT_TIMEOUT_MILLIS = 10_000L
+
+private val adbReaders: java.util.concurrent.ExecutorService =
+    java.util.concurrent.Executors.newCachedThreadPool { runnable ->
+        Thread(runnable, "adb-output").apply { isDaemon = true }
+    }
+
+// readProcessOutput returns a process's stdout, or "" when it does not arrive
+// inside timeoutMillis.
+//
+// The bound belongs on the READ, not on waitFor. readText ends at EOF, and a
+// wedged adb neither writes nor exits, so EOF never comes and a waitFor with a
+// timeout after it is a line that never runs. Waiting first and reading after
+// is worse still: a process with more to say than a pipe buffer holds, which
+// logcat and dumpsys both are, blocks writing while the waiter waits for it to
+// finish, and neither ever moves.
+//
+// So the read runs on a daemon thread and killing the process is what releases
+// it: destroy closes the pipe, the reader sees EOF, the thread ends. Returning
+// "" hands every caller the answer it already treats as "adb said nothing",
+// which is the safe direction for all of them.
+internal fun readProcessOutput(
+    process: Process,
+    timeoutMillis: Long,
+    describe: () -> String,
+    log: (String) -> Unit = { System.err.println(it) },
+): String {
+    val reader = adbReaders.submit<String> {
+        process.inputStream.bufferedReader().readText()
+    }
+    return try {
+        val output = reader.get(
+            timeoutMillis,
+            java.util.concurrent.TimeUnit.MILLISECONDS,
+        )
+        if (!process.waitFor(
+                timeoutMillis,
+                java.util.concurrent.TimeUnit.MILLISECONDS,
+            )
+        ) {
+            process.destroyForcibly()
+        }
         output
+    } catch (cause: java.util.concurrent.TimeoutException) {
+        process.destroyForcibly()
+        reader.cancel(true)
+        log(
+            "warn: ${describe()} gave nothing in ${timeoutMillis}ms; " +
+                "killed it and read no answer",
+        )
+        ""
+    } catch (cause: Exception) {
+        process.destroyForcibly()
+        ""
+    }
+}
+
+private fun adbOutput(serial: String?, arguments: List<String>): String = try {
+    val command = adbCmd(serial) + arguments
+    readProcessOutput(
+        ProcessBuilder(command).redirectErrorStream(false).start(),
+        ADB_OUTPUT_TIMEOUT_MILLIS,
+        describe = { command.joinToString(" ") },
+    )
 } catch (cause: Exception) {
     ""
 }
@@ -474,8 +538,14 @@ class StubDriverBackend(
     companion object {
         private const val IDLE_POLL_INTERVAL_MILLIS = 50L
 
+        // A count we could not read is not a count of zero. Defaulting it to
+        // zero made an unreadable dumpsys mean "nothing is animating, go
+        // ahead", which is the one answer the caller cannot check: it breaks
+        // out of the settle and snapshots whatever frame is on screen. Unknown
+        // keeps it waiting instead, inside the deadline waitForIdle already
+        // holds, and it agrees with the probe's own exception path.
         internal fun isAnimationCountIdle(grepOutput: String): Boolean =
-            (grepOutput.trim().toIntOrNull() ?: 0) == 0
+            grepOutput.trim().toIntOrNull() == 0
 
         internal fun parseResolvedActivity(
             bundleId: String,
@@ -799,11 +869,107 @@ internal fun typeChunks(
 // every InputText, which raises the IME again long before this probe runs. A
 // caller that dismissed twice in a row, or typed without focusing first, would
 // lose that margin.
+//
+// treeWithoutKeyboard closes a keyboard too, on the snapshot path, and does not
+// cost this one its margin: it reads no flag, and the two are a waitForIdle and
+// a hierarchy fetch apart, several times the window in which this one is stale.
 internal fun dismissSoftKeyboard(shell: (String) -> String) {
     if (!shell("dumpsys input_method").contains("mInputShown=true")) return
     shell("input keyevent 4")
 }
 
+// KEYBOARD_DISMISS_READS bounds the re-reads a snapshot spends waiting for the
+// IME window to leave the tree after BACK. The window leaves over an
+// animation, so the first read back can still carry it. A hierarchy read
+// measures at a 76ms median and a 168ms p90 on the API 34 emulator, so with
+// the interval these four reads watch most of a second: several retractions
+// over, without turning a keyboard the app keeps re-raising into a wait with
+// no end.
+internal const val KEYBOARD_DISMISS_READS = 4
+internal const val KEYBOARD_DISMISS_INTERVAL_MILLIS = 100L
+
+// imePackageOf takes the package half of an input-method component id
+// ("pkg/.Service"), the form both `settings get secure default_input_method`
+// and dumpsys' mCurMethodId use. Anything that is not a package name reads as
+// "no IME known", which disables the dismissal rather than guessing.
+internal fun imePackageOf(component: String): String? =
+    component.trim().substringBefore('/')
+        .takeIf { it.isNotEmpty() && it.contains('.') }
+
+// treeShowsIme reports whether the keyboard window is in the tree, by the view
+// ids the IME's own resources give it ("pkg:id/name").
+internal fun treeShowsIme(treeJson: String, imePackage: String): Boolean =
+    treeJson.contains("$imePackage:id/")
+
+// treeWithoutKeyboard closes a keyboard standing in the snapshot and returns a
+// tree read after it has gone, or the tree it was given when none is open.
+//
+// It belongs here, before the read the picker chooses from, rather than after
+// the tap that raised the keyboard. Two reasons. The picker only ever sees
+// snapshots, so a dismissal anywhere later leaves this step choosing between
+// the handful of targets an open keyboard left in the tree, which is the
+// budget the fuzzer was losing. And the state it has to judge is settled here:
+// the action landed a waitForIdle ago, where straight after the tap the
+// keyboard is still on its way up and nothing it could read would say so yet.
+//
+// The tree is also a better guard than mInputShown. BACK closes an open
+// keyboard and navigates when none is open, so pressing it is only safe on a
+// true reading; mInputShown trails the keyboard by up to 0.6s, while a tree
+// carrying the IME's own view ids is the keyboard being on screen, read a
+// moment ago. Once dismissed, the re-reads confirm it went rather than pressing
+// BACK again, so a keyboard the app puts straight back costs re-reads and never
+// a second back press.
+internal fun treeWithoutKeyboard(
+    tree: String,
+    imePackage: String?,
+    dismiss: () -> Unit,
+    reread: () -> String,
+    sleep: (Long) -> Unit = { Thread.sleep(it) },
+): String {
+    if (imePackage == null || !treeShowsIme(tree, imePackage)) return tree
+    dismiss()
+    var current = tree
+    repeat(KEYBOARD_DISMISS_READS) {
+        sleep(KEYBOARD_DISMISS_INTERVAL_MILLIS)
+        current = reread()
+        if (!treeShowsIme(current, imePackage)) return current
+    }
+    return current
+}
+
+// typingOwner picks what the mid-type foreground guard holds later reads
+// against. A dumpsys it could read names the resumed package, and that is the
+// answer.
+//
+// A read that failed is the interesting case, and neither obvious answer is
+// right. Passing null hands typeChunks "no owner", which switches the guard off
+// altogether and lets the rest of the string spray into whatever holds the
+// foreground: an unreadable probe must never read as focus being fine. But
+// refusing to type is worse in practice. The failure is a degraded link, which
+// lasts, so every InputText in the run becomes a no-op, the budget goes on
+// typing nothing, and the run ends green having tested nothing.
+//
+// So it falls back to the bundle the run launched, which leaves the guard armed
+// against the app the keystrokes were meant for. That reference is better than
+// the resumed package anyway: a foreground already stolen before typing began
+// reads as its own owner, and the guard then matches it happily chunk after
+// chunk.
+internal fun typingOwner(
+    dumpsys: String,
+    launchedBundleId: String?,
+    warn: (String) -> Unit,
+): String? {
+    parseResumedPackage(dumpsys)?.let { return it }
+    warn(
+        "warn: could not read the foreground app; guarding typing with " +
+            (
+                launchedBundleId?.let { "the launched bundle $it" }
+                    ?: "nothing, no launch was recorded"
+                ),
+    )
+    return launchedBundleId
+}
+
 // resumedActivityPackage matches a "package/activity" component, mirroring the
 // Go scope guard's regex so both read the same dumpsys wording.
 private val resumedActivityPackage =
@@ -856,6 +1022,14 @@ internal fun <T> retryOpen(
 class MaestroDriverBackend(private val serial: String?) : DriverBackend {
     private val dadb: dadb.Dadb = buildDadb(serial)
 
+    private val imePackage: String? by lazy {
+        imePackageOf(
+            runCatching {
+                dadb.shell("settings get secure default_input_method").allOutput
+            }.getOrDefault(""),
+        )
+    }
+
     // A fresh AndroidDriver per open attempt. Its gRPC channel is built once in
     // the constructor and permanently shut down by close(), so reopening a
     // closed instance would reuse a dead channel; rebuild it each try instead.
@@ -872,6 +1046,9 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
             }
         }
 
+    @Volatile
+    private var launchedBundleId: String? = null
+
     override fun launch(
         bundleId: String,
         clearState: Boolean,
@@ -879,6 +1056,7 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
     ) {
         if (clearState) driver.clearAppState(bundleId)
         driver.launchApp(bundleId, env)
+        launchedBundleId = bundleId
     }
 
     override fun terminate(bundleId: String) = driver.stopApp(bundleId)
@@ -922,8 +1100,14 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
     // started in has lost the foreground, the remaining keystrokes would spray
     // into whatever window stole it (the launcher search box, in practice), so
     // typing stops instead of leaking out of the app under test.
+    //
+    // typingOwner decides what "the app the type started in" means when the
+    // read that would name it fails: the launched bundle, so a link that cannot
+    // answer degrades the guard rather than switching it off.
     private fun typeShellSafe(text: String) {
-        val owner = foregroundPackage()
+        val owner = typingOwner(foregroundDumpsys(), launchedBundleId) {
+            System.err.println(it)
+        }
         val typed =
             typeChunks(chunkForInput(text, INPUT_CHUNK_CHARS), owner, {
                 foregroundPackage()
@@ -937,15 +1121,16 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
         }
     }
 
-    // foregroundPackage returns the package of the top resumed activity, or null
-    // if it cannot be read. Used to detect mid-type focus escapes.
-    private fun foregroundPackage(): String? = parseResumedPackage(
-        adbOutput(
+    private fun foregroundDumpsys(): String = adbOutput(
         serial,
         listOf("shell", "dumpsys", "activity", "activities"),
-        ),
     )
 
+    // foregroundPackage returns the package of the top resumed activity, or null
+    // if it cannot be read. Used to detect mid-type focus escapes.
+    private fun foregroundPackage(): String? =
+        parseResumedPackage(foregroundDumpsys())
+
     override fun eraseText(characterCount: Int) =
         driver.eraseText(characterCount)
 
@@ -991,8 +1176,15 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
     // read the snapshot was going to do anyway. That is what makes this
     // affordable, where the structural poll that used to run in waitForIdle was
     // not: it fetched the hierarchy ~4 more times on every mutating step.
-    override fun snapshot(): SnapshotSample =
-        SnapshotSample(awaitSettledTree { hierarchy() }, screenshot())
+    override fun snapshot(): SnapshotSample {
+        val tree = treeWithoutKeyboard(
+            awaitSettledTree { hierarchy() },
+            imePackage,
+            dismiss = { runCatching { dadb.shell("input keyevent 4") } },
+            reread = { awaitSettledTree { hierarchy() } },
+        )
+        return SnapshotSample(tree, screenshot())
+    }
 
     override fun waitForIdle(durationMillis: Long) {
         // waitForAppToSettle blocks on the View-system animation and maestro's
@@ -1044,16 +1236,76 @@ internal fun dadbTargetFor(serial: String?): DadbTarget {
     }
 }
 
+internal data class AdbServerEndpoint(val host: String, val port: Int)
+
+private const val ADB_SERVER_HOST = "localhost"
+private const val ADB_SERVER_PORT = 5037
+
+// adbServerEndpoint reads where the adb server listens the way the adb CLI
+// reads it: ADB_SERVER_SOCKET ("tcp:host:port", or "tcp:port" for a server on
+// this machine) outranks the older ANDROID_ADB_SERVER_ADDRESS /
+// ANDROID_ADB_SERVER_PORT pair, and unset means the loopback default.
+//
+// A value it cannot read throws instead of falling back to loopback. The
+// fallback is the dangerous answer: emulator serials are numbered per server,
+// so a run aimed at a remote emulator-5554 would quietly drive whatever this
+// machine calls emulator-5554 and report the results as the remote device's.
+internal fun adbServerEndpoint(
+    env: (String) -> String? = System::getenv,
+): AdbServerEndpoint {
+    val socket = env("ADB_SERVER_SOCKET")?.trim().orEmpty()
+    if (socket.isNotEmpty()) return parseAdbServerSocket(socket)
+    val host = env("ANDROID_ADB_SERVER_ADDRESS")?.trim().orEmpty()
+    val port = env("ANDROID_ADB_SERVER_PORT")?.trim().orEmpty()
+    return AdbServerEndpoint(
+        host.ifEmpty { ADB_SERVER_HOST },
+        if (port.isEmpty()) {
+            ADB_SERVER_PORT
+        } else {
+            adbServerPort(port, "ANDROID_ADB_SERVER_PORT=\"$port\"")
+        },
+    )
+}
+
+private fun parseAdbServerSocket(value: String): AdbServerEndpoint {
+    val named = "ADB_SERVER_SOCKET=\"$value\""
+    val address = value.removePrefix("tcp:")
+    if (address == value) rejectAdbServerSocket(named)
+    val colon = address.lastIndexOf(':')
+    if (colon < 0) {
+        return AdbServerEndpoint(
+            ADB_SERVER_HOST,
+            adbServerPort(address, named),
+        )
+    }
+    val host = address.substring(0, colon)
+    if (host.isEmpty()) rejectAdbServerSocket(named)
+    return AdbServerEndpoint(
+        host,
+        adbServerPort(address.substring(colon + 1), named),
+    )
+}
+
+private fun rejectAdbServerSocket(named: String): Nothing =
+    throw IllegalArgumentException("$named is not tcp:host:port")
+
+private fun adbServerPort(text: String, named: String): Int =
+    text.toIntOrNull()?.takeIf { it in 1..65535 }
+        ?: throw IllegalArgumentException("$named has no usable port")
+
 private fun buildDadb(serial: String?): dadb.Dadb =
     when (val target = dadbTargetFor(serial)) {
         is DadbTarget.Tcp -> dadb.Dadb.create(target.host, target.port)
 
-        is DadbTarget.Server -> dadb.adbserver.AdbServer.createDadb(
-            "localhost",
-            5037,
+        is DadbTarget.Server -> {
+            val server = adbServerEndpoint()
+            dadb.adbserver.AdbServer.createDadb(
+                server.host,
+                server.port,
                 "host:transport:${target.serial}",
             )
         }
+    }
 
 internal fun findBoundsBySelector(
     root: maestro.TreeNode,

sidecar/src/test/kotlin/dev/sanderling/sidecar/AdbOutputTimeoutTest.kt

@@ -0,0 +1,120 @@
+package dev.sanderling.sidecar
+
+import org.junit.Test
+import java.io.InputStream
+import java.io.OutputStream
+import java.util.concurrent.CountDownLatch
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+class AdbOutputTimeoutTest {
+
+    // An adb wedged on the link neither writes nor exits, so the read never
+    // reaches EOF. Without a bound on the READ the step waits for as long as
+    // adb feels like it: one such stall measured ~100s against a remote adb
+    // server. The bound has to release the reader as well as return, which is
+    // what killing the process does.
+    @Test(timeout = 20_000L)
+    fun aWedgedReadIsAbandonedAtTheBoundInsteadOfHangingForever() {
+        val process = FakeProcess(BlockingStream())
+        val logged = mutableListOf<String>()
+
+        val started = System.currentTimeMillis()
+        val output = readProcessOutput(process, 200L, { "adb shell pidof" }) {
+            logged.add(it)
+        }
+        val elapsed = System.currentTimeMillis() - started
+
+        assertEquals("", output, "a read that never lands is no answer")
+        assertTrue(process.destroyed, "the wedged adb must be killed, not left")
+        assertTrue(
+            elapsed < 10_000L,
+            "returned in ${elapsed}ms, not at a bound",
+        )
+        assertEquals(1, logged.size, "a silent timeout hides a degrading link")
+    }
+
+    @Test(timeout = 20_000L)
+    fun theAbandonedReadNamesTheCommandAndTheBound() {
+        val logged = mutableListOf<String>()
+        readProcessOutput(
+            FakeProcess(BlockingStream()),
+            200L,
+            { "adb -s emulator-5556 shell cat /proc/6103/stat" },
+        ) { logged.add(it) }
+
+        val line = logged.single()
+        assertTrue(
+            line.contains("adb -s emulator-5556 shell cat /proc/6103/stat"),
+            line,
+        )
+        assertTrue(line.contains("200"), line)
+    }
+
+    // The bound must cost the healthy path nothing: output that arrives comes
+    // back whole, and the process is left to exit on its own.
+    @Test(timeout = 20_000L)
+    fun outputThatArrivesComesBackWholeAndTheProcessSurvives() {
+        val text = "VmRSS:\t  123456 kB\nVmSize:\t  654321 kB\n"
+        val process = FakeProcess(text.byteInputStream())
+        val logged = mutableListOf<String>()
+
+        val output = readProcessOutput(process, 5_000L, { "adb shell cat" }) {
+            logged.add(it)
+        }
+
+        assertEquals(text, output)
+        assertTrue(!process.destroyed, "a process that answered is not killed")
+        assertTrue(logged.isEmpty(), "nothing to report on the healthy path")
+    }
+
+    // Output larger than a pipe buffer is why the read cannot be deferred
+    // until after the process exits: a process with more to say than the
+    // buffer holds blocks writing while a waiter waits for it to finish.
+    @Test(timeout = 20_000L)
+    fun outputLargerThanAPipeBufferComesBackWhole() {
+        val text = "x".repeat(512 * 1024)
+        val output = readProcessOutput(
+            FakeProcess(text.byteInputStream()),
+            5_000L,
+            { "adb logcat -d" },
+        ) {}
+        assertEquals(text.length, output.length)
+    }
+}
+
+// BlockingStream models a wedged adb: no bytes, and no EOF either, until the
+// process is killed and the pipe closes under the reader.
+private class BlockingStream : InputStream() {
+    private val released = CountDownLatch(1)
+
+    override fun read(): Int {
+        released.await()
+        return -1
+    }
+
+    override fun close() {
+        released.countDown()
+    }
+}
+
+private class FakeProcess(private val stream: InputStream) : Process() {
+    @Volatile var destroyed = false
+        private set
+
+    override fun getOutputStream(): OutputStream =
+        OutputStream.nullOutputStream()
+
+    override fun getInputStream(): InputStream = stream
+
+    override fun getErrorStream(): InputStream = InputStream.nullInputStream()
+
+    override fun waitFor(): Int = 0
+
+    override fun exitValue(): Int = 0
+
+    override fun destroy() {
+        destroyed = true
+        stream.close()
+    }
+}

sidecar/src/test/kotlin/dev/sanderling/sidecar/DadbTargetTest.kt

@@ -2,6 +2,8 @@ package dev.sanderling.sidecar
 
 import org.junit.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
+import kotlin.test.assertTrue
 
 class DadbTargetTest {
 
@@ -28,4 +30,103 @@ class DadbTargetTest {
             dadbTargetFor("emulator:5554x"),
         )
     }
+
+    // A serial-addressed device is reached through whichever adb server the
+    // environment names. Ignoring it sends the run to this machine's own
+    // server, where the serial either is missing or, worse, names a different
+    // device that happens to share the emulator numbering.
+    @Test fun adbServerSocketNamesARemoteServer() {
+        assertEquals(
+            AdbServerEndpoint("100.68.126.75", 5037),
+            adbServerEndpoint(
+                env("ADB_SERVER_SOCKET" to "tcp:100.68.126.75:5037"),
+            ),
+        )
+    }
+
+    @Test fun adbServerSocketWithOnlyAPortStaysLocal() {
+        assertEquals(
+            AdbServerEndpoint("localhost", 5038),
+            adbServerEndpoint(env("ADB_SERVER_SOCKET" to "tcp:5038")),
+        )
+    }
+
+    @Test fun androidAdbServerAddressAndPortPairIsHonoured() {
+        assertEquals(
+            AdbServerEndpoint("10.0.0.4", 5040),
+            adbServerEndpoint(
+                env(
+                    "ANDROID_ADB_SERVER_ADDRESS" to "10.0.0.4",
+                    "ANDROID_ADB_SERVER_PORT" to "5040",
+                ),
+            ),
+        )
+    }
+
+    @Test fun adbServerSocketOutranksTheOlderPair() {
+        assertEquals(
+            AdbServerEndpoint("100.68.126.75", 5037),
+            adbServerEndpoint(
+                env(
+                    "ADB_SERVER_SOCKET" to "tcp:100.68.126.75:5037",
+                    "ANDROID_ADB_SERVER_ADDRESS" to "10.0.0.4",
+                    "ANDROID_ADB_SERVER_PORT" to "5040",
+                ),
+            ),
+        )
+    }
+
+    @Test fun unsetEnvironmentKeepsTheLoopbackDefault() {
+        assertEquals(
+            AdbServerEndpoint("localhost", 5037),
+            adbServerEndpoint(env()),
+        )
+        assertEquals(
+            AdbServerEndpoint("localhost", 5037),
+            adbServerEndpoint(env("ADB_SERVER_SOCKET" to "")),
+        )
+    }
+
+    @Test fun eitherHalfOfTheOlderPairAloneKeepsTheOtherDefault() {
+        assertEquals(
+            AdbServerEndpoint("10.0.0.4", 5037),
+            adbServerEndpoint(env("ANDROID_ADB_SERVER_ADDRESS" to "10.0.0.4")),
+        )
+        assertEquals(
+            AdbServerEndpoint("localhost", 5040),
+            adbServerEndpoint(env("ANDROID_ADB_SERVER_PORT" to "5040")),
+        )
+    }
+
+    // A value that cannot be read must stop the run and say which variable
+    // held what. Falling back to loopback would drive this machine's devices
+    // while the operator believes the run is on the remote ones.
+    @Test fun malformedValuesFailNamingTheVariableAndItsContents() {
+        val cases = mapOf(
+            "ADB_SERVER_SOCKET" to listOf(
+                "100.68.126.75:5037",
+                "tcp:100.68.126.75:pear",
+                "tcp:",
+                "tcp::5037",
+                "unix:/tmp/adb",
+                "tcp:100.68.126.75:70000",
+            ),
+            "ANDROID_ADB_SERVER_PORT" to listOf("pear", "0", "-1"),
+        )
+        for ((variable, values) in cases) {
+            for (value in values) {
+                val failure = assertFailsWith<IllegalArgumentException>(value) {
+                    adbServerEndpoint(env(variable to value))
+                }
+                val message = failure.message.orEmpty()
+                assertTrue(message.contains(variable), message)
+                assertTrue(message.contains(value), message)
+            }
+        }
+    }
+}
+
+private fun env(vararg entries: Pair<String, String>): (String) -> String? {
+    val values = entries.toMap()
+    return { values[it] }
 }

sidecar/src/test/kotlin/dev/sanderling/sidecar/IdleDetectionTest.kt

@@ -21,11 +21,17 @@ class IdleDetectionTest {
         assertFalse(StubDriverBackend.isAnimationCountIdle("3\n"))
     }
 
-    @Test fun idleWhenOutputEmpty() {
-        assertTrue(StubDriverBackend.isAnimationCountIdle(""))
+    // A dumpsys that said nothing does not say the device is still. Reading
+    // absence as idle is how a settle returns instantly on a degraded link and
+    // hands the runner a screen caught mid-animation; the caller bounds its own
+    // wait, so the cost of being wrong the other way is a wait it already
+    // budgeted for. The exception path of the same probe already answers false.
+    @Test fun unreadableOutputIsNotIdle() {
+        assertFalse(StubDriverBackend.isAnimationCountIdle(""))
+        assertFalse(StubDriverBackend.isAnimationCountIdle("   \n"))
     }
 
-    @Test fun idleWhenOutputIsNotANumber() {
-        assertTrue(StubDriverBackend.isAnimationCountIdle("error: no service"))
+    @Test fun unparseableOutputIsNotIdle() {
+        assertFalse(StubDriverBackend.isAnimationCountIdle("error: no service"))
     }
 }

sidecar/src/test/kotlin/dev/sanderling/sidecar/InputTextTest.kt

@@ -207,6 +207,77 @@ class InputTextTest {
         )
     }
 
+    @Test fun aReadableDumpsysNamesTheResumedPackage() {
+        val warnings = mutableListOf<String>()
+        assertEquals(
+            "app.folio",
+            typingOwner(RESUMED_DUMPSYS, "app.folio") { warnings.add(it) },
+        )
+        assertTrue(warnings.isEmpty(), "nothing to report when the read worked")
+    }
+
+    // A dumpsys that said nothing is not evidence that focus is fine. Handing
+    // typeChunks a null owner turns the guard off outright, and the keystrokes
+    // then go wherever the foreground happens to be.
+    @Test fun anUnreadableDumpsysGuardsWithTheLaunchedBundle() {
+        val warnings = mutableListOf<String>()
+        assertEquals(
+            "app.folio",
+            typingOwner("", "app.folio") { warnings.add(it) },
+        )
+        assertEquals(1, warnings.size, "a degraded guard must not be silent")
+        assertTrue(warnings.single().contains("app.folio"), warnings.single())
+    }
+
+    // Wording no marker matches is the same "we do not know" as an empty read.
+    @Test fun dumpsysWithNoResumedMarkerGuardsWithTheLaunchedBundle() {
+        assertEquals(
+            "app.folio",
+            typingOwner("  mFocusedApp=null\n  nothing here\n", "app.folio") {},
+        )
+    }
+
+    // The whole point of the fallback: on a link that cannot answer, typing is
+    // still guarded, so a foreground that was stolen stops it after the first
+    // chunk instead of spraying the rest into whatever took focus.
+    @Test fun unreadableLinkStillStopsTypingWhenFocusWasStolen() {
+        val owner = typingOwner("", "app.folio") {}
+        val sent = mutableListOf<String>()
+
+        typeChunks(listOf("aaa", "bbb", "ccc"), owner, {
+            "com.android.launcher"
+        }) { sent.add(it) }
+
+        assertEquals(
+            listOf("aaa"),
+            sent,
+            "an unguarded type would have sent every chunk to the launcher",
+        )
+    }
+
+    // And the other half of the trade: the fallback must not turn a degraded
+    // link into a run that types nothing. A no-op InputText on every step is a
+    // green run that tested nothing, which is worse than the spray it avoids.
+    @Test fun unreadableLinkStillTypesWhenTheAppKeepsFocus() {
+        val owner = typingOwner("", "app.folio") {}
+        val sent = mutableListOf<String>()
+
+        val typed = typeChunks(listOf("aaa", "bbb", "cc"), owner, {
+            "app.folio"
+        }) { sent.add(it) }
+
+        assertEquals(listOf("aaa", "bbb", "cc"), sent)
+        assertEquals(8, typed)
+    }
+
+    // With no launch recorded there is nothing to guard against, and the honest
+    // answer is to say the guard is off rather than imply it ran.
+    @Test fun noLaunchedBundleLeavesTheGuardOffAndSaysSo() {
+        val warnings = mutableListOf<String>()
+        assertEquals(null, typingOwner("", null) { warnings.add(it) })
+        assertEquals(1, warnings.size)
+    }
+
     @Test fun maestroKeyForResolvesAndRejects() {
         assertEquals(maestro.KeyCode.BACK, maestroKeyFor("back"))
         assertEquals(maestro.KeyCode.BACK, maestroKeyFor("BACK"))
@@ -241,8 +312,134 @@ class InputTextTest {
         }
         assertEquals(listOf("dumpsys input_method"), commands)
     }
+
+    // Typing is not the only thing that raises the keyboard: tapping a field
+    // raises it too, and nothing was closing that one. The snapshot the picker
+    // chooses from is missing every app node the keyboard covers, so the step
+    // spends its budget choosing between the few targets left. Closing it
+    // before the tree is read is what gives the step its targets back.
+    @Test fun aKeyboardInTheTreeIsClosedBeforeTheTreeIsReturned() {
+        var backs = 0
+        val reads = mutableListOf<String>()
+        val settled = treeWithoutKeyboard(
+            IME_TREE,
+            IME_PACKAGE,
+            dismiss = { backs++ },
+            reread = { APP_TREE.also { reads.add(it) } },
+            sleep = {},
+        )
+        assertEquals(APP_TREE, settled)
+        assertEquals(1, backs, "one BACK closes the keyboard")
+        assertEquals(1, reads.size, "the tree is re-read once it is gone")
     }
 
+    // The guard has to be the tree itself. BACK with no keyboard open
+    // navigates out of the screen, so a snapshot that pressed it on every read
+    // would walk the fuzzer backwards out of the app a step at a time.
+    @Test fun aTreeWithNoKeyboardIsReturnedUntouched() {
+        var backs = 0
+        var reads = 0
+        val settled = treeWithoutKeyboard(
+            APP_TREE,
+            IME_PACKAGE,
+            dismiss = { backs++ },
+            reread = {
+                reads++
+                APP_TREE
+            },
+            sleep = {},
+        )
+        assertEquals(APP_TREE, settled)
+        assertEquals(0, backs, "no keyboard in the tree means no BACK")
+        assertEquals(0, reads, "and no second hierarchy read to pay for")
+    }
+
+    // An unknown IME package is the honest "cannot tell", and the safe way to
+    // be wrong is to leave the keyboard up rather than press BACK blind.
+    @Test fun anUnknownImePackageSendsNoBack() {
+        var backs = 0
+        assertEquals(
+            IME_TREE,
+            treeWithoutKeyboard(
+                IME_TREE,
+                null,
+                dismiss = { backs++ },
+                reread = { APP_TREE },
+                sleep = {},
+            ),
+        )
+        assertEquals(0, backs)
+    }
+
+    // A keyboard the app puts straight back gets ONE back press, not one per
+    // re-read. The flag behind the older dismissal lags a BACK by up to 0.6s,
+    // and a burst of them inside that window is how a dismissal turns into
+    // navigation.
+    @Test fun aKeyboardThatStaysUpIsNotBackPressedRepeatedly() {
+        var backs = 0
+        var reads = 0
+        val settled = treeWithoutKeyboard(
+            IME_TREE,
+            IME_PACKAGE,
+            dismiss = { backs++ },
+            reread = {
+                reads++
+                IME_TREE
+            },
+            sleep = {},
+        )
+        assertEquals(IME_TREE, settled, "the caller still gets a tree")
+        assertEquals(1, backs)
+        assertTrue(
+            reads in 1..KEYBOARD_DISMISS_READS,
+            "bounded re-reads, got $reads",
+        )
+    }
+
+    @Test fun imePackageOfReadsTheComponentAndRejectsNonsense() {
+        assertEquals(
+            "com.google.android.inputmethod.latin",
+            imePackageOf(
+                "com.google.android.inputmethod.latin/.LatinIME\n",
+            ),
+        )
+        assertEquals(null, imePackageOf("null"))
+        assertEquals(null, imePackageOf(""))
+        assertEquals(null, imePackageOf("  \n"))
+    }
+
+    @Test fun treeShowsImeMatchesTheImesOwnViewIdsOnly() {
+        assertTrue(treeShowsIme(IME_TREE, IME_PACKAGE))
+        assertTrue(!treeShowsIme(APP_TREE, IME_PACKAGE))
+    }
+}
+
+private val RESUMED_DUMPSYS =
+    """
+      mFocusedApp=ActivityRecord{1a u0 app.folio/.MainActivity t14}
+      topResumedActivity=ActivityRecord{f3 u0 app.folio/.MainActivity t14}
+    """.trimIndent()
+
+private const val IME_PACKAGE = "com.google.android.inputmethod.latin"
+
+private val APP_TREE =
+    """
+    {"attributes":{"resource-id":"AddAccountScreen"},"children":[
+      {"attributes":{"resource-id":"AccountNameField"},"children":[]},
+      {"attributes":{"resource-id":"AddAccountSubmit"},"children":[]}]}
+    """.trimIndent()
+
+// The submit control is gone: an open keyboard does not merely cover the node,
+// it takes it out of the tree the picker enumerates.
+private val IME_TREE =
+    """
+    {"attributes":{"resource-id":"AddAccountScreen"},"children":[
+      {"attributes":{"resource-id":"AccountNameField"},"children":[]},
+      {"attributes":{
+        "resource-id":"com.google.android.inputmethod.latin:id/keyboard_holder"
+      },"children":[]}]}
+    """.trimIndent()
+
 private val IME_OPEN_DUMPSYS =
     """
       mCurMethodId=com.google.android.inputmethod.latin/.LatinIME