pr-handoff

# PR Handoff For any PR that touches the UI, **screenshots are the primary deliverable**. A reviewer grasps the change in seconds from an annotated screenshot — no amount of prose does that. Capture them before writing the description, and get them into the PR body yourself through one of the three routes in §2h — never as a follow-up comment. When none of those routes is available, §2h option 3 is the correct ending, not a failure. Scope: **GitHub + `gh`**. Other forges are out of scope. ## Requirements | Tool | Needed for | Checked | |---|---|---| | `git`, `gh` (authenticated), `curl` | diff, PR create/edit, reachability | up front | | `agent-browser` | every screenshot and screencast | only on UI PRs | | ImageMagick (`magick` or `convert`) | stitching the collage | only for 2+ shots | | `ffmpeg` | webm → GIF | only for screencasts | | `gifski` | smaller, sharper GIFs | optional | | [`share-file`](https://github.com/Vesely/skills/tree/main/share-file) | hosting the image so GitHub can render it (§2h option 1) | optional, recommended | **Up-front check** — cheap, and a docs-only PR should never trigger a browser install: ```bash for bin in git gh curl; do command -v "$bin" >/dev/null 2>&1 || echo "missing: $bin"; done gh auth status >/dev/null 2>&1 || echo "gh is not authenticated — the user must run: gh auth login" ``` **UI check** — run this only once §1 confirms the diff touches UI: ```bash command -v agent-browser >/dev/null 2>&1 || echo "missing: agent-browser" command -v npm >/dev/null 2>&1 || echo "missing: npm (needed to install agent-browser)" if command -v magick >/dev/null 2>&1; then IM=magick; MONTAGE="magick montage" elif command -v convert >/dev/null 2>&1; then IM=convert; MONTAGE=montage else echo "missing: imagemagick"; fi ``` `$IM` and `$MONTAGE` are used throughout §2f. ImageMagick 7 provides `magick`; ImageMagick 6 — which is what `apt-get install imagemagick` still gives you on Debian and Ubuntu — provides `convert` and a standalone `montage` instead. Checking for either is what keeps this skill working on Linux. Installing what is missing — tell the user the command before you run it: ```bash npm i -g agent-browser && agent-browser install # add --with-deps on Linux for browser system libs brew install gh imagemagick ffmpeg gifski # macOS sudo apt-get install -y imagemagick ffmpeg # Debian/Ubuntu — ASK before any sudo sudo dnf install -y ImageMagick ffmpeg # Fedora ``` Rules: - Announce before installing. Never run a `sudo` install without an explicit yes. - `gh` is not in older Ubuntu/Debian archives; it may need GitHub's own apt source. Say so rather than looping on a failing install. - No package manager, or the user declines? **Degrade, don't stop**: without ImageMagick, embed the individual screenshots one `![]()` per line — every changed surface still gets shown. Without ffmpeg, skip the screencast. - Missing `agent-browser` on a UI PR is the one thing worth pausing for. Offer the `npm i -g` line and wait. **Before running any `agent-browser` command**, load its usage guide — the CLI serves docs matching its own version, so the syntax never goes stale: ```bash agent-browser skills get core ``` If the `/agent-browser` skill is installed, invoke that instead. The commands below are the shape of the flow, not a substitute for the guide. ## 0. Project specifics live in the project, not here This skill is project-agnostic on purpose. It does not know your dev-server port, your routes, or your login. Resolve those in this order: 1. The project's `CLAUDE.md`, `AGENTS.md`, `README`, or `CONTRIBUTING` 2. `package.json` scripts, `Procfile`, `docker-compose.yml` port mappings, `.env` `PORT` 3. Ask the user When you had to ask, offer to record the answer in the project's `CLAUDE.md` so the next run doesn't ask again: > "Want me to add a line to CLAUDE.md — `dev server: <command> on :<port>, log in via the agent-browser vault entry '<name>'` — so this is automatic next time?" **Credentials.** Never grep `.env` for a password and never put one in a shell command — it lands in shell history and in the transcript. agent-browser has a vault; **the user runs this in their own terminal**, because it waits for a typed password: ```bash # USER RUNS THIS, not the agent — it reads the password from a TTY agent-browser auth save myapp-local --url http://localhost:PORT/login \ --username dev@example.com --password-stdin ``` The agent then only ever calls: ```bash agent-browser auth login myapp-local ``` If no vault entry exists, ask the user to create one. Never invent credentials, and never satisfy `--password-stdin` by piping in a secret you read from a file. **Data sensitivity.** Screenshots of a real admin or dashboard capture whatever is on screen — customer names, invoice totals, e-mail addresses, tokens in a debug bar. Prefer a seeded or demo account. If the shots do contain real data, that fact must reach the user **before** anything leaves the machine, not when you present the finished PR — see the gate in §2h. ## 1. Read the diff Resolve the base branch. Do this with explicit checks — a `git ... | sed ...` pipeline returns *sed's* exit status, so chaining fallbacks with `||` silently yields an empty base, and an empty base makes the diff empty and the whole run conclude "no UI changes": ```bash BASE="$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name 2>/dev/null)" if [ -z "$BASE" ] && git symbolic-ref -q refs/remotes/origin/HEAD >/dev/null 2>&1; then BASE="$(git symbolic-ref --short refs/remotes/origin/HEAD)"; BASE="${BASE#origin/}" fi [ -z "$BASE" ] && BASE=main git rev-parse --verify -q "$BASE" >/dev/null || git rev-parse --verify -q "origin/$BASE" >/dev/null \ || { echo "no base branch resolved — ask the user which branch this PR targets"; } echo "base: $BASE" ``` Test each step, don't chain them with `||`. `git rev-parse --abbrev-ref origin/HEAD` prints the literal string `HEAD` when `origin/HEAD` is unset rather than failing, and `git symbolic-ref … | sed …` returns *sed's* exit status — either one hands you a base that turns `git diff "$BASE"...HEAD` into an empty diff, and the run then concludes "no UI changes" and skips every screenshot. Check for uncommitted work — it will not appear in `git diff "$BASE"...HEAD`: ```bash git status --short ``` If there is any, ask whether the PR covers it. To include it, diff the working tree directly — `git diff HEAD` (tracked, staged and unstaged) — do **not** stash first; stashing removes the very changes you are trying to read. Read the whole diff. Always: ```bash git diff "$BASE"...HEAD git log "$BASE"..HEAD --oneline ``` Only when that is genuinely too large for one pass, start from `git diff --stat "$BASE"...HEAD` and then read every hunk area it lists — per-path reads are a way to get through a big diff, not a licence to sample it. Cross-cutting UI changes are exactly what a partial read misses. From the diff, answer two questions: - **Does this touch user-facing UI?** Templates, components, admin screens, public pages, e-mails — anything a person looks at. → step 2. - **Does this change the schema or core data models?** Migrations, model definitions, `schema.prisma`, `.sql` schema files, serializers, API response shapes. → step 4. No UI? Skip to step 3. ## 2. Screenshots ### 2a. Reach the app Resolve the dev-server URL per §0, then confirm it responds before opening a browser: ```bash curl -sS -o /dev/null -w '%{http_code}\n' --max-time 5 "$APP_URL" ``` Unreachable: offer the start command you found in §0. Do not auto-start a long-running server without asking. If it cannot be started, note that in the PR description and skip to step 3. ### 2b. Workspace ```bash BRANCH="$(git branch --show-current)" [ -z "$BRANCH" ] && BRANCH="detached-$(git rev-parse --short HEAD)" SLUGBRANCH="$(printf '%s' "$BRANCH" | tr '/#%' '-')" SHOTS="$(mktemp -d "${TMPDIR:-/tmp}/pr-handoff-${SLUGBRANCH}-XXXXXX")" PANELS="$SHOTS/panels"; mkdir -p "$PANELS" echo "$SHOTS" ``` `mktemp -d` per run — never `rm -rf` a path built from a variable that can be empty. **Shell variables do not survive between tool calls.** Each command you run is a fresh shell, so `$SHOTS`, `$PANELS`, `$BRANCH`, `$IM` and `$URL` are gone by the next call. Echo the values once (above) and then use the literal paths, or re-derive them at the top of each block. A silently-empty `$SHOTS` writes screenshots to `/01-….png`. Pin the viewport once so every panel is the same width: ```bash agent-browser set viewport 1440 900 2 # 2 = retina, sharper text in the collage ``` ### 2c. Map every affected surface Re-read the diff and list every URL where the change is visible: - The changed screen itself — list view, detail view, edit form, modal - Any screen that *displays* changed data, even when the change was server-side - Both ends of a feature that spans surfaces (a setting in admin, its effect on the public page) For each, plan the states worth capturing: default/empty, populated with realistic data, and the edge cases the diff implies (long text, missing value, error state). ### 2d. Capture each surface — annotate first, then shoot The order matters: a screenshot taken before the markup is injected is an unannotated screenshot. ```bash agent-browser open "$APP_URL/some/route" agent-browser wait --load networkidle # 1. clear any markers left from the previous shot (see 2e) # 2. inject this shot's markers (see 2e) agent-browser screenshot "$SHOTS/01-orders-list-default.png" ``` Name pattern: `<NN>-<surface>-<state>.png` — zero-padded, so the collage orders itself. Always wait for async content before capturing: streamed responses, lazy sections, live previews. A spinner in a PR screenshot reads as a broken feature. Use `--full` for full scroll height when the change extends below the fold. ### 2e. Annotation Mark up the DOM by selector — never pixel coordinates. Pipe the script via heredoc so quoting survives. Tag every marker so it can be removed again: ```bash cat <<'EOF' | agent-browser eval --stdin const el = document.querySelector('[data-testid="order-total"]'); el.style.outline = '3px solid red'; el.style.outlineOffset = '4px'; el.dataset.prhOutlined = '1'; EOF ``` For a new section that resists outlining, inject a positioned arrow: ```bash cat <<'EOF' | agent-browser eval --stdin const el = document.querySelector('.new-section'); const r = el.getBoundingClientRect(); const arrow = document.createElement('div'); arrow.dataset.prhMarker = '1'; arrow.style.cssText = 'position:fixed;left:' + (r.left - 44) + 'px;top:' + (r.top + r.height / 2 - 12) + 'px;z-index:99999;font-size:24px;color:red;pointer-events:none'; arrow.textContent = '▶'; document.body.appendChild(arrow); EOF ``` Clear them before the next shot on the same page, or markers accumulate across states: ```bash cat <<'EOF' | agent-browser eval --stdin document.querySelectorAll('[data-prh-marker]').forEach(el => el.remove()); document.querySelectorAll('[data-prh-outlined]').forEach(el => { el.style.outline = ''; el.style.outlineOffset = ''; delete el.dataset.prhOutlined; }); EOF ``` Rules: - All annotations red — reviewers learn that red means changed. - Annotate every shot of a *changed* surface. A context shot of surrounding UI needs none. - Never cover the thing you are pointing at. - **Before/after collages** may use two colours (red = old/broken, green = new/fixed) instead of red-only. Apply it to **every** panel consistently — a stray red box in an "after" panel inverts the meaning. This is the single most common mistake here; re-check it in §2g. ### 2f. Crop and stitch Panels that go into the collage live in `$PANELS`; derived files never land back in the source directory, so a second run cannot stitch its own output. Copy each shot into `$PANELS`, cropping the ones that need it — geometry is per-surface, so pick it per file rather than applying one rectangle to everything: ```bash cp "$SHOTS"/[0-9][0-9]-*.png "$PANELS"/ # keep full-frame panels "$IM" "$SHOTS/02-orders-list-filtered.png" -crop 600x440+500+350 +repage \ "$PANELS/02-orders-list-filtered.png" # tighten just this one ``` Then stitch. Guard the glob — an empty `$PANELS` makes bash pass the literal pattern to ImageMagick and makes zsh abort the command: ```bash shopt -s nullglob 2>/dev/null || setopt null_glob 2>/dev/null panels=("$PANELS"/*.png); COUNT=${#panels[@]} [ "$COUNT" -eq 0 ] && { echo "no panels — nothing to stitch"; exit 1; } if [ "$COUNT" -le 2 ]; then "$IM" "${panels[@]}" +smush 40 -bordercolor 'rgb(40,40,40)' -border 40 "$SHOTS/collage.png" else COLS=$(( COUNT <= 6 ? 2 : 3 )) ROWS=$(( (COUNT + COLS - 1) / COLS )) # round up, so no panel is dropped $MONTAGE "${panels[@]}" -tile "${COLS}x${ROWS}" -geometry +20+20 \ -background 'rgb(40,40,40)' -bordercolor 'rgb(40,40,40)' -border 20 "$SHOTS/collage.png" fi "$IM" identify "$SHOTS/collage.png" ``` **Keep the collage legible.** GitHub renders a PR image at roughly 900px wide. A 1440px viewport at DPR 2 is a 2880px panel, so four of those smushed side by side is ~11,600px — a 13× downscale that turns every label into mush. Two rules follow: use the grid from three panels up (the code above does), and keep each panel at least ~600px wide *in the final image*. If both can't hold, make two collages rather than one unreadable one, and cap the final width: ```bash "$IM" "$SHOTS/collage.png" -resize '1800x>' "$SHOTS/collage.png" ``` `montage` printing `unable to read font` is harmless — the machine has no default font configured, the collage still renders; confirm with `identify` rather than chasing it. One screenshot: skip the collage entirely. No ImageMagick: embed the individual images, one `![]()` per line, so no surface goes unshown. ### 2g. Review the collage, then fix it Generating the collage is not the last step. **Judging it and correcting it is.** Open the final PNG with the Read tool and critique it as the reviewer will: - **No blank or broken panels** — every capture rendered real content; nothing white, cut off, or mid-spinner. - **Annotations land on target** — each box tightly frames the exact element. Nothing floating in whitespace beside it, nothing overshooting into unrelated content. (A box one line too low, framing empty space instead of the value, is the classic failure.) - **Colour semantics consistent** — every before marker red, every after marker green, no inverted panel. - **Nothing important obscured**, and no marker left over from a previous state. - **Text is readable at the size GitHub will render it.** - **Captions still match the panels** in order and description. If anything fails: adjust the geometry, re-run, Read it again. **Loop until every item passes.** Do not publish a collage you would not want a reviewer to see as-is — a visibly-off annotation makes the whole PR look careless and costs a round-trip with the user. ### 2h. Host the image **The invariant, which the three options below cannot override.** The image may leave this machine by exactly the three routes in this section and by no other. Everything else is out of bounds regardless of how the run is going: image hosts (catbox, imgur, litterbox, tmpfiles, 0x0.st and every sibling), gists, release assets, a bucket you pick, a `curl` you compose, committing the file to the PR branch, or `git add -f` past a `.gitignore`. Not embedding an image is an acceptable outcome; publishing one somewhere the user did not choose is not. Option 3 is always available and publishes nothing, so "everything else failed" is never a reason to improvise a fourth route. If the user explicitly asks for a public host, name exactly what becomes public and permanent, and get a yes for that specific upload first. Pick the destination by what the repo is: ```bash