fix(web-og): cache the OG card by terminality, not by result presence (#151) by lukaso-bot · Pull Request #158 · lukaso/released

lukaso-bot · 2026-08-27T11:02:19Z

Closes #151.

The defect

renderImage in packages/web-og/src/index.tsx picked its cache lifetime with:

const cacheControl = cacheOverride ?? (result ? LONG_CACHE : SHORT_CACHE);

That asks whether a result came back, not whether the answer it renders can still change. Two shapes are real LookupResults that are still in motion, and both took the 24h cache:

firstRelease: null — ResultCard renders it as the literal string "not yet released". This is the one card whose whole job is to flip when the release lands. A commit shared an hour before its release (the most likely thing to be freshly linked on Slack or X) unfurls as unreleased, and every crawler keeps that PNG for a day after the release ships.
a soft-deadline partial — carries a galloped firstRelease that is not confirmed to be the earliest one. /internal still lets a partial produced by its own load through (see fix(web): align /internal/* result cache with the public routes (#143) #144 round 7), so this arm is reachable on production today.

badge.ts gets this right — the liveness probe asserts both arms (released → max-age=86400, not-yet → max-age=300). web-og did not. The invariant is already written down in CLAUDE.md and priorities.md; it was just stated for badges alone, so the identical bug shipped on a second surface.

The fix

Key on terminality — the same test resolve.ts's hardTtlFor() applies on the web side:

function isTerminal(result: LookupResult | null): boolean {
  return result != null && result.firstRelease != null && !result.partial;
}
const cacheControl = cacheOverride ?? (isTerminal(result) ? LONG_CACHE : SHORT_CACHE);

Explicit cacheOverrides are untouched: the static /placeholder.png still opts into LONG_CACHE (byte-identical every request, version-busted by URL), and the deploy-window version gate still forces SHORT_CACHE.

Evidence — each guard watched red on the real defect

Run before the src/index.tsx change, against the defect on main:

× unreleased commit (firstRelease null): says "not yet released", NO SHIPPED, NO date
× not-yet-released result is SHORT-cached so the card flips when the release lands
× partial result is SHORT-cached even though it carries a firstRelease
AssertionError: expected 'public, no-transform, max-age=86400, …' to be 'public, no-transform, max-age=60'
Tests  3 failed | 45 passed (48)

That is the bug in this PR's own title, not an adjacent failure mode. The not-yet guard also asserts the card's rendered not yet released copy, so it pins the flippable shape rather than some unrelated result.

The complement is not vacuous. terminal released result keeps the LONG cache guards the other direction (a blanket short-cache would re-rasterize every settled card once a minute — ~700ms of satori+resvg wasm each time). Mutating isTerminal to return false:

Tests  11 failed | 37 passed (48)
   × terminal released result keeps the LONG cache
   + 10 pre-existing long-cache tests

One assertion deleted rather than shipped. I first wrote a /placeholder.png?v=<current> → LONG complement, then found routing.test.ts:325 already asserts exactly that. It rejected nothing the existing test doesn't, so it's gone instead of padding the file.

A test that codified the bug is corrected. unreleased commit (firstRelease null) asserted the 24h header with the comment "A long-cache header still applies — we DID get a result, it's just unreleased." That is the defect written down as intent; the assertion and its comment now match the invariant.

Scope

packages/web-og only — no overlap with #144, which rewrites packages/web/src/routes/internal.ts. Deliberately not included:

OG unfurls of a not-yet-released commit serve the neutral placeholder permanently (/internal 503s on NotYetReleasedError) #150 (/internal 503s on NotYetReleasedError) — the reason the firstRelease: null arm is not yet reachable through /internal on GitHub commits. This PR is the renderer half and is a prerequisite for OG unfurls of a not-yet-released commit serve the neutral placeholder permanently (/internal 503s on NotYetReleasedError) #150; landing OG unfurls of a not-yet-released commit serve the neutral placeholder permanently (/internal 503s on NotYetReleasedError) #150 without it would just swap a permanent placeholder for a 24h-pinned "not yet released".
Deadline-heavy repos get a permanently blank OG card and one full traversal per minute after #144 #156 (a deadline-heavy repo's blank card / traversal-per-minute) — the partial arm here is the "short-cache it via a response header" half of that remedy, but Deadline-heavy repos get a permanently blank OG card and one full traversal per minute after #144 #156's other half is /internal's refusal policy, which is fix(web): align /internal/* result cache with the public routes (#143) #144's file.
/internal/* blocks on findRelease with no deadline, so the FIRST unfurl of a large repo hands the crawler the placeholder #152, OG cards key on a 7-char SHA while permalinks key on the full SHA — the first unfurl never reuses the warm slot #147, partial-with-a-gallop-hit is cached as terminal: 30-day TTL, never revalidated #155 — separate code paths, separate review surfaces.

#141's note in the issue is stale: it is merged, so there is no rebase conflict on these lines.

Gate

pnpm lint 0 · pnpm typecheck 0 · pnpm build 0 · pnpm test — core 230, cli 45, web 277 (+6 skipped), web-og 47 + 8 workers-pool render tests. All green.

No changeset: @released/web-og is in .changeset/config.json ignore.

…#151) `renderImage` picked its lifetime with `result ? LONG_CACHE : SHORT_CACHE` — it asked whether a result came back, not whether the answer can still change. Two shapes are real LookupResults that are still in motion, and both took the 24h cache: - `firstRelease: null` renders "not yet released", the one card whose whole job is to flip once a release contains the commit. A commit shared an hour before its release unfurls as unreleased and Slack/X keep that PNG for a day after the release ships. - a soft-deadline `partial` carries a galloped `firstRelease` that is not confirmed to be the earliest, so it has to stay revalidatable. Key on terminality instead (`isTerminal`: a firstRelease AND no partial) — the same test `resolve.ts`'s `hardTtlFor()` applies on the web side, and the OG analogue of the badge invariant already stated in CLAUDE.md: released → long cache, not-yet/checking → short cache. `badge.ts` gets this right; web-og did not. Explicit `cacheOverride`s (the static /placeholder.png, the deploy-window version gate) are untouched. The existing test at "unreleased commit (firstRelease null)" asserted the bug ("A long-cache header still applies — we DID get a result"); its assertion and comment are corrected. Guards (each watched red on the real defect before the fix): - not-yet result → SHORT: red on main, "expected max-age=86400 to be max-age=60", with the card's "not yet released" copy asserted so it pins the flippable shape and not an unrelated one. - partial result → SHORT: red on main, same message. - terminal released result → LONG (complement): forcing isTerminal to false reddens it plus 10 pre-existing long-cache tests, so it is not vacuous. No changeset: @released/web-og is in .changeset/config.json `ignore`.

github-actions · 2026-08-27T11:03:22Z

Preview deployed

Federated GitLab lookups (freedesktop / GNOME) degrade to the "use the CLI" card — the Anubis relay is off in preview. GitHub lookups, permalinks, and OG render work once INTERNAL_SECRET/GITHUB_TOKEN are set on the preview env.

lukaso-bot · 2026-08-27T11:05:02Z

Preview cannot exercise this change — #148 blocks it. Recording the measurement rather than claiming a preview validation I did not get.

https://released-web-og-preview.lukaso.workers.dev/r/honojs/hono/c/f82aba8.png (a stably-released commit, the same fixture the liveness probe uses):

status 200 · cache-control: public, no-transform, max-age=60 · 14348 bytes · x-og-template: og.v1

14 KB is the neutral placeholder — real cards render 28–50 KB, and prod serves this exact URL at 28899 B with max-age=86400. So the preview OG Worker gets null from the Service Binding for every input, which is #148.

That makes the preview structurally useless for this PR specifically: a null result is short-cached both before and after this change, so every preview response reads max-age=60 whether the fix is present or not. There is no input I can send it that distinguishes the two behaviours.

Two further notes for whoever picks up #148: the OG preview target is shared, not per-PR (released-og:shared), so it serves whichever branch deployed last regardless — and prod itself is healthy on this URL, so this is a preview-environment gap, not a production one.

Verification here therefore rests on the gate: the three assertions were watched red against the real defect and the complement was mutation-proved, as detailed in the PR body.

lukaso-bot · 2026-08-27T11:10:58Z

Three more findings from the same review. They land on lines outside this diff, so they could not be posted inline.

1. packages/web/src/resolve.ts:54 — hardTtlFor pins a partial answer for 30 days (correctness)

hardTtlFor checks firstRelease before partial:

if (r.firstRelease) return HARD_TTL_RELEASED;  // 30 days
if (r.partial) return HARD_TTL_PARTIAL;        // 60s — unreachable when firstRelease is set

and isFresh (line 60) short-circuits if (entry.value.firstRelease) return true; for the same shape. So the exact shape this PR declares non-terminal is stored for 30 days and never revalidated.

Scenario: the soft deadline truncates the traversal and findRelease returns { firstRelease: v18.3.0, partial: { reason: 'soft_deadline' } } while the true earliest release is v18.2.0. resolveLookup then serves the unconfirmed v18.3.0 to /r/... (result.tsx), the badge, issue.tsx and pr.tsx for 30 days with no recompute — while the OG PNG now short-caches.

This also makes the new doc comment at packages/web-og/src/index.tsx:199 inaccurate: "This is the same test resolve.ts's hardTtlFor() applies on the web side" describes behavior hardTtlFor does not have for the partial arm — it does the opposite.

2. packages/web/src/routes/badge.ts:141 — badge long-caches a partial answer for 24h (correctness)

return badge(state, resolved.result.firstRelease ? LONG_CACHE : SHORT_CACHE);

That is presence, not terminality — the same #151 defect, still live on the badge surface for the partial arm. With the partial result above, the badge renders the unconfirmed tag under public, max-age=86400, s-maxage=86400, so shields / GitHub's camo proxy pin it for a day.

The PR description says "badge.ts gets this right". That holds for the firstRelease: null arm; the partial arm is unhandled there too.

3. packages/web-og/src/index.tsx:353 — partial OG card shows green SHIPPED with no caveat (correctness)

Now that partial is explicitly classified as an unconfirmed, non-terminal answer, ResultCard still renders it identically to a settled one: green SHIPPED pill, "First released in <tag>", and a date, with nothing marking it best-effort.

Scenario: the galloped firstRelease under a blown soft deadline is not the earliest release. The unfurl reads as authoritative. result-card.tsx surfaces a partial caveat on the web page; the OG card — the thing people actually see in Slack/X — does not. This is the case CLAUDE.md's algorithm guardrail calls out ("Partial state != 'not yet released'... must surface a best-effort answer with a caveat").

The diff itself looks correct: isTerminal handles both non-terminal arms, the cacheOverride paths (/placeholder.png LONG, the deploy-window version gate SHORT) are untouched, and no other renderImage caller relied on the old presence-based default.

Non-terminal cards (not-yet-released, soft-deadline partial) took the 60s SHORT_CACHE, which is the placeholder's retry window, not a freshness policy. `/internal` stores every computed result for 30 minutes (`cache.put(k, r, 30 * 60)`), so a 60s edge TTL bought no freshness the upstream has — it re-ran the ~700ms satori+resvg render up to 60x/hour per URL for byte-identical JSON, of which at most two per hour could differ. Splits out PENDING_CACHE (300s, matching what badge.ts already uses for the same pending state, still 6x fresher than the upstream cache it reads through) and leaves SHORT_CACHE at 60s for the paths it was written for: a binding miss/failure and an unrenderable template version, both of which want the fastest honest retry.

lukaso-bot · 2026-08-27T11:27:42Z

Note for merge order — how this interacts with #144 (no change needed here, but it should be on the record).

This PR reasons against main's flat 30-minute /internal TTL. #144 replaces that with per-state TTLs (packages/web/src/resolve.ts:36-39), which moves both arms of PENDING_CACHE, in opposite directions:

not-yet-released → HARD_TTL_PENDING = 24h upstream. The 300s card TTL goes from 6x fresher than its upstream to ~288x fresher, so this PR's rationale gets stronger, not weaker.
partial → after fix(web): align /internal/* result cache with the public routes (#143) #144, /internal never hands web-og a partial at all: it 503s it deliberately (internal.ts:265-283), so web-og renders the placeholder on the SHORT_CACHE arm. This PR's partial branch becomes unreachable in production until Deadline-heavy repos get a permanently blank OG card and one full traversal per minute after #144 #156 lands.

Two consequences worth stating:

Leaving SHORT_CACHE at 60s was load-bearing, not just conservative. fix(web): align /internal/* result cache with the public routes (#143) #144's comment at internal.ts:274-277 reasons explicitly about web-og short-caching the placeholder at max-age=60 against HARD_TTL_PARTIAL = 60. Bumping the one constant to 300s — the obvious version of this fix — would have silently invalidated that analysis in another open PR.
PENDING_CACHE is the piece Deadline-heavy repos get a permanently blank OG card and one full traversal per minute after #144 #156 is waiting on. That comment ends: "making the card render the gallop hit while keeping it revalidatable needs web-og to short-cache it, which is Deadline-heavy repos get a permanently blank OG card and one full traversal per minute after #144 #156". This PR supplies exactly that lifetime. When Deadline-heavy repos get a permanently blank OG card and one full traversal per minute after #144 #156 is picked up it should decide deliberately whether a partial card takes PENDING_CACHE (300s) or a tighter TTL matching the upstream's HARD_TTL_PARTIAL = 60 distrust window — 300s on a partial would outlive its upstream entry by 5x. That is Deadline-heavy repos get a permanently blank OG card and one full traversal per minute after #144 #156's call to make with the gallop-hit rendering in front of it, not a question this PR can settle.

No code change proposed from this; flagging it so the interaction is visible at merge and #156 does not have to rediscover it.

…TL in test titles Two review findings, both on this PR's own diff, neither a behaviour change. - The `isTerminal()` doc claimed it applies "the same test `hardTtlFor()` applies on the web side". It does not, for the `partial` arm: `hardTtlFor()` and `isFresh()` test `firstRelease` FIRST, so a partial that carries a firstRelease gets the 30-day terminal TTL there, and `badge.ts:141` long-caches the same shape for 24h. `isTerminal()` is deliberately stricter. Saying "same test" would lead the next reader to conclude the web/badge half of #151 is already fixed. Reworded to state the asymmetry and point at #159, which tracks that half. - Two tests were titled "is SHORT-cached" while asserting PENDING (300s). SHORT_CACHE is a separate 60s constant that the placeholder tests in the same file assert exactly, so a grep for SHORT-cached tests found these two green and read the 60s retry policy as covered. Retitled to PENDING-cached. Refs #151, #159.

Review round 4 found the doc comment and one test were built around `firstRelease: null` with no `partial` — a shape core does not emit. Verified: find-release.ts:316 is its only null return and always carries `partial: soft_deadline`; a genuine not-yet-released commit throws NotYetReleasedError (:326, :478), which /internal turns into a 503, so fetchResult returns null and web-og renders the PLACEHOLDER at SHORT_CACHE. The not-yet CARD is never reached from /internal. So the arm this PR actually changes in production is `partial` (24h -> 300s), which is real and stands. Three corrections: - The comment now leads with the partial arm as the behavioural change, and labels the bare-null arm what it is: a defensive guard on a shape the type permits and core keeps a fallback branch for (:486), not a bug that shipped. - It also names where #151's headline symptom actually lives — the unfurl of a not-yet-released commit is the neutral placeholder, not a pinned card. That is #150, and fixing it means changing /internal's error mapping, so it is deliberately left to its own PR rather than widened into this one. - isTerminal takes LookupResult, not LookupResult | null. The ternary keeps owning null (null must map to SHORT_CACHE, not PENDING_CACHE), so the helper's null clause was dead and advertised a guarantee no caller used. The unreachable-shape test is retitled DEFENSIVE and says why, and a new end-to-end test covers the reachable path: 503 -> placeholder, short-cached, asserting the card does NOT say "not yet released". If #150 is fixed by making /internal return a result, that test goes red and surfaces the lifetime decision that has to be made alongside it. Mutation-proven: making fetchResult parse the body regardless of res.ok — the careless shape of a #150 fix — reddens the new test ALONE. Mapping the null result to PENDING_CACHE instead of SHORT_CACHE reddens it plus 6 existing placeholder-lifetime tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

lukaso-bot · 2026-08-27T12:56:04Z

Preview exercised — and it did NOT verify this change. Recording that honestly rather than as a green tick.

https://released-web-og-preview.lukaso.workers.dev/r/honojs/hono/c/f82aba8.png → 200, valid PNG, 14348 bytes, cache-control: public, no-transform, max-age=60.

That is the neutral placeholder on the SHORT_CACHE path. Production renders the same commit as a real result card at 28899 bytes with the 24h cache. So the preview Worker is not reaching web-preview's /internal at all — which is #148 (INTERNAL_SECRET unset on preview), already open.

Why it matters here specifically: this PR changes the partial and terminal arms. The preview can only ever reach the null arm, so it is structurally blind to the thing being fixed — a green preview here would have been meaningless either way. The behaviour is covered by the 48 web-og unit tests instead, including the two mutation-proven guards added in 9038b47.

Not a blocker for this PR, but #148 should land before the next OG change so previews stop being decorative.

…e reachable shape Round 5 review of #158. The `isTerminal` docstring (and the DEFENSIVE test that leans on it) concluded web-og renders the neutral placeholder "never this card". That holds only for `firstRelease: null` with NO `partial`. The other shape — null WITH `partial: soft_deadline` — is returned as a normal VALUE by find-release.ts:312-322, cached and answered 200 by /internal, and renders `firstRelease?.tag ?? 'not yet released'`. So the "not yet released" OG card does ship on `main` today, via a blown soft deadline rather than NotYetReleasedError. The lifetime this PR picks is already right for it (PENDING via the `partial` arm), so no behaviour changes. What changes is that the comment no longer tells the next reader the card is unreachable, and the one reachable shape now has a copy guard instead of being disclaimed away: - `isTerminal` docstring splits the two shapes and states which route each takes, including that #144 (503 on every partial) closes the reachable route until #156 reopens it with a caveat. - the DEFENSIVE test comment is scoped to the bare shape only. - new test: `{ firstRelease: null, partial: soft_deadline }` asserts BOTH the rendered copy ("not yet released") and PENDING. Mutation-proven, both assertions: - lifetime: restore the pre-#158 rule (`result == null ? SHORT : LONG`) → the new test FAILS on cache-control (with 3 siblings). - copy: change the card's fallback to `'unknown'` → the new test FAILS on the rendered text (with 2 siblings). The first attempt at this mutation hit the docstring instead of the code and everything stayed green; re-run against index.tsx:292, it reddens. web-og 49 + 8 tests green.

lukaso-bot · 2026-08-27T17:31:26Z

Round 5 — 2 review threads addressed (`c9705be`), 0 unresolved

Finding	Disposition
`index.tsx:220` — the reachability claim is inverted for `partial` + `firstRelease: null`	Applied. Docstring splits bare-null (unreachable) from null-with-`partial` (reachable, ships the card today); the `DEFENSIVE` test comment is scoped to the bare shape; new test asserts the rendered copy and `PENDING` for the reachable one
`index.tsx:242` — `ResultCard` never reads `result.partial`	Split out as #160. Pre-existing, and the copy cannot be written correctly until #144 closes the route and #156 reopens it — #160 is scoped to land with #156

No behaviour change: the lifetime this PR picks was already correct for the shape. What changed is that it is guarded rather than disclaimed.

Mutation evidence (both assertions in the new test):

restore the pre-fix(web-og): cache the OG card by terminality, not by result presence (#151) #158 rule (result == null ? SHORT : LONG) → the new test fails on cache-control (3 siblings redden)
change the card fallback to 'unknown' (index.tsx:292) → the new test fails on the rendered text (2 siblings redden)

Worth recording: the first attempt at that second mutation edited the copy inside the docstring rather than the code, and all 49 tests stayed green — the same false-negative class this thread is about.

⚠️ Preview verification is still structurally blind here (#148): https://released-web-og-preview.lukaso.workers.dev/r/honojs/hono/c/f82aba8.png returns a 200 PNG, but it is the placeholder (~14 KB vs production's ~29 KB for the same commit), because og-preview cannot reach web-preview /internal. A green preview on this PR is not evidence any result-backed arm works.

Full gate green locally: core 230, web 277, web-og 49 + 8, cli 45, a11y 5, deploy-config 20, lint/shellcheck/actionlint/gitleaks/publint/osv.

…ndent Round 15 review on #144. Three findings on the diff itself; one declined. 1. resolve.ts — a pinning caller wrote HARD_TTL_PARTIAL onto the SHARED entry. Since this PR aligned /internal onto the public key, that 60s imposed itself on badge.ts and the permalink pages: one OG unfurl replaced the 30-day entry a page view had just written, so 61s later every human page view paid a fresh traversal and issue.tsx/pr.tsx's bot branch rendered the deferred card off a slot that had been warm. The entry now always carries hardTtlFor(); this caller's 60s distrust of a partial rides on its own `:pinpartial` marker, which is what shouldRecompute already read. Public semantics are now actually untouched, as the comment claimed. (The 30-day pin of a gallop partial is real and stays #155's to fix, on the public routes' own writes.) 2. resolve.ts — a bypassed load that failed RE-STAMPED the shared `:neg` marker. Under a once-a-minute unfurl cadence the marker never reached NEG_TTL, so a human on the same key sat on "checking..." for a whole outage and never got the per-minute retry the back-off exists to give them. The marker is now written only when it was cold; the bypass path leaves its age alone. 3. wrangler.toml / config guard — PUBLIC_BASE_URL also drives publicBaseUrl(), so preview's canonical/og:url/sitemap became "correct only while this literal matches". cacheOriginProblems() now asserts, for every named env on a workers.dev host, that the first label equals `[env.<name>] name` — a rename fails the build instead of shipping URLs for a host that does not exist. The ACCOUNT subdomain is not in the file at all, so it stays a comment. Declined: the 503-on-partial regression for deadline-heavy repos (#156). It is deliberate, the merge order #144 -> #158 -> #156 is written on #156, and the remedy is web-og short-caching the gallop shape, which is #158's change. Mutation-proven, each against the defect in its own claim: - restore `pinnedPartial ? HARD_TTL_PARTIAL : hardTtlFor(r)` -> 3 tests redden ("...caller-independent TTL", "...IDENTICAL entry TTL", "...via its own marker"). - drop `if (!backedOff)` -> "does NOT re-stamp a negative marker it bypassed" reddens; the cold-slot companion test stays green, so the back-off still exists. - neuter the name check -> "left behind by a `name` rename" reddens. - drop the `.workers.dev` condition -> the custom-domain and `app:8787` tests redden, so the guard is scoped, not merely present. pnpm test 352 web / 45 cli / 33 web-og green; typecheck clean.

lukaso-bot · 2026-08-29T01:39:40Z

Preview exercised — and it cannot validate this PR. Flagging so the green preview is not read as evidence.

https://released-web-og-preview.lukaso.workers.dev/r/honojs/hono/c/f82aba8.png returns 200 image/png, but at 14348 bytes / max-age=60, where production returns 28899 bytes for the same commit. A deliberately nonexistent SHA on the same preview returns 15001 bytes / max-age=60 — the same neutral treatment. Preview renders PlaceholderCard for every input, so the terminality-based cache decision this PR changes is never reached there.

That is #148 (og-preview can't reach web-preview's /internal, almost certainly INTERNAL_SECRET unset on the preview environment), not a defect in this branch. I have re-confirmed it on #148 with the measurements above and the one-line fix, which is a human wrangler secret put on both preview Workers.

Until that is set, this PR's evidence is the unit tests plus the CI gate (10/10 green), not the preview deploy. The preview proves the Worker boots and serves a PNG.

lukaso-bot mentioned this pull request Aug 27, 2026

Preview OG Worker renders the neutral card for every input — og-preview can't reach web-preview /internal (likely INTERNAL_SECRET unset on preview) #148

Open