From 590c60b125801614f7016554fa4882bcda7c1e55 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 22 Aug 2026 06:21:51 -0700 Subject: [PATCH] Add ui-test skill: bugs before beauty before drop-dead gorgeous Adapts browserbase/skills' ui-test to agent-browser and our own bar (DESIGN.md, consumer language, error-sink refIds), replacing "find any bug" with three ordered tiers that stop at the first one a surface fails. CL-6650 --- .claude/skills/ui-test/SKILL.md | 223 ++++++++++++++++++ .../references/agent-browser-recipes.md | 85 +++++++ .../ui-test/references/regression-seeding.md | 40 ++++ .gitignore | 3 +- 4 files changed, 350 insertions(+), 1 deletion(-) create mode 100644 .claude/skills/ui-test/SKILL.md create mode 100644 .claude/skills/ui-test/references/agent-browser-recipes.md create mode 100644 .claude/skills/ui-test/references/regression-seeding.md diff --git a/.claude/skills/ui-test/SKILL.md b/.claude/skills/ui-test/SKILL.md new file mode 100644 index 00000000..59bb2c59 --- /dev/null +++ b/.claude/skills/ui-test/SKILL.md @@ -0,0 +1,223 @@ +--- +name: ui-test +description: 'Grade a live Workbench UI surface through the owner progression bugs before beauty then drop-dead gorgeous. Drives agent-browser against a running app, checks three pass/fail tiers (entity sanity and dead-button bugs, DESIGN.md beauty, Vercel/Stripe/Apple-bar gorgeousness), and produces a surface-tier-blockers-fix report. Use when asked to QA, sweep, test, or grade a UI screen, dialog, or flow before it ships or demos.' +--- + +# ui-test — grade a surface, don't vibe it + +Adapted from [browserbase/skills ui-test](https://github.com/browserbase/skills/tree/main/skills/ui-test): +kept the adversarial stance, the before/after evidence discipline, and the +structured pass/fail markers. Replaced `browse` with **agent-browser** (our +driver), replaced generic UX heuristics with our own three gates (DESIGN.md, +consumer-language, CL-6650's tier ladder), and replaced "find any bug" with +"stop at the first tier you fail" — a surface that fails Tier 1 does not get +a beauty opinion, because a data bug undermines any beauty judgment made +around it. + +Your job is to **find the reason this surface would embarrass someone in a +demo**, not confirm it renders. Every verdict below Tier 3 must cite +snapshot/screenshot evidence; a "looks fine" without a ref or a quoted +snapshot line is not a passing check, it's a skipped one. + +## The three tiers (CL-6650) + +Walk them in order. **Stop at the first tier the surface fails** and report +that as the tier reached — don't grade beauty on a surface that still has +data bugs, and don't grade gorgeousness on a surface that's merely tidy. + +### Tier 1 — bugs (blocks everything) + +- **Entity sanity**: every list contains only its own kind. A routine + showing up in an agent list, a workbench in a skills list — a data bug, + not a display nuance. +- **Names in name slots**: the title/name position never renders a + description, an ID, or a template string. Check the actual text content + of the name element, not just that *some* text is present. +- **No placeholder content**: no "Lorem ipsum", no `TODO`, no seeded fixture + names a real user would never see (see repo's `no-customer-data-in-code` + rule — the reverse also holds: no leaked internal fixture names in a + screen meant to look real). +- **No dead buttons**: every visible actionable control does something + when activated. A button wired to nothing is a Tier 1 bug even if it + looks correct. +- **Feedback ≤100ms**: every action (click, submit, toggle) produces a + visible state change — a spinner, a disabled state, an optimistic + update — within 100ms. Silence after a click reads as broken even if + the real response eventually arrives. +- **Failures name their cause**: an error state shows a human-readable + reason and, where the app's error-sink is wired up, a `refId` a person + could quote to support. A bare "Something went wrong" with no ref is a + Tier 1 finding. + +### Tier 2 — beauty (blocks demo-path surfaces) + +Only graded once Tier 1 is clean. + +- **Identity on every row**: avatar/mark + name + one-line consumer + description — never a bare name floating with no visual identity, never + an ID standing in for a description. +- **Spacing per DESIGN.md**: tight, not loose. Loose spacing is a defect + per standing owner rule, not a style preference — see + `always-tighten-spacing`. +- **Consistent loading treatment**: no blank panes while data resolves, no + overlapping chrome (a skeleton and real content both mounted at once), no + layout jump when content lands. +- **Consumer language only**: copy speaks the user's world + ("Running now," a cron rendered as "every weekday at 9am"), never the + system's internals ("in flight," a raw cron expression, an enum value + leaking into a label). + +### Tier 3 — drop-dead gorgeous (the differentiator) + +Only graded once Tier 2 is clean. This tier is a judgment call, not a +checklist — but the judgment must be argued, not asserted. + +- **The side-by-side test**: screenshot the surface, then ask — if a + viewer put this next to OpenBot or another agent-desktop competitor, + would they say "that's better"? Name the specific thing that would make + them say it, or the specific thing that would make them say the + opposite. +- **Motion earns its place**: transitions encode a real state change + (something entering, transforming, focus moving) — durations in the + 150–300ms DESIGN.md range, easing from the two named curves. Motion that + could be deleted without losing information is decoration, not gorgeous. +- **States feel alive**: presence dots, typing indicators, live status — + anything idle-looking that should read as active is a Tier 3 miss. +- **Zero "is it working?" moments**: no point in the flow where a person + would pause and wonder whether their action registered. + +Report Tier 3 as **reached with notes**, not pass/fail — it's the +differentiator tier, so name what's already gorgeous and what's one step +short, rather than a binary verdict. + +## Driving the surface: agent-browser + +Use a named session for the whole run — never the shared default session, +which other agents and the human may be using concurrently: + +```bash +export AGENT_BROWSER_SESSION="$(agent-browser session id --scope worktree --prefix uitest)" +``` + +(Per CL-6650's acceptance run, `--session uitest` is also acceptable when a +literal fixed name is asked for — either way, name it, don't use the +default.) + +Core loop — identical discipline to the upstream skill, our driver: + +```bash +agent-browser open +agent-browser snapshot -i # interactive elements only +agent-browser click @e3 # act on a ref from the snapshot just taken +agent-browser snapshot -i # re-snapshot — refs go stale the instant the page changes +``` + +Log in once per run if the surface requires auth, then re-use the session +for every subsequent check rather than re-authing per tier. + +### The stale-match trap (read this before grepping a snapshot) + +**Never `grep`/search a whole-page snapshot for the text you're checking.** +A full snapshot includes every region on screen at once — sidebar bench +previews, a notifications panel, a command palette residue — and Workbench +intentionally echoes short strings (a bench's first message, an agent's +name) into more than one place. A grep over the entire tree will match a +sidebar preview and report a false pass ("found the name") when the actual +timeline or dialog you're grading still shows the bug. + +Always scope the assertion to the specific region under test: + +```bash +# Wrong: matches anywhere on the page, including the sidebar's bench preview +agent-browser snapshot -i | grep "Jimmy" + +# Right: scope to the region that actually renders the thing under test +agent-browser snapshot -i -s "[role=dialog]" # a modal/dialog +agent-browser snapshot -i -s "[data-testid=timeline]" # the conversation timeline +agent-browser snapshot -i -s "#main" # the page body, excluding the sidebar +``` + +If the surface has no stable selector for its region, screenshot it and +verify the specific row/element visually rather than trusting a +whole-tree text match. When in doubt, take a narrower snapshot, not a +wider grep. + +### Evidence per finding + +Every claim in the report — pass or fail — cites one of, in order of +rigor: + +1. A scoped snapshot line with its `@eN` ref and exact text. +2. A before/after pair of scoped snapshots showing what changed. +3. A screenshot (`agent-browser screenshot `), for visual-only + properties a snapshot can't capture (spacing, color, motion). + +Save screenshots to `.context/ui-test-screenshots/-.png`. + +## Running the sweep + +1. **Confirm the target renders.** `agent-browser open `, then a + scoped snapshot of the main region — not an error overlay, not a blank + body. +2. **Walk Tier 1 first**, end to end, on the whole surface. Any failure: + stop, that's the tier reached. +3. **If Tier 1 is clean, walk Tier 2.** Any failure: stop, that's the tier + reached — note it still blocks if this is a demo-path surface. +4. **If Tier 2 is clean, walk Tier 3** and write it up as judgment with + evidence, not pass/fail. +5. **Write the report** (format below). +6. `agent-browser close` when done with the session. + +## Report format + +``` +## UI Test — + +**Tier reached:** <1 (bugs) | 2 (beauty) | 3 (gorgeous)> +**URL:** +**Session:** + +### Blockers +1. + - Evidence: + - Smallest fix: + +2. ... + +### Tier 3 notes (only if Tiers 1–2 are clean) +- What already clears the Vercel/Stripe/Apple bar: ... +- What's one step short, and what would close the gap: ... + +### Passed checks +- — — +``` + +List blockers before passed checks — a reviewer needs the bad news first. +A surface with zero Tier 1 or Tier 2 blockers still gets its passed +checks listed, so the report proves the tiers were actually walked rather +than skipped. + +## References + +- [references/agent-browser-recipes.md](references/agent-browser-recipes.md) — + copy-paste command patterns: sessions, scoped snapshots, deterministic + checks (console errors, broken images), screenshot capture. +- [references/regression-seeding.md](references/regression-seeding.md) — + how to seed a known regression locally to verify the skill actually + catches it, per CL-6650's acceptance criteria. + +## Troubleshooting + +- **Stale refs**: re-`snapshot` — refs are assigned fresh every time and + go stale the moment the page changes. +- **Blank snapshot**: `agent-browser wait --load networkidle` before + snapshotting; Workbench routes can return 200 with a still-mounting + shell. +- **Session collision**: make sure every command in the run uses the same + `AGENT_BROWSER_SESSION` (or `--session `) — a command without it + falls back to the shared default session. +- **Can't tell if a match is the real region or a stale sidebar echo**: + narrow the snapshot's `-s` selector further, or fall back to a + screenshot and look at it. diff --git a/.claude/skills/ui-test/references/agent-browser-recipes.md b/.claude/skills/ui-test/references/agent-browser-recipes.md new file mode 100644 index 00000000..94247297 --- /dev/null +++ b/.claude/skills/ui-test/references/agent-browser-recipes.md @@ -0,0 +1,85 @@ +# agent-browser recipes for ui-test + +Copy-paste patterns for the checks SKILL.md calls out. Load `agent-browser +skills get core --full` for the full command reference if a pattern here +doesn't cover what you need — this file only holds the ui-test-specific +subset. + +## Session setup + +```bash +export AGENT_BROWSER_SESSION="$(agent-browser session id --scope worktree --prefix uitest)" +agent-browser open http://localhost:3000 +``` + +Or a fixed literal name when a run needs to be resumable across commands: + +```bash +agent-browser --session uitest open http://localhost:3000 +``` + +Always `agent-browser close` (or `close --all` if you opened more than one +tab/context) at the end of a run. + +## Scoped snapshots (the stale-match trap, operationalized) + +```bash +# Whole page — use only to orient yourself at the start of a run +agent-browser snapshot -i + +# A dialog/modal under test +agent-browser snapshot -i -s "[role=dialog]" + +# The conversation timeline, not the sidebar's preview of the same bench +agent-browser snapshot -i -s "[data-testid=timeline]" + +# The page body, explicitly excluding the sidebar +agent-browser snapshot -i -s "#main" +``` + +If the surface has no `data-testid`, scope by the nearest stable +landmark role (`snapshot -i -s "[role=main]"`, `-s "form"`) before +falling back to a screenshot. + +## Before/after comparison + +```bash +agent-browser snapshot -i -s "[role=dialog]" # BEFORE +agent-browser click @e4 # ACT +agent-browser snapshot -i -s "[role=dialog]" # AFTER — compare, don't assume +``` + +## Feedback-latency check (Tier 1: actions acknowledged ≤100ms) + +There is no built-in stopwatch — approximate it by snapshotting +immediately after the action and checking whether *any* visible change +(disabled state, spinner, optimistic row) is already present, versus a +`wait --text` call to see how long the real change takes to land: + +```bash +agent-browser click @e4 +agent-browser snapshot -i -s "[role=dialog]" # should already differ from BEFORE +``` + +If the immediate snapshot is byte-identical to BEFORE, that's a Tier 1 +finding regardless of whether the eventual result is correct. + +## Deterministic checks + +```bash +# Console errors since the page loaded +agent-browser eval "window.__uitestErrors ?? []" # if the app exposes a hook; otherwise: +agent-browser eval "Array.from(document.querySelectorAll('img')).filter(i => i.naturalWidth === 0).map(i => i.src)" +``` + +## Screenshot evidence + +```bash +mkdir -p .context/ui-test-screenshots +agent-browser screenshot .context/ui-test-screenshots/-.png +``` + +Take the screenshot at the moment of failure, not after any recovery +click. For Tier 3 visual judgment (spacing, motion, color), screenshot +the surface even when nothing is "wrong" — the report cites it as +evidence for the gorgeous-tier argument either way. diff --git a/.claude/skills/ui-test/references/regression-seeding.md b/.claude/skills/ui-test/references/regression-seeding.md new file mode 100644 index 00000000..88ebeb29 --- /dev/null +++ b/.claude/skills/ui-test/references/regression-seeding.md @@ -0,0 +1,40 @@ +# Seeding a regression to verify the skill actually catches it + +CL-6650's acceptance criteria requires proving the skill flags a known +Tier 1 bug, not just that it produces a report. Use this before trusting +a change to the skill itself. + +## Pattern + +1. Pick a surface and a specific Tier 1 rule (e.g. "names in name slots"). +2. Temporarily edit the source so the surface violates that rule — e.g. + rename an agent row's displayed name to its description string. +3. Run the ui-test sweep against the running dev server with the edit in + place. Confirm the report's Tier 1 blockers section names the exact + row and cites a scoped snapshot line showing the wrong text. +4. Revert the edit (`git checkout -- ` or undo the in-memory + change) before finishing — a seeded regression is a local-only probe, + never something that ships. + +## Worked example (Jimmy's row) + +```bash +# Seed: swap the name-slot text for the description text in the +# component that renders the roster row (path depends on the surface +# under test — locate the row-rendering component first). +# e.g. temporarily render {agent.description} where {agent.name} belongs. + +agent-browser open http://localhost:3000/ +agent-browser snapshot -i -s "[data-testid=roster]" +# Expect the report to flag: "name slot renders 'Handles onboarding +# flows for new hires' instead of 'Jimmy'" with the scoped snapshot ref +# as evidence. + +# Revert the seed once confirmed. +git checkout -- +``` + +A sweep that reports Tier 2/3 findings but misses the seeded Tier 1 swap +means the Tier 1 walk isn't actually checking name-slot content — fix the +skill's Tier 1 instructions before trusting its verdicts on anything +else. diff --git a/.gitignore b/.gitignore index 462aa908..d03989c4 100644 --- a/.gitignore +++ b/.gitignore @@ -12,7 +12,8 @@ coverage/ # Local orchestration / planning artifacts (not product source) .worktrees/ .agent-state/ -.claude/ +.claude/* +!.claude/skills/ dispatch/ specs/ plans/