Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions site/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down
25 changes: 14 additions & 11 deletions site/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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/<slug>` 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:

Expand All @@ -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,
Expand Down
126 changes: 75 additions & 51 deletions site/src/lib/spec-versions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -48,23 +48,31 @@ const ARCHIVE_1_0: SpecVersion = {
origin: "",
isCurrent: false,
isProduction: false,
existsUpstream: true,
hasOpenPullRequest: false,
kind: "archive",
};

/** Branches that we never want to advertise as spec versions. */
const EXCLUDED_BRANCHES = new Set<string>(["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/<slug>` 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 {
Expand Down Expand Up @@ -118,29 +126,29 @@ async function fetchDeployments(): Promise<VercelDeployment[]> {
}
}

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<Set<string> | null> {
async function fetchOpenPullRequestBranches(): Promise<Set<string> | null> {
const token = process.env.GITHUB_TOKEN;

if (!token) {
Expand All @@ -149,7 +157,8 @@ async function fetchExistingBranches(): Promise<Set<string> | 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;
}
Expand All @@ -165,35 +174,35 @@ async function fetchExistingBranches(): Promise<Set<string> | null> {
};

const branches = new Set<string>();
// 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;
}
Expand All @@ -214,15 +223,18 @@ export async function getSpecVersions(): Promise<SpecVersion[]> {
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<string>();
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);
Expand All @@ -240,7 +252,9 @@ export async function getSpecVersions(): Promise<SpecVersion[]> {
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",
});
}
Expand All @@ -263,35 +277,45 @@ export async function getSpecVersions(): Promise<SpecVersion[]> {
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.
*/
complete: boolean;
};

/**
* 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<ListedSpecVersions> {
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,
versions: versions.filter(
(version) =>
version.kind === "archive" ||
version.isProduction ||
version.existsUpstream === true,
(version.hasOpenPullRequest === true &&
isAdvertisableBranch(version.branch)),
),
};
}
Loading