diff --git a/README.md b/README.md index 0560478..ea1f082 100644 --- a/README.md +++ b/README.md @@ -88,6 +88,10 @@ file format, the verbs, and why list rows need their own step, and - **Never tap blindly.** A missing or disabled element is a loud failure, not a guessed coordinate. A named bad screen (`guard`) fails with its own reason, and a step may name its own failure with `else "..."`. +- **Detect a wedged app.** An app can stop consuming input while it keeps + drawing: Android delivers the tap, the app never acts on it. `expect_change` + fails with `the app stopped consuming input` when the screen does not change + after the previous step, instead of blaming the label the next step waited for. - **Never touch a locked phone.** A secure keyguard stops Artemis with `phone is locked; unlock it and retry`. No PIN is stored, read or typed (AGENTS.md section 13 Q2). diff --git a/internal/recipe/engine.go b/internal/recipe/engine.go index 1b6efb3..551cce7 100644 --- a/internal/recipe/engine.go +++ b/internal/recipe/engine.go @@ -273,6 +273,18 @@ func executeStep(r *adb.Runner, rec *Recipe, step Step, index int, params map[st sr.Detail = fmt.Sprintf("%q is gone", args[0]) return nil + case "expect_change": + // The wedged-app check. An app that has stopped consuming input + // still draws its old screen, so a delivered tap or key changes + // nothing. Failing here names the real problem instead of blaming + // the label the next step would have waited for (Phase 5). + _, _, err := ui.ExpectChange(r, timeout) + if err != nil { + return describeFailure(r, rec, fmt.Errorf("the app stopped consuming input: %w", err)) + } + sr.Detail = "screen changed" + return nil + case "tap": return tapByText(r, rec, args[0], step.Exact, timeout, &sr) diff --git a/internal/recipe/recipe.go b/internal/recipe/recipe.go index 76d8a9f..c6de907 100644 --- a/internal/recipe/recipe.go +++ b/internal/recipe/recipe.go @@ -40,6 +40,8 @@ // title matches the text; with no text, wait for PLAYING // alone (a radio plays a different track every few minutes) // - wait_gone "Recents" 30s wait until no element matches the text +// - expect_change 10s fail unless the screen changes within the time +// (the wedged-app check: a delivered input that the app never acts on) // - tap "Search" tap the element whose text matches (FR-4) // - tap exact "Pause" tap an element whose text matches exactly // - tap_if "Clear search query" tap it when it is on screen, else skip @@ -140,21 +142,22 @@ var verbs = map[string]struct { durArg bool // the single argument is a duration allowExact bool }{ - "launch": {0, 1, false, false}, - "foreground": {0, 1, false, false}, - "wait": {1, 2, false, true}, - "playing": {0, 2, false, false}, - "wait_gone": {1, 2, false, true}, - "tap": {1, 2, false, true}, - "tap_if": {1, 1, false, true}, - "tap_row": {1, 2, false, true}, - "menu_row": {1, 2, false, false}, - "scroll": {1, 1, false, false}, - "type": {1, 1, false, false}, - "key": {1, 1, false, false}, - "back": {0, 0, false, false}, - "home": {0, 0, false, false}, - "sleep": {1, 1, true, false}, + "launch": {0, 1, false, false}, + "foreground": {0, 1, false, false}, + "wait": {1, 2, false, true}, + "playing": {0, 2, false, false}, + "wait_gone": {1, 2, false, true}, + "expect_change": {1, 1, true, false}, + "tap": {1, 2, false, true}, + "tap_if": {1, 1, false, true}, + "tap_row": {1, 2, false, true}, + "menu_row": {1, 2, false, false}, + "scroll": {1, 1, false, false}, + "type": {1, 1, false, false}, + "key": {1, 1, false, false}, + "back": {0, 0, false, false}, + "home": {0, 0, false, false}, + "sleep": {1, 1, true, false}, } // keyNames are the key event names a recipe may use. @@ -367,8 +370,8 @@ func parseStep(body string, lineNo int) (*Step, error) { if err != nil || n < 1 { return nil, fmt.Errorf("line %d: retry needs a positive number, got %q", lineNo, args[i+1]) } - if verb != "wait" && verb != "playing" { - return nil, fmt.Errorf("line %d: retry is only allowed on a wait or playing step", lineNo) + if verb != "wait" && verb != "playing" && verb != "expect_change" { + return nil, fmt.Errorf("line %d: retry is only allowed on a wait, playing or expect_change step", lineNo) } step.Retry = n args = append(args[:i], args[i+2:]...) diff --git a/internal/recipe/recipe_test.go b/internal/recipe/recipe_test.go index 222fe1a..939d94b 100644 --- a/internal/recipe/recipe_test.go +++ b/internal/recipe/recipe_test.go @@ -230,11 +230,12 @@ func TestRetryOnWait(t *testing.T) { func TestRetryRejectsBadUse(t *testing.T) { for name, src := range map[string]string{ - "on a tap": "steps:\n - tap \"x\" retry 3\n", - "on tap_row": "steps:\n - tap_row \"x\" retry 3\n", - "no number": "steps:\n - tap_row \"x\"\n - wait \"y\" retry\n", - "zero": "steps:\n - tap_row \"x\"\n - wait \"y\" retry 0\n", - "not a number": "steps:\n - tap_row \"x\"\n - wait \"y\" retry soon\n", + "on a tap": "steps:\n - tap \"x\" retry 3\n", + "on tap_row": "steps:\n - tap_row \"x\" retry 3\n", + "no number": "steps:\n - tap_row \"x\"\n - wait \"y\" retry\n", + "zero": "steps:\n - tap_row \"x\"\n - wait \"y\" retry 0\n", + "not a number": "steps:\n - tap_row \"x\"\n - wait \"y\" retry soon\n", + "expect_change without a duration": "steps:\n - key enter\n - expect_change\n", } { if _, err := Parse(src); err == nil { t.Errorf("%s: expected a parse error", name) @@ -242,6 +243,26 @@ func TestRetryRejectsBadUse(t *testing.T) { } } +// TestExpectChangeStep covers the wedged-app check: a delivered input that the +// app never acts on must fail with its own reason, and may re-press the step +// above a bounded number of times (FR-11). +func TestExpectChangeStep(t *testing.T) { + rec, err := Parse("steps:\n - key enter\n - expect_change 12s retry 2 else \"the app stopped accepting input\"\n") + if err != nil { + t.Fatalf("Parse: %v", err) + } + step := rec.Steps[1] + if step.Verb != "expect_change" || step.Timeout != 12*time.Second { + t.Fatalf("expect_change step = %+v", step) + } + if step.Retry != 2 || step.Reason != "the app stopped accepting input" { + t.Fatalf("retry/reason = %d / %q", step.Retry, step.Reason) + } + if len(step.Args) != 1 || step.Args[0] != "12s" { + t.Fatalf("args = %v, want the duration as the only argument", step.Args) + } +} + func TestPlayingStep(t *testing.T) { rec, err := Parse("app: com.spotify.music\nsteps:\n - tap_row \"{query}\"\n - playing \"{query}\" 30s retry 3\n") if err != nil { diff --git a/internal/ui/change.go b/internal/ui/change.go new file mode 100644 index 0000000..26d10ef --- /dev/null +++ b/internal/ui/change.go @@ -0,0 +1,84 @@ +package ui + +import ( + "fmt" + "sort" + "strings" + "time" + + "gitea.lab.audasmedia.com.au/sam/artemis_control/internal/adb" +) + +// ScreenSignature returns a fingerprint of what is on screen: the foreground +// package plus every visible label and the bounds it sits in. +// +// Two dumps of an unchanged screen produce the same signature, and any change a +// human would see — new text, different text, an element moved or a different +// app in front — produces a different one. It is the raw material for wedge +// detection: if an input is delivered and the signature never moves, the app +// never acted on that input. +// +// The foreground package is best effort. If it cannot be read the labels alone +// still make a usable fingerprint, because a screen that stops changing is +// exactly the failure this guards against. +func ScreenSignature(r *adb.Runner) (string, error) { + h, err := Dump(r) + if err != nil { + return "", err + } + fg, _ := ForegroundPackage(r) + return h.Signature(fg), nil +} + +// Signature is the pure half of ScreenSignature: a stable fingerprint of one +// parsed dump. Labels are sorted so that the order uiautomator happened to +// report them in never looks like a change, and bounds are included so that an +// element moving is a change even when the words stay the same. +func (h *Hierarchy) Signature(foreground string) string { + parts := make([]string, 0, len(h.TextNodes())) + h.Walk(func(n *Node) { + label := strings.TrimSpace(n.Text) + if label == "" { + label = strings.TrimSpace(n.ContentDesc) + } + if label == "" { + return + } + parts = append(parts, strings.TrimSpace(n.Bounds)+"|"+label) + }) + sort.Strings(parts) + return foreground + "\n" + strings.Join(parts, "\n") +} + +// ExpectChange waits until the screen no longer looks like it did when the +// function was called, and returns the signatures before and after. +// +// This is the wedged-app check. An app that has stopped consuming input still +// draws the screen it had, so a delivered tap changes nothing. A timeout here +// means the last input was accepted by Android but never acted on by the app, +// which the recipe reports as "the app stopped consuming input" instead of +// blaming the label it was waiting for (AGENTS.md section 11 Phase 5). +func ExpectChange(r *adb.Runner, timeout time.Duration) (before, after string, err error) { + if timeout <= 0 { + timeout = 10 * time.Second + } + before, err = ScreenSignature(r) + if err != nil { + return "", "", err + } + + deadline := time.Now().Add(timeout) + for { + sig, derr := ScreenSignature(r) + if derr != nil { + return before, "", derr + } + if sig != before { + return before, sig, nil + } + if !time.Now().Before(deadline) { + return before, before, fmt.Errorf("the screen did not change within %s", timeout) + } + time.Sleep(PollInterval) + } +} diff --git a/internal/ui/change_test.go b/internal/ui/change_test.go new file mode 100644 index 0000000..916fa20 --- /dev/null +++ b/internal/ui/change_test.go @@ -0,0 +1,68 @@ +package ui + +import ( + "strings" + "testing" +) + +// TestSignatureIsStable guards the wedged-app check: two reads of an unchanged +// screen must produce the same signature, or every screen would look like it +// changed and no wedge could ever be detected. +func TestSignatureIsStable(t *testing.T) { + h := parseSample(t) + first := h.Signature("com.android.settings") + second := h.Signature("com.android.settings") + if first != second { + t.Fatalf("signature is not stable:\n%q\n%q", first, second) + } + if !strings.Contains(first, "com.android.settings") { + t.Fatalf("signature %q does not name the foreground package", first) + } +} + +// TestSignatureSeesRealChanges checks the three things a human would call a +// change: different text, a moved element, and a different app in front. +func TestSignatureSeesRealChanges(t *testing.T) { + h := parseSample(t) + base := h.Signature("com.android.settings") + + if h.Signature("com.spotify.music") == base { + t.Fatal("a different foreground package must change the signature") + } + + renamed, err := parseHierarchy([]byte(strings.Replace(sampleDump, "Connected", "Disconnected", 1))) + if err != nil { + t.Fatalf("parseHierarchy: %v", err) + } + if renamed.Signature("com.android.settings") == base { + t.Fatal("different visible text must change the signature") + } + + moved, err := parseHierarchy([]byte(strings.Replace(sampleDump, "[221,217][453,277]", "[221,300][453,360]", 1))) + if err != nil { + t.Fatalf("parseHierarchy: %v", err) + } + if moved.Signature("com.android.settings") == base { + t.Fatal("an element that moved must change the signature") + } +} + +// TestSignatureIgnoresNodeOrder keeps the fingerprint about the screen, not the +// order uiautomator happened to walk it in. +func TestSignatureIgnoresNodeOrder(t *testing.T) { + a, err := parseHierarchy([]byte(`` + + `` + + ``)) + if err != nil { + t.Fatalf("parseHierarchy: %v", err) + } + b, err := parseHierarchy([]byte(`` + + `` + + ``)) + if err != nil { + t.Fatalf("parseHierarchy: %v", err) + } + if a.Signature("app.one") != b.Signature("app.one") { + t.Fatal("node order must not change the signature") + } +}