From 52be032ec87dd34b85b4556175ee796ead8d8482 Mon Sep 17 00:00:00 2001 From: liveapp-bot Date: Tue, 11 Aug 2026 19:59:08 -0400 Subject: [PATCH 1/4] fix(web-og): stop workers-og's 1-year immutable default overriding OG cache-control MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every OG image shipped a merged cache-control header: public, immutable, no-transform, max-age=31536000, public, max-age=60 workers-og builds its response headers as { 'Content-Type': …, 'Cache-Control': <1-year immutable default>, ...opts.headers } and object spread is case-SENSITIVE, so renderImage's lowercase 'cache-control' never replaced the capitalized default. Both keys reached `new Response`, where Headers merged them into one comma-joined value — and caches honor the FIRST max-age. Impact is on the short-cached cards. A placeholder render (cold lookup, upstream rate-limit, service-binding miss, or the notFound path hit during the web→web-og deploy window) is deliberately max-age=60 so the unfurl refreshes once the real answer is available. Instead it went out immutable for a year, so a transient failure froze a wrong social card in every downstream cache. This is the mechanism behind the "sustained placeholder on a stably-released MR" seen on the federated PR OG route. Fix: set cache-control on the Response after construction. Headers.set is case-insensitive, so it replaces the library default whatever casing it uses. Tests: the routing suite mocked ImageResponse with a stub that passed the caller's headers straight through, so it never saw the library default — every cache-control assertion there was unfalsifiable. The mock now mirrors workers-og's real header construction (capitalized default + case-sensitive spread), the assertions are exact-match instead of substring (a substring match passes on the merged value), and cards.render.test.ts asserts the header against the REAL library under workerd. --- packages/web-og/src/index.tsx | 16 +++++++++-- packages/web-og/test/cards.render.test.ts | 28 ++++++++++++++++++ packages/web-og/test/routing.test.ts | 35 ++++++++++++++++------- 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/packages/web-og/src/index.tsx b/packages/web-og/src/index.tsx index 4f4643b..548b59b 100644 --- a/packages/web-og/src/index.tsx +++ b/packages/web-og/src/index.tsx @@ -138,13 +138,25 @@ export function renderImage( const node = result ? ResultCard(result) : PlaceholderCard(ctx); - return new ImageResponse(node, { + const res = new ImageResponse(node, { ...SIZE, headers: { - 'cache-control': cacheControl, 'x-og-template': OG_TEMPLATE_VERSION, }, }); + // Set cache-control on the RESPONSE, not through ImageResponse's `headers` + // option. workers-og builds its header object as + // { 'Content-Type': …, 'Cache-Control': <1-year immutable default>, ...opts.headers } + // and object spread is case-SENSITIVE, so a lowercase 'cache-control' passed + // in above does NOT replace that default — both keys reach `new Response`, + // where Headers merges them into one value ("public, immutable, no-transform, + // max-age=31536000, public, max-age=60") and caches honor the FIRST max-age. + // That pinned every short-cached card (placeholder, cold lookup, the + // deploy-window notFound render) as immutable for a year, so a transient + // failure's unfurl could never refresh. Headers.set is case-insensitive and + // replaces the default outright, whatever casing the library uses. + res.headers.set('cache-control', cacheControl); + return res; } function ResultCard(r: LookupResult) { diff --git a/packages/web-og/test/cards.render.test.ts b/packages/web-og/test/cards.render.test.ts index a0bb181..963730b 100644 --- a/packages/web-og/test/cards.render.test.ts +++ b/packages/web-og/test/cards.render.test.ts @@ -125,3 +125,31 @@ it('placeholder card (null result): renders a real non-empty PNG', async () => { const res = renderImage(null, { owner: 'facebook', repo: 'react', sha: 'abc1234' }); await expectValidCardPng(res, 'placeholder'); }); + +// Cache semantics against the REAL workers-og (the routing tests mock it, so +// they cannot see this). workers-og builds its response headers as +// { 'Content-Type': …, 'Cache-Control': , ...opts.headers } +// and object spread is case-SENSITIVE: a lowercase 'cache-control' from the +// caller does not replace the capitalized default, so BOTH keys reach +// `new Response`, where Headers merges them into one comma-joined value +// ("public, immutable, no-transform, max-age=31536000, public, max-age=60"). +// Caches read the first max-age, so the short-cached cards — placeholder, +// cold-lookup, the deploy-window notFound render — were pinned immutable for a +// year instead of 60s and could never refresh. Assert the header EXACTLY: a +// substring match passes on the merged value and would not have caught this. +it('placeholder card: cache-control is EXACTLY the short cache (no 1-year default merged in)', () => { + const res = renderImage(null, { owner: 'facebook', repo: 'react', sha: 'abc1234' }); + expect(res.headers.get('cache-control')).toBe('public, max-age=60'); +}); + +it('result card: cache-control is EXACTLY the long cache (no 1-year default merged in)', () => { + const res = renderImage( + result({ + input: { kind: 'commit', repo: repo('github.com', 'facebook/react'), sha: 'a'.repeat(40) }, + canonicalSha: 'a'.repeat(40), + firstRelease: { tag: 'v18.2.0', sha: 's', date: '2024-01-01T00:00:00Z', url: '' }, + }), + { owner: 'facebook', repo: 'react', sha: 'abc1234' }, + ); + expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); +}); diff --git a/packages/web-og/test/routing.test.ts b/packages/web-og/test/routing.test.ts index e04e891..c965b29 100644 --- a/packages/web-og/test/routing.test.ts +++ b/packages/web-og/test/routing.test.ts @@ -17,7 +17,20 @@ vi.mock('workers-og', () => ({ ImageResponse: class extends Response { constructor(node: unknown, init?: { headers?: Record }) { lastRenderedNode = node; - super('PNG-BYTES', { headers: init?.headers ?? {} }); + // Mirror workers-og's real header construction, including its own + // capitalized 'Cache-Control' default and the case-SENSITIVE spread of + // the caller's `headers` after it. The old mock passed `init.headers` + // straight through, which made every cache-control assertion below + // unfalsifiable: prod merged the library's 1-year immutable default in + // front of ours, while these tests read back exactly what the caller + // passed and stayed green. + super('PNG-BYTES', { + headers: { + 'Content-Type': 'image/png', + 'Cache-Control': 'public, immutable, no-transform, max-age=31536000', + ...(init?.headers ?? {}), + }, + }); } }, })); @@ -148,7 +161,7 @@ describe('web-og routing', () => { // The service binding was called. expect(env.WEB.fetch).toHaveBeenCalled(); // The cache-control should be the LONG one because we got a real result. - expect(res.headers.get('cache-control')).toMatch(/max-age=86400/); + expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); }); it('returns a placeholder PNG with SHORT cache when the service binding misses', async () => { @@ -158,7 +171,7 @@ describe('web-og routing', () => { env, ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toMatch(/max-age=60/); + expect(res.headers.get('cache-control')).toBe('public, max-age=60'); }); // Federated OG (issue #8): the /h/:host/r/:projectPath path renders unfurls for @@ -191,7 +204,7 @@ describe('web-og routing', () => { const calledUrl = String(calls[0]?.[0]); expect(calledUrl).toBe('https://web/internal/h/gitlab.gnome.org/r/GNOME%2Fgimp/a1b2c3d'); // Real result → long cache. - expect(res.headers.get('cache-control')).toMatch(/max-age=86400/); + expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); }); it('federated: rejects a non-.png URL with 404', async () => { @@ -209,7 +222,7 @@ describe('web-og routing', () => { env, ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toMatch(/max-age=60/); + expect(res.headers.get('cache-control')).toBe('public, max-age=60'); }); // Deploy-order safety: an unmatched .png (a stale crawler URL, or a permalink @@ -219,7 +232,7 @@ describe('web-og routing', () => { it('notFound: an unmatched .png renders a short-cached placeholder PNG, not 404', async () => { const res = await app.fetch(new Request('https://og.example/totally/unknown.png'), makeEnv()); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toMatch(/max-age=60/); + expect(res.headers.get('cache-control')).toBe('public, max-age=60'); const text = collectText(lastRenderedNode); expect(text).toContain('Looking up…'); }); @@ -290,7 +303,7 @@ describe('web-og card content', () => { 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')).toMatch(/max-age=86400/); + expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); }); it('placeholder card (binding miss): shows "Looking up…" and the owner/repo label', async () => { @@ -377,7 +390,7 @@ describe('web-og issue/PR cards (#79)', () => { expect(text).toContain('v0.0.11'); expect(text).toContain('honojs/hono'); // Real result → long cache. - expect(res.headers.get('cache-control')).toMatch(/max-age=86400/); + expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); }); it('pr route: calls /internal/pr/:owner/:repo/:number and renders "PR #N" + title', async () => { @@ -432,7 +445,7 @@ describe('web-og issue/PR cards (#79)', () => { const env = makeEnv(new Response('not found', { status: 404 })); const res = await app.fetch(new Request('https://og.example/i/honojs/hono/11.png'), env); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toMatch(/max-age=60/); + expect(res.headers.get('cache-control')).toBe('public, max-age=60'); const text = collectText(lastRenderedNode); expect(text).toContain('Looking up…'); expect(text.join(' ')).toContain('honojs/hono #11'); @@ -521,7 +534,7 @@ describe('web-og issue/PR cards (#79)', () => { env, ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toMatch(/max-age=60/); + expect(res.headers.get('cache-control')).toBe('public, max-age=60'); const text = collectText(lastRenderedNode); expect(text).toContain('Looking up…'); }); @@ -537,7 +550,7 @@ describe('web-og issue/PR cards (#79)', () => { env, ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toMatch(/max-age=60/); + expect(res.headers.get('cache-control')).toBe('public, max-age=60'); }); // A verbose issue/PR title (GitHub allows 256 chars) must not overflow the From 48919c90f9ec7fe8bd679d0b35e94bdde2ba98da Mon Sep 17 00:00:00 2001 From: liveapp-bot Date: Wed, 12 Aug 2026 11:20:41 -0400 Subject: [PATCH 2/4] fix(web-og): keep the static /placeholder.png on the long cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The un-merge fix in c102b89 corrected every short-cached card, but it also dropped /placeholder.png from an (accidentally) effectively-immutable header to public, max-age=60 — it routes through renderImage(null, …), so it inherits the null-result short cache. That route is the one null-result render that is not transient: it takes no owner/repo/sha, so the PNG is byte-identical on every request, and web only ever links it as /placeholder.png?v=${OG_TEMPLATE_VERSION} (og-meta.tsx:56), so a template change busts the URL rather than waiting out a TTL. At 60s every homepage/result-less unfurl re-runs a ~700ms satori+resvg wasm render for an image that cannot differ. renderImage takes an explicit cacheOverride and the two cache strings are hoisted to LONG_CACHE / SHORT_CACHE. Only the static route passes it; the genuinely transient null renders (service-binding miss, the notFound deploy-window path) keep the short cache, which is the point of c102b89. Co-Authored-By: Claude Opus 5 --- packages/web-og/src/index.tsx | 18 ++++++++++++++---- packages/web-og/test/routing.test.ts | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/packages/web-og/src/index.tsx b/packages/web-og/src/index.tsx index 548b59b..40ecf87 100644 --- a/packages/web-og/src/index.tsx +++ b/packages/web-og/src/index.tsx @@ -108,7 +108,15 @@ app.get('/h/:host/p/:projectPath/:numberPng', async (c) => { return renderImage(result, { owner, repo, number }); }); -app.get('/placeholder.png', () => renderImage(null, { owner: '', repo: '' })); +// The one null-result render that is NOT transient: no owner/repo/sha, so the +// PNG is byte-identical on every request, and `web` only ever links it as +// `/placeholder.png?v=${OG_TEMPLATE_VERSION}` (packages/web/src/ui/og-meta.tsx) +// — a template change busts the URL instead of waiting out a TTL. Inheriting +// the null-result SHORT_CACHE would re-run a ~700ms satori+resvg wasm render +// every 60s for an image that can never differ, so it opts into LONG_CACHE +// explicitly. The default stays short for the genuinely transient null +// renders (service-binding miss, the notFound deploy-window path below). +app.get('/placeholder.png', () => renderImage(null, { owner: '', repo: '' }, LONG_CACHE)); app.get('/healthz', (c) => c.text('ok')); @@ -127,14 +135,16 @@ export default app; // --- rendering --------------------------------------------------------------- +export const LONG_CACHE = `public, max-age=${24 * 60 * 60}, s-maxage=${24 * 60 * 60}`; +export const SHORT_CACHE = 'public, max-age=60'; + 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 longCache = `public, max-age=${24 * 60 * 60}, s-maxage=${24 * 60 * 60}`; - const shortCache = 'public, max-age=60'; - const cacheControl = result ? longCache : shortCache; + const cacheControl = cacheOverride ?? (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 c965b29..e266265 100644 --- a/packages/web-og/test/routing.test.ts +++ b/packages/web-og/test/routing.test.ts @@ -225,6 +225,21 @@ describe('web-og routing', () => { expect(res.headers.get('cache-control')).toBe('public, max-age=60'); }); + // The static /placeholder.png is the ONE null-result render that is NOT + // transient: it is byte-identical on every request (no owner/repo/sha), and + // `web` only ever links it with `?v=${OG_TEMPLATE_VERSION}` + // (packages/web/src/ui/og-meta.tsx), so a template change busts the URL + // rather than needing the TTL to expire. Short-caching it would re-run a + // ~700ms satori+resvg wasm render every 60s for an image that can never + // differ. It opts into the long cache explicitly — the null-result default + // stays SHORT for the genuinely transient callers (binding miss, notFound). + it('/placeholder.png: the static route gets the LONG cache, not the null-result short cache', async () => { + const res = await app.fetch(new Request('https://og.example/placeholder.png'), makeEnv()); + expect(res.status).toBe(200); + expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); + expect(collectText(lastRenderedNode)).toContain('Looking up…'); + }); + // Deploy-order safety: an unmatched .png (a stale crawler URL, or a permalink // OG URL hit during the web→web-og deploy window before web-og ships the // matching route) renders a placeholder PNG, not a 404 text body — so a social From e312411dfe54b6f6100b1e95941379f8fd7968b2 Mon Sep 17 00:00:00 2001 From: liveapp-bot Date: Wed, 12 Aug 2026 12:31:30 -0400 Subject: [PATCH 3/4] fix(web-og): gate /placeholder.png's long cache on a renderable version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit release.yml deploys `web` before `web-og`, so on an OG_TEMPLATE_VERSION bump `web` is already emitting `og:image=.../placeholder.png?v=og.vNEXT` while this Worker is still the old build. The old route ignored the query and stamped the 24h cache on a stale-template card — poisoning the busting URL itself, with no second URL left to bump. That is the same "one transient render freezes a wrong social card" class this PR exists to fix. Take the long cache only when the requested `v` is a version this build can actually render; anything else falls back to the 60s cache and self-heals once web-og lands. Co-Authored-By: Claude Opus 5 --- packages/web-og/src/index.tsx | 15 +++++++++++++- packages/web-og/test/routing.test.ts | 30 +++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/packages/web-og/src/index.tsx b/packages/web-og/src/index.tsx index 40ecf87..ba63081 100644 --- a/packages/web-og/src/index.tsx +++ b/packages/web-og/src/index.tsx @@ -116,7 +116,20 @@ app.get('/h/:host/p/:projectPath/:numberPng', async (c) => { // every 60s for an image that can never differ, so it opts into LONG_CACHE // explicitly. The default stays short for the genuinely transient null // renders (service-binding miss, the notFound deploy-window path below). -app.get('/placeholder.png', () => renderImage(null, { owner: '', repo: '' }, LONG_CACHE)); +// +// The long cache is gated on the requested version being one THIS build can +// render, because `release.yml` deploys `web` before `web-og`: on a template +// bump `web` emits `?v=og.vNEXT` while this Worker is still the old build, and +// long-caching that URL would pin a stale-template card for 24h with no second +// URL left to bust. Falling back to SHORT_CACHE self-heals 60s after web-og +// lands, then the version matches and the 24h cache resumes. +app.get('/placeholder.png', (c) => + renderImage( + null, + { owner: '', repo: '' }, + c.req.query('v') === OG_TEMPLATE_VERSION ? LONG_CACHE : SHORT_CACHE, + ), +); app.get('/healthz', (c) => c.text('ok')); diff --git a/packages/web-og/test/routing.test.ts b/packages/web-og/test/routing.test.ts index e266265..25314d3 100644 --- a/packages/web-og/test/routing.test.ts +++ b/packages/web-og/test/routing.test.ts @@ -2,6 +2,7 @@ // PNG rendering depends on WASM and the Workers runtime, which we verify with // `wrangler dev` rather than in vitest. +import { OG_TEMPLATE_VERSION } from '@released/core'; import { describe, expect, it, vi } from 'vitest'; // The last satori node tree handed to ImageResponse. The real PNG render is @@ -234,12 +235,39 @@ describe('web-og routing', () => { // differ. It opts into the long cache explicitly — the null-result default // stays SHORT for the genuinely transient callers (binding miss, notFound). it('/placeholder.png: the static route gets the LONG cache, not the null-result short cache', async () => { - const res = await app.fetch(new Request('https://og.example/placeholder.png'), makeEnv()); + const res = await app.fetch( + new Request(`https://og.example/placeholder.png?v=${OG_TEMPLATE_VERSION}`), + makeEnv(), + ); expect(res.status).toBe(200); expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); expect(collectText(lastRenderedNode)).toContain('Looking up…'); }); + // ...but only for a version THIS build can render. `release.yml` deploys + // `web` BEFORE `web-og`, so on a template bump `web` is already emitting + // `?v=og.vNEXT` while this Worker is still the OLD build. Long-caching that + // URL would pin a stale-template card for 24h at every scraper that unfurled + // during the deploy window — and the busting URL is already spent, so there + // is no second URL to bump. An unrenderable version falls back to the SHORT + // cache and self-heals 60s after web-og lands. + it('/placeholder.png: a version this build cannot render falls back to the SHORT cache', async () => { + const res = await app.fetch( + new Request('https://og.example/placeholder.png?v=og.vNEXT'), + makeEnv(), + ); + expect(res.status).toBe(200); + expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + }); + + // An unversioned hit is not a URL `web` ever emits (og-meta.tsx always + // appends `?v=`), so it gets no long-cache guarantee either. + it('/placeholder.png: an unversioned request gets the SHORT cache', async () => { + const res = await app.fetch(new Request('https://og.example/placeholder.png'), makeEnv()); + expect(res.status).toBe(200); + expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + }); + // Deploy-order safety: an unmatched .png (a stale crawler URL, or a permalink // OG URL hit during the web→web-og deploy window before web-og ships the // matching route) renders a placeholder PNG, not a 404 text body — so a social From ea85570d153a51fc602bc3953e7572b0a2d9720d Mon Sep 17 00:00:00 2001 From: liveapp-bot Date: Wed, 12 Aug 2026 19:36:57 -0400 Subject: [PATCH 4/4] fix(web-og): gate every card route on the template version, keep no-transform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on this PR's own diff. 1. The deploy-window version gate landed on `/placeholder.png` only, but `web` links EVERY card as `.png?v=${OG_TEMPLATE_VERSION}` (og-meta.tsx) and `release.yml` deploys `web` before `web-og`. So during the window the six dynamic routes (/r, /h/../r, /i, /p, /h/../i, /h/../p) still took the 24h cache for a `?v=og.vNEXT` URL rendered from the OLD template — pinning a stale card, on exactly the routes whose content differs per commit/issue/PR, with no second URL left to bust. The gate now lives in a shared `renderCard` helper the routes call: a `v` this build cannot render forces SHORT_CACHE and self-heals 60s after web-og lands. No `v` at all is not evidence of a mismatch, so it keeps the default. 2. `res.headers.set` replaces workers-og's whole default value, which silently dropped the `no-transform` that shipped on every OG response before this PR. Folded back into LONG_CACHE/SHORT_CACHE with a comment saying why: these PNGs are the byte-exact social card, and a transforming edge (Polish/Mirage) must not recompress them. --- packages/web-og/src/index.tsx | 48 +++++++-- packages/web-og/test/cards.render.test.ts | 6 +- packages/web-og/test/routing.test.ts | 120 +++++++++++++++++++--- 3 files changed, 150 insertions(+), 24 deletions(-) diff --git a/packages/web-og/src/index.tsx b/packages/web-og/src/index.tsx index ba63081..0639876 100644 --- a/packages/web-og/src/index.tsx +++ b/packages/web-og/src/index.tsx @@ -8,12 +8,37 @@ // (never a long-cached error). import { type LookupResult, OG_TEMPLATE_VERSION } from '@released/core'; -import { Hono } from 'hono'; +import { type Context, Hono } from 'hono'; import { ImageResponse } from 'workers-og'; import type { Env } from './env.js'; const app = new Hono<{ Bindings: Env }>(); +/** Render a card for a route that `web` links with `?v=${OG_TEMPLATE_VERSION}`. + * + * `release.yml` deploys `web` BEFORE `web-og`, so during the deploy window this + * build can be asked for a template version it cannot render: `web` is already + * emitting `?v=og.vNEXT` while this Worker is still the old build. Serving that + * URL from the old template under the 24h cache would pin a stale card in every + * downstream cache — and the version-busting URL is already spent, so there is + * no second URL left to bump. An unrenderable version falls back to SHORT_CACHE + * and self-heals 60s after web-og lands. + * + * No `v` at all is not evidence of a mismatch (a hand-typed or pre-#55 crawler + * URL), so it keeps the result-based default. */ +function renderCard( + c: Context, + result: LookupResult | null, + ctx: { owner: string; repo: string; sha?: string; number?: string }, +): Response { + const v = c.req.query('v'); + return renderImage( + result, + ctx, + v !== undefined && v !== OG_TEMPLATE_VERSION ? SHORT_CACHE : undefined, + ); +} + /** Fetch the result JSON from the `web` Worker via Service Binding. Returns null * on any miss/error so the caller renders a short-cached placeholder. */ async function fetchResult(env: Env, internalUrl: string): Promise { @@ -36,7 +61,7 @@ app.get('/r/:owner/:repo/c/:shaPng', async (c) => { const internalUrl = `https://web/internal/result/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${encodeURIComponent(sha)}`; const result = await fetchResult(c.env, internalUrl); - return renderImage(result, { owner, repo, sha }); + return renderCard(c, result, { owner, repo, sha }); }); // Federated permalinks (any non-GitHub provider, #8). projectPath is URL-encoded @@ -58,7 +83,7 @@ app.get('/h/:host/r/:projectPath/c/:shaPng', async (c) => { const slash = projectPath.indexOf('/'); const owner = slash === -1 ? projectPath : projectPath.slice(0, slash); const repo = slash === -1 ? '' : projectPath.slice(slash + 1); - return renderImage(result, { owner, repo, sha }); + return renderCard(c, result, { owner, repo, sha }); }); // GitHub issue/PR permalinks (#79): title-aware OG card. Fetches the result @@ -70,7 +95,7 @@ app.get('/i/:owner/:repo/:numberPng', async (c) => { const number = numberPng.slice(0, -4); const internalUrl = `https://web/internal/issue/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${encodeURIComponent(number)}`; const result = await fetchResult(c.env, internalUrl); - return renderImage(result, { owner, repo, number }); + return renderCard(c, result, { owner, repo, number }); }); app.get('/p/:owner/:repo/:numberPng', async (c) => { @@ -79,7 +104,7 @@ app.get('/p/:owner/:repo/:numberPng', async (c) => { const number = numberPng.slice(0, -4); const internalUrl = `https://web/internal/pr/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/${encodeURIComponent(number)}`; const result = await fetchResult(c.env, internalUrl); - return renderImage(result, { owner, repo, number }); + return renderCard(c, result, { owner, repo, number }); }); // Federated issue/PR permalinks (#79). projectPath URL-encoded into one segment, @@ -93,7 +118,7 @@ app.get('/h/:host/i/:projectPath/:numberPng', async (c) => { const slash = projectPath.indexOf('/'); const owner = slash === -1 ? projectPath : projectPath.slice(0, slash); const repo = slash === -1 ? '' : projectPath.slice(slash + 1); - return renderImage(result, { owner, repo, number }); + return renderCard(c, result, { owner, repo, number }); }); app.get('/h/:host/p/:projectPath/:numberPng', async (c) => { @@ -105,7 +130,7 @@ app.get('/h/:host/p/:projectPath/:numberPng', async (c) => { const slash = projectPath.indexOf('/'); const owner = slash === -1 ? projectPath : projectPath.slice(0, slash); const repo = slash === -1 ? '' : projectPath.slice(slash + 1); - return renderImage(result, { owner, repo, number }); + return renderCard(c, result, { owner, repo, number }); }); // The one null-result render that is NOT transient: no owner/repo/sha, so the @@ -148,8 +173,13 @@ export default app; // --- rendering --------------------------------------------------------------- -export const LONG_CACHE = `public, max-age=${24 * 60 * 60}, s-maxage=${24 * 60 * 60}`; -export const SHORT_CACHE = 'public, max-age=60'; +// `no-transform` is carried deliberately: workers-og's own default included it, +// and `res.headers.set` below replaces that value outright, so dropping it here +// would silently let a transforming edge (Polish/Mirage, or any proxy in front +// 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}`; +export const SHORT_CACHE = 'public, no-transform, max-age=60'; export function renderImage( result: LookupResult | null, diff --git a/packages/web-og/test/cards.render.test.ts b/packages/web-og/test/cards.render.test.ts index 963730b..92065c7 100644 --- a/packages/web-og/test/cards.render.test.ts +++ b/packages/web-og/test/cards.render.test.ts @@ -139,7 +139,7 @@ it('placeholder card (null result): renders a real non-empty PNG', async () => { // substring match passes on the merged value and would not have caught this. it('placeholder card: cache-control is EXACTLY the short cache (no 1-year default merged in)', () => { const res = renderImage(null, { owner: 'facebook', repo: 'react', sha: 'abc1234' }); - expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); }); it('result card: cache-control is EXACTLY the long cache (no 1-year default merged in)', () => { @@ -151,5 +151,7 @@ it('result card: cache-control is EXACTLY the long cache (no 1-year default merg }), { owner: 'facebook', repo: 'react', sha: 'abc1234' }, ); - expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); + expect(res.headers.get('cache-control')).toBe( + 'public, no-transform, max-age=86400, s-maxage=86400', + ); }); diff --git a/packages/web-og/test/routing.test.ts b/packages/web-og/test/routing.test.ts index 25314d3..89b2ab1 100644 --- a/packages/web-og/test/routing.test.ts +++ b/packages/web-og/test/routing.test.ts @@ -162,7 +162,9 @@ describe('web-og routing', () => { // The service binding was called. expect(env.WEB.fetch).toHaveBeenCalled(); // The cache-control should be the LONG one because we got a real result. - expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); + expect(res.headers.get('cache-control')).toBe( + 'public, no-transform, max-age=86400, s-maxage=86400', + ); }); it('returns a placeholder PNG with SHORT cache when the service binding misses', async () => { @@ -172,7 +174,7 @@ describe('web-og routing', () => { env, ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); }); // Federated OG (issue #8): the /h/:host/r/:projectPath path renders unfurls for @@ -205,7 +207,9 @@ describe('web-og routing', () => { const calledUrl = String(calls[0]?.[0]); expect(calledUrl).toBe('https://web/internal/h/gitlab.gnome.org/r/GNOME%2Fgimp/a1b2c3d'); // Real result → long cache. - expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); + expect(res.headers.get('cache-control')).toBe( + 'public, no-transform, max-age=86400, s-maxage=86400', + ); }); it('federated: rejects a non-.png URL with 404', async () => { @@ -223,7 +227,91 @@ describe('web-og routing', () => { env, ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); + }); + + // The deploy-window version gate is NOT specific to /placeholder.png: `web` + // links EVERY card as `.png?v=${OG_TEMPLATE_VERSION}` (og-meta.tsx), and + // release.yml deploys `web` before `web-og`, so during the window this build + // is asked for `?v=og.vNEXT` on the dynamic routes too — the ones whose + // content actually differs per commit/issue/PR. Long-caching a version-busted + // URL rendered from the OLD template pins a stale card for 24h with no second + // URL left to bust, so a version this build cannot render forces SHORT_CACHE. + const VERSIONED_CARD_ROUTES = [ + ['github commit', '/r/facebook/react/c/a1b2c3d.png'], + ['federated commit', '/h/gitlab.gnome.org/r/GNOME%2Fgimp/c/a1b2c3d.png'], + ['github issue', '/i/facebook/react/11.png'], + ['github PR', '/p/facebook/react/4834.png'], + ['federated issue', '/h/gitlab.com/i/gitlab-org%2Fgitlab-runner/39607.png'], + ['federated PR', '/h/gitlab.com/p/gitlab-org%2Fgitlab-runner/6867.png'], + ] as const; + + // A real (non-null) result, so the route takes the LONG cache branch — the + // only branch the version gate can change. + function realResultEnv(): ReturnType { + const sha40 = 'a'.repeat(40); + return makeEnv( + new Response( + JSON.stringify({ + input: { kind: 'commit', repo: { owner: 'facebook', repo: 'react' }, sha: sha40 }, + canonicalSha: sha40, + firstRelease: { tag: 'v1.0.0', sha: 's', date: '2024-01-01T00:00:00Z', url: '' }, + alsoIn: [], + releaseNotesHtml: null, + rateLimit: null, + }), + ), + ); + } + + for (const [label, path] of VERSIONED_CARD_ROUTES) { + it(`${label}: a version this build cannot render falls back to the SHORT cache`, async () => { + const res = await app.fetch( + new Request(`https://og.example${path}?v=og.vNEXT`), + realResultEnv(), + ); + expect(res.status).toBe(200); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); + }); + + it(`${label}: the CURRENT template version still gets the LONG cache`, async () => { + const res = await app.fetch( + new Request(`https://og.example${path}?v=${OG_TEMPLATE_VERSION}`), + realResultEnv(), + ); + expect(res.status).toBe(200); + expect(res.headers.get('cache-control')).toBe( + 'public, no-transform, max-age=86400, s-maxage=86400', + ); + }); + } + + // No `v` at all is not a URL `web` emits, but it is also not evidence of a + // template mismatch (a hand-typed or pre-#55 crawler URL) — it keeps the + // result-based default rather than being punished into the short cache. + it('dynamic card: an unversioned request keeps the LONG cache for a real result', async () => { + const res = await app.fetch( + new Request('https://og.example/r/facebook/react/c/a1b2c3d.png'), + realResultEnv(), + ); + expect(res.status).toBe(200); + expect(res.headers.get('cache-control')).toBe( + 'public, no-transform, max-age=86400, s-maxage=86400', + ); + }); + + // `Headers.set` replaces workers-og's whole default value, which silently + // dropped the `no-transform` that shipped on every OG response before the + // override fix. These PNGs are the byte-exact social card `web` links; a + // transforming edge (Polish/Mirage or any proxy) must not recompress them. + it('cache-control keeps no-transform on both the long and the short cache', async () => { + const long = await app.fetch( + new Request('https://og.example/r/facebook/react/c/a1b2c3d.png'), + realResultEnv(), + ); + const short = await app.fetch(new Request('https://og.example/placeholder.png'), makeEnv()); + expect(long.headers.get('cache-control')).toContain('no-transform'); + expect(short.headers.get('cache-control')).toContain('no-transform'); }); // The static /placeholder.png is the ONE null-result render that is NOT @@ -240,7 +328,9 @@ describe('web-og routing', () => { makeEnv(), ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); + expect(res.headers.get('cache-control')).toBe( + 'public, no-transform, max-age=86400, s-maxage=86400', + ); expect(collectText(lastRenderedNode)).toContain('Looking up…'); }); @@ -257,7 +347,7 @@ describe('web-og routing', () => { makeEnv(), ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); }); // An unversioned hit is not a URL `web` ever emits (og-meta.tsx always @@ -265,7 +355,7 @@ describe('web-og routing', () => { it('/placeholder.png: an unversioned request gets the SHORT cache', async () => { const res = await app.fetch(new Request('https://og.example/placeholder.png'), makeEnv()); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); }); // Deploy-order safety: an unmatched .png (a stale crawler URL, or a permalink @@ -275,7 +365,7 @@ describe('web-og routing', () => { it('notFound: an unmatched .png renders a short-cached placeholder PNG, not 404', async () => { const res = await app.fetch(new Request('https://og.example/totally/unknown.png'), makeEnv()); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); const text = collectText(lastRenderedNode); expect(text).toContain('Looking up…'); }); @@ -346,7 +436,9 @@ describe('web-og card content', () => { 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, max-age=86400, s-maxage=86400'); + expect(res.headers.get('cache-control')).toBe( + 'public, no-transform, max-age=86400, s-maxage=86400', + ); }); it('placeholder card (binding miss): shows "Looking up…" and the owner/repo label', async () => { @@ -433,7 +525,9 @@ describe('web-og issue/PR cards (#79)', () => { expect(text).toContain('v0.0.11'); expect(text).toContain('honojs/hono'); // Real result → long cache. - expect(res.headers.get('cache-control')).toBe('public, max-age=86400, s-maxage=86400'); + expect(res.headers.get('cache-control')).toBe( + 'public, no-transform, max-age=86400, s-maxage=86400', + ); }); it('pr route: calls /internal/pr/:owner/:repo/:number and renders "PR #N" + title', async () => { @@ -488,7 +582,7 @@ describe('web-og issue/PR cards (#79)', () => { const env = makeEnv(new Response('not found', { status: 404 })); const res = await app.fetch(new Request('https://og.example/i/honojs/hono/11.png'), env); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); const text = collectText(lastRenderedNode); expect(text).toContain('Looking up…'); expect(text.join(' ')).toContain('honojs/hono #11'); @@ -577,7 +671,7 @@ describe('web-og issue/PR cards (#79)', () => { env, ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); const text = collectText(lastRenderedNode); expect(text).toContain('Looking up…'); }); @@ -593,7 +687,7 @@ describe('web-og issue/PR cards (#79)', () => { env, ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toBe('public, max-age=60'); + expect(res.headers.get('cache-control')).toBe('public, no-transform, max-age=60'); }); // A verbose issue/PR title (GitHub allows 256 chars) must not overflow the