From d09afbc580aefe0f012d7d03cdcbf4c1f12f4971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Fri, 2 Oct 2026 15:16:08 -0400 Subject: [PATCH 1/8] feat(commit): Document attaching PR screenshots with gh --attach Agents were claiming gh can't attach images and asking users to drag screenshots in, though gh pr create and gh pr edit support --attach. --- skills/commit/SKILL.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/skills/commit/SKILL.md b/skills/commit/SKILL.md index f50eadf..fcfbea8 100644 --- a/skills/commit/SKILL.md +++ b/skills/commit/SKILL.md @@ -1,6 +1,6 @@ --- name: commit -description: Use for every request to commit changes or draft a commit message. Creates Sentry-style conventional commits with issue references. +description: Use for every request to commit changes or draft a commit message. Creates Sentry-style conventional commits with issue references, and covers attaching screenshots to the pull request with gh. --- # Sentry Commit Messages @@ -60,3 +60,18 @@ git commit -m "fix(api): Handle null response in user endpoint" \ -m "Return 404 when the user API finds a deleted account." \ -m "Fixes SENTRY-5678" ``` + +## Attaching Images to the Pull Request + +`gh pr create` and `gh pr edit` upload images and videos with +`--attach '#'`, so attach screenshots directly rather than +asking the user to drag them in. A body reference to the local path, such as +`![Before](./before.png)`, is rewritten to the uploaded asset; without one, +the attachment is appended to the body. + +```bash +gh pr edit --attach './before.png#Before' --attach './after.png#After' \ + --body "... ![Before](./before.png) ![After](./after.png)" +``` + +Without a body flag, `gh pr edit --attach` keeps the existing body. From 7291c516520f8619737f9faa9eaab6cd5dfe74e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Fri, 2 Oct 2026 15:29:05 -0400 Subject: [PATCH 2/8] fix(commit): Note gh 2.99.0 requirement for --attach Older gh versions reject --attach as an unknown flag. Co-Authored-By: Claude Opus 5.5 --- skills/commit/SKILL.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/skills/commit/SKILL.md b/skills/commit/SKILL.md index fcfbea8..f82c07b 100644 --- a/skills/commit/SKILL.md +++ b/skills/commit/SKILL.md @@ -63,9 +63,10 @@ git commit -m "fix(api): Handle null response in user endpoint" \ ## Attaching Images to the Pull Request -`gh pr create` and `gh pr edit` upload images and videos with +Since gh 2.99.0, `gh pr create` and `gh pr edit` upload images and videos with `--attach '#'`, so attach screenshots directly rather than -asking the user to drag them in. A body reference to the local path, such as +asking the user to drag them in. If `gh` rejects `--attach` as an unknown flag, +ask the user to upgrade `gh`. A body reference to the local path, such as `![Before](./before.png)`, is rewritten to the uploaded asset; without one, the attachment is appended to the body. From e7e9cd7585385f3cfcd99b6ee966daa39ebe5ed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Ghoulberg=20=F0=9F=91=BB?= Date: Fri, 9 Oct 2026 10:27:26 -0400 Subject: [PATCH 3/8] Update skills/commit/SKILL.md --- skills/commit/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/commit/SKILL.md b/skills/commit/SKILL.md index f82c07b..b923cae 100644 --- a/skills/commit/SKILL.md +++ b/skills/commit/SKILL.md @@ -1,6 +1,6 @@ --- name: commit -description: Use for every request to commit changes or draft a commit message. Creates Sentry-style conventional commits with issue references, and covers attaching screenshots to the pull request with gh. +description: Use for every request to commit changes or draft a commit message. Creates Sentry-style conventional commits with issue references. --- # Sentry Commit Messages From f2077469b1690da632bd0342de01f96378abe086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Fri, 9 Oct 2026 10:55:30 -0400 Subject: [PATCH 4/8] expand the heck out of it --- .claude/settings.json | 1 + README.md | 1 + skills/claude-settings-audit/SKILL.md | 1 + skills/commit/SKILL.md | 16 ---- skills/pr-screenshots/SKILL.md | 123 ++++++++++++++++++++++++++ skills/pr-screenshots/SPEC.md | 35 ++++++++ 6 files changed, 161 insertions(+), 16 deletions(-) create mode 100644 skills/pr-screenshots/SKILL.md create mode 100644 skills/pr-screenshots/SPEC.md diff --git a/.claude/settings.json b/.claude/settings.json index c362285..6b955a7 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -55,6 +55,7 @@ "Skill(sentry-skills:gha-security-review)", "Skill(sentry-skills:iterate-pr)", "Skill(sentry-skills:pr-link-issue)", + "Skill(sentry-skills:pr-screenshots)", "Skill(sentry-skills:pr-writer)", "Skill(sentry-skills:presentation-creator)", "Skill(sentry-skills:prompt-optimizer)", diff --git a/README.md b/README.md index 36be568..bcacb35 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,7 @@ Works with Claude Code, Cursor, Cline, GitHub Copilot, and other compatible agen | [iterate-pr](skills/iterate-pr/SKILL.md) | Iterate on a PR until CI passes and actionable review feedback is addressed. | | [presentation-creator](skills/presentation-creator/SKILL.md) | Create data-driven presentation slides using React, Vite, and Recharts with Sentry branding. | | [pr-link-issue](skills/pr-link-issue/SKILL.md) | Append a GitHub issue link and its Linear ticket to the current PR's description. | +| [pr-screenshots](skills/pr-screenshots/SKILL.md) | Capture real-product UI screenshots, lay them out as before/after tables in PR descriptions, and upload them with `gh --attach`. | | [pr-writer](skills/pr-writer/SKILL.md) | Create, refresh, and rewrite pull request titles and descriptions following Sentry conventions. | | [prompt-optimizer](skills/prompt-optimizer/SKILL.md) | Optimize prompts with evals, model-family adapters, and exact external context references. | | [secret-serialization](skills/secret-serialization/SKILL.md) | Find secrets and tokens that leak through generated serialization such as dataclass repr, asdict, model_dump, JSON.stringify, logs, and tracing spans. | diff --git a/skills/claude-settings-audit/SKILL.md b/skills/claude-settings-audit/SKILL.md index 3955ad0..3fc185a 100644 --- a/skills/claude-settings-audit/SKILL.md +++ b/skills/claude-settings-audit/SKILL.md @@ -159,6 +159,7 @@ If this is a Sentry project (or sentry-skills plugin is installed), include: "Skill(sentry-skills:gha-security-review)", "Skill(sentry-skills:iterate-pr)", "Skill(sentry-skills:pr-link-issue)", + "Skill(sentry-skills:pr-screenshots)", "Skill(sentry-skills:pr-writer)", "Skill(sentry-skills:presentation-creator)", "Skill(sentry-skills:prompt-optimizer)", diff --git a/skills/commit/SKILL.md b/skills/commit/SKILL.md index b923cae..f50eadf 100644 --- a/skills/commit/SKILL.md +++ b/skills/commit/SKILL.md @@ -60,19 +60,3 @@ git commit -m "fix(api): Handle null response in user endpoint" \ -m "Return 404 when the user API finds a deleted account." \ -m "Fixes SENTRY-5678" ``` - -## Attaching Images to the Pull Request - -Since gh 2.99.0, `gh pr create` and `gh pr edit` upload images and videos with -`--attach '#'`, so attach screenshots directly rather than -asking the user to drag them in. If `gh` rejects `--attach` as an unknown flag, -ask the user to upgrade `gh`. A body reference to the local path, such as -`![Before](./before.png)`, is rewritten to the uploaded asset; without one, -the attachment is appended to the body. - -```bash -gh pr edit --attach './before.png#Before' --attach './after.png#After' \ - --body "... ![Before](./before.png) ![After](./after.png)" -``` - -Without a body flag, `gh pr edit --attach` keeps the existing body. diff --git a/skills/pr-screenshots/SKILL.md b/skills/pr-screenshots/SKILL.md new file mode 100644 index 0000000..a54a151 --- /dev/null +++ b/skills/pr-screenshots/SKILL.md @@ -0,0 +1,123 @@ +--- +name: pr-screenshots +description: Capture UI screenshots of the real product and lay them out in a pull request description as before/after tables, then upload them with `gh --attach`. +--- + +# PR Screenshots + +Screenshots in a PR are QA evidence. Each one shows what the real product +renders at one location, framed so a reviewer who has never opened the page can +tell what changed. + +## Step 1: List the Locations + +List every place in the product that the diff reaches: each page, drawer, +modal, or widget that renders a changed component, found by tracing its +callers and consumers. Then add the states the change affects at each of them, +such as loading, error, empty, hover, selected, menu open, expanded, scrolled, +narrow viewport, and light or dark theme. + +Each location and state gets its own row. Never group surfaces into one row: +"GridEditable, as in Discover, dashboards and insights" is three rows. + +## Step 2: Capture From the Real Product + +| Rule | Detail | +|------|--------| +| Real build only | Screenshot the running app on the PR branch for "after" and on the base branch for "before". Never screenshot a story, mock page, or harness built for the PR, unless the change is to that story. | +| No injected changes | Never inject CSS or change the DOM to imitate the change. Stubbing API responses is fine; the frontend code must be the real code. | +| Seed missing data | If a state needs data no account has, create it in a local environment. Do not report a location as impossible to screenshot until seeding has been tried, and then say exactly what failed. | +| Identical framing | Capture the same region at the same dimensions for both shots of a pair, even when one state is taller. Emulate a fixed viewport instead of resizing the user's own browser window. | +| Context | Keep the heading or label that names the thing, nearby rows or controls, and a little padding on every side. For a hover, focus, or open-menu state, keep the trigger and the overlay in one frame. When the bug only makes sense in its page, show more of the page. | +| No empty bands | Crop out browser chrome, app navigation, and regions the change does not touch. For full-width layouts, shrink the viewport instead of leaving wide blank space beside short content. | + +## Step 3: Redact and Audit + +1. Blur only private values: emails, usernames, customer or organization names + and slugs, IDs, and internal hostnames. Keep keys, labels, and non-sensitive + values readable, such as `browser.name` → `chrome`. +2. Paint over each region before blurring it so the blur can't be reversed. + Never use solid black or dark boxes, and never blur whole tables or rows. +3. Read every image before uploading it and check for credentials: tokens, auth + headers, cookies, signed URLs, secret keys, and webhook endpoints. Remove + real leaks. Do not treat a public client DSN as a leak. + +## Step 4: Choose the Table Shape + +| Change | Shape | +|--------|-------| +| Visual change to existing UI | **Changed** table, plus an **Unchanged** table for locations the diff reaches that render the same. | +| New page or feature | No before/after. A few "after" screenshots, each introduced by a short sentence. When the new UI appears in several locations, use a `Location \| Screenshot` table instead. | +| Alternatives with no "before", such as prototype treatments or design options | `Treatment \| Screenshot` table, one row per alternative. | +| A change that differs by theme | Add a light and a dark row for each location, such as `Explore › Logs, dark`. For new UI, use `Light \| Dark` columns instead. | + +Pixel-diff every before/after pair, then confirm by eye. Move a pair with no +visible difference, including anti-aliasing noise, to **Unchanged** with one +image. Leave out a table that would be empty. + +```markdown +**Changed** + +| Location | Before | After | +| --- | --- | --- | +| Explore › Logs | ![Explore › Logs before](./shots/logs-before.png) | ![Explore › Logs after](./shots/logs-after.png) | +| Explore › Logs, loading | ![Explore › Logs, loading before](./shots/logs-loading-before.png) | ![Explore › Logs, loading after](./shots/logs-loading-after.png) | + +**Unchanged** + +| Location | Before & After | +| --- | --- | +| Trace view › Logs | ![Trace view › Logs](./shots/trace-logs.png) | +``` + +Formatting rules: + +- Name the first column `Location`, or `Treatment` for alternatives. Never + leave its header empty. +- Write each location as a `›` breadcrumb of what the reader clicks through, + then a comma and the state: `Monitors › cron monitor details › Check-Ins, loading`. +- Use only the columns shown above. Never add a "What changed" or notes column; + the screenshots show the change. +- Give each image alt text naming its row and side, such as + `Explore › Logs before`. +- Label the tables with bold `**Changed**` and `**Unchanged**` lines. Do not add + an intro sentence describing where or how the screenshots were taken. +- Use Markdown tables by default, because `gh --attach` rewrites only Markdown + image references. Switch to an HTML table only when cells must span rows, such + as a light and dark row under one location + (`Location | Theme | Before | After` with `rowspan="2"` on the location cell). + Do not add `align` attributes. + +## Step 5: Upload With gh + +`gh pr create`, `gh pr edit`, `gh pr comment`, and the matching `gh issue` +commands accept `--attach '#'` (gh 2.99.0+). If `gh` rejects +the flag, ask the user to upgrade it. Do not push asset branches, commit +images, or upload through the browser's comment box. + +Write the body file with relative Markdown image references, then pass the same +paths to `--attach`. gh uploads each file and rewrites the matching reference +to the uploaded URL in place: + +```bash +gh pr edit 123 --body-file pr-body.md \ + --attach './shots/logs-before.png' --attach './shots/logs-after.png' +``` + +| Behavior | Detail | +|----------|--------| +| Paths | Resolve against the current directory, and must match the body's references exactly. | +| Unreferenced attachments | Get appended to the end of the body as `![alt](url)`. | +| No body flag | `gh pr edit --attach` keeps the existing body and appends the images. | +| HTML tables | `` is not rewritten. Attach first, copy the appended URLs into the `` tags, delete the appended lines, then write the final body with `--body-file`. | + +Before writing a body the user may have edited, fetch it again and compare it +with the version you last read. Keep the user's wording and table edits, swap in +only your rows and URLs, and keep the body's line endings. Removing an image +from the body does not delete the uploaded file. + +## Step 6: Keep Screenshots Current + +When a later commit changes what a screenshot shows, retake the affected pairs, +re-check which pairs belong in **Changed** and **Unchanged**, and update the +body without waiting to be asked. diff --git a/skills/pr-screenshots/SPEC.md b/skills/pr-screenshots/SPEC.md new file mode 100644 index 0000000..83af4e0 --- /dev/null +++ b/skills/pr-screenshots/SPEC.md @@ -0,0 +1,35 @@ +# PR Screenshots Specification + +## Intent + +Turn a UI change into screenshot evidence in a PR description. It is separate +from `commit` and `pr-writer`, so attaching images is not part of committing +and the table conventions can change without widening the PR prose guidance. + +## Out of Scope + +- PR titles and prose bodies (`pr-writer`). +- Commit messages (`commit`). +- Product-specific setup for running a local app, logging in, or seeding data. +- Browser automation tool setup. + +## Should Not Trigger For + +- PRs without visual changes. +- Screenshots for bug reports outside a PR or issue body. +- Design mockups. + +## Acceptance Gates + +- No synthetic stories or injected styles used as evidence. +- One row per location and state, never grouped. +- Pixel-identical pairs move to the Unchanged table. +- `Location` first-column header, and no "What changed" column. +- New pages get "after" screenshots only. + +## Known Limitations + +- Pixel diffs need an image tool in the environment. Without one, the agent + falls back to an eye check and should say so. +- Capturing the base branch may require rebuilding the app, which the skill + does not automate. From 87ab07c1727f0cdd892094ad9a042663bf7dd448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Ghoulberg=20=F0=9F=91=BB?= Date: Tue, 13 Oct 2026 09:11:08 -0400 Subject: [PATCH 5/8] Update skills/pr-screenshots/SKILL.md Co-authored-by: Erica Pisani --- skills/pr-screenshots/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/pr-screenshots/SKILL.md b/skills/pr-screenshots/SKILL.md index a54a151..3e1b8e2 100644 --- a/skills/pr-screenshots/SKILL.md +++ b/skills/pr-screenshots/SKILL.md @@ -1,6 +1,6 @@ --- name: pr-screenshots -description: Capture UI screenshots of the real product and lay them out in a pull request description as before/after tables, then upload them with `gh --attach`. +description: Capture screenshots of the Sentry UI in a web browser, lay them out in a pull request description as before/after tables, and upload them with `gh --attach`. --- # PR Screenshots From a2aae31b19018217a1df89a825461d15dbcbda3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 13 Oct 2026 09:13:50 -0400 Subject: [PATCH 6/8] remove 'Each location and state gets its own row.' --- skills/pr-screenshots/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/pr-screenshots/SKILL.md b/skills/pr-screenshots/SKILL.md index 3e1b8e2..d4c4593 100644 --- a/skills/pr-screenshots/SKILL.md +++ b/skills/pr-screenshots/SKILL.md @@ -17,7 +17,7 @@ callers and consumers. Then add the states the change affects at each of them, such as loading, error, empty, hover, selected, menu open, expanded, scrolled, narrow viewport, and light or dark theme. -Each location and state gets its own row. Never group surfaces into one row: +Never group surfaces into one row: for example, "GridEditable, as in Discover, dashboards and insights" is three rows. ## Step 2: Capture From the Real Product From 76a05dbd09828f6754df7f28eb2504f17f5e6e12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 13 Oct 2026 09:14:41 -0400 Subject: [PATCH 7/8] add when_to_use --- skills/pr-screenshots/SKILL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/skills/pr-screenshots/SKILL.md b/skills/pr-screenshots/SKILL.md index d4c4593..5d63a43 100644 --- a/skills/pr-screenshots/SKILL.md +++ b/skills/pr-screenshots/SKILL.md @@ -1,6 +1,7 @@ --- name: pr-screenshots description: Capture screenshots of the Sentry UI in a web browser, lay them out in a pull request description as before/after tables, and upload them with `gh --attach`. +when_to_use: When a pull request intentionally modifies the frontend such that users would see a difference. --- # PR Screenshots From c93fb1e82454278a0caeb08e062a75a2e18feebb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 13 Oct 2026 09:16:26 -0400 Subject: [PATCH 8/8] remove 'then confirm by eye' --- skills/pr-screenshots/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/pr-screenshots/SKILL.md b/skills/pr-screenshots/SKILL.md index 5d63a43..8ddd91c 100644 --- a/skills/pr-screenshots/SKILL.md +++ b/skills/pr-screenshots/SKILL.md @@ -52,7 +52,7 @@ Never group surfaces into one row: for example, | Alternatives with no "before", such as prototype treatments or design options | `Treatment \| Screenshot` table, one row per alternative. | | A change that differs by theme | Add a light and a dark row for each location, such as `Explore › Logs, dark`. For new UI, use `Light \| Dark` columns instead. | -Pixel-diff every before/after pair, then confirm by eye. Move a pair with no +Pixel-diff every before/after pair. Move a pair with no visible difference, including anti-aliasing noise, to **Unchanged** with one image. Leave out a table that would be empty.