feat(skills): add the issue verification skill (screenshot and video generator) by vbudhram · Pull Request #21170 · mozilla/fxa

vbudhram · 2026-09-09T12:44:24Z

Because

Sometimes you want visual proof that an issue is fixed

This pull request

Adds .claude/skills/fxa-issue-verification: the skill, a Playwright fixture and Sync-flow gotchas reference, and a Marionette frame recorder.
This skill builds a throw away Playwright test that records going through the flow.

Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-14525

Checklist

My commit is GPG signed.
If applicable, I have modified or added tests which pass locally.
I have added necessary documentation (if appropriate).
I have verified that my changes render correctly in RTL (if appropriate).
I have manually reviewed all AI generated code.

Other information

I've used this skill a few times to generate videos of before and after screens. This is helpful to verify the issue its supposed to fix. This skill can also record a video of the flow (utilizes Playwright recording).

Ex. #21126 #21139

Copilot

🟡 Changes recommended

Unsafe checkout detection, contradictory secret handling, incomplete workflows, and recorder reliability issues must be addressed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a reusable workflow for capturing visual evidence of FxA fixes and user flows.

Changes:

Adds engine selection and verification workflow guidance.
Adds Marionette frame recording.
Documents Playwright Sync-specific constraints.

File summaries

File	Description
`SKILL.md`	Defines verification, capture, review, and upload workflows.
`recorder.ts`	Records Firefox chrome frames through Marionette.
`playwright-sync-gotchas.md`	Documents Sync and functional-test caveats.

Review details

Files reviewed: 3/3 changed files
Comments generated: 7
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+
+## Hard rules
+
+- **Switch branches only in a clone this session owns, and only when it is clean.** An `fxa-agent*` style full clone (`git rev-parse --git-dir` is a directory, not a file) with `node_modules` is the session's own; `gh pr checkout <n>` or `git checkout` there is the normal way to put the code under test in place, after `git status --short` is empty and the user confirms. Never switch branches in a linked worktree (`.claude/worktrees/*`, `git rev-parse --git-dir` is a file) or in a clone another session is using, and never `git stash`.


+| Diff is a component, page state, copy, style, or layout change with a `*.stories.tsx` next to it, and no server state is needed to show it; or `libs/accounts/email-renderer/**` | **Storybook**: screenshots of stories, no stack |
+| Anything else with a user-visible page, including Sync sign-in through `/pair` | **Playwright**: standard fixtures, page objects, native WebM |
+| `$ARGUMENTS` contains `chrome`, or the acceptance criterion is about the toolbar, the app menu, a Firefox panel, or a Firefox dialog | **Marionette** (opt-in, see Step 3c). Say first that panels appear as DOM text, never as pixels. |
+| `$ARGUMENTS` contains `pairing`, or the change is a firefox-ios or FxA pairing PR | Stop and use `/fxa-pairing-verification` instead; it records the Simulator and the desktop authority together. |


+| Anything else with a user-visible page, including Sync sign-in through `/pair` | **Playwright**: standard fixtures, page objects, native WebM |
+| `$ARGUMENTS` contains `chrome`, or the acceptance criterion is about the toolbar, the app menu, a Firefox panel, or a Firefox dialog | **Marionette** (opt-in, see Step 3c). Say first that panels appear as DOM text, never as pixels. |
+| `$ARGUMENTS` contains `pairing`, or the change is a firefox-ios or FxA pairing PR | Stop and use `/fxa-pairing-verification` instead; it records the Simulator and the desktop authority together. |
+| `packages/fxa-auth-server/**`, `libs/**`, `packages/fxa-profile-server/**` only | **API**: Playwright spec asserting on `target.authClient` responses, evidence posted as text |


+**`main` comparison, only when `$ARGUMENTS` contains `before` or the user asks.** Confirm the plan once, then each command.
+
+1. Get `main` in place. In an owned clone (Step 1, case 2), the after media is already captured, so `git fetch origin main --quiet && git checkout origin/main --detach` in the same clone is enough, and Step 5.3 then restarts the stack from it; return to the branch afterwards. Otherwise, a worktree beside the clones, never inside another checkout: `ROOT=$(dirname "$(git rev-parse --path-format=absolute --git-common-dir)")` is the main clone, so the target is `$(dirname "$ROOT")/fxa-verify-<NAME>` (a sibling of `fxa`, `fxa-agent2`, and the rest); a session inside `.claude/worktrees/` must not use `..`, which lands there. `git worktree add <target> origin/main` then `(cd <target> && yarn install)`.
+2. Storybook: skip the worktree. Swap the changed files in place, rebuild the stylesheet, capture, then put the branch back. The tracked tree must be clean first, and it must be clean again at the end.


+  start(intervalMs = 150) {
+    this.timer = setInterval(() => void this.frame().catch(() => undefined), intervalMs);
+  }


+      await this.raw('Marionette:SetContext', { value: 'chrome' });
+      const found = (await this.raw('WebDriver:FindElement', { using: 'css selector', value: ':root' })) as any;
+      const id = found.value['element-6066-11e4-a52e-4f735466cecf'];
+      const png = (await this.raw('WebDriver:TakeScreenshot', { id })) as any;
+      await this.raw('Marionette:SetContext', { value: restore });


+
+## The 123done relier
+
+- `relier.goto()` opens `http://localhost:8080`; `clickEmailFirst()` waits for the `123done` heading first. The relier needs `packages/123done/secrets.json` (gitignored) for the token exchange; without it the auth server rejects `/api/oauth` with a validation error on `client_secret`, the page returns to 123done, and `#loggedin` never appears. A fresh worktree has no copy; link one in and `pm2 restart 123done 321done`.


dschom · 2026-09-15T16:36:38Z

+
+# Verify a fix, or record a flow, and produce media for the PR
+
+Playwright drives the flow and records native video. Storybook covers component-only changes. Marionette exists only for evidence that must show Firefox's own chrome, and only when asked. The pipeline: pick the engine, capture on the branch, optionally capture on `main`, then hand the media over or attach it with `gh --attach`, whichever the user picks.


Maybe say that capture on main is for comparison purposes? I feel like I'm kinda reading between the lines there?

I also sort of feel like there's a lot of complexity/instructions just around switching branches and stuff. Maybe that's better handled another way. But maybe it's fine. Happy to start using it and iterate as needed.

dschom · 2026-09-15T16:37:10Z

+Run in parallel:
+
+- `gh --version` — 2.99.0 or newer for `--attach`. Older: stop and say so.
+- `gh pr view --json number,url,body,isDraft`, or `gh pr view <n>` when `$ARGUMENTS` is a number. No PR: ask for the number; if none exists yet, run Steps 2 to 6 and hand over the media and markdown for `/fxa-pr-open`, skip Step 7.


nit: number steps so this is more obvious that bullet points.

dschom · 2026-09-15T16:39:41Z

+- `gh --version` — 2.99.0 or newer for `--attach`. Older: stop and say so.
+- `gh pr view --json number,url,body,isDraft`, or `gh pr view <n>` when `$ARGUMENTS` is a number. No PR: ask for the number; if none exists yet, run Steps 2 to 6 and hand over the media and markdown for `/fxa-pr-open`, skip Step 7.
+- `git diff origin/main...HEAD --name-only` — the changed surface. Empty in Verify mode: stop. Empty in Record mode: fine.
+- **Which checkout the stack serves** — `pm2 jlist 2>/dev/null | sed -n '/^\[/,$p' | jq -r '.[] | select(.name=="settings-react" or .name=="auth" or .name=="content") | "\(.name) \(.pm2_env.pm_cwd)"'`. Pick an app service, never `.[0]`: index 0 is `mysql`, so it reports whichever clone started infrastructure and hides the split case this check exists to catch. It must be the checkout under test, or the run tests other code. To repoint it, run in the target clone: `yarn delete services`, then `pm2 delete auth auth-ftl inbox` (the Nx target misses those three), then `yarn start mza`. Infrastructure stays up. Confirm before running it, because it stops the app services. Record mode with an empty diff may proceed on any stack; name its checkout in the report.


I found this section a little confusing from the title to the instructions. I'm kind of confused why it wouldn't be the branch under test? Maybe I'm just missing a common pitfall when running this.

This unfortunately came up with multiple worktrees and trying to record (and start stack). It sees if there are any running pm2 processes which could be a from a different worktree and stops them and start the current branch. Let me see if this can be worded better, I originally had this in a script but wanted a more self contained skill.

dschom · 2026-09-15T16:41:52Z

+---
+name: fxa-issue-verification
+description: Use when a bug fix or UI change on the current FxA branch needs proof that it works, or when a user flow or component state should be recorded as video or screenshots and attached to the pull request. Triggers include "verify the fix", "prove FXA-N is fixed", "record this flow", "record before and after", "add a screen recording to the PR".
+argument-hint: Jira key, PR number, or a quoted flow to record (e.g. FXA-14500, 21150, or "sign in to Sync and land on /pair"). Add "before" for a main comparison, "comment" to post as a PR comment, "chrome" to force real Firefox chrome via Marionette.


I feel like just targeting the current branch and associated PR would be simpler and less confusing that specifying as an argument hint.

I comment about this later, but a lot of the sort of complexity (and parts I found semi-hard to follow) in the instructions seems to had to do with ensuring the correct branch is being used.

dschom · 2026-09-15T16:44:24Z

+  - It is a linked Claude worktree without `node_modules`: edit-only. Use a clone that has the branch and repoint the stack at it, or `yarn install` here only after confirmation.
+  - Nothing has the branch: a sibling worktree per Step 5, the last resort.
+- `npx playwright install --dry-run firefox` from `packages/functional-tests` — confirm before any download.
+- `ffmpeg -version` — every video mode needs it: trimming and converting the Playwright WebM to MP4, encoding Marionette frames, and extracting the Step 4 review frames. Missing: stop, say so, and offer screenshots only (`brew install ffmpeg` on macOS). Not needed for Storybook or a screenshots-only run.


I wonder if there should be some pre-reqs right at the beginning of the doc? Might save Caude from doing a bunch of stuff and then hitting this wall. Perhaps, it's smart enough to load this into context and realize this check needs to happen pretty early...

dschom

Excited to try this more! A couple comments, just opinions weakly held, take 'em or leave 'em.

It'll be cool to iterate on this. I think it could pair nicely with the /fxa-pr-debug skill. e.g. You could see if circle is running the devstack and use that instead of a local pm2. But that's probably a future iteration. Or maybe some another skill that ties these two together.

Because: - Proving a branch fix works meant re-deriving the Playwright and Marionette setup each time, and the functional-test fixture traps were written down nowhere in the repo. - The skill was local to one machine, so nobody else could run it. This commit: - Adds .claude/skills/fxa-issue-verification: the skill, the Playwright fixture and Sync-flow gotchas, and the Marionette frame recorder. - Drives the stack with yarn ports, pm2 jlist and lsof rather than a personal script that is not in this repo. - Adds an ffmpeg preflight check, which every video mode depends on. - Gates every PR write behind an approval step at the top of Step 7, with handing the media over and leaving the PR alone as a first-class answer.

vbudhram self-assigned this Sep 9, 2026

Copilot AI balanced review requested due to automatic review settings September 9, 2026 12:44

vbudhram requested a review from a team as a code owner September 9, 2026 12:44

Copilot started reviewing on behalf of vbudhram September 9, 2026 12:44 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

vbudhram force-pushed the add-screenshot-video branch from fa8f043 to a35b356 Compare September 10, 2026 20:58

dschom reviewed Sep 15, 2026

View reviewed changes

dschom approved these changes Sep 15, 2026

View reviewed changes

vbudhram force-pushed the add-screenshot-video branch from a35b356 to 3c55bc0 Compare September 15, 2026 17:16

vbudhram merged commit 1a62cbe into main Sep 15, 2026
17 of 19 checks passed

vbudhram deleted the add-screenshot-video branch September 15, 2026 17:44

Navigation Menu

Uh oh!

feat(skills): add the issue verification skill (screenshot and video generator) - #21170

feat(skills): add the issue verification skill (screenshot and video generator)#21170
vbudhram merged 1 commit into
mainfrom
add-screenshot-video

vbudhram commented Sep 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

dschom Sep 15, 2026

Uh oh!

dschom Sep 15, 2026 •

edited

Loading

Uh oh!

dschom Sep 15, 2026

Uh oh!

vbudhram Sep 15, 2026

Uh oh!

dschom Sep 15, 2026

Uh oh!

dschom Sep 15, 2026

Uh oh!

dschom left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		## Hard rules

		- Switch branches only in a clone this session owns, and only when it is clean. An `fxa-agent` style full clone (`git rev-parse --git-dir` is a directory, not a file) with `node_modules` is the session's own; `gh pr checkout <n>` or `git checkout` there is the normal way to put the code under test in place, after `git status --short` is empty and the user confirms. Never switch branches in a linked worktree (`.claude/worktrees/`, `git rev-parse --git-dir` is a file) or in a clone another session is using, and never `git stash`.


		## The 123done relier

		- `relier.goto()` opens `http://localhost:8080`; `clickEmailFirst()` waits for the `123done` heading first. The relier needs `packages/123done/secrets.json` (gitignored) for the token exchange; without it the auth server rejects `/api/oauth` with a validation error on `client_secret`, the page returns to 123done, and `#loggedin` never appears. A fresh worktree has no copy; link one in and `pm2 restart 123done 321done`.


		# Verify a fix, or record a flow, and produce media for the PR

		Playwright drives the flow and records native video. Storybook covers component-only changes. Marionette exists only for evidence that must show Firefox's own chrome, and only when asked. The pipeline: pick the engine, capture on the branch, optionally capture on `main`, then hand the media over or attach it with `gh --attach`, whichever the user picks.

Uh oh!

Conversation

vbudhram commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Because

This pull request

Issue that this pull request solves

Checklist

Other information

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

dschom Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

dschom Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschom Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

vbudhram Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

dschom Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

dschom Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

dschom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vbudhram commented Sep 9, 2026 •

edited

Loading

dschom Sep 15, 2026 •

edited

Loading