diff --git a/site/.env.example b/site/.env.example index 2d8f2ba..6838a1e 100644 --- a/site/.env.example +++ b/site/.env.example @@ -22,13 +22,13 @@ VERCEL_ORG_ID= # Fine-grained token setup: # Resource owner EKGF # Repository access Only select repositories -> EKGF/dprod -# Permissions Contents: Read-only +# Permissions Pull requests: 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 +# "Pull requests (read)" is what GET /repos/{owner}/{repo}/pulls requires; see +# https://docs.github.com/en/rest/pulls/pulls. 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. +# selected. No write permissions, and no Contents permission — the listing +# only asks which pull requests are open. # # A classic token needs `public_repo` instead. # diff --git a/site/README.md b/site/README.md index 365887a..191627b 100644 --- a/site/README.md +++ b/site/README.md @@ -11,17 +11,20 @@ The Next.js app served at `https://ekgf.org/dprod` (proxied verbatim from the | 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` | +| GitHub pulls API | Which of those branches have an open pull request? | `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. +The listing is `main` + `develop` + every branch with an **open pull request**. +Everything else is left out: a merged branch nobody deleted, an abandoned +experiment, a `dependabot/*` bump. Those previews stay routable — a link shared +on a pull request keeps working after it merges — but they are not advertised +as versions of the specification. + +`ballot/*` is not special-cased. A ballot branch is listed while its pull +request is open; afterwards it lives on as the frozen archive entry (`main`) +if it was adopted, or as an unlisted but still-resolvable preview URL. There are two entry points, and the difference matters: @@ -37,11 +40,11 @@ There are two entry points, and the difference matters: 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 +A fine-grained token needs **`Pull requests: Read-only`** on `EKGF/dprod` — +that is the permission `GET /repos/{owner}/{repo}/pulls` +[requires](https://docs.github.com/en/rest/pulls/pulls). GitHub adds the mandatory `Metadata: Read-only` automatically. No write permissions, and no -Pull requests permission: the lookup does not use that endpoint. +`Contents` permission. 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, diff --git a/site/src/lib/spec-versions.ts b/site/src/lib/spec-versions.ts index af5e02c..be1a4b1 100644 --- a/site/src/lib/spec-versions.ts +++ b/site/src/lib/spec-versions.ts @@ -33,10 +33,10 @@ export type SpecVersion = { /** 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". + * Whether the branch has an open pull request. `null` means the lookup was + * unavailable, so it is unknown — never treat that as "yes". */ - existsUpstream: boolean | null; + hasOpenPullRequest: boolean | null; kind: SpecVersionKind; }; @@ -48,7 +48,7 @@ const ARCHIVE_1_0: SpecVersion = { origin: "", isCurrent: false, isProduction: false, - existsUpstream: true, + hasOpenPullRequest: false, kind: "archive", }; @@ -56,15 +56,23 @@ const ARCHIVE_1_0: SpecVersion = { 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. + * Branch prefixes that are not *advertised* as spec versions. + * + * These branches still deploy, and `/spec/` still routes to them — a + * preview link shared on a pull request has to keep working. They are simply + * not listed, because the picker answers "which versions of the specification + * are there?", and a dependency bump is not one even while its pull request + * is open. + * + * Applied in `getListedSpecVersions()` only, never in `getSpecVersions()`. + * Filtering during discovery would unroute the deployments as well as hide + * them. */ -const EXCLUDED_BRANCH_PREFIXES = ["dependabot/"]; +const UNADVERTISED_BRANCH_PREFIXES = ["dependabot/"]; function isAdvertisableBranch(branch: string): boolean { if (EXCLUDED_BRANCHES.has(branch)) return false; - return !EXCLUDED_BRANCH_PREFIXES.some((prefix) => branch.startsWith(prefix)); + return !UNADVERTISED_BRANCH_PREFIXES.some((prefix) => branch.startsWith(prefix)); } function branchToSlug(branch: string): string { @@ -118,29 +126,29 @@ async function fetchDeployments(): Promise { } } -type GitHubBranch = { - name?: string; +type GitHubPullRequest = { + head?: { ref?: string }; }; /** - * Every branch that currently exists in the repository, or `null` when the - * lookup was unavailable. + * The branches with an open pull request, 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. + * This is what "a version worth listing" means here: `main` and `develop` are + * shown unconditionally, and every other branch earns its place by having a + * pull request in flight. A branch whose PR has merged stops being listed even + * if nobody deleted it, which is the case that made the picker unusable — it + * was showing seven merged branches alongside one live one. * - * Returns `null` rather than an empty set on failure, so callers can tell - * "no branches" apart from "could not ask" — very different things. + * Returns `null` rather than an empty set on failure, so callers can tell "no + * open pull requests" apart from "could not ask" — very different things. * - * 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. + * Every failure path logs the status and GitHub's own message. The original + * bug (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 fetchExistingBranches(): Promise | null> { +async function fetchOpenPullRequestBranches(): Promise | null> { const token = process.env.GITHUB_TOKEN; if (!token) { @@ -149,7 +157,8 @@ async function fetchExistingBranches(): Promise | null> { // 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.", + "branches have open pull requests. Showing the archive and develop " + + "only.", ); return null; } @@ -165,35 +174,35 @@ async function fetchExistingBranches(): Promise | null> { }; const branches = new Set(); - // The repository has far fewer than 100 branches today, but paginate - // anyway: silently truncating would hide live branches. + // Paginate: silently truncating at 100 would hide live pull requests. 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}`, + "https://api.github.com/repos/EKGF/dprod/pulls" + + `?state=open&per_page=100&page=${page}`, { headers, next: { revalidate: 60 } }, ); } catch (error) { - console.warn("[spec-versions] GitHub branch lookup threw:", error); + console.warn("[spec-versions] GitHub pull request lookup threw:", error); 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). + // (401, token expired or revoked), "API rate limit exceeded" (403), and + // "Resource not accessible" (403, token lacks Pull requests: Read-only). const detail = await res.text().catch(() => ""); console.warn( - `[spec-versions] GitHub branch lookup failed: ${res.status} ` + + `[spec-versions] GitHub pull request 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); + const pulls = (await res.json()) as GitHubPullRequest[]; + for (const pull of pulls) { + if (pull.head?.ref) branches.add(pull.head.ref); } - if (pageBranches.length < 100) break; + if (pulls.length < 100) break; } return branches; } @@ -214,15 +223,18 @@ export async function getSpecVersions(): Promise { const currentBranch = process.env.VERCEL_GIT_COMMIT_REF; const versions: SpecVersion[] = [ARCHIVE_1_0]; - const [deployments, existingBranches] = await Promise.all([ + const [deployments, openPullRequestBranches] = await Promise.all([ fetchDeployments(), - fetchExistingBranches(), + fetchOpenPullRequestBranches(), ]); const seen = new Set(); for (const dep of deployments) { const branch = dep.meta?.githubCommitRef; - if (!branch || !isAdvertisableBranch(branch) || seen.has(branch)) continue; + // Deliberately not filtered by isAdvertisableBranch(): this list is what + // the middleware routes with, and a preview URL already shared must keep + // resolving even when the branch is not advertised. + if (!branch || EXCLUDED_BRANCHES.has(branch) || seen.has(branch)) continue; seen.add(branch); const slug = branchToSlug(branch); @@ -240,7 +252,9 @@ 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, + hasOpenPullRequest: openPullRequestBranches + ? openPullRequestBranches.has(branch) + : null, kind: "vercel-branch", }); } @@ -263,7 +277,7 @@ export async function getSpecVersions(): Promise { export type ListedSpecVersions = { versions: SpecVersion[]; /** - * False when branch existence could not be determined, so the list is the + * False when the pull request lookup failed, 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. */ @@ -271,19 +285,28 @@ export type ListedSpecVersions = { }; /** - * The spec versions to show a reader: the archive, develop, and branches that - * still exist upstream. + * The spec versions to show a reader: + * + * - `main`, the frozen OMG standard, as the archive entry; + * - `develop`, the current working draft; + * - every branch with an open pull request, i.e. work actually in flight. + * + * Anything else — a merged branch nobody deleted, an abandoned experiment, a + * dependency bump — is not a version of the specification and is left out. + * Those previews stay routable through `getSpecVersions()`, so a link shared + * on a pull request keeps working after it merges; it is only the listing that + * is opinionated. * - * 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. + * Fails *closed*. When the lookup fails 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. */ export async function getListedSpecVersions(): Promise { const versions = await getSpecVersions(); const complete = !versions.some( - (version) => version.kind === "vercel-branch" && version.existsUpstream === null, + (version) => + version.kind === "vercel-branch" && version.hasOpenPullRequest === null, ); return { complete, @@ -291,7 +314,8 @@ export async function getListedSpecVersions(): Promise { (version) => version.kind === "archive" || version.isProduction || - version.existsUpstream === true, + (version.hasOpenPullRequest === true && + isAdvertisableBranch(version.branch)), ), }; }