SKILL.md
3,763 tokens · o200k_base · 15,229 bytes
Source excerpt starting at line 1.---name: lite-screenshotsdescription: Use when asked to add before/after screenshots of a Lite UI change to a pull request, when a PR touching apps/lite/ui needs its visual change shown, or when extending the screenshot catalogue in apps/lite/e2e/tests/screenshots.spec.ts. Captures both sides locally against seeded fixtures, publishes the surfaces that changed, and posts them to the PR.--- A reviewer reading a CSS diff cannot see the result. This captures the Lite UIwith the branch applied and again with it unapplied, and posts the surfaces thatdiffer to the pull request. Capture runs **on this machine**, not in CI. It was tried in CI and abandoned:under the bare X server on the Linux runners this Electron build paints a frameon load and not reliably afterwards, so several surfaces hung indefinitelywhether the capture went through Playwright or the DevTools protocol. The samespec passes here. The CI workflow only leaves a reminder. ## Before capturing: is the surface covered? Work this out first, by reading the diff — not by capturing and inferring it fromthe result. A run that reports "8 surfaces, 0 changed" looks identical whether thechange is invisible or whether nothing looked at it. 1. Identify which screen the change affects.2. Check it against the catalogue in `apps/lite/e2e/tests/screenshots.spec.ts` (currently: workspace sidebar, diff pane, branches tab, upstream tab, project picker, uncommitted rows, commit form, settings).3. **Covered** — capture, as below.4. **Not covered** — stop and ask the developer, offering the two lanes below. Either way it is their call, not one to make quietly inside a screenshot request. Tell them which screen is uncovered and which fixture would reach it. - **Add it to the catalogue** when the gap is a _screen_ — something anyone working nearby will change again. It edits a shared file and becomes permanent coverage for every later pull request. See "Extending the catalogue" below. - **Capture it ad hoc** when the gap is a _state_ — a conflicted commit mid-resolution, an empty list, one specific dialog — that no future pull request is likely to want. Nothing shared changes and nothing is left behind. See "One-off capture" below. Prefer the catalogue when it is a close call. The ad-hoc lane photographs the change once and protects nothing afterwards, and a catalogue that stops growing makes every later request re-derive the same fixtures. If they decline both, **do not post a comment**. A normal comment would report"N surfaces, 0 changed", which asserts the change is not visual when in truth itwas never photographed. Say plainly that the affected screen is not in thecatalogue and no screenshots were taken. ## Changes the capture cannot see Some diffs are invisible to this harness for reasons that have nothing to do withthe change being non-visual. Rule these out before reading a "0 changed" resultas an answer: - **Scrollbars and their gutter.** macOS hides scrollbars unless a mouse is attached, and a styled `::-webkit-scrollbar` does _not_ override that — the scroller simply reserves no gutter. On a machine set to "When scrolling", any change to the track, thumb, gutter width, or a separator meeting the gutter captures identically on both sides. Set System Settings → Appearance → Show scroll bars → **Always** for the run, and say in the comment that you did.- **Hover, focus, and drag states** are not entered by the spec.- **Anything behind a scroll position** — surfaces are captured at the top. ## Prerequisites - `cargo build -p but` once, giving `target/debug/but`.- The branch under test applied, and **not yet merged** — the base side comes from unapplying it. Check `but status` first.- **Run every capture through `env -u ELECTRON_RUN_AS_NODE`.** An agent hosted inside VS Code (itself Electron) inherits `ELECTRON_RUN_AS_NODE=1`, which makes the Electron the test launches behave as plain Node. Every surface then fails with `Error: Process failed to launch!` and nothing says why — the binary is fine, and running it by hand from another shell works. Check with `env | grep ELECTRON`. ## The flow Run every command from the repository root. ### 1. Capture the branch as it stands ```console$ env -u ELECTRON_RUN_AS_NODE SCREENSHOT_OUT=head BUT=$PWD/target/debug/but \ pnpm -F @gitbutler/lite test:e2e screenshots.spec.ts``` Output lands in `apps/lite/e2e/screenshots/head/` (gitignored). ### 2. Capture the base ```console$ but unapply <branch>$ but status # confirm the branch is gone from the workspace$ env -u ELECTRON_RUN_AS_NODE SCREENSHOT_OUT=base BUT=$PWD/target/debug/but \ pnpm -F @gitbutler/lite test:e2e screenshots.spec.ts$ but apply <branch>``` **Check the unapply took effect.** It silently does nothing on a stale id or analready-merged branch, and both runs then capture identical code — which looksexactly like "this change is not visual". If every pair matches in step 3, assumethis went wrong before concluding anything about the change. Re-apply immediately, before anything else can fail and leave the workspace shorta branch. **Stacked branches take the whole stack down.** `but unapply` on a branch that ispart of a stack unapplies every branch in it, including ones this capture dependson — unapplying a stacked change also removes the spec, and the base run then hasno tests to execute. Check `but status` first. When the branch is stacked, capturethe base by reverting the change in the working tree instead: 1. Edit the changed files back to their pre-change state by hand.2. Capture into `base`.3. `but discard <file-id>` to restore the committed version. Confirm the restore: the change must be back before anything else happens. ### 3. Select what changed ```console$ node apps/lite/e2e/compare-screenshots.mjs \ apps/lite/e2e/screenshots/base \ apps/lite/e2e/screenshots/head \ /tmp/publish \ "https://raw.githubusercontent.com/<owner>/<repo>/pr-screenshots/pr-<number>/<short-sha>" \ /tmp/section.md``` It prints `total=`, `changed=`, `added=`, stages only the differing pairs into`/tmp/publish`, and writes the comment body. Surfaces that did not change arebyte-identical, so they are folded into a collapsed list rather than shown. ### 4. Publish the images Images go on an orphan `pr-screenshots` branch, under `pr-<number>/<short-sha>/`.A per-commit directory matters: reusing one path lets GitHub's image proxy servethe previous run's screenshots from cache, which reads as "my change did nothing". Write them with the git data API rather than a local clone. There is nothing toclone, nothing to clean up, and no local `git` writes — which agents are oftennot permitted to make outside the project, and which cannot touch the GitButlerworkspace by accident. ```console$ REPO=<owner>/<repo>; DIR=pr-<number>/<short-sha>$ HEAD_SHA=$(gh api repos/$REPO/git/refs/heads/pr-screenshots --jq '.object.sha')$ BASE_TREE=$(gh api repos/$REPO/git/commits/$HEAD_SHA --jq '.tree.sha') # One blob per image. base64 must be stripped of newlines.$ jq -n --rawfile c <(base64 -i /tmp/publish/<name>.png) \ '{content: ($c | gsub("\n";"")), encoding: "base64"}' > /tmp/blob.json$ BLOB=$(gh api -X POST repos/$REPO/git/blobs --input /tmp/blob.json --jq '.sha') # One tree and one commit for all of them, then move the ref.$ jq -n --arg t "$BASE_TREE" --arg b "$BLOB" --arg d "$DIR" '{base_tree: $t, tree: [ {path: ($d + "/<name>.png"), mode: "100644", type: "blob", sha: $b}]}' > /tmp/tree.json$ TREE=$(gh api -X POST repos/$REPO/git/trees --input /tmp/tree.json --jq '.sha')$ jq -n --arg t "$TREE" --arg p "$HEAD_SHA" \ '{message: "Screenshots for #<number>", tree: $t, parents: [$p]}' > /tmp/commit.json$ COMMIT=$(gh api -X POST repos/$REPO/git/commits --input /tmp/commit.json --jq '.sha')$ gh api -X PATCH repos/$REPO/git/refs/heads/pr-screenshots -f sha="$COMMIT"``` When the branch does not exist yet, omit `base_tree` and `parents`, then createthe ref with `POST git/refs -f ref=refs/heads/pr-screenshots -f sha=$COMMIT`. **Fetch one raw URL before posting.** A comment full of broken images is worsethan no comment, and the failure is invisible until someone opens the pullrequest: ```console$ curl -s -o /dev/null -w '%{http_code}\n' \ https://raw.githubusercontent.com/<owner>/<repo>/pr-screenshots/<dir>/<name>.png``` ### 5. Post to the pull request Upsert a single comment rather than stacking one per run — find a previouscomment starting with `<!-- lite-screenshots -->` and PATCH it, else POST. ```console$ jq -Rs '{body: .}' /tmp/section.md > /tmp/payload.json$ gh api "repos/<owner>/<repo>/issues/<number>/comments" --paginate \ --jq 'map(select(.body | startswith("<!-- lite-screenshots -->"))) | first | .id // empty'$ gh api -X PATCH "repos/<owner>/<repo>/issues/comments/<id>" --input /tmp/payload.json``` Do not edit the pull request description: rewriting text a human owns risksclobbering their concurrent edits. ### 6. Swap the label `screenshots needed` is the request; `screenshots` is the record. Move the pullrequest from one to the other once the comment is posted, so the label says whatis true: ```console$ gh pr edit <number> --remove-label "screenshots needed" --add-label "screenshots"``` Only after a comment with images actually went up. A pull request labelled`screenshots` with none attached is worse than an unlabelled one — it tells areviewer the change has been shown when it has not. If you declined to post (anuncovered surface, a capture that failed), leave `screenshots needed` where it is. `screenshots needed` is asked for once per pull request and never re-applied, sothe swap is final — a later push does not put it back, and a set posted here isnot a claim about code pushed after it. Recapture when the surfaces move; thecommand above is idempotent either way. If you ever post images that are **not** captures of the running app — a pagerendered from the branch's CSS, say, when the harness cannot be run — label themas such in the comment and mark it `<!-- lite-screenshots-replica -->` instead.A reviewer assumes a screenshot is a photograph of the app; anything else has tosay so, and the different marker keeps a later real run from overwriting it. ## Before finishing - **Look at the images**, do not just trust exit codes. A green run proves files were written, not that they show the surface.- Confirm the workspace is whole: `but status` should show the branch applied and no stray uncommitted changes. ## One-off capture For a state the catalogue should not carry permanently. The run command filtersby filename, so any spec whose name contains `screenshots` is picked up by thesame command and writes into the same `SCREENSHOT_OUT` directory — the comparestep cannot tell the difference. Write `apps/lite/e2e/tests/screenshots-adhoc.spec.ts`, importing the sharedhelpers rather than copying them (every workaround in them was earned, and a copyloses the comments explaining why): ```tsimport { enabled, openProject, shoot } from "../screenshot-helpers.ts";import { test } from "../test.ts"; test.describe("screenshots", () => { test.skip(!enabled, "set SCREENSHOT_OUT to capture screenshots"); test.describe.configure({ timeout: 180_000 }); test.use({ scenario: "project-in-single-branch-three-branch-stack.sh" }); test("<surface>", async ({ appWindow }) => { await openProject(appWindow); // ...reach the state... await shoot(appWindow, "<surface-name>", "<selector>"); });});``` The same fixture and selector rules as the catalogue apply — see below. Then capture both sides as usual, and: - **Delete the file** once the base run is done. It is untracked and not ignored, so left behind it shows up as a stray change in `but status` and eventually in someone's commit. The "confirm the workspace is whole" check at the end is what catches this.- **Say in the comment** that the surface was captured ad hoc and is not covered going forward, so nobody reads it as a regression guarantee. **Never run the two specs in one command.** The config sets `fullyParallel`, andthe catalogue spec wipes `SCREENSHOT_OUT` in a `beforeAll` — run concurrently, thatwipe lands in the middle of the ad-hoc capture and deletes it. What survives is aplausible-looking directory missing exactly the surface you were asked for. The trailing argument is a **substring match on the file path**, so a bare`screenshots` selects the ad-hoc spec as well and puts you straight into that race.Name the file: `screenshots.spec.ts` for the catalogue, `screenshots-adhoc` for thead-hoc spec. Steps 1 and 2 above already do. Capture the ad-hoc surface **alone**, filtering to its filename, into its ownoutput directory on each side: ```console$ env -u ELECTRON_RUN_AS_NODE SCREENSHOT_OUT=adhoc-head BUT=$PWD/target/debug/but \ pnpm -F @gitbutler/lite test:e2e screenshots-adhoc``` Its own directory also avoids inheriting stale PNGs from an earlier catalogue run,since only the catalogue spec cleans up after itself. Then compare`adhoc-base` against `adhoc-head` as in step 3. When a change touches both a covered screen and an uncovered state, run thecatalogue first and the ad-hoc spec second, in separate commands, pointing both atthe same directory — that order is safe because only the first one wipes. ## Extending the catalogue Only after the developer has agreed (see the top of this document). Add thesurface to `apps/lite/e2e/tests/screenshots.spec.ts`, then capture as usual —the same run proves the new surface works and shows the change. Coverage appliesto every later pull request, not just this one. ```tstest.describe("<area>", () => { test.use({ scenario: "<fixture>.sh" }); test("<surface>", async ({ appWindow }) => { await openProject(appWindow); await goToTab(appWindow, "branches"); // when it lives on another tab await shoot(appWindow, "<surface-name>", "<selector>"); });});``` - `<surface-name>` becomes the filename and the heading a reviewer reads: name it `commit-form`, not `outline-panel-2`.- Only fixtures calling `"$BUT" setup` register a project; the rest leave the app on "Select a project." and every capture fails. Known good: `project-in-single-branch-three-branch-stack.sh`, `project-with-remote-branches.sh`, `project-with-conflicting-commits.sh`.- If no fixture reaches the state, build it in the test — `uncommitted` writes files into `testEnvironment.workdir/local-clone` and reloads.- Prefer a stable id for the clip (`#outline-panel`, `#details-panel`), else a CSS-module prefix or an ARIA hook. Never nth-child or a generated class.- One screenshot per test, taken after a fresh load. This costs nothing here and keeps the spec usable if CI capture is ever revisited. ## Out of scope - **Native context menus** cannot be captured; they are OS windows.- **Dark mode** is not covered — the harness seeds `theme: "light"`.- **States needing a real remote or credentials** cannot be fixtured.
Discovery context
Discovered by repository scan. No exact path reference found in the snapshot’s root AGENTS.md.