perf(seo): send ETags on edge cached pages and answer 304

.github/workflows/deploy-cloudflare.yml

@@ -160,9 +160,10 @@ jobs:
       # Workers Cache stores `/skills/trending` (see routeRules in
       # nuxt.config.ts). This proves a repeat visit is a hit, and that no stored
       # response sets a cookie, hides Markdown from an agent, or carries a
-      # session. It fails the job without a rollback: the rollback smoke
-      # re-checks the previous version, and a version from before the rule
-      # could never pass these checks.
+      # session. It also proves a stored page answers its ETag with a 304, and
+      # that a 304 never replaces the stored page. It fails the job without a
+      # rollback: the rollback smoke re-checks the previous version, and a
+      # version from before the rule could never pass these checks.
       - name: Check the trending edge cache
         run: pnpm production:edge-cache
 

docs/arch/README.md

@@ -73,6 +73,7 @@ See [ADR-0007](../adr/0007-contract-in-the-cli-repository.md).
 - **Public pages render signed out**: the server never reads the session for a public page. The server render must not branch on User-Agent either, because `Vary` does not name it. Only then can one stored copy serve every visitor. A page that calls `useBotDetection()` on the server needs that call removed before it gets an edge cache rule. The browser loads the session after hydration. `useAuth().state` is `pending`, `anonymous`, or `signed-in`, and an auth-dependent control renders a same-size placeholder while it is `pending`. A page that needs the session on the server uses the `session` or `auth` route middleware, which also marks the response `private, no-store`.
 - **Session reads**: server code reads the session with `readUserSession()`. It opens a session only when the request carries one, because h3 sets a new session cookie on every other read. ESLint bans a direct `getUserSession()`.
 - **Markdown negotiation**: `shared/content-negotiation.ts` decides HTML or a 307 to the `.md` URL from `Accept` and `Sec-Fetch-Dest` only, and every page response names both in `Vary`.
+- **HTML validators**: `shared/html-validators.ts` gives a page under an `edgeCache` rule a weak ETag: the build id plus a hash of the HTML. A request that sends it back gets an empty 304. A request with a cookie or an `Authorization` header gets no ETag. No page sends `Last-Modified`, because no one date bounds what a page renders.
 
 ## URL canonicals
 

scripts/lib/edge-cache-check.ts

@@ -16,6 +16,9 @@
  * 6. Every board is stored under its own key: boards that differ only by query
  *    string render a different title or canonical URL. The bare path and the
  *    default range are one board, so they may share both.
+ * 7. A stored page carries an ETag, and a request that sends it back gets a
+ *    304. A 304 must never be stored as the page: after a conditional request
+ *    on a cold entry, the next plain request still gets the whole page.
  */
 
 import { varyKeysEdgeCache } from '../../shared/content-negotiation'
@@ -46,7 +49,7 @@ const SERVED_FROM_CACHE = new Set(['HIT', 'STALE', 'UPDATING'])
 
 export type EdgeCacheFetch = (input: string, init: RequestInit) => Promise<Response>
 
-export type EdgeCacheRequest = 'anonymous' | 'markdown' | 'session-cookie'
+export type EdgeCacheRequest = 'anonymous' | 'markdown' | 'session-cookie' | 'revalidation'
 
 export type EdgeCacheFailure
   = | { _tag: 'network-error', path: string, request: EdgeCacheRequest, message: string }
@@ -58,6 +61,9 @@ export type EdgeCacheFailure
     | { _tag: 'rendered-for-session', path: string, reason: string }
     | { _tag: 'www-redirect', path: string, status: number | null, location: string | null, message?: string }
     | { _tag: 'shared-cache-entry', paths: string[], identity: string | null }
+    | { _tag: 'no-etag', path: string }
+    | { _tag: 'not-revalidated', path: string, status: number }
+    | { _tag: 'stored-not-modified', path: string, status: number }
 
 export type EdgeCacheCheckResult
   = | { _tag: 'passed', checks: Array<{ path: string, cacheStatuses: string[] }> }
@@ -77,6 +83,8 @@ export interface EdgeCacheCheckDependencies {
    */
   hitAttempts?: number
   retryDelayMs?: number
+  /** Makes the cold cache entry for the revalidation check. Defaults to a random UUID. */
+  nonce?: string
 }
 
 type Fetched
@@ -227,6 +235,42 @@ async function checkSessionCookie(path: string, url: string, fetch: EdgeCacheFet
   return null
 }
 
+/**
+ * Runs after the anonymous check, so the entry is warm. Workers Cache stores
+ * one variant per `Accept` value, so an `Accept` that names the nonce reaches
+ * a cold entry that still renders HTML.
+ */
+async function checkRevalidation(path: string, url: string, fetch: EdgeCacheFetch, nonce: string): Promise<EdgeCacheFailure | null> {
+  const networkError = (message: string): EdgeCacheFailure => ({ _tag: 'network-error', path, request: 'revalidation', message })
+
+  const stored = await request(fetch, url, BROWSER_HEADERS)
+  if (stored._tag === 'error')
+    return networkError(stored.message)
+  if (stored.response.status !== 200)
+    return { _tag: 'status', path, request: 'revalidation', expected: 200, actual: stored.response.status }
+  const etag = stored.response.headers.get('etag')
+  if (!etag)
+    return { _tag: 'no-etag', path }
+
+  const conditional = await request(fetch, url, { ...BROWSER_HEADERS, 'if-none-match': etag })
+  if (conditional._tag === 'error')
+    return networkError(conditional.message)
+  if (conditional.response.status !== 304)
+    return { _tag: 'not-revalidated', path, status: conditional.response.status }
+
+  // Either a 200 or a 304 is right here. The plain request after it is the check.
+  const cold = { ...BROWSER_HEADERS, accept: `${BROWSER_HEADERS.accept},application/x-edge-check;q=0.1;n=${nonce}` }
+  const coldConditional = await request(fetch, url, { ...cold, 'if-none-match': etag })
+  if (coldConditional._tag === 'error')
+    return networkError(coldConditional.message)
+  const next = await request(fetch, url, cold)
+  if (next._tag === 'error')
+    return networkError(next.message)
+  if (next.response.status !== 200 || !next.body.includes('</html>'))
+    return { _tag: 'stored-not-modified', path, status: next.response.status }
+  return null
+}
+
 function defaultWwwBaseUrl(baseUrl: string): string | null {
   const { protocol, hostname } = new URL(baseUrl)
   return hostname === 'skilld.dev' ? `${protocol}//www.${hostname}` : null
@@ -251,6 +295,7 @@ export async function checkEdgeCache(dependencies: EdgeCacheCheckDependencies):
   const retryDelayMs = Math.max(0, Math.floor(dependencies.retryDelayMs ?? 5_000))
   const paths = dependencies.paths ?? EDGE_CACHED_PATHS
   const wwwBaseUrl = dependencies.wwwBaseUrl === undefined ? defaultWwwBaseUrl(dependencies.baseUrl) : dependencies.wwwBaseUrl
+  const nonce = dependencies.nonce ?? crypto.randomUUID()
 
   const failures: EdgeCacheFailure[] = []
   const checks: Array<{ path: string, cacheStatuses: string[] }> = []
@@ -264,6 +309,9 @@ export async function checkEdgeCache(dependencies: EdgeCacheCheckDependencies):
     else {
       checks.push({ path, cacheStatuses: anonymous.cacheStatuses })
       identities.set(anonymous.identity, [...identities.get(anonymous.identity) ?? [], path])
+      const revalidation = await checkRevalidation(path, url, fetch, nonce)
+      if (revalidation)
+        failures.push(revalidation)
     }
 
     // After the anonymous loop, so the apex entry is warm.

server/plugins/html-validators.ts

@@ -0,0 +1,34 @@
+import { getRequestHeader, getResponseHeader, setResponseHeader } from 'h3'
+import { useRuntimeConfig } from 'nitropack/runtime'
+import { decideHtmlValidators } from '#shared/html-validators'
+
+/**
+ * Give an edge cached page an ETag, and answer a request that already holds
+ * the page with an empty 304. `shared/html-validators.ts` decides both.
+ *
+ * The 304 keeps the route's cache headers, so a revalidating Workers Cache
+ * entry stays fresh for the route's lifetime again.
+ */
+export default defineNitroPlugin((nitroApp) => {
+  nitroApp.hooks.hook('render:response', async (response, { event }) => {
+    const header = (value: string | number | string[] | undefined): string | null =>
+      value === undefined ? null : Array.isArray(value) ? value.join(', ') : String(value)
+    const decision = await decideHtmlValidators({
+      buildId: useRuntimeConfig().app.buildId,
+      method: event.method,
+      status: response.statusCode ?? 200,
+      body: response.body,
+      request: { get: name => header(getRequestHeader(event, name)) },
+      response: { get: name => header(getResponseHeader(event, name)) },
+    })
+    if (decision._tag === 'skip')
+      return
+
+    setResponseHeader(event, 'etag', decision.etag)
+    if (decision._tag === 'not-modified') {
+      response.statusCode = 304
+      response.statusMessage = 'Not Modified'
+      response.body = ''
+    }
+  })
+})

shared/html-validators.ts

@@ -0,0 +1,103 @@
+/**
+ * ETags for the HTML pages Workers Cache stores, and the 304 that answers one.
+ *
+ * Without a validator, every recrawl downloads every page again, and Workers
+ * Cache replaces a stale page with a full render. With one, a crawler that
+ * sends `If-None-Match` gets a 304, and a revalidating cache keeps its body.
+ *
+ * The ETag is weak: a weak ETag promises the same page, not the same bytes.
+ * Cloudflare compresses responses and may rewrite a strong ETag. Each render
+ * also carries a fresh Sentry trace id, which is not page content.
+ *
+ * There is no `Last-Modified`. A Skill page shows stars, ages and related
+ * Skills, so no single date bounds what it renders. A date that misses a
+ * change makes a cache answer `If-Modified-Since` with a wrong 304. A request
+ * that sends only `If-Modified-Since` gets the full page.
+ */
+
+export interface HeaderReader {
+  get: (name: string) => string | null
+}
+
+export interface RenderedPage {
+  /** The Nuxt build id. A deploy changes it, so no ETag survives a deploy. */
+  buildId: string
+  method: string
+  status: number
+  /** A string for a whole rendered document. */
+  body: unknown
+  request: HeaderReader
+  response: HeaderReader
+}
+
+export type HtmlValidatorSkipReason = 'method' | 'status' | 'not-html' | 'credentials' | 'sets-cookie' | 'not-shared'
+
+export type HtmlValidatorDecision
+  = | { _tag: 'skip', reason: HtmlValidatorSkipReason }
+    | { _tag: 'send', etag: string }
+    | { _tag: 'not-modified', etag: string }
+
+/**
+ * The request headers `@harlan-zw/nuxt-cloudflare` treats as credentials.
+ * A page rendered for a request with one is never stored, so it gets no ETag.
+ */
+const CREDENTIAL_HEADERS = ['cookie', 'authorization', 'proxy-authorization', 'cf-access-jwt-assertion', 'cf-access-client-id', 'x-api-key']
+
+const HTML_TYPE_RE = /^(?:text\/html|application\/xhtml\+xml)\s*(?:;|$)/i
+
+/** Sentry writes these into every render with a new trace id. */
+const PER_RENDER_META_RE = /<meta name="(?:sentry-trace|baggage)" content="[^"]*"\s*\/?>/g
+
+const ENTITY_TAG_RE = /(?:W\/)?"[^"]*"/g
+
+export async function decideHtmlValidators(page: RenderedPage): Promise<HtmlValidatorDecision> {
+  if (page.method !== 'GET' && page.method !== 'HEAD')
+    return { _tag: 'skip', reason: 'method' }
+  if (page.status !== 200)
+    return { _tag: 'skip', reason: 'status' }
+  if (typeof page.body !== 'string' || !HTML_TYPE_RE.test(page.response.get('content-type') ?? ''))
+    return { _tag: 'skip', reason: 'not-html' }
+  if (CREDENTIAL_HEADERS.some(name => page.request.get(name)))
+    return { _tag: 'skip', reason: 'credentials' }
+  if (page.response.get('set-cookie'))
+    return { _tag: 'skip', reason: 'sets-cookie' }
+  if (!sharedCacheMayStore(page.response.get('cloudflare-cdn-cache-control')))
+    return { _tag: 'skip', reason: 'not-shared' }
+
+  const etag = `W/"${page.buildId}-${await contentHash(page.body.replace(PER_RENDER_META_RE, ''))}"`
+  return ifNoneMatchHits(page.request.get('if-none-match'), etag)
+    ? { _tag: 'not-modified', etag }
+    : { _tag: 'send', etag }
+}
+
+/**
+ * True when the edge policy lets Workers Cache store the page for one visitor
+ * and serve it to the next. Only such a page is the same for everyone.
+ */
+function sharedCacheMayStore(cacheControl: string | null): boolean {
+  let lifetime = 0
+  for (const directive of (cacheControl ?? '').toLowerCase().split(',')) {
+    const [name = '', value] = directive.trim().split('=', 2)
+    // `private="set-cookie"` keeps one header private, never the page.
+    if (name === 'no-store' || (name === 'private' && value === undefined))
+      return false
+    if (name === 'max-age' || name === 's-maxage')
+      lifetime = Math.max(lifetime, Number(value) || 0)
+  }
+  return lifetime > 0
+}
+
+/** Weak comparison (RFC 9110 section 8.8.3.2), which `If-None-Match` uses. */
+function ifNoneMatchHits(header: string | null, etag: string): boolean {
+  if (!header)
+    return false
+  if (header.trim() === '*')
+    return true
+  const opaque = etag.replace(/^W\//, '')
+  return (header.match(ENTITY_TAG_RE) ?? []).some(tag => tag.replace(/^W\//, '') === opaque)
+}
+
+async function contentHash(html: string): Promise<string> {
+  const digest = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(html))
+  return Array.from(new Uint8Array(digest).slice(0, 16), byte => byte.toString(16).padStart(2, '0')).join('')
+}

test/unit/edge-cache-check.test.ts

@@ -23,22 +23,28 @@ interface Answer {
   body?: string
 }
 
+const STORED_ETAG = 'W/"build-1-stored"'
+
 function response(answer: Answer, path = '/skills/trending'): Response {
   const headers = new Headers({
     'content-type': 'text/html;charset=utf-8',
     'vary': 'Accept, Sec-Fetch-Dest, Host',
+    ...((answer.status ?? 200) === 200 ? { etag: STORED_ETAG } : {}),
     ...answer.headers,
   })
   for (const cookie of answer.cookies ?? [])
     headers.append('set-cookie', cookie)
-  return new Response(answer.body ?? page(null, false, path), { status: answer.status ?? 200, headers })
+  const status = answer.status ?? 200
+  // A 304 has no body, and the Response constructor refuses one.
+  return new Response(status === 304 ? null : answer.body ?? page(null, false, path), { status, headers })
 }
 
 /**
  * A fake edge. Every anonymous request after the first to a path is a hit,
- * unless a test overrides one kind of request.
+ * and a request that sends the stored ETag back gets a 304, unless a test
+ * overrides one kind of request.
  */
-function edge(overrides: Partial<Record<'anonymous' | 'markdown' | 'session' | 'www', (path: string, seen: number) => Answer>> = {}): EdgeCacheFetch {
+function edge(overrides: Partial<Record<'anonymous' | 'markdown' | 'session' | 'www' | 'conditional', (path: string, seen: number) => Answer>> = {}): EdgeCacheFetch {
   const seen = new Map<string, number>()
   return async (url, init) => {
     const { pathname, search } = new URL(url)
@@ -63,6 +69,13 @@ function edge(overrides: Partial<Record<'anonymous' | 'markdown' | 'session' | '
         headers: { 'cf-cache-status': 'HIT' },
       }, path)
     }
+    if (headers.get('if-none-match')) {
+      return response(overrides.conditional?.(path, 0) ?? {
+        status: 304,
+        headers: { etag: STORED_ETAG },
+        body: '',
+      }, path)
+    }
     const count = (seen.get(path) ?? 0) + 1
     seen.set(path, count)
     return response(overrides.anonymous?.(path, count) ?? {
@@ -291,6 +304,50 @@ describe('checkEdgeCache', () => {
     })
   })
 
+  it('sends the stored page\'s ETag back and expects a 304', async () => {
+    const conditional: Array<string | null> = []
+    const inner = edge()
+    const result = await run(async (url, init) => {
+      const headers = new Headers(init.headers)
+      if (headers.get('if-none-match'))
+        conditional.push(headers.get('if-none-match'))
+      return inner(url, init)
+    })
+
+    expect(result._tag).toBe('passed')
+    expect(conditional).toEqual([STORED_ETAG, STORED_ETAG])
+  })
+
+  it('fails when a stored page carries no ETag', async () => {
+    const result = await run(edge({
+      anonymous: (_path, count) => ({ headers: { 'cf-cache-status': count === 1 ? 'MISS' : 'HIT', 'etag': '' } }),
+    }))
+
+    expect(result).toMatchObject({ _tag: 'failed', failures: [{ _tag: 'no-etag', path: '/skills/trending' }] })
+  })
+
+  it('fails when the edge answers a matching ETag with the whole page', async () => {
+    const result = await run(edge({ conditional: () => ({ headers: { 'cf-cache-status': 'HIT' } }) }))
+
+    expect(result).toMatchObject({ _tag: 'failed', failures: [{ _tag: 'not-revalidated', path: '/skills/trending', status: 200 }] })
+  })
+
+  it('fails when a conditional request on a cold entry leaves a 304 for the next visitor', async () => {
+    const inner = edge()
+    const storedNotModified = new Set<string>()
+    const result = await run(async (url, init) => {
+      const headers = new Headers(init.headers)
+      const variant = headers.get('accept') ?? ''
+      if (headers.get('if-none-match') && variant.includes('x-edge-check'))
+        storedNotModified.add(variant)
+      if (!headers.get('if-none-match') && storedNotModified.has(variant))
+        return response({ status: 304, body: '' })
+      return inner(url, init)
+    })
+
+    expect(result).toMatchObject({ _tag: 'failed', failures: [{ _tag: 'stored-not-modified', path: '/skills/trending', status: 304 }] })
+  })
+
   it('reports a network error instead of throwing', async () => {
     const result = await run(async () => {
       throw new Error('connect ECONNREFUSED')