diff --git a/docs/phase-3-spotify-recipe.md b/docs/phase-3-spotify-recipe.md index 68fc332..0de82c8 100644 --- a/docs/phase-3-spotify-recipe.md +++ b/docs/phase-3-spotify-recipe.md @@ -53,10 +53,13 @@ Run it with: ./bin/artemis run spotify_play query="Rolling Stones Honky Tonk" ``` -The verbs are `launch`, `wait`, `wait_gone`, `tap`, `tap_if`, `tap_row`, `type`, -`key`, `back`, `home` and `sleep`. `tap exact "Pause"` matches the whole text, -not a substring. `tap_if` taps only when the text is already on screen, which -lets one recipe cope with an app that remembers its last screen. +The verbs are `launch`, `foreground`, `wait`, `wait_gone`, `tap`, `tap_if`, +`tap_row`, `type`, `key`, `back`, `home` and `sleep`. `tap exact "Pause"` +matches the whole text, not a substring. `tap_if` taps only when the text is +already on screen, which lets one recipe cope with an app that remembers its +last screen. `foreground` waits until the recipe's app is really in front, so a +word like "Search" cannot be matched from a different app that happens to be +open — every recipe should use it straight after `launch`. ## Why `tap_row` exists @@ -185,15 +188,44 @@ keyguard; reading the state first would say "unlocked" about a locked phone. | Recipes file format, guards, parameters, timeouts | unit tested (`internal/recipe`) | | Row selection against a real Spotify dump | unit tested (`internal/ui`) | | Keyguard parsing and secure-lock detection | unit tested (`internal/ui`) | +| Step durations reach the run log | unit tested (`internal/recipe`) | | All acting commands refuse on a locked phone | **live, verified** (above) | | `artemis run spotify_play` refuses on a locked phone | **live, verified** (above) | | `spotify_play` plays the right song | **not yet run end to end** — see below | | Spotify search reaches the result list | live (Home → Search → type → results) | -The live end-to-end run of `spotify_play` (Phase 3 DoD) has not been completed. -The phone locked itself part-way through the session (5-minute screen timeout, -secure keyguard) and Artemis will not unlock it by design. It needs a human to -unlock the phone, and then: +## Live run log + +Six attempts, in order. Each failure was clean, named its screen and tapped +nothing random — which is the point of the phase. + +1. **Refused, locked phone.** `phone is locked; unlock it and retry`. The phone + had locked itself on its 5-minute screen timeout while the code was being + written. +2. **Refused after the prescribed sleep/wake test.** `keyguard showing=true + secure=true`, exit 1 from `keyguard`, `tap` and `run`. The lock screen was + still on screen afterwards. +3. **Refused, dozing phone that only *looked* unlocked.** A dozing phone reports + `showing=false`. Pressing WAKEUP then revealed the secure keyguard, and the + run refused. This is why every acting command wakes first. +4. **Wrong app matched "Search".** The phone was sitting on the Android + Settings app. `wait "Search"` was satisfied by Settings' own "Search + Settings" list entry, so the recipe tapped the Settings search bar. Fixed by + adding the `foreground` step, which waits until the recipe's app really is in + front before any text is trusted. +5. **Blocked by a biometric prompt.** Settings was left showing a + `Verify that it's you` dialog (`Touch the fingerprint sensor` / `Use PIN`), + which is drawn over Spotify. The run failed with: + `text "Search" did not appear within 30s; foreground=com.spotify.music; + screen shows: "Cancel", "App logo", "Settings", "Verify that it's you" ...`. + Artemis did not touch the prompt, and it must not: it needs Sam's fingerprint + or PIN, exactly like the lock screen. +6. **Still blocked** by the same prompt on the next attempt. + +### What is left + +The live end-to-end run of `spotify_play` (Phase 3 DoD) has not completed. It +needs the phone unlocked and free of the biometric prompt, and then: ```sh ./bin/artemis run spotify_play query="Rolling Stones Honky Tonk" diff --git a/internal/recipe/engine.go b/internal/recipe/engine.go index 0f75004..bc3664b 100644 --- a/internal/recipe/engine.go +++ b/internal/recipe/engine.go @@ -113,10 +113,10 @@ func Run(r *adb.Runner, rec *Recipe, opts Options) *RunResult { } func executeStep(r *adb.Runner, rec *Recipe, step Step, index int, params map[string]string, - w, h int, deadline time.Time, logf func(string, ...any)) StepResult { + w, h int, deadline time.Time, logf func(string, ...any)) (sr StepResult) { started := time.Now() - sr := StepResult{Index: index + 1, Verb: step.Verb, Args: step.Args} + sr = StepResult{Index: index + 1, Verb: step.Verb, Args: step.Args} defer func() { sr.Duration = time.Since(started) sr.DurationMS = sr.Duration.Milliseconds() @@ -160,6 +160,33 @@ func executeStep(r *adb.Runner, rec *Recipe, step Step, index int, params map[st sr.Detail = "launched " + pkg return nil + case "foreground": + // Confirm the recipe's app is really in front. Without this, a text + // like "Search" can match whatever app happened to be open. + pkg := rec.App + if len(args) == 1 { + pkg = args[0] + } + if pkg == "" { + return fmt.Errorf("foreground needs an app: add an \"app:\" setting or pass a package") + } + fgDeadline := time.Now().Add(timeout) + for { + fg, ferr := ui.ForegroundPackage(r) + if ferr == nil && fg == pkg { + sr.Detail = "foreground is " + pkg + return nil + } + if time.Now().After(fgDeadline) { + got := "unknown" + if ferr == nil { + got = fg + } + return fmt.Errorf("%s never came to the foreground within %s (foreground=%s)", pkg, timeout, got) + } + time.Sleep(400 * time.Millisecond) + } + case "wait": node, err := ui.WaitForText(r, args[0], step.Exact, timeout) if err != nil { diff --git a/internal/recipe/recipe.go b/internal/recipe/recipe.go index b7d0255..35abec3 100644 --- a/internal/recipe/recipe.go +++ b/internal/recipe/recipe.go @@ -29,6 +29,7 @@ // Steps are "- " lines, run in order: // // - launch start the recipe's app +// - foreground 30s wait until the recipe's app is in front // - wait "Search" 30s wait for an element whose text matches // - wait_gone "Recents" 30s wait until no element matches the text // - tap "Search" tap the element whose text matches (FR-4) @@ -105,17 +106,18 @@ var verbs = map[string]struct { durArg bool // the single argument is a duration allowExact bool }{ - "launch": {0, 1, false, false}, - "wait": {1, 2, false, true}, - "wait_gone": {1, 2, false, true}, - "tap": {1, 2, false, true}, - "tap_if": {1, 1, false, true}, - "tap_row": {1, 2, false, true}, - "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}, + "wait_gone": {1, 2, false, true}, + "tap": {1, 2, false, true}, + "tap_if": {1, 1, false, true}, + "tap_row": {1, 2, false, true}, + "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. diff --git a/internal/recipe/recipe_test.go b/internal/recipe/recipe_test.go index 41e4b1d..b821979 100644 --- a/internal/recipe/recipe_test.go +++ b/internal/recipe/recipe_test.go @@ -6,6 +6,8 @@ import ( "strings" "testing" "time" + + "gitea.lab.audasmedia.com.au/sam/artemis_control/internal/adb" ) const sample = `# a comment @@ -155,7 +157,17 @@ func TestSpotifyRecipeIsValid(t *testing.T) { t.Fatalf("step %d uses unknown verb %q", i+1, s.Verb) } } - // The recipe must wait for playback, not assume it. + // The recipe must confirm it is in Spotify before it trusts a text match, + // and it must wait for playback rather than assume it. + var sawForeground bool + for _, s := range rec.Steps { + if s.Verb == "foreground" { + sawForeground = true + } + } + if !sawForeground { + t.Fatal("spotify_play must pin the foreground app before matching text") + } last := rec.Steps[len(rec.Steps)-1] if last.Verb != "wait" || last.Args[0] != "Pause" { t.Fatalf("the last step must confirm playback, got %+v", last) @@ -192,3 +204,31 @@ func TestLoadAndList(t *testing.T) { t.Fatalf("Find unknown task: %v", err) } } + +func TestForegroundStep(t *testing.T) { + rec, err := Parse("app: com.spotify.music\nsteps:\n - launch\n - foreground 30s\n") + if err != nil { + t.Fatalf("Parse: %v", err) + } + fg := rec.Steps[1] + if fg.Verb != "foreground" || len(fg.Args) != 0 || fg.Timeout != 30*time.Second { + t.Fatalf("foreground step = %+v", fg) + } +} + +// TestStepDurationsAreRecorded guards the run log (FR-10): a step's duration +// must reach the caller, not be swallowed by the return value. +func TestStepDurationsAreRecorded(t *testing.T) { + rec := &Recipe{StepTimeout: time.Second} + step := Step{Verb: "sleep", Timeout: 50 * time.Millisecond} + sr := executeStep(adb.New(""), rec, step, 2, nil, 0, 0, time.Now().Add(time.Minute), func(string, ...any) {}) + if !sr.OK { + t.Fatalf("sleep step failed: %s", sr.Error) + } + if sr.Index != 3 { + t.Fatalf("index = %d, want 3", sr.Index) + } + if sr.Duration < 40*time.Millisecond || sr.DurationMS < 40 { + t.Fatalf("duration not recorded: %v (%d ms)", sr.Duration, sr.DurationMS) + } +} diff --git a/recipes/spotify_play.recipe b/recipes/spotify_play.recipe index 53eedf1..c8f979f 100644 --- a/recipes/spotify_play.recipe +++ b/recipes/spotify_play.recipe @@ -35,6 +35,7 @@ steps: # 1. Make sure Spotify is up. Launching an app that is already open is a # no-op, so this is safe to repeat. - launch + - foreground 30s - wait "Search" 30s # 2. Open the search tab. Spotify remembers the last tab, so a search screen