diff --git a/.claude/skills/create-verifier/SKILL.md b/.claude/skills/create-verifier/SKILL.md index b9bda98..cffdc48 100644 --- a/.claude/skills/create-verifier/SKILL.md +++ b/.claude/skills/create-verifier/SKILL.md @@ -13,10 +13,10 @@ The verifier drives `io.github.stozo04.openloop/.MainActivity` on an emulator li ## Workflow -1. Read `docs/OPERATING_INSTRUCTIONS.md`, `docs/OPENLOOP_INSTRUCTIONS.md`, the matching `.cursor/skills/verify-openloop/features/.md`, and the relevant product code/strings. Use `.cursor/skills/verify-openloop/helpers/onboarding_loop.py` as the proven reference. +1. Read `docs/OPERATING_INSTRUCTIONS.md`, `docs/OPENLOOP_INSTRUCTIONS.md`, the matching `.claude/skills/verify-openloop/features/.md`, and the relevant product code/strings. Use `.claude/skills/verify-openloop/helpers/onboarding_loop.py` as the proven reference. 2. Derive observable acceptance criteria from the shipped product: precondition, entry point, exact must-have/must-not-have UI, user actions, persisted/resulting state, and any required non-UI proof such as logcat or a file. -3. Create `.cursor/skills/verify-openloop/helpers/_loop.py` with Python's standard library. Reuse an existing helper when one fits; extract shared code only after two real loops demonstrate material duplication. -4. Update the matching feature recipe with the direct run command. Run `python scripts/sync-harness-skills.py --fix --from cursor`, then `--check`. +3. Create `.claude/skills/verify-openloop/helpers/_loop.py` with Python's standard library. Reuse an existing helper when one fits; extract shared code only after two real loops demonstrate material duplication. +4. Update the matching feature recipe with the direct run command. Run `python scripts/sync-harness-skills.py --fix` (it takes the direction from git and retargets each copy's own paths), then `--check`. 5. Build the current debug APK if needed and run the new loop on a booted emulator. Do not report completion from syntax checks or exit code alone; independently confirm its final PASS marker and evidence artifacts. `scripts/run-verification-loops.py --changed` discovers shipped `*_loop.py` files automatically, so there is no registry or roadmap to update. diff --git a/.claude/skills/harness-sync/SKILL.md b/.claude/skills/harness-sync/SKILL.md index c113f7c..eb53cc3 100644 --- a/.claude/skills/harness-sync/SKILL.md +++ b/.claude/skills/harness-sync/SKILL.md @@ -1,6 +1,6 @@ --- name: harness-sync -description: Check and repair byte-drift between the three LLM harness skill trees — `.claude/skills/`, `.cursor/skills/` and `.codex/skills/` — which the Definition of Done (M5) requires to be byte-identical. Use this skill whenever you edit, add, delete or rename ANYTHING under any of those three directories, and whenever the user says "sync harness", "sync the skills", "/harness-sync", "harness drift", "gate 6d", "M5", "sweep gate 6d is red", "the skill trees have drifted", "propagate this skill to the other LLMs", or asks whether the three harness folders match. Also use it proactively before committing a skill change and when a pre-PR sweep or CI run reports the "Harness skill trees byte-identical" gate as failing — a skills edit that reaches only one harness is the exact failure this catches. +description: Check and repair byte-drift between the three LLM harness skill trees — `.claude/skills/`, `.cursor/skills/` and `.codex/skills/` — which the Definition of Done (M5) requires to be identical apart from each copy's pointer at its own tree. Use this skill whenever you edit, add, delete or rename ANYTHING under any of those three directories, and whenever the user says "sync harness", "sync the skills", "/harness-sync", "harness drift", "gate 6d", "M5", "sweep gate 6d is red", "the skill trees have drifted", "propagate this skill to the other LLMs", or asks whether the three harness folders match. Also use it proactively before committing a skill change and when a pre-PR sweep or CI run reports the "Harness skill trees byte-identical" gate as failing — a skills edit that reaches only one harness is the exact failure this catches. --- # harness-sync — keep the three harness skill trees identical @@ -79,6 +79,20 @@ must NOT be synced: They are Claude-only by design and live outside `skills/`, which is why the script never sees them. +One difference inside the compared files IS allowed, and only one: **a skill's pointer at its own +tree**. A recipe that points at `/skills/verify-openloop/helpers/onboarding_loop.py` names +a different directory in each copy, because otherwise two of the three send their LLM to a path it +cannot read (owner instruction, 2026-08-31). The leading `.claude` / `.cursor` / `.codex` of such a +path is compared as one token, and `--fix` rewrites it for each destination — so you still edit one +tree and propagate. Three rules keep it honest: + +- Each copy must point at **itself**. A `.cursor` copy naming the `.codex` tree is drift, and the + gate says so. +- Only a path that continues past the tree root counts. Prose listing the three directories — the + `## Scope` heading below does it — is a list, not a pointer, and is compared literally. +- Anything outside a harness's `skills/` (`~/.cursor/mcp.json`, `.claude/commands/`) belongs to one + harness for real and stays literal in all three copies. + If you are adding a fourth harness, create `./skills/`, add the name to `HARNESSES` in the script, and add the allowlist entries in `.github/workflows/doc-layout.yml` and `docs/README.md` § Enforcement — in the same PR, per Definition of Done M1. diff --git a/.claude/skills/verify-openloop/SKILL.md b/.claude/skills/verify-openloop/SKILL.md index 246b949..4900e56 100644 --- a/.claude/skills/verify-openloop/SKILL.md +++ b/.claude/skills/verify-openloop/SKILL.md @@ -31,7 +31,7 @@ Isolation: **one emulator**. Parallel AVDs fight over host CPU and codecs and in $env:JAVA_HOME = "C:\Program Files\Android\Android Studio\jbr" $env:VERIFY_RUN_ID = Get-Date -Format "yyyyMMdd_HHmmss" # Start one AVD first (Pixel_6 / Pixel_8 / Pixel_8_API34 are the known ones). -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 launch +pwsh .claude/skills/verify-openloop/helpers/control.ps1 launch ``` Ready when `control.ps1 doctor` prints `ok` and `control.ps1 dump` shows one of: @@ -49,7 +49,7 @@ If launch fails, stop. Do not tap some other OpenLoop build (`com.OpenLoop.app` Read-only. Run before the first drive, after any failed drive, and whenever the instance looks wrong. ```powershell -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 doctor +pwsh .claude/skills/verify-openloop/helpers/control.ps1 doctor ``` Must confirm: serial is an emulator (or an explicitly allowed test device), `io.github.stozo04.openloop` is installed, `versionName`/`versionCode` match this checkout (`1.0.49` / `49` on the branch this skill was written against — re-read `app/build.gradle.kts` if they drifted), and `adb get-state` is `device`. @@ -61,8 +61,8 @@ If doctor fails, relaunch or abort. Never drive an instance you did not start. Harness: `uiauto.ps1` (uiautomator dump → tap by text or content-desc). Screenshots hit a per-session image limit; the dump is the eyes. ```powershell -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 dump -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 tap -Label "Start recording" +pwsh .claude/skills/verify-openloop/helpers/control.ps1 dump +pwsh .claude/skills/verify-openloop/helpers/control.ps1 tap -Label "Start recording" ``` Prefer these handles, in order: @@ -99,7 +99,7 @@ Name artifacts `$VERIFY_EVIDENCE_DIR//`. Kill what this run started. Never `pkill` / `adb shell pkill` by name. ```powershell -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 cleanup +pwsh .claude/skills/verify-openloop/helpers/control.ps1 cleanup ``` Force-stops `io.github.stozo04.openloop` on **this serial only**. Leaves the APK installed. Does not delete `$VERIFY_EVIDENCE_DIR`. Does not wipe gallery clips unless the recipe says so. Does not reset onboarding unless you ran `.claude/skills/reset-storage/`. diff --git a/.claude/skills/verify-openloop/features/INVENTORY.md b/.claude/skills/verify-openloop/features/INVENTORY.md index 5b2d351..41b38a9 100644 --- a/.claude/skills/verify-openloop/features/INVENTORY.md +++ b/.claude/skills/verify-openloop/features/INVENTORY.md @@ -5,28 +5,32 @@ Does **not** need to stay perfectly current between audits — the [README compl **PRDs are optional.** Prefer `strings.xml` + UI chrome. Add a PRD column only when a PRD exists. -| Surface (user-visible) | Source hint | Status | Feature file | -| ------------------------------------ | ---------------------------- | ------------------ | -------------------------------------------------- | -| Onboarding `LET'S GO!` | strings / OnboardingScreen | automated | [onboarding.md](./onboarding.md) | -| Record / stop video | `Start recording` | mapped | [record-clip.md](./record-clip.md) | -| Lenses drawer + catalogue | `Lenses and Photo Booth` | mapped | [lenses.md](./lenses.md) | -| Multi-face (1–2) lens | FaceRoster / live preview | folded into lenses | [lenses.md](./lenses.md) | -| Photo stills mode | `Camera` / `Take photo` | mapped | [photo-capture.md](./photo-capture.md) | -| Photo booth | booth tab / countdown | mapped | [photo-booth.md](./photo-booth.md) | -| Pinch zoom | zoom chip | mapped | [pinch-zoom.md](./pinch-zoom.md) | -| Import from Photos | `Import a video` | mapped | [import-video.md](./import-video.md) | -| Gallery browse / play | Gallery | mapped | [gallery.md](./gallery.md) | -| Gallery SEND | `SEND` | folded into share | [share-and-library.md](./share-and-library.md) | -| Save to Photos + share sheet | `Saved to Photos` | mapped | [share-and-library.md](./share-and-library.md) | -| Trim | Trim screen / tab | mapped | [edit-trim.md](./edit-trim.md) | -| Speed constant + curve | Speed tab | mapped | [edit-speed.md](./edit-speed.md) | -| Loop direction | Loop tab | mapped | [edit-loop.md](./edit-loop.md) | -| Filter / Looks | Filter tab | mapped | [edit-filter.md](./edit-filter.md) | -| Delete in-progress clip | toolbar Delete | mapped | [edit-delete.md](./edit-delete.md) | -| Save boomerang | `Save boomerang` | mapped | [edit-save.md](./edit-save.md) | -| Full edit → save path | run-e2e | orchestrator | [edit-and-save.md](./edit-and-save.md) | -| Permission rationale / denied | MainActivity strings | out of scope | support chrome — drive only when blocking a recipe | -| In-app review / Play update snackbar | review/update packages | out of scope | system/Play prompts, not core verify map | -| Debug report share | reverse-failed / save-failed | out of scope | support path | +| Surface (user-visible) | Source hint | Status | Feature file | +| ------------------------------------ | ---------------------------- | -------------------- | -------------------------------------------------- | +| Onboarding `LET'S GO!` | strings / OnboardingScreen | automated 2026-08-31 | [onboarding.md](./onboarding.md) | +| Record / stop video | `Start recording` | automated 2026-08-31 | [record-clip.md](./record-clip.md) | +| Lenses drawer + catalogue | `Lenses and Photo Booth` | mapped | [lenses.md](./lenses.md) | +| Multi-face (1–2) lens | FaceRoster / live preview | folded into lenses | [lenses.md](./lenses.md) | +| Photo stills mode | `Camera` / `Take photo` | mapped | [photo-capture.md](./photo-capture.md) | +| Photo booth | booth tab / countdown | mapped | [photo-booth.md](./photo-booth.md) | +| Pinch zoom | zoom chip | mapped | [pinch-zoom.md](./pinch-zoom.md) | +| Import from Photos | `Import a video` | mapped | [import-video.md](./import-video.md) | +| Gallery browse / play | Gallery | mapped | [gallery.md](./gallery.md) | +| Gallery SEND | `SEND` | folded into share | [share-and-library.md](./share-and-library.md) | +| Save to Photos + share sheet | `Saved to Photos` | mapped | [share-and-library.md](./share-and-library.md) | +| Trim | Trim screen / tab | mapped | [edit-trim.md](./edit-trim.md) | +| Speed constant + curve | Speed tab | mapped | [edit-speed.md](./edit-speed.md) | +| Loop direction | Loop tab | mapped | [edit-loop.md](./edit-loop.md) | +| Filter / Looks | Filter tab | mapped | [edit-filter.md](./edit-filter.md) | +| Delete in-progress clip | toolbar Delete | mapped | [edit-delete.md](./edit-delete.md) | +| Save boomerang | `Save boomerang` | mapped | [edit-save.md](./edit-save.md) | +| Full edit → save path | run-e2e | orchestrator | [edit-and-save.md](./edit-and-save.md) | +| Permission rationale / denied | MainActivity strings | out of scope | support chrome — drive only when blocking a recipe | +| In-app review / Play update snackbar | review/update packages | out of scope | system/Play prompts, not core verify map | +| Debug report share | reverse-failed / save-failed | out of scope | support path | When you find a new shipped control in `strings.xml` or chrome that is not listed, add a row as `missing` until a feature file exists. + +`automated ` means a `*_loop.py` verifier drives that surface on an emulator and was seen +passing on that date — not that the recipe merely mentions one. `mapped` is a written recipe with +no autonomous check behind it yet. diff --git a/.claude/skills/verify-openloop/features/README.md b/.claude/skills/verify-openloop/features/README.md index f914b61..c9fba60 100644 --- a/.claude/skills/verify-openloop/features/README.md +++ b/.claude/skills/verify-openloop/features/README.md @@ -6,7 +6,7 @@ This directory is the maintained source for verifying the user-facing behavior o - One emulator attached. Serial in `VERIFY_SERIAL` if more than one `adb devices` line is `device`. - Debug APK from **this** checkout installed (`io.github.stozo04.openloop`). -- `pwsh .cursor/skills/verify-openloop/helpers/control.ps1 doctor` passes. +- `pwsh .claude/skills/verify-openloop/helpers/control.ps1 doctor` passes. - Camera permission granted (`control.ps1 grant-camera` or install `-g`). - Never drive a physical phone unless `VERIFY_ALLOW_DEVICE=1`. - Never drive `com.OpenLoop.app`. diff --git a/.claude/skills/verify-openloop/features/record-clip.md b/.claude/skills/verify-openloop/features/record-clip.md index ca58213..763be58 100644 --- a/.claude/skills/verify-openloop/features/record-clip.md +++ b/.claude/skills/verify-openloop/features/record-clip.md @@ -16,6 +16,31 @@ From the camera viewfinder the user taps the shutter to start a video (up to 30 - Confirm capture mode is **Video** (`capture_mode_selector`, label `Video`) not Camera/stills and not Photo Booth. - Tap the large shutter at the bottom center. +## Autonomous check + +Run it from the repository root: + +```powershell +python .claude/skills/verify-openloop/helpers/record_clip_loop.py +``` + +`python scripts/run-verification-loops.py --changed` runs it alongside every other loop. Both take +`VERIFY_SERIAL` when more than one emulator is online and `VERIFY_EVIDENCE_DIR` for the artifacts. + +It installs the current debug APK, grants CAMERA, and drives all three outcomes of a shutter tap: +a sub-400 ms double tap (stays on camera, `That was quick!` snackbar, no clip), a few seconds +(Trim opens, scratch clip on disk, `Capture finalized (Nms)` in logcat), and a recording left to +run out (the 30 s cap finalizes it with no stop tap). Every countdown sample is asserted as +`s / 30s` — never a `mm:ss` clock. Both clips are thrown away through the Discard dialog, +so nothing is saved and the gallery is untouched. + +Expect roughly **75 seconds** on a healthy Pixel_8_API34 AVD, where the cap produces a 31.2 s clip. +The elapsed counter accumulates 33 ms per tick instead of reading a clock, so a device that cannot +hold the cadence overruns: the same AVD under host memory pressure took 219 s and produced a +**143 s** clip for the same 30 s cap. The cap is therefore asserted as "no stop tap, no error +finalize, Trim opened, clip ≥ 25 s" — never a wall-clock ceiling, and never how high the chip had +climbed at the last dump, which measures the polling interval rather than the product. + ## Driving it with control.ps1 Preconditions: @@ -38,4 +63,7 @@ Preconditions: - Import (`Import a video` / gallery empty `…or import one`) is a different entry into Trim — see [import-video](./import-video.md). Do not count import as `record-clip`. - Active [lenses](./lenses.md) bake into the recording; prove lens UI separately if that is the claim. - Permission rationale can sit on top of the shutter. Dump first. +- The too-short snackbar covers the shutter row: while it is up, `Start recording` / `Flip Camera` are not in the dump at all. Prove "still on camera" with the mode selector, then re-check the shutter once it clears. +- A uiautomator dump takes seconds and the snackbar lasts four, so polling with dumps loses that race. Wait on logcat (`Video burst recording failed` / `below the 400ms minimum`), then dump once. +- Under host memory pressure the AVD's own system ANRs (`System UI isn't responding`, package `android`, in the dump) and every recipe here fails at the first step. That is the host, not the app — free memory and cold-boot the AVD (`-no-snapshot-load`); `adb reboot` restores the same broken state. - Front/back flip (`Flip Camera`) does not by itself prove a recording. diff --git a/.claude/skills/verify-openloop/helpers/onboarding_loop.py b/.claude/skills/verify-openloop/helpers/onboarding_loop.py index 1339865..b1e720f 100755 --- a/.claude/skills/verify-openloop/helpers/onboarding_loop.py +++ b/.claude/skills/verify-openloop/helpers/onboarding_loop.py @@ -1,23 +1,52 @@ #!/usr/bin/env python3 +"""Autonomous verifier for `features/onboarding.md` — first run through `LET'S GO!`. + +First run shows the onboarding pitch; tapping the CTA writes the DataStore flag and lands on the +camera; a relaunch goes straight to the viewfinder on the back lens with no onboarding and no +permission rationale. + + python .claude/skills/verify-openloop/helpers/onboarding_loop.py + + VERIFY_SERIAL=emulator-5556 pick a device when more than one is online + VERIFY_EVIDENCE_DIR= where the XML/PNG/logcat evidence lands + +The adb/uiautomator plumbing lives in `verify_common.py`, shared with the other loops here. +""" from __future__ import annotations -import html -import os import re -import shutil import subprocess import sys -import tempfile -import time -import xml.etree.ElementTree as ET -from dataclasses import dataclass -from datetime import datetime from pathlib import Path -PACKAGE = "io.github.stozo04.openloop" -ACTIVITY = f"{PACKAGE}/.MainActivity" +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from verify_common import ( # noqa: E402 + PACKAGE, + adb_out, + assert_absent, + assert_contains, + clear_logcat, + dump_blob, + dump_strings, + dump_ui, + ensure_installed, + ensure_serial_allowed, + evidence_dir, + fail, + find_exact, + force_stop, + grant_camera, + require_online, + resolve_serial, + run_adb, + save_screencap, + start_activity, + tap_node, + wait_until, +) + DATASTORE_REL = "files/datastore/openloop_preferences.preferences_pb" -APK_REL = "app/build/outputs/apk/debug/app-debug.apk" ONBOARDING_MUST_HAVE = [ "Free. Forever.", @@ -32,314 +61,22 @@ FACING_FRONT_RE = re.compile(r"Camera bound \(lens=front\)") -@dataclass(frozen=True) -class UiNode: - text: str - desc: str - bounds: tuple[int, int, int, int] | None - - -def repo_root() -> Path: - return Path(__file__).resolve().parent.parent.parent.parent.parent - - -def run_adb(serial: str, *args: str, check: bool = True) -> subprocess.CompletedProcess[str]: - cmd = ["adb", "-s", serial, *args] - return subprocess.run( - cmd, - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - check=check, - ) - - -def adb_out(serial: str, *args: str, check: bool = True) -> str: - result = run_adb(serial, *args, check=check) - return (result.stdout or "") + (result.stderr or "") - - -def resolve_serial() -> str: - if not shutil.which("adb"): - fail("adb not found on PATH") - env_serial = os.environ.get("VERIFY_SERIAL", "").strip() - if env_serial: - return env_serial - - devices_out = subprocess.run( - ["adb", "devices"], - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - check=True, - ).stdout - emulators = [ - line.split()[0] - for line in devices_out.splitlines() - if re.match(r"emulator-\d+\s+device$", line) - ] - if len(emulators) == 1: - return emulators[0] - if len(emulators) > 1: - fail(f"multiple emulators: {', '.join(emulators)}; set VERIFY_SERIAL") - - physical = [ - line.split()[0] - for line in devices_out.splitlines() - if re.search(r"\s+device$", line) and not line.startswith("emulator-") - ] - if physical: - if os.environ.get("VERIFY_ALLOW_DEVICE") == "1" and env_serial: - return env_serial - fail( - f"physical device {physical[0]} attached; start an emulator or set " - "VERIFY_ALLOW_DEVICE=1 and VERIFY_SERIAL" - ) - fail("no emulator or device (adb devices)") - - -def ensure_serial_allowed(serial: str) -> None: - if serial.startswith("emulator-"): - return - if os.environ.get("VERIFY_ALLOW_DEVICE") == "1": - return - fail(f"serial {serial} is not an emulator; set VERIFY_ALLOW_DEVICE=1 for a test phone") - - -def evidence_dir() -> Path: - base = os.environ.get("VERIFY_EVIDENCE_DIR") - if base: - path = Path(base) - else: - stamp = datetime.now().strftime("%Y%m%d_%H%M%S") - path = Path(tempfile.gettempdir()) / "openloop-verify" / stamp / "onboarding" - path.mkdir(parents=True, exist_ok=True) - return path - - -def decode_entities(value: str) -> str: - if not value: - return value - return html.unescape(value) - - -def parse_bounds(raw: str) -> tuple[int, int, int, int] | None: - match = re.match(r"\[(\d+),(\d+)\]\[(\d+),(\d+)\]", raw) - if not match: - return None - return tuple(int(g) for g in match.groups()) # type: ignore[return-value] - - -def parse_nodes_regex(xml_text: str) -> list[UiNode]: - nodes: list[UiNode] = [] - for match in re.finditer(r"]*>", xml_text): - fragment = match.group(0) - text_m = re.search(r'text="([^"]*)"', fragment) - desc_m = re.search(r'content-desc="([^"]*)"', fragment) - bounds_m = re.search(r'bounds="(\[[^\]]+\]\[[^\]]+\])"', fragment) - bounds = parse_bounds(bounds_m.group(1)) if bounds_m else None - nodes.append( - UiNode( - text=decode_entities(text_m.group(1) if text_m else ""), - desc=decode_entities(desc_m.group(1) if desc_m else ""), - bounds=bounds, - ) - ) - return nodes - - -def parse_nodes_etree(xml_text: str) -> list[UiNode]: - nodes: list[UiNode] = [] - root = ET.fromstring(xml_text) - for elem in root.iter("node"): - bounds = parse_bounds(elem.attrib.get("bounds", "")) - nodes.append( - UiNode( - text=decode_entities(elem.attrib.get("text", "")), - desc=decode_entities(elem.attrib.get("content-desc", "")), - bounds=bounds, - ) - ) - return nodes - - -def parse_nodes(xml_text: str) -> list[UiNode]: - try: - return parse_nodes_etree(xml_text) - except ET.ParseError: - return parse_nodes_regex(xml_text) - - -def dump_ui(serial: str) -> tuple[str, list[UiNode]]: - dump_result = run_adb(serial, "shell", "uiautomator", "dump", "/sdcard/ui.xml", check=False) - if dump_result.returncode != 0: - return "", [] - result = run_adb(serial, "shell", "cat", "/sdcard/ui.xml", check=False) - xml_text = (result.stdout or "").strip() - marker = " set[str]: - values: set[str] = set() - for node in nodes: - if node.text: - values.add(node.text) - if node.desc: - values.add(node.desc) - return values - - -def dump_blob(nodes: list[UiNode]) -> str: - parts: list[str] = [] - for node in nodes: - if node.text: - parts.append(node.text) - if node.desc: - parts.append(node.desc) - return "\n".join(parts) - - -def find_exact(nodes: list[UiNode], label: str) -> UiNode | None: - for node in nodes: - if not node.bounds: - continue - if node.text == label or node.desc == label: - return node - return None - - -def tap_node(serial: str, node: UiNode) -> None: - if not node.bounds: - fail("no bounds for tap target") - x1, y1, x2, y2 = node.bounds - cx = (x1 + x2) // 2 - cy = (y1 + y2) // 2 - run_adb(serial, "shell", "input", "tap", str(cx), str(cy)) - - -def save_screencap(serial: str, path: Path) -> None: - proc = subprocess.run( - ["adb", "-s", serial, "exec-out", "screencap", "-p"], - capture_output=True, - check=True, - ) - path.write_bytes(proc.stdout) - - -def package_installed(serial: str) -> bool: - out = adb_out(serial, "shell", "pm", "path", PACKAGE, check=False) - return "package:" in out - - -def ensure_installed(serial: str) -> None: - apk = repo_root() / APK_REL - if not apk.is_file(): - if package_installed(serial): - return - fail(f"{PACKAGE} not installed and debug APK missing at {apk}") - install = subprocess.run( - ["adb", "-s", serial, "install", "-r", "-g", str(apk)], - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - ) - combined = (install.stdout or "") + (install.stderr or "") - if install.returncode != 0: - fail(f"adb install failed: {combined.strip()}") - if not package_installed(serial): - fail(f"adb install reported success but {PACKAGE} is still missing") - - -def grant_camera(serial: str) -> None: - run_adb(serial, "shell", "pm", "grant", PACKAGE, "android.permission.CAMERA") - - -def force_stop(serial: str) -> None: - run_adb(serial, "shell", "am", "force-stop", PACKAGE) - - def reset_onboarding_store(serial: str) -> None: force_stop(serial) - run_adb( - serial, - "shell", - "run-as", - PACKAGE, - "rm", - "-f", - DATASTORE_REL, - check=False, - ) + run_adb(serial, "shell", "run-as", PACKAGE, "rm", "-f", DATASTORE_REL, check=False) def datastore_exists(serial: str) -> bool: - out = adb_out( - serial, - "shell", - "run-as", - PACKAGE, - "ls", - DATASTORE_REL, - check=False, - ) + out = adb_out(serial, "shell", "run-as", PACKAGE, "ls", DATASTORE_REL, check=False) return DATASTORE_REL.split("/")[-1] in out and "No such file" not in out -def clear_logcat(serial: str) -> None: - run_adb(serial, "logcat", "-c", check=False) - - -def start_activity(serial: str) -> None: - run_adb(serial, "shell", "am", "start", "-n", ACTIVITY) - - -def wait_until( - predicate, - timeout_s: float, - interval_s: float = 0.5, -) -> bool: - deadline = time.monotonic() + timeout_s - while time.monotonic() < deadline: - if predicate(): - return True - time.sleep(interval_s) - return False - - -def assert_contains(strings: set[str], blob: str, required: list[str], context: str) -> None: - missing = [item for item in required if item not in strings and item not in blob] - if missing: - fail(f"{context}: missing {missing[0]!r} (also checked: {missing[1:]})") - - -def assert_absent(strings: set[str], blob: str, forbidden: list[str], context: str) -> None: - for item in forbidden: - if item in strings or item in blob: - fail(f"{context}: must not contain {item!r}") - - -def fail(message: str) -> None: - print(f"FAIL {message}") - sys.exit(1) - - def main() -> int: serial = resolve_serial() ensure_serial_allowed(serial) + require_online(serial) - state = adb_out(serial, "get-state").strip() - if state != "device": - fail(f"serial={serial} get-state={state!r}") - - evidence = evidence_dir() + evidence = evidence_dir("onboarding") ensure_installed(serial) grant_camera(serial) reset_onboarding_store(serial) @@ -348,7 +85,7 @@ def main() -> int: start_activity(serial) first_xml = "" - first_nodes: list[UiNode] = [] + first_nodes: list = [] def poll_first_run() -> bool: nonlocal first_xml, first_nodes @@ -410,7 +147,7 @@ def poll_after_cta() -> bool: start_activity(serial) returning_xml = "" - returning_nodes: list[UiNode] = [] + returning_nodes: list = [] def poll_returning() -> bool: nonlocal returning_xml, returning_nodes diff --git a/.claude/skills/verify-openloop/helpers/record_clip_loop.py b/.claude/skills/verify-openloop/helpers/record_clip_loop.py new file mode 100644 index 0000000..bb0c478 --- /dev/null +++ b/.claude/skills/verify-openloop/helpers/record_clip_loop.py @@ -0,0 +1,447 @@ +#!/usr/bin/env python3 +"""Autonomous verifier for `features/record-clip.md` — record / stop video. + +Drives the installed debug APK on an emulator through the three outcomes a shutter tap can have, +and pins the countdown chip's format in every one of them: + + 1. too-short — start+stop inside the 400 ms minimum: stays on camera, snackbar + "That was quick! Record a little longer to make a loop.", no clip. + 2. mid-length — a few seconds: Trim opens and a scratch clip exists on disk. + 3. cap — never tap stop: recording finalizes itself at the 30 s cap and Trim opens. + +The countdown chip is asserted on every sample as `s / 30s` and never as a `mm:ss` clock +— issue #154 shipped it reading `00:00`, and a format regression is invisible to a Compose test +that only checks the chip exists. Both clips are discarded through the product's own Discard +dialog, so the run leaves no media behind (saving is `edit-save.md`, not this feature). + + python .claude/skills/verify-openloop/helpers/record_clip_loop.py + + VERIFY_SERIAL=emulator-5556 pick a device when more than one is online + VERIFY_EVIDENCE_DIR= where the XML/PNG/logcat evidence lands + +Runtime is dominated by scenario 3 and depends on how well the device holds a 33 ms tick: the +counter accumulates ticks instead of reading a clock. A healthy Pixel_8_API34 AVD runs the whole +loop in ~75 s and the cap yields a 31.2 s clip; the same AVD memory-starved took 219 s and +produced a 143 s clip for the same cap. Scenario 3 therefore asserts the cap through what does +not vary with that — no stop tap, no error finalize, Trim opens, and the clip comes back at least +25 s — never a wall-clock ceiling and never how high the chip had climbed at the last dump. See +the docstring on `scenario_cap`. +""" +from __future__ import annotations + +import re +import sys +import time +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from verify_common import ( # noqa: E402 + PACKAGE, + adb_out, + app_nodes, + assert_absent, + assert_contains, + center, + clear_logcat, + dump_blob, + dump_strings, + dump_ui, + ensure_installed, + ensure_serial_allowed, + evidence_dir, + fail, + find_exact, + force_stop, + grant_camera, + require_online, + resolve_serial, + run_adb, + save_screencap, + start_activity, + tap_node, + wait_until, +) + +CAMERA_IDLE_MUST_HAVE = ["Start recording", "Gallery", "Flip Camera"] +TRIM_MUST_HAVE = ["TRIM YOUR VIDEO", "SAVE"] +TOO_SHORT_SNACKBAR = "That was quick! Record a little longer to make a loop." + +# The shipped chip is `"${elapsed / 1_000}s / $capLabel"` (CameraScreen.kt) — whole seconds both +# sides. CLOCK_FORMAT_RE is the regression it replaced: a zero-padded mm:ss clock, which for a +# sub-minute cap sat at "00:00" for the first second of every recording (issue #154). +CHIP_RE = re.compile(r"^(\d+)s / (\d+)s$") +CLOCK_FORMAT_RE = re.compile(r"\b\d{1,2}:\d{2}\b") + +FINALIZED_RE = re.compile(r"Capture finalized \((\d+)ms\)") +RECORDING_FAILED_RE = re.compile(r"Video burst recording failed") +TOO_SHORT_LOG_RE = re.compile(r"Capture finalized below the \d+ms minimum|Video burst recording failed") +SCRATCH_REL = "files/scratch" + +# MIN_TRIM_DURATION / MAX_RECORDING in OpenLoopViewModel. A clip under the minimum is discarded +# with the snackbar instead of opening a Trim screen whose SAVE would be dead. +MIN_CLIP_MS = 400 +CAP_SECONDS = 30 +# A recording left to run out must come back as most of the cap. Floor, not equality: the counter +# accumulates ticks rather than reading a clock, so a device that cannot hold the cadence overruns +# (143 s measured on a memory-starved AVD) while a healthy one lands just past 30 s. Both are the +# cap firing; a clip that stopped at 8 s is not. +CAP_MIN_CLIP_MS = 25_000 + + +class Chip: + """Every countdown-chip value seen across the whole run, asserted as it is collected.""" + + def __init__(self) -> None: + self.samples: list[int] = [] + + def observe(self, nodes: list, context: str) -> int | None: + # App-drawn nodes only: the system status bar's own clock reads "8:52" and would trip the + # mm:ss check on every single dump. + strings = dump_strings(app_nodes(nodes)) + for value in strings: + match = CHIP_RE.match(value) + if not match: + continue + if int(match.group(2)) != CAP_SECONDS: + fail(f"{context}: countdown chip reads {value!r}; the cap side should be {CAP_SECONDS}s") + seconds = int(match.group(1)) + if seconds > CAP_SECONDS: + fail(f"{context}: countdown chip reads {value!r}, past the {CAP_SECONDS}s cap") + self.samples.append(seconds) + return seconds + # No chip in this dump is only an error while recording, which the callers decide. A + # mm:ss clock drawn by the app is the issue #154 regression and is never allowed. + offender = next((v for v in strings if CLOCK_FORMAT_RE.search(v)), None) + if offender: + fail(f"{context}: countdown shows a mm:ss clock {offender!r}; it must read 's / 30s'") + return None + + +def snapshot(serial: str, evidence: Path, name: str, xml: str, screenshot: bool = True) -> Path: + path = evidence / f"{name}.xml" + path.write_text(xml, encoding="utf-8") + if screenshot: + save_screencap(serial, evidence / f"{name}.png") + return path + + +def scratch_clips(serial: str) -> set[str]: + out = adb_out(serial, "shell", "run-as", PACKAGE, "ls", SCRATCH_REL, check=False) + return {line.strip() for line in out.splitlines() if line.strip().startswith("raw_")} + + +class Scratch: + """Per-capture clip files under `filesDir/scratch`, measured against what was already there. + + Only clips THIS run created are asserted on. A scratch file left by an abandoned session + elsewhere is not this feature's to judge — and deleting it to get a clean slate would throw + away someone else's in-progress capture, which the verifier contract forbids. + """ + + def __init__(self, serial: str) -> None: + self.baseline = scratch_clips(serial) + + def new(self, serial: str) -> list[str]: + return sorted(scratch_clips(serial) - self.baseline) + + +def wait_for_camera_idle(serial: str, evidence: Path, context: str, timeout_s: float = 45.0) -> tuple[str, list]: + """Poll until the viewfinder is idle, tapping through onboarding or stills mode if needed. + + Deliberately does NOT reset the onboarding DataStore: this feature does not own that state, + and wiping it would make the loop destructive to a device someone else is mid-test on. + """ + seen: dict = {"xml": "", "nodes": []} + + def ready() -> bool: + xml, nodes = dump_ui(serial) + if not nodes: + return False + seen["xml"], seen["nodes"] = xml, nodes + strings, blob = dump_strings(nodes), dump_blob(nodes) + if "Start recording" in strings: + return True + cta = find_exact(nodes, "LET'S GO!") + if cta: # first run on a fresh install — walk through it like a user + tap_node(serial, cta) + return False + if "Take photo" in strings: # stills mode left over from another recipe + video = find_exact(nodes, "Video") + if video: + tap_node(serial, video) + return False + + if not wait_until(ready, timeout_s=timeout_s, interval_s=1.0): + path = snapshot(serial, evidence, f"{context}-not-idle", seen["xml"]) + fail(f"{context}: camera never reached idle (no 'Start recording'); evidence={path}") + strings, blob = dump_strings(seen["nodes"]), dump_blob(seen["nodes"]) + assert_contains(strings, blob, CAMERA_IDLE_MUST_HAVE, context) + assert_absent(strings, blob, ["TRIM YOUR VIDEO"], context) + return seen["xml"], seen["nodes"] + + +def shutter(serial: str, nodes: list, label: str, context: str, evidence: Path): + node = find_exact(nodes, label) + if not node: + fail(f"{context}: no tap target for {label!r}; evidence={evidence}") + return node + + +def wait_for_trim(serial: str, chip: Chip, context: str, timeout_s: float, interval_s: float = 2.0) -> str: + """Poll for the Trim screen, sampling the countdown chip on every dump along the way.""" + seen = {"xml": ""} + + def arrived() -> bool: + xml, nodes = dump_ui(serial) + if not nodes: + return False + seen["xml"] = xml + strings, blob = dump_strings(nodes), dump_blob(nodes) + chip.observe(nodes, context) + return "TRIM YOUR VIDEO" in strings or "TRIM YOUR VIDEO" in blob + + ok = wait_until(arrived, timeout_s=timeout_s, interval_s=interval_s) + return seen["xml"] if ok else "" + + +def discard_clip(serial: str, evidence: Path, context: str, scratch: Scratch) -> None: + """Throw the session clip away through the product's Discard dialog, back to the camera.""" + xml, nodes = dump_ui(serial) + opener = find_exact(nodes, "Discard clip") or find_exact(nodes, "Delete") + if not opener: + path = snapshot(serial, evidence, f"{context}-no-discard", xml) + fail(f"{context}: editor has no 'Discard clip'/'Delete' affordance; evidence={path}") + tap_node(serial, opener) + + seen = {"xml": "", "nodes": []} + + def dialog_up() -> bool: + seen["xml"], seen["nodes"] = dump_ui(serial) + return find_exact(seen["nodes"], "Discard") is not None and any( + "Discard this clip?" in v for v in dump_strings(seen["nodes"]) + ) + + if not wait_until(dialog_up, timeout_s=20.0, interval_s=1.0): + path = snapshot(serial, evidence, f"{context}-no-dialog", seen["xml"]) + fail(f"{context}: 'Discard this clip?' dialog never appeared; evidence={path}") + tap_node(serial, find_exact(seen["nodes"], "Discard")) + + if not wait_until(lambda: "Start recording" in dump_strings(dump_ui(serial)[1]), + timeout_s=45.0, interval_s=1.0): + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, f"{context}-discard-stuck", xml) + fail(f"{context}: discard did not return to the camera; evidence={path}") + left = scratch.new(serial) + if left: + fail(f"{context}: discard left the scratch clip behind: {left}") + + +def scenario_too_short(serial: str, evidence: Path, chip: Chip, scratch: Scratch) -> str: + """Start and stop inside the minimum: no clip, no Trim, and the 'record longer' snackbar. + + Both taps go out in ONE `adb shell`, because a dump-parse-tap round trip is seconds long and + would always produce a valid clip instead of the too-short case. Establishing that precondition + is still timing-dependent, so a run that accidentally records a real clip is retried (and its + clip discarded) rather than asserted against. + """ + for attempt in range(1, 4): + _, nodes = wait_for_camera_idle(serial, evidence, "too-short") + cx, cy = center(shutter(serial, nodes, "Start recording", "too-short", evidence)) + clear_logcat(serial) + run_adb(serial, "shell", f"input tap {cx} {cy}; input tap {cx} {cy}") + + # The snackbar is on screen for four seconds and a uiautomator dump takes about five, so + # polling with dumps loses the race: the first snapshot lands before the capture has even + # finalized and the second lands after the snackbar is gone. Logcat costs a fraction of a + # second and says exactly when the capture ended, so wait on THAT and dump once, into the + # open window. + outcome = {"logcat": ""} + + def finalized() -> bool: + outcome["logcat"] = adb_out(serial, "logcat", "-d", "-s", "OpenLoopViewModel:*") + return bool(TOO_SHORT_LOG_RE.search(outcome["logcat"])) or bool( + FINALIZED_RE.search(outcome["logcat"]) + ) + + if not wait_until(finalized, timeout_s=30.0, interval_s=0.3): + fail("too-short: the double tap produced no capture at all within 30s") + logcat = outcome["logcat"] + seen: dict = {"xml": "", "nodes": []} + seen["xml"], seen["nodes"] = dump_ui(serial) + strings, blob = dump_strings(seen["nodes"]), dump_blob(seen["nodes"]) + + if (FINALIZED_RE.search(logcat) and not TOO_SHORT_LOG_RE.search(logcat)) or "TRIM YOUR VIDEO" in strings: + # The two taps landed far enough apart to encode a real clip: the precondition failed, + # not the product. Clean up and try again. + print(f" too-short attempt {attempt}: taps produced a full clip, discarding and retrying") + discard_clip(serial, evidence, f"too-short-retry{attempt}", scratch) + continue + + snapshot(serial, evidence, "too-short", seen["xml"]) + (evidence / "too-short-logcat.txt").write_text(logcat, encoding="utf-8") + if TOO_SHORT_SNACKBAR not in strings and attempt < 3: + # The capture was rejected as it should be, but the dump landed outside the snackbar's + # four seconds. That is this harness losing a race, not the product; the last attempt + # asserts for real rather than retrying forever. + print(f" too-short attempt {attempt}: dump missed the snackbar window, retrying") + continue + chip.observe(seen["nodes"], "too-short") + # While the snackbar is up it sits over the shutter row, and those controls drop out of the + # hierarchy — so the "still on camera" proof here is the mode selector plus the absence of + # any editor, and the shutter itself is checked once the snackbar has gone. + assert_contains(strings, blob, [TOO_SHORT_SNACKBAR, "Video", "Camera"], "too-short") + assert_absent(strings, blob, ["TRIM YOUR VIDEO", "Stop recording"], "too-short") + left = scratch.new(serial) + if left: + fail(f"too-short: a clip was kept anyway: {left}") + wait_for_camera_idle(serial, evidence, "too-short-after") + return f"attempt{attempt}" + + fail("too-short: three double-taps in a row recorded a real clip; the shutter never saw a sub-400ms press") + + +def scenario_mid_length(serial: str, evidence: Path, chip: Chip, scratch: Scratch) -> int: + """A few seconds of recording: the chip counts in seconds, stop opens Trim, a clip exists.""" + _, nodes = wait_for_camera_idle(serial, evidence, "mid") + clear_logcat(serial) + tap_node(serial, shutter(serial, nodes, "Start recording", "mid", evidence)) + + seen = {"xml": "", "nodes": []} + + def recording() -> bool: + seen["xml"], seen["nodes"] = dump_ui(serial) + strings, blob = dump_strings(seen["nodes"]), dump_blob(seen["nodes"]) + if "Stop recording" not in strings: + return False + seconds = chip.observe(seen["nodes"], "mid-recording") + return seconds is not None and seconds >= 1 + + if not wait_until(recording, timeout_s=90.0, interval_s=1.0): + path = snapshot(serial, evidence, "mid-recording-stalled", seen["xml"]) + fail(f"mid: countdown never reached 1s while recording; evidence={path}") + snapshot(serial, evidence, "mid-recording", seen["xml"]) + + tap_node(serial, shutter(serial, seen["nodes"], "Stop recording", "mid", evidence)) + trim_xml = wait_for_trim(serial, chip, "mid-stop", timeout_s=90.0) + if not trim_xml: + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, "mid-no-trim", xml) + fail(f"mid: stop did not open Trim; evidence={path}") + snapshot(serial, evidence, "mid-trim", trim_xml) + + _, trim_nodes = dump_ui(serial) + strings, blob = dump_strings(trim_nodes), dump_blob(trim_nodes) + assert_contains(strings, blob, TRIM_MUST_HAVE, "mid-trim") + + logcat = adb_out(serial, "logcat", "-d", "-s", "OpenLoopViewModel:*") + (evidence / "mid-logcat.txt").write_text(logcat, encoding="utf-8") + match = FINALIZED_RE.search(logcat) + if not match: + fail(f"mid: no 'Capture finalized (Nms)' in logcat; evidence={evidence / 'mid-logcat.txt'}") + duration_ms = int(match.group(1)) + if duration_ms < MIN_CLIP_MS: + fail(f"mid: clip finalized at {duration_ms}ms, under the {MIN_CLIP_MS}ms minimum") + clips = scratch.new(serial) + if not clips: + fail(f"mid: Trim is open but no scratch clip exists under {SCRATCH_REL}") + + discard_clip(serial, evidence, "mid", scratch) + return duration_ms + + +def scenario_cap(serial: str, evidence: Path, chip: Chip, scratch: Scratch) -> tuple[int, int]: + """Never tap stop: the 30 s cap finalizes the recording on its own and Trim opens. + + Four things make that the cap and not something else: no stop tap is ever sent, Trim arrives + anyway, logcat has no error finalize, and the clip that comes back is at least + [CAP_MIN_CLIP_MS] — a recording cut short by anything else would not be. + + Deliberately NOT asserted: how high the chip had climbed when the last dump landed. That + number measures the sampling interval, not the product — a dump costs seconds, so the faster + the device runs the counter, the further from the cap the final sample sits (17s on a + cold-booted AVD, 29s on a loaded one). Nor is a wall-clock ceiling asserted: the counter + accumulates 33 ms per tick instead of reading a clock, so a device that cannot hold the + cadence overruns the cap in real seconds (143 s measured on a memory-starved AVD). Both are + the cap firing. The chip is still asserted on every sample here — for its *format*, which is + what issue #154 broke. + """ + _, nodes = wait_for_camera_idle(serial, evidence, "cap") + clear_logcat(serial) + tap_node(serial, shutter(serial, nodes, "Start recording", "cap", evidence)) + + if not wait_until(lambda: "Stop recording" in dump_strings(dump_ui(serial)[1]), + timeout_s=60.0, interval_s=1.0): + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, "cap-never-started", xml) + fail(f"cap: recording never started; evidence={path}") + + before = len(chip.samples) + # 3 s between dumps: a dump is seconds of work on the device's main thread, and hammering it + # starves the very tick loop under test — but sample too rarely and the countdown's format + # goes unchecked for most of the recording. + trim_xml = wait_for_trim(serial, chip, "cap-recording", timeout_s=480.0, interval_s=3.0) + if not trim_xml: + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, "cap-no-trim", xml) + fail(f"cap: recording did not finalize itself within 480s; evidence={path}") + snapshot(serial, evidence, "cap-trim", trim_xml) + + samples = chip.samples[before:] + if not samples: + fail("cap: the countdown chip never rendered during a full-length recording") + peak = max(samples) + + logcat = adb_out(serial, "logcat", "-d", "-s", "OpenLoopViewModel:*") + (evidence / "cap-logcat.txt").write_text(logcat, encoding="utf-8") + if RECORDING_FAILED_RE.search(logcat): + fail(f"cap: recording ended in an error, not the cap; evidence={evidence / 'cap-logcat.txt'}") + match = FINALIZED_RE.search(logcat) + if not match: + fail(f"cap: no 'Capture finalized (Nms)' in logcat; evidence={evidence / 'cap-logcat.txt'}") + duration_ms = int(match.group(1)) + if duration_ms < CAP_MIN_CLIP_MS: + fail( + f"cap: the recording stopped itself at {duration_ms}ms, short of the {CAP_SECONDS}s cap " + f"— something other than the cap ended it; evidence={evidence / 'cap-logcat.txt'}" + ) + + discard_clip(serial, evidence, "cap", scratch) + return peak, duration_ms + + +def main() -> int: + serial = resolve_serial() + ensure_serial_allowed(serial) + require_online(serial) + + evidence = evidence_dir("record-clip") + ensure_installed(serial) + grant_camera(serial) + force_stop(serial) + clear_logcat(serial) + start_activity(serial) + + chip = Chip() + scratch = Scratch(serial) + started = time.monotonic() + short = scenario_too_short(serial, evidence, chip, scratch) + mid_ms = scenario_mid_length(serial, evidence, chip, scratch) + peak, cap_ms = scenario_cap(serial, evidence, chip, scratch) + + force_stop(serial) + print( + f"PASS serial={serial} too-short={short} mid={mid_ms}ms cap={cap_ms}ms cap-chip-peak={peak}s " + f"chip-samples={len(chip.samples)} took={int(time.monotonic() - started)}s evidence={evidence}" + ) + return 0 + + +if __name__ == "__main__": + import subprocess + + try: + raise SystemExit(main()) + except subprocess.CalledProcessError as exc: + cmd = " ".join(exc.cmd if isinstance(exc.cmd, list) else [str(exc.cmd)]) + fail(f"adb command failed ({cmd}): {(exc.stderr or exc.stdout or '').strip()}") diff --git a/.claude/skills/verify-openloop/helpers/verify_common.py b/.claude/skills/verify-openloop/helpers/verify_common.py new file mode 100644 index 0000000..3bd9ce8 --- /dev/null +++ b/.claude/skills/verify-openloop/helpers/verify_common.py @@ -0,0 +1,325 @@ +#!/usr/bin/env python3 +"""Shared adb/uiautomator plumbing for the `*_loop.py` verifiers in this directory. + +Extracted when the second real loop (`record_clip_loop.py`) landed and duplicated ~200 lines of +`onboarding_loop.py` verbatim — the point the create-verifier skill names for extracting, and not +before. Nothing feature-specific lives here: a loop owns its own strings, assertions and evidence. + +Not a loop itself. `scripts/run-verification-loops.py` discovers `*_loop.py`, so this file is +never executed as a verifier; the loops import it as a sibling module. +""" +from __future__ import annotations + +import html +import os +import re +import shutil +import subprocess +import sys +import tempfile +import xml.etree.ElementTree as ET +from dataclasses import dataclass +from datetime import datetime +from pathlib import Path + +PACKAGE = "io.github.stozo04.openloop" +ACTIVITY = f"{PACKAGE}/.MainActivity" +APK_REL = "app/build/outputs/apk/debug/app-debug.apk" + + +@dataclass(frozen=True) +class UiNode: + text: str + desc: str + bounds: tuple[int, int, int, int] | None + # Owning package. A dump is the whole screen, so the system status bar (its clock above all) + # and the navigation bar come back alongside the app — filter on this before asserting that + # some text is or is not on screen "in the app". + pkg: str = "" + + +def fail(message: str) -> None: + print(f"FAIL {message}") + sys.exit(1) + + +def repo_root() -> Path: + return Path(__file__).resolve().parent.parent.parent.parent.parent + + +def run_adb(serial: str, *args: str, check: bool = True) -> subprocess.CompletedProcess[str]: + cmd = ["adb", "-s", serial, *args] + return subprocess.run( + cmd, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + check=check, + ) + + +def adb_out(serial: str, *args: str, check: bool = True) -> str: + result = run_adb(serial, *args, check=check) + return (result.stdout or "") + (result.stderr or "") + + +def resolve_serial() -> str: + if not shutil.which("adb"): + fail("adb not found on PATH") + env_serial = os.environ.get("VERIFY_SERIAL", "").strip() + if env_serial: + return env_serial + + devices_out = subprocess.run( + ["adb", "devices"], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + check=True, + ).stdout + emulators = [ + line.split()[0] + for line in devices_out.splitlines() + if re.match(r"emulator-\d+\s+device$", line) + ] + if len(emulators) == 1: + return emulators[0] + if len(emulators) > 1: + fail(f"multiple emulators: {', '.join(emulators)}; set VERIFY_SERIAL") + + physical = [ + line.split()[0] + for line in devices_out.splitlines() + if re.search(r"\s+device$", line) and not line.startswith("emulator-") + ] + if physical: + if os.environ.get("VERIFY_ALLOW_DEVICE") == "1" and env_serial: + return env_serial + fail( + f"physical device {physical[0]} attached; start an emulator or set " + "VERIFY_ALLOW_DEVICE=1 and VERIFY_SERIAL" + ) + fail("no emulator or device (adb devices)") + + +def ensure_serial_allowed(serial: str) -> None: + if serial.startswith("emulator-"): + return + if os.environ.get("VERIFY_ALLOW_DEVICE") == "1": + return + fail(f"serial {serial} is not an emulator; set VERIFY_ALLOW_DEVICE=1 for a test phone") + + +def require_online(serial: str) -> None: + state = adb_out(serial, "get-state").strip() + if state != "device": + fail(f"serial={serial} get-state={state!r}") + + +def evidence_dir(feature: str) -> Path: + base = os.environ.get("VERIFY_EVIDENCE_DIR") + if base: + path = Path(base) + else: + stamp = datetime.now().strftime("%Y%m%d_%H%M%S") + path = Path(tempfile.gettempdir()) / "openloop-verify" / stamp / feature + path.mkdir(parents=True, exist_ok=True) + return path + + +def decode_entities(value: str) -> str: + if not value: + return value + return html.unescape(value) + + +def parse_bounds(raw: str) -> tuple[int, int, int, int] | None: + match = re.match(r"\[(\d+),(\d+)\]\[(\d+),(\d+)\]", raw) + if not match: + return None + return tuple(int(g) for g in match.groups()) # type: ignore[return-value] + + +def parse_nodes_regex(xml_text: str) -> list[UiNode]: + nodes: list[UiNode] = [] + for match in re.finditer(r"]*>", xml_text): + fragment = match.group(0) + text_m = re.search(r'text="([^"]*)"', fragment) + desc_m = re.search(r'content-desc="([^"]*)"', fragment) + bounds_m = re.search(r'bounds="(\[[^\]]+\]\[[^\]]+\])"', fragment) + pkg_m = re.search(r'package="([^"]*)"', fragment) + bounds = parse_bounds(bounds_m.group(1)) if bounds_m else None + nodes.append( + UiNode( + text=decode_entities(text_m.group(1) if text_m else ""), + desc=decode_entities(desc_m.group(1) if desc_m else ""), + bounds=bounds, + pkg=pkg_m.group(1) if pkg_m else "", + ) + ) + return nodes + + +def parse_nodes_etree(xml_text: str) -> list[UiNode]: + nodes: list[UiNode] = [] + root = ET.fromstring(xml_text) + for elem in root.iter("node"): + bounds = parse_bounds(elem.attrib.get("bounds", "")) + nodes.append( + UiNode( + text=decode_entities(elem.attrib.get("text", "")), + desc=decode_entities(elem.attrib.get("content-desc", "")), + bounds=bounds, + pkg=elem.attrib.get("package", ""), + ) + ) + return nodes + + +def parse_nodes(xml_text: str) -> list[UiNode]: + try: + return parse_nodes_etree(xml_text) + except ET.ParseError: + return parse_nodes_regex(xml_text) + + +def dump_ui(serial: str) -> tuple[str, list[UiNode]]: + """Current hierarchy, or ("", []) when uiautomator could not produce one. + + The dump file is only read when THIS dump wrote it: uiautomator leaves the previous XML in + place when it fails ("ERROR: null root node returned by UiTestAutomationBridge", seen on a + busy camera preview), and reading that back would assert against a screen that is gone. + """ + dump_result = run_adb(serial, "shell", "uiautomator", "dump", "/sdcard/ui.xml", check=False) + out = (dump_result.stdout or "") + (dump_result.stderr or "") + if dump_result.returncode != 0 or "dumped to" not in out: + return "", [] + result = run_adb(serial, "shell", "cat", "/sdcard/ui.xml", check=False) + xml_text = (result.stdout or "").strip() + marker = " list[UiNode]: + """Only the nodes OpenLoop drew — no system status bar, no navigation bar.""" + return [node for node in nodes if node.pkg == PACKAGE] + + +def dump_strings(nodes: list[UiNode]) -> set[str]: + values: set[str] = set() + for node in nodes: + if node.text: + values.add(node.text) + if node.desc: + values.add(node.desc) + return values + + +def dump_blob(nodes: list[UiNode]) -> str: + parts: list[str] = [] + for node in nodes: + if node.text: + parts.append(node.text) + if node.desc: + parts.append(node.desc) + return "\n".join(parts) + + +def find_exact(nodes: list[UiNode], label: str) -> UiNode | None: + for node in nodes: + if not node.bounds: + continue + if node.text == label or node.desc == label: + return node + return None + + +def center(node: UiNode) -> tuple[int, int]: + if not node.bounds: + fail("no bounds for tap target") + x1, y1, x2, y2 = node.bounds + return (x1 + x2) // 2, (y1 + y2) // 2 + + +def tap_node(serial: str, node: UiNode) -> None: + cx, cy = center(node) + run_adb(serial, "shell", "input", "tap", str(cx), str(cy)) + + +def save_screencap(serial: str, path: Path) -> None: + proc = subprocess.run( + ["adb", "-s", serial, "exec-out", "screencap", "-p"], + capture_output=True, + check=True, + ) + path.write_bytes(proc.stdout) + + +def package_installed(serial: str) -> bool: + out = adb_out(serial, "shell", "pm", "path", PACKAGE, check=False) + return "package:" in out + + +def ensure_installed(serial: str) -> None: + apk = repo_root() / APK_REL + if not apk.is_file(): + if package_installed(serial): + return + fail(f"{PACKAGE} not installed and debug APK missing at {apk}") + install = subprocess.run( + ["adb", "-s", serial, "install", "-r", "-g", str(apk)], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + combined = (install.stdout or "") + (install.stderr or "") + if install.returncode != 0: + fail(f"adb install failed: {combined.strip()}") + if not package_installed(serial): + fail(f"adb install reported success but {PACKAGE} is still missing") + + +def grant_camera(serial: str) -> None: + run_adb(serial, "shell", "pm", "grant", PACKAGE, "android.permission.CAMERA") + + +def force_stop(serial: str) -> None: + run_adb(serial, "shell", "am", "force-stop", PACKAGE) + + +def clear_logcat(serial: str) -> None: + run_adb(serial, "logcat", "-c", check=False) + + +def start_activity(serial: str) -> None: + run_adb(serial, "shell", "am", "start", "-n", ACTIVITY) + + +def wait_until(predicate, timeout_s: float, interval_s: float = 0.5) -> bool: + import time + + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(interval_s) + return False + + +def assert_contains(strings: set[str], blob: str, required: list[str], context: str) -> None: + missing = [item for item in required if item not in strings and item not in blob] + if missing: + fail(f"{context}: missing {missing[0]!r} (also checked: {missing[1:]})") + + +def assert_absent(strings: set[str], blob: str, forbidden: list[str], context: str) -> None: + for item in forbidden: + if item in strings or item in blob: + fail(f"{context}: must not contain {item!r}") diff --git a/.codex/skills/create-verifier/SKILL.md b/.codex/skills/create-verifier/SKILL.md index b9bda98..4251a32 100644 --- a/.codex/skills/create-verifier/SKILL.md +++ b/.codex/skills/create-verifier/SKILL.md @@ -13,10 +13,10 @@ The verifier drives `io.github.stozo04.openloop/.MainActivity` on an emulator li ## Workflow -1. Read `docs/OPERATING_INSTRUCTIONS.md`, `docs/OPENLOOP_INSTRUCTIONS.md`, the matching `.cursor/skills/verify-openloop/features/.md`, and the relevant product code/strings. Use `.cursor/skills/verify-openloop/helpers/onboarding_loop.py` as the proven reference. +1. Read `docs/OPERATING_INSTRUCTIONS.md`, `docs/OPENLOOP_INSTRUCTIONS.md`, the matching `.codex/skills/verify-openloop/features/.md`, and the relevant product code/strings. Use `.codex/skills/verify-openloop/helpers/onboarding_loop.py` as the proven reference. 2. Derive observable acceptance criteria from the shipped product: precondition, entry point, exact must-have/must-not-have UI, user actions, persisted/resulting state, and any required non-UI proof such as logcat or a file. -3. Create `.cursor/skills/verify-openloop/helpers/_loop.py` with Python's standard library. Reuse an existing helper when one fits; extract shared code only after two real loops demonstrate material duplication. -4. Update the matching feature recipe with the direct run command. Run `python scripts/sync-harness-skills.py --fix --from cursor`, then `--check`. +3. Create `.codex/skills/verify-openloop/helpers/_loop.py` with Python's standard library. Reuse an existing helper when one fits; extract shared code only after two real loops demonstrate material duplication. +4. Update the matching feature recipe with the direct run command. Run `python scripts/sync-harness-skills.py --fix` (it takes the direction from git and retargets each copy's own paths), then `--check`. 5. Build the current debug APK if needed and run the new loop on a booted emulator. Do not report completion from syntax checks or exit code alone; independently confirm its final PASS marker and evidence artifacts. `scripts/run-verification-loops.py --changed` discovers shipped `*_loop.py` files automatically, so there is no registry or roadmap to update. diff --git a/.codex/skills/harness-sync/SKILL.md b/.codex/skills/harness-sync/SKILL.md index c113f7c..eb53cc3 100644 --- a/.codex/skills/harness-sync/SKILL.md +++ b/.codex/skills/harness-sync/SKILL.md @@ -1,6 +1,6 @@ --- name: harness-sync -description: Check and repair byte-drift between the three LLM harness skill trees — `.claude/skills/`, `.cursor/skills/` and `.codex/skills/` — which the Definition of Done (M5) requires to be byte-identical. Use this skill whenever you edit, add, delete or rename ANYTHING under any of those three directories, and whenever the user says "sync harness", "sync the skills", "/harness-sync", "harness drift", "gate 6d", "M5", "sweep gate 6d is red", "the skill trees have drifted", "propagate this skill to the other LLMs", or asks whether the three harness folders match. Also use it proactively before committing a skill change and when a pre-PR sweep or CI run reports the "Harness skill trees byte-identical" gate as failing — a skills edit that reaches only one harness is the exact failure this catches. +description: Check and repair byte-drift between the three LLM harness skill trees — `.claude/skills/`, `.cursor/skills/` and `.codex/skills/` — which the Definition of Done (M5) requires to be identical apart from each copy's pointer at its own tree. Use this skill whenever you edit, add, delete or rename ANYTHING under any of those three directories, and whenever the user says "sync harness", "sync the skills", "/harness-sync", "harness drift", "gate 6d", "M5", "sweep gate 6d is red", "the skill trees have drifted", "propagate this skill to the other LLMs", or asks whether the three harness folders match. Also use it proactively before committing a skill change and when a pre-PR sweep or CI run reports the "Harness skill trees byte-identical" gate as failing — a skills edit that reaches only one harness is the exact failure this catches. --- # harness-sync — keep the three harness skill trees identical @@ -79,6 +79,20 @@ must NOT be synced: They are Claude-only by design and live outside `skills/`, which is why the script never sees them. +One difference inside the compared files IS allowed, and only one: **a skill's pointer at its own +tree**. A recipe that points at `/skills/verify-openloop/helpers/onboarding_loop.py` names +a different directory in each copy, because otherwise two of the three send their LLM to a path it +cannot read (owner instruction, 2026-08-31). The leading `.claude` / `.cursor` / `.codex` of such a +path is compared as one token, and `--fix` rewrites it for each destination — so you still edit one +tree and propagate. Three rules keep it honest: + +- Each copy must point at **itself**. A `.cursor` copy naming the `.codex` tree is drift, and the + gate says so. +- Only a path that continues past the tree root counts. Prose listing the three directories — the + `## Scope` heading below does it — is a list, not a pointer, and is compared literally. +- Anything outside a harness's `skills/` (`~/.cursor/mcp.json`, `.claude/commands/`) belongs to one + harness for real and stays literal in all three copies. + If you are adding a fourth harness, create `./skills/`, add the name to `HARNESSES` in the script, and add the allowlist entries in `.github/workflows/doc-layout.yml` and `docs/README.md` § Enforcement — in the same PR, per Definition of Done M1. diff --git a/.codex/skills/verify-openloop/SKILL.md b/.codex/skills/verify-openloop/SKILL.md index 246b949..27fe7d9 100644 --- a/.codex/skills/verify-openloop/SKILL.md +++ b/.codex/skills/verify-openloop/SKILL.md @@ -13,9 +13,9 @@ Also present, not this skill's default: the GitHub Pages store site. Ignore it h Sibling skills you must reuse, not copy: -- `.claude/skills/run-e2e/` — full capture → editor → save with logcat scan -- `.claude/skills/run-e2e-pixel-sweep/` — 4-emulator import → save quality gate -- `.claude/skills/reset-storage/` — delete onboarding DataStore only +- `.codex/skills/run-e2e/` — full capture → editor → save with logcat scan +- `.codex/skills/run-e2e-pixel-sweep/` — 4-emulator import → save quality gate +- `.codex/skills/reset-storage/` — delete onboarding DataStore only This skill is the feature map plus a thin `helpers/control.ps1` wrapper. The pixel sweep remains the codec/FGS proof. A feature-map pass that skips a mapped entry point is incomplete. @@ -31,7 +31,7 @@ Isolation: **one emulator**. Parallel AVDs fight over host CPU and codecs and in $env:JAVA_HOME = "C:\Program Files\Android\Android Studio\jbr" $env:VERIFY_RUN_ID = Get-Date -Format "yyyyMMdd_HHmmss" # Start one AVD first (Pixel_6 / Pixel_8 / Pixel_8_API34 are the known ones). -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 launch +pwsh .codex/skills/verify-openloop/helpers/control.ps1 launch ``` Ready when `control.ps1 doctor` prints `ok` and `control.ps1 dump` shows one of: @@ -49,7 +49,7 @@ If launch fails, stop. Do not tap some other OpenLoop build (`com.OpenLoop.app` Read-only. Run before the first drive, after any failed drive, and whenever the instance looks wrong. ```powershell -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 doctor +pwsh .codex/skills/verify-openloop/helpers/control.ps1 doctor ``` Must confirm: serial is an emulator (or an explicitly allowed test device), `io.github.stozo04.openloop` is installed, `versionName`/`versionCode` match this checkout (`1.0.49` / `49` on the branch this skill was written against — re-read `app/build.gradle.kts` if they drifted), and `adb get-state` is `device`. @@ -61,8 +61,8 @@ If doctor fails, relaunch or abort. Never drive an instance you did not start. Harness: `uiauto.ps1` (uiautomator dump → tap by text or content-desc). Screenshots hit a per-session image limit; the dump is the eyes. ```powershell -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 dump -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 tap -Label "Start recording" +pwsh .codex/skills/verify-openloop/helpers/control.ps1 dump +pwsh .codex/skills/verify-openloop/helpers/control.ps1 tap -Label "Start recording" ``` Prefer these handles, in order: @@ -99,10 +99,10 @@ Name artifacts `$VERIFY_EVIDENCE_DIR//`. Kill what this run started. Never `pkill` / `adb shell pkill` by name. ```powershell -pwsh .cursor/skills/verify-openloop/helpers/control.ps1 cleanup +pwsh .codex/skills/verify-openloop/helpers/control.ps1 cleanup ``` -Force-stops `io.github.stozo04.openloop` on **this serial only**. Leaves the APK installed. Does not delete `$VERIFY_EVIDENCE_DIR`. Does not wipe gallery clips unless the recipe says so. Does not reset onboarding unless you ran `.claude/skills/reset-storage/`. +Force-stops `io.github.stozo04.openloop` on **this serial only**. Leaves the APK installed. Does not delete `$VERIFY_EVIDENCE_DIR`. Does not wipe gallery clips unless the recipe says so. Does not reset onboarding unless you ran `.codex/skills/reset-storage/`. After cleanup, confirm the evidence directory still exists and is non-empty if a feature was driven. @@ -112,8 +112,8 @@ Leave the emulator running unless you started it for this run; if you started it `helpers/control.ps1` is the wrapper. Invocation is in Launch / Doctor / Drive / Cleanup above. -It calls `.claude/skills/run-e2e/scripts/uiauto.ps1` for dump/tap. Do not reimplement dump parsing. +It calls `.codex/skills/run-e2e/scripts/uiauto.ps1` for dump/tap. Do not reimplement dump parsing. Onboarding repeatable loop (adb + Python 3 stdlib, no Gradle): `python scripts/run-verification-loops.py --changed`. Recipe: `features/onboarding.md`. Windows: `python` or `py -3`, not Git Bash `python3`. -For the full editor-tab + logcat report, run `.claude/skills/run-e2e/SKILL.md` and keep that report under `docs/e2e/`. That satisfies **edit-and-save** when you also store the dumps in `$VERIFY_EVIDENCE_DIR/edit-and-save/`. Single-tab claims use `features/edit-trim.md`, `edit-speed.md`, `edit-loop.md`, `edit-filter.md`, `edit-delete.md`, or `edit-save.md`. +For the full editor-tab + logcat report, run `.codex/skills/run-e2e/SKILL.md` and keep that report under `docs/e2e/`. That satisfies **edit-and-save** when you also store the dumps in `$VERIFY_EVIDENCE_DIR/edit-and-save/`. Single-tab claims use `features/edit-trim.md`, `edit-speed.md`, `edit-loop.md`, `edit-filter.md`, `edit-delete.md`, or `edit-save.md`. diff --git a/.codex/skills/verify-openloop/features/INVENTORY.md b/.codex/skills/verify-openloop/features/INVENTORY.md index 5b2d351..41b38a9 100644 --- a/.codex/skills/verify-openloop/features/INVENTORY.md +++ b/.codex/skills/verify-openloop/features/INVENTORY.md @@ -5,28 +5,32 @@ Does **not** need to stay perfectly current between audits — the [README compl **PRDs are optional.** Prefer `strings.xml` + UI chrome. Add a PRD column only when a PRD exists. -| Surface (user-visible) | Source hint | Status | Feature file | -| ------------------------------------ | ---------------------------- | ------------------ | -------------------------------------------------- | -| Onboarding `LET'S GO!` | strings / OnboardingScreen | automated | [onboarding.md](./onboarding.md) | -| Record / stop video | `Start recording` | mapped | [record-clip.md](./record-clip.md) | -| Lenses drawer + catalogue | `Lenses and Photo Booth` | mapped | [lenses.md](./lenses.md) | -| Multi-face (1–2) lens | FaceRoster / live preview | folded into lenses | [lenses.md](./lenses.md) | -| Photo stills mode | `Camera` / `Take photo` | mapped | [photo-capture.md](./photo-capture.md) | -| Photo booth | booth tab / countdown | mapped | [photo-booth.md](./photo-booth.md) | -| Pinch zoom | zoom chip | mapped | [pinch-zoom.md](./pinch-zoom.md) | -| Import from Photos | `Import a video` | mapped | [import-video.md](./import-video.md) | -| Gallery browse / play | Gallery | mapped | [gallery.md](./gallery.md) | -| Gallery SEND | `SEND` | folded into share | [share-and-library.md](./share-and-library.md) | -| Save to Photos + share sheet | `Saved to Photos` | mapped | [share-and-library.md](./share-and-library.md) | -| Trim | Trim screen / tab | mapped | [edit-trim.md](./edit-trim.md) | -| Speed constant + curve | Speed tab | mapped | [edit-speed.md](./edit-speed.md) | -| Loop direction | Loop tab | mapped | [edit-loop.md](./edit-loop.md) | -| Filter / Looks | Filter tab | mapped | [edit-filter.md](./edit-filter.md) | -| Delete in-progress clip | toolbar Delete | mapped | [edit-delete.md](./edit-delete.md) | -| Save boomerang | `Save boomerang` | mapped | [edit-save.md](./edit-save.md) | -| Full edit → save path | run-e2e | orchestrator | [edit-and-save.md](./edit-and-save.md) | -| Permission rationale / denied | MainActivity strings | out of scope | support chrome — drive only when blocking a recipe | -| In-app review / Play update snackbar | review/update packages | out of scope | system/Play prompts, not core verify map | -| Debug report share | reverse-failed / save-failed | out of scope | support path | +| Surface (user-visible) | Source hint | Status | Feature file | +| ------------------------------------ | ---------------------------- | -------------------- | -------------------------------------------------- | +| Onboarding `LET'S GO!` | strings / OnboardingScreen | automated 2026-08-31 | [onboarding.md](./onboarding.md) | +| Record / stop video | `Start recording` | automated 2026-08-31 | [record-clip.md](./record-clip.md) | +| Lenses drawer + catalogue | `Lenses and Photo Booth` | mapped | [lenses.md](./lenses.md) | +| Multi-face (1–2) lens | FaceRoster / live preview | folded into lenses | [lenses.md](./lenses.md) | +| Photo stills mode | `Camera` / `Take photo` | mapped | [photo-capture.md](./photo-capture.md) | +| Photo booth | booth tab / countdown | mapped | [photo-booth.md](./photo-booth.md) | +| Pinch zoom | zoom chip | mapped | [pinch-zoom.md](./pinch-zoom.md) | +| Import from Photos | `Import a video` | mapped | [import-video.md](./import-video.md) | +| Gallery browse / play | Gallery | mapped | [gallery.md](./gallery.md) | +| Gallery SEND | `SEND` | folded into share | [share-and-library.md](./share-and-library.md) | +| Save to Photos + share sheet | `Saved to Photos` | mapped | [share-and-library.md](./share-and-library.md) | +| Trim | Trim screen / tab | mapped | [edit-trim.md](./edit-trim.md) | +| Speed constant + curve | Speed tab | mapped | [edit-speed.md](./edit-speed.md) | +| Loop direction | Loop tab | mapped | [edit-loop.md](./edit-loop.md) | +| Filter / Looks | Filter tab | mapped | [edit-filter.md](./edit-filter.md) | +| Delete in-progress clip | toolbar Delete | mapped | [edit-delete.md](./edit-delete.md) | +| Save boomerang | `Save boomerang` | mapped | [edit-save.md](./edit-save.md) | +| Full edit → save path | run-e2e | orchestrator | [edit-and-save.md](./edit-and-save.md) | +| Permission rationale / denied | MainActivity strings | out of scope | support chrome — drive only when blocking a recipe | +| In-app review / Play update snackbar | review/update packages | out of scope | system/Play prompts, not core verify map | +| Debug report share | reverse-failed / save-failed | out of scope | support path | When you find a new shipped control in `strings.xml` or chrome that is not listed, add a row as `missing` until a feature file exists. + +`automated ` means a `*_loop.py` verifier drives that surface on an emulator and was seen +passing on that date — not that the recipe merely mentions one. `mapped` is a written recipe with +no autonomous check behind it yet. diff --git a/.codex/skills/verify-openloop/features/README.md b/.codex/skills/verify-openloop/features/README.md index f914b61..6fc9043 100644 --- a/.codex/skills/verify-openloop/features/README.md +++ b/.codex/skills/verify-openloop/features/README.md @@ -6,7 +6,7 @@ This directory is the maintained source for verifying the user-facing behavior o - One emulator attached. Serial in `VERIFY_SERIAL` if more than one `adb devices` line is `device`. - Debug APK from **this** checkout installed (`io.github.stozo04.openloop`). -- `pwsh .cursor/skills/verify-openloop/helpers/control.ps1 doctor` passes. +- `pwsh .codex/skills/verify-openloop/helpers/control.ps1 doctor` passes. - Camera permission granted (`control.ps1 grant-camera` or install `-g`). - Never drive a physical phone unless `VERIFY_ALLOW_DEVICE=1`. - Never drive `com.OpenLoop.app`. diff --git a/.codex/skills/verify-openloop/features/record-clip.md b/.codex/skills/verify-openloop/features/record-clip.md index ca58213..9442a05 100644 --- a/.codex/skills/verify-openloop/features/record-clip.md +++ b/.codex/skills/verify-openloop/features/record-clip.md @@ -16,6 +16,31 @@ From the camera viewfinder the user taps the shutter to start a video (up to 30 - Confirm capture mode is **Video** (`capture_mode_selector`, label `Video`) not Camera/stills and not Photo Booth. - Tap the large shutter at the bottom center. +## Autonomous check + +Run it from the repository root: + +```powershell +python .codex/skills/verify-openloop/helpers/record_clip_loop.py +``` + +`python scripts/run-verification-loops.py --changed` runs it alongside every other loop. Both take +`VERIFY_SERIAL` when more than one emulator is online and `VERIFY_EVIDENCE_DIR` for the artifacts. + +It installs the current debug APK, grants CAMERA, and drives all three outcomes of a shutter tap: +a sub-400 ms double tap (stays on camera, `That was quick!` snackbar, no clip), a few seconds +(Trim opens, scratch clip on disk, `Capture finalized (Nms)` in logcat), and a recording left to +run out (the 30 s cap finalizes it with no stop tap). Every countdown sample is asserted as +`s / 30s` — never a `mm:ss` clock. Both clips are thrown away through the Discard dialog, +so nothing is saved and the gallery is untouched. + +Expect roughly **75 seconds** on a healthy Pixel_8_API34 AVD, where the cap produces a 31.2 s clip. +The elapsed counter accumulates 33 ms per tick instead of reading a clock, so a device that cannot +hold the cadence overruns: the same AVD under host memory pressure took 219 s and produced a +**143 s** clip for the same 30 s cap. The cap is therefore asserted as "no stop tap, no error +finalize, Trim opened, clip ≥ 25 s" — never a wall-clock ceiling, and never how high the chip had +climbed at the last dump, which measures the polling interval rather than the product. + ## Driving it with control.ps1 Preconditions: @@ -38,4 +63,7 @@ Preconditions: - Import (`Import a video` / gallery empty `…or import one`) is a different entry into Trim — see [import-video](./import-video.md). Do not count import as `record-clip`. - Active [lenses](./lenses.md) bake into the recording; prove lens UI separately if that is the claim. - Permission rationale can sit on top of the shutter. Dump first. +- The too-short snackbar covers the shutter row: while it is up, `Start recording` / `Flip Camera` are not in the dump at all. Prove "still on camera" with the mode selector, then re-check the shutter once it clears. +- A uiautomator dump takes seconds and the snackbar lasts four, so polling with dumps loses that race. Wait on logcat (`Video burst recording failed` / `below the 400ms minimum`), then dump once. +- Under host memory pressure the AVD's own system ANRs (`System UI isn't responding`, package `android`, in the dump) and every recipe here fails at the first step. That is the host, not the app — free memory and cold-boot the AVD (`-no-snapshot-load`); `adb reboot` restores the same broken state. - Front/back flip (`Flip Camera`) does not by itself prove a recording. diff --git a/.codex/skills/verify-openloop/helpers/onboarding_loop.py b/.codex/skills/verify-openloop/helpers/onboarding_loop.py index 1339865..ce80eaf 100755 --- a/.codex/skills/verify-openloop/helpers/onboarding_loop.py +++ b/.codex/skills/verify-openloop/helpers/onboarding_loop.py @@ -1,23 +1,52 @@ #!/usr/bin/env python3 +"""Autonomous verifier for `features/onboarding.md` — first run through `LET'S GO!`. + +First run shows the onboarding pitch; tapping the CTA writes the DataStore flag and lands on the +camera; a relaunch goes straight to the viewfinder on the back lens with no onboarding and no +permission rationale. + + python .codex/skills/verify-openloop/helpers/onboarding_loop.py + + VERIFY_SERIAL=emulator-5556 pick a device when more than one is online + VERIFY_EVIDENCE_DIR= where the XML/PNG/logcat evidence lands + +The adb/uiautomator plumbing lives in `verify_common.py`, shared with the other loops here. +""" from __future__ import annotations -import html -import os import re -import shutil import subprocess import sys -import tempfile -import time -import xml.etree.ElementTree as ET -from dataclasses import dataclass -from datetime import datetime from pathlib import Path -PACKAGE = "io.github.stozo04.openloop" -ACTIVITY = f"{PACKAGE}/.MainActivity" +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from verify_common import ( # noqa: E402 + PACKAGE, + adb_out, + assert_absent, + assert_contains, + clear_logcat, + dump_blob, + dump_strings, + dump_ui, + ensure_installed, + ensure_serial_allowed, + evidence_dir, + fail, + find_exact, + force_stop, + grant_camera, + require_online, + resolve_serial, + run_adb, + save_screencap, + start_activity, + tap_node, + wait_until, +) + DATASTORE_REL = "files/datastore/openloop_preferences.preferences_pb" -APK_REL = "app/build/outputs/apk/debug/app-debug.apk" ONBOARDING_MUST_HAVE = [ "Free. Forever.", @@ -32,314 +61,22 @@ FACING_FRONT_RE = re.compile(r"Camera bound \(lens=front\)") -@dataclass(frozen=True) -class UiNode: - text: str - desc: str - bounds: tuple[int, int, int, int] | None - - -def repo_root() -> Path: - return Path(__file__).resolve().parent.parent.parent.parent.parent - - -def run_adb(serial: str, *args: str, check: bool = True) -> subprocess.CompletedProcess[str]: - cmd = ["adb", "-s", serial, *args] - return subprocess.run( - cmd, - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - check=check, - ) - - -def adb_out(serial: str, *args: str, check: bool = True) -> str: - result = run_adb(serial, *args, check=check) - return (result.stdout or "") + (result.stderr or "") - - -def resolve_serial() -> str: - if not shutil.which("adb"): - fail("adb not found on PATH") - env_serial = os.environ.get("VERIFY_SERIAL", "").strip() - if env_serial: - return env_serial - - devices_out = subprocess.run( - ["adb", "devices"], - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - check=True, - ).stdout - emulators = [ - line.split()[0] - for line in devices_out.splitlines() - if re.match(r"emulator-\d+\s+device$", line) - ] - if len(emulators) == 1: - return emulators[0] - if len(emulators) > 1: - fail(f"multiple emulators: {', '.join(emulators)}; set VERIFY_SERIAL") - - physical = [ - line.split()[0] - for line in devices_out.splitlines() - if re.search(r"\s+device$", line) and not line.startswith("emulator-") - ] - if physical: - if os.environ.get("VERIFY_ALLOW_DEVICE") == "1" and env_serial: - return env_serial - fail( - f"physical device {physical[0]} attached; start an emulator or set " - "VERIFY_ALLOW_DEVICE=1 and VERIFY_SERIAL" - ) - fail("no emulator or device (adb devices)") - - -def ensure_serial_allowed(serial: str) -> None: - if serial.startswith("emulator-"): - return - if os.environ.get("VERIFY_ALLOW_DEVICE") == "1": - return - fail(f"serial {serial} is not an emulator; set VERIFY_ALLOW_DEVICE=1 for a test phone") - - -def evidence_dir() -> Path: - base = os.environ.get("VERIFY_EVIDENCE_DIR") - if base: - path = Path(base) - else: - stamp = datetime.now().strftime("%Y%m%d_%H%M%S") - path = Path(tempfile.gettempdir()) / "openloop-verify" / stamp / "onboarding" - path.mkdir(parents=True, exist_ok=True) - return path - - -def decode_entities(value: str) -> str: - if not value: - return value - return html.unescape(value) - - -def parse_bounds(raw: str) -> tuple[int, int, int, int] | None: - match = re.match(r"\[(\d+),(\d+)\]\[(\d+),(\d+)\]", raw) - if not match: - return None - return tuple(int(g) for g in match.groups()) # type: ignore[return-value] - - -def parse_nodes_regex(xml_text: str) -> list[UiNode]: - nodes: list[UiNode] = [] - for match in re.finditer(r"]*>", xml_text): - fragment = match.group(0) - text_m = re.search(r'text="([^"]*)"', fragment) - desc_m = re.search(r'content-desc="([^"]*)"', fragment) - bounds_m = re.search(r'bounds="(\[[^\]]+\]\[[^\]]+\])"', fragment) - bounds = parse_bounds(bounds_m.group(1)) if bounds_m else None - nodes.append( - UiNode( - text=decode_entities(text_m.group(1) if text_m else ""), - desc=decode_entities(desc_m.group(1) if desc_m else ""), - bounds=bounds, - ) - ) - return nodes - - -def parse_nodes_etree(xml_text: str) -> list[UiNode]: - nodes: list[UiNode] = [] - root = ET.fromstring(xml_text) - for elem in root.iter("node"): - bounds = parse_bounds(elem.attrib.get("bounds", "")) - nodes.append( - UiNode( - text=decode_entities(elem.attrib.get("text", "")), - desc=decode_entities(elem.attrib.get("content-desc", "")), - bounds=bounds, - ) - ) - return nodes - - -def parse_nodes(xml_text: str) -> list[UiNode]: - try: - return parse_nodes_etree(xml_text) - except ET.ParseError: - return parse_nodes_regex(xml_text) - - -def dump_ui(serial: str) -> tuple[str, list[UiNode]]: - dump_result = run_adb(serial, "shell", "uiautomator", "dump", "/sdcard/ui.xml", check=False) - if dump_result.returncode != 0: - return "", [] - result = run_adb(serial, "shell", "cat", "/sdcard/ui.xml", check=False) - xml_text = (result.stdout or "").strip() - marker = " set[str]: - values: set[str] = set() - for node in nodes: - if node.text: - values.add(node.text) - if node.desc: - values.add(node.desc) - return values - - -def dump_blob(nodes: list[UiNode]) -> str: - parts: list[str] = [] - for node in nodes: - if node.text: - parts.append(node.text) - if node.desc: - parts.append(node.desc) - return "\n".join(parts) - - -def find_exact(nodes: list[UiNode], label: str) -> UiNode | None: - for node in nodes: - if not node.bounds: - continue - if node.text == label or node.desc == label: - return node - return None - - -def tap_node(serial: str, node: UiNode) -> None: - if not node.bounds: - fail("no bounds for tap target") - x1, y1, x2, y2 = node.bounds - cx = (x1 + x2) // 2 - cy = (y1 + y2) // 2 - run_adb(serial, "shell", "input", "tap", str(cx), str(cy)) - - -def save_screencap(serial: str, path: Path) -> None: - proc = subprocess.run( - ["adb", "-s", serial, "exec-out", "screencap", "-p"], - capture_output=True, - check=True, - ) - path.write_bytes(proc.stdout) - - -def package_installed(serial: str) -> bool: - out = adb_out(serial, "shell", "pm", "path", PACKAGE, check=False) - return "package:" in out - - -def ensure_installed(serial: str) -> None: - apk = repo_root() / APK_REL - if not apk.is_file(): - if package_installed(serial): - return - fail(f"{PACKAGE} not installed and debug APK missing at {apk}") - install = subprocess.run( - ["adb", "-s", serial, "install", "-r", "-g", str(apk)], - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - ) - combined = (install.stdout or "") + (install.stderr or "") - if install.returncode != 0: - fail(f"adb install failed: {combined.strip()}") - if not package_installed(serial): - fail(f"adb install reported success but {PACKAGE} is still missing") - - -def grant_camera(serial: str) -> None: - run_adb(serial, "shell", "pm", "grant", PACKAGE, "android.permission.CAMERA") - - -def force_stop(serial: str) -> None: - run_adb(serial, "shell", "am", "force-stop", PACKAGE) - - def reset_onboarding_store(serial: str) -> None: force_stop(serial) - run_adb( - serial, - "shell", - "run-as", - PACKAGE, - "rm", - "-f", - DATASTORE_REL, - check=False, - ) + run_adb(serial, "shell", "run-as", PACKAGE, "rm", "-f", DATASTORE_REL, check=False) def datastore_exists(serial: str) -> bool: - out = adb_out( - serial, - "shell", - "run-as", - PACKAGE, - "ls", - DATASTORE_REL, - check=False, - ) + out = adb_out(serial, "shell", "run-as", PACKAGE, "ls", DATASTORE_REL, check=False) return DATASTORE_REL.split("/")[-1] in out and "No such file" not in out -def clear_logcat(serial: str) -> None: - run_adb(serial, "logcat", "-c", check=False) - - -def start_activity(serial: str) -> None: - run_adb(serial, "shell", "am", "start", "-n", ACTIVITY) - - -def wait_until( - predicate, - timeout_s: float, - interval_s: float = 0.5, -) -> bool: - deadline = time.monotonic() + timeout_s - while time.monotonic() < deadline: - if predicate(): - return True - time.sleep(interval_s) - return False - - -def assert_contains(strings: set[str], blob: str, required: list[str], context: str) -> None: - missing = [item for item in required if item not in strings and item not in blob] - if missing: - fail(f"{context}: missing {missing[0]!r} (also checked: {missing[1:]})") - - -def assert_absent(strings: set[str], blob: str, forbidden: list[str], context: str) -> None: - for item in forbidden: - if item in strings or item in blob: - fail(f"{context}: must not contain {item!r}") - - -def fail(message: str) -> None: - print(f"FAIL {message}") - sys.exit(1) - - def main() -> int: serial = resolve_serial() ensure_serial_allowed(serial) + require_online(serial) - state = adb_out(serial, "get-state").strip() - if state != "device": - fail(f"serial={serial} get-state={state!r}") - - evidence = evidence_dir() + evidence = evidence_dir("onboarding") ensure_installed(serial) grant_camera(serial) reset_onboarding_store(serial) @@ -348,7 +85,7 @@ def main() -> int: start_activity(serial) first_xml = "" - first_nodes: list[UiNode] = [] + first_nodes: list = [] def poll_first_run() -> bool: nonlocal first_xml, first_nodes @@ -410,7 +147,7 @@ def poll_after_cta() -> bool: start_activity(serial) returning_xml = "" - returning_nodes: list[UiNode] = [] + returning_nodes: list = [] def poll_returning() -> bool: nonlocal returning_xml, returning_nodes diff --git a/.codex/skills/verify-openloop/helpers/record_clip_loop.py b/.codex/skills/verify-openloop/helpers/record_clip_loop.py new file mode 100644 index 0000000..0081849 --- /dev/null +++ b/.codex/skills/verify-openloop/helpers/record_clip_loop.py @@ -0,0 +1,447 @@ +#!/usr/bin/env python3 +"""Autonomous verifier for `features/record-clip.md` — record / stop video. + +Drives the installed debug APK on an emulator through the three outcomes a shutter tap can have, +and pins the countdown chip's format in every one of them: + + 1. too-short — start+stop inside the 400 ms minimum: stays on camera, snackbar + "That was quick! Record a little longer to make a loop.", no clip. + 2. mid-length — a few seconds: Trim opens and a scratch clip exists on disk. + 3. cap — never tap stop: recording finalizes itself at the 30 s cap and Trim opens. + +The countdown chip is asserted on every sample as `s / 30s` and never as a `mm:ss` clock +— issue #154 shipped it reading `00:00`, and a format regression is invisible to a Compose test +that only checks the chip exists. Both clips are discarded through the product's own Discard +dialog, so the run leaves no media behind (saving is `edit-save.md`, not this feature). + + python .codex/skills/verify-openloop/helpers/record_clip_loop.py + + VERIFY_SERIAL=emulator-5556 pick a device when more than one is online + VERIFY_EVIDENCE_DIR= where the XML/PNG/logcat evidence lands + +Runtime is dominated by scenario 3 and depends on how well the device holds a 33 ms tick: the +counter accumulates ticks instead of reading a clock. A healthy Pixel_8_API34 AVD runs the whole +loop in ~75 s and the cap yields a 31.2 s clip; the same AVD memory-starved took 219 s and +produced a 143 s clip for the same cap. Scenario 3 therefore asserts the cap through what does +not vary with that — no stop tap, no error finalize, Trim opens, and the clip comes back at least +25 s — never a wall-clock ceiling and never how high the chip had climbed at the last dump. See +the docstring on `scenario_cap`. +""" +from __future__ import annotations + +import re +import sys +import time +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from verify_common import ( # noqa: E402 + PACKAGE, + adb_out, + app_nodes, + assert_absent, + assert_contains, + center, + clear_logcat, + dump_blob, + dump_strings, + dump_ui, + ensure_installed, + ensure_serial_allowed, + evidence_dir, + fail, + find_exact, + force_stop, + grant_camera, + require_online, + resolve_serial, + run_adb, + save_screencap, + start_activity, + tap_node, + wait_until, +) + +CAMERA_IDLE_MUST_HAVE = ["Start recording", "Gallery", "Flip Camera"] +TRIM_MUST_HAVE = ["TRIM YOUR VIDEO", "SAVE"] +TOO_SHORT_SNACKBAR = "That was quick! Record a little longer to make a loop." + +# The shipped chip is `"${elapsed / 1_000}s / $capLabel"` (CameraScreen.kt) — whole seconds both +# sides. CLOCK_FORMAT_RE is the regression it replaced: a zero-padded mm:ss clock, which for a +# sub-minute cap sat at "00:00" for the first second of every recording (issue #154). +CHIP_RE = re.compile(r"^(\d+)s / (\d+)s$") +CLOCK_FORMAT_RE = re.compile(r"\b\d{1,2}:\d{2}\b") + +FINALIZED_RE = re.compile(r"Capture finalized \((\d+)ms\)") +RECORDING_FAILED_RE = re.compile(r"Video burst recording failed") +TOO_SHORT_LOG_RE = re.compile(r"Capture finalized below the \d+ms minimum|Video burst recording failed") +SCRATCH_REL = "files/scratch" + +# MIN_TRIM_DURATION / MAX_RECORDING in OpenLoopViewModel. A clip under the minimum is discarded +# with the snackbar instead of opening a Trim screen whose SAVE would be dead. +MIN_CLIP_MS = 400 +CAP_SECONDS = 30 +# A recording left to run out must come back as most of the cap. Floor, not equality: the counter +# accumulates ticks rather than reading a clock, so a device that cannot hold the cadence overruns +# (143 s measured on a memory-starved AVD) while a healthy one lands just past 30 s. Both are the +# cap firing; a clip that stopped at 8 s is not. +CAP_MIN_CLIP_MS = 25_000 + + +class Chip: + """Every countdown-chip value seen across the whole run, asserted as it is collected.""" + + def __init__(self) -> None: + self.samples: list[int] = [] + + def observe(self, nodes: list, context: str) -> int | None: + # App-drawn nodes only: the system status bar's own clock reads "8:52" and would trip the + # mm:ss check on every single dump. + strings = dump_strings(app_nodes(nodes)) + for value in strings: + match = CHIP_RE.match(value) + if not match: + continue + if int(match.group(2)) != CAP_SECONDS: + fail(f"{context}: countdown chip reads {value!r}; the cap side should be {CAP_SECONDS}s") + seconds = int(match.group(1)) + if seconds > CAP_SECONDS: + fail(f"{context}: countdown chip reads {value!r}, past the {CAP_SECONDS}s cap") + self.samples.append(seconds) + return seconds + # No chip in this dump is only an error while recording, which the callers decide. A + # mm:ss clock drawn by the app is the issue #154 regression and is never allowed. + offender = next((v for v in strings if CLOCK_FORMAT_RE.search(v)), None) + if offender: + fail(f"{context}: countdown shows a mm:ss clock {offender!r}; it must read 's / 30s'") + return None + + +def snapshot(serial: str, evidence: Path, name: str, xml: str, screenshot: bool = True) -> Path: + path = evidence / f"{name}.xml" + path.write_text(xml, encoding="utf-8") + if screenshot: + save_screencap(serial, evidence / f"{name}.png") + return path + + +def scratch_clips(serial: str) -> set[str]: + out = adb_out(serial, "shell", "run-as", PACKAGE, "ls", SCRATCH_REL, check=False) + return {line.strip() for line in out.splitlines() if line.strip().startswith("raw_")} + + +class Scratch: + """Per-capture clip files under `filesDir/scratch`, measured against what was already there. + + Only clips THIS run created are asserted on. A scratch file left by an abandoned session + elsewhere is not this feature's to judge — and deleting it to get a clean slate would throw + away someone else's in-progress capture, which the verifier contract forbids. + """ + + def __init__(self, serial: str) -> None: + self.baseline = scratch_clips(serial) + + def new(self, serial: str) -> list[str]: + return sorted(scratch_clips(serial) - self.baseline) + + +def wait_for_camera_idle(serial: str, evidence: Path, context: str, timeout_s: float = 45.0) -> tuple[str, list]: + """Poll until the viewfinder is idle, tapping through onboarding or stills mode if needed. + + Deliberately does NOT reset the onboarding DataStore: this feature does not own that state, + and wiping it would make the loop destructive to a device someone else is mid-test on. + """ + seen: dict = {"xml": "", "nodes": []} + + def ready() -> bool: + xml, nodes = dump_ui(serial) + if not nodes: + return False + seen["xml"], seen["nodes"] = xml, nodes + strings, blob = dump_strings(nodes), dump_blob(nodes) + if "Start recording" in strings: + return True + cta = find_exact(nodes, "LET'S GO!") + if cta: # first run on a fresh install — walk through it like a user + tap_node(serial, cta) + return False + if "Take photo" in strings: # stills mode left over from another recipe + video = find_exact(nodes, "Video") + if video: + tap_node(serial, video) + return False + + if not wait_until(ready, timeout_s=timeout_s, interval_s=1.0): + path = snapshot(serial, evidence, f"{context}-not-idle", seen["xml"]) + fail(f"{context}: camera never reached idle (no 'Start recording'); evidence={path}") + strings, blob = dump_strings(seen["nodes"]), dump_blob(seen["nodes"]) + assert_contains(strings, blob, CAMERA_IDLE_MUST_HAVE, context) + assert_absent(strings, blob, ["TRIM YOUR VIDEO"], context) + return seen["xml"], seen["nodes"] + + +def shutter(serial: str, nodes: list, label: str, context: str, evidence: Path): + node = find_exact(nodes, label) + if not node: + fail(f"{context}: no tap target for {label!r}; evidence={evidence}") + return node + + +def wait_for_trim(serial: str, chip: Chip, context: str, timeout_s: float, interval_s: float = 2.0) -> str: + """Poll for the Trim screen, sampling the countdown chip on every dump along the way.""" + seen = {"xml": ""} + + def arrived() -> bool: + xml, nodes = dump_ui(serial) + if not nodes: + return False + seen["xml"] = xml + strings, blob = dump_strings(nodes), dump_blob(nodes) + chip.observe(nodes, context) + return "TRIM YOUR VIDEO" in strings or "TRIM YOUR VIDEO" in blob + + ok = wait_until(arrived, timeout_s=timeout_s, interval_s=interval_s) + return seen["xml"] if ok else "" + + +def discard_clip(serial: str, evidence: Path, context: str, scratch: Scratch) -> None: + """Throw the session clip away through the product's Discard dialog, back to the camera.""" + xml, nodes = dump_ui(serial) + opener = find_exact(nodes, "Discard clip") or find_exact(nodes, "Delete") + if not opener: + path = snapshot(serial, evidence, f"{context}-no-discard", xml) + fail(f"{context}: editor has no 'Discard clip'/'Delete' affordance; evidence={path}") + tap_node(serial, opener) + + seen = {"xml": "", "nodes": []} + + def dialog_up() -> bool: + seen["xml"], seen["nodes"] = dump_ui(serial) + return find_exact(seen["nodes"], "Discard") is not None and any( + "Discard this clip?" in v for v in dump_strings(seen["nodes"]) + ) + + if not wait_until(dialog_up, timeout_s=20.0, interval_s=1.0): + path = snapshot(serial, evidence, f"{context}-no-dialog", seen["xml"]) + fail(f"{context}: 'Discard this clip?' dialog never appeared; evidence={path}") + tap_node(serial, find_exact(seen["nodes"], "Discard")) + + if not wait_until(lambda: "Start recording" in dump_strings(dump_ui(serial)[1]), + timeout_s=45.0, interval_s=1.0): + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, f"{context}-discard-stuck", xml) + fail(f"{context}: discard did not return to the camera; evidence={path}") + left = scratch.new(serial) + if left: + fail(f"{context}: discard left the scratch clip behind: {left}") + + +def scenario_too_short(serial: str, evidence: Path, chip: Chip, scratch: Scratch) -> str: + """Start and stop inside the minimum: no clip, no Trim, and the 'record longer' snackbar. + + Both taps go out in ONE `adb shell`, because a dump-parse-tap round trip is seconds long and + would always produce a valid clip instead of the too-short case. Establishing that precondition + is still timing-dependent, so a run that accidentally records a real clip is retried (and its + clip discarded) rather than asserted against. + """ + for attempt in range(1, 4): + _, nodes = wait_for_camera_idle(serial, evidence, "too-short") + cx, cy = center(shutter(serial, nodes, "Start recording", "too-short", evidence)) + clear_logcat(serial) + run_adb(serial, "shell", f"input tap {cx} {cy}; input tap {cx} {cy}") + + # The snackbar is on screen for four seconds and a uiautomator dump takes about five, so + # polling with dumps loses the race: the first snapshot lands before the capture has even + # finalized and the second lands after the snackbar is gone. Logcat costs a fraction of a + # second and says exactly when the capture ended, so wait on THAT and dump once, into the + # open window. + outcome = {"logcat": ""} + + def finalized() -> bool: + outcome["logcat"] = adb_out(serial, "logcat", "-d", "-s", "OpenLoopViewModel:*") + return bool(TOO_SHORT_LOG_RE.search(outcome["logcat"])) or bool( + FINALIZED_RE.search(outcome["logcat"]) + ) + + if not wait_until(finalized, timeout_s=30.0, interval_s=0.3): + fail("too-short: the double tap produced no capture at all within 30s") + logcat = outcome["logcat"] + seen: dict = {"xml": "", "nodes": []} + seen["xml"], seen["nodes"] = dump_ui(serial) + strings, blob = dump_strings(seen["nodes"]), dump_blob(seen["nodes"]) + + if (FINALIZED_RE.search(logcat) and not TOO_SHORT_LOG_RE.search(logcat)) or "TRIM YOUR VIDEO" in strings: + # The two taps landed far enough apart to encode a real clip: the precondition failed, + # not the product. Clean up and try again. + print(f" too-short attempt {attempt}: taps produced a full clip, discarding and retrying") + discard_clip(serial, evidence, f"too-short-retry{attempt}", scratch) + continue + + snapshot(serial, evidence, "too-short", seen["xml"]) + (evidence / "too-short-logcat.txt").write_text(logcat, encoding="utf-8") + if TOO_SHORT_SNACKBAR not in strings and attempt < 3: + # The capture was rejected as it should be, but the dump landed outside the snackbar's + # four seconds. That is this harness losing a race, not the product; the last attempt + # asserts for real rather than retrying forever. + print(f" too-short attempt {attempt}: dump missed the snackbar window, retrying") + continue + chip.observe(seen["nodes"], "too-short") + # While the snackbar is up it sits over the shutter row, and those controls drop out of the + # hierarchy — so the "still on camera" proof here is the mode selector plus the absence of + # any editor, and the shutter itself is checked once the snackbar has gone. + assert_contains(strings, blob, [TOO_SHORT_SNACKBAR, "Video", "Camera"], "too-short") + assert_absent(strings, blob, ["TRIM YOUR VIDEO", "Stop recording"], "too-short") + left = scratch.new(serial) + if left: + fail(f"too-short: a clip was kept anyway: {left}") + wait_for_camera_idle(serial, evidence, "too-short-after") + return f"attempt{attempt}" + + fail("too-short: three double-taps in a row recorded a real clip; the shutter never saw a sub-400ms press") + + +def scenario_mid_length(serial: str, evidence: Path, chip: Chip, scratch: Scratch) -> int: + """A few seconds of recording: the chip counts in seconds, stop opens Trim, a clip exists.""" + _, nodes = wait_for_camera_idle(serial, evidence, "mid") + clear_logcat(serial) + tap_node(serial, shutter(serial, nodes, "Start recording", "mid", evidence)) + + seen = {"xml": "", "nodes": []} + + def recording() -> bool: + seen["xml"], seen["nodes"] = dump_ui(serial) + strings, blob = dump_strings(seen["nodes"]), dump_blob(seen["nodes"]) + if "Stop recording" not in strings: + return False + seconds = chip.observe(seen["nodes"], "mid-recording") + return seconds is not None and seconds >= 1 + + if not wait_until(recording, timeout_s=90.0, interval_s=1.0): + path = snapshot(serial, evidence, "mid-recording-stalled", seen["xml"]) + fail(f"mid: countdown never reached 1s while recording; evidence={path}") + snapshot(serial, evidence, "mid-recording", seen["xml"]) + + tap_node(serial, shutter(serial, seen["nodes"], "Stop recording", "mid", evidence)) + trim_xml = wait_for_trim(serial, chip, "mid-stop", timeout_s=90.0) + if not trim_xml: + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, "mid-no-trim", xml) + fail(f"mid: stop did not open Trim; evidence={path}") + snapshot(serial, evidence, "mid-trim", trim_xml) + + _, trim_nodes = dump_ui(serial) + strings, blob = dump_strings(trim_nodes), dump_blob(trim_nodes) + assert_contains(strings, blob, TRIM_MUST_HAVE, "mid-trim") + + logcat = adb_out(serial, "logcat", "-d", "-s", "OpenLoopViewModel:*") + (evidence / "mid-logcat.txt").write_text(logcat, encoding="utf-8") + match = FINALIZED_RE.search(logcat) + if not match: + fail(f"mid: no 'Capture finalized (Nms)' in logcat; evidence={evidence / 'mid-logcat.txt'}") + duration_ms = int(match.group(1)) + if duration_ms < MIN_CLIP_MS: + fail(f"mid: clip finalized at {duration_ms}ms, under the {MIN_CLIP_MS}ms minimum") + clips = scratch.new(serial) + if not clips: + fail(f"mid: Trim is open but no scratch clip exists under {SCRATCH_REL}") + + discard_clip(serial, evidence, "mid", scratch) + return duration_ms + + +def scenario_cap(serial: str, evidence: Path, chip: Chip, scratch: Scratch) -> tuple[int, int]: + """Never tap stop: the 30 s cap finalizes the recording on its own and Trim opens. + + Four things make that the cap and not something else: no stop tap is ever sent, Trim arrives + anyway, logcat has no error finalize, and the clip that comes back is at least + [CAP_MIN_CLIP_MS] — a recording cut short by anything else would not be. + + Deliberately NOT asserted: how high the chip had climbed when the last dump landed. That + number measures the sampling interval, not the product — a dump costs seconds, so the faster + the device runs the counter, the further from the cap the final sample sits (17s on a + cold-booted AVD, 29s on a loaded one). Nor is a wall-clock ceiling asserted: the counter + accumulates 33 ms per tick instead of reading a clock, so a device that cannot hold the + cadence overruns the cap in real seconds (143 s measured on a memory-starved AVD). Both are + the cap firing. The chip is still asserted on every sample here — for its *format*, which is + what issue #154 broke. + """ + _, nodes = wait_for_camera_idle(serial, evidence, "cap") + clear_logcat(serial) + tap_node(serial, shutter(serial, nodes, "Start recording", "cap", evidence)) + + if not wait_until(lambda: "Stop recording" in dump_strings(dump_ui(serial)[1]), + timeout_s=60.0, interval_s=1.0): + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, "cap-never-started", xml) + fail(f"cap: recording never started; evidence={path}") + + before = len(chip.samples) + # 3 s between dumps: a dump is seconds of work on the device's main thread, and hammering it + # starves the very tick loop under test — but sample too rarely and the countdown's format + # goes unchecked for most of the recording. + trim_xml = wait_for_trim(serial, chip, "cap-recording", timeout_s=480.0, interval_s=3.0) + if not trim_xml: + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, "cap-no-trim", xml) + fail(f"cap: recording did not finalize itself within 480s; evidence={path}") + snapshot(serial, evidence, "cap-trim", trim_xml) + + samples = chip.samples[before:] + if not samples: + fail("cap: the countdown chip never rendered during a full-length recording") + peak = max(samples) + + logcat = adb_out(serial, "logcat", "-d", "-s", "OpenLoopViewModel:*") + (evidence / "cap-logcat.txt").write_text(logcat, encoding="utf-8") + if RECORDING_FAILED_RE.search(logcat): + fail(f"cap: recording ended in an error, not the cap; evidence={evidence / 'cap-logcat.txt'}") + match = FINALIZED_RE.search(logcat) + if not match: + fail(f"cap: no 'Capture finalized (Nms)' in logcat; evidence={evidence / 'cap-logcat.txt'}") + duration_ms = int(match.group(1)) + if duration_ms < CAP_MIN_CLIP_MS: + fail( + f"cap: the recording stopped itself at {duration_ms}ms, short of the {CAP_SECONDS}s cap " + f"— something other than the cap ended it; evidence={evidence / 'cap-logcat.txt'}" + ) + + discard_clip(serial, evidence, "cap", scratch) + return peak, duration_ms + + +def main() -> int: + serial = resolve_serial() + ensure_serial_allowed(serial) + require_online(serial) + + evidence = evidence_dir("record-clip") + ensure_installed(serial) + grant_camera(serial) + force_stop(serial) + clear_logcat(serial) + start_activity(serial) + + chip = Chip() + scratch = Scratch(serial) + started = time.monotonic() + short = scenario_too_short(serial, evidence, chip, scratch) + mid_ms = scenario_mid_length(serial, evidence, chip, scratch) + peak, cap_ms = scenario_cap(serial, evidence, chip, scratch) + + force_stop(serial) + print( + f"PASS serial={serial} too-short={short} mid={mid_ms}ms cap={cap_ms}ms cap-chip-peak={peak}s " + f"chip-samples={len(chip.samples)} took={int(time.monotonic() - started)}s evidence={evidence}" + ) + return 0 + + +if __name__ == "__main__": + import subprocess + + try: + raise SystemExit(main()) + except subprocess.CalledProcessError as exc: + cmd = " ".join(exc.cmd if isinstance(exc.cmd, list) else [str(exc.cmd)]) + fail(f"adb command failed ({cmd}): {(exc.stderr or exc.stdout or '').strip()}") diff --git a/.codex/skills/verify-openloop/helpers/verify_common.py b/.codex/skills/verify-openloop/helpers/verify_common.py new file mode 100644 index 0000000..3bd9ce8 --- /dev/null +++ b/.codex/skills/verify-openloop/helpers/verify_common.py @@ -0,0 +1,325 @@ +#!/usr/bin/env python3 +"""Shared adb/uiautomator plumbing for the `*_loop.py` verifiers in this directory. + +Extracted when the second real loop (`record_clip_loop.py`) landed and duplicated ~200 lines of +`onboarding_loop.py` verbatim — the point the create-verifier skill names for extracting, and not +before. Nothing feature-specific lives here: a loop owns its own strings, assertions and evidence. + +Not a loop itself. `scripts/run-verification-loops.py` discovers `*_loop.py`, so this file is +never executed as a verifier; the loops import it as a sibling module. +""" +from __future__ import annotations + +import html +import os +import re +import shutil +import subprocess +import sys +import tempfile +import xml.etree.ElementTree as ET +from dataclasses import dataclass +from datetime import datetime +from pathlib import Path + +PACKAGE = "io.github.stozo04.openloop" +ACTIVITY = f"{PACKAGE}/.MainActivity" +APK_REL = "app/build/outputs/apk/debug/app-debug.apk" + + +@dataclass(frozen=True) +class UiNode: + text: str + desc: str + bounds: tuple[int, int, int, int] | None + # Owning package. A dump is the whole screen, so the system status bar (its clock above all) + # and the navigation bar come back alongside the app — filter on this before asserting that + # some text is or is not on screen "in the app". + pkg: str = "" + + +def fail(message: str) -> None: + print(f"FAIL {message}") + sys.exit(1) + + +def repo_root() -> Path: + return Path(__file__).resolve().parent.parent.parent.parent.parent + + +def run_adb(serial: str, *args: str, check: bool = True) -> subprocess.CompletedProcess[str]: + cmd = ["adb", "-s", serial, *args] + return subprocess.run( + cmd, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + check=check, + ) + + +def adb_out(serial: str, *args: str, check: bool = True) -> str: + result = run_adb(serial, *args, check=check) + return (result.stdout or "") + (result.stderr or "") + + +def resolve_serial() -> str: + if not shutil.which("adb"): + fail("adb not found on PATH") + env_serial = os.environ.get("VERIFY_SERIAL", "").strip() + if env_serial: + return env_serial + + devices_out = subprocess.run( + ["adb", "devices"], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + check=True, + ).stdout + emulators = [ + line.split()[0] + for line in devices_out.splitlines() + if re.match(r"emulator-\d+\s+device$", line) + ] + if len(emulators) == 1: + return emulators[0] + if len(emulators) > 1: + fail(f"multiple emulators: {', '.join(emulators)}; set VERIFY_SERIAL") + + physical = [ + line.split()[0] + for line in devices_out.splitlines() + if re.search(r"\s+device$", line) and not line.startswith("emulator-") + ] + if physical: + if os.environ.get("VERIFY_ALLOW_DEVICE") == "1" and env_serial: + return env_serial + fail( + f"physical device {physical[0]} attached; start an emulator or set " + "VERIFY_ALLOW_DEVICE=1 and VERIFY_SERIAL" + ) + fail("no emulator or device (adb devices)") + + +def ensure_serial_allowed(serial: str) -> None: + if serial.startswith("emulator-"): + return + if os.environ.get("VERIFY_ALLOW_DEVICE") == "1": + return + fail(f"serial {serial} is not an emulator; set VERIFY_ALLOW_DEVICE=1 for a test phone") + + +def require_online(serial: str) -> None: + state = adb_out(serial, "get-state").strip() + if state != "device": + fail(f"serial={serial} get-state={state!r}") + + +def evidence_dir(feature: str) -> Path: + base = os.environ.get("VERIFY_EVIDENCE_DIR") + if base: + path = Path(base) + else: + stamp = datetime.now().strftime("%Y%m%d_%H%M%S") + path = Path(tempfile.gettempdir()) / "openloop-verify" / stamp / feature + path.mkdir(parents=True, exist_ok=True) + return path + + +def decode_entities(value: str) -> str: + if not value: + return value + return html.unescape(value) + + +def parse_bounds(raw: str) -> tuple[int, int, int, int] | None: + match = re.match(r"\[(\d+),(\d+)\]\[(\d+),(\d+)\]", raw) + if not match: + return None + return tuple(int(g) for g in match.groups()) # type: ignore[return-value] + + +def parse_nodes_regex(xml_text: str) -> list[UiNode]: + nodes: list[UiNode] = [] + for match in re.finditer(r"]*>", xml_text): + fragment = match.group(0) + text_m = re.search(r'text="([^"]*)"', fragment) + desc_m = re.search(r'content-desc="([^"]*)"', fragment) + bounds_m = re.search(r'bounds="(\[[^\]]+\]\[[^\]]+\])"', fragment) + pkg_m = re.search(r'package="([^"]*)"', fragment) + bounds = parse_bounds(bounds_m.group(1)) if bounds_m else None + nodes.append( + UiNode( + text=decode_entities(text_m.group(1) if text_m else ""), + desc=decode_entities(desc_m.group(1) if desc_m else ""), + bounds=bounds, + pkg=pkg_m.group(1) if pkg_m else "", + ) + ) + return nodes + + +def parse_nodes_etree(xml_text: str) -> list[UiNode]: + nodes: list[UiNode] = [] + root = ET.fromstring(xml_text) + for elem in root.iter("node"): + bounds = parse_bounds(elem.attrib.get("bounds", "")) + nodes.append( + UiNode( + text=decode_entities(elem.attrib.get("text", "")), + desc=decode_entities(elem.attrib.get("content-desc", "")), + bounds=bounds, + pkg=elem.attrib.get("package", ""), + ) + ) + return nodes + + +def parse_nodes(xml_text: str) -> list[UiNode]: + try: + return parse_nodes_etree(xml_text) + except ET.ParseError: + return parse_nodes_regex(xml_text) + + +def dump_ui(serial: str) -> tuple[str, list[UiNode]]: + """Current hierarchy, or ("", []) when uiautomator could not produce one. + + The dump file is only read when THIS dump wrote it: uiautomator leaves the previous XML in + place when it fails ("ERROR: null root node returned by UiTestAutomationBridge", seen on a + busy camera preview), and reading that back would assert against a screen that is gone. + """ + dump_result = run_adb(serial, "shell", "uiautomator", "dump", "/sdcard/ui.xml", check=False) + out = (dump_result.stdout or "") + (dump_result.stderr or "") + if dump_result.returncode != 0 or "dumped to" not in out: + return "", [] + result = run_adb(serial, "shell", "cat", "/sdcard/ui.xml", check=False) + xml_text = (result.stdout or "").strip() + marker = " list[UiNode]: + """Only the nodes OpenLoop drew — no system status bar, no navigation bar.""" + return [node for node in nodes if node.pkg == PACKAGE] + + +def dump_strings(nodes: list[UiNode]) -> set[str]: + values: set[str] = set() + for node in nodes: + if node.text: + values.add(node.text) + if node.desc: + values.add(node.desc) + return values + + +def dump_blob(nodes: list[UiNode]) -> str: + parts: list[str] = [] + for node in nodes: + if node.text: + parts.append(node.text) + if node.desc: + parts.append(node.desc) + return "\n".join(parts) + + +def find_exact(nodes: list[UiNode], label: str) -> UiNode | None: + for node in nodes: + if not node.bounds: + continue + if node.text == label or node.desc == label: + return node + return None + + +def center(node: UiNode) -> tuple[int, int]: + if not node.bounds: + fail("no bounds for tap target") + x1, y1, x2, y2 = node.bounds + return (x1 + x2) // 2, (y1 + y2) // 2 + + +def tap_node(serial: str, node: UiNode) -> None: + cx, cy = center(node) + run_adb(serial, "shell", "input", "tap", str(cx), str(cy)) + + +def save_screencap(serial: str, path: Path) -> None: + proc = subprocess.run( + ["adb", "-s", serial, "exec-out", "screencap", "-p"], + capture_output=True, + check=True, + ) + path.write_bytes(proc.stdout) + + +def package_installed(serial: str) -> bool: + out = adb_out(serial, "shell", "pm", "path", PACKAGE, check=False) + return "package:" in out + + +def ensure_installed(serial: str) -> None: + apk = repo_root() / APK_REL + if not apk.is_file(): + if package_installed(serial): + return + fail(f"{PACKAGE} not installed and debug APK missing at {apk}") + install = subprocess.run( + ["adb", "-s", serial, "install", "-r", "-g", str(apk)], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + combined = (install.stdout or "") + (install.stderr or "") + if install.returncode != 0: + fail(f"adb install failed: {combined.strip()}") + if not package_installed(serial): + fail(f"adb install reported success but {PACKAGE} is still missing") + + +def grant_camera(serial: str) -> None: + run_adb(serial, "shell", "pm", "grant", PACKAGE, "android.permission.CAMERA") + + +def force_stop(serial: str) -> None: + run_adb(serial, "shell", "am", "force-stop", PACKAGE) + + +def clear_logcat(serial: str) -> None: + run_adb(serial, "logcat", "-c", check=False) + + +def start_activity(serial: str) -> None: + run_adb(serial, "shell", "am", "start", "-n", ACTIVITY) + + +def wait_until(predicate, timeout_s: float, interval_s: float = 0.5) -> bool: + import time + + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(interval_s) + return False + + +def assert_contains(strings: set[str], blob: str, required: list[str], context: str) -> None: + missing = [item for item in required if item not in strings and item not in blob] + if missing: + fail(f"{context}: missing {missing[0]!r} (also checked: {missing[1:]})") + + +def assert_absent(strings: set[str], blob: str, forbidden: list[str], context: str) -> None: + for item in forbidden: + if item in strings or item in blob: + fail(f"{context}: must not contain {item!r}") diff --git a/.cursor/skills/create-verifier/SKILL.md b/.cursor/skills/create-verifier/SKILL.md index b9bda98..d85a7f5 100644 --- a/.cursor/skills/create-verifier/SKILL.md +++ b/.cursor/skills/create-verifier/SKILL.md @@ -16,7 +16,7 @@ The verifier drives `io.github.stozo04.openloop/.MainActivity` on an emulator li 1. Read `docs/OPERATING_INSTRUCTIONS.md`, `docs/OPENLOOP_INSTRUCTIONS.md`, the matching `.cursor/skills/verify-openloop/features/.md`, and the relevant product code/strings. Use `.cursor/skills/verify-openloop/helpers/onboarding_loop.py` as the proven reference. 2. Derive observable acceptance criteria from the shipped product: precondition, entry point, exact must-have/must-not-have UI, user actions, persisted/resulting state, and any required non-UI proof such as logcat or a file. 3. Create `.cursor/skills/verify-openloop/helpers/_loop.py` with Python's standard library. Reuse an existing helper when one fits; extract shared code only after two real loops demonstrate material duplication. -4. Update the matching feature recipe with the direct run command. Run `python scripts/sync-harness-skills.py --fix --from cursor`, then `--check`. +4. Update the matching feature recipe with the direct run command. Run `python scripts/sync-harness-skills.py --fix` (it takes the direction from git and retargets each copy's own paths), then `--check`. 5. Build the current debug APK if needed and run the new loop on a booted emulator. Do not report completion from syntax checks or exit code alone; independently confirm its final PASS marker and evidence artifacts. `scripts/run-verification-loops.py --changed` discovers shipped `*_loop.py` files automatically, so there is no registry or roadmap to update. diff --git a/.cursor/skills/harness-sync/SKILL.md b/.cursor/skills/harness-sync/SKILL.md index c113f7c..eb53cc3 100644 --- a/.cursor/skills/harness-sync/SKILL.md +++ b/.cursor/skills/harness-sync/SKILL.md @@ -1,6 +1,6 @@ --- name: harness-sync -description: Check and repair byte-drift between the three LLM harness skill trees — `.claude/skills/`, `.cursor/skills/` and `.codex/skills/` — which the Definition of Done (M5) requires to be byte-identical. Use this skill whenever you edit, add, delete or rename ANYTHING under any of those three directories, and whenever the user says "sync harness", "sync the skills", "/harness-sync", "harness drift", "gate 6d", "M5", "sweep gate 6d is red", "the skill trees have drifted", "propagate this skill to the other LLMs", or asks whether the three harness folders match. Also use it proactively before committing a skill change and when a pre-PR sweep or CI run reports the "Harness skill trees byte-identical" gate as failing — a skills edit that reaches only one harness is the exact failure this catches. +description: Check and repair byte-drift between the three LLM harness skill trees — `.claude/skills/`, `.cursor/skills/` and `.codex/skills/` — which the Definition of Done (M5) requires to be identical apart from each copy's pointer at its own tree. Use this skill whenever you edit, add, delete or rename ANYTHING under any of those three directories, and whenever the user says "sync harness", "sync the skills", "/harness-sync", "harness drift", "gate 6d", "M5", "sweep gate 6d is red", "the skill trees have drifted", "propagate this skill to the other LLMs", or asks whether the three harness folders match. Also use it proactively before committing a skill change and when a pre-PR sweep or CI run reports the "Harness skill trees byte-identical" gate as failing — a skills edit that reaches only one harness is the exact failure this catches. --- # harness-sync — keep the three harness skill trees identical @@ -79,6 +79,20 @@ must NOT be synced: They are Claude-only by design and live outside `skills/`, which is why the script never sees them. +One difference inside the compared files IS allowed, and only one: **a skill's pointer at its own +tree**. A recipe that points at `/skills/verify-openloop/helpers/onboarding_loop.py` names +a different directory in each copy, because otherwise two of the three send their LLM to a path it +cannot read (owner instruction, 2026-08-31). The leading `.claude` / `.cursor` / `.codex` of such a +path is compared as one token, and `--fix` rewrites it for each destination — so you still edit one +tree and propagate. Three rules keep it honest: + +- Each copy must point at **itself**. A `.cursor` copy naming the `.codex` tree is drift, and the + gate says so. +- Only a path that continues past the tree root counts. Prose listing the three directories — the + `## Scope` heading below does it — is a list, not a pointer, and is compared literally. +- Anything outside a harness's `skills/` (`~/.cursor/mcp.json`, `.claude/commands/`) belongs to one + harness for real and stays literal in all three copies. + If you are adding a fourth harness, create `./skills/`, add the name to `HARNESSES` in the script, and add the allowlist entries in `.github/workflows/doc-layout.yml` and `docs/README.md` § Enforcement — in the same PR, per Definition of Done M1. diff --git a/.cursor/skills/verify-openloop/SKILL.md b/.cursor/skills/verify-openloop/SKILL.md index 246b949..a247b3f 100644 --- a/.cursor/skills/verify-openloop/SKILL.md +++ b/.cursor/skills/verify-openloop/SKILL.md @@ -13,9 +13,9 @@ Also present, not this skill's default: the GitHub Pages store site. Ignore it h Sibling skills you must reuse, not copy: -- `.claude/skills/run-e2e/` — full capture → editor → save with logcat scan -- `.claude/skills/run-e2e-pixel-sweep/` — 4-emulator import → save quality gate -- `.claude/skills/reset-storage/` — delete onboarding DataStore only +- `.cursor/skills/run-e2e/` — full capture → editor → save with logcat scan +- `.cursor/skills/run-e2e-pixel-sweep/` — 4-emulator import → save quality gate +- `.cursor/skills/reset-storage/` — delete onboarding DataStore only This skill is the feature map plus a thin `helpers/control.ps1` wrapper. The pixel sweep remains the codec/FGS proof. A feature-map pass that skips a mapped entry point is incomplete. @@ -102,7 +102,7 @@ Kill what this run started. Never `pkill` / `adb shell pkill` by name. pwsh .cursor/skills/verify-openloop/helpers/control.ps1 cleanup ``` -Force-stops `io.github.stozo04.openloop` on **this serial only**. Leaves the APK installed. Does not delete `$VERIFY_EVIDENCE_DIR`. Does not wipe gallery clips unless the recipe says so. Does not reset onboarding unless you ran `.claude/skills/reset-storage/`. +Force-stops `io.github.stozo04.openloop` on **this serial only**. Leaves the APK installed. Does not delete `$VERIFY_EVIDENCE_DIR`. Does not wipe gallery clips unless the recipe says so. Does not reset onboarding unless you ran `.cursor/skills/reset-storage/`. After cleanup, confirm the evidence directory still exists and is non-empty if a feature was driven. @@ -112,8 +112,8 @@ Leave the emulator running unless you started it for this run; if you started it `helpers/control.ps1` is the wrapper. Invocation is in Launch / Doctor / Drive / Cleanup above. -It calls `.claude/skills/run-e2e/scripts/uiauto.ps1` for dump/tap. Do not reimplement dump parsing. +It calls `.cursor/skills/run-e2e/scripts/uiauto.ps1` for dump/tap. Do not reimplement dump parsing. Onboarding repeatable loop (adb + Python 3 stdlib, no Gradle): `python scripts/run-verification-loops.py --changed`. Recipe: `features/onboarding.md`. Windows: `python` or `py -3`, not Git Bash `python3`. -For the full editor-tab + logcat report, run `.claude/skills/run-e2e/SKILL.md` and keep that report under `docs/e2e/`. That satisfies **edit-and-save** when you also store the dumps in `$VERIFY_EVIDENCE_DIR/edit-and-save/`. Single-tab claims use `features/edit-trim.md`, `edit-speed.md`, `edit-loop.md`, `edit-filter.md`, `edit-delete.md`, or `edit-save.md`. +For the full editor-tab + logcat report, run `.cursor/skills/run-e2e/SKILL.md` and keep that report under `docs/e2e/`. That satisfies **edit-and-save** when you also store the dumps in `$VERIFY_EVIDENCE_DIR/edit-and-save/`. Single-tab claims use `features/edit-trim.md`, `edit-speed.md`, `edit-loop.md`, `edit-filter.md`, `edit-delete.md`, or `edit-save.md`. diff --git a/.cursor/skills/verify-openloop/features/INVENTORY.md b/.cursor/skills/verify-openloop/features/INVENTORY.md index 5b2d351..41b38a9 100644 --- a/.cursor/skills/verify-openloop/features/INVENTORY.md +++ b/.cursor/skills/verify-openloop/features/INVENTORY.md @@ -5,28 +5,32 @@ Does **not** need to stay perfectly current between audits — the [README compl **PRDs are optional.** Prefer `strings.xml` + UI chrome. Add a PRD column only when a PRD exists. -| Surface (user-visible) | Source hint | Status | Feature file | -| ------------------------------------ | ---------------------------- | ------------------ | -------------------------------------------------- | -| Onboarding `LET'S GO!` | strings / OnboardingScreen | automated | [onboarding.md](./onboarding.md) | -| Record / stop video | `Start recording` | mapped | [record-clip.md](./record-clip.md) | -| Lenses drawer + catalogue | `Lenses and Photo Booth` | mapped | [lenses.md](./lenses.md) | -| Multi-face (1–2) lens | FaceRoster / live preview | folded into lenses | [lenses.md](./lenses.md) | -| Photo stills mode | `Camera` / `Take photo` | mapped | [photo-capture.md](./photo-capture.md) | -| Photo booth | booth tab / countdown | mapped | [photo-booth.md](./photo-booth.md) | -| Pinch zoom | zoom chip | mapped | [pinch-zoom.md](./pinch-zoom.md) | -| Import from Photos | `Import a video` | mapped | [import-video.md](./import-video.md) | -| Gallery browse / play | Gallery | mapped | [gallery.md](./gallery.md) | -| Gallery SEND | `SEND` | folded into share | [share-and-library.md](./share-and-library.md) | -| Save to Photos + share sheet | `Saved to Photos` | mapped | [share-and-library.md](./share-and-library.md) | -| Trim | Trim screen / tab | mapped | [edit-trim.md](./edit-trim.md) | -| Speed constant + curve | Speed tab | mapped | [edit-speed.md](./edit-speed.md) | -| Loop direction | Loop tab | mapped | [edit-loop.md](./edit-loop.md) | -| Filter / Looks | Filter tab | mapped | [edit-filter.md](./edit-filter.md) | -| Delete in-progress clip | toolbar Delete | mapped | [edit-delete.md](./edit-delete.md) | -| Save boomerang | `Save boomerang` | mapped | [edit-save.md](./edit-save.md) | -| Full edit → save path | run-e2e | orchestrator | [edit-and-save.md](./edit-and-save.md) | -| Permission rationale / denied | MainActivity strings | out of scope | support chrome — drive only when blocking a recipe | -| In-app review / Play update snackbar | review/update packages | out of scope | system/Play prompts, not core verify map | -| Debug report share | reverse-failed / save-failed | out of scope | support path | +| Surface (user-visible) | Source hint | Status | Feature file | +| ------------------------------------ | ---------------------------- | -------------------- | -------------------------------------------------- | +| Onboarding `LET'S GO!` | strings / OnboardingScreen | automated 2026-08-31 | [onboarding.md](./onboarding.md) | +| Record / stop video | `Start recording` | automated 2026-08-31 | [record-clip.md](./record-clip.md) | +| Lenses drawer + catalogue | `Lenses and Photo Booth` | mapped | [lenses.md](./lenses.md) | +| Multi-face (1–2) lens | FaceRoster / live preview | folded into lenses | [lenses.md](./lenses.md) | +| Photo stills mode | `Camera` / `Take photo` | mapped | [photo-capture.md](./photo-capture.md) | +| Photo booth | booth tab / countdown | mapped | [photo-booth.md](./photo-booth.md) | +| Pinch zoom | zoom chip | mapped | [pinch-zoom.md](./pinch-zoom.md) | +| Import from Photos | `Import a video` | mapped | [import-video.md](./import-video.md) | +| Gallery browse / play | Gallery | mapped | [gallery.md](./gallery.md) | +| Gallery SEND | `SEND` | folded into share | [share-and-library.md](./share-and-library.md) | +| Save to Photos + share sheet | `Saved to Photos` | mapped | [share-and-library.md](./share-and-library.md) | +| Trim | Trim screen / tab | mapped | [edit-trim.md](./edit-trim.md) | +| Speed constant + curve | Speed tab | mapped | [edit-speed.md](./edit-speed.md) | +| Loop direction | Loop tab | mapped | [edit-loop.md](./edit-loop.md) | +| Filter / Looks | Filter tab | mapped | [edit-filter.md](./edit-filter.md) | +| Delete in-progress clip | toolbar Delete | mapped | [edit-delete.md](./edit-delete.md) | +| Save boomerang | `Save boomerang` | mapped | [edit-save.md](./edit-save.md) | +| Full edit → save path | run-e2e | orchestrator | [edit-and-save.md](./edit-and-save.md) | +| Permission rationale / denied | MainActivity strings | out of scope | support chrome — drive only when blocking a recipe | +| In-app review / Play update snackbar | review/update packages | out of scope | system/Play prompts, not core verify map | +| Debug report share | reverse-failed / save-failed | out of scope | support path | When you find a new shipped control in `strings.xml` or chrome that is not listed, add a row as `missing` until a feature file exists. + +`automated ` means a `*_loop.py` verifier drives that surface on an emulator and was seen +passing on that date — not that the recipe merely mentions one. `mapped` is a written recipe with +no autonomous check behind it yet. diff --git a/.cursor/skills/verify-openloop/features/record-clip.md b/.cursor/skills/verify-openloop/features/record-clip.md index ca58213..7a22c78 100644 --- a/.cursor/skills/verify-openloop/features/record-clip.md +++ b/.cursor/skills/verify-openloop/features/record-clip.md @@ -16,6 +16,31 @@ From the camera viewfinder the user taps the shutter to start a video (up to 30 - Confirm capture mode is **Video** (`capture_mode_selector`, label `Video`) not Camera/stills and not Photo Booth. - Tap the large shutter at the bottom center. +## Autonomous check + +Run it from the repository root: + +```powershell +python .cursor/skills/verify-openloop/helpers/record_clip_loop.py +``` + +`python scripts/run-verification-loops.py --changed` runs it alongside every other loop. Both take +`VERIFY_SERIAL` when more than one emulator is online and `VERIFY_EVIDENCE_DIR` for the artifacts. + +It installs the current debug APK, grants CAMERA, and drives all three outcomes of a shutter tap: +a sub-400 ms double tap (stays on camera, `That was quick!` snackbar, no clip), a few seconds +(Trim opens, scratch clip on disk, `Capture finalized (Nms)` in logcat), and a recording left to +run out (the 30 s cap finalizes it with no stop tap). Every countdown sample is asserted as +`s / 30s` — never a `mm:ss` clock. Both clips are thrown away through the Discard dialog, +so nothing is saved and the gallery is untouched. + +Expect roughly **75 seconds** on a healthy Pixel_8_API34 AVD, where the cap produces a 31.2 s clip. +The elapsed counter accumulates 33 ms per tick instead of reading a clock, so a device that cannot +hold the cadence overruns: the same AVD under host memory pressure took 219 s and produced a +**143 s** clip for the same 30 s cap. The cap is therefore asserted as "no stop tap, no error +finalize, Trim opened, clip ≥ 25 s" — never a wall-clock ceiling, and never how high the chip had +climbed at the last dump, which measures the polling interval rather than the product. + ## Driving it with control.ps1 Preconditions: @@ -38,4 +63,7 @@ Preconditions: - Import (`Import a video` / gallery empty `…or import one`) is a different entry into Trim — see [import-video](./import-video.md). Do not count import as `record-clip`. - Active [lenses](./lenses.md) bake into the recording; prove lens UI separately if that is the claim. - Permission rationale can sit on top of the shutter. Dump first. +- The too-short snackbar covers the shutter row: while it is up, `Start recording` / `Flip Camera` are not in the dump at all. Prove "still on camera" with the mode selector, then re-check the shutter once it clears. +- A uiautomator dump takes seconds and the snackbar lasts four, so polling with dumps loses that race. Wait on logcat (`Video burst recording failed` / `below the 400ms minimum`), then dump once. +- Under host memory pressure the AVD's own system ANRs (`System UI isn't responding`, package `android`, in the dump) and every recipe here fails at the first step. That is the host, not the app — free memory and cold-boot the AVD (`-no-snapshot-load`); `adb reboot` restores the same broken state. - Front/back flip (`Flip Camera`) does not by itself prove a recording. diff --git a/.cursor/skills/verify-openloop/helpers/onboarding_loop.py b/.cursor/skills/verify-openloop/helpers/onboarding_loop.py index 1339865..597c13c 100755 --- a/.cursor/skills/verify-openloop/helpers/onboarding_loop.py +++ b/.cursor/skills/verify-openloop/helpers/onboarding_loop.py @@ -1,23 +1,52 @@ #!/usr/bin/env python3 +"""Autonomous verifier for `features/onboarding.md` — first run through `LET'S GO!`. + +First run shows the onboarding pitch; tapping the CTA writes the DataStore flag and lands on the +camera; a relaunch goes straight to the viewfinder on the back lens with no onboarding and no +permission rationale. + + python .cursor/skills/verify-openloop/helpers/onboarding_loop.py + + VERIFY_SERIAL=emulator-5556 pick a device when more than one is online + VERIFY_EVIDENCE_DIR= where the XML/PNG/logcat evidence lands + +The adb/uiautomator plumbing lives in `verify_common.py`, shared with the other loops here. +""" from __future__ import annotations -import html -import os import re -import shutil import subprocess import sys -import tempfile -import time -import xml.etree.ElementTree as ET -from dataclasses import dataclass -from datetime import datetime from pathlib import Path -PACKAGE = "io.github.stozo04.openloop" -ACTIVITY = f"{PACKAGE}/.MainActivity" +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from verify_common import ( # noqa: E402 + PACKAGE, + adb_out, + assert_absent, + assert_contains, + clear_logcat, + dump_blob, + dump_strings, + dump_ui, + ensure_installed, + ensure_serial_allowed, + evidence_dir, + fail, + find_exact, + force_stop, + grant_camera, + require_online, + resolve_serial, + run_adb, + save_screencap, + start_activity, + tap_node, + wait_until, +) + DATASTORE_REL = "files/datastore/openloop_preferences.preferences_pb" -APK_REL = "app/build/outputs/apk/debug/app-debug.apk" ONBOARDING_MUST_HAVE = [ "Free. Forever.", @@ -32,314 +61,22 @@ FACING_FRONT_RE = re.compile(r"Camera bound \(lens=front\)") -@dataclass(frozen=True) -class UiNode: - text: str - desc: str - bounds: tuple[int, int, int, int] | None - - -def repo_root() -> Path: - return Path(__file__).resolve().parent.parent.parent.parent.parent - - -def run_adb(serial: str, *args: str, check: bool = True) -> subprocess.CompletedProcess[str]: - cmd = ["adb", "-s", serial, *args] - return subprocess.run( - cmd, - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - check=check, - ) - - -def adb_out(serial: str, *args: str, check: bool = True) -> str: - result = run_adb(serial, *args, check=check) - return (result.stdout or "") + (result.stderr or "") - - -def resolve_serial() -> str: - if not shutil.which("adb"): - fail("adb not found on PATH") - env_serial = os.environ.get("VERIFY_SERIAL", "").strip() - if env_serial: - return env_serial - - devices_out = subprocess.run( - ["adb", "devices"], - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - check=True, - ).stdout - emulators = [ - line.split()[0] - for line in devices_out.splitlines() - if re.match(r"emulator-\d+\s+device$", line) - ] - if len(emulators) == 1: - return emulators[0] - if len(emulators) > 1: - fail(f"multiple emulators: {', '.join(emulators)}; set VERIFY_SERIAL") - - physical = [ - line.split()[0] - for line in devices_out.splitlines() - if re.search(r"\s+device$", line) and not line.startswith("emulator-") - ] - if physical: - if os.environ.get("VERIFY_ALLOW_DEVICE") == "1" and env_serial: - return env_serial - fail( - f"physical device {physical[0]} attached; start an emulator or set " - "VERIFY_ALLOW_DEVICE=1 and VERIFY_SERIAL" - ) - fail("no emulator or device (adb devices)") - - -def ensure_serial_allowed(serial: str) -> None: - if serial.startswith("emulator-"): - return - if os.environ.get("VERIFY_ALLOW_DEVICE") == "1": - return - fail(f"serial {serial} is not an emulator; set VERIFY_ALLOW_DEVICE=1 for a test phone") - - -def evidence_dir() -> Path: - base = os.environ.get("VERIFY_EVIDENCE_DIR") - if base: - path = Path(base) - else: - stamp = datetime.now().strftime("%Y%m%d_%H%M%S") - path = Path(tempfile.gettempdir()) / "openloop-verify" / stamp / "onboarding" - path.mkdir(parents=True, exist_ok=True) - return path - - -def decode_entities(value: str) -> str: - if not value: - return value - return html.unescape(value) - - -def parse_bounds(raw: str) -> tuple[int, int, int, int] | None: - match = re.match(r"\[(\d+),(\d+)\]\[(\d+),(\d+)\]", raw) - if not match: - return None - return tuple(int(g) for g in match.groups()) # type: ignore[return-value] - - -def parse_nodes_regex(xml_text: str) -> list[UiNode]: - nodes: list[UiNode] = [] - for match in re.finditer(r"]*>", xml_text): - fragment = match.group(0) - text_m = re.search(r'text="([^"]*)"', fragment) - desc_m = re.search(r'content-desc="([^"]*)"', fragment) - bounds_m = re.search(r'bounds="(\[[^\]]+\]\[[^\]]+\])"', fragment) - bounds = parse_bounds(bounds_m.group(1)) if bounds_m else None - nodes.append( - UiNode( - text=decode_entities(text_m.group(1) if text_m else ""), - desc=decode_entities(desc_m.group(1) if desc_m else ""), - bounds=bounds, - ) - ) - return nodes - - -def parse_nodes_etree(xml_text: str) -> list[UiNode]: - nodes: list[UiNode] = [] - root = ET.fromstring(xml_text) - for elem in root.iter("node"): - bounds = parse_bounds(elem.attrib.get("bounds", "")) - nodes.append( - UiNode( - text=decode_entities(elem.attrib.get("text", "")), - desc=decode_entities(elem.attrib.get("content-desc", "")), - bounds=bounds, - ) - ) - return nodes - - -def parse_nodes(xml_text: str) -> list[UiNode]: - try: - return parse_nodes_etree(xml_text) - except ET.ParseError: - return parse_nodes_regex(xml_text) - - -def dump_ui(serial: str) -> tuple[str, list[UiNode]]: - dump_result = run_adb(serial, "shell", "uiautomator", "dump", "/sdcard/ui.xml", check=False) - if dump_result.returncode != 0: - return "", [] - result = run_adb(serial, "shell", "cat", "/sdcard/ui.xml", check=False) - xml_text = (result.stdout or "").strip() - marker = " set[str]: - values: set[str] = set() - for node in nodes: - if node.text: - values.add(node.text) - if node.desc: - values.add(node.desc) - return values - - -def dump_blob(nodes: list[UiNode]) -> str: - parts: list[str] = [] - for node in nodes: - if node.text: - parts.append(node.text) - if node.desc: - parts.append(node.desc) - return "\n".join(parts) - - -def find_exact(nodes: list[UiNode], label: str) -> UiNode | None: - for node in nodes: - if not node.bounds: - continue - if node.text == label or node.desc == label: - return node - return None - - -def tap_node(serial: str, node: UiNode) -> None: - if not node.bounds: - fail("no bounds for tap target") - x1, y1, x2, y2 = node.bounds - cx = (x1 + x2) // 2 - cy = (y1 + y2) // 2 - run_adb(serial, "shell", "input", "tap", str(cx), str(cy)) - - -def save_screencap(serial: str, path: Path) -> None: - proc = subprocess.run( - ["adb", "-s", serial, "exec-out", "screencap", "-p"], - capture_output=True, - check=True, - ) - path.write_bytes(proc.stdout) - - -def package_installed(serial: str) -> bool: - out = adb_out(serial, "shell", "pm", "path", PACKAGE, check=False) - return "package:" in out - - -def ensure_installed(serial: str) -> None: - apk = repo_root() / APK_REL - if not apk.is_file(): - if package_installed(serial): - return - fail(f"{PACKAGE} not installed and debug APK missing at {apk}") - install = subprocess.run( - ["adb", "-s", serial, "install", "-r", "-g", str(apk)], - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - ) - combined = (install.stdout or "") + (install.stderr or "") - if install.returncode != 0: - fail(f"adb install failed: {combined.strip()}") - if not package_installed(serial): - fail(f"adb install reported success but {PACKAGE} is still missing") - - -def grant_camera(serial: str) -> None: - run_adb(serial, "shell", "pm", "grant", PACKAGE, "android.permission.CAMERA") - - -def force_stop(serial: str) -> None: - run_adb(serial, "shell", "am", "force-stop", PACKAGE) - - def reset_onboarding_store(serial: str) -> None: force_stop(serial) - run_adb( - serial, - "shell", - "run-as", - PACKAGE, - "rm", - "-f", - DATASTORE_REL, - check=False, - ) + run_adb(serial, "shell", "run-as", PACKAGE, "rm", "-f", DATASTORE_REL, check=False) def datastore_exists(serial: str) -> bool: - out = adb_out( - serial, - "shell", - "run-as", - PACKAGE, - "ls", - DATASTORE_REL, - check=False, - ) + out = adb_out(serial, "shell", "run-as", PACKAGE, "ls", DATASTORE_REL, check=False) return DATASTORE_REL.split("/")[-1] in out and "No such file" not in out -def clear_logcat(serial: str) -> None: - run_adb(serial, "logcat", "-c", check=False) - - -def start_activity(serial: str) -> None: - run_adb(serial, "shell", "am", "start", "-n", ACTIVITY) - - -def wait_until( - predicate, - timeout_s: float, - interval_s: float = 0.5, -) -> bool: - deadline = time.monotonic() + timeout_s - while time.monotonic() < deadline: - if predicate(): - return True - time.sleep(interval_s) - return False - - -def assert_contains(strings: set[str], blob: str, required: list[str], context: str) -> None: - missing = [item for item in required if item not in strings and item not in blob] - if missing: - fail(f"{context}: missing {missing[0]!r} (also checked: {missing[1:]})") - - -def assert_absent(strings: set[str], blob: str, forbidden: list[str], context: str) -> None: - for item in forbidden: - if item in strings or item in blob: - fail(f"{context}: must not contain {item!r}") - - -def fail(message: str) -> None: - print(f"FAIL {message}") - sys.exit(1) - - def main() -> int: serial = resolve_serial() ensure_serial_allowed(serial) + require_online(serial) - state = adb_out(serial, "get-state").strip() - if state != "device": - fail(f"serial={serial} get-state={state!r}") - - evidence = evidence_dir() + evidence = evidence_dir("onboarding") ensure_installed(serial) grant_camera(serial) reset_onboarding_store(serial) @@ -348,7 +85,7 @@ def main() -> int: start_activity(serial) first_xml = "" - first_nodes: list[UiNode] = [] + first_nodes: list = [] def poll_first_run() -> bool: nonlocal first_xml, first_nodes @@ -410,7 +147,7 @@ def poll_after_cta() -> bool: start_activity(serial) returning_xml = "" - returning_nodes: list[UiNode] = [] + returning_nodes: list = [] def poll_returning() -> bool: nonlocal returning_xml, returning_nodes diff --git a/.cursor/skills/verify-openloop/helpers/record_clip_loop.py b/.cursor/skills/verify-openloop/helpers/record_clip_loop.py new file mode 100644 index 0000000..0a02c77 --- /dev/null +++ b/.cursor/skills/verify-openloop/helpers/record_clip_loop.py @@ -0,0 +1,447 @@ +#!/usr/bin/env python3 +"""Autonomous verifier for `features/record-clip.md` — record / stop video. + +Drives the installed debug APK on an emulator through the three outcomes a shutter tap can have, +and pins the countdown chip's format in every one of them: + + 1. too-short — start+stop inside the 400 ms minimum: stays on camera, snackbar + "That was quick! Record a little longer to make a loop.", no clip. + 2. mid-length — a few seconds: Trim opens and a scratch clip exists on disk. + 3. cap — never tap stop: recording finalizes itself at the 30 s cap and Trim opens. + +The countdown chip is asserted on every sample as `s / 30s` and never as a `mm:ss` clock +— issue #154 shipped it reading `00:00`, and a format regression is invisible to a Compose test +that only checks the chip exists. Both clips are discarded through the product's own Discard +dialog, so the run leaves no media behind (saving is `edit-save.md`, not this feature). + + python .cursor/skills/verify-openloop/helpers/record_clip_loop.py + + VERIFY_SERIAL=emulator-5556 pick a device when more than one is online + VERIFY_EVIDENCE_DIR= where the XML/PNG/logcat evidence lands + +Runtime is dominated by scenario 3 and depends on how well the device holds a 33 ms tick: the +counter accumulates ticks instead of reading a clock. A healthy Pixel_8_API34 AVD runs the whole +loop in ~75 s and the cap yields a 31.2 s clip; the same AVD memory-starved took 219 s and +produced a 143 s clip for the same cap. Scenario 3 therefore asserts the cap through what does +not vary with that — no stop tap, no error finalize, Trim opens, and the clip comes back at least +25 s — never a wall-clock ceiling and never how high the chip had climbed at the last dump. See +the docstring on `scenario_cap`. +""" +from __future__ import annotations + +import re +import sys +import time +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from verify_common import ( # noqa: E402 + PACKAGE, + adb_out, + app_nodes, + assert_absent, + assert_contains, + center, + clear_logcat, + dump_blob, + dump_strings, + dump_ui, + ensure_installed, + ensure_serial_allowed, + evidence_dir, + fail, + find_exact, + force_stop, + grant_camera, + require_online, + resolve_serial, + run_adb, + save_screencap, + start_activity, + tap_node, + wait_until, +) + +CAMERA_IDLE_MUST_HAVE = ["Start recording", "Gallery", "Flip Camera"] +TRIM_MUST_HAVE = ["TRIM YOUR VIDEO", "SAVE"] +TOO_SHORT_SNACKBAR = "That was quick! Record a little longer to make a loop." + +# The shipped chip is `"${elapsed / 1_000}s / $capLabel"` (CameraScreen.kt) — whole seconds both +# sides. CLOCK_FORMAT_RE is the regression it replaced: a zero-padded mm:ss clock, which for a +# sub-minute cap sat at "00:00" for the first second of every recording (issue #154). +CHIP_RE = re.compile(r"^(\d+)s / (\d+)s$") +CLOCK_FORMAT_RE = re.compile(r"\b\d{1,2}:\d{2}\b") + +FINALIZED_RE = re.compile(r"Capture finalized \((\d+)ms\)") +RECORDING_FAILED_RE = re.compile(r"Video burst recording failed") +TOO_SHORT_LOG_RE = re.compile(r"Capture finalized below the \d+ms minimum|Video burst recording failed") +SCRATCH_REL = "files/scratch" + +# MIN_TRIM_DURATION / MAX_RECORDING in OpenLoopViewModel. A clip under the minimum is discarded +# with the snackbar instead of opening a Trim screen whose SAVE would be dead. +MIN_CLIP_MS = 400 +CAP_SECONDS = 30 +# A recording left to run out must come back as most of the cap. Floor, not equality: the counter +# accumulates ticks rather than reading a clock, so a device that cannot hold the cadence overruns +# (143 s measured on a memory-starved AVD) while a healthy one lands just past 30 s. Both are the +# cap firing; a clip that stopped at 8 s is not. +CAP_MIN_CLIP_MS = 25_000 + + +class Chip: + """Every countdown-chip value seen across the whole run, asserted as it is collected.""" + + def __init__(self) -> None: + self.samples: list[int] = [] + + def observe(self, nodes: list, context: str) -> int | None: + # App-drawn nodes only: the system status bar's own clock reads "8:52" and would trip the + # mm:ss check on every single dump. + strings = dump_strings(app_nodes(nodes)) + for value in strings: + match = CHIP_RE.match(value) + if not match: + continue + if int(match.group(2)) != CAP_SECONDS: + fail(f"{context}: countdown chip reads {value!r}; the cap side should be {CAP_SECONDS}s") + seconds = int(match.group(1)) + if seconds > CAP_SECONDS: + fail(f"{context}: countdown chip reads {value!r}, past the {CAP_SECONDS}s cap") + self.samples.append(seconds) + return seconds + # No chip in this dump is only an error while recording, which the callers decide. A + # mm:ss clock drawn by the app is the issue #154 regression and is never allowed. + offender = next((v for v in strings if CLOCK_FORMAT_RE.search(v)), None) + if offender: + fail(f"{context}: countdown shows a mm:ss clock {offender!r}; it must read 's / 30s'") + return None + + +def snapshot(serial: str, evidence: Path, name: str, xml: str, screenshot: bool = True) -> Path: + path = evidence / f"{name}.xml" + path.write_text(xml, encoding="utf-8") + if screenshot: + save_screencap(serial, evidence / f"{name}.png") + return path + + +def scratch_clips(serial: str) -> set[str]: + out = adb_out(serial, "shell", "run-as", PACKAGE, "ls", SCRATCH_REL, check=False) + return {line.strip() for line in out.splitlines() if line.strip().startswith("raw_")} + + +class Scratch: + """Per-capture clip files under `filesDir/scratch`, measured against what was already there. + + Only clips THIS run created are asserted on. A scratch file left by an abandoned session + elsewhere is not this feature's to judge — and deleting it to get a clean slate would throw + away someone else's in-progress capture, which the verifier contract forbids. + """ + + def __init__(self, serial: str) -> None: + self.baseline = scratch_clips(serial) + + def new(self, serial: str) -> list[str]: + return sorted(scratch_clips(serial) - self.baseline) + + +def wait_for_camera_idle(serial: str, evidence: Path, context: str, timeout_s: float = 45.0) -> tuple[str, list]: + """Poll until the viewfinder is idle, tapping through onboarding or stills mode if needed. + + Deliberately does NOT reset the onboarding DataStore: this feature does not own that state, + and wiping it would make the loop destructive to a device someone else is mid-test on. + """ + seen: dict = {"xml": "", "nodes": []} + + def ready() -> bool: + xml, nodes = dump_ui(serial) + if not nodes: + return False + seen["xml"], seen["nodes"] = xml, nodes + strings, blob = dump_strings(nodes), dump_blob(nodes) + if "Start recording" in strings: + return True + cta = find_exact(nodes, "LET'S GO!") + if cta: # first run on a fresh install — walk through it like a user + tap_node(serial, cta) + return False + if "Take photo" in strings: # stills mode left over from another recipe + video = find_exact(nodes, "Video") + if video: + tap_node(serial, video) + return False + + if not wait_until(ready, timeout_s=timeout_s, interval_s=1.0): + path = snapshot(serial, evidence, f"{context}-not-idle", seen["xml"]) + fail(f"{context}: camera never reached idle (no 'Start recording'); evidence={path}") + strings, blob = dump_strings(seen["nodes"]), dump_blob(seen["nodes"]) + assert_contains(strings, blob, CAMERA_IDLE_MUST_HAVE, context) + assert_absent(strings, blob, ["TRIM YOUR VIDEO"], context) + return seen["xml"], seen["nodes"] + + +def shutter(serial: str, nodes: list, label: str, context: str, evidence: Path): + node = find_exact(nodes, label) + if not node: + fail(f"{context}: no tap target for {label!r}; evidence={evidence}") + return node + + +def wait_for_trim(serial: str, chip: Chip, context: str, timeout_s: float, interval_s: float = 2.0) -> str: + """Poll for the Trim screen, sampling the countdown chip on every dump along the way.""" + seen = {"xml": ""} + + def arrived() -> bool: + xml, nodes = dump_ui(serial) + if not nodes: + return False + seen["xml"] = xml + strings, blob = dump_strings(nodes), dump_blob(nodes) + chip.observe(nodes, context) + return "TRIM YOUR VIDEO" in strings or "TRIM YOUR VIDEO" in blob + + ok = wait_until(arrived, timeout_s=timeout_s, interval_s=interval_s) + return seen["xml"] if ok else "" + + +def discard_clip(serial: str, evidence: Path, context: str, scratch: Scratch) -> None: + """Throw the session clip away through the product's Discard dialog, back to the camera.""" + xml, nodes = dump_ui(serial) + opener = find_exact(nodes, "Discard clip") or find_exact(nodes, "Delete") + if not opener: + path = snapshot(serial, evidence, f"{context}-no-discard", xml) + fail(f"{context}: editor has no 'Discard clip'/'Delete' affordance; evidence={path}") + tap_node(serial, opener) + + seen = {"xml": "", "nodes": []} + + def dialog_up() -> bool: + seen["xml"], seen["nodes"] = dump_ui(serial) + return find_exact(seen["nodes"], "Discard") is not None and any( + "Discard this clip?" in v for v in dump_strings(seen["nodes"]) + ) + + if not wait_until(dialog_up, timeout_s=20.0, interval_s=1.0): + path = snapshot(serial, evidence, f"{context}-no-dialog", seen["xml"]) + fail(f"{context}: 'Discard this clip?' dialog never appeared; evidence={path}") + tap_node(serial, find_exact(seen["nodes"], "Discard")) + + if not wait_until(lambda: "Start recording" in dump_strings(dump_ui(serial)[1]), + timeout_s=45.0, interval_s=1.0): + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, f"{context}-discard-stuck", xml) + fail(f"{context}: discard did not return to the camera; evidence={path}") + left = scratch.new(serial) + if left: + fail(f"{context}: discard left the scratch clip behind: {left}") + + +def scenario_too_short(serial: str, evidence: Path, chip: Chip, scratch: Scratch) -> str: + """Start and stop inside the minimum: no clip, no Trim, and the 'record longer' snackbar. + + Both taps go out in ONE `adb shell`, because a dump-parse-tap round trip is seconds long and + would always produce a valid clip instead of the too-short case. Establishing that precondition + is still timing-dependent, so a run that accidentally records a real clip is retried (and its + clip discarded) rather than asserted against. + """ + for attempt in range(1, 4): + _, nodes = wait_for_camera_idle(serial, evidence, "too-short") + cx, cy = center(shutter(serial, nodes, "Start recording", "too-short", evidence)) + clear_logcat(serial) + run_adb(serial, "shell", f"input tap {cx} {cy}; input tap {cx} {cy}") + + # The snackbar is on screen for four seconds and a uiautomator dump takes about five, so + # polling with dumps loses the race: the first snapshot lands before the capture has even + # finalized and the second lands after the snackbar is gone. Logcat costs a fraction of a + # second and says exactly when the capture ended, so wait on THAT and dump once, into the + # open window. + outcome = {"logcat": ""} + + def finalized() -> bool: + outcome["logcat"] = adb_out(serial, "logcat", "-d", "-s", "OpenLoopViewModel:*") + return bool(TOO_SHORT_LOG_RE.search(outcome["logcat"])) or bool( + FINALIZED_RE.search(outcome["logcat"]) + ) + + if not wait_until(finalized, timeout_s=30.0, interval_s=0.3): + fail("too-short: the double tap produced no capture at all within 30s") + logcat = outcome["logcat"] + seen: dict = {"xml": "", "nodes": []} + seen["xml"], seen["nodes"] = dump_ui(serial) + strings, blob = dump_strings(seen["nodes"]), dump_blob(seen["nodes"]) + + if (FINALIZED_RE.search(logcat) and not TOO_SHORT_LOG_RE.search(logcat)) or "TRIM YOUR VIDEO" in strings: + # The two taps landed far enough apart to encode a real clip: the precondition failed, + # not the product. Clean up and try again. + print(f" too-short attempt {attempt}: taps produced a full clip, discarding and retrying") + discard_clip(serial, evidence, f"too-short-retry{attempt}", scratch) + continue + + snapshot(serial, evidence, "too-short", seen["xml"]) + (evidence / "too-short-logcat.txt").write_text(logcat, encoding="utf-8") + if TOO_SHORT_SNACKBAR not in strings and attempt < 3: + # The capture was rejected as it should be, but the dump landed outside the snackbar's + # four seconds. That is this harness losing a race, not the product; the last attempt + # asserts for real rather than retrying forever. + print(f" too-short attempt {attempt}: dump missed the snackbar window, retrying") + continue + chip.observe(seen["nodes"], "too-short") + # While the snackbar is up it sits over the shutter row, and those controls drop out of the + # hierarchy — so the "still on camera" proof here is the mode selector plus the absence of + # any editor, and the shutter itself is checked once the snackbar has gone. + assert_contains(strings, blob, [TOO_SHORT_SNACKBAR, "Video", "Camera"], "too-short") + assert_absent(strings, blob, ["TRIM YOUR VIDEO", "Stop recording"], "too-short") + left = scratch.new(serial) + if left: + fail(f"too-short: a clip was kept anyway: {left}") + wait_for_camera_idle(serial, evidence, "too-short-after") + return f"attempt{attempt}" + + fail("too-short: three double-taps in a row recorded a real clip; the shutter never saw a sub-400ms press") + + +def scenario_mid_length(serial: str, evidence: Path, chip: Chip, scratch: Scratch) -> int: + """A few seconds of recording: the chip counts in seconds, stop opens Trim, a clip exists.""" + _, nodes = wait_for_camera_idle(serial, evidence, "mid") + clear_logcat(serial) + tap_node(serial, shutter(serial, nodes, "Start recording", "mid", evidence)) + + seen = {"xml": "", "nodes": []} + + def recording() -> bool: + seen["xml"], seen["nodes"] = dump_ui(serial) + strings, blob = dump_strings(seen["nodes"]), dump_blob(seen["nodes"]) + if "Stop recording" not in strings: + return False + seconds = chip.observe(seen["nodes"], "mid-recording") + return seconds is not None and seconds >= 1 + + if not wait_until(recording, timeout_s=90.0, interval_s=1.0): + path = snapshot(serial, evidence, "mid-recording-stalled", seen["xml"]) + fail(f"mid: countdown never reached 1s while recording; evidence={path}") + snapshot(serial, evidence, "mid-recording", seen["xml"]) + + tap_node(serial, shutter(serial, seen["nodes"], "Stop recording", "mid", evidence)) + trim_xml = wait_for_trim(serial, chip, "mid-stop", timeout_s=90.0) + if not trim_xml: + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, "mid-no-trim", xml) + fail(f"mid: stop did not open Trim; evidence={path}") + snapshot(serial, evidence, "mid-trim", trim_xml) + + _, trim_nodes = dump_ui(serial) + strings, blob = dump_strings(trim_nodes), dump_blob(trim_nodes) + assert_contains(strings, blob, TRIM_MUST_HAVE, "mid-trim") + + logcat = adb_out(serial, "logcat", "-d", "-s", "OpenLoopViewModel:*") + (evidence / "mid-logcat.txt").write_text(logcat, encoding="utf-8") + match = FINALIZED_RE.search(logcat) + if not match: + fail(f"mid: no 'Capture finalized (Nms)' in logcat; evidence={evidence / 'mid-logcat.txt'}") + duration_ms = int(match.group(1)) + if duration_ms < MIN_CLIP_MS: + fail(f"mid: clip finalized at {duration_ms}ms, under the {MIN_CLIP_MS}ms minimum") + clips = scratch.new(serial) + if not clips: + fail(f"mid: Trim is open but no scratch clip exists under {SCRATCH_REL}") + + discard_clip(serial, evidence, "mid", scratch) + return duration_ms + + +def scenario_cap(serial: str, evidence: Path, chip: Chip, scratch: Scratch) -> tuple[int, int]: + """Never tap stop: the 30 s cap finalizes the recording on its own and Trim opens. + + Four things make that the cap and not something else: no stop tap is ever sent, Trim arrives + anyway, logcat has no error finalize, and the clip that comes back is at least + [CAP_MIN_CLIP_MS] — a recording cut short by anything else would not be. + + Deliberately NOT asserted: how high the chip had climbed when the last dump landed. That + number measures the sampling interval, not the product — a dump costs seconds, so the faster + the device runs the counter, the further from the cap the final sample sits (17s on a + cold-booted AVD, 29s on a loaded one). Nor is a wall-clock ceiling asserted: the counter + accumulates 33 ms per tick instead of reading a clock, so a device that cannot hold the + cadence overruns the cap in real seconds (143 s measured on a memory-starved AVD). Both are + the cap firing. The chip is still asserted on every sample here — for its *format*, which is + what issue #154 broke. + """ + _, nodes = wait_for_camera_idle(serial, evidence, "cap") + clear_logcat(serial) + tap_node(serial, shutter(serial, nodes, "Start recording", "cap", evidence)) + + if not wait_until(lambda: "Stop recording" in dump_strings(dump_ui(serial)[1]), + timeout_s=60.0, interval_s=1.0): + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, "cap-never-started", xml) + fail(f"cap: recording never started; evidence={path}") + + before = len(chip.samples) + # 3 s between dumps: a dump is seconds of work on the device's main thread, and hammering it + # starves the very tick loop under test — but sample too rarely and the countdown's format + # goes unchecked for most of the recording. + trim_xml = wait_for_trim(serial, chip, "cap-recording", timeout_s=480.0, interval_s=3.0) + if not trim_xml: + xml, _ = dump_ui(serial) + path = snapshot(serial, evidence, "cap-no-trim", xml) + fail(f"cap: recording did not finalize itself within 480s; evidence={path}") + snapshot(serial, evidence, "cap-trim", trim_xml) + + samples = chip.samples[before:] + if not samples: + fail("cap: the countdown chip never rendered during a full-length recording") + peak = max(samples) + + logcat = adb_out(serial, "logcat", "-d", "-s", "OpenLoopViewModel:*") + (evidence / "cap-logcat.txt").write_text(logcat, encoding="utf-8") + if RECORDING_FAILED_RE.search(logcat): + fail(f"cap: recording ended in an error, not the cap; evidence={evidence / 'cap-logcat.txt'}") + match = FINALIZED_RE.search(logcat) + if not match: + fail(f"cap: no 'Capture finalized (Nms)' in logcat; evidence={evidence / 'cap-logcat.txt'}") + duration_ms = int(match.group(1)) + if duration_ms < CAP_MIN_CLIP_MS: + fail( + f"cap: the recording stopped itself at {duration_ms}ms, short of the {CAP_SECONDS}s cap " + f"— something other than the cap ended it; evidence={evidence / 'cap-logcat.txt'}" + ) + + discard_clip(serial, evidence, "cap", scratch) + return peak, duration_ms + + +def main() -> int: + serial = resolve_serial() + ensure_serial_allowed(serial) + require_online(serial) + + evidence = evidence_dir("record-clip") + ensure_installed(serial) + grant_camera(serial) + force_stop(serial) + clear_logcat(serial) + start_activity(serial) + + chip = Chip() + scratch = Scratch(serial) + started = time.monotonic() + short = scenario_too_short(serial, evidence, chip, scratch) + mid_ms = scenario_mid_length(serial, evidence, chip, scratch) + peak, cap_ms = scenario_cap(serial, evidence, chip, scratch) + + force_stop(serial) + print( + f"PASS serial={serial} too-short={short} mid={mid_ms}ms cap={cap_ms}ms cap-chip-peak={peak}s " + f"chip-samples={len(chip.samples)} took={int(time.monotonic() - started)}s evidence={evidence}" + ) + return 0 + + +if __name__ == "__main__": + import subprocess + + try: + raise SystemExit(main()) + except subprocess.CalledProcessError as exc: + cmd = " ".join(exc.cmd if isinstance(exc.cmd, list) else [str(exc.cmd)]) + fail(f"adb command failed ({cmd}): {(exc.stderr or exc.stdout or '').strip()}") diff --git a/.cursor/skills/verify-openloop/helpers/verify_common.py b/.cursor/skills/verify-openloop/helpers/verify_common.py new file mode 100644 index 0000000..3bd9ce8 --- /dev/null +++ b/.cursor/skills/verify-openloop/helpers/verify_common.py @@ -0,0 +1,325 @@ +#!/usr/bin/env python3 +"""Shared adb/uiautomator plumbing for the `*_loop.py` verifiers in this directory. + +Extracted when the second real loop (`record_clip_loop.py`) landed and duplicated ~200 lines of +`onboarding_loop.py` verbatim — the point the create-verifier skill names for extracting, and not +before. Nothing feature-specific lives here: a loop owns its own strings, assertions and evidence. + +Not a loop itself. `scripts/run-verification-loops.py` discovers `*_loop.py`, so this file is +never executed as a verifier; the loops import it as a sibling module. +""" +from __future__ import annotations + +import html +import os +import re +import shutil +import subprocess +import sys +import tempfile +import xml.etree.ElementTree as ET +from dataclasses import dataclass +from datetime import datetime +from pathlib import Path + +PACKAGE = "io.github.stozo04.openloop" +ACTIVITY = f"{PACKAGE}/.MainActivity" +APK_REL = "app/build/outputs/apk/debug/app-debug.apk" + + +@dataclass(frozen=True) +class UiNode: + text: str + desc: str + bounds: tuple[int, int, int, int] | None + # Owning package. A dump is the whole screen, so the system status bar (its clock above all) + # and the navigation bar come back alongside the app — filter on this before asserting that + # some text is or is not on screen "in the app". + pkg: str = "" + + +def fail(message: str) -> None: + print(f"FAIL {message}") + sys.exit(1) + + +def repo_root() -> Path: + return Path(__file__).resolve().parent.parent.parent.parent.parent + + +def run_adb(serial: str, *args: str, check: bool = True) -> subprocess.CompletedProcess[str]: + cmd = ["adb", "-s", serial, *args] + return subprocess.run( + cmd, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + check=check, + ) + + +def adb_out(serial: str, *args: str, check: bool = True) -> str: + result = run_adb(serial, *args, check=check) + return (result.stdout or "") + (result.stderr or "") + + +def resolve_serial() -> str: + if not shutil.which("adb"): + fail("adb not found on PATH") + env_serial = os.environ.get("VERIFY_SERIAL", "").strip() + if env_serial: + return env_serial + + devices_out = subprocess.run( + ["adb", "devices"], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + check=True, + ).stdout + emulators = [ + line.split()[0] + for line in devices_out.splitlines() + if re.match(r"emulator-\d+\s+device$", line) + ] + if len(emulators) == 1: + return emulators[0] + if len(emulators) > 1: + fail(f"multiple emulators: {', '.join(emulators)}; set VERIFY_SERIAL") + + physical = [ + line.split()[0] + for line in devices_out.splitlines() + if re.search(r"\s+device$", line) and not line.startswith("emulator-") + ] + if physical: + if os.environ.get("VERIFY_ALLOW_DEVICE") == "1" and env_serial: + return env_serial + fail( + f"physical device {physical[0]} attached; start an emulator or set " + "VERIFY_ALLOW_DEVICE=1 and VERIFY_SERIAL" + ) + fail("no emulator or device (adb devices)") + + +def ensure_serial_allowed(serial: str) -> None: + if serial.startswith("emulator-"): + return + if os.environ.get("VERIFY_ALLOW_DEVICE") == "1": + return + fail(f"serial {serial} is not an emulator; set VERIFY_ALLOW_DEVICE=1 for a test phone") + + +def require_online(serial: str) -> None: + state = adb_out(serial, "get-state").strip() + if state != "device": + fail(f"serial={serial} get-state={state!r}") + + +def evidence_dir(feature: str) -> Path: + base = os.environ.get("VERIFY_EVIDENCE_DIR") + if base: + path = Path(base) + else: + stamp = datetime.now().strftime("%Y%m%d_%H%M%S") + path = Path(tempfile.gettempdir()) / "openloop-verify" / stamp / feature + path.mkdir(parents=True, exist_ok=True) + return path + + +def decode_entities(value: str) -> str: + if not value: + return value + return html.unescape(value) + + +def parse_bounds(raw: str) -> tuple[int, int, int, int] | None: + match = re.match(r"\[(\d+),(\d+)\]\[(\d+),(\d+)\]", raw) + if not match: + return None + return tuple(int(g) for g in match.groups()) # type: ignore[return-value] + + +def parse_nodes_regex(xml_text: str) -> list[UiNode]: + nodes: list[UiNode] = [] + for match in re.finditer(r"]*>", xml_text): + fragment = match.group(0) + text_m = re.search(r'text="([^"]*)"', fragment) + desc_m = re.search(r'content-desc="([^"]*)"', fragment) + bounds_m = re.search(r'bounds="(\[[^\]]+\]\[[^\]]+\])"', fragment) + pkg_m = re.search(r'package="([^"]*)"', fragment) + bounds = parse_bounds(bounds_m.group(1)) if bounds_m else None + nodes.append( + UiNode( + text=decode_entities(text_m.group(1) if text_m else ""), + desc=decode_entities(desc_m.group(1) if desc_m else ""), + bounds=bounds, + pkg=pkg_m.group(1) if pkg_m else "", + ) + ) + return nodes + + +def parse_nodes_etree(xml_text: str) -> list[UiNode]: + nodes: list[UiNode] = [] + root = ET.fromstring(xml_text) + for elem in root.iter("node"): + bounds = parse_bounds(elem.attrib.get("bounds", "")) + nodes.append( + UiNode( + text=decode_entities(elem.attrib.get("text", "")), + desc=decode_entities(elem.attrib.get("content-desc", "")), + bounds=bounds, + pkg=elem.attrib.get("package", ""), + ) + ) + return nodes + + +def parse_nodes(xml_text: str) -> list[UiNode]: + try: + return parse_nodes_etree(xml_text) + except ET.ParseError: + return parse_nodes_regex(xml_text) + + +def dump_ui(serial: str) -> tuple[str, list[UiNode]]: + """Current hierarchy, or ("", []) when uiautomator could not produce one. + + The dump file is only read when THIS dump wrote it: uiautomator leaves the previous XML in + place when it fails ("ERROR: null root node returned by UiTestAutomationBridge", seen on a + busy camera preview), and reading that back would assert against a screen that is gone. + """ + dump_result = run_adb(serial, "shell", "uiautomator", "dump", "/sdcard/ui.xml", check=False) + out = (dump_result.stdout or "") + (dump_result.stderr or "") + if dump_result.returncode != 0 or "dumped to" not in out: + return "", [] + result = run_adb(serial, "shell", "cat", "/sdcard/ui.xml", check=False) + xml_text = (result.stdout or "").strip() + marker = " list[UiNode]: + """Only the nodes OpenLoop drew — no system status bar, no navigation bar.""" + return [node for node in nodes if node.pkg == PACKAGE] + + +def dump_strings(nodes: list[UiNode]) -> set[str]: + values: set[str] = set() + for node in nodes: + if node.text: + values.add(node.text) + if node.desc: + values.add(node.desc) + return values + + +def dump_blob(nodes: list[UiNode]) -> str: + parts: list[str] = [] + for node in nodes: + if node.text: + parts.append(node.text) + if node.desc: + parts.append(node.desc) + return "\n".join(parts) + + +def find_exact(nodes: list[UiNode], label: str) -> UiNode | None: + for node in nodes: + if not node.bounds: + continue + if node.text == label or node.desc == label: + return node + return None + + +def center(node: UiNode) -> tuple[int, int]: + if not node.bounds: + fail("no bounds for tap target") + x1, y1, x2, y2 = node.bounds + return (x1 + x2) // 2, (y1 + y2) // 2 + + +def tap_node(serial: str, node: UiNode) -> None: + cx, cy = center(node) + run_adb(serial, "shell", "input", "tap", str(cx), str(cy)) + + +def save_screencap(serial: str, path: Path) -> None: + proc = subprocess.run( + ["adb", "-s", serial, "exec-out", "screencap", "-p"], + capture_output=True, + check=True, + ) + path.write_bytes(proc.stdout) + + +def package_installed(serial: str) -> bool: + out = adb_out(serial, "shell", "pm", "path", PACKAGE, check=False) + return "package:" in out + + +def ensure_installed(serial: str) -> None: + apk = repo_root() / APK_REL + if not apk.is_file(): + if package_installed(serial): + return + fail(f"{PACKAGE} not installed and debug APK missing at {apk}") + install = subprocess.run( + ["adb", "-s", serial, "install", "-r", "-g", str(apk)], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + combined = (install.stdout or "") + (install.stderr or "") + if install.returncode != 0: + fail(f"adb install failed: {combined.strip()}") + if not package_installed(serial): + fail(f"adb install reported success but {PACKAGE} is still missing") + + +def grant_camera(serial: str) -> None: + run_adb(serial, "shell", "pm", "grant", PACKAGE, "android.permission.CAMERA") + + +def force_stop(serial: str) -> None: + run_adb(serial, "shell", "am", "force-stop", PACKAGE) + + +def clear_logcat(serial: str) -> None: + run_adb(serial, "logcat", "-c", check=False) + + +def start_activity(serial: str) -> None: + run_adb(serial, "shell", "am", "start", "-n", ACTIVITY) + + +def wait_until(predicate, timeout_s: float, interval_s: float = 0.5) -> bool: + import time + + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(interval_s) + return False + + +def assert_contains(strings: set[str], blob: str, required: list[str], context: str) -> None: + missing = [item for item in required if item not in strings and item not in blob] + if missing: + fail(f"{context}: missing {missing[0]!r} (also checked: {missing[1:]})") + + +def assert_absent(strings: set[str], blob: str, forbidden: list[str], context: str) -> None: + for item in forbidden: + if item in strings or item in blob: + fail(f"{context}: must not contain {item!r}") diff --git a/docs/DEFINITION_OF_DONE.md b/docs/DEFINITION_OF_DONE.md index 43cf29f..b58ad95 100644 --- a/docs/DEFINITION_OF_DONE.md +++ b/docs/DEFINITION_OF_DONE.md @@ -105,7 +105,7 @@ Then fix, in this order: Leave alone: historical records that were true when written (a PRD's "per X I searched…", a lessons-learned entry, a changelog). Those describe the past, not the current layout. -### M5. Harness skill packages are byte-identical across `.claude/`, `.cursor/`, `.codex/` +### M5. Harness skill packages are identical across `.claude/`, `.cursor/`, `.codex/` The same rule as M3, one level down. The owner runs three LLM providers on this repo routinely and each harness auto-discovers skills **only** under its own directory, so the skills cannot live in one @@ -137,6 +137,14 @@ Enforced as sweep gate **6d** and as a CI step in has its own free, offline test — `python scripts/test-sync-harness-skills.py` — run it after changing the script. +- **One difference is allowed: a skill's pointer at its own tree.** A recipe that says "run + `.claude/skills/verify-openloop/helpers/onboarding_loop.py`" has to name a different directory in + each copy, or two of the three send their LLM to a path it cannot read (owner instruction, + 2026-08-31). `.claude/skills`, `.cursor/skills` and `.codex/skills` are therefore compared as one + token and `--fix` rewrites it per destination. Every other byte still has to match, and each copy + must point at **itself** — `.cursor`'s copy naming `.codex/skills` is drift. Paths outside + `/skills` (`~/.cursor/mcp.json`, `.claude/commands/`) are genuinely one harness's and + stay literal in all three. - **Scope is `skills/**` only.** `settings.json` is harness-specific by design (Claude's carries marketplaces, plugins and hooks; the other two a plugin stub) and `.claude/commands/` has no counterpart — none of that is compared. diff --git a/scripts/pre-pr-sweep.ps1 b/scripts/pre-pr-sweep.ps1 index 50000b7..6db24ec 100644 --- a/scripts/pre-pr-sweep.ps1 +++ b/scripts/pre-pr-sweep.ps1 @@ -324,7 +324,10 @@ Gate "5b. Onboarding loop — autonomous first-run + returning-user proof" { if (Test-Path $loopErr) { Get-Content $loopErr | Add-Content $log } $tail = "" if (Test-Path $loopLog) { $tail = (@(Get-Content $loopLog) | Select-Object -Last 1) } - if ($code -eq 0 -and "$tail" -match '^PASS loops=[a-z0-9,-]+$') { return "PASS ($tail)" } + # Underscores are in the class because a loop's name is its `_loop.py` stem, and those + # are Python module names: `record_clip`, not `record-clip`. Written when onboarding was the + # only loop, this pattern rejected the marker the runner actually printed for the second one. + if ($code -eq 0 -and "$tail" -match '^PASS loops=[a-z0-9_,-]+$') { return "PASS ($tail)" } return "FAIL: exit=$code final=$tail — expected a final PASS loops= marker" } diff --git a/scripts/sync-harness-skills.py b/scripts/sync-harness-skills.py index 9fabb4c..a424b43 100644 --- a/scripts/sync-harness-skills.py +++ b/scripts/sync-harness-skills.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Keep the per-harness skill packages byte-identical. +"""Keep the per-harness skill packages identical, modulo each one's own path. The owner drives this repo with three LLM harnesses, and each one auto-discovers skills only under its own directory — `.claude/skills/`, `.cursor/skills/`, `.codex/skills/`. The content is @@ -8,6 +8,15 @@ nothing says which copy is current. Same rule as `docs/DEFINITION_OF_DONE.md` M3 for the shared instruction files, one level down (M5). +ONE exception to "identical", and it exists because the trees are copies: a skill that points at +its own tree — `.claude/skills/verify-openloop/helpers/onboarding_loop.py` — has to name a +different directory in each copy, or two of the three send their LLM to a path it cannot read. +So `.claude/skills`, `.cursor/skills` and `.codex/skills` are compared as one self-reference +token, and `--fix` rewrites that token to the destination harness instead of copying it verbatim. +Every other byte still has to match, and each copy must reference ITSELF: `.cursor`'s copy naming +`.codex/skills` is drift, not a self-reference. Nothing outside `/skills` is normalized — +`~/.cursor/mcp.json` and `.claude/commands/` are genuinely one harness's, and stay literal. + Scope is `skills/**` only. `settings.json` is deliberately NOT compared — it is harness-specific (Claude's carries marketplaces, plugins and hooks; the others a plugin stub) — and `.claude/commands/` has no counterpart in the other two. @@ -23,6 +32,7 @@ """ import argparse import filecmp +import re import shutil import subprocess import sys @@ -32,6 +42,39 @@ HARNESSES = ("claude", "cursor", "codex") SUBTREE = "skills" +# A reference to something INSIDE the harness's own skills tree, in either slash flavor — the +# skills are written on Windows and quote both `.claude/skills/verify-openloop/...` and +# `pwsh \.claude\skills\run-e2e\...`. A deeper path is required so that prose enumerating +# the three trees ("`.claude/skills/`, `.cursor/skills/`, `.codex/skills/`", which harness-sync's +# own SKILL.md does) is left alone: that is a list of all three, not a pointer at one. +SELF_REF = re.compile(r"\.(claude|cursor|codex)([/\\]" + SUBTREE + r"[/\\][A-Za-z0-9_.-])") +SELF_TOKEN = "." + + +def normalized(path, harness): + """File text with a reference to `harness`'s own skills tree collapsed to one token. + + Only that harness's token is replaced, so each copy has to point at ITSELF to compare equal: + `.cursor`'s copy naming `.codex/skills` normalizes to nothing and reads as drift, which is + what it is. None means "not UTF-8 text" — those are compared byte-for-byte instead. + """ + try: + # newline="": no universal-newline translation. Without it a CRLF skill file would come + # back with LF endings and --fix would rewrite every line of it as a "sync". + with path.open(encoding="utf-8", newline="") as fh: + text = fh.read() + except (UnicodeDecodeError, OSError): + return None + return re.sub(rf"\.{harness}([/\\]{SUBTREE}[/\\][A-Za-z0-9_.-])", SELF_TOKEN + r"\1", text) + + +def same_content(a, a_harness, b, b_harness): + """True when two copies differ only in which harness tree they point at.""" + if filecmp.cmp(a, b, shallow=False): # contents, never size + mtime — a checkout rewrites those + return True + na = normalized(a, a_harness) + return na is not None and na == normalized(b, b_harness) + def tracked(harness): """Relative paths under /skills that git can see, mapped to their absolute path. @@ -94,8 +137,7 @@ def drift(): elif rel not in trees[base] or not trees[base][rel].exists() or h == base: state[h] = "same" else: - # shallow=False: compare contents, never size + modification time — a checkout rewrites those. - state[h] = "same" if filecmp.cmp(trees[base][rel], trees[h][rel], shallow=False) else "differs" + state[h] = "same" if same_content(trees[base][rel], base, trees[h][rel], h) else "differs" # All-missing is consistent, not drift: git still lists a path deleted from every # harness (staged or not), and flagging that would leave --fix with nothing to do # and the gate permanently red until the delete was committed. @@ -182,7 +224,14 @@ def main(argv): print(f" removed .{h}/{SUBTREE}/{rel}") continue dst.parent.mkdir(parents=True, exist_ok=True) - shutil.copyfile(src_tree[rel], dst) + # Retarget the source's self-reference at the harness being written, so its copy points + # at a tree that LLM can actually read. Everything else is copied verbatim; a non-text + # file (or one with no self-reference) round-trips byte-for-byte through the same path. + text = normalized(src_tree[rel], args.source) + if text is None: + shutil.copyfile(src_tree[rel], dst) + else: + dst.write_text(text.replace(SELF_TOKEN, f".{h}"), encoding="utf-8", newline="") copied += 1 print(f" wrote .{h}/{SUBTREE}/{rel}") print(f"\nsynced from .{args.source}: {copied} copied, {removed} removed — `git add` the result") diff --git a/scripts/test-sync-harness-skills.py b/scripts/test-sync-harness-skills.py index b78749e..609ce04 100644 --- a/scripts/test-sync-harness-skills.py +++ b/scripts/test-sync-harness-skills.py @@ -23,6 +23,9 @@ * fix_mirrors_delete— a delete in the source is mirrored, not silently re-created forever. * fix_needs_source — bare --fix refuses rather than guessing, because a wrong guess reverts the edit being propagated and then passes the check, hiding the loss. + * self_reference — the one allowed difference: each copy points at its OWN skills tree. The + three cases below pin all of it, because the normalization that permits it + is also the way real drift could start slipping past the gate. """ import shutil import subprocess @@ -163,6 +166,46 @@ def _(tmp): assert "cursor edit" in (tmp / ".cursor" / SKILL).read_text(encoding="utf-8") assert "codex edit" in (tmp / ".codex" / SKILL).read_text(encoding="utf-8") + @case("self_reference — each copy pointing at its own skills tree is not drift") + def _(tmp): + for h in HARNESSES: + (tmp / f".{h}" / SKILL).write_text( + f"# demo skill\nrun .{h}/skills/demo/go.py and pwsh .{h}\\skills\\demo\\go.ps1\n", + encoding="utf-8") + code, out = run(tmp) + assert code == 0, f"self-references must be allowed in both slash flavors:\n{out}" + + @case("self_reference_wrong_target — pointing at ANOTHER harness's tree is still drift") + def _(tmp): + # The hole this closes: normalizing every harness token to one placeholder would make + # `.codex/skills` inside .cursor's copy compare equal, and Cursor would be sent to a + # directory it cannot read with the gate green. + (tmp / ".cursor" / SKILL).write_text("# demo skill\nrun .codex/skills/demo/go.py\n", + encoding="utf-8") + (tmp / ".claude" / SKILL).write_text("# demo skill\nrun .claude/skills/demo/go.py\n", + encoding="utf-8") + (tmp / ".codex" / SKILL).write_text("# demo skill\nrun .codex/skills/demo/go.py\n", + encoding="utf-8") + assert run(tmp)[0] == 1, "a copy pointing at someone else's tree must be reported" + + @case("fix_retargets — --fix rewrites the self-reference for each destination harness") + def _(tmp): + (tmp / ".claude" / SKILL).write_text( + "# demo skill\nread .claude/skills/demo/note.md\n", encoding="utf-8") + code, out = run(tmp, "--fix", "--from", "claude") + assert code == 0, out + for h in HARNESSES: + body = (tmp / f".{h}" / SKILL).read_text(encoding="utf-8") + assert f".{h}/skills/demo/note.md" in body, f".{h} was not retargeted: {body!r}" + assert run(tmp)[0] == 0, "a retargeted tree must satisfy the check it was written for" + + @case("fix_preserves_crlf — a CRLF skill is not silently rewritten to LF") + def _(tmp): + (tmp / ".claude" / SKILL).write_bytes(b"# demo skill\r\nedited\r\n") + assert run(tmp, "--fix", "--from", "claude")[0] == 0 + for h in HARNESSES: + assert (tmp / f".{h}" / SKILL).read_bytes() == b"# demo skill\r\nedited\r\n", h + print("all cases passed") return 0