jetpack-screenshot skill: attach images with gh --attach instead of pushing a screenshots ref

[Nikschavan]

Fixes N/A

Proposed changes

• The jetpack-screenshot skill published images by force-pushing a screenshots/<branch> ref to origin. Every run left a branch on the public repository that nothing merged or cleaned up.
• The skill now uploads the images with gh pr edit --attach (or gh pr create --attach). GitHub stores each file as a user-attachments asset and rewrites the ./before-<slug>.png references in the body to the uploaded URLs, so no branch is pushed.
• Because --attach uploads only while it posts, the skill now writes the screenshots block into the PR body itself instead of printing markdown to paste. The block sits between two marker comments, so a second run replaces it instead of adding a second copy.
• For review and inline comments, which have no --attach, the skill mints asset URLs through the user-attachments upload endpoint without posting anything.
• Capture, compositing and the Jurassic Ninja steps are unchanged.

Related product discussion/links

• N/A

Does this pull request change what data or activity we track or use?

No.

Testing instructions

• Check out this branch and confirm gh pr edit --help lists --attach.
• On a draft PR of your own, run /jetpack-screenshot and confirm the Before/After table renders in the body with user-attachments image URLs.
• Run it a second time and confirm the body still has one screenshots block.
• Confirm git ls-remote origin 'refs/heads/screenshots/*' shows no new ref for your branch.

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
3. You can use GitHub's Reviewers functionality to request a review.
4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

[Nikschavan]

This step now writes the screenshots block into the PR body. Before, the skill only printed markdown to paste. The reason is that gh --attach uploads a file only when it posts the body or comment. There is no flag that only uploads and returns the URL. The marker comments let a second run replace the block, so the body does not collect copies.

[Copilot]

Copilot review overview

🟡 Changes recommended

The workflow can overwrite a PR body after a failed fetch, and attachment uploads are not reliably redirect-safe.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• Pipeline masks failed gh pr view and can overwrite PR body · New
• Allowlist missing commands and screenshot file write permission · New
• Raw upload is not redirect-safe and can lose image uploads · New

What changed in this PR

Replaces screenshot-ref publishing with GitHub user attachments and repeatable PR-body updates.

Changes:

• Uploads screenshots using gh --attach.
• Replaces marker-delimited screenshot blocks on reruns.
• Adds direct asset uploads for review comments.

File	Description
.agents/​skills/​jetpack-screenshot.md	Updates screenshot publishing and attachment workflows.

---

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

[xavier-lc]

Summary

This fixes the problem we found on #52629 and #52643. The skill stops pushing a screenshots/<branch> ref and hosts the images as user-attachments instead.

• Route 1 (gh --attach) is sound. It was merged in cli/cli#14183 and ships in gh v2.99.0 and later. It rewrites ./file.png references in the body and requires write access to the repo.
• Route 2 (upload with gh api) uploads fine, but its check of the result always fails. The skill then says to upload again, and uploads can't be deleted.
• Neither route says what to do with an image over the size limit.

Findings

1. [blocker] Route 2's URL check always fails, and the skill then says to upload again.

   • Where:

     jetpack/.agents/skills/jetpack-screenshot.md

     Lines 166 to 170 in ea67575

     	`curl -I` shows only the 302, so follow the redirect to check each URL before it goes in a body; anything but `200 image/png` is a failed upload, so mint it again:
     	```bash
     	curl -sL -o /tmp/asset.png -w '%{http_code} %{content_type}\n' <minted url> && file /tmp/asset.png

   • What happens: the check uses an anonymous curl -sL. GitHub serves an upload that no published body references yet only to authenticated requests; anonymous requests get 404.
   • Tested: I uploaded a PNG with the skill's own command (gh 2.45, gho_ token) and got a URL back. Anonymous requests returned 404 text/plain, and requests with the token returned 200 image/png. An image already referenced in a PR body returned 200 anonymously.
   • Impact: every route-2 upload looks like a failure, and "mint it again" repeats it, each time storing a copy that can't be deleted. This route triggers on gh older than v2.99 and always for review or inline comments.
   • Fix: authenticate the check with curl -sL -H "Authorization: Bearer $(gh auth token)" …. With that header, the download matched the uploaded file.

2. [suggestion] Route 2 saves the body with the command that silently fails on older gh.

   • Where:

     jetpack/.agents/skills/jetpack-screenshot.md

     Line 173 in ea67575

     Replace each `./before-<slug>.png` in the block with its minted URL, then post the comment, or run the step 7 command without the `--attach` flags.

     , which says to rerun step 7 without --attach, meaning gh pr edit --body-file.
   • What happens: route 2 exists for older gh. On gh 2.45 that command prints a "Projects (classic)" GraphQL warning and leaves the body unchanged, so the run reports success with no screenshots in the PR.
   • Fix: use gh api -X PATCH repos/Automattic/jetpack/pulls/$PR -F body=@body.md, then read the body back to confirm it changed.

3. [suggestion] The skill doesn't say what to do with a screenshot over 10 MiB.

   • Where:

     jetpack/.agents/skills/jetpack-screenshot.md

     Line 155 in ea67575

     Pass one `--attach` per PNG that the block references. If the PR does not exist yet, hand the block and the PNG paths to whatever creates the PR, which passes the same `--attach` flags to `gh pr create`. A top-level PR comment takes the same flags on `gh pr comment`.

   • Tested: the limit is 10 MiB on both routes; a 10,481,193-byte PNG was accepted and an 11.1 MiB one was rejected.
     • Route 1: gh refuses the file locally, before looking up the PR or uploading anything: images must be at most 10.0 MB, exit 1. The body is left untouched.
     • Route 2: the server returns 422 Validation Failed ("Yowza that's a big file… less than 10MB") and stores nothing. Following the skill's "mint it again" just repeats the 422.
   • When it happens: 1440×900 viewport shots are well under the limit. The realistic case is a side-by-side composite at HiDPI (5760×1800) of an image-heavy page.
   • Fix: one line in the skill: "If a PNG is over 10 MiB, downscale it or convert it to JPEG, or attach the before and after images separately instead of the composite."

4. [suggestion] A failed upload partway through goes unnoticed.

   • Where:

     jetpack/.agents/skills/jetpack-screenshot.md

     Lines 150 to 151 in ea67575

     	gh pr edit "$PR" -R Automattic/jetpack --body-file body.md \
     	--attach ./before-<slug>.png --attach ./after-<slug>.png

   • What happens: gh checks every file before uploading, so an oversized file can't cause this. A network error or rate limit partway through still can. gh stops at that file but writes the body if any earlier upload succeeded. The failed references keep their ./before-<slug>.png path and show as broken images; cli/cli#14181 says this is expected.
   • Fix: after the edit, read the body with gh pr view --json body and fail if it still contains ](./.

5. [suggestion] A lone start marker deletes the rest of the PR body.

   • Where:

     jetpack/.agents/skills/jetpack-screenshot.md

     Line 148 in ea67575

     awk '/<!-- jetpack-screenshot:start -->/{skip=1} !skip{print} /<!-- jetpack-screenshot:end -->/{skip=0}' original-body.md > body.md &&

   • What happens: if the jetpack-screenshot:end marker was edited out, the awk skips everything after the start marker, and gh pr edit saves the shortened body. The original stays in original-body.md, but nothing tells anyone to look there.
   • Fix: remove the block only when both markers are present; otherwise stop and ask.

6. [suggestion] allowed-tools pre-approves more than the skill needs.

   • Where:

     jetpack/.agents/skills/jetpack-screenshot.md

     Line 10 in ea67575

     allowed-tools: Bash(git rev-parse:*), Bash(git diff:*), Bash(mktemp:*), Bash(command -v:*), Bash(magick:*), Bash(gh pr view:*), Bash(gh pr edit:*), Bash(gh api:*), Bash(awk:*), Bash(curl:*), Bash(file:*), Bash(grep:*), Bash(cut:*), Bash(sort:*), Bash(cat:*), Bash(echo:*), Write

   • What happens: Bash(gh api:*) pre-approves any API write, and Bash(curl:*) allows any network request. With uploads that can't be undone, and finding 1's retry loop, a permission prompt before the upload is a useful brake.
   • Fix: narrow both patterns. The step 7 subshell starts with (, so it matches no pattern and prompts anyway, which is probably not deliberate.

7. [suggestion] The URL check writes to /tmp/asset.png.

   • Where:

     jetpack/.agents/skills/jetpack-screenshot.md

     Line 170 in ea67575

     curl -sL -o /tmp/asset.png -w '%{http_code} %{content_type}\n' <minted url> && file /tmp/asset.png

   • What happens: the path is shared, so parallel runs can overwrite each other's file.
   • Fix: use "$OUT/check.png".

Testing done

• Route 1: run with gh v2.101.0. The size check was confirmed against a PR number that doesn't exist, so nothing was posted. Reference rewriting and the write-access check were verified from gh's source and PR notes, not by running them. A full end-to-end run would need a PR to post to.
• Route 2: run with gh 2.45.0 against Automattic/jetpack. The upload, the anonymous-vs-authenticated responses, and the 422 for oversized files were all observed. Nothing was posted.

Verdict

Needs changes before merge. Route 2's URL check (finding 1) makes the skill upload copies that can't be deleted, in a loop. Findings 2–5 are about the skill reporting success, or failing with no next step, when the PR body isn't what it should be. The confidentiality check, the confirm-before-upload step, the privacy note, and the order of fallback routes all look fine.

[Nikschavan]

Thank you for testing both routes against the real endpoint. I applied all seven fixes:

• The minted-URL check now sends the gh auth token, and it writes to $OUT/check.png instead of /tmp.
• Route 2 saves the body with gh api -X PATCH and reads it back to confirm it changed.
• There is a line for PNGs over 10 MiB, and a read-back after the edit that fails if a ](./ reference is left.
• The old block is removed only when both markers are present; otherwise the skill stops and asks.
• allowed-tools now allows gh api only for the repository ID lookup and curl only in the authenticated form, so the upload and the body update ask for permission.

I kept the ( subshell in step 7 as it is. It always asks for permission, and I think that is useful right before an upload that cannot be deleted.

[xavier-lc]

Thanks, all seven findings are addressed in 56901e9. I reran the new URL check exactly as written against an asset I had uploaded, and it returns 200 image/png with a byte-identical file, so route 2 no longer treats a good upload as a failure.

One optional follow-up, not blocking: the marker rule in step 7 is prose only, so an agent that copies the command still gets the old behaviour. If the end marker was deleted by hand, the awk drops everything after the start marker and gh pr edit saves the shortened body. A guard inside the chain would stop it before the edit:

gh pr view "$PR" -R Automattic/jetpack --json body --jq .body > original-body.md &&
[ "$(grep -c 'jetpack-screenshot:start' original-body.md)" = "$(grep -c 'jetpack-screenshot:end' original-body.md)" ] &&
awk '…' original-body.md > body.md &&

[Nikschavan]

Thank you for checking the new URL check again. I added the marker guard to the step 7 command, so the chain now stops before gh pr edit when the start and end markers do not match. I tried it on a body with no markers, one with both, and one with only the start marker, and only the last one stopped.