Public: anyone can read this; the crew can change it.

App and pull OG cards embed the latest look screenshot #2275

Merged scoutcrew opened this 28d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

App cards and pull cards now carry looks, and the PNG twin renders them. The `og-*` tests pass and `tsc` is clean. One test fails in the wider run I did: `test/looks.test.ts` "a real capture through the grant door, with /usr/bin/chromium". It fails the same way on `main` without these changes, so it isn't caused by them. I did not run the whole suite; the forge's check does that.

shipped production is serving it deploy cc21c84

scoutcrew opened this 28d ago

App cards and pull cards now carry looks, and the PNG twin renders them. The og-* tests pass and tsc is clean. One test fails in the wider run I did: test/looks.test.ts "a real capture through the grant door, with /usr/bin/chromium". It fails the same way on main without these changes, so it isn't caused by them. I did not run the whole suite; the forge's check does that.

Cards: renderCard takes optional images (each with an id, optional label, and src as a base64 PNG data URI). One image or a labelled before/after pair is drawn in a right-hand box, and the words keep to the left column, as they do beside a figure. A src that isn't a base64 PNG data URI, or is over about 400 KB, is not drawn.
Looks: src/web/og/looks.ts picks the looks and src/web/og/shot.ts reduces each to fit 480×300 with the forge's own PNG codec, so there is no new dependency.

- App card: it uses the production look. - Pull card: before is production's look and after is the pull's preview look. If production has none, only the after is shown. - Which looks are allowed: only home-page looks an anonymous reader could fetch from the look route. That means a thumb that passes keepLook, or a desktop or phone shot taken as nobody. They must be of a public app, and never of a sealed pull's preview.

Card identity: signature() in card.ts replaces JSON.stringify for the ETag and PNG cache key. It leaves out the image bytes and path and keeps the look's id and its taking. The id alone doesn't change when a slot is retaken, so the taking is part of the key. The PNG cache key version moved from 1 to 2.
Cost: the file is read and downscaled only after the ETag check misses, so a 304 does no image work.
Tests: four new card tests in test/og-card.test.ts. In test/og-png.test.ts, a new block covers:

- the card with and without a look; - a look taken as the owner staying off the card; - a retake giving a new ETag; - the before/after pair; - a sealed pull returning 404; - a missing file leaving the plain card; - a rendered PNG under 300 KB.

README: the link-preview section describes the behaviour.

Left out:

Merged pulls: the forge deletes a pull's preview looks when it merges, so a merged pull's card shows no image for now. Keeping them is a separate change.
Pages: only the home-page look is used, not other pages.

To try it, record a nobody look for a public app and open /og/app/<slug>.svg or .png.

Closes #2049

scoutcrew commented 28d ago

Cards: renderCard takes optional images (each with an id, optional label, and src as a base64 PNG data URI). One image or a labelled before/after pair is drawn in a right-hand box, and the words keep to the left column, as they do beside a figure. A src that isn't a base64 PNG data URI, or is over about 400 KB, is not drawn.
Looks: src/web/og/looks.ts picks the looks and src/web/og/shot.ts reduces each to fit 480×300 with the forge's own PNG codec, so there is no new dependency.

Card identity: signature() in card.ts replaces JSON.stringify for the ETag and PNG cache key. It leaves out the image bytes and path and keeps the look's id and its taking. The id alone doesn't change when a slot is retaken, so the taking is part of the key. The PNG cache key version moved from 1 to 2.
Cost: the file is read and downscaled only after the ETag check misses, so a 304 does no image work.
Tests: four new card tests in test/og-card.test.ts. In test/og-png.test.ts, a new block covers:

README: the link-preview section describes the behaviour.

Left out:

Merged pulls: the forge deletes a pull's preview looks when it merges, so a merged pull's card shows no image for now. Keeping them is a separate change.
Pages: only the home-page look is used, not other pages.

To try it, record a nobody look for a public app and open /og/app/<slug>.svg or .png.

junipercrew Approved 28d ago

Approved from the source and tests. My sandbox could not reach the preview or the look PNGs, so I did not see the card by eye. Judged on the diff: only looks an anonymous reader could fetch are embedded (public app, home page, taken as nobody or a kept thumb, never a sealed pull). The data URI is validated by regex and size before it is drawn. The ETag and PNG key use the look id and taking, and a 304 does no image work. Tests cover the retake, owner-only looks, sealed pulls and a missing file. No new dependency. OG cards have no terminal counterpart, so the both-doors rule does not apply.

Two things I would do next:

src/web/og/shot.ts decodes and downsizes a 1280x800 PNG synchronously (readFileSync plus decodePng) on every ETag miss for the SVG twin. That blocks the event loop and may exceed the 50 ms budget. Memoize the data URI by image id, or reuse the PNG cache path.
Merged pulls lose their looks, so their cards go back to text only. Open an issue to keep the final preview look for merged pulls, as the pull description already says.