diff --git a/site/.env.example b/site/.env.example new file mode 100644 index 0000000..2d8f2ba --- /dev/null +++ b/site/.env.example @@ -0,0 +1,38 @@ +# Environment for the DPROD marketing/spec site (site/). +# +# All four are read at request time by src/lib/spec-versions.ts. Set them on +# the Vercel project (Settings -> Environment Variables), not in the repo. + +# --- Vercel API: which branches have a READY deployment --------------------- +# A Vercel access token with read access to the dprod project. +VERCEL_TOKEN= +# Both are also injected automatically by Vercel at build time, but the +# spec-versions lookup runs per-request, so they must exist as real +# environment variables too. +VERCEL_PROJECT_ID= +VERCEL_ORG_ID= + +# --- GitHub API: which of those branches still exist ------------------------ +# REQUIRED, and it must be VALID: an expired or revoked token fails exactly +# like a missing one. Without a working token the lookup either runs +# unauthenticated (60 requests/hour per IP, shared across all of Vercel's +# egress, so exhausted continuously) or is rejected outright, and the version +# picker degrades to the archive and develop. +# +# Fine-grained token setup: +# Resource owner EKGF +# Repository access Only select repositories -> EKGF/dprod +# Permissions Contents: Read-only +# +# "Contents (read)" is what GET /repos/{owner}/{repo}/branches requires; see +# https://docs.github.com/en/rest/branches/branches. GitHub adds the mandatory +# "Metadata: Read-only" automatically once any repository permission is +# selected. No write permissions, and no Pull requests permission — the branch +# lookup does not touch that endpoint. +# +# A classic token needs `public_repo` instead. +# +# Fine-grained tokens expire. When the picker degrades, check the deployment +# logs: the failure is logged with GitHub's own message, which distinguishes +# "Bad credentials" (expired/revoked) from a rate limit or a missing scope. +GITHUB_TOKEN= diff --git a/site/.gitignore b/site/.gitignore index e985853..3e36448 100644 --- a/site/.gitignore +++ b/site/.gitignore @@ -1 +1,4 @@ .vercel + +# TypeScript incremental build info +tsconfig.tsbuildinfo diff --git a/site/README.md b/site/README.md new file mode 100644 index 0000000..365887a --- /dev/null +++ b/site/README.md @@ -0,0 +1,65 @@ +# DPROD site + +The Next.js app served at `https://ekgf.org/dprod` (proxied verbatim from the +`ekgf-website` zone) and at `https://dprod.ekgf.vercel.app`. + +## Spec version discovery + +`src/lib/spec-versions.ts` builds the list behind `/spec-versions` and the +`/spec/` routing in `middleware.ts`. It combines two sources: + +| Source | Question it answers | Env vars | +|---|---|---| +| Vercel deployments API | Which branches have a READY deployment? | `VERCEL_TOKEN`, `VERCEL_PROJECT_ID`, `VERCEL_ORG_ID` | +| GitHub branches API | Which of those branches still exist? | `GITHUB_TOKEN` | + +Both are cached for 60 seconds via `next: { revalidate: 60 }`, so a new branch +appears without redeploying `develop`. + +A branch deleted from `origin` drops off the listing, **including `ballot/*`**. +Its Vercel deployment survives and `/spec/` still resolves, so any URL +already published keeps working — it is simply no longer advertised. This is +deliberate (issue #249): the listing answers "which versions exist now?", and +a deleted branch does not. If a ballot needs to stay listed after its branch is +gone, keep the branch on `origin` rather than special-casing it here. + +There are two entry points, and the difference matters: + +- **`getSpecVersions()`** — everything routable. Permissive on purpose: the + middleware resolves explicit `/spec/` URLs with it, and a URL someone + already holds should keep working even when GitHub is unreachable. +- **`getListedSpecVersions()`** — what a reader is shown. Fails **closed**: if + branch existence cannot be determined, it lists only the archive and + `develop`. + +## `GITHUB_TOKEN` + +Required, and it must be **valid** — an expired or revoked token behaves +exactly like a missing one. Fine-grained tokens expire, so this will recur. + +A fine-grained token needs **`Contents: Read-only`** on `EKGF/dprod` — that is +the permission `GET /repos/{owner}/{repo}/branches` +[requires](https://docs.github.com/en/rest/branches/branches). GitHub adds the +mandatory `Metadata: Read-only` automatically. No write permissions, and no +Pull requests permission: the lookup does not use that endpoint. + +The repository is public, so the endpoint would also answer an unauthenticated +request — but that path carries GitHub's 60-requests-per-hour-per-IP limit, +shared across Vercel's egress, which is exactly the failure mode being avoided. +Granting `Contents: Read-only` is what makes the request authenticated, at +5,000 requests per hour. + +See `.env.example`. + +When the branch lookup fails, two things happen and neither is silent: + +- the reason is logged with `console.warn`, including GitHub's own message — + `Bad credentials` for an expired or revoked token, a rate-limit message when + running unauthenticated, `Resource not accessible` for a missing scope; +- the page renders a "Branch list unavailable" notice. + +This matters because issue #249 was caused by the *absence* of both. The +picker advertised 22 branches, 11 of them deleted months earlier, while only 6 +pull requests were open — and nothing anywhere reported that the filter had +stopped working. The token was configured the whole time; it simply was not +being accepted, which the old code could not distinguish from success. diff --git a/site/src/app/spec-versions/page.tsx b/site/src/app/spec-versions/page.tsx index 030aa61..70ebb91 100644 --- a/site/src/app/spec-versions/page.tsx +++ b/site/src/app/spec-versions/page.tsx @@ -1,7 +1,7 @@ import type { Metadata } from "next"; import Link from "next/link"; import { ArrowRight, CheckCircle2, Clock, Archive } from "lucide-react"; -import { getSpecVersions, type SpecVersion } from "@/lib/spec-versions"; +import { getListedSpecVersions, type SpecVersion } from "@/lib/spec-versions"; export const metadata: Metadata = { title: "Spec Versions — DPROD", @@ -48,7 +48,10 @@ function BadgeFor({ version }: { version: SpecVersion }) { } export default async function SpecVersionsPage() { - const versions = await getSpecVersions(); + // Deliberately the *listed* set, not the routable one: a branch that no + // longer exists must not be advertised, even though its preview URL still + // resolves. See issue #249. + const { versions, complete } = await getListedSpecVersions(); return (
@@ -74,6 +77,20 @@ export default async function SpecVersionsPage() {
+ {!complete && ( +
+

+ Branch list unavailable +

+

+ The GitHub lookup that checks which branches still exist did + not succeed, so only the archive and the production draft are + listed. In-flight preview branches are hidden rather than + shown unverified. The deployment logs record the reason. +

+
+ )} +
{versions.map((version) => { const href = @@ -119,9 +136,11 @@ export default async function SpecVersionsPage() { Every branch in the repository automatically gets its own Vercel preview deployment. This page queries the Vercel API at request time (cached for 60 seconds) so new branches show - up without needing to redeploy develop. Each - version link routes through Next.js middleware to the correct - deployment's own /spec/ page. + up without needing to redeploy develop, and + cross-checks GitHub so that branches which have since been + deleted drop off the list. Each version link routes through + Next.js middleware to the correct deployment's own{" "} + /spec/ page.

diff --git a/site/src/lib/spec-versions.ts b/site/src/lib/spec-versions.ts index a9c85c1..af5e02c 100644 --- a/site/src/lib/spec-versions.ts +++ b/site/src/lib/spec-versions.ts @@ -32,6 +32,11 @@ export type SpecVersion = { isCurrent: boolean; /** True if this version is the production branch (develop) */ isProduction: boolean; + /** + * Whether the branch still exists upstream. `null` means the lookup was + * unavailable, so existence is unknown — never treat that as "yes". + */ + existsUpstream: boolean | null; kind: SpecVersionKind; }; @@ -43,12 +48,25 @@ const ARCHIVE_1_0: SpecVersion = { origin: "", isCurrent: false, isProduction: false, + existsUpstream: true, kind: "archive", }; /** Branches that we never want to advertise as spec versions. */ const EXCLUDED_BRANCHES = new Set(["main"]); +/** + * Branch prefixes that are never spec versions. A dependency bump produces a + * perfectly valid preview deployment, but it is not a version of the + * specification and only adds noise to the picker. + */ +const EXCLUDED_BRANCH_PREFIXES = ["dependabot/"]; + +function isAdvertisableBranch(branch: string): boolean { + if (EXCLUDED_BRANCHES.has(branch)) return false; + return !EXCLUDED_BRANCH_PREFIXES.some((prefix) => branch.startsWith(prefix)); +} + function branchToSlug(branch: string): string { return branch.replace(/\//g, "-"); } @@ -100,83 +118,111 @@ async function fetchDeployments(): Promise { } } -type GitHubPullRequest = { - head?: { ref?: string }; - state?: string; +type GitHubBranch = { + name?: string; }; /** - * Returns the set of branches that have at least one open Pull Request on - * GitHub, plus the production branch "develop". Used to filter out Vercel - * deployments whose branch has already been merged: once a PR closes, its - * branch typically gets auto-deleted (or at least becomes irrelevant), and - * its Vercel preview deployment — while still reachable by URL — should no - * longer appear in the user-facing version picker. + * Every branch that currently exists in the repository, or `null` when the + * lookup was unavailable. + * + * Existence — not open-PR status — is the right question. A branch whose PR + * has merged is normally deleted, and its Vercel preview, while still + * reachable by URL, is no longer a version of anything. Conversely a branch + * can legitimately exist with no open PR (merged but kept, or pushed before + * the PR is raised), and an open-PR filter hid those too. + * + * Returns `null` rather than an empty set on failure, so callers can tell + * "no branches" apart from "could not ask" — very different things. * - * On failure (network error, missing token, rate limit, non-2xx response) - * the function returns `null` so callers can fail *open* — i.e. show every - * branch Vercel knows about rather than silently hide valid ones. + * Every failure path logs the status and GitHub's own message. The bug this + * replaced (issue #249) was undiagnosable from outside precisely because it + * failed silently: a present-but-rejected token looks exactly like a missing + * one when nothing is logged. */ -async function fetchActiveBranches(): Promise | null> { +async function fetchExistingBranches(): Promise | null> { const token = process.env.GITHUB_TOKEN; + if (!token) { + // Unauthenticated GitHub allows 60 requests/hour per IP, shared across + // every function on that egress address, so this is not a + // degraded-but-workable path — it fails continuously. + console.warn( + "[spec-versions] GITHUB_TOKEN is not set; cannot determine which " + + "branches still exist. Showing the archive and develop only.", + ); + return null; + } + const headers: Record = { Accept: "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28", + Authorization: `Bearer ${token}`, + // GitHub rejects requests without a User-Agent with 403. Runtimes differ + // in whether they supply a default, so set one explicitly rather than + // depending on the platform. + "User-Agent": "ekgf-dprod-site", }; - if (token) headers.Authorization = `Bearer ${token}`; - try { - const res = await fetch( - "https://api.github.com/repos/EKGF/dprod/pulls?state=open&per_page=100", - { - headers, - next: { revalidate: 60 }, - }, - ); - if (!res.ok) return null; - const prs = (await res.json()) as GitHubPullRequest[]; - const branches = new Set(); - for (const pr of prs) { - const ref = pr.head?.ref; - if (ref) branches.add(ref); + const branches = new Set(); + // The repository has far fewer than 100 branches today, but paginate + // anyway: silently truncating would hide live branches. + for (let page = 1; page <= 10; page++) { + let res: Response; + try { + res = await fetch( + `https://api.github.com/repos/EKGF/dprod/branches?per_page=100&page=${page}`, + { headers, next: { revalidate: 60 } }, + ); + } catch (error) { + console.warn("[spec-versions] GitHub branch lookup threw:", error); + return null; } - // Always treat the production branch as active. - branches.add("develop"); - return branches; - } catch { - return null; + if (!res.ok) { + // GitHub's body distinguishes the cases that matter: "Bad credentials" + // (401, token expired or revoked), "API rate limit exceeded" (403), + // and resource-not-accessible (403, token lacks Metadata: Read-only). + const detail = await res.text().catch(() => ""); + console.warn( + `[spec-versions] GitHub branch lookup failed: ${res.status} ` + + `${res.statusText}. ${detail.slice(0, 300)}`, + ); + return null; + } + const pageBranches = (await res.json()) as GitHubBranch[]; + for (const branch of pageBranches) { + if (branch.name) branches.add(branch.name); + } + if (pageBranches.length < 100) break; } + return branches; } /** - * Returns the list of spec versions, in display order: - * 1. The frozen 1.0 archive (always first, always present). - * 2. develop (if deployed). - * 3. Every other branch with a READY deployment, latest first. + * Every spec version this deployment can *route* to: the frozen archive plus + * each branch with a READY Vercel deployment. * - * Never throws — on any failure, returns at least the archive entry so + * Intentionally permissive. The middleware uses this to resolve an explicit + * /spec/ URL, and a URL someone already holds should keep working even + * while the GitHub lookup is unavailable. Use `getListedSpecVersions()` for + * anything user-facing. + * + * Never throws — on any failure it still returns the archive entry, so * /spec/main keeps working. */ export async function getSpecVersions(): Promise { const currentBranch = process.env.VERCEL_GIT_COMMIT_REF; const versions: SpecVersion[] = [ARCHIVE_1_0]; - const [deployments, activeBranches] = await Promise.all([ + const [deployments, existingBranches] = await Promise.all([ fetchDeployments(), - fetchActiveBranches(), + fetchExistingBranches(), ]); const seen = new Set(); for (const dep of deployments) { const branch = dep.meta?.githubCommitRef; - if (!branch || EXCLUDED_BRANCHES.has(branch) || seen.has(branch)) continue; - - // Fail open: when the GitHub lookup failed, keep every branch. When it - // succeeded, only keep branches with an open PR (or the production - // branch, which fetchActiveBranches() adds unconditionally). - if (activeBranches && !activeBranches.has(branch)) continue; - + if (!branch || !isAdvertisableBranch(branch) || seen.has(branch)) continue; seen.add(branch); const slug = branchToSlug(branch); @@ -194,6 +240,7 @@ export async function getSpecVersions(): Promise { origin: `https://${slug}.dprod-preview.ekgf.org/dprod`, isCurrent: branch === currentBranch, isProduction: branch === "develop", + existsUpstream: existingBranches ? existingBranches.has(branch) : null, kind: "vercel-branch", }); } @@ -209,3 +256,42 @@ export async function getSpecVersions(): Promise { return versions; } + +/** + * What a reader is shown, plus whether the list could be filtered at all. + */ +export type ListedSpecVersions = { + versions: SpecVersion[]; + /** + * False when branch existence could not be determined, so the list is the + * fail-closed minimum rather than the real set. Surfaced in the UI: a + * silently short list is as misleading as a silently long one. + */ + complete: boolean; +}; + +/** + * The spec versions to show a reader: the archive, develop, and branches that + * still exist upstream. + * + * Fails *closed*. When branch existence is unknown the picker shows only the + * archive and develop, because the alternative — what shipped before issue + * #249 — was every branch ever deployed, including many deleted months + * earlier. A short list is a smaller lie than a wrong one, and + * `getSpecVersions()` still routes any preview URL that has been handed out. + */ +export async function getListedSpecVersions(): Promise { + const versions = await getSpecVersions(); + const complete = !versions.some( + (version) => version.kind === "vercel-branch" && version.existsUpstream === null, + ); + return { + complete, + versions: versions.filter( + (version) => + version.kind === "archive" || + version.isProduction || + version.existsUpstream === true, + ), + }; +}