Skip to content

fix(web-og): real per-commit OG card rendered a 0-byte PNG (blank unfurl)#56

Merged
lukaso merged 1 commit into
mainfrom
worktree-fix-og-resultcard-empty-png
Jun 28, 2026
Merged

fix(web-og): real per-commit OG card rendered a 0-byte PNG (blank unfurl)#56
lukaso merged 1 commit into
mainfrom
worktree-fix-og-resultcard-empty-png

Conversation

@lukaso-bot

Copy link
Copy Markdown
Collaborator

What

The dynamic OG image — /r/:owner/:repo/c/:sha.png and the federated /h/:host/r/:projectPath/c/:sha.png — served HTTP 200 image/png with an empty (0-byte) 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 fine.

Confirmed live before the fix (read-only):

URL branch bytes valid PNG
/r/honojs/hono/c/f82aba… (→ v4.12.11) real result 0
/h/gitlab.gnome.org/r/GNOME%2Fglib/c/0a90037… (→ 2.88.2) real result 0
/r/facebook/react/c/<bad sha> placeholder 14662
/placeholder.png placeholder 11757

It was invisible to the liveness probe (it only checks placeholder.png) and to any status/content-type check, because the failure is a 200 with the right headers and an empty body.

Root cause

satori (inside workers-og) throws:

Expected <div> to have explicit "display: flex" or "display: none" if it has more than one child node.

<div>commit {sha}</div> is two child nodes — the literal "commit " and the {sha} expression — on a div with no display: flex. satori throws mid-stream, after the 200 + headers are already flushed, so the body comes back empty. Reproduced in the workerd runtime via wrangler dev (ResultCard → 0 bytes; fixed → 28 KB valid PNG). The placeholder card has only single-child text nodes, so it always rendered.

Fix

One line: make it a single text node — {commit ${sha}}.

Regression guard (TDD)

The test harness already captures the satori node tree, so this adds satoriDisplayViolations() — a structural assertion encoding satori's "multi-child element needs an explicit 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.

Verification

  • wrangler dev real render: ResultCard 0 → 28618 bytes, valid PNG, no satori error.
  • Full gate green locally (scripts/validate.sh): build, typecheck, all package tests (incl. the 3 new web-og cases), a11y contrast, biome lint, shellcheck, actionlint, gitleaks, publint; osv clean (only the documented warn-only js-yaml 3.14.2 Medium).

Coordination

PR #55 wires bot deferred unfurls onto this exact dynamic path. This should land on top of (or before) #55, otherwise #55 switches bot unfurls from the working placeholder to the blank dynamic card.

🤖 Generated with Claude Code

…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>
@lukaso-bot

Copy link
Copy Markdown
Collaborator Author

In-house adversarial review (Copilot is quota-blocked on this repo, so no automated review lands — per review-policy.md I run a skeptical in-house pass before calling a PR mergeable; this is a prod-fix PR, so it doesn't get to skip review).

Verdict: ✅ approve — merge-ready. Skeptical pass found no blocking issues.

What I checked, and why each concern cleared:

  1. Is the fix actually the root cause? Yes. commit {sha} in JSX is two children (the literal "commit " and the {sha} expression). Satori throws "expected <div> to have explicit display: flex… if it has more than one child" — and because the throw happens mid-stream after the 200 image/png header is flushed, the body comes back 0 bytes. The template-literal {commit ${sha}} collapses to one string child. No visual change (same rendered text).

  2. Is the test vacuous? No — this was my main worry. vi.mock('workers-og') (test L12-19) captures the exact node handed to ImageResponse, and index.tsx:86-88 passes ResultCard(result) straight into new ImageResponse(node, …). So satoriDisplayViolations() walks the real render tree, not a stand-in.

  3. Is the fix complete (no other multi-child violations)? Yes. The assertion runs over the whole captured tree for all three branches (released / not-yet / placeholder), not just the one div. CI green (8/0) + local pnpm --filter @released/web-og test (10/10) ⇒ no other element in the tree trips satori's rule.

  4. Does the walker encode satori's rule correctly? childCount counts non-empty strings, numbers, and elements; flags any element with >1 child whose style.display is neither flex nor none. Matches satori's actual constraint, and the prod evidence (0-byte → 28 KB after fix, cycle 61) validates the encoding. All styling here is inline style objects, so props.style.display is the right place to look.

  5. Base freshness: PR head 18db7a4 is off origin/main 29e3498, which is still current — no rebase needed.

This was a day-one latent iceberg: a shipped feature (#8 federated OG) blank in every real per-commit unfurl, invisible to status/content-type checks. The fix is minimal and TDD-guarded, and the live dynamic OG render probe + this CI guard close the observability gap. Recommend merging #56 before #55 (#55 wires bots onto this same render path).

@lukaso
lukaso merged commit 8c87e34 into main Jun 28, 2026
8 checks passed
@lukaso
lukaso deleted the worktree-fix-og-resultcard-empty-png branch June 28, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants