feat(web): real per-commit OG card on cold-cache unfurls (#53)#55
Conversation
Bot-driven unfurls hit the deferred path exactly when a link is first shared — the moment its cache is most likely cold — and got the generic placeholder OG image instead of the per-commit card from #8. Point the bot-deferred commit path at the dynamic web-og image URL, built from the route params (repo + already-validated sha) with no resolved result. web-og resolves the commit itself (cache-first then findRelease via the relay-aware provider) and degrades to its own placeholder when it can't, so unfurl content is strictly better with the failure path already handled on the web-og side. Scope: the commit permalink path only. The PR-deferred path has no merge SHA pre-compute and web-og has no PR->image route, so it can't point at a dynamic per-commit card without computing — left on the placeholder. Tradeoff (per #53, human approved "do 1"): crosses the "bots never trigger a compute" line — one findRelease per unique cold permalink, on the web-og image fetch (a URL already public + already computing on a direct hit), cached 30 min after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
In-house adversarial review (Copilot quota-blocked on this repo, so no automated review landed — per review-policy I run the skeptical pass myself): Verdict: mergeable, no findings. CI green across the full matrix (test ubuntu/windows × node 20/22, a11y, osv, gitleaks, shell+workflow lint — 8/8). What I checked:
Human-merge-only from here (the loop never merges). |
|
How can I test this? |
…url) The dynamic OG image (`/r/:owner/:repo/c/:sha.png` and the federated `/h/...` variant) served HTTP 200 `image/png` with an EMPTY body for every resolved result — so every social unfurl that used the real per-commit card (issue #8's whole feature) showed a blank image. The placeholder branch was unaffected, which is why the liveness probe (it only checks `placeholder.png`) and any status/content-type check were blind to it. Root cause: satori (inside workers-og) throws "Expected <div> to have explicit display:flex ... if it has more than one child node" when it renders `<div>commit {sha}</div>` — that JSX is TWO children (the literal "commit " and the {sha} expression) on a div with no `display: flex`. The throw happens mid-stream, AFTER the 200 + headers are flushed, so the response is a valid-looking 200 with a 0-byte body. Reproduced in the workerd runtime via `wrangler dev` (ResultCard → 0 bytes; fixed → 28KB valid PNG); the placeholder has only single-child text nodes, so it always rendered. Fix: make it a single text node (`{`commit ${sha}`}`). Guard: the test harness already captures the satori node tree, so this adds `satoriDisplayViolations()` — a structural assertion encoding satori's multi-child-needs-display rule over the captured tree, run for the released, not-yet-released, and placeholder cards. It fails on the old `commit {sha}` and passes after the fix, catching any future violation without a real WASM render. Note: PR #55 wires bot deferred unfurls onto this dynamic path — it should land on top of (or after) this fix, or bots get the blank card. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Three ways, fastest first. 1. The automated check (already in the PR). The contract this changes is "a bot-UA cold-cache commit unfurl points at the per-commit card, not the placeholder": The relevant case lives in 2. Locally, by hand. Bot detection is just a User-Agent allowlist ( Before this PR that prints 3. The real thing (post-merge). Since #56 is live, that per-commit image URL already renders a real PNG in prod — so once this merges, the end-to-end test is: paste a fresh commit permalink (one not recently looked up) into Slack/Discord/X and watch the unfurl show the actual release card instead of the generic placeholder. Same as a curl if you don't want to spam a channel: The catch with #3: the commit has to be cold (the deferred path only fires on a cache miss), so use one you haven't looked up in the last 30 min. |
In-house adversarial review (Copilot is quota-blocked here, so no automated review lands)Skeptical pass over the diff — no findings, safe to merge. Walked the failure modes that could turn a freshly-shared link into a broken unfurl:
Tests: unit (3, incl. federated scheme + equivalence) + integration (positive Ready for your merge button. |
Closes #53 (human approved "Do 1").
What
Bot-driven unfurls hit the deferred path exactly when a link is first shared — the moment its cache is most likely cold — so they got the generic
placeholder.pngOG image instead of the per-commit card from #8.This points the bot-deferred commit path (
renderDeferredinresult.tsx) at the dynamic web-og image URL, built straight from the route params (repo + the already-validated sha) with no resolved result:ogImageUrlForCommit(repo, sha, ogBaseUrl)inog-meta.tsx(shares the URL-builder withogImageUrl, so it produces the same URL a resolved result would — covered by a test),imageOverrideprop onOgMeta/ogImageOverrideonLayoutthat wins over the result-derived image,web-og already resolves the commit itself (cache-first, then
findReleasevia the relay-aware provider, single-flighted, 30-min cache) and degrades to its own placeholder on a miss/503. So the unfurl content is strictly better, with the failure path already handled on the web-og side.Scope note (why PRs aren't included)
Only the commit permalink path. The PR-deferred path (
pr.tsx) has no merge SHA before compute, and web-og has no PR→image route — so it can't point at a dynamic per-commit card without computing. Left on the placeholder, as today.The tradeoff (already accepted on #53)
Crosses the explicit "bots never trigger a compute" line: one
findReleaseper unique cold permalink, on the web-og image fetch (a URL that's already public and already computes on a direct hit), cached 30 min after. Not a new attack surface.Tests (TDD)
og-meta.test.ts:ogImageUrlForCommitmirrors the GitHub + federated schemes (sha shortened to 7) and equals the resolved-result URL.integration.test.ts: the Slackbot deferred commit path now emitsog:image = …/r/facebook/react/c/abc1234.pngand not/placeholder.png.Full gate (
scripts/validate.sh) green locally: build, typecheck, 193 tests, a11y contrast, lint, gitleaks, publint, osv (no High/Critical).🤖 Generated with Claude Code