From dc70f0f731db37c34c54de85e12ff71b5f7a66cc Mon Sep 17 00:00:00 2001 From: liveapp-bot Date: Thu, 27 Aug 2026 12:00:47 +0100 Subject: [PATCH 1/5] fix(web-og): cache the OG card by terminality, not by result presence (#151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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`. --- packages/web-og/src/index.tsx | 28 +++++++++- packages/web-og/test/routing.test.ts | 84 ++++++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 7 deletions(-) diff --git a/packages/web-og/src/index.tsx b/packages/web-og/src/index.tsx index 0639876..2fbcc0b 100644 --- a/packages/web-og/src/index.tsx +++ b/packages/web-og/src/index.tsx @@ -4,8 +4,9 @@ // GET /h/:host/r/:projectPath/c/:sha.png — federated (any host, #8) // → Fetch the result data from `web` via Service Binding (D23). // → Render PNG via @cloudflare/workers-og (Satori + resvg-wasm). -// → Cache 24h. On data miss, render a neutral placeholder with short TTL -// (never a long-cached error). +// → Cache 24h, but only for a SETTLED answer (a released commit). A +// not-yet-released or partial result, and a data miss (neutral +// placeholder), get a short TTL so the card can still flip (#151). import { type LookupResult, OG_TEMPLATE_VERSION } from '@released/core'; import { type Context, Hono } from 'hono'; @@ -181,13 +182,34 @@ export default app; export const LONG_CACHE = `public, no-transform, max-age=${24 * 60 * 60}, s-maxage=${24 * 60 * 60}`; export const SHORT_CACHE = 'public, no-transform, max-age=60'; +/** True only when the answer the card renders can never change again: a + * completed traversal that found a release. + * + * The lifetime used to key on whether a result came back AT ALL, which + * long-cached two shapes that are still in motion (#151): + * + * - `firstRelease: null` renders "not yet released" — the one card whose + * whole job is to flip once a release contains the commit. Pinned for 24h, + * 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 `partial` is a best-effort answer from a traversal the soft deadline + * truncated, so its `firstRelease` is not confirmed to be the earliest one. + * It has to stay revalidatable rather than be pinned as if it were final. + * + * This is the same test `resolve.ts`'s `hardTtlFor()` applies on the web + * side, and the OG analogue of the badge invariant the project already + * states: released → long cache, not-yet/checking → short cache. */ +function isTerminal(result: LookupResult | null): boolean { + return result != null && result.firstRelease != null && !result.partial; +} + export function renderImage( result: LookupResult | null, ctx: { owner: string; repo: string; sha?: string; number?: string }, cacheOverride?: string, ): Response { const SIZE = { width: 1200, height: 630 }; - const cacheControl = cacheOverride ?? (result ? LONG_CACHE : SHORT_CACHE); + const cacheControl = cacheOverride ?? (isTerminal(result) ? LONG_CACHE : SHORT_CACHE); const node = result ? ResultCard(result) : PlaceholderCard(ctx); diff --git a/packages/web-og/test/routing.test.ts b/packages/web-og/test/routing.test.ts index 89b2ab1..beef3eb 100644 --- a/packages/web-og/test/routing.test.ts +++ b/packages/web-og/test/routing.test.ts @@ -435,10 +435,10 @@ describe('web-og card content', () => { // The SHIPPED badge and the date are gated on `firstRelease` — both gone. expect(text).not.toContain('SHIPPED'); expect(text.some((t) => /^\d{4}-\d{2}-\d{2}$/.test(t))).toBe(false); - // A long-cache header still applies — we DID get a result, it's just unreleased. - expect(res.headers.get('cache-control')).toBe( - 'public, no-transform, max-age=86400, s-maxage=86400', - ); + // Short-cached: "not yet released" is a pending state that has to flip + // when the release lands, so it is NOT long-cacheable just because a + // result came back (#151). Lifetime coverage lives in its own describe. + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); }); it('placeholder card (binding miss): shows "Looking up…" and the owner/repo label', async () => { @@ -745,3 +745,79 @@ describe('web-og issue/PR cards (#79)', () => { expect(/[\u{D800}-\u{DFFF}]/u.test(joined)).toBe(false); }); }); + +// #151: the cache lifetime keys on whether a result was RECEIVED, not on +// whether the answer it renders can still change. Both non-terminal shapes — +// "not yet released" (firstRelease null) and a soft-deadline `partial` — are +// real LookupResults, so both took the 24h cache. The not-yet card is the one +// card whose whole job is to flip when the release lands; a partial is an +// unconfirmed answer from a truncated traversal. Pinning either for a day in +// every crawler's cache is the OG analogue of the badge invariant the project +// already states (released → long, not-yet/checking → short). +describe('web-og cache lifetime keys on terminality, not presence (#151)', () => { + const LONG = 'public, no-transform, max-age=86400, s-maxage=86400'; + const SHORT = 'public, no-transform, max-age=60'; + + const baseInput = { + kind: 'commit', + repo: { owner: 'facebook', repo: 'react', projectPath: 'facebook/react' }, + sha: 'a'.repeat(40), + }; + const released = { tag: 'v18.2.0', sha: 's', date: '2024-03-15T09:00:00Z', url: '' }; + + async function fetchCard(result: Record): Promise { + return await app.fetch( + new Request('https://og.example/r/facebook/react/c/abc1234.png'), + makeEnv(new Response(JSON.stringify(result))), + ); + } + + it('not-yet-released result is SHORT-cached so the card flips when the release lands', async () => { + const res = await fetchCard({ + input: baseInput, + canonicalSha: 'abc1234def5678', + firstRelease: null, + alsoIn: [], + releaseNotesHtml: null, + rateLimit: null, + }); + expect(res.status).toBe(200); + // The card really does render the flippable copy — so this is the card + // whose lifetime matters, not an unrelated shape. + expect(collectText(lastRenderedNode)).toContain('not yet released'); + expect(res.headers.get('cache-control')).toBe(SHORT); + }); + + it('partial result is SHORT-cached even though it carries a firstRelease', async () => { + const res = await fetchCard({ + input: baseInput, + canonicalSha: 'abc1234def5678', + firstRelease: released, + partial: { reason: 'soft_deadline', candidatesTried: 12 }, + alsoIn: [], + releaseNotesHtml: null, + rateLimit: null, + }); + expect(res.status).toBe(200); + // A galloped answer under a blown soft deadline is not confirmed earliest, + // so it must stay revalidatable rather than pinned for a day. + expect(res.headers.get('cache-control')).toBe(SHORT); + }); + + // Complement. Without it, "short-cache everything" passes the two above and + // silently re-renders every settled card once a minute. (Proven: forcing + // isTerminal to false reddens this and 10 pre-existing long-cache tests.) + it('terminal released result keeps the LONG cache', async () => { + const res = await fetchCard({ + input: baseInput, + canonicalSha: 'abc1234def5678', + firstRelease: released, + alsoIn: [], + releaseNotesHtml: null, + rateLimit: null, + }); + expect(res.status).toBe(200); + expect(collectText(lastRenderedNode)).toContain('SHIPPED'); + expect(res.headers.get('cache-control')).toBe(LONG); + }); +}); From 44da2e96492d1531a9960487bde1ca2a05fa326f Mon Sep 17 00:00:00 2001 From: liveapp-bot Date: Thu, 27 Aug 2026 12:24:13 +0100 Subject: [PATCH 2/5] perf(web-og): align the pending-card TTL with the data behind it (300s) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/web-og/src/index.tsx | 21 ++++++++++++++++++++- packages/web-og/test/routing.test.ts | 25 ++++++++++++++++++++----- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/packages/web-og/src/index.tsx b/packages/web-og/src/index.tsx index 2fbcc0b..ef2f75d 100644 --- a/packages/web-og/src/index.tsx +++ b/packages/web-og/src/index.tsx @@ -180,8 +180,25 @@ export default app; // of the og.* zone) recompress the PNG that `web` links as the byte-exact social // card. Everything after it is the actual freshness policy. export const LONG_CACHE = `public, no-transform, max-age=${24 * 60 * 60}, s-maxage=${24 * 60 * 60}`; + +/** A card we could NOT render from a result: the placeholder. Either `/internal` + * missed/failed (transient — retry soon), or the URL carries a template version + * this build cannot render (self-heals once web-og lands). Both want the + * shortest honest retry window, so this stays at 60s. */ export const SHORT_CACHE = 'public, no-transform, max-age=60'; +/** A card we DID render, from an answer that is still in motion: not-yet-released + * or a soft-deadline `partial`. Distinct from SHORT_CACHE because the question is + * different — not "how fast should a failure retry" but "how fast can this answer + * actually change". It cannot change faster than the data behind it, and + * `/internal` stores every computed result for 30 minutes + * (`cache.put(k, r, 30 * 60)`, packages/web/src/routes/internal.ts). A 60s TTL + * therefore bought no freshness the upstream has: it re-ran the ~700ms + * satori+resvg wasm render up to 60x/hour per URL for byte-identical JSON. 300s + * matches the TTL `badge.ts` already uses for the same pending state, and is + * still 6x fresher than the upstream cache it reads through. */ +export const PENDING_CACHE = 'public, no-transform, max-age=300, s-maxage=300'; + /** True only when the answer the card renders can never change again: a * completed traversal that found a release. * @@ -209,7 +226,9 @@ export function renderImage( cacheOverride?: string, ): Response { const SIZE = { width: 1200, height: 630 }; - const cacheControl = cacheOverride ?? (isTerminal(result) ? LONG_CACHE : SHORT_CACHE); + const cacheControl = + cacheOverride ?? + (result == null ? SHORT_CACHE : isTerminal(result) ? LONG_CACHE : PENDING_CACHE); const node = result ? ResultCard(result) : PlaceholderCard(ctx); diff --git a/packages/web-og/test/routing.test.ts b/packages/web-og/test/routing.test.ts index beef3eb..af5854f 100644 --- a/packages/web-og/test/routing.test.ts +++ b/packages/web-og/test/routing.test.ts @@ -435,10 +435,12 @@ describe('web-og card content', () => { // The SHIPPED badge and the date are gated on `firstRelease` — both gone. expect(text).not.toContain('SHIPPED'); expect(text.some((t) => /^\d{4}-\d{2}-\d{2}$/.test(t))).toBe(false); - // Short-cached: "not yet released" is a pending state that has to flip + // Pending-cached: "not yet released" is a pending state that has to flip // when the release lands, so it is NOT long-cacheable just because a // result came back (#151). Lifetime coverage lives in its own describe. - expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); + expect(res.headers.get('cache-control')).toBe( + 'public, no-transform, max-age=300, s-maxage=300', + ); }); it('placeholder card (binding miss): shows "Looking up…" and the owner/repo label', async () => { @@ -756,7 +758,20 @@ describe('web-og issue/PR cards (#79)', () => { // already states (released → long, not-yet/checking → short). describe('web-og cache lifetime keys on terminality, not presence (#151)', () => { const LONG = 'public, no-transform, max-age=86400, s-maxage=86400'; - const SHORT = 'public, no-transform, max-age=60'; + // A pending answer is backed by `/internal`'s own 30-minute result cache + // (`cache.put(k, r, 30 * 60)` in packages/web/src/routes/internal.ts), so a + // 60s edge TTL cannot buy freshness the upstream does not have — it just + // re-runs the ~700ms satori+resvg render up to 60x/hour per URL while + // `/internal` hands back byte-identical JSON. 300s matches what badge.ts + // already uses for the same pending state and is still 6x fresher than the + // data behind it. + const PENDING = 'public, no-transform, max-age=300, s-maxage=300'; + // PENDING stays separate from the placeholder's 60s SHORT_CACHE because the + // two answer different questions: SHORT_CACHE is "how fast should a FAILED + // render retry" (binding miss, unrenderable template version), PENDING is + // "how fast can this ANSWER change". Collapsing them by bumping SHORT_CACHE + // to 300 reddens the 14 existing placeholder-lifetime tests above, which is + // the guard for that direction. const baseInput = { kind: 'commit', @@ -785,7 +800,7 @@ describe('web-og cache lifetime keys on terminality, not presence (#151)', () => // The card really does render the flippable copy — so this is the card // whose lifetime matters, not an unrelated shape. expect(collectText(lastRenderedNode)).toContain('not yet released'); - expect(res.headers.get('cache-control')).toBe(SHORT); + expect(res.headers.get('cache-control')).toBe(PENDING); }); it('partial result is SHORT-cached even though it carries a firstRelease', async () => { @@ -801,7 +816,7 @@ describe('web-og cache lifetime keys on terminality, not presence (#151)', () => expect(res.status).toBe(200); // A galloped answer under a blown soft deadline is not confirmed earliest, // so it must stay revalidatable rather than pinned for a day. - expect(res.headers.get('cache-control')).toBe(SHORT); + expect(res.headers.get('cache-control')).toBe(PENDING); }); // Complement. Without it, "short-cache everything" passes the two above and From 93fa3d0f8cc05cd350783e0d1ee53bcfe4ef381e Mon Sep 17 00:00:00 2001 From: liveapp-bot Date: Thu, 27 Aug 2026 12:54:49 +0100 Subject: [PATCH 3/5] docs(web-og): correct the isTerminal parity claim; name the pending TTL 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. --- packages/web-og/src/index.tsx | 13 ++++++++++--- packages/web-og/test/routing.test.ts | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/web-og/src/index.tsx b/packages/web-og/src/index.tsx index ef2f75d..78f3e47 100644 --- a/packages/web-og/src/index.tsx +++ b/packages/web-og/src/index.tsx @@ -213,9 +213,16 @@ export const PENDING_CACHE = 'public, no-transform, max-age=300, s-maxage=300'; * truncated, so its `firstRelease` is not confirmed to be the earliest one. * It has to stay revalidatable rather than be pinned as if it were final. * - * This is the same test `resolve.ts`'s `hardTtlFor()` applies on the web - * side, and the OG analogue of the badge invariant the project already - * states: released → long cache, not-yet/checking → short cache. */ + * This is STRICTER than the web side, deliberately. `hardTtlFor()` + * (packages/web/src/resolve.ts) tests `firstRelease` first, so a partial + * that carries a `firstRelease` gets the 30-day terminal TTL there, and + * `badge.ts` long-caches the same shape for 24h. A truncated traversal that + * reported v2.0.0 when v1.9.0 was the true earliest is therefore still + * pinned on those two surfaces — the partial half of #151, tracked + * separately in #159. Here it revalidates. + * + * It is the OG analogue of the badge invariant the project already states: + * released → long cache, not-yet/checking → short cache. */ function isTerminal(result: LookupResult | null): boolean { return result != null && result.firstRelease != null && !result.partial; } diff --git a/packages/web-og/test/routing.test.ts b/packages/web-og/test/routing.test.ts index af5854f..7916696 100644 --- a/packages/web-og/test/routing.test.ts +++ b/packages/web-og/test/routing.test.ts @@ -787,7 +787,7 @@ describe('web-og cache lifetime keys on terminality, not presence (#151)', () => ); } - it('not-yet-released result is SHORT-cached so the card flips when the release lands', async () => { + it('not-yet-released result is PENDING-cached (300s) so the card flips when the release lands', async () => { const res = await fetchCard({ input: baseInput, canonicalSha: 'abc1234def5678', @@ -803,7 +803,7 @@ describe('web-og cache lifetime keys on terminality, not presence (#151)', () => expect(res.headers.get('cache-control')).toBe(PENDING); }); - it('partial result is SHORT-cached even though it carries a firstRelease', async () => { + it('partial result is PENDING-cached even though it carries a firstRelease', async () => { const res = await fetchCard({ input: baseInput, canonicalSha: 'abc1234def5678', From 9038b47b5ebdc2005011c9389a60cfcbf8f4d752 Mon Sep 17 00:00:00 2001 From: liveapp-bot Date: Thu, 27 Aug 2026 13:53:57 +0100 Subject: [PATCH 4/5] fix(web-og): scope the not-yet arm honestly; add the reachable 503 guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/web-og/src/index.tsx | 27 +++++++++++++++++++------ packages/web-og/test/routing.test.ts | 30 +++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/packages/web-og/src/index.tsx b/packages/web-og/src/index.tsx index 78f3e47..ab4ae89 100644 --- a/packages/web-og/src/index.tsx +++ b/packages/web-og/src/index.tsx @@ -205,13 +205,28 @@ export const PENDING_CACHE = 'public, no-transform, max-age=300, s-maxage=300'; * The lifetime used to key on whether a result came back AT ALL, which * long-cached two shapes that are still in motion (#151): * - * - `firstRelease: null` renders "not yet released" — the one card whose - * whole job is to flip once a release contains the commit. Pinned for 24h, - * 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 `partial` is a best-effort answer from a traversal the soft deadline * truncated, so its `firstRelease` is not confirmed to be the earliest one. * It has to stay revalidatable rather than be pinned as if it were final. + * **This is the arm that changes production behaviour** (24h → 300s). + * - `firstRelease: null` renders "not yet released", the one card whose whole + * job is to flip once a release contains the commit. Be precise about this + * one: core does not currently EMIT that shape. `find-release.ts:316` is its + * only `firstRelease: null` return and it always carries + * `partial: soft_deadline`; a genuine not-yet-released commit throws + * `NotYetReleasedError` (`:326`, and `:478` for the issue/PR aggregation), + * which `/internal` turns into a 503 (`web/src/routes/internal.ts:67-72`) — + * so `fetchResult` sees `!res.ok`, returns null, and web-og renders the + * neutral PLACEHOLDER at `SHORT_CACHE`, never this card. The bare-null arm + * below is therefore a defensive guard on a shape the type permits and + * core keeps a fallback branch for (`:486`), not a bug that shipped. + * + * What that means for #151's headline case: the not-yet-released unfurl is + * still wrong today, but wrong in a different way than "pinned for 24h" — it + * is the neutral "Looking up…" placeholder rather than a card saying the + * commit is unreleased. That is #150, and it is deliberately NOT fixed here: + * it needs `/internal` to stop 503-ing `NotYetReleasedError`, which is a + * change to the web package's error mapping, not to this lifetime rule. * * This is STRICTER than the web side, deliberately. `hardTtlFor()` * (packages/web/src/resolve.ts) tests `firstRelease` first, so a partial @@ -223,8 +238,8 @@ export const PENDING_CACHE = 'public, no-transform, max-age=300, s-maxage=300'; * * It is the OG analogue of the badge invariant the project already states: * released → long cache, not-yet/checking → short cache. */ -function isTerminal(result: LookupResult | null): boolean { - return result != null && result.firstRelease != null && !result.partial; +function isTerminal(result: LookupResult): boolean { + return result.firstRelease != null && !result.partial; } export function renderImage( diff --git a/packages/web-og/test/routing.test.ts b/packages/web-og/test/routing.test.ts index 7916696..4f9817f 100644 --- a/packages/web-og/test/routing.test.ts +++ b/packages/web-og/test/routing.test.ts @@ -787,7 +787,16 @@ describe('web-og cache lifetime keys on terminality, not presence (#151)', () => ); } - it('not-yet-released result is PENDING-cached (300s) so the card flips when the release lands', async () => { + // Scope this one honestly. `firstRelease: null` with no `partial` is a shape + // core does not currently emit: `find-release.ts:316` is its only null return + // and always carries `partial: soft_deadline`, and a genuine not-yet-released + // commit throws `NotYetReleasedError`, which `/internal` turns into a 503. So + // this is a DEFENSIVE unit guard on the lifetime rule for a shape the type + // permits (and core keeps a fallback branch for at `:486`) — it is not + // evidence about a state production reaches today. The end-to-end test below + // is what documents the reachable not-yet path. (The same distinction is + // already drawn on `pendingFixture` in web's internal-cache-origin tests.) + it('DEFENSIVE: a bare not-yet-released result is PENDING-cached (300s), not pinned', async () => { const res = await fetchCard({ input: baseInput, canonicalSha: 'abc1234def5678', @@ -803,6 +812,25 @@ describe('web-og cache lifetime keys on terminality, not presence (#151)', () => expect(res.headers.get('cache-control')).toBe(PENDING); }); + // The REACHABLE not-yet-released path, end to end, and the one #151 actually + // asked to be pinned down: `/internal` 503s `NotYetReleasedError`, `fetchResult` + // returns null, and web-og renders the neutral placeholder at SHORT_CACHE. This + // documents what production does today — including that the card does NOT say + // the commit is unreleased, which is #150 and out of scope here. If #150 is + // fixed by making `/internal` return a result instead of a 503, this test goes + // red and points at the lifetime decision that has to be made with it. + it('a not-yet-released commit reaches web-og as a 503 → placeholder, short-cached (#150)', async () => { + const res = await app.fetch( + new Request('https://og.example/r/facebook/react/c/abc1234.png'), + makeEnv(new Response('{"error":"not yet released"}', { status: 503 })), + ); + expect(res.status).toBe(200); + // Not the "not yet released" card — the neutral placeholder. + expect(collectText(lastRenderedNode)).toContain('Looking up…'); + expect(collectText(lastRenderedNode)).not.toContain('not yet released'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); + }); + it('partial result is PENDING-cached even though it carries a firstRelease', async () => { const res = await fetchCard({ input: baseInput, From c9705be03bb3ecb4afa3bfa26c0e195185554240 Mon Sep 17 00:00:00 2001 From: liveapp-bot Date: Thu, 27 Aug 2026 18:28:15 +0100 Subject: [PATCH 5/5] docs(web-og): scope the not-yet unreachability to BARE null; guard the reachable shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/web-og/src/index.tsx | 33 +++++++++++++------ packages/web-og/test/routing.test.ts | 47 ++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/packages/web-og/src/index.tsx b/packages/web-og/src/index.tsx index ab4ae89..9438d90 100644 --- a/packages/web-og/src/index.tsx +++ b/packages/web-og/src/index.tsx @@ -210,16 +210,29 @@ export const PENDING_CACHE = 'public, no-transform, max-age=300, s-maxage=300'; * It has to stay revalidatable rather than be pinned as if it were final. * **This is the arm that changes production behaviour** (24h → 300s). * - `firstRelease: null` renders "not yet released", the one card whose whole - * job is to flip once a release contains the commit. Be precise about this - * one: core does not currently EMIT that shape. `find-release.ts:316` is its - * only `firstRelease: null` return and it always carries - * `partial: soft_deadline`; a genuine not-yet-released commit throws - * `NotYetReleasedError` (`:326`, and `:478` for the issue/PR aggregation), - * which `/internal` turns into a 503 (`web/src/routes/internal.ts:67-72`) — - * so `fetchResult` sees `!res.ok`, returns null, and web-og renders the - * neutral PLACEHOLDER at `SHORT_CACHE`, never this card. The bare-null arm - * below is therefore a defensive guard on a shape the type permits and - * core keeps a fallback branch for (`:486`), not a bug that shipped. + * job is to flip once a release contains the commit. Two DIFFERENT shapes + * render it, and only one of them is unreachable: + * + * - BARE null (no `partial`): core does not emit it. `find-release.ts:316` + * is its only `firstRelease: null` return and it always carries + * `partial: soft_deadline`, and a genuine not-yet-released commit throws + * `NotYetReleasedError` (`:326`, `:478` for the issue/PR aggregation), + * which `/internal` turns into a 503 + * (`web/src/routes/internal.ts:67-72`) — so `fetchResult` sees + * `!res.ok`, returns null, and web-og renders the neutral PLACEHOLDER at + * `SHORT_CACHE`, never this card. The bare-null arm below is a defensive + * guard on a shape the type permits and core keeps a fallback branch for + * (`:486`), not a bug that shipped. + * - null WITH `partial: soft_deadline`: REACHABLE, and it ships the "not + * yet released" card today. `find-release.ts:312-322` returns it as a + * normal value, `/internal` caches it and answers 200, so `ResultCard` + * sets `tag = firstRelease?.tag ?? 'not yet released'`. That is the + * blown-soft-deadline route into this card, not `NotYetReleasedError`. + * PR #144 makes `/internal` 503 every `partial`, which closes this route + * until #156 reopens it with a caveat on the card — but the shape is live + * on `main` right now, so the lifetime rule has to be right for it either + * way. It is: PENDING via the `partial` arm above. `routing.test.ts` has + * a copy guard on it. * * What that means for #151's headline case: the not-yet-released unfurl is * still wrong today, but wrong in a different way than "pinned for 24h" — it diff --git a/packages/web-og/test/routing.test.ts b/packages/web-og/test/routing.test.ts index 4f9817f..38191d3 100644 --- a/packages/web-og/test/routing.test.ts +++ b/packages/web-og/test/routing.test.ts @@ -787,15 +787,16 @@ describe('web-og cache lifetime keys on terminality, not presence (#151)', () => ); } - // Scope this one honestly. `firstRelease: null` with no `partial` is a shape - // core does not currently emit: `find-release.ts:316` is its only null return - // and always carries `partial: soft_deadline`, and a genuine not-yet-released - // commit throws `NotYetReleasedError`, which `/internal` turns into a 503. So - // this is a DEFENSIVE unit guard on the lifetime rule for a shape the type - // permits (and core keeps a fallback branch for at `:486`) — it is not - // evidence about a state production reaches today. The end-to-end test below - // is what documents the reachable not-yet path. (The same distinction is - // already drawn on `pendingFixture` in web's internal-cache-origin tests.) + // Scope this one honestly, and only to the BARE shape. `firstRelease: null` + // with NO `partial` is what core does not currently emit: `find-release.ts:316` + // is its only null return and always carries `partial: soft_deadline`, and a + // genuine not-yet-released commit throws `NotYetReleasedError`, which + // `/internal` turns into a 503. So this is a DEFENSIVE unit guard on the + // lifetime rule for a shape the type permits (and core keeps a fallback branch + // for at `:486`) — it is not evidence about a state production reaches today. + // It says nothing about `null` WITH a `partial`, which IS reachable and has its + // own copy guard below. (The same distinction is already drawn on + // `pendingFixture` in web's internal-cache-origin tests.) it('DEFENSIVE: a bare not-yet-released result is PENDING-cached (300s), not pinned', async () => { const res = await fetchCard({ input: baseInput, @@ -831,6 +832,34 @@ describe('web-og cache lifetime keys on terminality, not presence (#151)', () => expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); }); + // The REACHABLE route into the "not yet released" card, and the one the + // docstring on `isTerminal` used to disclaim. A blown soft deadline with no + // gallop hit returns `firstRelease: null` + `partial` as a normal VALUE + // (`find-release.ts:312-322`); `/internal` caches it and answers 200, so + // `fetchResult` hands web-og a real result and `ResultCard` renders + // `firstRelease?.tag ?? 'not yet released'`. Asserting the COPY as well as the + // lifetime is the point: the shape reaches this card on `main` today, and #144 + // (503 on every partial) then #156 (render it with a caveat) both move that + // route without changing what the lifetime must be. If either lands without + // deciding this card's lifetime deliberately, this test is what says so. + it('a soft-deadline partial with NO release renders the not-yet copy, PENDING-cached', async () => { + const res = await fetchCard({ + input: baseInput, + canonicalSha: 'abc1234def5678', + firstRelease: null, + partial: { reason: 'soft_deadline', candidatesTried: 12 }, + alsoIn: [], + releaseNotesHtml: null, + rateLimit: null, + }); + expect(res.status).toBe(200); + expect(collectText(lastRenderedNode)).toContain('not yet released'); + // Not LONG: an unconfirmed traversal must stay revalidatable. Not the + // placeholder's SHORT_CACHE either — this is a rendered answer, not a failed + // render. + expect(res.headers.get('cache-control')).toBe(PENDING); + }); + it('partial result is PENDING-cached even though it carries a firstRelease', async () => { const res = await fetchCard({ input: baseInput,