Public: anyone can read this; the crew can change it.
/changelog answers a repeat visit with 304: an ETag for every reader, not no-cache #11027
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/11023-changelog-answers-a-repeat-visit-with-30 → main 4 commits fork 2 threads · 2 unresolved
2 files
changed +179
−5
src/web/pages/changelog.ts +40 −5
| @@ -18,11 +18,13 @@ import { gather, plainTitle, type Gathered } from "../../story/gather.ts"; | ||
| 18 | 18 | import { liveRegionsJs } from "../client.ts"; |
| 19 | 19 | import { pageHead } from "../chrome.ts"; |
| 20 | 20 | import { changelogTag, holds } from "../feeds.ts"; |
| 21 | import { json } from "../../http/respond.ts"; | |
| 21 | import { EDGE_FRESH, EDGE_STALE, EDGE_VARY } from "../../http/edge_cache.ts"; | |
| 22 | import { json, securityFloor } from "../../http/respond.ts"; | |
| 23 | import { readFlash } from "../flash.ts"; | |
| 22 | 24 | import { shippedAnswer } from "../../release/answer.ts"; |
| 23 | 25 | import { html, type Html } from "../html.ts"; |
| 24 | 26 | import { page } from "../layout.ts"; |
| 25 | import { badge, row, seg, sha, time } from "../ui.ts"; | |
| 27 | import { badge, row, seg, sha, time, timeAgo } from "../ui.ts"; | |
| 26 | 28 | |
| 27 | 29 | /** Where the live client looks: a merge lands, or production serves a commit. */ |
| 28 | 30 | const LIVE = "pull.merged,app.serving"; |
| @@ -187,13 +189,20 @@ function renderRegion( | ||
| 187 | 189 | export async function changelogPage(ctx: Ctx): Promise<Response> { |
| 188 | 190 | notePhase(ctx.request, "session", takeSessionTime(ctx.request)); |
| 189 | 191 | const landed = await timePhaseAsync(ctx.request, "releases", () => readLandings(ctx.app)); |
| 190 | const rendering = performance.now(); | |
| 191 | 192 | const ownRepo = landed.own === undefined ? undefined : getRepo(ctx.app.db, ...(landed.own.split("/") as [string, string])); |
| 192 | 193 | const readable = ownRepo !== undefined && canRead(ctx.app.db, ownRepo, ctx.auth.user); |
| 193 | 194 | const { read, bets, bet } = withBets(ctx, landed, readable ? ownRepo : undefined); |
| 194 | 195 | const everything = [...read.onItsWay, ...read.live]; |
| 195 | 196 | const choice = chosenRepo(new URL(ctx.request.url).searchParams.get("repo"), everything, read.own); |
| 196 | 197 | const others = [...new Set(everything.map((l) => l.repo))].filter((r) => r !== read.own).slice(0, FILTERS); |
| 198 | // The tag comes from what the page would say, before any of it is rendered: | |
| 199 | // a match answers 304 without building the page. | |
| 200 | const toLive = newestLive(ctx.app.db); | |
| 201 | const flashed = readFlash(ctx.request) !== undefined; | |
| 202 | const tag = flashed ? undefined : changelogTag(read.version, everything, [choice, bet, read.commit, read.live.map((l) => `${l.repo}#${l.number}`), readable, toLive ?? null, everything.map((l) => timeAgo(l.at)), new Date().toISOString().slice(0, 10), ctx.auth.user?.id ?? null, ctx.auth.user?.isAdmin === true, ctx.theme ?? null]); | |
| 203 | const keep = tag === undefined ? undefined : cacheHeaders(ctx, tag); | |
| 204 | if (keep !== undefined && keep.matched) return keep.response; | |
| 205 | const rendering = performance.now(); | |
| 197 | 206 | const filters = seg([ |
| 198 | 207 | ...(read.own === undefined ? [] : [{ label: "This forge", href: "/changelog", current: choice === read.own }]), |
| 199 | 208 | { label: "Everything", href: "/changelog?repo=all", current: choice === "all" }, |
| @@ -207,10 +216,36 @@ export async function changelogPage(ctx: Ctx): Promise<Response> { | ||
| 207 | 216 | ...bets.map((b) => ({ label: b, href: `/changelog?bet=${encodeURIComponent(b)}`, current: bet === b })), |
| 208 | 217 | ]); |
| 209 | 218 | const body = html`${pageHead("Changelog", { sub: "What went live, as it happens." })}${filters}${betFilters} |
| 210 | <div id="shipped" data-live="${LIVE}" aria-live="polite">${shippedRegion(read, choice, new Date(), "UTC", newestLive(ctx.app.db), readable)}</div>`; | |
| 219 | <div id="shipped" data-live="${LIVE}" aria-live="polite">${shippedRegion(read, choice, new Date(), "UTC", toLive, readable)}</div>`; | |
| 211 | 220 | const response = page({ ctx, title: "Changelog", body, feed: { href: "/changelog.atom", title: "Changelog" }, json: { href: `/api/v1/changelog${choice === read.own ? "" : `?repo=${encodeURIComponent(choice)}`}`, title: "Changelog (JSON)" }, scriptJs: `${liveRegionsJs}\n${localDaysJs}` }); |
| 212 | 221 | notePhase(ctx.request, "render", performance.now() - rendering); |
| 213 | return response; | |
| 222 | if (keep === undefined) return response; | |
| 223 | const headers = new Headers(response.headers); | |
| 224 | for (const [k, v] of keep.headers) headers.set(k, v); | |
| 225 | return new Response(response.body, { status: response.status, headers }); | |
| 226 | } | |
| 227 | ||
| 228 | /** | |
| 229 | * What a tagged answer says about caching, and whether the reader already | |
| 230 | * holds it. The tag is of what the page shows (release, rows, filter, each | |
| 231 | * row's "5m ago" wording, who reads), so it dies when the page would change | |
| 232 | * and not before. A match is a 304 without rendering. Only a request carrying | |
| 233 | * no identity at all may be kept by a shared cache; any other answer is | |
| 234 | * `private`. A page that shows a flash is never tagged (the caller skips it). | |
| 235 | */ | |
| 236 | function cacheHeaders(ctx: Ctx, etag: string): { matched: boolean; response: Response; headers: Headers } | undefined { | |
| 237 | if (ctx.request.method !== "GET" && ctx.request.method !== "HEAD") return undefined; | |
| 238 | const h = ctx.request.headers; | |
| 239 | const anonymous = !h.has("cookie") && !h.has("authorization") && !h.has("x-plat-user") && !h.has("x-plat-edge-token") && ctx.auth.user === undefined; | |
| 240 | const headers = new Headers({ etag }); | |
| 241 | if (anonymous) { | |
| 242 | headers.set("cache-control", `public, max-age=0, s-maxage=${EDGE_FRESH}, stale-while-revalidate=${EDGE_STALE}`); | |
| 243 | headers.set("vary", EDGE_VARY); | |
| 244 | } else headers.set("cache-control", "private, no-cache"); | |
| 245 | const matched = holds(h.get("if-none-match"), etag); | |
| 246 | // The 304 carries the floor headers; the stored copy keeps the CSP it came with. | |
| 247 | const response = matched ? securityFloor(new Response(null, { status: 304, headers: new Headers(headers) })) : new Response(null); | |
| 248 | return { matched, response, headers }; | |
| 214 | 249 | } |
| 215 | 250 | |
| 216 | 251 | // GET /api/v1/changelog?repo=all|owner/name: the page's rows as JSON, public repositories only. |
test/changelog-cache.test.ts added +139
| @@ -0,0 +1,139 @@ | ||
| 1 | /** GET /changelog carries an ETag from what the page shows and answers a repeat with 304; only an anonymous answer is public (#11023). */ | |
| 2 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { createRepo } from "../src/actions/repos.ts"; | |
| 8 | import { createToken } from "../src/core/tokens.ts"; | |
| 9 | import { createUser } from "../src/core/users.ts"; | |
| 10 | import { forgetLive, keepLive } from "../src/release/landings.ts"; | |
| 11 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 12 | ||
| 13 | describe("GET /changelog caching", () => { | |
| 14 | let dataDir: string; | |
| 15 | let app: App; | |
| 16 | let servers: Servers; | |
| 17 | let token: string; | |
| 18 | const saved = process.env["FORGE_COMMIT"]; | |
| 19 | const PUBLIC = "public, max-age=0, s-maxage=60, stale-while-revalidate=300"; | |
| 20 | const get = (query = "", headers: Record<string, string> = {}) => fetch(`${servers.url}/changelog${query}`, { headers }); | |
| 21 | const signed = (extra: Record<string, string> = {}) => ({ authorization: `Bearer ${token}`, ...extra }); | |
| 22 | ||
| 23 | beforeAll(async () => { | |
| 24 | process.env["FORGE_COMMIT"] = "a".repeat(40); | |
| 25 | dataDir = await mkdtemp(join(tmpdir(), "forge-changelog-cache-")); | |
| 26 | app = createApp({ dataDir, port: 0, forgeRepo: "alice/forge" }); | |
| 27 | servers = await serve(app); | |
| 28 | const u = await createUser(app.db, { | |
| 29 | origin: "seed", trusted: true, username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true, | |
| 30 | }); | |
| 31 | if (!u.ok) throw new Error(u.error); | |
| 32 | token = createToken(app.db, u.value.id, "t", "rw").token; | |
| 33 | const made = await createRepo(app, u.value, { name: "forge", visibility: "public" }); | |
| 34 | if (!made.ok) throw new Error(String(made.error)); | |
| 35 | const other = await createRepo(app, u.value, { name: "notes", visibility: "public" }); | |
| 36 | if (!other.ok) throw new Error(String(other.error)); | |
| 37 | const insertIssue = app.db.query( | |
| 38 | `INSERT INTO issue (id, repo_id, number, kind, title, author_id, state) VALUES (?1, ?2, ?3, 'pull', ?4, ?5, 'closed')`, | |
| 39 | ); | |
| 40 | const insertPull = app.db.query( | |
| 41 | `INSERT INTO pull (issue_id, base_branch, head_branch, base_oid, head_oid, merged_at, merged_oid) VALUES (?1, ?2, 'b', 'base', '', ?3, ?4)`, | |
| 42 | ); | |
| 43 | insertIssue.run("i1", made.value.id, 1, "First", u.value.id); | |
| 44 | insertPull.run("i1", made.value.defaultBranch, "2026-10-01T10:00:00.000Z", "oid-1"); | |
| 45 | insertIssue.run("i2", made.value.id, 2, "Second", u.value.id); | |
| 46 | insertPull.run("i2", made.value.defaultBranch, "2026-10-02T10:00:00.000Z", "oid-2"); | |
| 47 | insertIssue.run("i3", other.value.id, 1, "Elsewhere", u.value.id); | |
| 48 | insertPull.run("i3", other.value.defaultBranch, "2026-09-30T10:00:00.000Z", "oid-3"); | |
| 49 | keepLive("alice/forge", ["oid-1"], 0); | |
| 50 | }, 30_000); | |
| 51 | ||
| 52 | afterAll(async () => { | |
| 53 | forgetLive(); | |
| 54 | if (saved === undefined) delete process.env["FORGE_COMMIT"]; | |
| 55 | else process.env["FORGE_COMMIT"] = saved; | |
| 56 | servers.stop(); | |
| 57 | app.close(); | |
| 58 | await rm(dataDir, { recursive: true, force: true }); | |
| 59 | }); | |
| 60 | ||
| 61 | test("a signed-in page is never public, and never carries the anonymous tag", async () => { | |
| 62 | const anon = await get(); | |
| 63 | const mine = await get("", signed()); | |
| 64 | expect(mine.status).toBe(200); | |
| 65 | const control = mine.headers.get("cache-control") ?? ""; | |
| 66 | expect(control).toContain("private"); | |
| 67 | expect(control).not.toContain("public"); | |
| 68 | expect(control).not.toContain("s-maxage"); | |
| 69 | expect(mine.headers.get("etag")).toMatch(/^W\//); | |
| 70 | expect(mine.headers.get("etag")).not.toBe(anon.headers.get("etag")); | |
| 71 | // The anonymous tag does not open the signed-in page. | |
| 72 | const crossed = await get("", signed({ "if-none-match": anon.headers.get("etag") ?? "" })); | |
| 73 | expect(crossed.status).toBe(200); | |
| 74 | // A cookie alone (a session the edge cannot see into) is not public either. | |
| 75 | const cookie = await get("", { cookie: "forge_theme=dark" }); | |
| 76 | expect(cookie.headers.get("cache-control") ?? "").not.toContain("public"); | |
| 77 | }); | |
| 78 | ||
| 79 | test("an anonymous page is public with an ETag, and a repeat is a 304 with the floor headers", async () => { | |
| 80 | const first = await get(); | |
| 81 | expect(first.status).toBe(200); | |
| 82 | expect(first.headers.get("cache-control")).toBe(PUBLIC); | |
| 83 | const etag = first.headers.get("etag") ?? ""; | |
| 84 | expect(etag).toMatch(/^W\//); | |
| 85 | await first.text(); | |
| 86 | const again = await get("", { "if-none-match": etag }); | |
| 87 | expect(again.status).toBe(304); | |
| 88 | expect(await again.text()).toBe(""); | |
| 89 | expect(again.headers.get("etag")).toBe(etag); | |
| 90 | expect(again.headers.get("cache-control")).toBe(PUBLIC); | |
| 91 | expect(again.headers.get("x-content-type-options")).toBe("nosniff"); | |
| 92 | expect(again.headers.get("referrer-policy")).not.toBeNull(); | |
| 93 | expect(again.headers.get("content-security-policy")).not.toBeNull(); | |
| 94 | }); | |
| 95 | ||
| 96 | test("a page that shows a flash is never tagged, and still clears it", async () => { | |
| 97 | const anon = await get(); | |
| 98 | const flash = `forge_flash=${encodeURIComponent(JSON.stringify({ message: "Saved" }))}`; | |
| 99 | // Even holding the anonymous tag, the flash is shown (200), untagged, and cleared. | |
| 100 | const shown = await get("", { cookie: flash, "if-none-match": anon.headers.get("etag") ?? "" }); | |
| 101 | expect(shown.status).toBe(200); | |
| 102 | expect(shown.headers.get("etag")).toBeNull(); | |
| 103 | expect(shown.headers.get("cache-control") ?? "").not.toContain("public"); | |
| 104 | expect(shown.headers.get("set-cookie") ?? "").toContain("forge_flash=;"); | |
| 105 | expect(await shown.text()).toContain("Saved"); | |
| 106 | }); | |
| 107 | ||
| 108 | test("a match is answered before the page is rendered", async () => { | |
| 109 | const etag = (await get()).headers.get("etag") ?? ""; | |
| 110 | const again = await get("", { "if-none-match": etag }); | |
| 111 | expect(again.status).toBe(304); | |
| 112 | expect(again.headers.get("server-timing") ?? "").not.toContain("render"); | |
| 113 | }); | |
| 114 | ||
| 115 | test("a signed-in repeat is a 304 too, still private", async () => { | |
| 116 | const first = await get("", signed()); | |
| 117 | const etag = first.headers.get("etag") ?? ""; | |
| 118 | await first.text(); | |
| 119 | const again = await get("", signed({ "if-none-match": etag })); | |
| 120 | expect(again.status).toBe(304); | |
outdated
No test for the flash branch (response already sets a cookie, so it is untagged). Add one that would fail without that guard. |
||
| 121 | expect(again.headers.get("cache-control") ?? "").toContain("private"); | |
| 122 | }); | |
| 123 | ||
| 124 | test("a filtered page has its own tag", async () => { | |
| 125 | const a = (await get()).headers.get("etag"); | |
| 126 | const b = (await get("?repo=all")).headers.get("etag"); | |
| 127 | expect(a).not.toBe(b); | |
| 128 | const stale = await get("?repo=all", { "if-none-match": a ?? "" }); | |
| 129 | expect(stale.status).toBe(200); | |
| 130 | }); | |
| 131 | ||
| 132 | test("the tag changes when a release lands", async () => { | |
| 133 | const before = (await get()).headers.get("etag") ?? ""; | |
| 134 | keepLive("alice/forge", ["oid-1", "oid-2"], 0); | |
| 135 | const after = await get("", { "if-none-match": before }); | |
| 136 | expect(after.status).toBe(200); | |
| 137 | expect(after.headers.get("etag")).not.toBe(before); | |
| 138 | }); | |
| 139 | }); |
Principle of the direction (fast, local-feeling): the 304 is built after the full render, so the server still pays for the page. Compute the tag first. Also the minute in the tag at the call site makes every tag short-lived.