diff --git a/README.md b/README.md index 9f2928c..5cc5a20 100644 --- a/README.md +++ b/README.md @@ -136,6 +136,19 @@ pnpm --filter @released/web-og dev # wrangler dev for web-og pnpm --filter git-released dev -- # tsx-run the CLI in place ``` +Working on OG rendering locally? Put this in `packages/web/.dev.vars`: + +``` +PUBLIC_BASE_URL=http://localhost:8787 +``` + +`/internal/*` (what web-og calls) keys the result cache on the deployment's own +public origin. `wrangler dev` loads `[vars]`, where `PROD_HOST` is set, so +without the override the OG path keys on `https://released.blabberate.com` while +your public routes key on `http://localhost:8787`: two namespaces on one +machine, and every local OG request pays a full lookup instead of hitting the +slot the permalink just warmed. Prod and the preview Worker set it themselves. + ### Local checks reference | Command | What it does | Gate? | diff --git a/packages/web/src/cache.ts b/packages/web/src/cache.ts index ca7def8..8376f20 100644 --- a/packages/web/src/cache.ts +++ b/packages/web/src/cache.ts @@ -15,8 +15,16 @@ import type { CacheStore } from '@released/core'; -/** A cached value plus how long ago it was written (seconds). */ -export type CacheEntry = { value: T; ageSeconds: number }; +/** A cached value, how long ago it was written (seconds), and the raw + * `x-cached-at` wall-clock stamp it was derived from (null for an entry + * written before this header existed). + * + * `ageSeconds` is floored from a `Date.now()` sample taken at READ time, so two + * entries read one Cache API round trip apart carry ages sampled at two + * different instants — comparing them can invert the true write order when the + * floors straddle a second boundary. `stampedAt` is the write-time instant + * itself, so an ordering test between two entries is exact. */ +export type CacheEntry = { value: T; ageSeconds: number; stampedAt: number | null }; export type WorkerCache = CacheStore & { /** Like get(), but also reports the entry's age so callers can judge staleness. */ @@ -53,11 +61,10 @@ export function makeWorkerCache(req: Request, ttlSecondsDefault = 1800): WorkerC return null; } const stamped = Number(res.headers.get('x-cached-at')); + const stampedAt = Number.isFinite(stamped) && stamped > 0 ? stamped : null; const ageSeconds = - Number.isFinite(stamped) && stamped > 0 - ? Math.max(0, Math.floor((Date.now() - stamped) / 1000)) - : 0; - return { value, ageSeconds }; + stampedAt === null ? 0 : Math.max(0, Math.floor((Date.now() - stampedAt) / 1000)); + return { value, ageSeconds, stampedAt }; }, async put(key: string, value: T, ttlSeconds?: number): Promise { diff --git a/packages/web/src/resolve.ts b/packages/web/src/resolve.ts index f4f4fa1..e8a9cbc 100644 --- a/packages/web/src/resolve.ts +++ b/packages/web/src/resolve.ts @@ -13,6 +13,9 @@ // 3. Don't hammer a down host. A transient failure writes a short-lived // "negative" marker; while it's warm we skip the upstream call entirely // (serving stale if we have it, otherwise a soft "checking…" transient). +// A caller whose consumer asks only ONCE can opt out of the cold half of +// that (`bypassBackOffWhenUnservable`) — backing off is cheap for a page a human +// can reload and permanent for a crawler that unfurls once. // // "not yet released" is a thrown NotYetReleasedError (not a cacheable result), // so it surfaces as its own status and, during an outage with no prior, degrades @@ -34,6 +37,23 @@ const HARD_TTL_RELEASED = 30 * 24 * 60 * 60; // terminal — keep ~30 days const HARD_TTL_PENDING = 24 * 60 * 60; // long enough to stale-serve through an outage const HARD_TTL_PARTIAL = 60; // partial is itself a soft state; don't trust it long const NEG_TTL = 60; // back off this long when upstream is down +// Upper bound on the age of a prior we will hand to a consumer that PINS what we +// give it. web-og long-caches any non-null result for 24h (renderImage: +// `result ? longCache : shortCache`), so an answer served to it is stuck in the +// crawler's cache for a day and no later refresh can invalidate a PNG already +// rendered from it — a "not yet released" prior that has since shipped would +// show the wrong card until tomorrow. 30 minutes is what /internal used as its +// flat TTL before it shared this 24h slot, so bounding is never worse than the +// code it replaced. +// +// It bounds EVERY exit that hands back a prior. Sharing the 24h slot is exactly +// what made a 23h-old prior possible on the /internal path (`consumerPinsResult`, +// set by the OG route): before it, that route had a flat 30-minute TTL of its own +// and an entry that old could not exist. Public HTML +// routes leave the flag unset and keep stale-if-error UNBOUNDED: a human sees an +// explicit "stale as of" caveat and can reload, and their answer is not pinned +// anywhere, so serving through a long outage is the right degrade for them. +const MAX_STALE_PINNED = 30 * 60; // Error kinds a later retry might succeed on → eligible for stale-serve (when we // have a prior) or a short negative cache (when we don't). Everything else is a @@ -64,11 +84,54 @@ function isFresh(entry: CacheEntry): boolean { return entry.ageSeconds < FRESH_WINDOW_PENDING; } +type PinnedPartialMarker = { pinnedPartial: true }; type NegMarker = { transient: true; kind: string; status?: number; anubis?: boolean }; function negKey(key: string): string { return `${key}:neg`; } +/** Companion marker for `${key}`, written ONLY when a `consumerPinsResult` + * caller computed a partial itself. The result slot is shared across callers + * that do not agree on a soft deadline, so the entry alone cannot say whose + * truncation produced it; this marker can, and it expires on its own after + * `HARD_TTL_PARTIAL`. */ +function partialKey(key: string): string { + return `${key}:pinpartial`; +} + +/** True when `mark` was written no earlier than `entry`. + * + * Compare the STORED `x-cached-at` stamps, not the two `ageSeconds` values. + * `cache.getEntry` floors each age from a `Date.now()` sample taken when THAT + * entry was read, and the two reads are one Cache API round trip apart, so the + * floors can straddle a second boundary and invert the ordering for a pair that + * really was written entry-then-marker: + * + * entry stamped t=1000ms, read at 2990ms -> floor(1990/1000) = 1 + * marker stamped t=1010ms, read at 3015ms -> floor(2005/1000) = 2 + * + * `2 <= 1` is false, so the caller would disown its own 2-second-old partial and + * run another full traversal — precisely the traversal this throttle exists to + * avoid, on the deadline-blowing repos it exists for. The write-time stamps say + * 1010 >= 1000, which is the quantity the guard actually reasons about. + * + * With either stamp missing (an entry written by a version that predates the + * `x-cached-at` header, or one whose header an intermediary dropped) the + * ordering is unprovable, so this says no and the partial is recomputed. + * + * It deliberately does NOT fall back to comparing the two `ageSeconds`. That + * reads as the conservative choice and is the opposite of one: `cache.getEntry` + * reports `ageSeconds: 0` for an unstamped entry (cache.ts:65-66), so an + * unstamped pair evaluated `0 <= 0` -> true and the marker vouched for the entry + * unconditionally — someone else's truncation trusted rather than recomputed. + * Unreachable in production, where every write goes through + * `makeWorkerCache.put` and is stamped; reachable from a seeded fixture, which + * is exactly where a claim like this one gets believed. */ +function writtenNoEarlierThan(mark: CacheEntry, entry: CacheEntry): boolean { + if (mark.stampedAt === null || entry.stampedAt === null) return false; + return mark.stampedAt >= entry.stampedAt; +} + export type Resolved = | { status: 'ok'; @@ -90,12 +153,148 @@ export async function resolveLookup(args: { key: string; load: () => Promise; now?: () => number; + /** Opt-in for callers whose consumer only ever asks ONCE, so a soft failure + * becomes permanent for them (the OG crawler). When set, the shared negative + * back-off marker is honoured only if there is a prior we can actually SERVE + * — with nothing servable, an attempt beats handing back a placeholder that + * gets cached forever. A bypassed attempt that then FAILS does not re-stamp + * the marker (`!backedOff`, see the catch below), so opting in never resets + * the clock the public routes read; a failure on the ordinary path still + * writes it. Callers that can retry (the public HTML routes) omit this flag + * and keep backing off. + * + * Be honest about the reachable behaviour. The only caller that sets this also + * sets `consumerPinsResult`, and `findRelease` emits just two entry shapes: + * TERMINAL (fresh forever, so it returns at the fresh exit above) and PARTIAL + * (handed back inside its own 60s throttle, recomputed after). Neither can reach the stale-serve on + * the back-off line below, so on that path the bypass is UNCONDITIONAL, not + * cold-only. That is accepted, not overlooked: a crawler asks once, so the + * alternative is a placeholder pinned long after the host recovers, and the + * cost is the unthrottled load documented at the fall-through. The + * `prior && !shouldRecompute(prior)` stale-serve is live for the public routes, + * which do not set either flag. */ + bypassBackOffWhenUnservable?: boolean; + /** Opt-in for callers whose consumer CACHES whatever we hand back, for longer + * than we can correct (the OG crawler pins a rendered PNG for 24h). + * + * What it does TODAY is entirely the PARTIAL handling at `shouldRecompute` + * below: someone else's truncated traversal is recomputed rather than served, + * and one this caller itself produced within `HARD_TTL_PARTIAL` is handed back + * so the caller can refuse it (internal.ts:285) without paying another + * traversal. A TERMINAL answer is servable at any age and is deliberately + * outside every bound. Refusing to PIN a partial is the caller's own job, not + * this flag's. + * + * It ALSO bounds a PENDING prior (no `firstRelease`, no `partial`) at + * `MAX_STALE_PINNED`, so a day-old "not yet released" that has since shipped is + * never handed to a pinning consumer. Be honest about that arm: `findRelease` + * cannot currently emit that shape — its three value returns are the gallop hit + * (`find-release.ts:299`, always `partial`), the soft-deadline miss (`:316`, + * always `partial`) and the terminal answer (`:391`); a genuine "not yet + * released" is THROWN (`:326`) and `resolve.ts` returns it as `status: + * 'not_yet'` without ever calling `cache.put`. So no PENDING entry exists to + * bound, and the arm is a defensive guard on a shape the TYPE permits (core + * keeps a matching fallback at `:486`), not a live protection. The failure it + * is named for is real but is fixed elsewhere: the 24h pin of a fresh answer is + * #151, and the `not_yet` 503 is #150. Do not read this bound as covering the + * OG path. (`HARD_TTL_PENDING` and `FRESH_WINDOW_PENDING` are dead for the same + * reason, and predate this PR.) */ + consumerPinsResult?: boolean; + /** Override the in-isolate single-flight key, which otherwise IS `key`. + * + * `singleFlight` hands every joiner the FIRST registrant's promise and runs + * only that owner's `load`, so two callers sharing a key also share a loader — + * including its deadlines. Callers on this same cache slot do not agree on + * those: badge.ts runs an 8s/9s findRelease so a slow repo returns a + * short-cached "checking…", while the permalink and /internal callers run the + * 24s/28s defaults. Sharing a slot is deliberate (that is the whole point of a + * common key); sharing a TRUNCATION is not, so a caller whose consumer cannot + * caveat a partial passes its own flight key and always runs its own lookup. + * Concurrent calls from that same caller still collapse into one. */ + flightKey?: string; }): Promise { - const { cache, key, load } = args; + const { cache, key, load, bypassBackOffWhenUnservable, consumerPinsResult } = args; + const flightKey = args.flightKey ?? key; const now = args.now ?? Date.now; const prior = await cache.getEntry(key); - if (prior && isFresh(prior)) { + + /** True when the partial in the slot is one THIS caller computed less than + * `HARD_TTL_PARTIAL` ago — the only case the throttle below is allowed to + * honour. + * + * Aligning the key put `/internal` on the same slot as badge.ts and the + * permalink pages, and those callers do not agree on a soft deadline + * (badge.ts runs 8s/9s, everyone else 24s/28s). Round 11 stopped the + * truncation travelling through the in-isolate FLIGHT (`flightKey`); this + * stops it travelling through the CACHE, which is the same hole one hop + * later. Without it: a README badge on `/i/kubernetes/kubernetes/12345` + * makes camo fetch `badge.svg`, badge.ts's 8s deadline truncates and writes + * a partial to the byte-identical key, and an unfurl ten seconds later reads + * that partial as "recent", 503s it, and Slack pins the neutral placeholder + * — on a link where this route's own 24s deadline finds the answer. On + * `main` that could not happen, because /internal keyed on a key of its own. + * + * Read once, here, and only when there IS a prior partial to throttle, so + * the common paths pay no extra cache read. `run()`'s re-read uses the same + * value: a partial that appeared in between is by definition not one this + * call throttled, and recomputing it is the safe direction. */ + let ownRecentPartial = false; + if (consumerPinsResult && prior?.value.partial) { + const mark = await cache.getEntry(partialKey(key)); + ownRecentPartial = + mark?.value?.pinnedPartial === true && + mark.ageSeconds < HARD_TTL_PARTIAL && + // ...and the marker has to actually IDENTIFY the entry it vouches for, not + // merely be young. `run()` writes the slot first and the marker second, so + // for a pair this caller produced the marker can never be OLDER than the + // entry. When it is, the slot was overwritten AFTER we marked it — by a + // caller on the same key with a different deadline (badge.ts, 8s) — so the + // partial sitting there is someone else's truncation wearing our marker. + // Recompute it, which is exactly what the marker exists to make possible. + writtenNoEarlierThan(mark, prior); + } + + /** True when a cached entry must NOT be served to this caller and the lookup + * has to run again. Named for what it decides, not for pinning-safety: it is + * NOT the invariant "this entry may be pinned". A pinning caller still has to + * make its own call on what it does with a partial it is handed — see the + * refusal at internal.ts:285 — because inside the throttle window below this + * predicate deliberately returns false for one. Always false for callers that + * did not opt in, so the public HTML routes are unchanged. */ + const shouldRecompute = (entry: CacheEntry): boolean => { + if (!consumerPinsResult) return false; + // TERMINAL (`firstRelease`, no `partial`) — always servable, at any age. + // Which release first contains a commit cannot change, which is why + // `isFresh()` treats it as fresh forever and `hardTtlFor()` keeps it 30 + // days. `MAX_STALE_PINNED` exists for the opposite case, a "not yet + // released" prior that has since shipped; applying it here would discard + // exactly the warm entries this route joined the public key to reuse, + // paying a full findRelease per unfurl. + if (entry.value.firstRelease && !entry.value.partial) return false; + // PARTIAL (either shape) — a truncated traversal. With `firstRelease: null` + // it means "we stopped looking", which web-og renders as a definite "not yet + // released"; WITH a `firstRelease` it carries the gallop hit, and the bisect + // that would confirm no EARLIER release contains the commit is what the + // deadline cut short (find-release.ts:288-292). The result card renders that + // caveat, web-og cannot. So the caller must refuse to PIN either shape — but + // recomputing one this caller itself produced under its OWN deadline, within + // its 60s TTL, would only reproduce it, so inside that window we hand it back + // (the caller 503s it into a short-cached placeholder) instead of running a + // full traversal on the shared token for every crawler. Someone ELSE's + // partial earns no such trust: recompute it. + if (entry.value.partial) return !ownRecentPartial; + // PENDING (no `firstRelease`, no `partial`) — bounded by `MAX_STALE_PINNED`: + // an answer older than that may have shipped since, and a PNG already + // rendered from it cannot be invalidated. DEFENSIVE ONLY: `findRelease` emits + // no such entry today (a real "not yet released" is thrown, never cached — see + // the `consumerPinsResult` doc above), so this line does not fire in + // production. It stays because the TYPE permits the shape and core keeps a + // fallback branch that would return it (`find-release.ts:486`); it must not be + // read as the bound that protects the OG path. + return entry.ageSeconds >= MAX_STALE_PINNED; + }; + if (prior && isFresh(prior) && !shouldRecompute(prior)) { return { status: 'ok', result: prior.value, stale: false, staleAsOf: null, cached: true }; } @@ -114,23 +313,88 @@ export async function resolveLookup(args: { const backedOff = Boolean(neg?.value?.transient) && (neg?.ageSeconds ?? Number.POSITIVE_INFINITY) < NEG_TTL; if (backedOff) { - if (prior) return staleHit(); - return { - status: 'transient', - kind: neg?.value.kind ?? 'provider_server_error', - upstreamStatus: neg?.value.status, - anubis: neg?.value.anubis, - }; + if (prior && !shouldRecompute(prior)) return staleHit(); + if (!bypassBackOffWhenUnservable) { + return { + status: 'transient', + kind: neg?.value.kind ?? 'provider_server_error', + upstreamStatus: neg?.value.status, + anubis: neg?.value.anubis, + }; + } + // Cold + opted out: fall through and attempt the load. This is NOT throttled. + // singleFlight collapses only CONCURRENT calls, and only inside ONE isolate + // (see its header), so during a host outage every cold unfurl — a different + // colo, a different social platform, or simply a later one — runs its own + // findRelease out to the hard deadline against the down host and re-stamps + // the marker. NEG_TTL throttles the human page views on this key, not this + // path. That cost is accepted deliberately: the crawler asks ONCE, so the + // alternative is a placeholder pinned in its cache long after the host + // recovers. Gating the bypass on a fraction of NEG_TTL would only move which + // unfurls get the permanent placeholder, not stop them. + // + // What it must NOT also cost is the humans' recovery window. A bypassed load + // that fails does NOT re-stamp the marker (see the catch below): re-stamping + // would reset the age that the callers who DO honour the marker — the public + // HTML routes, on this same shared key — read, so under a steady crawler + // cadence (a link circulating on Slack unfurls roughly once a minute) the + // marker would never reach NEG_TTL and a human would sit on the "checking..." + // card for the whole outage rather than getting a fresh attempt each minute. + // The marker now ages out on its own clock, as it does on `main`, where + // /internal kept a negative marker of its own. #157 tracks the wider question + // of what the unthrottled bypass should cost; this is the half the shared key + // introduced. } try { - const result = await singleFlight(key, async () => { + const run = async () => { const re = await cache.getEntry(key); - if (re && isFresh(re)) return re.value; + if (re && isFresh(re) && !shouldRecompute(re)) return re.value; const r = await load(); + // The ENTRY's TTL is the caller-independent one, always. This slot is SHARED + // with badge.ts and the permalink pages, so a TTL picked to suit this caller + // is silently imposed on theirs. `hardTtlFor()` puts a gallop partial + // (`firstRelease` set + `partial: soft_deadline`) on the terminal 30-day + // branch; writing HARD_TTL_PARTIAL here instead would drop the PUBLIC routes' + // effective TTL on that shape from 30 days to 60 seconds for as long as a + // link is being unfurled — one OG unfurl replacing the month-long entry a + // human page view just wrote, so the next page view pays another full + // traversal and issue.tsx/pr.tsx's bot branch (`if (!cached) return + // renderDeferred(...)`) falls back to the deferred card off a slot that was + // warm a minute ago. That misclassification is a real defect — it is just not + // this caller's to fix on someone else's entry. It is tracked in #155/#159, + // and the public routes' semantics are deliberately untouched here. + // + // Be explicit about what this PR does change, which is not the + // misclassification but WHO can trigger it. On `main` /internal keyed on a + // key of its own, so only a human page view or a badge fetch could write a + // gallop partial into the shared slot. Sharing the key makes an UNFURL a + // writer of it: one Slack post of a deadline-blowing repo can now pin + // `badge.svg` and the permalink to an unconfirmed gallop tag — rendered + // with no caveat, because a badge has nowhere to put one — for the full 30 + // days, and each further unfurl restarts that clock. Accepting it here is a + // scope call, not a claim that it is harmless: the fix belongs in + // `hardTtlFor()`/`isFresh()`, on the public routes' side, where #155/#159 + // can be reviewed as the behaviour change to those routes that it is. + // + // What THIS caller needs — never trusting a partial for longer than + // HARD_TTL_PARTIAL — rides on the companion marker below instead. The marker + // is caller-private and expires on its own clock, and `shouldRecompute` reads + // the MARKER, never the entry's TTL, so the 60s throttle window is exactly + // what it was. + const pinnedPartial = Boolean(consumerPinsResult && r.partial); await cache.put(key, r, hardTtlFor(r)); + // Record WHOSE truncation this was, so the throttle above honours it only for + // this caller, and for no longer than HARD_TTL_PARTIAL. The entry may now + // outlive the marker (a gallop partial keeps hardTtlFor's 30 days); that is + // the safe direction — no marker means `ownRecentPartial` is false and the + // partial is recomputed rather than served to a pinning consumer. + if (pinnedPartial) { + await cache.put(partialKey(key), { pinnedPartial: true }, HARD_TTL_PARTIAL); + } return r; - }); + }; + const result = await singleFlight(flightKey, run); return { status: 'ok', result, stale: false, staleAsOf: null, cached: false }; } catch (err) { if (err instanceof NotYetReleasedError) return { status: 'not_yet', error: err }; @@ -138,12 +402,21 @@ export async function resolveLookup(args: { // Throttle the next retry, then serve last-known-good if we have it. const upstreamStatus = upstreamStatusOf(err); const anubis = err instanceof ProviderJsonError && err.looksLikeAnubis; - await cache.put( - negKey(key), - { transient: true, kind: err.kind, status: upstreamStatus, anubis }, - NEG_TTL, - ); - if (prior) return staleHit(); + // ...but never re-stamp a marker this call walked straight past. `backedOff` + // is still true here only on the bypass fall-through above, which means the + // marker was already warm and this caller ignored it. Writing it again resets + // its age for everyone reading the shared key, which is what would starve the + // public routes' retry window (see the fall-through comment). Skipping the + // write costs this caller nothing — it does not read the marker on this path, + // and the warm marker it bypassed already records the host as down. + if (!backedOff) { + await cache.put( + negKey(key), + { transient: true, kind: err.kind, status: upstreamStatus, anubis }, + NEG_TTL, + ); + } + if (prior && !shouldRecompute(prior)) return staleHit(); return { status: 'transient', kind: err.kind, upstreamStatus, anubis }; } return { status: 'error', error: err }; diff --git a/packages/web/src/routes/internal.ts b/packages/web/src/routes/internal.ts index 3fb52d0..91a1306 100644 --- a/packages/web/src/routes/internal.ts +++ b/packages/web/src/routes/internal.ts @@ -4,12 +4,13 @@ // web-og calls these via a Cloudflare Service Binding (env.WEB.fetch(...)) to get // the result JSON for rendering the OG PNG. Direct public hits are rejected. -import { cacheKey, findRelease, type LookupInput, type LookupResult } from '@released/core'; +import { cacheKey, findRelease, type LookupInput } from '@released/core'; import type { Context } from 'hono'; +import type { CacheEntry, WorkerCache } from '../cache.js'; import { makeWorkerCache } from '../cache.js'; import type { Env } from '../env.js'; import { makeProvider } from '../provider.js'; -import { singleFlight } from '../single-flight.js'; +import { type Resolved, resolveLookup } from '../resolve.js'; /** Marker header set by the web-og Service Binding to identify itself. * Cloudflare Service Binding requests can also be checked via the routing @@ -30,46 +31,340 @@ function isServiceBinding(c: Context): boolean { return !!marker && marker === secret; } +/** Origin for the result-cache key URL. + * + * web-og hardcodes `https://web/internal/...` as the Service-Binding target, so + * keying the cache on the incoming request broke OG renders two ways (#143): + * `web` is not a routable hostname, which the Cache API silently declines to + * store (see cache.ts's header note), and it is a different namespace from the + * public permalink routes'. The OG path could therefore neither reuse a warm + * public entry nor persist its own, so every cold unfurl paid a full lookup, + * blew web-og's deadline, and got the placeholder cached by the crawler. + * + * Resolve the origin this deployment's own public routes key on: an explicit + * PUBLIC_BASE_URL, else the committed PROD_HOST var, else the request's origin. + * + * PUBLIC_BASE_URL is what makes that per-environment. PROD_HOST is committed in + * BOTH [vars] and [env.preview.vars] (it gates analytics, which must stay + * prod-only), so without an explicit override the preview Worker — and + * `wrangler dev`, which loads [vars] — would key on the production origin while + * their public routes key on the origin they actually serve. wrangler.toml sets + * PUBLIC_BASE_URL for preview; for `wrangler dev`, put + * `PUBLIC_BASE_URL=http://localhost:8787` in packages/web/.dev.vars (README, + * "Daily flow", says the same where a dev will actually look). + * + * The fallback is NOT passed through `originOf`, because a request origin has no + * configured spelling to normalise — but it is the one input `originOf` exists to + * reject: a real Service Binding arrives as the non-routable `https://web` that + * #143 was about. Today only the unit tests reach it, and only with a routable + * origin; that holds because BOTH deployed envs set a var. If one ever stops — + * PROD_HOST dropped from [vars], blanked in the dashboard, or a new [env.*] + * deployed that the committed wrangler.toml does not describe — every read here + * misses and every write no-ops, and `neverFatal` renders that as "served, just + * not cached". Silence of exactly that kind is why #143 looked green for weeks, + * so say it out loud: the wrangler.toml guard in internal-cache-origin.test.ts + * covers the committed file, and this covers the config it cannot see. */ +function cacheOrigin(env: Env, req: Request): string { + // A var that is SET but rejected by `originOf` is otherwise indistinguishable + // from unset — the `??` chain falls through, `configured` comes back truthy from + // the next arm, and the fallback warning below is never reached. That silence + // matters most on preview: PUBLIC_BASE_URL exists to stop preview keying on + // production, but PROD_HOST is committed in [env.preview.vars] too, so a + // dashboard-set PUBLIC_BASE_URL typo is caught by neither the `??` chain nor the + // wrangler.toml guard, and preview writes onto the PRODUCTION origin silently. + for (const [name, raw] of [ + ['PUBLIC_BASE_URL', env.PUBLIC_BASE_URL], + ['PROD_HOST', env.PROD_HOST], + ] as const) { + if (raw && !originOf(raw)) { + console.warn( + `released: ${name} \`${raw}\` is not a routable cache origin — ignored. ` + + 'Its /internal cache entries key on whatever origin resolves next, which ' + + "may be another environment's (#143).", + ); + } + } + + const configured = originOf(env.PUBLIC_BASE_URL) ?? originOf(env.PROD_HOST); + if (configured) return configured; + + const fallback = new URL(req.url).origin; + if (!isRoutableOrigin(fallback)) { + console.warn( + `released: /internal has no routable cache origin (${fallback}) — ` + + 'the result cache is disabled and every OG unfurl pays a full lookup (#143). ' + + 'Set PUBLIC_BASE_URL or PROD_HOST for this environment.', + ); + } + return fallback; +} + +/** The routability rule `originOf()` applies, exported so the wrangler.toml + * config guard can run the SHIPPED predicate instead of a lookalike. A + * duplicate drifts: an earlier copy of this rule in the test read `URL.host` + * and `startsWith('localhost')`, so it rejected `http://app:8787` — which + * production accepts — and accepted `localhostx`, which production rejects. + * + * Note the port is NOT part of `hostname` (it lives in `URL.port`; `hostname` + * holds a colon only for a bracketed IPv6 literal), so it has to be read + * separately. */ +export function isRoutableOrigin(origin: string): boolean { + const { hostname, port } = new URL(origin); + return hostname.includes('.') || hostname === 'localhost' || port !== ''; +} + +/** Normalise a configured host or base URL to a bare origin, or null if it is + * unset/unparseable. + * + * Both spellings have to be tolerated, because both vars are already written + * both ways: PUBLIC_BASE_URL carries a scheme, PROD_HOST does not, and + * isProdRequest() (analytics.ts) documents PROD_HOST as forgiving of a value + * "copied from PUBLIC_BASE_URL". Concatenating a scheme blindly does NOT throw + * on the mixed case — `new URL('https://https://host')` yields origin + * `https://https` — so it would silently key every entry on a non-routable host + * the Cache API drops, which is #143 exactly, with neverFatal swallowing it. + * The reverse slip is worse: a scheme-less PUBLIC_BASE_URL made `new Request()` + * throw OUTSIDE neverFatal, turning a computed answer into a 503 → placeholder. + * Parsing both through URL and taking .origin also drops any path/trailing slash. */ +export function originOf(value: string | undefined): string | null { + if (!value) return null; + try { + const origin = new URL(value.includes('//') ? value : `https://${value}`).origin; + // `URL.origin` is the literal string "null" for any opaque origin (a + // non-special scheme: file:, foo:). Such a value contains '//', so it skips + // the scheme-prefix branch and parses cleanly — returning a non-null, + // non-URL string that satisfies `??` and then throws out of `new Request()` + // below, OUTSIDE neverFatal. That is the scheme-less slip again: a 500 where + // a computed answer was available, rendered as the neutral placeholder. + if (origin === 'null') return null; + // A single-label host (`web`, `localhost:8787`'s sibling shapes) is not + // routable, and the Cache API silently declines a key URL on one — #143's + // mechanism. `https://web` parses cleanly and yields a plausible-looking + // origin, so a var accidentally set to the Service-Binding target would + // otherwise sail through here looking configured while caching nothing. + // + // Be precise about the reach of this guard, because the ?? chain it falls + // through to ends at the REQUEST origin. It fixes the case where the request + // origin is routable (a `wrangler dev` or public request to /internal): the + // key lands on a host this Worker actually serves and the entry persists. It + // does NOT fix the Service-Binding case, where the request origin is that same + // `https://web` — there is no routable candidate left, and a synthetic constant + // would not help, because cache.ts's rule is that the key URL must be on the + // Worker's OWN hostname (a made-up one no-ops exactly like `cache.invalid`). + // That case is unguardable at runtime and is guarded in config instead, by the + // wrangler.toml suite that requires every env to set PROD_HOST or + // PUBLIC_BASE_URL. + // + // Single-label hosts are the ones to reject, with two real exceptions. + // `localhost` is one — `wrangler dev` serves on it and README tells + // developers to put it in PUBLIC_BASE_URL. An explicit PORT is the other: a + // dev on Codespaces / Docker / WSL reaches the Worker at a service name like + // `http://app:8787`, which is a host this Worker really does serve. Note the + // port is NOT in `hostname` (it lives in `URL.port`; `hostname` holds a colon + // only for a bracketed IPv6 literal), so it has to be read separately. + // + // Rejecting one of those would not be a harmless over-strictness: a REJECTED + // PUBLIC_BASE_URL is indistinguishable from an unset one, so the ?? chain + // falls through to PROD_HOST and every /internal entry keys on the production + // origin while the public routes key on the dev one — the two-namespace split + // of #143, reached silently despite the var being set correctly. + return isRoutableOrigin(origin) ? origin : null; + } catch { + return null; + } +} + +/** Wrap a cache so no Cache API call can be fatal. The key URL is deliberately + * not this request's own origin (see cacheOrigin) and the Cache API is entitled + * to refuse such a read or write; that must degrade to "served, just not + * cached". A 503 here is what web-og renders as the neutral placeholder — the + * #143 symptom — so a cache fault must never throw away a computed answer. */ +function neverFatal(cache: WorkerCache): WorkerCache { + return { + async get(key: string): Promise { + try { + return await cache.get(key); + } catch { + return null; + } + }, + async getEntry(key: string): Promise | null> { + try { + return await cache.getEntry(key); + } catch { + return null; + } + }, + async put(key: string, value: T, ttlSeconds: number): Promise { + try { + await cache.put(key, value, ttlSeconds); + } catch { + // Served, just not cached. + } + }, + }; +} + +/** Message for the 503 web-og reads as "no result" (it renders the neutral card + * for any non-OK response, so only the body text differs by cause). */ +function failureMessage(resolved: Exclude): string { + if (resolved.status === 'not_yet') return resolved.error.message; + if (resolved.status === 'transient') return resolved.kind; + return (resolved.error as Error)?.message ?? 'failed'; +} + /** Resolve the LookupResult JSON for a lookup input. Cache-first, then compute - * via the (relay-aware) provider. Host-aware cache key so OG renders share slots - * with the public routes' `${host}/${projectPath}` prefix; the input kind+id - * distinguishes the slot (`sha:` / `issue:` / `pr:`), mirroring the commit - * endpoint's `sha:${sha}` scheme. */ + * via the (relay-aware) provider. + * + * The cache key MUST match the public permalink routes' exactly (result.tsx, + * issue.tsx, pr.tsx) or the OG card renders into a namespace no public hit can + * ever warm — that was half of #143. That means all five parts, including the + * `cull`/`nopre` suffixes for the default (non-strict, no-prerelease) options an + * OG card always renders, and the public `issue#`/`pr#` id spelling rather than + * the `issue:`/`pr:` this endpoint used to invent. + * + * CAVEAT — this alignment is complete for the issue and PR routes only. On the + * COMMIT route `input.sha` is whatever web-og was handed, and `ui/og-meta.tsx` + * builds the `og:image` URL from `shortSha()` (7 chars) while `result.tsx` keys + * the permalink on the full 40. The two digests differ, so a commit unfurl still + * misses the slot the permalink warmed and still pays a full `findRelease`. + * Closing that needs a change to the PUBLIC routes' key namespace and is tracked + * as #147 — do not read this function as having fixed #143 for commit links. */ async function resolveResult(c: Context, input: LookupInput): Promise { const env = c.env as Env; const req = c.req.raw; const { host, projectPath } = input.repo; - const idPart = input.kind === 'commit' ? `sha:${input.sha}` : `${input.kind}:${input.number}`; - const k = await cacheKey('res', `${host}/${projectPath}`, idPart); - const cache = makeWorkerCache(req); - let result: LookupResult | null = await cache.get(k); - if (result) { - return new Response(JSON.stringify(result), { - headers: { 'content-type': 'application/json' }, - }); - } + const idPart = input.kind === 'commit' ? `sha:${input.sha}` : `${input.kind}#${input.number}`; + const k = await cacheKey('res', `${host}/${projectPath}`, idPart, 'cull', 'nopre'); + const cache = neverFatal(makeWorkerCache(new Request(cacheOrigin(env, req)))); - // Cache miss: compute. The web-og caller chose to wait for this on its side. - // Anubis-protected hosts get a relay-backed fetch (see makeProvider/relay.ts). - try { - const client = makeProvider(env, req, host); - result = await singleFlight(k, async () => { - const re = await cache.get(k); - if (re) return re; - const r = await findRelease(input, { client }); - await cache.put(k, r, 30 * 60); - return r; - }); - return new Response(JSON.stringify(result), { - headers: { 'content-type': 'application/json' }, - }); - } catch (err) { - return new Response(JSON.stringify({ error: (err as Error)?.message ?? 'failed' }), { + // Same resolver the public routes use, on the same slot — sharing a cache slot + // means sharing the policy that governs it: per-state hard TTLs (30 days + // terminal / 24h pending / 60s partial), a 5-minute freshness window, and the + // negative back-off that keeps a down host from being pounded. A flat TTL here + // would downgrade a terminal slot the permalink would have kept for 30 days, + // and a bare read would keep serving a 60-second partial for far longer than + // the public page does. This caller BLOCKS on a refresh when the slot is not + // servable. An earlier round added a stale-while-revalidate opt-out for it, but + // `findRelease` emits only two entry shapes — TERMINAL (fresh forever) and + // PARTIAL (fresh for its 60s, then unpinnable to a pinning consumer) — and + // neither can be both stale and servable, so nothing could ever reach it (round + // 9: dead code, removed). web-og waiting on a cold or unpinnable slot is #152. + // Options are stated explicitly because they are what the + // `cull`/`nopre` key parts promise. Anubis-protected hosts get a relay-backed + // fetch (see makeProvider/relay.ts). The web-og caller chose to wait for this. + const resolved = await resolveLookup({ + cache, + key: k, + load: () => + findRelease(input, { + client: makeProvider(env, req, host), + strict: false, + includePrereleases: false, + }), + // The shared key means a public page view's failed lookup also writes the + // shared `:neg` back-off marker. Honouring that on a COLD slot would 503 here + // without ever calling findRelease, and the crawler caches the resulting + // placeholder for good — #143 all over again, via the alignment that fixes it. + // + // On THIS path the opt-out is unconditional, not cold-only, and the flag name + // reads more conditional than the code is: of the two entry shapes findRelease + // emits, a TERMINAL prior returns at the fresh exit and a PARTIAL one is either + // fresh (same exit) or unpinnable, so no prior ever reaches the marker's + // stale-serve. The cost is real and accepted — during a + // host outage each unfurl runs its own findRelease out to the hard deadline + // (see resolveLookup's fall-through comment) — because the alternative for a + // consumer that asks ONCE is a placeholder pinned long after the host recovers. + // + // The asymmetry is deliberate: this caller opts out of READING the marker on + // a cold slot, but resolveLookup still WRITES it when the slot was cold, so a + // failure discovered here can back off a human permalink for up to 60s. That + // is the point of sharing the slot — the marker describes the HOST being down, + // not who found it out, and the host is equally down for the human. They get + // the "checking…" recovery card (never a wrong "not yet released") and can + // reload; the crawler asks once and keeps what it got. Suppressing the write + // would instead leave the key with no back-off at all whenever the crawler + // touches it first, and every human reload would pound the down host. + // + // What it does NOT do is re-stamp a marker it bypassed. Writing on every + // bypassed failure would keep the shared marker permanently younger than + // NEG_TTL under a once-a-minute unfurl cadence, and the humans on this key — + // who do not bypass — would never see it expire, i.e. never get the retry + // window the back-off exists to give them. resolveLookup writes the marker + // only when it was cold (`!backedOff`), so their clock runs uninterrupted. + bypassBackOffWhenUnservable: true, + // web-og renders whatever we return into a PNG it long-caches for 24h, and + // nothing here can invalidate that PNG afterwards. So no exit may hand this + // caller a CACHED entry that is either older than the stale bound — before + // #143 this route had a flat 30-minute TTL and could not, and it now shares + // the public routes' 24h slot, where a 23h-old answer is representable — or + // a truncated `partial`, whose `firstRelease: null` web-og would render as a + // definite "not yet released" (badge.ts writes those onto this same key with + // an 8s soft deadline). Rather than pin either, we pay a fresh lookup, or + // 503 into a short-cached placeholder. web-og long-caching a partial that + // THIS route's own lookup produced is the remaining half, tracked in #151. + consumerPinsResult: true, + // Share the SLOT with badge.ts and the permalink pages, but not the in-isolate + // FLIGHT. Aligning the cache key also aligned the single-flight key, and + // `singleFlight` runs the first registrant's loader for every joiner: a badge + // request that registers first (badge.ts:110 builds a byte-identical key for + // `issue#N`/`pr#N`) would hand this route its own 8-second-deadline result. On + // a repo that needs longer than that, this caller would inherit a `partial` and + // 503 into a pinned placeholder — #143's symptom, on a link where this route's + // own 24s deadline finds the answer. On main that could not happen, because + // /internal keyed on a three-part key of its own. Concurrent OG unfurls of the + // same key still collapse into one flight; only the cross-caller join is cut. + flightKey: `${k}:og`, + }); + // Refusing to SERVE a cached partial (consumerPinsResult, above) is only half + // the guardrail: on a repo that reliably blows findRelease's soft deadline the + // recompute it forces returns a partial too, and a 200 here pins exactly the + // answer the refusal exists to prevent. + // + // NO partial is servable to this caller, whichever shape it has. With + // `firstRelease: null`, web-og renders `firstRelease?.tag ?? 'not yet released'` + // and long-caches it, so a truncated traversal of a RELEASED commit becomes a + // definite "not yet released" for 24h — the CLAUDE.md guardrail ("Partial state + // != 'not yet released'"). WITH a `firstRelease` it is no safer: that tag is the + // gallop hit, and the bisect that would confirm no EARLIER release contains the + // commit is precisely what the deadline cut short ("the gallop-found tag is + // almost always the right answer; bisect just verifies could there be an earlier + // one", find-release.ts:288-292). "Almost always" is a caveat the result card + // renders and an OG card cannot. Answering "which release FIRST contains this + // commit" with a possibly-later release is the one thing this product must not + // do, so both fall through to the 503: web-og caches the neutral placeholder at + // max-age=60, which claims nothing and self-heals on the next unfurl, and the + // permalink it links to still shows the best-effort answer WITH its caveat. + // + // resolveLookup hands back a partial it computed less than HARD_TTL_PARTIAL ago + // rather than recomputing it (see `shouldRecompute`), so this 503 is throttled to + // one traversal per 60s per key, and ONLY for a partial this route itself computed + // (a badge-truncated one in the shared slot is recomputed, not inherited). Do NOT read that as "the cost is bounded": web-og + // short-caches the neutral placeholder at max-age=60 and HARD_TTL_PARTIAL is + // also 60, so for a URL under active unfurling the two cadences COINCIDE and the + // throttle buys close to nothing. On a repo that reliably blows the soft deadline + // (GNOME/gimp: large tag set, single-instance relay) the card never converges — + // it is the placeholder forever — and upstream load for that key goes from ~0 to + // a full traversal per minute for as long as the link is being unfurled. That is + // the accepted price of not pinning an unconfirmed answer for 24h; making the + // card render the gallop hit while keeping it revalidatable needs web-og to + // short-cache it, which is #156 (adjacent to #151). + if (resolved.status === 'ok') { + if (!resolved.result.partial) { + return new Response(JSON.stringify(resolved.result), { + headers: { 'content-type': 'application/json' }, + }); + } + return new Response(JSON.stringify({ error: 'partial' }), { status: 503, headers: { 'content-type': 'application/json' }, }); } + return new Response(JSON.stringify({ error: failureMessage(resolved) }), { + status: 503, + headers: { 'content-type': 'application/json' }, + }); } /** Parse a permalink :number param into a positive int, or null if invalid. */ diff --git a/packages/web/test/cache.test.ts b/packages/web/test/cache.test.ts index e417aa4..a33b3ee 100644 --- a/packages/web/test/cache.test.ts +++ b/packages/web/test/cache.test.ts @@ -109,6 +109,10 @@ describe('makeWorkerCache', () => { expect(entry?.value).toEqual({ x: 7 }); expect(entry?.ageSeconds).toBeGreaterThanOrEqual(41); expect(entry?.ageSeconds).toBeLessThanOrEqual(44); + // The raw write-time stamp is reported too: `ageSeconds` is floored from a + // read-time `Date.now()`, so an ordering test between two entries read at + // different instants needs the stamp itself (resolve.ts writtenNoEarlierThan). + expect(entry?.stampedAt).toBe(cachedAt); }); it('getEntry returns null when there is no cached entry', async () => { @@ -127,5 +131,10 @@ describe('makeWorkerCache', () => { const cache = makeWorkerCache(new Request('https://released-web.lukaso.workers.dev/')); const entry = await cache.getEntry('k'); expect(entry?.ageSeconds).toBe(0); + // ...and reports the stamp as absent rather than inventing one, so an + // ordering test refuses the pair instead of comparing against a fake 0: + // `writtenNoEarlierThan` (resolve.ts) returns false when either stamp is + // null, so an unstamped marker/entry pair is always recomputed, never served. + expect(entry?.stampedAt).toBeNull(); }); }); diff --git a/packages/web/test/integration.test.ts b/packages/web/test/integration.test.ts index 032bafc..cf5e3ea 100644 --- a/packages/web/test/integration.test.ts +++ b/packages/web/test/integration.test.ts @@ -36,7 +36,7 @@ const INTERNAL_SECRET = 'test-shared-secret'; // typed `unknown`, not LookupResult (a full LookupResult isn't required to exercise // the route, and the seeded fixtures below don't populate one). async function seedFederatedResult(sha: string, result: unknown) { - const key = await cacheKey('res', 'gitlab.gnome.org/GNOME/gimp', `sha:${sha}`); + const key = await cacheKey('res', 'gitlab.gnome.org/GNOME/gimp', `sha:${sha}`, 'cull', 'nopre'); cacheStore.set( `https://released.example/__cache__/${encodeURIComponent(key)}`, new Response(JSON.stringify(result), { headers: { 'content-type': 'application/json' } }), @@ -600,9 +600,10 @@ describe('web Worker — issue/PR internal endpoints (#79)', () => { releaseNotesHtml: null, rateLimit: null, }; - // Seed the slot the route reads: cacheKey('res', `${host}/${projectPath}`, - // `issue:${number}`) — mirroring the commit endpoint's `sha:${sha}` key. - const key = await cacheKey('res', 'github.com/honojs/hono', 'issue:11'); + // Seed the slot the route reads. Since #143 that is the SAME key the public + // /i/ permalink writes: cacheKey('res', `${host}/${projectPath}`, + // `issue#${number}`, 'cull', 'nopre') — see routes/internal.ts. + const key = await cacheKey('res', 'github.com/honojs/hono', 'issue#11', 'cull', 'nopre'); cacheStore.set( `https://released.example/__cache__/${encodeURIComponent(key)}`, new Response(JSON.stringify(seeded), { headers: { 'content-type': 'application/json' } }), @@ -631,7 +632,7 @@ describe('web Worker — issue/PR internal endpoints (#79)', () => { releaseNotesHtml: null, rateLimit: null, }; - const key = await cacheKey('res', 'github.com/honojs/hono', 'pr:17'); + const key = await cacheKey('res', 'github.com/honojs/hono', 'pr#17', 'cull', 'nopre'); cacheStore.set( `https://released.example/__cache__/${encodeURIComponent(key)}`, new Response(JSON.stringify(seeded), { headers: { 'content-type': 'application/json' } }), @@ -664,7 +665,7 @@ describe('web Worker — issue/PR internal endpoints (#79)', () => { rateLimit: null, }; // Proves the route keys the cache by host on the (Hono-decoded) projectPath. - const key = await cacheKey('res', 'gitlab.gnome.org/GNOME/glib', 'issue:1234'); + const key = await cacheKey('res', 'gitlab.gnome.org/GNOME/glib', 'issue#1234', 'cull', 'nopre'); cacheStore.set( `https://released.example/__cache__/${encodeURIComponent(key)}`, new Response(JSON.stringify(seeded), { headers: { 'content-type': 'application/json' } }), @@ -705,7 +706,7 @@ describe('web Worker — issue/PR internal endpoints (#79)', () => { rateLimit: null, }; // Seed the slot the FIXED route keys on (Hono already decoded bad%25 → bad%). - const key = await cacheKey('res', 'gitlab.gnome.org/bad%', 'issue:1'); + const key = await cacheKey('res', 'gitlab.gnome.org/bad%', 'issue#1', 'cull', 'nopre'); cacheStore.set( `https://released.example/__cache__/${encodeURIComponent(key)}`, new Response(JSON.stringify(seeded), { headers: { 'content-type': 'application/json' } }), @@ -721,7 +722,7 @@ describe('web Worker — issue/PR internal endpoints (#79)', () => { it('GET /internal/issue/... fails CLOSED when INTERNAL_SECRET is unset (no web-og fallback)', async () => { cacheStore.clear(); - const key = await cacheKey('res', 'github.com/honojs/hono', 'issue:11'); + const key = await cacheKey('res', 'github.com/honojs/hono', 'issue#11', 'cull', 'nopre'); cacheStore.set( `https://released.example/__cache__/${encodeURIComponent(key)}`, new Response( diff --git a/packages/web/test/internal-cache-origin.test.ts b/packages/web/test/internal-cache-origin.test.ts new file mode 100644 index 0000000..cf3b18f --- /dev/null +++ b/packages/web/test/internal-cache-origin.test.ts @@ -0,0 +1,1299 @@ +// Guard for #143: every cold-cache OG unfurl served the neutral placeholder, +// because the /internal/* result endpoint web-og calls could not see — or write — +// the result cache the public permalink routes use. +// +// Two independent misalignments caused it, and this file pins BOTH: +// +// 1. ORIGIN. web-og calls `env.WEB.fetch('https://web/internal/...')`, so +// makeWorkerCache derived the key URL `https://web/__cache__/...`. That is a +// different namespace from the public routes' `https:///__cache__/...`, +// and `web` is a non-routable hostname, which the Cache API silently declines to +// store — the same class of bug cache.ts's header note records for `cache.invalid`. +// So the OG path neither read nor wrote the cache, and never self-healed. +// 2. KEY PARTS. The public routes key on the 5-part +// ('res', host/path, 'sha:' | 'issue#' | 'pr#', 'cull', 'nopre'); +// /internal keyed on a 3-part ('res', host/path, 'sha:' | 'issue:'). +// Even with the origin fixed, that can never hit a public-route entry. +// +// Every test here calls the route with the REAL production URL shape +// (`https://web/internal/...`). The pre-existing /internal tests in +// integration.test.ts all use a public-looking `https://released.example/...`, which +// is exactly why this regression shipped green. + +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { cacheKey, ProviderServerError } from '@released/core'; +import { parse as parseToml } from 'smol-toml'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// findRelease is the upstream lookup. Mocking it lets a cache HIT be proven by +// "the provider was never consulted", and lets the cold path complete without network. +const findReleaseMock = vi.hoisted(() => vi.fn()); +vi.mock('@released/core', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, findRelease: findReleaseMock }; +}); + +// Polyfill the Workers-only `caches.default` so cache.ts works under Node. +// `cacheFault` lets a test make the Cache API itself fail, which is the failure +// mode the real edge has for a key URL on an origin this Worker doesn't serve. +const cacheStore = new Map(); +let cacheFault: 'none' | 'match' | 'put' = 'none'; +(globalThis as unknown as { caches: { default: Cache } }).caches = { + default: { + async match(req: Request | string) { + if (cacheFault === 'match') throw new Error('cache read refused'); + const url = typeof req === 'string' ? req : req.url; + const stored = cacheStore.get(url); + return stored ? stored.clone() : undefined; + }, + async put(req: Request | string, res: Response) { + if (cacheFault === 'put') throw new Error('cache write refused'); + const url = typeof req === 'string' ? req : req.url; + cacheStore.set(url, res); + }, + } as unknown as Cache, +}; + +const { default: app } = await import('../src/index.js'); +// The real in-isolate flight registry the routes use — a test can register a +// flight the way a concurrent badge request would, and see who runs the loader. +const { singleFlight } = await import('../src/single-flight.js'); +// The SHIPPED routability rule, so the wrangler.toml guard below cannot drift +// from the route it guards. +const { originOf } = await import('../src/routes/internal.js'); + +const INTERNAL_SECRET = 'test-shared-secret'; +const PROD_HOST = 'released.blabberate.com'; +const PUBLIC_ORIGIN = `https://${PROD_HOST}`; +const ENV = { INTERNAL_SECRET, PROD_HOST }; +const SHA = 'a'.repeat(40); +// What web-og ACTUALLY puts in the internal URL for a commit: ogImageUrlForCommit +// -> shortSha(sha) -> 7 characters (ui/og-meta.tsx). The rest of this file uses the +// 40-char form; these two must both work, and #147 tracks the fact that they are +// two different slots. +const SHORT_SHA = SHA.slice(0, 7); + +/** The cache key the PUBLIC permalink routes write (result.tsx / issue.tsx / pr.tsx): + * 5 parts, ending in the default `cull` + `nopre` option suffixes. */ +function publicKey(repo: string, idPart: string): Promise { + return cacheKey('res', repo, idPart, 'cull', 'nopre'); +} + +function keyUrl(origin: string, key: string): string { + return `${origin}/__cache__/${encodeURIComponent(key)}`; +} + +function seed(origin: string, key: string, value: unknown): void { + cacheStore.set( + keyUrl(origin, key), + new Response(JSON.stringify(value), { headers: { 'content-type': 'application/json' } }), + ); +} + +/** Partial LookupResult — only the fields the route serializes back. */ +function fixture(tag: string): unknown { + return { + input: { kind: 'commit', repo: { host: 'github.com', projectPath: 'honojs/hono' }, sha: SHA }, + canonicalSha: SHA, + firstRelease: { tag, sha: 'tagsha', date: '2024-02-01T00:00:00Z', url: '' }, + alsoIn: [], + releaseNotesHtml: null, + rateLimit: null, + }; +} + +/** A request in the shape web-og actually sends over the Service Binding. */ +function svc(url: string): Request { + return new Request(url, { headers: { 'x-released-internal': INTERNAL_SECRET } }); +} + +async function tagOf(res: Response): Promise { + const body = (await res.json()) as { firstRelease?: { tag?: string } }; + return body.firstRelease?.tag; +} + +/** Seed a slot with an explicit age, the way cache.ts stamps `x-cached-at`. */ +function seedAged(origin: string, key: string, value: unknown, ageSeconds: number): void { + cacheStore.set( + keyUrl(origin, key), + new Response(JSON.stringify(value), { + headers: { + 'content-type': 'application/json', + 'x-cached-at': String(Date.now() - ageSeconds * 1000), + }, + }), + ); +} + +/** A non-terminal result — looked up, not in a release yet. The shared policy + * (resolve.ts) revalidates these every 5 minutes; `subject` identifies which + * copy of the answer a response came from. + * + * Honest scope (round 9, #144): `findRelease` does not currently EMIT this shape. + * Its three LookupResult return sites are a terminal answer, a gallop-hit + * `partial`, and a soft-deadline `partial` with `firstRelease: null`; a genuine + * "not yet released" is a thrown NotYetReleasedError, never a cached result. So + * the tests below that seed it exercise resolve.ts's GENERIC pending branch + * (HARD_TTL_PENDING / FRESH_WINDOW_PENDING, both pre-dating this PR) through this + * route — they are not evidence about a state production can reach today. Any + * claim about the OG path's reachable behaviour must be pinned on one of the two + * shapes above instead. */ +function pendingFixture(subject: string): unknown { + return { ...(fixture('unused') as Record), firstRelease: null, subject }; +} + +/** A soft-deadline best-effort answer that ran out of budget before finding a + * containing release. The shared policy trusts these for 60 seconds — and the + * OG path is the deadline-pressured one, so it produces them most. */ +function partialFixture(): unknown { + return { + ...(fixture('unused') as Record), + firstRelease: null, + partial: { reason: 'soft_deadline', candidatesTried: 3 }, + }; +} + +async function subjectOf(res: Response): Promise { + const body = (await res.json()) as { subject?: string }; + return body.subject; +} + +function cacheControlOf(origin: string, key: string): string | null { + return cacheStore.get(keyUrl(origin, key))?.headers.get('cache-control') ?? null; +} + +/** Let a background (waitUntil) refresh run to completion. Must flush MACROtasks, + * not just microtasks: the refresh chain awaits several real promises, and + * singleFlight keeps a module-level in-flight entry per key that only clears when + * the load settles — leaving one pending would make the NEXT test join it. */ +async function settle(): Promise { + for (let i = 0; i < 3; i++) await new Promise((r) => setTimeout(r, 0)); +} + +/** The tag currently stored in a cache slot — how a BACKGROUND refresh is proven + * to have landed, since it by definition isn't in the response body. */ +async function tagOfSlot(origin: string, key: string): Promise { + const stored = cacheStore.get(keyUrl(origin, key)); + if (!stored) return undefined; + const body = (await stored.clone().json()) as { firstRelease?: { tag?: string } }; + return body.firstRelease?.tag; +} + +beforeEach(() => { + cacheStore.clear(); + cacheFault = 'none'; + findReleaseMock.mockReset(); +}); + +describe('/internal/* reads the cache the PUBLIC routes populate (#143)', () => { + it('serves a commit result the public permalink route already cached', async () => { + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + seed(PUBLIC_ORIGIN, k, fixture('v4.8.12')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.8.12'); + // The whole point: a warm public route means the OG render costs no lookup. + expect(findReleaseMock).not.toHaveBeenCalled(); + }); + + it('serves an issue result on the public `issue#` key, not the legacy `issue:`', async () => { + const k = await publicKey('github.com/honojs/hono', 'issue#11'); + seed(PUBLIC_ORIGIN, k, fixture('v0.0.11')); + + const res = await app.fetch(svc('https://web/internal/issue/honojs/hono/11'), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v0.0.11'); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); + + it('serves a PR result on the public `pr#` key, not the legacy `pr:`', async () => { + const k = await publicKey('github.com/honojs/hono', 'pr#4800'); + seed(PUBLIC_ORIGIN, k, fixture('v4.9.0')); + + const res = await app.fetch(svc('https://web/internal/pr/honojs/hono/4800'), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.9.0'); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); + + it('serves a federated (non-GitHub) commit result from the host-keyed public slot', async () => { + const k = await publicKey('gitlab.gnome.org/GNOME/gimp', `sha:${SHA}`); + seed(PUBLIC_ORIGIN, k, fixture('GIMP_2_10_36')); + + const res = await app.fetch( + svc(`https://web/internal/h/gitlab.gnome.org/r/GNOME%2Fgimp/${SHA}`), + ENV, + ); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('GIMP_2_10_36'); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); +}); + +describe('/internal/* WRITES back to the slot the public routes read (#143)', () => { + it('warms the public cache key on a cold lookup, and writes nothing under `https://web`', async () => { + findReleaseMock.mockResolvedValue(fixture('v4.10.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.10.0'); + expect(findReleaseMock).toHaveBeenCalledOnce(); + + // The cold OG render must leave the answer where the public route will find it, + // otherwise every unfurl pays a full lookup forever (the #143 "does not self-heal"). + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + expect([...cacheStore.keys()]).toContain(keyUrl(PUBLIC_ORIGIN, k)); + + // And nothing may land in the non-routable Service-Binding namespace, which the + // real Cache API silently drops. + expect([...cacheStore.keys()].filter((u) => u.startsWith('https://web/'))).toEqual([]); + }); +}); + +// The commit URL web-og really sends is the 7-char one, so the origin fix has to +// hold for that shape too — the rest of this file exercises the 40-char form. +// +// NOTE the half this PR deliberately does NOT fix: the public permalink route keys +// on the sha as it appears in the page URL, and /lookup redirects to the FULL 40 +// chars (index.ts, "short prefixes collide in large repos"). So `sha:<7>` and +// `sha:<40>` are different digests and the first unfurl of a full-sha permalink is +// still cold. That is #147 — its fix lives in ui/og-meta.tsx / result.tsx, files +// this PR does not touch, and changes the PUBLIC routes' key namespace. +describe('/internal/* keys a commit on the sha web-og really sends (7 chars, #147)', () => { + it('serves a cached result on the short-sha public key shape', async () => { + const k = await publicKey('github.com/honojs/hono', `sha:${SHORT_SHA}`); + seed(PUBLIC_ORIGIN, k, fixture('v4.9.9')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHORT_SHA}`), ENV); + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.9.9'); + // A cache HIT is proven by never consulting the provider. + expect(findReleaseMock).not.toHaveBeenCalled(); + }); + + it('writes a cold short-sha lookup back to the public origin, not `https://web`', async () => { + findReleaseMock.mockResolvedValue(fixture('v4.10.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHORT_SHA}`), ENV); + expect(res.status).toBe(200); + expect(findReleaseMock).toHaveBeenCalledOnce(); + + const k = await publicKey('github.com/honojs/hono', `sha:${SHORT_SHA}`); + expect([...cacheStore.keys()]).toContain(keyUrl(PUBLIC_ORIGIN, k)); + expect([...cacheStore.keys()].filter((u) => u.startsWith('https://web/'))).toEqual([]); + }); +}); + +describe('/internal/* cache origin falls back to the request origin', () => { + it('uses the request origin when no PROD_HOST/PUBLIC_BASE_URL is configured', async () => { + // `wrangler dev` and the unit tests have neither var set; the route must still + // key on something routable rather than hardcoding the production hostname. + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + seed('https://released.example', k, fixture('v4.7.0')); + + const res = await app.fetch( + svc(`https://released.example/internal/result/honojs/hono/${SHA}`), + { INTERNAL_SECRET }, + ); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.7.0'); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); + + it('prefers an explicit PUBLIC_BASE_URL over PROD_HOST', async () => { + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + seed('https://staging.example', k, fixture('v4.6.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), { + INTERNAL_SECRET, + PROD_HOST, + PUBLIC_BASE_URL: 'https://staging.example/', + }); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.6.0'); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); +}); + +// The fallback arm of `cacheOrigin` is the ONE input `originOf` exists to reject: +// a real Service Binding arrives as `https://web`, the non-routable origin the +// Cache API silently declines. `originOf` guards the two CONFIGURED arms, but the +// request-origin fallback is not passed through it — so if PROD_HOST is dropped +// from [vars], blanked in the dashboard, or a new [env.*] ships without one, every +// /internal read misses and every write no-ops, `neverFatal` reports "served, just +// not cached", and #143 is back with no error, no log and no metric. That silence +// is what made #143 look green for weeks. The wrangler.toml guard further down +// cannot see a dashboard-set var or an env added outside the committed file, so +// the relapse has to be observable at RUNTIME too. +describe('/internal/* makes a non-routable cache origin observable (#143 relapse)', () => { + it('warns when it falls back to a request origin the Cache API will decline', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + findReleaseMock.mockResolvedValue(fixture('v4.9.0')); + + // Production's exact shape with PROD_HOST lost: the Service Binding's own + // non-routable `https://web` origin is all that is left to key on. + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), { + INTERNAL_SECRET, + }); + + // The warning is observability, never a behaviour change: the answer still serves. + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.9.0'); + + const said = warn.mock.calls.map((c) => String(c[0])).join('\n'); + warn.mockRestore(); + expect(said).toContain('https://web'); + expect(said).toMatch(/cache/i); + }); + + it('stays silent when a routable origin IS configured', async () => { + // Fails if the warning is unconditional rather than gated on routability — + // production would then log this on every single OG unfurl. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + findReleaseMock.mockResolvedValue(fixture('v4.9.1')); + + await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + + const calls = warn.mock.calls.length; + warn.mockRestore(); + expect(calls).toBe(0); + }); + + it('stays silent when the request-origin fallback is itself routable', async () => { + // `wrangler dev` and the unit tests: neither var set, but the request origin is + // a real hostname, so the cache works and there is nothing to report. Fails if + // the warning keys on "took the fallback arm" instead of "origin is unusable". + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + findReleaseMock.mockResolvedValue(fixture('v4.9.2')); + + await app.fetch(svc(`https://released.example/internal/result/honojs/hono/${SHA}`), { + INTERNAL_SECRET, + }); + + const calls = warn.mock.calls.length; + warn.mockRestore(); + expect(calls).toBe(0); + }); + + // The fallback warning above only fires when NEITHER var is set. A var that IS + // set but rejected by `originOf` is indistinguishable from unset: the `??` chain + // falls straight through to the next arm, `configured` is truthy, and the operator + // never learns their override was discarded. + // + // That is not cosmetic on THIS app, because PUBLIC_BASE_URL exists precisely to + // stop preview keying on production: wrangler.toml sets it for [env.preview], and + // PROD_HOST is committed in [env.preview.vars] too (it gates analytics, which must + // stay prod-only). Mistype PUBLIC_BASE_URL in the dashboard — where the + // wrangler.toml guard below cannot see it — and preview writes every /internal + // entry onto the PRODUCTION origin, silently, with a perfectly routable origin + // hiding the fault. + it('warns when PUBLIC_BASE_URL is set but rejected, even though PROD_HOST saves it', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + findReleaseMock.mockResolvedValue(fixture('v4.9.2')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), { + INTERNAL_SECRET, + PROD_HOST, + // Single-label host: `originOf` rejects it for the same reason it rejects + // `web` — the Cache API declines a key URL on a non-routable hostname. + PUBLIC_BASE_URL: 'web-preview', + }); + + // Observability only: the answer still serves off the PROD_HOST origin. + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.9.2'); + + const said = warn.mock.calls.map((c) => String(c[0])).join('\n'); + warn.mockRestore(); + expect(said).toContain('PUBLIC_BASE_URL'); + expect(said).toContain('web-preview'); + }); + + // The mirror case, and NOT a duplicate: it fails if the check only ever looks at + // PUBLIC_BASE_URL. Here the override is valid and the committed var is the broken + // one, so nothing about the served origin is wrong — only the operator's belief + // about which var is doing the work. + it('warns when PROD_HOST is set but rejected, even though PUBLIC_BASE_URL saves it', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + findReleaseMock.mockResolvedValue(fixture('v4.9.3')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), { + INTERNAL_SECRET, + PROD_HOST: 'https:released.example.com', + PUBLIC_BASE_URL: PUBLIC_ORIGIN, + }); + + expect(res.status).toBe(200); + const said = warn.mock.calls.map((c) => String(c[0])).join('\n'); + warn.mockRestore(); + expect(said).toContain('PROD_HOST'); + }); +}); + +// Sharing the slot with the public routes means sharing the POLICY that governs +// it (resolve.ts): 30-day terminal / 24h pending / 60s partial hard TTLs, a +// 5-minute freshness window, and a negative back-off. /internal used to invent a +// flat 30-minute TTL and a bare read, so a cold OG render could downgrade a +// terminal slot to 30 minutes and keep serving a 60-second partial for half an hour. +describe('/internal/* follows the cache policy that governs the shared slot', () => { + it('writes a terminal answer with the public routes 30-day hard TTL', async () => { + findReleaseMock.mockResolvedValue(fixture('v4.10.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + expect(res.status).toBe(200); + + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + expect(cacheControlOf(PUBLIC_ORIGIN, k)).toBe(`public, max-age=${30 * 24 * 60 * 60}`); + }); + + it('writes a soft-deadline partial with the 60-second TTL, not a flat 30 minutes', async () => { + findReleaseMock.mockResolvedValue(partialFixture()); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + // The RESPONSE is a 503 (a computed partial is never pinned — see the + // guardrail suite below); the point here is that the write-back still + // happens, and lands on the shared 60-second partial TTL rather than + // /internal's old flat 30 minutes, so a public page view can reuse it. + expect(res.status).toBe(503); + + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + expect(cacheControlOf(PUBLIC_ORIGIN, k)).toBe('public, max-age=60'); + }); + + // Rounds 9 and 15 (#144). The partial above has `firstRelease: null`, which + // `hardTtlFor()` already routes to its 60-second branch. The OTHER partial shape + // does not: find-release.ts (~295) returns the gallop hit WITH `partial`, and + // `hardTtlFor()` tests `firstRelease` BEFORE `partial`, so it takes the terminal + // 30-day branch — and `isFresh()` reports it fresh forever for the same reason. + // That IS a real defect, and #155 tracks it. + // + // Round 9 tried to contain it by writing 60s from THIS caller. Round 15 showed + // why that is the wrong lever: the whole point of this PR is that the slot is + // SHARED, so a TTL chosen here is imposed on badge.ts and the permalink pages + // too. A public page view writes the gallop answer at 30 days; one OG unfurl a + // moment later replaced it with a 60-second entry, and 61 seconds on every later + // human page view paid a fresh traversal while issue.tsx/pr.tsx's bot branch + // rendered the deferred card off a slot that had been warm. Narrowing the public + // routes' TTL is #155's call to make, on the public routes' own writes — not a + // side effect of an unfurl. + // + // So the ENTRY carries the caller-independent TTL, and this caller's refusal to + // trust a partial for more than 60 seconds rides on its own `:pinpartial` marker. + // The route still 503s the partial rather than pinning it (round 8), and + // `shouldRecompute` still recomputes it after 60s (resolve.test.ts) — what + // changed is that nobody else's entry is shortened to buy that. + it('writes a gallop partial on the caller-independent TTL, throttling via its own marker', async () => { + const sha = '9'.repeat(40); + findReleaseMock.mockResolvedValue({ + ...(fixture('v4.19.0') as Record), + partial: { reason: 'soft_deadline', candidatesTried: 3 }, + }); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + expect(res.status).toBe(503); // never pinned to the crawler — the round-8 guard + + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + // Same value a public caller writes for this shape: the unfurl did not + // shorten anyone's entry. Restoring the round-9 `HARD_TTL_PARTIAL` write + // reddens this with `max-age=60`. + expect(cacheControlOf(PUBLIC_ORIGIN, k)).toBe(`public, max-age=${30 * 24 * 60 * 60}`); + // ...and the 60-second distrust lives on the caller-private marker instead. + expect(cacheControlOf(PUBLIC_ORIGIN, `${k}:pinpartial`)).toBe('public, max-age=60'); + }); + + // The complement: narrowing the TTL for a pinning consumer must not touch the + // answer the route actually exists to reuse. A terminal result still gets 30 days. + it('a terminal answer keeps the 30-day TTL when the SAME caller writes it', async () => { + const sha = 'a'.repeat(40); + findReleaseMock.mockResolvedValue(fixture('v4.18.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + expect(res.status).toBe(200); + + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + expect(cacheControlOf(PUBLIC_ORIGIN, k)).toBe(`public, max-age=${30 * 24 * 60 * 60}`); + }); + + it('revalidates a pending answer past its 5-minute freshness window', async () => { + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + seedAged(PUBLIC_ORIGIN, k, pendingFixture('stale pending'), 10 * 60); + findReleaseMock.mockResolvedValue(fixture('v4.12.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + await settle(); + + expect(res.status).toBe(200); + // The revalidation happens on the render path (this route blocks; #152) and + // the slot ends up refreshed, so the OG card can't keep rendering the stale + // answer for another 30 minutes. + expect(findReleaseMock).toHaveBeenCalledOnce(); + expect(await tagOfSlot(PUBLIC_ORIGIN, k)).toBe('v4.12.0'); + }); + + it('serves the last-known-good answer when the upstream blips, and backs off', async () => { + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + seedAged(PUBLIC_ORIGIN, k, pendingFixture('last known good'), 10 * 60); + findReleaseMock.mockRejectedValue( + new ProviderServerError('github.com', 503, 'Service Unavailable'), + ); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + await settle(); + + expect(res.status).toBe(200); + expect(await subjectOf(res)).toBe('last known good'); + // And the next unfurl inside the back-off window must not re-hit the down host. + expect([...cacheStore.keys()]).toContain(keyUrl(PUBLIC_ORIGIN, `${k}:neg`)); + }); +}); + +// The key URL is deliberately NOT this request's own origin (the Service Binding +// arrives on `https://web`), and the Cache API is entitled to refuse such a +// write. A refusal must degrade to "served, just not cached" — a 503 here is +// what web-og turns into the neutral placeholder, which IS the #143 symptom. +describe('/internal/* never turns a Cache API failure into a placeholder', () => { + it('serves the computed answer when the cache WRITE is refused', async () => { + findReleaseMock.mockResolvedValue(fixture('v4.13.0')); + cacheFault = 'put'; + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.13.0'); + }); + + it('serves the computed answer when the cache READ throws', async () => { + findReleaseMock.mockResolvedValue(fixture('v4.14.0')); + cacheFault = 'match'; + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.14.0'); + expect(findReleaseMock).toHaveBeenCalledOnce(); + }); +}); + +// PROD_HOST is committed in BOTH [vars] and [env.preview.vars], so without an +// explicit override the preview Worker would key /internal on the production +// origin — a host it does not serve — while its own public routes key on the +// preview origin. That is the #143 misalignment, still live in the one +// environment OG changes get reviewed in. +describe('preview keys the result cache on an origin it actually serves', () => { + const cfg = parseToml( + readFileSync(fileURLToPath(new URL('../wrangler.toml', import.meta.url)), 'utf8'), + ) as { + vars: { PROD_HOST: string }; + env: { preview: { name: string; vars: { PUBLIC_BASE_URL?: string } } }; + }; + + it('sets PUBLIC_BASE_URL in [env.preview.vars] to the preview Worker own origin', () => { + const url = cfg.env.preview.vars.PUBLIC_BASE_URL; + expect(url, 'preview must override the inherited prod cache origin').toBeTypeOf('string'); + const host = new URL(url as string).host; + expect(host).not.toBe(cfg.vars.PROD_HOST); + expect(host).toContain(cfg.env.preview.name); + }); +}); + +// The other half of the same failure, and the one that has no fix at runtime: +// `cacheOrigin` falls back to the REQUEST origin, which for a real Service +// Binding is web-og's hardcoded `https://web` — a non-routable single-label host +// the Cache API declines, i.e. #143 exactly, swallowed by neverFatal. The config +// is the only place that can be guarded, so guard it there. +// +// Guard the invariant that actually broke, not a weaker one that survives it. +// `[env.preview]` DID declare a var — it copied the production `PROD_HOST`, which +// every env pins because it gates analytics — and keyed /internal on the prod +// origin while serving `released-web-preview.*.workers.dev`. So "PROD_HOST or +// PUBLIC_BASE_URL is set and routable" goes GREEN on the exact config this PR is +// fixing. What a named env must have is a cache origin OF ITS OWN. +type WranglerCfg = { + vars?: Record; + env?: Record }>; +}; + +/** The invariant as a pure function, so it can be run against configs that are + * KNOWN BAD as well as the committed one. A checker that has only ever seen the + * good config demonstrates nothing about what it rejects. Returns one string per + * violation; `[]` means the config cannot reintroduce #143's namespace split. */ +function cacheOriginProblems(cfg: WranglerCfg): string[] { + const problems: string[] = []; + // Run the SHIPPED normaliser (`originOf`, exported from the route), not a + // second copy of its rule. The copy this replaced tested `URL.host` with + // `host.includes('.') || host.startsWith('localhost')`, which disagreed with + // production on two inputs: it rejected `http://app:8787` — the Docker / + // Codespaces shape the route deliberately accepts, so a legitimate config + // would have reddened the build — and accepted `localhostx`, which the route + // rejects. A guard that can pass or fail differently from the code it guards + // is not a guard. `originOf` also returns null for an unparseable value, so + // the two cases collapse into one branch here. + const prodOrigin = cfg.vars?.PUBLIC_BASE_URL ?? cfg.vars?.PROD_HOST; + const prodCacheOrigin = originOf(prodOrigin); + if (!prodOrigin) { + problems.push('[vars] sets neither PROD_HOST nor PUBLIC_BASE_URL'); + } else if (!prodCacheOrigin) { + problems.push(`[vars] cache origin \`${prodOrigin}\` is not routable`); + } + + for (const [name, e] of Object.entries(cfg.env ?? {})) { + const url = e.vars?.PUBLIC_BASE_URL; + if (!url) { + problems.push( + `[env.${name}.vars] does not set its own PUBLIC_BASE_URL — /internal falls back to PROD_HOST and keys the result cache on an origin this Worker does not serve`, + ); + continue; + } + const origin = originOf(url); + if (!origin) { + problems.push(`[env.${name}.vars] PUBLIC_BASE_URL \`${url}\` is not routable`); + continue; + } + if (origin === prodCacheOrigin) { + problems.push( + `[env.${name}.vars] PUBLIC_BASE_URL keys on the PRODUCTION origin \`${origin}\``, + ); + continue; + } + // ...and on a `*.workers.dev` host the origin is DERIVED from the Worker name: + // the first label IS `[env.] name`. So the literal can be checked against + // the deployment it claims to describe, and a rename that leaves the URL behind + // fails the build instead of shipping an env whose canonical links, `og:url` and + // sitemap point at a host that no longer exists (`publicBaseUrl()` prefers this + // var over the request origin, so those are no longer correct by construction). + // Only for workers.dev: an env on a custom domain has no such relationship, and + // asserting one there would reject a legitimate config. The account subdomain + // (`..workers.dev`) is NOT in this file at all, so no config guard can + // check it — a deploy under a different account is caught by the preview + // liveness check, not here. + const host = new URL(origin).host; + const workerName = e.name; + if (workerName && host.endsWith('.workers.dev') && host.split('.')[0] !== workerName) { + problems.push( + `[env.${name}.vars] PUBLIC_BASE_URL \`${origin}\` does not match [env.${name}] name \`${workerName}\``, + ); + } + } + return problems; +} + +describe('every deployed environment configures a routable cache origin of its OWN', () => { + const cfg = parseToml( + readFileSync(fileURLToPath(new URL('../wrangler.toml', import.meta.url)), 'utf8'), + ) as WranglerCfg; + + it('holds for the committed wrangler.toml', () => { + expect(cacheOriginProblems(cfg)).toEqual([]); + }); + + it('runs against the real named environments, so it cannot pass vacuously', () => { + expect(Object.keys(cfg.env ?? {})).toContain('preview'); + }); + + it('rejects the pre-fix [env.preview] — the config that actually shipped #143', () => { + const preFix: WranglerCfg = { + vars: { PROD_HOST }, + // Exactly what was committed before this PR: the prod PROD_HOST copied in + // (it gates analytics, so every env pins it) and no origin of its own. + env: { preview: { name: 'released-web-preview', vars: { ANUBIS_HOSTS: '', PROD_HOST } } }, + }; + expect(cacheOriginProblems(preFix)).toEqual([ + expect.stringContaining('[env.preview.vars] does not set its own PUBLIC_BASE_URL'), + ]); + }); + + it('rejects a NEW env that copies PROD_HOST the way [env.preview] once did', () => { + const withStaging: WranglerCfg = { + ...cfg, + env: { + ...cfg.env, + staging: { name: 'released-web-staging', vars: { ANUBIS_HOSTS: '', PROD_HOST } }, + }, + }; + expect(cacheOriginProblems(withStaging)).toEqual([ + expect.stringContaining('[env.staging.vars] does not set its own PUBLIC_BASE_URL'), + ]); + }); + + it('rejects an env whose PUBLIC_BASE_URL IS the production origin', () => { + const aliased: WranglerCfg = { + ...cfg, + env: { + ...cfg.env, + staging: { + name: 'released-web-staging', + vars: { PUBLIC_BASE_URL: PUBLIC_ORIGIN }, + }, + }, + }; + expect(cacheOriginProblems(aliased)).toEqual([ + expect.stringContaining('keys on the PRODUCTION origin'), + ]); + }); + + // The rename this guard exists for. `PUBLIC_BASE_URL` is a literal, but on + // workers.dev the host it must equal is generated from `name` — so the two can + // drift with a one-line edit and nothing at runtime would notice: the origin + // stays set, routable and non-prod, and `publicBaseUrl()` goes on serving + // canonical/`og:url`/sitemap URLs for a Worker that no longer answers. + it('rejects an env whose PUBLIC_BASE_URL was left behind by a `name` rename', () => { + const renamed: WranglerCfg = { + ...cfg, + env: { + ...cfg.env, + preview: { + name: 'released-web-pr-preview', + vars: { PUBLIC_BASE_URL: 'https://released-web-preview.lukaso.workers.dev' }, + }, + }, + }; + expect(cacheOriginProblems(renamed)).toEqual([ + expect.stringContaining('does not match [env.preview] name'), + ]); + }); + + // ...and the same check must not fire on an env served from a custom domain, + // where the host bears no relation to the Worker name. Without the + // `.workers.dev` condition this config would be rejected outright. + it('accepts an env on a custom domain, whose host cannot match the Worker name', () => { + const custom: WranglerCfg = { + ...cfg, + env: { + ...cfg.env, + staging: { + name: 'released-web-staging', + vars: { PUBLIC_BASE_URL: 'https://staging.blabberate.com' }, + }, + }, + }; + expect(cacheOriginProblems(custom)).toEqual([]); + }); + + // The reason this suite calls `originOf` instead of restating its rule. A + // single-label host WITH a port is what a dev on Docker / Codespaces / WSL + // reaches the Worker at, and the route accepts it (internal.ts's + // `isRoutableOrigin`). The duplicated predicate this replaced tested + // `host.includes('.') || host.startsWith('localhost')` on `URL.host`, so + // `app:8787` matched neither arm: production accepted the config and the + // build failed on it. Proven by restoring that predicate — this test and the + // `localhostx` one below are the two that redden (`expected [ Array(1) ] to + // deeply equal []`); the four pre-existing cases stay green either way, which + // is why they could not catch the drift. + it('accepts an env on a single-label host WITH a port, exactly as the route does', () => { + const docker: WranglerCfg = { + ...cfg, + env: { + ...cfg.env, + dev: { name: 'released-web-dev', vars: { PUBLIC_BASE_URL: 'http://app:8787' } }, + }, + }; + expect(cacheOriginProblems(docker)).toEqual([]); + }); + + // The other half of the divergence: the old predicate's `startsWith` accepted + // any host merely PREFIXED with localhost. `originOf` requires the whole + // hostname (or a port, which this has neither of). + it('rejects `localhostx`, which only a prefix match would accept', () => { + const typo: WranglerCfg = { + ...cfg, + env: { + ...cfg.env, + dev: { name: 'released-web-dev', vars: { PUBLIC_BASE_URL: 'http://localhostx' } }, + }, + }; + expect(cacheOriginProblems(typo)).toEqual([expect.stringContaining('is not routable')]); + }); + + it('rejects an env whose PUBLIC_BASE_URL is a non-routable single-label host', () => { + const bound: WranglerCfg = { + ...cfg, + env: { + ...cfg.env, + staging: { name: 'released-web-staging', vars: { PUBLIC_BASE_URL: 'https://web' } }, + }, + }; + expect(cacheOriginProblems(bound)).toEqual([expect.stringContaining('is not routable')]); + }); +}); + +// Belt to that suspenders: a value that IS set but is not routable must not be +// used either. `https://web` parses fine and yields a plausible-looking origin, +// so without this it would sail through and reinstate #143 while looking configured. +describe('/internal/* refuses a configured cache origin that is not routable', () => { + it('ignores a single-label PROD_HOST rather than keying on a host it cannot cache', async () => { + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + seed('https://released.example', k, fixture('v4.5.0')); + + const res = await app.fetch( + svc(`https://released.example/internal/result/honojs/hono/${SHA}`), + { INTERNAL_SECRET, PROD_HOST: 'web' }, + ); + + // Fell through to the request origin, where the warm entry actually is. + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.5.0'); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); + + it('still accepts localhost, which `wrangler dev` really serves on', async () => { + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + seed('http://localhost:8787', k, fixture('v4.4.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), { + INTERNAL_SECRET, + PUBLIC_BASE_URL: 'http://localhost:8787', + }); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.4.0'); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); + + it('accepts a single-label host WITH a port, which Docker/Codespaces dev serves on', async () => { + // `URL.hostname` never carries the port (that is `URL.port`), so a check for a + // colon in the hostname matches only a bracketed IPv6 literal — it never sees + // `app:8787`. Rejecting this origin is not harmless over-strictness: a rejected + // PUBLIC_BASE_URL is indistinguishable from an unset one, so the ?? chain lands + // on PROD_HOST and /internal keys on the production origin while the public + // routes key on the dev one. That is #143's split, reached with the var set. + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + seed('http://app:8787', k, fixture('v4.2.0')); + // Only reachable if the origin was rejected and the chain fell to PROD_HOST. + findReleaseMock.mockResolvedValue(fixture('v9.9.9')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), { + INTERNAL_SECRET, + PROD_HOST, + PUBLIC_BASE_URL: 'http://app:8787', + }); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.2.0'); + expect(findReleaseMock).not.toHaveBeenCalled(); + expect(cacheStore.has(keyUrl(PUBLIC_ORIGIN, k))).toBe(false); + }); + + it('documents the unguardable case: unset vars key on the Service Binding origin', async () => { + // Nothing at runtime can recover the public origin here, so this pins what + // actually happens rather than implying it is safe: the entry is written under + // `https://web`, which the real Cache API drops. The wrangler.toml suite above + // is what keeps this shape from ever shipping. + const sha = '7'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + findReleaseMock.mockResolvedValue(fixture('v4.3.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), { + INTERNAL_SECRET, + }); + + expect(res.status).toBe(200); + expect(cacheStore.has(keyUrl(PUBLIC_ORIGIN, k))).toBe(false); + expect(cacheStore.has(keyUrl('https://web', k))).toBe(true); + }); +}); + +// web-og renders `firstRelease?.tag ?? 'not yet released'` and long-caches any +// non-null result for 24h, and nothing here can invalidate a PNG already made. A +// `partial` is a truncated traversal, so neither of its shapes may be pinned: with +// `firstRelease: null` a RELEASED commit becomes a definite "not yet released", and +// WITH one the tag is the gallop hit the bisect never confirmed is the earliest. +// Both fall through to a 503, which web-og renders as the neutral placeholder at +// max-age=60 — it claims nothing and self-heals. The refusal is then throttled by +// the partial the lookup just recorded, so a repo that reliably blows the soft +// deadline costs one traversal per 60s per key, not one per unfurl. +describe('/internal/* refuses to pin a partial, and throttles the refusal', () => { + // Round 7 (#144). This used to assert the opposite — that a cached `partial` + // was stale-served to web-og while a refresh ran behind it. That is the bug: + // web-og renders `firstRelease?.tag ?? 'not yet released'` and long-caches any + // non-null result, so a truncated traversal of a RELEASED commit got pinned as + // "not yet released" for 24h. `badge.ts` writes those onto this very key (8s + // soft deadline), which only this PR's key alignment made visible here. + it('does NOT hand back a cached PARTIAL — it blocks and recomputes', async () => { + const sha = 'c'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + seedAged(PUBLIC_ORIGIN, k, partialFixture(), 5 * 60); + findReleaseMock.mockResolvedValue(fixture('v4.12.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.12.0'); // the real answer, not the partial + expect(findReleaseMock).toHaveBeenCalledTimes(1); + expect(await tagOfSlot(PUBLIC_ORIGIN, k)).toBe('v4.12.0'); // and the slot is corrected + }); + + // Round 12 (#144). Round 11 stopped badge's 8-second truncation reaching this + // route through the in-isolate FLIGHT (`flightKey: `${k}:og``). The same + // truncation still reached it one hop later, through the CACHE: the throttle + // that hands a <60s partial back rather than recomputing it did not ask WHOSE + // deadline produced it. So the README-badge case — camo fetches `badge.svg`, + // badge.ts writes a partial to the byte-identical key, Slack unfurls the + // permalink ten seconds later — read that partial as "recent", 503'd, and + // pinned the neutral placeholder, on a link this route's own 24s deadline + // answers. `main` could not do this: /internal keyed on a key of its own. + // + // The throttle now keys on a companion marker resolveLookup writes ONLY for a + // partial IT produced under this caller's deadline, so a foreign one is + // recomputed. (Proven: restoring the age-only test — `return + // entry.ageSeconds >= HARD_TTL_PARTIAL` in `shouldRecompute` — reddens this + // test alone, with `expected 503 to be 200`.) + it('recomputes a 10s-old partial ANOTHER caller wrote, rather than 503 into a pinned placeholder', async () => { + const sha = 'b'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + // badge.ts's 8s deadline truncated and wrote this; no `:pinpartial` marker, + // because badge does not set consumerPinsResult. + seedAged(PUBLIC_ORIGIN, k, partialFixture(), 10); + findReleaseMock.mockResolvedValue(fixture('v4.13.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.13.0'); + expect(findReleaseMock).toHaveBeenCalledTimes(1); + }); + + // Round 13 (#144). The marker above says "a partial THIS caller wrote is in the + // slot", but the previous round trusted it on its own age alone — it never + // checked that the marker still describes the entry actually sitting there. The + // slot is shared, so it can be overwritten between our marker write and the next + // read, and the marker is then inherited by a stranger's partial: + // + // T=0 an unfurl blows the 24s soft deadline → writes its partial + marker + // T=20 camo fetches badge.svg on the same key; badge's 8s deadline truncates + // harder and OVERWRITES the slot (it writes no marker of its own) + // T=30 the next unfurl reads badge's partial (age 10s) under OUR marker + // (age 30s), calls it ours, and 503s into a pinned placeholder — on a + // link this route's own 24s deadline answers. + // + // `run()` puts the slot before the marker, so our marker can never be OLDER than + // our entry; when it is, the slot moved under us. (Proven: dropping the + // `mark.ageSeconds <= prior.ageSeconds` clause reddens this test alone, with + // `expected 503 to be 200`.) + it('recomputes when the slot was overwritten AFTER our marker, rather than inheriting the marker', async () => { + const sha = '8'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + // Ours, written at T=0 and still inside its 60s TTL... + seedAged(PUBLIC_ORIGIN, `${k}:pinpartial`, { pinnedPartial: true }, 30); + // ...but the partial in the slot is badge.ts's, written at T=20 — NEWER than + // the marker, so the marker cannot be describing it. + seedAged(PUBLIC_ORIGIN, k, partialFixture(), 10); + findReleaseMock.mockResolvedValue(fixture('v4.14.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.14.0'); + expect(findReleaseMock).toHaveBeenCalledTimes(1); + }); + + // The complement, and what keeps the clause above from silently disabling the + // throttle altogether: when the marker IS younger than the entry — the ordinary + // own-partial pair `run()` writes — the throttle still engages and the route + // still 503s without a second traversal. + it('still honours the marker for a pair this caller wrote (marker younger than entry)', async () => { + const sha = '9'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + seedAged(PUBLIC_ORIGIN, k, partialFixture(), 30); + seedAged(PUBLIC_ORIGIN, `${k}:pinpartial`, { pinnedPartial: true }, 29); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(503); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); + + it('still blocks (and write-backs) when the slot is genuinely cold', async () => { + const sha = 'd'.repeat(40); + findReleaseMock.mockResolvedValue(fixture('v4.15.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.15.0'); + }); + + // Round 7 (#144), the other half of the same guardrail. Refusing to SERVE a + // cached partial is only half the job: on a repo that reliably blows the soft + // deadline the forced recompute returns a partial too, and returning that as a + // 200 pins the same wrong "not yet released" card for 24h that the refusal + // exists to prevent (web-og: `firstRelease?.tag ?? 'not yet released'`, + // long-cached for any non-null result). A 503 instead renders the neutral + // placeholder at max-age=60, which self-heals on the next unfurl. + it('503s rather than pin a COMPUTED partial as "not yet released"', async () => { + const sha = '1'.repeat(40); + findReleaseMock.mockResolvedValue(partialFixture()); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(503); + expect(findReleaseMock).toHaveBeenCalledTimes(1); + }); + + // Round 8 (#144). Earlier rounds bounded this on `partial && !firstRelease`, + // reasoning that a truncated traversal which DID find a containing release + // carries a tag web-og renders correctly. find-release.ts says otherwise: the + // tag in that shape is the GALLOP hit, and the bisect that would confirm no + // EARLIER release contains the commit is exactly what the deadline cut short + // ("the gallop-found tag is almost always the right answer; bisect just + // verifies could there be an earlier one", find-release.ts:288-292). "Almost + // always" is a caveat the result card renders and an OG card cannot: web-og + // pins the bare tag for 24h. Answering "which release FIRST contains this + // commit" with a possibly-later release is the failure this product exists to + // avoid, so for a consumer that pins, no partial is servable — the neutral + // placeholder claims nothing, and the permalink it links to shows the + // best-effort answer WITH its caveat. + it('503s rather than pin a partial whose tag the bisect never confirmed', async () => { + const sha = '2'.repeat(40); + findReleaseMock.mockResolvedValue({ + ...(fixture('v4.19.0') as Record), + partial: { reason: 'soft_deadline', candidatesTried: 3 }, + }); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(503); + }); + + // ...and the same shape read back from the SHARED slot. `hardTtlFor()` and + // `isFresh()` both test `firstRelease` before `partial`, so a gallop-only + // answer is stored for 30 days and reported fresh forever. Joining the public + // key is what first exposed the OG path to an entry that old (its own cache was + // a flat 30 minutes), so the pin bound has to reject it on the way OUT. + // The underlying terminal-classification bug is on `main` and affects the + // public routes too — tracked separately, not widened into this PR. + it('does not serve a 20-day-old partial the cache classified as terminal', async () => { + const sha = '5'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + seedAged( + PUBLIC_ORIGIN, + k, + { ...(fixture('v4.9.0') as Record), partial: { reason: 'soft_deadline' } }, + 20 * 24 * 60 * 60, + ); + findReleaseMock.mockResolvedValue(fixture('v4.8.0')); // the real earliest + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.8.0'); + expect(findReleaseMock).toHaveBeenCalledTimes(1); + }); + + // Refusing to serve a partial must not turn into refusing to CACHE the refusal. + // resolveLookup writes the computed partial to the slot at HARD_TTL_PARTIAL, but + // every read path rejects it — the fresh exit and the back-off exit alike — so `run()` + // falls through to `load()` again. On a repo that reliably blows the 24s soft + // deadline that is a full traversal per unfurl on the shared token, forever, + // where the flat 30-minute TTL this route replaced made zero upstream calls. + it('does not re-run the lookup for every unfurl of a deadline-blowing repo', async () => { + const sha = '3'.repeat(40); + findReleaseMock.mockResolvedValue(partialFixture()); + + const first = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + const second = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(first.status).toBe(503); + expect(second.status).toBe(503); + expect(findReleaseMock).toHaveBeenCalledTimes(1); + }); + + // The complement, and what stops the throttle above from becoming a permanent + // placeholder: the recorded partial is only honoured inside its own 60-second + // TTL, the same window the shared policy already trusts a partial for. + it('recomputes once the recorded partial ages out of its 60-second window', async () => { + const sha = '4'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + seedAged(PUBLIC_ORIGIN, k, partialFixture(), 61); + findReleaseMock.mockResolvedValue(fixture('v4.20.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.20.0'); + expect(findReleaseMock).toHaveBeenCalledTimes(1); + }); + + // ...and the throttle must never outrank a REAL answer. A public page view has + // no soft-deadline pressure from web-og and can land a terminal result in the + // shared slot inside that 60-second window; the next unfurl must serve it. + it('serves a real answer that landed in the slot inside the partial window', async () => { + const sha = '6'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + seedAged(PUBLIC_ORIGIN, k, fixture('v4.21.0'), 30); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.21.0'); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); +}); + +// PROD_HOST is shared with isProdRequest() (analytics.ts), which documents itself +// as tolerant of a value written WITH a scheme ("copied from PUBLIC_BASE_URL"). +// `https://` + that value parses without throwing — to origin `https://https` — +// so an un-normalised read here would key every /internal entry on a non-routable +// host the Cache API drops: #143 again, silent, with neverFatal swallowing it. +describe('/internal/* normalises a configured cache origin', () => { + it('keys on the real origin when PROD_HOST is written WITH a scheme', async () => { + const sha = 'e'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + seed(PUBLIC_ORIGIN, k, fixture('v4.16.0')); + // Distinguishable from the seeded slot: if the key lands on `https://https` + // the seeded entry is invisible and this recomputed answer is what comes back. + findReleaseMock.mockResolvedValue(fixture('MISSED-THE-PUBLIC-SLOT')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), { + INTERNAL_SECRET, + PROD_HOST: `https://${PROD_HOST}`, + }); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.16.0'); // the PUBLIC slot was hit + expect(findReleaseMock).not.toHaveBeenCalled(); + }); + + it('keys on the real origin when PUBLIC_BASE_URL is written WITHOUT one', async () => { + const sha = 'f'.repeat(40); + findReleaseMock.mockResolvedValue(fixture('v4.17.0')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), { + INTERNAL_SECRET, + PUBLIC_BASE_URL: PROD_HOST, + }); + + // Pre-fix this threw out of `new Request(...)` — outside neverFatal — so the + // computed answer became a 503, which web-og renders as the placeholder. + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.17.0'); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + expect(await tagOfSlot(PUBLIC_ORIGIN, k)).toBe('v4.17.0'); + }); + + it('ignores a configured value whose origin is OPAQUE, rather than 500ing', async () => { + const sha = '9'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + seed(PUBLIC_ORIGIN, k, fixture('v4.18.0')); + // Distinguishable from the seeded slot, as above. + findReleaseMock.mockResolvedValue(fixture('MISSED-THE-PUBLIC-SLOT')); + + // `URL.origin` is the literal string "null" for any opaque (non-special + // scheme) origin. `file:///srv/web` contains '//', so it skips the + // scheme-prefix branch, parses fine, and yields "null" — a non-null, + // non-URL string. Unguarded that satisfies `??` and reaches + // `new Request(...)`, which throws OUTSIDE neverFatal: app.onError turns a + // computable OG lookup into a 500 and web-og renders the neutral + // placeholder. Same failure class as the scheme-less PUBLIC_BASE_URL above. + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), { + INTERNAL_SECRET, + PUBLIC_BASE_URL: 'file:///srv/web', + PROD_HOST, + }); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.18.0'); // fell through to PROD_HOST's public slot + expect(findReleaseMock).not.toHaveBeenCalled(); + }); +}); + +// Aligning the /internal key with the public one (the fix above) also aligned the +// NEGATIVE back-off marker `:neg`, which public page views write. That marker +// is a good idea for a page a human can reload, and a bad one for a crawler: a +// crawler unfurls ONCE and keeps what it got. So a 60-second marker left by an +// unrelated human page view could hand the crawler a permanent placeholder — the +// exact #143 symptom this PR exists to remove, re-introduced through the shared key. +// The back-off is therefore honoured only when there is a prior to stale-serve. +describe('/internal/* does not let a shared back-off marker cause a permanent placeholder', () => { + it('computes on a COLD slot even when a public page view left a warm `:neg` marker', async () => { + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + // A human loaded the permalink seconds ago, GitHub 502'd, the public route + // wrote the shared back-off marker. No result was ever cached. + seedAged(PUBLIC_ORIGIN, `${k}:neg`, { transient: true, kind: 'github_server_error' }, 10); + // Upstream has since recovered. + findReleaseMock.mockResolvedValue(fixture('v4.12.11')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + await settle(); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.12.11'); + expect(findReleaseMock).toHaveBeenCalledTimes(1); + // ...and the answer is warm for the next unfurl. + expect(await tagOfSlot(PUBLIC_ORIGIN, k)).toBe('v4.12.11'); + }); + + // Round 9 (#144). This used to claim the opposite — "the marker still holds when + // there IS a prior" — on a seeded `firstRelease: null, no partial` entry, a shape + // findRelease never emits (see pendingFixture). On the two shapes it DOES emit the + // bypass is unconditional, and that is worth pinning honestly rather than papering + // over: a TERMINAL prior returns at the fresh exit before the marker is ever read, + // and a PARTIAL prior past its 60s is unpinnable, so the marker is skipped and the + // lookup runs. The cost (an outage is re-probed by every unfurl) is accepted in + // resolveLookup's fall-through comment; the alternative for a consumer that asks + // once is a placeholder pinned long after the host recovers. + it('skips a warm marker even WITH a stale partial prior — the bypass is not cold-only', async () => { + const k = await publicKey('github.com/honojs/hono', `sha:${SHA}`); + seedAged(PUBLIC_ORIGIN, k, partialFixture(), 10 * 60); // past its 60s: unpinnable + seedAged(PUBLIC_ORIGIN, `${k}:neg`, { transient: true, kind: 'github_server_error' }, 10); + findReleaseMock.mockResolvedValue(fixture('v4.12.11')); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${SHA}`), ENV); + await settle(); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.12.11'); + expect(findReleaseMock).toHaveBeenCalledTimes(1); + }); + + // ...and the complement, so the guard above cannot pass by simply never reading + // the marker: a TERMINAL prior is served from the fresh exit, upstream untouched. + it('a terminal prior is still served from cache while the marker is warm', async () => { + const sha = '8'.repeat(40); + const k = await publicKey('github.com/honojs/hono', `sha:${sha}`); + seedAged(PUBLIC_ORIGIN, k, fixture('v4.11.0'), 10 * 60); + seedAged(PUBLIC_ORIGIN, `${k}:neg`, { transient: true, kind: 'github_server_error' }, 10); + + const res = await app.fetch(svc(`https://web/internal/result/honojs/hono/${sha}`), ENV); + + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.11.0'); + expect(findReleaseMock).not.toHaveBeenCalled(); + }); +}); + +// Aligning the cache key also aligned the in-isolate SINGLE-FLIGHT key, which is a +// sharing this route did not have on main (it keyed on a three-part key of its +// own). `singleFlight` hands every joiner the FIRST registrant's promise and runs +// only that owner's loader, and badge.ts builds a byte-identical key for +// `issue#N`/`pr#N` (badge.ts:110) with a deliberately tighter 8s/9s deadline. So a +// badge request landing ~1s earlier in the same isolate would hand this route a +// truncated `partial`, which it 503s into a pinned neutral placeholder — #143's +// symptom, on a link where this route's own 24s deadline finds the real answer. +describe('/internal/* runs its own lookup instead of joining the badge flight', () => { + it('does not inherit an 8-second-deadline partial registered by badge.ts', async () => { + // The key badge.ts registers its flight under for `/badge/.../issue/11.svg`. + const k = await publicKey('github.com/honojs/hono', 'issue#11'); + let finishBadge: (value: unknown) => void = () => {}; + const badgeFlight = singleFlight(k, () => new Promise((r) => (finishBadge = r))); + + findReleaseMock.mockResolvedValue(fixture('v4.13.0')); + const pending = app.fetch(svc('https://web/internal/issue/honojs/hono/11'), ENV); + await settle(); + // Badge's tighter deadline ran out: a truncated traversal, all it can offer. + finishBadge(partialFixture()); + await badgeFlight; + + const res = await pending; + expect(res.status).toBe(200); + expect(await tagOf(res)).toBe('v4.13.0'); + expect(findReleaseMock).toHaveBeenCalledOnce(); + }); +}); diff --git a/packages/web/test/resolve.test.ts b/packages/web/test/resolve.test.ts index f0b2d2b..99abcab 100644 --- a/packages/web/test/resolve.test.ts +++ b/packages/web/test/resolve.test.ts @@ -37,23 +37,43 @@ function mkResult(opts: { released: boolean; partial?: boolean }): LookupResult /** In-memory WorkerCache whose entry ages are set explicitly by the test. */ function makeFakeCache() { - const store = new Map(); + const store = new Map(); + const puts: { key: string; ttlSeconds?: number }[] = []; const cache: WorkerCache = { async get(key: string) { return (store.get(key)?.value as T) ?? null; }, async getEntry(key: string): Promise | null> { const e = store.get(key); - return e ? { value: e.value as T, ageSeconds: e.ageSeconds } : null; + return e ? { value: e.value as T, ageSeconds: e.ageSeconds, stampedAt: e.stampedAt } : null; }, - async put(key: string, value: T) { - store.set(key, { value, ageSeconds: 0 }); + async put(key: string, value: T, ttlSeconds?: number) { + puts.push({ key, ttlSeconds }); + store.set(key, { value, ageSeconds: 0, stampedAt: Date.now() }); }, }; return { cache, - seed(key: string, value: unknown, ageSeconds: number) { - store.set(key, { value, ageSeconds }); + /** Every `cache.put` in call order, with the TTL the caller asked for. The + * slot is shared across callers that do not agree on a deadline, so the TTL + * a write imposes on it is observable behaviour, not an implementation + * detail — asserting only the stored VALUE cannot see a caller shortening + * someone else's entry. */ + puts, + /** `stampedAt` is the write-time `x-cached-at` the real cache stores. It is + * INDEPENDENT of `ageSeconds` here on purpose: production derives the two + * from different `Date.now()` samples, so a test has to be able to express a + * pair whose ages disagree with their true write order. Default it to a + * stamp consistent with the age, so existing tests are unaffected. */ + seed(key: string, value: unknown, ageSeconds: number, stampedAt?: number | null) { + // `null` seeds an UNSTAMPED entry (one written before `x-cached-at` + // existed, or whose header an intermediary dropped) — distinct from + // omitting the argument, which derives the stamp from the age. + store.set(key, { + value, + ageSeconds, + stampedAt: stampedAt === undefined ? Date.now() - ageSeconds * 1000 : stampedAt, + }); }, has: (key: string) => store.has(key), get: (key: string) => store.get(key), @@ -62,6 +82,7 @@ function makeFakeCache() { const KEY = 'res:gtk:pr#9951'; const negKey = `${KEY}:neg`; +const pinPartialKey = `${KEY}:pinpartial`; describe('isTransientError', () => { it('treats 5xx / network / timeout / rate-limit as transient', () => { @@ -254,3 +275,518 @@ describe('resolveLookup — real answers pass through', () => { expect(f.has(negKey)).toBe(false); }); }); + +// Round-6 review of #144. Earlier rounds bounded only one exit, so both +// stale-if-error exits — the back-off short-circuit and the transient catch — +// could still hand the OG crawler a prior of any age. Sharing +// the public routes' 24h slot is what made that reachable: before #143 this +// route had a flat 30-minute TTL, so a 23h-old prior could not exist on it. +// A pinned consumer renders whatever it gets into a PNG cached for a day. +describe('resolveLookup — a pinned consumer is never handed a prior past the bound', () => { + const aged = () => mkResult({ released: false }); + + it('back-off exit: attempts a fresh lookup instead of serving a 23h-old prior', async () => { + const f = makeFakeCache(); + f.seed(KEY, aged(), 23 * 60 * 60); // inside HARD_TTL_PENDING (24h), way past the bound + f.seed(negKey, { transient: true, kind: 'github_server_error' }, 10); // a page view just failed + const fresh = mkResult({ released: true }); + const load = vi.fn().mockResolvedValue(fresh); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + bypassBackOffWhenUnservable: true, + consumerPinsResult: true, + }); + + expect(load).toHaveBeenCalledTimes(1); + expect(r.status).toBe('ok'); + if (r.status === 'ok') { + expect(r.stale).toBe(false); + expect(r.result.firstRelease?.tag).toBe('4.18.0'); + } + }); + + it('transient-catch exit: returns transient rather than the 23h-old prior', async () => { + const f = makeFakeCache(); + f.seed(KEY, aged(), 23 * 60 * 60); + const load = vi.fn(async () => { + throw new ProviderServerError('github.com', 503, 'Service Unavailable'); + }); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + bypassBackOffWhenUnservable: true, + consumerPinsResult: true, + }); + + expect(load).toHaveBeenCalledTimes(1); + expect(r.status).toBe('transient'); + }); + + it('a prior INSIDE the bound is still stale-served to a pinned consumer', async () => { + const f = makeFakeCache(); + // Deliberately NOT a `partial`: round 7 made those unpinnable at every exit + // regardless of age, so a partial would no longer isolate the age bound. + const pending = mkResult({ released: false }); + f.seed(KEY, pending, 10 * 60); // stale (past the 5-min freshness window), inside the 30-min bound + f.seed(negKey, { transient: true, kind: 'github_server_error' }, 10); + const load = vi.fn(); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + bypassBackOffWhenUnservable: true, + consumerPinsResult: true, + }); + + expect(load).not.toHaveBeenCalled(); + expect(r.status).toBe('ok'); + if (r.status === 'ok') { + expect(r.stale).toBe(true); + expect(r.result).toEqual(pending); + } + }); + + it('public routes keep UNBOUNDED stale-if-error — the bound is opt-in only', async () => { + const f = makeFakeCache(); + const old = aged(); + f.seed(KEY, old, 23 * 60 * 60); + const load = vi.fn(async () => { + throw new ProviderServerError('github.com', 503, 'Service Unavailable'); + }); + + // Same 23h prior, same failure — but no consumerPinsResult: a human page is + // not pinned anywhere and shows an explicit "stale as of" caveat, so serving + // through a long outage stays the right degrade. + const r = await resolveLookup({ cache: f.cache, key: KEY, load }); + + expect(r.status).toBe('ok'); + if (r.status === 'ok') { + expect(r.stale).toBe(true); + expect(r.result).toEqual(old); + } + }); +}); + +// Round-7 review of #144. The round-6 bound was consulted only on the three +// STALE exits. The fresh-hit return above them never asked — and `isFresh()` +// calls a `partial` fresh for its whole 60s life. Aligning this route onto the +// public five-part key (this PR) is what made that reachable: `badge.ts` loads +// the identical key with an 8s soft deadline, so on a large repo it writes a +// `partial` (firstRelease `null`) that `/internal` then served as a plain 200. +// web-og renders `firstRelease?.tag ?? 'not yet released'` and long-caches any +// non-null result, so a RELEASED commit gets a "not yet released" card pinned +// for a day — the CLAUDE.md guardrail ("Partial state != not yet released") +// and the exact outcome `consumerPinsResult` exists to prevent. +describe('resolveLookup — a pinned consumer is never handed a cached PARTIAL', () => { + // Round 8 narrows this ONE case, and only where it costs nothing: a partial + // still inside its own 60s TTL is handed BACK rather than recomputed. It is not + // thereby pinnable — the route refuses every `partial` (routes/internal.ts) and + // 503s it into a neutral placeholder at max-age=60. Recomputing here instead is + // what made the refusal unthrottled: no read path accepted the entry + // resolveLookup had just written, so on a repo that reliably blows the 24s soft + // deadline every unfurl ran another full traversal on the shared token, where + // the flat 30-minute TTL this route replaced made zero upstream calls. + it('fresh exit: hands back a 10s-old partial THIS caller wrote rather than re-run the lookup', async () => { + const f = makeFakeCache(); + const truncated = mkResult({ released: false, partial: true }); + f.seed(KEY, truncated, 10); // well inside the 60s partial freshness window + f.seed(pinPartialKey, { pinnedPartial: true }, 10); // ...and this caller produced it + const fresh = mkResult({ released: true }); + const load = vi.fn().mockResolvedValue(fresh); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + }); + + expect(load).not.toHaveBeenCalled(); + expect(r.status).toBe('ok'); + // Still flagged, so the pinning caller refuses it — the refusal just costs a + // cache read now instead of a findRelease. + if (r.status === 'ok') { + expect(r.result.partial).toBeTruthy(); + expect(r.result.firstRelease).toBeNull(); + } + }); + + // find-release.ts returns a SECOND partial shape: a gallop hit the bisect never + // confirmed is the earliest containing release (find-release.ts:293-305). Both + // `hardTtlFor()` and `isFresh()` test `firstRelease` before `partial`, so that + // shape is stored for 30 days and reported fresh forever — a pinning consumer + // would get a possibly-wrong tag from an entry of any age, with no caveat and + // no revalidation. (The terminal misclassification itself is on `main` and + // affects the public routes too; the pin bound rejecting it on the way out is + // what this PR owes.) + // Round 14 review of #144. Round 13 bound the marker to the entry it vouches + // for with `mark.ageSeconds <= prior.ageSeconds` — but those two ages are + // floored from two DIFFERENT `Date.now()` samples, one Cache API round trip + // apart, so the floors can straddle a second boundary and report the marker as + // OLDER than an entry it was in fact written AFTER. The guard then disowns this + // caller's own seconds-old partial and runs a full traversal — on exactly the + // repos the throttle exists for. The stored `x-cached-at` stamps are the + // quantity the ordering actually depends on. + it('honours its own marker when the two READ ages straddle a second boundary', async () => { + const f = makeFakeCache(); + const truncated = mkResult({ released: false, partial: true }); + // The reviewer's arithmetic, seeded directly: entry written at t=1000 and + // read back as 1s old; marker written 10ms LATER at t=1010 and read back as + // 2s old, because its read happened 25ms further on. + f.seed(KEY, truncated, 1, 1000); + f.seed(pinPartialKey, { pinnedPartial: true }, 2, 1010); + const load = vi.fn().mockResolvedValue(mkResult({ released: true })); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + }); + + expect(load).not.toHaveBeenCalled(); + expect(r.status).toBe('ok'); + if (r.status === 'ok') expect(r.result.partial).toBeTruthy(); + }); + + // Round 15 review of #144. The header claims the missing-stamp fallback goes + // "the same conservative direction as before — an unprovable ordering + // recomputes". It did the opposite: `cache.getEntry` reports `ageSeconds: 0` + // for an entry with no `x-cached-at` (cache.ts:65-66), so a pair where BOTH + // stamps are absent evaluated `0 <= 0` -> true and the unstamped marker + // vouched for the unstamped entry unconditionally. + it('recomputes when NEITHER the marker nor the entry carries a stamp', async () => { + const f = makeFakeCache(); + const someoneElses = mkResult({ released: false, partial: true }); + // Both unstamped, so `ageSeconds` is 0 on both sides — the shape that made + // the fallback vouch instead of recompute. + f.seed(KEY, someoneElses, 0, null); + f.seed(pinPartialKey, { pinnedPartial: true }, 0, null); + const load = vi.fn().mockResolvedValue(mkResult({ released: true })); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + }); + + expect(load).toHaveBeenCalledTimes(1); + expect(r.status).toBe('ok'); + }); + + // ...and one stamp present is just as unprovable as none: there is nothing to + // compare the stamped side AGAINST, so this must recompute too. + it('recomputes when only ONE side of the pair carries a stamp', async () => { + const f = makeFakeCache(); + const someoneElses = mkResult({ released: false, partial: true }); + // The entry is unstamped and read as 5s old; the marker IS stamped and read + // as 0s old. Ages alone say `0 <= 5` -> vouch, which is the same wrong + // answer by a different route. + f.seed(KEY, someoneElses, 5, null); + f.seed(pinPartialKey, { pinnedPartial: true }, 0, 1000); + const load = vi.fn().mockResolvedValue(mkResult({ released: true })); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + }); + + expect(load).toHaveBeenCalledTimes(1); + expect(r.status).toBe('ok'); + }); + + // ...and the ordering test must still REJECT a slot genuinely overwritten after + // the marker was written, or the fix above is just a way of deleting the guard. + it('still disowns a partial written AFTER the marker (badge.ts overwrote the slot)', async () => { + const f = makeFakeCache(); + const someoneElses = mkResult({ released: false, partial: true }); + // Marker at t=1000; the slot then overwritten at t=5000 by an 8s-deadline + // caller on the same key. Ages alone would say the marker is the younger one. + f.seed(KEY, someoneElses, 1, 5000); + f.seed(pinPartialKey, { pinnedPartial: true }, 5, 1000); + const fresh = mkResult({ released: true }); + const load = vi.fn().mockResolvedValue(fresh); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + }); + + expect(load).toHaveBeenCalledTimes(1); + expect(r.status).toBe('ok'); + if (r.status === 'ok') expect(r.result.firstRelease?.tag).toBe('4.18.0'); + }); + + it('a gallop-only partial is not treated as terminal, however old', async () => { + const f = makeFakeCache(); + const gallopOnly = { ...mkResult({ released: true }), partial: { reason: 'soft_deadline' } }; + f.seed(KEY, gallopOnly, 20 * 24 * 60 * 60); // 20 days — "fresh forever" today + const load = vi.fn().mockResolvedValue(mkResult({ released: true })); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + }); + + expect(load).toHaveBeenCalledTimes(1); + expect(r.status).toBe('ok'); + if (r.status === 'ok') expect(r.result.partial).toBeUndefined(); + }); + + it('...but a TERMINAL answer of the same age is still served, never recomputed', async () => { + // The complement that stops the bound above from swallowing the warm entries + // this route joined the public key to reuse. + const f = makeFakeCache(); + f.seed(KEY, mkResult({ released: true }), 20 * 24 * 60 * 60); + const load = vi.fn().mockResolvedValue(mkResult({ released: true })); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + }); + + expect(load).not.toHaveBeenCalled(); + expect(r.status).toBe('ok'); + }); + + it('stale exit: a partial inside the 30-min bound is still not pinnable', async () => { + const f = makeFakeCache(); + const truncated = mkResult({ released: false, partial: true }); + f.seed(KEY, truncated, 120); // past the 60s partial window, inside the 30-min bound + f.seed(negKey, { transient: true, kind: 'github_server_error' }, 10); + const fresh = mkResult({ released: true }); + const load = vi.fn().mockResolvedValue(fresh); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + bypassBackOffWhenUnservable: true, + consumerPinsResult: true, + }); + + expect(load).toHaveBeenCalledTimes(1); + expect(r.status).toBe('ok'); + if (r.status === 'ok') expect(r.result.firstRelease?.tag).toBe('4.18.0'); + }); + + it('public routes still get the fresh partial — the guard is opt-in only', async () => { + const f = makeFakeCache(); + const truncated = mkResult({ released: false, partial: true }); + f.seed(KEY, truncated, 10); + const load = vi.fn(); + + // No consumerPinsResult: the result card renders `partial` as an explicit + // best-effort caveat (CLAUDE.md), so a fresh partial is the right answer. + const r = await resolveLookup({ cache: f.cache, key: KEY, load }); + + expect(load).not.toHaveBeenCalled(); + expect(r.status).toBe('ok'); + if (r.status === 'ok') expect(r.result).toEqual(truncated); + }); +}); + +// A RELEASED answer is terminal: which release first contains a commit cannot +// change, which is why `isFresh()` treats it as fresh forever and `hardTtlFor()` +// gives it a 30-day TTL. `MAX_STALE_PINNED` exists for the opposite case — a +// "not yet released" prior that has since shipped — so applying its 30-minute +// age bound to a terminal answer discards exactly the warm entries `/internal/*` +// joined the public key to reuse (#143), and pays a full findRelease on the +// crawler's critical path for every unfurl more than 30 minutes after the last +// write. +describe('resolveLookup — a terminal RELEASED prior stays pinnable at any age', () => { + it('fresh exit: serves a 2h-old released prior instead of recomputing', async () => { + const f = makeFakeCache(); + const released = mkResult({ released: true }); + f.seed(KEY, released, 2 * 60 * 60); // the normal state of a 30-day slot + const load = vi.fn(); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + }); + + expect(load).not.toHaveBeenCalled(); + expect(r.status).toBe('ok'); + if (r.status === 'ok') { + expect(r.cached).toBe(true); + expect(r.result.firstRelease?.tag).toBe('4.18.0'); + } + }); + + it('back-off exit: an upstream outage still serves the 2h-old released prior', async () => { + const f = makeFakeCache(); + const released = mkResult({ released: true }); + f.seed(KEY, released, 2 * 60 * 60); + f.seed(negKey, { transient: true, kind: 'github_server_error' }, 10); + const load = vi.fn(); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + bypassBackOffWhenUnservable: true, + consumerPinsResult: true, + }); + + // Without the terminal exemption this returns `transient`, which web-og + // renders as the neutral placeholder at max-age=60 — and each unfurl during + // the outage runs its own lookup out to the soft deadline against the down + // host. + expect(load).not.toHaveBeenCalled(); + expect(r.status).toBe('ok'); + if (r.status === 'ok') expect(r.result.firstRelease?.tag).toBe('4.18.0'); + }); + + it('a 2h-old NOT-YET-released prior is still unpinnable — the bound it exists for', async () => { + const f = makeFakeCache(); + const notYet = mkResult({ released: false }); + f.seed(KEY, notYet, 2 * 60 * 60); + const fresh = mkResult({ released: true }); + const load = vi.fn().mockResolvedValue(fresh); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + }); + + expect(load).toHaveBeenCalledTimes(1); + expect(r.status).toBe('ok'); + if (r.status === 'ok') expect(r.result.firstRelease?.tag).toBe('4.18.0'); + }); +}); + +// Round 15. Two defects the SHARED key introduced, both invisible to a test that +// looks only at what a single caller is handed back: the key is shared with +// badge.ts and the permalink pages, so what /internal writes to it — the entry's +// TTL, and the negative marker's age — is imposed on THEM. +describe('resolveLookup — a pinning caller does not rewrite the SHARED slot for everyone else', () => { + const HARD_TTL_RELEASED = 30 * 24 * 60 * 60; + const HARD_TTL_PARTIAL = 60; + + const galloped = () => mkResult({ released: true, partial: true }); + + it('writes a gallop partial on the caller-independent TTL, not its own 60s throttle', async () => { + const f = makeFakeCache(); + const load = vi.fn().mockResolvedValue(galloped()); + + await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + bypassBackOffWhenUnservable: true, + }); + + // The ENTRY keeps hardTtlFor()'s terminal branch — the same value a public + // page view writes — so an unfurl cannot shorten the permalink's month-long + // slot to a minute. Writing HARD_TTL_PARTIAL here reddens this. + expect(f.puts.find((p) => p.key === KEY)?.ttlSeconds).toBe(HARD_TTL_RELEASED); + // The caller's own distrust of the partial rides on the marker instead. + expect(f.puts.find((p) => p.key === pinPartialKey)?.ttlSeconds).toBe(HARD_TTL_PARTIAL); + }); + + it('a public caller and a pinning caller write the IDENTICAL entry TTL', async () => { + const pub = makeFakeCache(); + const pin = makeFakeCache(); + const load = vi.fn().mockResolvedValue(galloped()); + + await resolveLookup({ cache: pub.cache, key: KEY, load }); + await resolveLookup({ + cache: pin.cache, + key: KEY, + load, + consumerPinsResult: true, + bypassBackOffWhenUnservable: true, + }); + + const ttlOf = (f: ReturnType) => + f.puts.find((p) => p.key === KEY)?.ttlSeconds; + expect(ttlOf(pin)).toBe(ttlOf(pub)); + }); + + it('still recomputes its OWN partial once the marker has expired', async () => { + const f = makeFakeCache(); + // A 30-day entry sitting in the slot, and a marker that has aged out. + f.seed(KEY, galloped(), 5 * 60); + f.seed(pinPartialKey, { pinnedPartial: true }, HARD_TTL_PARTIAL + 1); + const load = vi.fn().mockResolvedValue(mkResult({ released: true })); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + bypassBackOffWhenUnservable: true, + }); + + // The longer entry TTL must not make a stale partial servable: the throttle + // reads the MARKER. Making shouldRecompute trust the entry reddens this. + expect(load).toHaveBeenCalledOnce(); + expect(r.status).toBe('ok'); + if (r.status === 'ok') expect(r.result.partial).toBeUndefined(); + }); + + it('does NOT re-stamp a negative marker it bypassed — the humans keep their clock', async () => { + const f = makeFakeCache(); + // An outage: a page view failed 55s ago, so the shared marker is 5s from + // expiring and a human is 5s from their next real attempt. + f.seed(negKey, { transient: true, kind: 'github_server_error' }, 55); + const load = vi.fn().mockRejectedValue(new ProviderServerError('gitlab.gnome.org', 503, 'x')); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + bypassBackOffWhenUnservable: true, + }); + + // The bypass still ran the load (that is its whole point)... + expect(load).toHaveBeenCalledOnce(); + expect(r.status).toBe('transient'); + // ...but left the marker's age alone. Removing the `!backedOff` guard reddens + // this: the marker is rewritten at age 0 and the human's window restarts, + // which under a once-a-minute unfurl cadence never lets it expire at all. + expect(f.puts.some((p) => p.key === negKey)).toBe(false); + expect(f.get(negKey)?.ageSeconds).toBe(55); + }); + + it('DOES stamp the marker when the slot was cold — the back-off still exists', async () => { + const f = makeFakeCache(); + const load = vi.fn().mockRejectedValue(new ProviderServerError('gitlab.gnome.org', 503, 'x')); + + const r = await resolveLookup({ + cache: f.cache, + key: KEY, + load, + consumerPinsResult: true, + bypassBackOffWhenUnservable: true, + }); + + expect(r.status).toBe('transient'); + // Suppressing the write unconditionally would leave the key with NO back-off + // whenever the crawler touches it first, and every human reload would pound + // the down host. Guarding on `backedOff` keeps this arm live. + expect(f.puts.some((p) => p.key === negKey)).toBe(true); + }); +}); diff --git a/packages/web/wrangler.toml b/packages/web/wrangler.toml index 8498843..2b21915 100644 --- a/packages/web/wrangler.toml +++ b/packages/web/wrangler.toml @@ -122,6 +122,30 @@ ANUBIS_HOSTS = "" # the preview never writes the prod released_events dataset. See isProdRequest() # in src/analytics.ts. The analytics binding below is isolated regardless. PROD_HOST = "released.blabberate.com" +# Cache origin + canonical URLs for THIS deployment. Without it the /internal/* +# endpoints web-og calls fall back to PROD_HOST above and key the result cache on +# a host the preview Worker does not serve. Must stay the preview Worker's own +# URL (name above + the account's workers.dev subdomain). +# +# NOTE: this does NOT make preview a place the #143 cache alignment can be +# exercised. Cloudflare documents Cache API operations as functional only for +# Workers on custom domains, so on a *.workers.dev preview neither the public +# routes nor /internal ever warm a slot, and a second unfurl still pays a full +# lookup however the origins line up. The alignment is proven by the unit tests +# (test/internal-cache-origin.test.ts) and only takes effect on the prod custom +# domain. The var is still correct to set: it keeps preview off the prod key +# namespace and drives canonical URLs. +# +# COUPLING: this var has a SECOND effect. `publicBaseUrl()` (src/env.ts) prefers +# it over the incoming request origin, so preview's canonical links, `og:url` and +# every `pubBase`-derived href come from this literal rather than being correct by +# construction. The `[env.preview] name` half of that is guarded: on a workers.dev +# host the first label IS the Worker name, and `cacheOriginProblems` +# (test/internal-cache-origin.test.ts) fails the build when the two drift, so a +# rename cannot silently leave this line behind. The ACCOUNT subdomain half is not +# guardable from this file — `.lukaso.workers.dev` appears nowhere else in it — so +# deploying under a different account still needs this line updated by hand. +PUBLIC_BASE_URL = "https://released-web-preview.lukaso.workers.dev" [env.preview.assets] directory = "./public"