From 35f9ebc0f754038037cea1740665fb4777582cb5 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 15:22:21 +0100 Subject: [PATCH 01/17] Add previews/v1 service for preview build lookup Resolves current-main, per-PR, and per-commit FOSSBilling preview builds. GitHub Actions is the source of truth for PR/commit previews (this repo's ci.yml already uploads one unified 'FOSSBilling Preview' artifact per PR build, branch push, and main push); main is answered from an R2 object HEAD instead, since the R2 zip and the GitHub artifact zip are built as two independent byte streams with different digests. - GET /previews/v1/main, /pr/{number}, /commit/{sha} - GET /pr/{number}/download, /commit/{sha}/download (live 302 redirects) - OpenAPI docs at /previews/v1/openapi.json and /previews/v1/docs - New PREVIEW_BUCKET R2 binding (wrangler.jsonc) against the existing 'fossbilling' bucket - not yet verified in the Cloudflare dashboard as the bucket actually bound to download.fossbilling.org - CACHE_KV caches successful lookups for 60s; /download never cached FOSSBilling/FOSSBilling's ci.yml still needs a follow-up change to set sha256/commit-sha as R2 custom metadata on the main preview upload - until then /previews/v1/main reports null digest/commit_sha. --- README.md | 8 +- src/app/index.ts | 2 + src/services/previews/v1/README.md | 114 +++++++++++++ src/services/previews/v1/cache.ts | 33 ++++ src/services/previews/v1/github/artifacts.ts | 171 +++++++++++++++++++ src/services/previews/v1/index.ts | 57 +++++++ src/services/previews/v1/r2.ts | 36 ++++ src/services/previews/v1/resolve.ts | 37 ++++ src/services/previews/v1/routes/app.ts | 5 + src/services/previews/v1/routes/commit.ts | 119 +++++++++++++ src/services/previews/v1/routes/errors.ts | 22 +++ src/services/previews/v1/routes/main.ts | 62 +++++++ src/services/previews/v1/routes/pr.ts | 136 +++++++++++++++ src/services/previews/v1/schemas/previews.ts | 81 +++++++++ test/services/previews/v1/commit.test.ts | 168 ++++++++++++++++++ test/services/previews/v1/main.test.ts | 100 +++++++++++ test/services/previews/v1/pr.test.ts | 140 +++++++++++++++ worker-configuration.d.ts | 9 +- wrangler.jsonc | 12 ++ 19 files changed, 1307 insertions(+), 5 deletions(-) create mode 100644 src/services/previews/v1/README.md create mode 100644 src/services/previews/v1/cache.ts create mode 100644 src/services/previews/v1/github/artifacts.ts create mode 100644 src/services/previews/v1/index.ts create mode 100644 src/services/previews/v1/r2.ts create mode 100644 src/services/previews/v1/resolve.ts create mode 100644 src/services/previews/v1/routes/app.ts create mode 100644 src/services/previews/v1/routes/commit.ts create mode 100644 src/services/previews/v1/routes/errors.ts create mode 100644 src/services/previews/v1/routes/main.ts create mode 100644 src/services/previews/v1/routes/pr.ts create mode 100644 src/services/previews/v1/schemas/previews.ts create mode 100644 test/services/previews/v1/commit.test.ts create mode 100644 test/services/previews/v1/main.test.ts create mode 100644 test/services/previews/v1/pr.test.ts diff --git a/README.md b/README.md index 8096ea6..78b2bc1 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,10 @@ The worker exposes three main services: generated HTTPS API client; it must not bind or migrate `DB_EXTENSIONS`. See [`src/services/extensions/v2/README.md`](src/services/extensions/v2/README.md). +- **Previews** (`/previews/v1`) + Resolves FOSSBilling preview builds — the current main preview and per-PR/per-commit builds produced by FOSSBilling/FOSSBilling's GitHub Actions workflows. Read-only; GitHub Actions and R2 are the sources of truth, not this service. + See [`src/services/previews/v1/README.md`](src/services/previews/v1/README.md). + ## Architecture We've structured the app to separate the core logic from the specific runtime environment (Cloudflare, Node, etc.). @@ -41,8 +45,9 @@ Each service documents its own endpoints and behaviour: | Central Alerts | `/central-alerts/v1` | [`src/services/central-alerts/v1/README.md`](src/services/central-alerts/v1/README.md) | | Stats | `/stats/v1` | [`src/services/stats/v1/README.md`](src/services/stats/v1/README.md) | | Extensions | `/extensions/v1`, `/extensions/v2` | [`src/services/extensions/v2/README.md`](src/services/extensions/v2/README.md) | +| Previews | `/previews/v1` | [`src/services/previews/v1/README.md`](src/services/previews/v1/README.md) | -Extensions v2 also publishes a live OpenAPI document at `/extensions/v2/openapi.json` and a reference UI at `/extensions/v2/docs`. +Extensions v2 and Previews v1 also publish a live OpenAPI document (`/extensions/v2/openapi.json`, `/previews/v1/openapi.json`) and a reference UI (`/extensions/v2/docs`, `/previews/v1/docs`). ## Configuration @@ -57,6 +62,7 @@ We use [Cloudflare D1](https://developers.cloudflare.com/d1/) and [KV](https://d Migrations are owned by extensions v2 and applied only from this repository — see [its README](src/services/extensions/v2/README.md#database) for the migration and adoption procedure. - **KV Namespace** (`CACHE_KV`): Caches GitHub API responses so we don't hit rate limits. - **KV Namespace** (`AUTH_KV`): Stores the `UPDATE_TOKEN` value for `/versions/v1/update`. +- **R2 Bucket** (`PREVIEW_BUCKET`): Backs `/previews/v1/main` — see [`src/services/previews/v1/README.md`](src/services/previews/v1/README.md) and the comment in `wrangler.jsonc` for which bucket this points at and why. ### Environment Variables diff --git a/src/app/index.ts b/src/app/index.ts index 4716414..fd5641f 100644 --- a/src/app/index.ts +++ b/src/app/index.ts @@ -4,6 +4,7 @@ import { HTTPException } from "hono/http-exception"; import centralAlertsV1 from "../services/central-alerts/v1"; import extensionsV1 from "../services/extensions/v1"; import extensionsV2 from "../services/extensions/v2"; +import previewsV1 from "../services/previews/v1"; import versionsV1 from "../services/versions/v1"; import statsV1 from "../services/stats/v1"; import { platformMiddleware } from "../lib/middleware"; @@ -23,6 +24,7 @@ app.use("*", async (c, next) => { app.route("/central-alerts/v1", centralAlertsV1); app.route("/extensions/v1", extensionsV1); app.route("/extensions/v2", extensionsV2); +app.route("/previews/v1", previewsV1); app.route("/versions/v1", versionsV1); app.route("/stats/v1", statsV1); diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md new file mode 100644 index 0000000..718fcb6 --- /dev/null +++ b/src/services/previews/v1/README.md @@ -0,0 +1,114 @@ +# Previews Service + +**Base Path:** `/previews/v1` + +Read-only lookup of FOSSBilling preview builds. GitHub Actions is the source +of truth for PR/commit previews - `FOSSBilling/FOSSBilling`'s `ci.yml` +already uploads a single unified artifact (`FOSSBilling Preview`) for every +PR build, non-main branch push, and main push, and this service resolves +against that artifact list rather than maintaining its own registry. The +`main` preview is answered from R2 instead, because the R2-hosted zip and +the GitHub artifact zip for the same commit are two independently-built byte +streams with different digests - whichever one is reported has to match the +bytes actually served. + +There is no publish/write endpoint: nothing pushes data into this service, +it only resolves and redirects. + +## Resource model + +- `GET /main` and `GET /pr/{number}` are **pointers** - they always resolve + to whatever is current. +- `GET /commit/{sha}` is a **fixed point** - one commit, one build, + permanently addressable (until GitHub's artifact retention expires it). +- `pr/{number}`'s handler resolves the PR to its head SHA + (`GET /pulls/{number}`) and delegates to the same resolver `commit/{sha}` + uses - one GitHub-facing code path, not two. + +## Endpoints + +### GET `/main` + +Current main preview, sourced from an R2 object HEAD (no GitHub API call). + +**Response:** + +```json +{ + "result": { + "commit_sha": "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + "short_sha": "a1b2c3d", + "digest": "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", + "size_bytes": 31229553, + "last_modified": "2026-08-13T13:11:41.000Z", + "download_url": "https://download.fossbilling.org/FOSSBilling-preview.zip", + "source": "r2" + } +} +``` + +`commit_sha` and `digest` are `null` until FOSSBilling/FOSSBilling's +`upload-preview` CI job is updated to set `sha256`/`commit-sha` as R2 custom +object metadata on the upload - see that repo's `ci.yml`. + +### GET `/pr/{number}` and GET `/commit/{sha}` + +Preview build for a pull request's current head, or for one exact commit. +`sha` accepts a full or abbreviated (7+ char) hex SHA. + +**Response:** + +```json +{ + "result": { + "commit_sha": "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + "short_sha": "a1b2c3d", + "pr_number": 123, + "run_id": 999999, + "artifact_id": 555555, + "digest": "sha256:...", + "size_bytes": 12345, + "created_at": "2026-08-13T10:00:00Z", + "expires_at": "2026-08-27T10:00:00Z", + "download_url": "/previews/v1/commit/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/download", + "source": "actions_artifact" + } +} +``` + +`digest` is GitHub's own artifact digest - the exact bytes served by the +`/download` route. `pr_number` is only set when resolved via `/pr/{number}`; +a direct `/commit/{sha}` lookup has no way to know which PR (if any) built +that commit, and reports `null`. + +`download_url` always points at the canonical `/commit/{sha}/download` +route using the fully-resolved SHA, not `/pr/{number}/download` - a PR's +head SHA moves as new commits land, a specific commit's build does not. + +### GET `/pr/{number}/download` and GET `/commit/{sha}/download` + +302 redirect to GitHub's live, short-lived artifact download URL. Resolved +fresh on every request - never served from cache, since GitHub's signed URL +expires in about a minute. + +## Error Responses + +```json +{ "error": { "message": "No pull request #999 was found, or it has no preview build yet.", "code": "NOT_FOUND" } } +``` + +`code` is one of `NOT_FOUND`, `VALIDATION_ERROR` (422, malformed path +param), or GitHub's own `errorCode` (`rate_limit_error`, `auth_error`, +etc., surfaced as 429/503/500 depending on severity). + +## Notes + +- `GET /pr/{number}` and `GET /commit/{sha}` responses are cached in + `CACHE_KV` for 60 seconds (`preview:pr:{number}` / `preview:commit:{sha}`); + `GET /main` for 60 seconds (`preview:main`). Only successful lookups are + cached - a not-yet-built PR or a transient GitHub error always re-resolves + on the next request. +- `GITHUB_TOKEN` is required for GitHub API access (shared with + `versions/v1`). +- `PREVIEW_BUCKET` (R2 binding) backs `/main` - see `wrangler.jsonc` for the + bucket this points at and why. diff --git a/src/services/previews/v1/cache.ts b/src/services/previews/v1/cache.ts new file mode 100644 index 0000000..e2ba679 --- /dev/null +++ b/src/services/previews/v1/cache.ts @@ -0,0 +1,33 @@ +import { GithubLookupResult } from "./github/artifacts"; + +// Previews churn often (a new commit on a PR supersedes the last build +// within minutes), so a short TTL keeps CACHE_KV useful without serving +// meaningfully stale data - matches download-worker's existing choice for +// the same trade-off. +const CACHE_TTL_SECONDS = 60; + +// Only "found" results are cached. "not_found"/"unavailable" always +// re-resolve, so a transient GitHub hiccup or a not-yet-built PR doesn't +// get stuck negative for the TTL window. +export async function cachedLookup( + kv: KVNamespace, + key: string, + resolve: () => Promise> +): Promise> { + const cached = await kv.get(key); + if (cached !== null) { + try { + return { status: "found", data: JSON.parse(cached) as T }; + } catch { + // Corrupt cache entry - fall through to a fresh resolve. + } + } + + const result = await resolve(); + if (result.status === "found") { + await kv.put(key, JSON.stringify(result.data), { + expirationTtl: CACHE_TTL_SECONDS + }); + } + return result; +} diff --git a/src/services/previews/v1/github/artifacts.ts b/src/services/previews/v1/github/artifacts.ts new file mode 100644 index 0000000..efd9ce4 --- /dev/null +++ b/src/services/previews/v1/github/artifacts.ts @@ -0,0 +1,171 @@ +import { request as ghRequest } from "@octokit/request"; +import { + classifyGitHubError, + GitHubError, + NotFoundError +} from "../../../../lib/github-errors"; +import { logWarn } from "../../../../lib/logger"; + +// FOSSBilling/FOSSBilling's ci.yml uploads one unified artifact under this +// name for every PR build, non-main branch push, and main push - see +// upload-preview in that repo's .github/workflows/ci.yml. +const REPO_OWNER = "FOSSBilling"; +const REPO_NAME = "FOSSBilling"; +const PREVIEW_ARTIFACT_NAME = "FOSSBilling Preview"; + +export interface PreviewArtifact { + runId: number; + artifactId: number; + commitSha: string; + digest: string | null; + sizeBytes: number; + createdAt: string; + expiresAt: string; +} + +export type GithubLookupResult = + | { status: "found"; data: T } + | { status: "not_found" } + | { status: "unavailable"; error: GitHubError }; + +interface RawArtifact { + id: number; + size_in_bytes: number; + created_at: string | null; + expires_at: string | null; + expired: boolean; + digest: string | null; + workflow_run?: { + id: number; + head_sha: string; + } | null; +} + +function unavailable( + context: string, + error: unknown, + url: string +): GithubLookupResult { + const githubError = classifyGitHubError(error, url); + logWarn("previews", `${context} unavailable`, { + message: githubError.message, + httpStatus: githubError.httpStatus + }); + return { status: "unavailable", error: githubError }; +} + +// Lists artifacts named `FOSSBilling Preview` (server-side filtered, so the +// result set is bounded to one live artifact per recent successful run +// rather than the repo's entire artifact history) and returns the newest +// non-expired one whose triggering run built the given commit. +export async function findPreviewArtifactByCommitSha( + githubToken: string, + sha: string +): Promise> { + const url = `https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/actions/artifacts`; + try { + const result = await ghRequest( + "GET /repos/{owner}/{repo}/actions/artifacts", + { + owner: REPO_OWNER, + repo: REPO_NAME, + name: PREVIEW_ARTIFACT_NAME, + per_page: 100, + headers: { Authorization: `Bearer ${githubToken}` } + } + ); + + const artifacts = result.data.artifacts as RawArtifact[]; + const shaLower = sha.toLowerCase(); + let match: RawArtifact | null = null; + + for (const artifact of artifacts) { + if (artifact.expired || !artifact.workflow_run) continue; + if (!artifact.workflow_run.head_sha.toLowerCase().startsWith(shaLower)) { + continue; + } + if (!match || (artifact.created_at ?? "") > (match.created_at ?? "")) { + match = artifact; + } + } + + if (!match || !match.workflow_run) { + return { status: "not_found" }; + } + + return { + status: "found", + data: { + runId: match.workflow_run.id, + artifactId: match.id, + commitSha: match.workflow_run.head_sha, + digest: match.digest ?? null, + sizeBytes: match.size_in_bytes, + createdAt: match.created_at ?? "", + expiresAt: match.expires_at ?? "" + } + }; + } catch (error) { + return unavailable("Preview artifact lookup", error, url); + } +} + +// Resolves a PR number to its current head commit SHA. +export async function resolvePullRequestHeadSha( + githubToken: string, + prNumber: number +): Promise> { + const url = `https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/pulls/${prNumber}`; + try { + const result = await ghRequest( + "GET /repos/{owner}/{repo}/pulls/{pull_number}", + { + owner: REPO_OWNER, + repo: REPO_NAME, + pull_number: prNumber, + headers: { Authorization: `Bearer ${githubToken}` } + } + ); + return { status: "found", data: result.data.head.sha }; + } catch (error) { + const githubError = classifyGitHubError(error, url); + if (githubError instanceof NotFoundError) return { status: "not_found" }; + return unavailable("Pull request lookup", error, url); + } +} + +// Resolves an artifact's live, short-lived download URL. Mirrors +// download-worker/src/preview.ts's getArtifactDownloadUrl - GitHub answers +// with a 302 to a signed, temporary URL rather than the file itself. +export async function getArtifactDownloadUrl( + githubToken: string, + artifactId: number +): Promise> { + const url = `https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/actions/artifacts/${artifactId}/zip`; + try { + const result = await ghRequest( + "GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/{archive_format}", + { + owner: REPO_OWNER, + repo: REPO_NAME, + artifact_id: artifactId, + archive_format: "zip", + request: { redirect: "manual" }, + headers: { Authorization: `Bearer ${githubToken}` } + } + ); + + if (result.status === 302 && result.headers.location) { + return { status: "found", data: result.headers.location }; + } + return unavailable( + "Artifact download redirect", + new Error(`Unexpected status ${result.status}`), + url + ); + } catch (error) { + const githubError = classifyGitHubError(error, url); + if (githubError instanceof NotFoundError) return { status: "not_found" }; + return unavailable("Artifact download redirect", error, url); + } +} diff --git a/src/services/previews/v1/index.ts b/src/services/previews/v1/index.ts new file mode 100644 index 0000000..5a4012d --- /dev/null +++ b/src/services/previews/v1/index.ts @@ -0,0 +1,57 @@ +import { OpenAPIHono } from "@hono/zod-openapi"; +import { Scalar } from "@scalar/hono-api-reference"; +import { cors } from "hono/cors"; +import { trimTrailingSlash } from "hono/trailing-slash"; +import { registerMainRoutes } from "./routes/main"; +import { registerPrRoutes } from "./routes/pr"; +import { registerCommitRoutes } from "./routes/commit"; + +const previewsV1 = new OpenAPIHono<{ Bindings: CloudflareBindings }>({ + defaultHook: (result, c) => { + if (!result.success) { + return c.json( + { + error: { + message: "Invalid request", + code: "VALIDATION_ERROR", + details: result.error.issues + } + }, + 422 + ); + } + } +}); + +previewsV1.use("/*", cors({ origin: "*" })); +previewsV1.use("/*", trimTrailingSlash()); + +registerMainRoutes(previewsV1); +registerPrRoutes(previewsV1); +registerCommitRoutes(previewsV1); + +previewsV1.doc31("/openapi.json", { + openapi: "3.1.0", + info: { + title: "FOSSBilling Previews API (v1)", + version: "1.0.0", + description: + "Read-only lookup of FOSSBilling preview builds - the current main preview and per-PR/per-commit builds produced by FOSSBilling/FOSSBilling's GitHub Actions workflows." + }, + servers: [{ url: "/previews/v1" }] +}); + +previewsV1.get( + "/docs", + Scalar({ + url: "/previews/v1/openapi.json", + pageTitle: "FOSSBilling Previews API (v1)", + agent: { disabled: true }, + documentDownloadType: "none", + hideClientButton: true, + hideModels: true, + telemetry: false + }) +); + +export default previewsV1; diff --git a/src/services/previews/v1/r2.ts b/src/services/previews/v1/r2.ts new file mode 100644 index 0000000..ec1e44a --- /dev/null +++ b/src/services/previews/v1/r2.ts @@ -0,0 +1,36 @@ +// The object FOSSBilling/FOSSBilling's ci.yml `upload-preview` job syncs to +// R2 on every main push - the same path served publicly at +// https://download.fossbilling.org/FOSSBilling-preview.zip. +const MAIN_PREVIEW_KEY = "FOSSBilling-preview.zip"; +const MAIN_PREVIEW_DOWNLOAD_URL = + "https://download.fossbilling.org/FOSSBilling-preview.zip"; + +export interface MainPreviewObject { + commitSha: string | null; + digest: string | null; + sizeBytes: number; + lastModified: string; + downloadUrl: string; +} + +// R2's zip and GitHub's own `FOSSBilling Preview` artifact zip for the same +// commit are two independently-built byte streams (see previews/v1's +// README), so the digest reported here has to come from this object's own +// metadata, not GitHub's. `sha256`/`commit-sha` are custom metadata the CI +// upload step sets explicitly - both are absent (null) until that step is +// added to FOSSBilling/FOSSBilling's ci.yml. +export async function getMainPreviewObject( + bucket: R2Bucket +): Promise { + const object = await bucket.head(MAIN_PREVIEW_KEY); + if (!object) return null; + + const digest = object.customMetadata?.sha256; + return { + commitSha: object.customMetadata?.["commit-sha"] ?? null, + digest: digest ? `sha256:${digest}` : null, + sizeBytes: object.size, + lastModified: object.uploaded.toISOString(), + downloadUrl: MAIN_PREVIEW_DOWNLOAD_URL + }; +} diff --git a/src/services/previews/v1/resolve.ts b/src/services/previews/v1/resolve.ts new file mode 100644 index 0000000..1f74f56 --- /dev/null +++ b/src/services/previews/v1/resolve.ts @@ -0,0 +1,37 @@ +import { ArtifactPreview } from "./schemas/previews"; +import { + findPreviewArtifactByCommitSha, + GithubLookupResult +} from "./github/artifacts"; + +export type PreviewLookupResult = GithubLookupResult; + +// download_url always points at the canonical /commit/{full_sha}/download +// route, using the fully-resolved SHA rather than whatever prefix or PR +// number the caller looked it up by. A PR's head SHA moves as new commits +// land; a specific commit's build does not, so that's the one stable link +// to hand back regardless of which route resolved it. +export async function resolveArtifactPreview( + githubToken: string, + sha: string, + prNumber: number | null +): Promise { + const found = await findPreviewArtifactByCommitSha(githubToken, sha); + if (found.status !== "found") return found; + + const { data } = found; + const preview: ArtifactPreview = { + commit_sha: data.commitSha, + short_sha: data.commitSha.slice(0, 7), + pr_number: prNumber, + run_id: data.runId, + artifact_id: data.artifactId, + digest: data.digest, + size_bytes: data.sizeBytes, + created_at: data.createdAt, + expires_at: data.expiresAt, + download_url: `/previews/v1/commit/${data.commitSha}/download`, + source: "actions_artifact" + }; + return { status: "found", data: preview }; +} diff --git a/src/services/previews/v1/routes/app.ts b/src/services/previews/v1/routes/app.ts new file mode 100644 index 0000000..0a77e42 --- /dev/null +++ b/src/services/previews/v1/routes/app.ts @@ -0,0 +1,5 @@ +import { OpenAPIHono } from "@hono/zod-openapi"; + +export type PreviewsV1App = OpenAPIHono<{ + Bindings: CloudflareBindings; +}>; diff --git a/src/services/previews/v1/routes/commit.ts b/src/services/previews/v1/routes/commit.ts new file mode 100644 index 0000000..5c717ac --- /dev/null +++ b/src/services/previews/v1/routes/commit.ts @@ -0,0 +1,119 @@ +import { createRoute } from "@hono/zod-openapi"; +import { + ArtifactPreviewResponseSchema, + CommitShaParamSchema, + errorResponse +} from "../schemas/previews"; +import { getArtifactDownloadUrl } from "../github/artifacts"; +import { resolveArtifactPreview } from "../resolve"; +import { cachedLookup } from "../cache"; +import { githubErrorBody, notFoundBody, statusFromGithubError } from "./errors"; +import { PreviewsV1App } from "./app"; + +export function registerCommitRoutes(app: PreviewsV1App): void { + const commitRoute = createRoute({ + method: "get", + path: "/commit/{sha}", + tags: ["Previews"], + summary: "Preview build for a specific commit", + request: { params: CommitShaParamSchema }, + responses: { + 200: { + content: { + "application/json": { schema: ArtifactPreviewResponseSchema } + }, + description: "The preview build for that commit" + }, + 404: errorResponse("No preview artifact exists for that commit"), + 422: errorResponse("sha param failed validation"), + 429: errorResponse("GitHub API rate limit exceeded"), + 500: errorResponse("Unexpected error"), + 503: errorResponse("GitHub is temporarily unavailable") + } + }); + + app.openapi(commitRoute, async (c) => { + const { sha } = c.req.valid("param"); + const githubToken = c.env.GITHUB_TOKEN; + + const result = await cachedLookup( + c.env.CACHE_KV, + `preview:commit:${sha.toLowerCase()}`, + () => resolveArtifactPreview(githubToken, sha, null) + ); + + if (result.status === "found") { + return c.json({ result: result.data }, 200); + } + if (result.status === "not_found") { + return c.json( + notFoundBody(`No preview artifact exists for commit ${sha}.`), + 404 + ); + } + return c.json( + githubErrorBody(result.error, "Failed to look up the preview artifact"), + statusFromGithubError(result.error) + ); + }); + + const commitDownloadRoute = createRoute({ + method: "get", + path: "/commit/{sha}/download", + tags: ["Previews"], + summary: "Download the preview build for a specific commit", + request: { params: CommitShaParamSchema }, + responses: { + 302: { description: "Redirect to GitHub's live artifact download URL" }, + 404: errorResponse("No preview artifact exists for that commit"), + 422: errorResponse("sha param failed validation"), + 429: errorResponse("GitHub API rate limit exceeded"), + 500: errorResponse("Unexpected error"), + 503: errorResponse("GitHub is temporarily unavailable") + } + }); + + app.openapi(commitDownloadRoute, async (c) => { + const { sha } = c.req.valid("param"); + const githubToken = c.env.GITHUB_TOKEN; + + // Always resolved live - GitHub's artifact download URL is a signed + // link that expires in about a minute, so it can never be served from + // CACHE_KV alongside the longer-lived metadata response. + const artifact = await resolveArtifactPreview(githubToken, sha, null); + if (artifact.status === "not_found") { + return c.json( + notFoundBody(`No preview artifact exists for commit ${sha}.`), + 404 + ); + } + if (artifact.status === "unavailable") { + return c.json( + githubErrorBody( + artifact.error, + "Failed to look up the preview artifact" + ), + statusFromGithubError(artifact.error) + ); + } + + const redirect = await getArtifactDownloadUrl( + githubToken, + artifact.data.artifact_id + ); + if (redirect.status === "not_found") { + return c.json(notFoundBody("The preview artifact has expired."), 404); + } + if (redirect.status === "unavailable") { + return c.json( + githubErrorBody( + redirect.error, + "Failed to resolve the artifact download URL" + ), + statusFromGithubError(redirect.error) + ); + } + + return c.redirect(redirect.data, 302); + }); +} diff --git a/src/services/previews/v1/routes/errors.ts b/src/services/previews/v1/routes/errors.ts new file mode 100644 index 0000000..9e9c5a0 --- /dev/null +++ b/src/services/previews/v1/routes/errors.ts @@ -0,0 +1,22 @@ +import { GitHubError, RateLimitError } from "../../../../lib/github-errors"; + +// A GitHub outage/rate-limit is a 503 (retry later); anything else +// unexpected from classifyGitHubError is a 500. +export function statusFromGithubError(error: GitHubError): 429 | 503 | 500 { + if (error instanceof RateLimitError) return 429; + if (error.httpStatus !== undefined && error.httpStatus >= 500) return 503; + return 500; +} + +export function githubErrorBody(error: GitHubError, fallbackMessage: string) { + return { + error: { + message: error.message || fallbackMessage, + code: error.errorCode ?? "GITHUB_ERROR" + } + }; +} + +export function notFoundBody(message: string) { + return { error: { message, code: "NOT_FOUND" } }; +} diff --git a/src/services/previews/v1/routes/main.ts b/src/services/previews/v1/routes/main.ts new file mode 100644 index 0000000..b1f23b9 --- /dev/null +++ b/src/services/previews/v1/routes/main.ts @@ -0,0 +1,62 @@ +import { createRoute } from "@hono/zod-openapi"; +import { MainPreviewResponseSchema, errorResponse } from "../schemas/previews"; +import { getMainPreviewObject } from "../r2"; +import { PreviewsV1App } from "./app"; + +const MAIN_CACHE_KEY = "preview:main"; +const MAIN_CACHE_TTL_SECONDS = 60; + +export function registerMainRoutes(app: PreviewsV1App): void { + const mainRoute = createRoute({ + method: "get", + path: "/main", + tags: ["Previews"], + summary: "Current main preview", + responses: { + 200: { + content: { + "application/json": { schema: MainPreviewResponseSchema } + }, + description: "The current main preview build" + }, + 404: errorResponse("No main preview has been published yet"), + 500: errorResponse("R2 lookup failed") + } + }); + + app.openapi(mainRoute, async (c) => { + const cached = await c.env.CACHE_KV.get(MAIN_CACHE_KEY); + if (cached) { + return c.json({ result: JSON.parse(cached) }, 200); + } + + const object = await getMainPreviewObject(c.env.PREVIEW_BUCKET); + if (!object) { + return c.json( + { + error: { + message: "No main preview has been published yet", + code: "NOT_FOUND" + } + }, + 404 + ); + } + + const result = { + commit_sha: object.commitSha, + short_sha: object.commitSha?.slice(0, 7) ?? null, + digest: object.digest, + size_bytes: object.sizeBytes, + last_modified: object.lastModified, + download_url: object.downloadUrl, + source: "r2" as const + }; + + await c.env.CACHE_KV.put(MAIN_CACHE_KEY, JSON.stringify(result), { + expirationTtl: MAIN_CACHE_TTL_SECONDS + }); + + return c.json({ result }, 200); + }); +} diff --git a/src/services/previews/v1/routes/pr.ts b/src/services/previews/v1/routes/pr.ts new file mode 100644 index 0000000..0b76073 --- /dev/null +++ b/src/services/previews/v1/routes/pr.ts @@ -0,0 +1,136 @@ +import { createRoute } from "@hono/zod-openapi"; +import { + ArtifactPreviewResponseSchema, + errorResponse, + PrNumberParamSchema +} from "../schemas/previews"; +import { + getArtifactDownloadUrl, + resolvePullRequestHeadSha +} from "../github/artifacts"; +import { PreviewLookupResult, resolveArtifactPreview } from "../resolve"; +import { cachedLookup } from "../cache"; +import { githubErrorBody, notFoundBody, statusFromGithubError } from "./errors"; +import { PreviewsV1App } from "./app"; + +// Resolves a PR number to its artifact preview by first finding the head +// SHA, then delegating to the same commit-based resolver /commit/{sha} +// uses - one GitHub-facing code path handles both routes. +async function resolvePrPreview( + githubToken: string, + prNumber: number +): Promise { + const head = await resolvePullRequestHeadSha(githubToken, prNumber); + if (head.status !== "found") return head; + return resolveArtifactPreview(githubToken, head.data, prNumber); +} + +export function registerPrRoutes(app: PreviewsV1App): void { + const prRoute = createRoute({ + method: "get", + path: "/pr/{number}", + tags: ["Previews"], + summary: "Current preview build for a pull request", + request: { params: PrNumberParamSchema }, + responses: { + 200: { + content: { + "application/json": { schema: ArtifactPreviewResponseSchema } + }, + description: "The current preview build for that pull request" + }, + 404: errorResponse("No such pull request, or it has no preview build"), + 422: errorResponse("number param failed validation"), + 429: errorResponse("GitHub API rate limit exceeded"), + 500: errorResponse("Unexpected error"), + 503: errorResponse("GitHub is temporarily unavailable") + } + }); + + app.openapi(prRoute, async (c) => { + const { number } = c.req.valid("param"); + const githubToken = c.env.GITHUB_TOKEN; + + const result = await cachedLookup( + c.env.CACHE_KV, + `preview:pr:${number}`, + () => resolvePrPreview(githubToken, number) + ); + + if (result.status === "found") { + return c.json({ result: result.data }, 200); + } + if (result.status === "not_found") { + return c.json( + notFoundBody( + `No pull request #${number} was found, or it has no preview build yet.` + ), + 404 + ); + } + return c.json( + githubErrorBody(result.error, "Failed to look up the preview artifact"), + statusFromGithubError(result.error) + ); + }); + + const prDownloadRoute = createRoute({ + method: "get", + path: "/pr/{number}/download", + tags: ["Previews"], + summary: "Download the current preview build for a pull request", + request: { params: PrNumberParamSchema }, + responses: { + 302: { description: "Redirect to GitHub's live artifact download URL" }, + 404: errorResponse("No such pull request, or it has no preview build"), + 422: errorResponse("number param failed validation"), + 429: errorResponse("GitHub API rate limit exceeded"), + 500: errorResponse("Unexpected error"), + 503: errorResponse("GitHub is temporarily unavailable") + } + }); + + app.openapi(prDownloadRoute, async (c) => { + const { number } = c.req.valid("param"); + const githubToken = c.env.GITHUB_TOKEN; + + // Always resolved live, same reasoning as /commit/{sha}/download. + const artifact = await resolvePrPreview(githubToken, number); + if (artifact.status === "not_found") { + return c.json( + notFoundBody( + `No pull request #${number} was found, or it has no preview build yet.` + ), + 404 + ); + } + if (artifact.status === "unavailable") { + return c.json( + githubErrorBody( + artifact.error, + "Failed to look up the preview artifact" + ), + statusFromGithubError(artifact.error) + ); + } + + const redirect = await getArtifactDownloadUrl( + githubToken, + artifact.data.artifact_id + ); + if (redirect.status === "not_found") { + return c.json(notFoundBody("The preview artifact has expired."), 404); + } + if (redirect.status === "unavailable") { + return c.json( + githubErrorBody( + redirect.error, + "Failed to resolve the artifact download URL" + ), + statusFromGithubError(redirect.error) + ); + } + + return c.redirect(redirect.data, 302); + }); +} diff --git a/src/services/previews/v1/schemas/previews.ts b/src/services/previews/v1/schemas/previews.ts new file mode 100644 index 0000000..492353b --- /dev/null +++ b/src/services/previews/v1/schemas/previews.ts @@ -0,0 +1,81 @@ +import { z } from "@hono/zod-openapi"; + +export const ErrorResponseSchema = z + .object({ + error: z.object({ + message: z.string(), + code: z.string() + }) + }) + .openapi("Error"); + +// Every non-2xx response in this service carries ErrorResponseSchema and +// differs only by description, mirroring extensions/v2's schemas/common.ts. +export const errorResponse = (description: string) => + ({ + content: { "application/json": { schema: ErrorResponseSchema } }, + description + }) as const; + +export const PrNumberParamSchema = z.object({ + number: z.coerce + .number() + .int() + .positive() + .openapi({ + param: { name: "number", in: "path" }, + example: 123 + }) +}); + +// Full or abbreviated (7+ char) hex commit SHA - GitHub accepts either as a +// git ref, and workflow_run.head_sha in the artifacts API is always the full +// 40-char form, so a short SHA here is matched as a prefix by the resolver. +export const CommitShaParamSchema = z.object({ + sha: z + .string() + .regex(/^[0-9a-f]{7,40}$/i, { message: "must be a hex commit SHA" }) + .openapi({ + param: { name: "sha", in: "path" }, + example: "a1b2c3d" + }) +}); + +const MainPreviewSchema = z + .object({ + commit_sha: z.string().nullable(), + short_sha: z.string().nullable(), + digest: z.string().nullable(), + size_bytes: z.number(), + last_modified: z.string(), + download_url: z.string(), + source: z.literal("r2") + }) + .openapi("MainPreview"); + +export const MainPreviewResponseSchema = z + .object({ result: MainPreviewSchema }) + .openapi("MainPreviewResponse"); + +const ArtifactPreviewSchema = z + .object({ + commit_sha: z.string(), + short_sha: z.string(), + pr_number: z.number().nullable(), + run_id: z.number(), + artifact_id: z.number(), + digest: z.string().nullable(), + size_bytes: z.number(), + created_at: z.string(), + expires_at: z.string(), + download_url: z.string(), + source: z.literal("actions_artifact") + }) + .openapi("ArtifactPreview"); + +export const ArtifactPreviewResponseSchema = z + .object({ result: ArtifactPreviewSchema }) + .openapi("ArtifactPreviewResponse"); + +export type MainPreview = z.infer; +export type ArtifactPreview = z.infer; diff --git a/test/services/previews/v1/commit.test.ts b/test/services/previews/v1/commit.test.ts new file mode 100644 index 0000000..19e1544 --- /dev/null +++ b/test/services/previews/v1/commit.test.ts @@ -0,0 +1,168 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { + createExecutionContext, + waitOnExecutionContext +} from "cloudflare:test"; +import { env } from "cloudflare:workers"; +import app from "../../../../src/app"; +import { MockGitHubRequest } from "../../../utils/test-types"; +import { suppressConsole } from "../../../utils/mock-helpers"; + +vi.mock("@octokit/request", async () => + (await import("../../../mocks/octokit")).octokitRequestMock() +); + +import { request as ghRequest } from "@octokit/request"; + +const SHA = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + +const SAMPLE_ARTIFACTS = { + total_count: 1, + artifacts: [ + { + id: 555, + size_in_bytes: 12345, + created_at: "2026-08-13T10:00:00Z", + expires_at: "2026-08-27T10:00:00Z", + expired: false, + digest: "sha256:deadbeef", + workflow_run: { id: 999, head_sha: SHA } + } + ] +}; + +async function get(path: string) { + const ctx = createExecutionContext(); + const res = await app.request(path, {}, env, ctx); + await waitOnExecutionContext(ctx); + return res; +} + +let restoreConsole: (() => void) | null = null; + +describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { + beforeEach(async () => { + restoreConsole = suppressConsole(); + await env.CACHE_KV.delete(`preview:commit:${SHA.toLowerCase()}`); + vi.clearAllMocks(); + }); + + afterEach(() => { + restoreConsole?.(); + restoreConsole = null; + }); + + it("returns the matching artifact's metadata", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async (route: string) => { + if (route === "GET /repos/{owner}/{repo}/actions/artifacts") { + return { data: SAMPLE_ARTIFACTS }; + } + throw new Error(`Unexpected route: ${route}`); + } + ); + + const res = await get(`/previews/v1/commit/${SHA}`); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { + commit_sha: string; + short_sha: string; + pr_number: number | null; + run_id: number; + artifact_id: number; + digest: string | null; + download_url: string; + source: string; + }; + }; + expect(body.result.commit_sha).toBe(SHA); + expect(body.result.short_sha).toBe(SHA.slice(0, 7)); + expect(body.result.pr_number).toBeNull(); + expect(body.result.run_id).toBe(999); + expect(body.result.artifact_id).toBe(555); + expect(body.result.digest).toBe("sha256:deadbeef"); + expect(body.result.download_url).toBe( + `/previews/v1/commit/${SHA}/download` + ); + expect(body.result.source).toBe("actions_artifact"); + }); + + it("matches on a short SHA prefix", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ data: SAMPLE_ARTIFACTS }) + ); + + const res = await get(`/previews/v1/commit/${SHA.slice(0, 7)}`); + expect(res.status).toBe(200); + const body = (await res.json()) as { result: { commit_sha: string } }; + expect(body.result.commit_sha).toBe(SHA); + }); + + it("ignores expired artifacts", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ + data: { + total_count: 1, + artifacts: [{ ...SAMPLE_ARTIFACTS.artifacts[0], expired: true }] + } + }) + ); + + const res = await get(`/previews/v1/commit/${SHA}`); + expect(res.status).toBe(404); + }); + + it("404s when no artifact matches the commit", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ data: { total_count: 0, artifacts: [] } }) + ); + + const res = await get(`/previews/v1/commit/${SHA}`); + expect(res.status).toBe(404); + const body = (await res.json()) as { error: { code: string } }; + expect(body.error.code).toBe("NOT_FOUND"); + }); + + it("422s on a malformed sha", async () => { + const res = await get("/previews/v1/commit/not-a-sha"); + expect(res.status).toBe(422); + }); + + it("returns 503 when GitHub is unavailable", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation(async () => { + throw Object.assign(new Error("Service Unavailable"), { + status: 502 + }); + }); + + const res = await get(`/previews/v1/commit/${SHA}`); + expect(res.status).toBe(503); + }); + + it("follows the redirect for /commit/:sha/download", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async (route: string) => { + if (route === "GET /repos/{owner}/{repo}/actions/artifacts") { + return { data: SAMPLE_ARTIFACTS }; + } + if ( + route === + "GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/{archive_format}" + ) { + return { + status: 302, + headers: { location: "https://example.com/signed-download" } + }; + } + throw new Error(`Unexpected route: ${route}`); + } + ); + + const res = await get(`/previews/v1/commit/${SHA}/download`); + expect(res.status).toBe(302); + expect(res.headers.get("location")).toBe( + "https://example.com/signed-download" + ); + }); +}); diff --git a/test/services/previews/v1/main.test.ts b/test/services/previews/v1/main.test.ts new file mode 100644 index 0000000..660a027 --- /dev/null +++ b/test/services/previews/v1/main.test.ts @@ -0,0 +1,100 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { + createExecutionContext, + waitOnExecutionContext +} from "cloudflare:test"; +import { env } from "cloudflare:workers"; +import app from "../../../../src/app"; + +const MAIN_PREVIEW_KEY = "FOSSBilling-preview.zip"; + +async function get(path: string) { + const ctx = createExecutionContext(); + const res = await app.request(path, {}, env, ctx); + await waitOnExecutionContext(ctx); + return res; +} + +describe("Previews API v1 - GET /previews/v1/main", () => { + beforeEach(async () => { + await env.CACHE_KV.delete("preview:main"); + await env.PREVIEW_BUCKET.delete(MAIN_PREVIEW_KEY); + }); + + it("returns 404 when no main preview has been published", async () => { + const res = await get("/previews/v1/main"); + expect(res.status).toBe(404); + const body = (await res.json()) as { error: { code: string } }; + expect(body.error.code).toBe("NOT_FOUND"); + }); + + it("returns the R2 object's metadata, including the sha256 digest", async () => { + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + customMetadata: { + sha256: + "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", + "commit-sha": "abc1234567890abc1234567890abc1234567890" + } + }); + + const res = await get("/previews/v1/main"); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { + commit_sha: string | null; + short_sha: string | null; + digest: string | null; + size_bytes: number; + download_url: string; + source: string; + }; + }; + + expect(body.result.commit_sha).toBe( + "abc1234567890abc1234567890abc1234567890" + ); + expect(body.result.short_sha).toBe("abc1234"); + expect(body.result.digest).toBe( + "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08" + ); + expect(body.result.size_bytes).toBe("test archive contents".length); + expect(body.result.download_url).toBe( + "https://download.fossbilling.org/FOSSBilling-preview.zip" + ); + expect(body.result.source).toBe("r2"); + }); + + it("reports a null digest and commit_sha when the object has no custom metadata", async () => { + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents"); + + const res = await get("/previews/v1/main"); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { commit_sha: string | null; digest: string | null }; + }; + expect(body.result.commit_sha).toBeNull(); + expect(body.result.digest).toBeNull(); + }); + + it("serves the second request from CACHE_KV without re-reading R2", async () => { + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "v1", { + customMetadata: { "commit-sha": "111" } + }); + const first = await get("/previews/v1/main"); + expect( + ((await first.json()) as { result: { commit_sha: string } }).result + .commit_sha + ).toBe("111"); + + // Overwrite the R2 object directly - a cache hit should still serve the + // first response's data rather than reflecting this change. + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "v2", { + customMetadata: { "commit-sha": "222" } + }); + const second = await get("/previews/v1/main"); + const secondBody = (await second.json()) as { + result: { commit_sha: string }; + }; + expect(secondBody.result.commit_sha).toBe("111"); + }); +}); diff --git a/test/services/previews/v1/pr.test.ts b/test/services/previews/v1/pr.test.ts new file mode 100644 index 0000000..aa0e71a --- /dev/null +++ b/test/services/previews/v1/pr.test.ts @@ -0,0 +1,140 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { + createExecutionContext, + waitOnExecutionContext +} from "cloudflare:test"; +import { env } from "cloudflare:workers"; +import app from "../../../../src/app"; +import { MockGitHubRequest } from "../../../utils/test-types"; +import { suppressConsole } from "../../../utils/mock-helpers"; + +vi.mock("@octokit/request", async () => + (await import("../../../mocks/octokit")).octokitRequestMock() +); + +import { request as ghRequest } from "@octokit/request"; + +const PR_NUMBER = 123; +const SHA = "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"; + +const SAMPLE_ARTIFACTS = { + total_count: 1, + artifacts: [ + { + id: 555, + size_in_bytes: 12345, + created_at: "2026-08-13T10:00:00Z", + expires_at: "2026-08-27T10:00:00Z", + expired: false, + digest: "sha256:deadbeef", + workflow_run: { id: 999, head_sha: SHA } + } + ] +}; + +function mockGithub(routes: Record) { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async (route: string) => { + if (route in routes) return routes[route]; + throw new Error(`Unexpected route: ${route}`); + } + ); +} + +async function get(path: string) { + const ctx = createExecutionContext(); + const res = await app.request(path, {}, env, ctx); + await waitOnExecutionContext(ctx); + return res; +} + +let restoreConsole: (() => void) | null = null; + +describe("Previews API v1 - GET /previews/v1/pr/:number", () => { + beforeEach(async () => { + restoreConsole = suppressConsole(); + await env.CACHE_KV.delete(`preview:pr:${PR_NUMBER}`); + vi.clearAllMocks(); + }); + + afterEach(() => { + restoreConsole?.(); + restoreConsole = null; + }); + + it("resolves the PR to its head SHA, then to that commit's artifact", async () => { + mockGithub({ + "GET /repos/{owner}/{repo}/pulls/{pull_number}": { + data: { head: { sha: SHA } } + }, + "GET /repos/{owner}/{repo}/actions/artifacts": { data: SAMPLE_ARTIFACTS } + }); + + const res = await get(`/previews/v1/pr/${PR_NUMBER}`); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { + commit_sha: string; + pr_number: number | null; + download_url: string; + }; + }; + expect(body.result.commit_sha).toBe(SHA); + expect(body.result.pr_number).toBe(PR_NUMBER); + // Always canonicalized to the fixed /commit/{sha} resource, not + // /pr/{number} - see resolve.ts. + expect(body.result.download_url).toBe( + `/previews/v1/commit/${SHA}/download` + ); + }); + + it("404s when the pull request does not exist", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation(async () => { + throw Object.assign(new Error("Not Found"), { status: 404 }); + }); + + const res = await get(`/previews/v1/pr/${PR_NUMBER}`); + expect(res.status).toBe(404); + }); + + it("404s when the PR exists but has no preview artifact yet", async () => { + mockGithub({ + "GET /repos/{owner}/{repo}/pulls/{pull_number}": { + data: { head: { sha: SHA } } + }, + "GET /repos/{owner}/{repo}/actions/artifacts": { + data: { total_count: 0, artifacts: [] } + } + }); + + const res = await get(`/previews/v1/pr/${PR_NUMBER}`); + expect(res.status).toBe(404); + }); + + it("422s on a non-numeric PR number", async () => { + const res = await get("/previews/v1/pr/not-a-number"); + expect(res.status).toBe(422); + }); + + it("follows the redirect for /pr/:number/download", async () => { + mockGithub({ + "GET /repos/{owner}/{repo}/pulls/{pull_number}": { + data: { head: { sha: SHA } } + }, + "GET /repos/{owner}/{repo}/actions/artifacts": { + data: SAMPLE_ARTIFACTS + }, + "GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/{archive_format}": + { + status: 302, + headers: { location: "https://example.com/signed-download" } + } + }); + + const res = await get(`/previews/v1/pr/${PR_NUMBER}/download`); + expect(res.status).toBe(302); + expect(res.headers.get("location")).toBe( + "https://example.com/signed-download" + ); + }); +}); diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index f161eef..4df02c1 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,9 +1,10 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --env-interface=CloudflareBindings` (hash: 8a84d14f01f95073fe2873a2e9b82b24) -// Runtime types generated with workerd@1.20260730.1 2026-06-24 nodejs_compat +// Generated by Wrangler by running `wrangler types --env-interface=CloudflareBindings` (hash: 85d5d01372efbcd1de1d834068043962) +// Runtime types generated with workerd@1.20260801.1 2026-06-24 nodejs_compat interface __BaseEnv_CloudflareBindings { AUTH_KV: KVNamespace; CACHE_KV: KVNamespace; + PREVIEW_BUCKET: R2Bucket; DB_CENTRAL_ALERTS: D1Database; DB_EXTENSIONS: D1Database; PROFILE_CREATION_RATE_LIMITER: RateLimit; @@ -10342,7 +10343,7 @@ type AIGatewayHeaders = { [key: string]: string | number | boolean | object; }; type AIGatewayUniversalRequest = { - provider: AIGatewayProviders | string; + provider: AIGatewayProviders | string; // eslint-disable-line endpoint: string; headers: Partial; query: unknown; @@ -10359,7 +10360,7 @@ declare abstract class AiGateway { extraHeaders?: object; signal?: AbortSignal; }): Promise; - getUrl(provider?: AIGatewayProviders | string): Promise; + getUrl(provider?: AIGatewayProviders | string): Promise; // eslint-disable-line } // Copyright (c) 2022-2025 Cloudflare, Inc. // Licensed under the Apache 2.0 license found in the LICENSE file or at: diff --git a/wrangler.jsonc b/wrangler.jsonc index 86c277f..e3905e0 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -47,6 +47,18 @@ "id": "0771957093ae481b9cd974ffa83f8263" } ], + // Backs previews/v1's `main` route. Pre-existing bucket (created + // 2024-09-27) that FOSSBilling/FOSSBilling's ci.yml already syncs the + // main-branch preview zip to, served publicly at + // download.fossbilling.org - not previously bound to any Worker. + // Double-check in the Cloudflare dashboard that this is in fact the + // bucket behind that custom domain before relying on it in production. + "r2_buckets": [ + { + "binding": "PREVIEW_BUCKET", + "bucket_name": "fossbilling" + } + ], "ratelimits": [ { "name": "PROFILE_CREATION_RATE_LIMITER", From 51a38d266214a5559039449fcc347200e6636259 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 17:49:49 +0100 Subject: [PATCH 02/17] Match previews/v1 resolver to the real artifact naming/metadata FOSSBilling/FOSSBilling's ci.yml (PR #4157) shipped differently than the initial placeholder assumed: - GitHub Actions artifacts are now named per-commit (FOSSBilling-preview-{short_sha}.zip, archive: false) rather than sharing one literal 'FOSSBilling Preview' name across every run. Switched the resolver to query the exact expected name instead of listing everything under a shared name and filtering by workflow_run.head_sha - fewer results back, and the head_sha check is now just a short-SHA-collision guard rather than the primary match. - R2's custom metadata key is 'digest' (already sha256:-prefixed), not 'sha256' - r2.ts was reading the wrong key and would have double-prefixed the value. Verified against the live repo: fetched real artifacts (confirming the name pattern and that GitHub's reported digest is populated even under archive: false), then downloaded one via the same redirect this service uses and confirmed the bytes hash to exactly the digest GitHub reports - archive: false does not get double-zipped by the /actions/artifacts/{id}/zip endpoint, so the existing download-redirect logic needed no change. --- src/services/previews/v1/README.md | 32 +++++++++++++------- src/services/previews/v1/github/artifacts.ts | 25 +++++++++------ src/services/previews/v1/r2.ts | 12 +++----- test/services/previews/v1/commit.test.ts | 10 ++++++ test/services/previews/v1/main.test.ts | 4 +-- 5 files changed, 53 insertions(+), 30 deletions(-) diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md index 718fcb6..b0dfdb8 100644 --- a/src/services/previews/v1/README.md +++ b/src/services/previews/v1/README.md @@ -4,13 +4,17 @@ Read-only lookup of FOSSBilling preview builds. GitHub Actions is the source of truth for PR/commit previews - `FOSSBilling/FOSSBilling`'s `ci.yml` -already uploads a single unified artifact (`FOSSBilling Preview`) for every -PR build, non-main branch push, and main push, and this service resolves -against that artifact list rather than maintaining its own registry. The -`main` preview is answered from R2 instead, because the R2-hosted zip and -the GitHub artifact zip for the same commit are two independently-built byte -streams with different digests - whichever one is reported has to match the -bytes actually served. +uploads one artifact per commit, named `FOSSBilling-preview-{short_sha}.zip` +(`archive: false`, so the zip itself is the artifact - no extra wrapping), +for every PR build, non-main branch push, and main push. This service +resolves by querying that exact name rather than listing every preview +artifact and filtering. The `main` preview is answered from R2 instead, +sourced from `digest`/`commit-sha` custom object metadata the same CI job +sets on the R2 upload - kept separate from the GitHub-artifact path because +the R2 zip and the GitHub artifact zip for a given commit are two +independently-built files (a `cp` of the same bytes, in the current CI job, +but not guaranteed to stay that way), so whichever one is reported as the +digest has to match the bytes `main` actually serves. There is no publish/write endpoint: nothing pushes data into this service, it only resolves and redirects. @@ -47,9 +51,10 @@ Current main preview, sourced from an R2 object HEAD (no GitHub API call). } ``` -`commit_sha` and `digest` are `null` until FOSSBilling/FOSSBilling's -`upload-preview` CI job is updated to set `sha256`/`commit-sha` as R2 custom -object metadata on the upload - see that repo's `ci.yml`. +`commit_sha` and `digest` come straight from the R2 object's `commit-sha`/ +`digest` custom metadata (`digest` already carries the `sha256:` prefix) - +both are `null` if that object has no custom metadata (e.g. it predates the +CI job setting it). ### GET `/pr/{number}` and GET `/commit/{sha}` @@ -94,7 +99,12 @@ expires in about a minute. ## Error Responses ```json -{ "error": { "message": "No pull request #999 was found, or it has no preview build yet.", "code": "NOT_FOUND" } } +{ + "error": { + "message": "No pull request #999 was found, or it has no preview build yet.", + "code": "NOT_FOUND" + } +} ``` `code` is one of `NOT_FOUND`, `VALIDATION_ERROR` (422, malformed path diff --git a/src/services/previews/v1/github/artifacts.ts b/src/services/previews/v1/github/artifacts.ts index efd9ce4..ff45c1f 100644 --- a/src/services/previews/v1/github/artifacts.ts +++ b/src/services/previews/v1/github/artifacts.ts @@ -6,12 +6,19 @@ import { } from "../../../../lib/github-errors"; import { logWarn } from "../../../../lib/logger"; -// FOSSBilling/FOSSBilling's ci.yml uploads one unified artifact under this -// name for every PR build, non-main branch push, and main push - see -// upload-preview in that repo's .github/workflows/ci.yml. const REPO_OWNER = "FOSSBilling"; const REPO_NAME = "FOSSBilling"; -const PREVIEW_ARTIFACT_NAME = "FOSSBilling Preview"; + +// FOSSBilling/FOSSBilling's ci.yml uploads one artifact per commit for +// every PR build, non-main branch push, and main push - named after that +// commit's short SHA (archive: false, so the file's own basename becomes +// the artifact name - see upload-preview in that repo's +// .github/workflows/ci.yml). Querying by the exact name this produces +// returns at most the handful of runs that ever targeted this one commit, +// rather than every preview artifact in the retention window. +function artifactNameForSha(sha: string): string { + return `FOSSBilling-preview-${sha.slice(0, 7)}.zip`; +} export interface PreviewArtifact { runId: number; @@ -54,10 +61,10 @@ function unavailable( return { status: "unavailable", error: githubError }; } -// Lists artifacts named `FOSSBilling Preview` (server-side filtered, so the -// result set is bounded to one live artifact per recent successful run -// rather than the repo's entire artifact history) and returns the newest -// non-expired one whose triggering run built the given commit. +// Queries the exact artifact name this commit's build would have produced +// (see artifactNameForSha) and returns the newest non-expired match. The +// head_sha check below is defense against a short-SHA collision, not the +// primary matching mechanism - the name filter already does that. export async function findPreviewArtifactByCommitSha( githubToken: string, sha: string @@ -69,7 +76,7 @@ export async function findPreviewArtifactByCommitSha( { owner: REPO_OWNER, repo: REPO_NAME, - name: PREVIEW_ARTIFACT_NAME, + name: artifactNameForSha(sha), per_page: 100, headers: { Authorization: `Bearer ${githubToken}` } } diff --git a/src/services/previews/v1/r2.ts b/src/services/previews/v1/r2.ts index ec1e44a..4da7ba1 100644 --- a/src/services/previews/v1/r2.ts +++ b/src/services/previews/v1/r2.ts @@ -13,22 +13,18 @@ export interface MainPreviewObject { downloadUrl: string; } -// R2's zip and GitHub's own `FOSSBilling Preview` artifact zip for the same -// commit are two independently-built byte streams (see previews/v1's -// README), so the digest reported here has to come from this object's own -// metadata, not GitHub's. `sha256`/`commit-sha` are custom metadata the CI -// upload step sets explicitly - both are absent (null) until that step is -// added to FOSSBilling/FOSSBilling's ci.yml. +// `digest`/`commit-sha` are custom metadata FOSSBilling/FOSSBilling's +// ci.yml sets explicitly on the R2 upload (`digest` already carries the +// "sha256:" prefix - see that repo's ci.yml `upload-preview` job). export async function getMainPreviewObject( bucket: R2Bucket ): Promise { const object = await bucket.head(MAIN_PREVIEW_KEY); if (!object) return null; - const digest = object.customMetadata?.sha256; return { commitSha: object.customMetadata?.["commit-sha"] ?? null, - digest: digest ? `sha256:${digest}` : null, + digest: object.customMetadata?.digest ?? null, sizeBytes: object.size, lastModified: object.uploaded.toISOString(), downloadUrl: MAIN_PREVIEW_DOWNLOAD_URL diff --git a/test/services/previews/v1/commit.test.ts b/test/services/previews/v1/commit.test.ts index 19e1544..de0d6fa 100644 --- a/test/services/previews/v1/commit.test.ts +++ b/test/services/previews/v1/commit.test.ts @@ -86,6 +86,16 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { `/previews/v1/commit/${SHA}/download` ); expect(body.result.source).toBe("actions_artifact"); + + // Regression check: FOSSBilling/FOSSBilling's ci.yml names each + // artifact after the commit's short SHA rather than sharing one name + // across every run - querying the wrong name silently returns nothing. + expect(ghRequest).toHaveBeenCalledWith( + "GET /repos/{owner}/{repo}/actions/artifacts", + expect.objectContaining({ + name: `FOSSBilling-preview-${SHA.slice(0, 7)}.zip` + }) + ); }); it("matches on a short SHA prefix", async () => { diff --git a/test/services/previews/v1/main.test.ts b/test/services/previews/v1/main.test.ts index 660a027..5958233 100644 --- a/test/services/previews/v1/main.test.ts +++ b/test/services/previews/v1/main.test.ts @@ -31,8 +31,8 @@ describe("Previews API v1 - GET /previews/v1/main", () => { it("returns the R2 object's metadata, including the sha256 digest", async () => { await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { customMetadata: { - sha256: - "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", + digest: + "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", "commit-sha": "abc1234567890abc1234567890abc1234567890" } }); From 6accfc45f004e765c8ac0554d2935d65a617f42a Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 17:58:01 +0100 Subject: [PATCH 03/17] Deslop previews/v1: dedupe route dispatch, drop dead defensive check - Extracted routes/respond.ts (respondWithLookup, respondWithDownloadRedirect) so commit.ts and pr.ts stop repeating the same found/not_found/unavailable -> redirect chain almost verbatim - mirrors the shared-helper pattern extensions/v2's routes/errors.ts already established in this repo. - findPreviewArtifactByCommitSha no longer re-checks match.workflow_run after the loop - it was already guaranteed non-null by the loop's own continue guard, so the check was always false and dead. - main.ts now uses the shared notFoundBody() helper instead of hand-inlining the same {error:{message,code}} shape errors.ts already provides. - Dropped a comment on resolvePullRequestHeadSha that only restated its name/signature. No behavior change - full previews/v1 suite (16 tests) and the whole repo suite (495 + 47) still pass. --- src/services/previews/v1/github/artifacts.ts | 41 ++++++++----- src/services/previews/v1/routes/commit.ts | 55 +++-------------- src/services/previews/v1/routes/main.ts | 8 +-- src/services/previews/v1/routes/pr.ts | 64 +++----------------- src/services/previews/v1/routes/respond.ts | 60 ++++++++++++++++++ 5 files changed, 106 insertions(+), 122 deletions(-) create mode 100644 src/services/previews/v1/routes/respond.ts diff --git a/src/services/previews/v1/github/artifacts.ts b/src/services/previews/v1/github/artifacts.ts index ff45c1f..13fa778 100644 --- a/src/services/previews/v1/github/artifacts.ts +++ b/src/services/previews/v1/github/artifacts.ts @@ -84,32 +84,42 @@ export async function findPreviewArtifactByCommitSha( const artifacts = result.data.artifacts as RawArtifact[]; const shaLower = sha.toLowerCase(); - let match: RawArtifact | null = null; + let match: { + artifact: RawArtifact; + runId: number; + headSha: string; + } | null = null; for (const artifact of artifacts) { - if (artifact.expired || !artifact.workflow_run) continue; - if (!artifact.workflow_run.head_sha.toLowerCase().startsWith(shaLower)) { - continue; - } - if (!match || (artifact.created_at ?? "") > (match.created_at ?? "")) { - match = artifact; + const workflowRun = artifact.workflow_run; + if (artifact.expired || !workflowRun) continue; + if (!workflowRun.head_sha.toLowerCase().startsWith(shaLower)) continue; + if ( + !match || + (artifact.created_at ?? "") > (match.artifact.created_at ?? "") + ) { + match = { + artifact, + runId: workflowRun.id, + headSha: workflowRun.head_sha + }; } } - if (!match || !match.workflow_run) { + if (!match) { return { status: "not_found" }; } return { status: "found", data: { - runId: match.workflow_run.id, - artifactId: match.id, - commitSha: match.workflow_run.head_sha, - digest: match.digest ?? null, - sizeBytes: match.size_in_bytes, - createdAt: match.created_at ?? "", - expiresAt: match.expires_at ?? "" + runId: match.runId, + artifactId: match.artifact.id, + commitSha: match.headSha, + digest: match.artifact.digest ?? null, + sizeBytes: match.artifact.size_in_bytes, + createdAt: match.artifact.created_at ?? "", + expiresAt: match.artifact.expires_at ?? "" } }; } catch (error) { @@ -117,7 +127,6 @@ export async function findPreviewArtifactByCommitSha( } } -// Resolves a PR number to its current head commit SHA. export async function resolvePullRequestHeadSha( githubToken: string, prNumber: number diff --git a/src/services/previews/v1/routes/commit.ts b/src/services/previews/v1/routes/commit.ts index 5c717ac..e819064 100644 --- a/src/services/previews/v1/routes/commit.ts +++ b/src/services/previews/v1/routes/commit.ts @@ -4,10 +4,9 @@ import { CommitShaParamSchema, errorResponse } from "../schemas/previews"; -import { getArtifactDownloadUrl } from "../github/artifacts"; import { resolveArtifactPreview } from "../resolve"; import { cachedLookup } from "../cache"; -import { githubErrorBody, notFoundBody, statusFromGithubError } from "./errors"; +import { respondWithDownloadRedirect, respondWithLookup } from "./respond"; import { PreviewsV1App } from "./app"; export function registerCommitRoutes(app: PreviewsV1App): void { @@ -42,18 +41,10 @@ export function registerCommitRoutes(app: PreviewsV1App): void { () => resolveArtifactPreview(githubToken, sha, null) ); - if (result.status === "found") { - return c.json({ result: result.data }, 200); - } - if (result.status === "not_found") { - return c.json( - notFoundBody(`No preview artifact exists for commit ${sha}.`), - 404 - ); - } - return c.json( - githubErrorBody(result.error, "Failed to look up the preview artifact"), - statusFromGithubError(result.error) + return respondWithLookup( + c, + result, + `No preview artifact exists for commit ${sha}.` ); }); @@ -81,39 +72,11 @@ export function registerCommitRoutes(app: PreviewsV1App): void { // link that expires in about a minute, so it can never be served from // CACHE_KV alongside the longer-lived metadata response. const artifact = await resolveArtifactPreview(githubToken, sha, null); - if (artifact.status === "not_found") { - return c.json( - notFoundBody(`No preview artifact exists for commit ${sha}.`), - 404 - ); - } - if (artifact.status === "unavailable") { - return c.json( - githubErrorBody( - artifact.error, - "Failed to look up the preview artifact" - ), - statusFromGithubError(artifact.error) - ); - } - - const redirect = await getArtifactDownloadUrl( + return respondWithDownloadRedirect( + c, githubToken, - artifact.data.artifact_id + artifact, + `No preview artifact exists for commit ${sha}.` ); - if (redirect.status === "not_found") { - return c.json(notFoundBody("The preview artifact has expired."), 404); - } - if (redirect.status === "unavailable") { - return c.json( - githubErrorBody( - redirect.error, - "Failed to resolve the artifact download URL" - ), - statusFromGithubError(redirect.error) - ); - } - - return c.redirect(redirect.data, 302); }); } diff --git a/src/services/previews/v1/routes/main.ts b/src/services/previews/v1/routes/main.ts index b1f23b9..0a5548c 100644 --- a/src/services/previews/v1/routes/main.ts +++ b/src/services/previews/v1/routes/main.ts @@ -1,6 +1,7 @@ import { createRoute } from "@hono/zod-openapi"; import { MainPreviewResponseSchema, errorResponse } from "../schemas/previews"; import { getMainPreviewObject } from "../r2"; +import { notFoundBody } from "./errors"; import { PreviewsV1App } from "./app"; const MAIN_CACHE_KEY = "preview:main"; @@ -33,12 +34,7 @@ export function registerMainRoutes(app: PreviewsV1App): void { const object = await getMainPreviewObject(c.env.PREVIEW_BUCKET); if (!object) { return c.json( - { - error: { - message: "No main preview has been published yet", - code: "NOT_FOUND" - } - }, + notFoundBody("No main preview has been published yet"), 404 ); } diff --git a/src/services/previews/v1/routes/pr.ts b/src/services/previews/v1/routes/pr.ts index 0b76073..3eade8c 100644 --- a/src/services/previews/v1/routes/pr.ts +++ b/src/services/previews/v1/routes/pr.ts @@ -4,13 +4,10 @@ import { errorResponse, PrNumberParamSchema } from "../schemas/previews"; -import { - getArtifactDownloadUrl, - resolvePullRequestHeadSha -} from "../github/artifacts"; +import { resolvePullRequestHeadSha } from "../github/artifacts"; import { PreviewLookupResult, resolveArtifactPreview } from "../resolve"; import { cachedLookup } from "../cache"; -import { githubErrorBody, notFoundBody, statusFromGithubError } from "./errors"; +import { respondWithDownloadRedirect, respondWithLookup } from "./respond"; import { PreviewsV1App } from "./app"; // Resolves a PR number to its artifact preview by first finding the head @@ -25,6 +22,9 @@ async function resolvePrPreview( return resolveArtifactPreview(githubToken, head.data, prNumber); } +const notFoundMessage = (prNumber: number) => + `No pull request #${prNumber} was found, or it has no preview build yet.`; + export function registerPrRoutes(app: PreviewsV1App): void { const prRoute = createRoute({ method: "get", @@ -57,21 +57,7 @@ export function registerPrRoutes(app: PreviewsV1App): void { () => resolvePrPreview(githubToken, number) ); - if (result.status === "found") { - return c.json({ result: result.data }, 200); - } - if (result.status === "not_found") { - return c.json( - notFoundBody( - `No pull request #${number} was found, or it has no preview build yet.` - ), - 404 - ); - } - return c.json( - githubErrorBody(result.error, "Failed to look up the preview artifact"), - statusFromGithubError(result.error) - ); + return respondWithLookup(c, result, notFoundMessage(number)); }); const prDownloadRoute = createRoute({ @@ -96,41 +82,11 @@ export function registerPrRoutes(app: PreviewsV1App): void { // Always resolved live, same reasoning as /commit/{sha}/download. const artifact = await resolvePrPreview(githubToken, number); - if (artifact.status === "not_found") { - return c.json( - notFoundBody( - `No pull request #${number} was found, or it has no preview build yet.` - ), - 404 - ); - } - if (artifact.status === "unavailable") { - return c.json( - githubErrorBody( - artifact.error, - "Failed to look up the preview artifact" - ), - statusFromGithubError(artifact.error) - ); - } - - const redirect = await getArtifactDownloadUrl( + return respondWithDownloadRedirect( + c, githubToken, - artifact.data.artifact_id + artifact, + notFoundMessage(number) ); - if (redirect.status === "not_found") { - return c.json(notFoundBody("The preview artifact has expired."), 404); - } - if (redirect.status === "unavailable") { - return c.json( - githubErrorBody( - redirect.error, - "Failed to resolve the artifact download URL" - ), - statusFromGithubError(redirect.error) - ); - } - - return c.redirect(redirect.data, 302); }); } diff --git a/src/services/previews/v1/routes/respond.ts b/src/services/previews/v1/routes/respond.ts new file mode 100644 index 0000000..7fea326 --- /dev/null +++ b/src/services/previews/v1/routes/respond.ts @@ -0,0 +1,60 @@ +import { Context } from "hono"; +import { getArtifactDownloadUrl } from "../github/artifacts"; +import { PreviewLookupResult } from "../resolve"; +import { githubErrorBody, notFoundBody, statusFromGithubError } from "./errors"; + +// Shared by /commit/{sha} and /pr/{number}: both resolve to a +// PreviewLookupResult and only differ in their not-found message. +export function respondWithLookup( + c: Context, + result: PreviewLookupResult, + notFoundMessage: string +) { + if (result.status === "found") { + return c.json({ result: result.data }, 200); + } + if (result.status === "not_found") { + return c.json(notFoundBody(notFoundMessage), 404); + } + return c.json( + githubErrorBody(result.error, "Failed to look up the preview artifact"), + statusFromGithubError(result.error) + ); +} + +// Shared by /commit/{sha}/download and /pr/{number}/download. +export async function respondWithDownloadRedirect( + c: Context, + githubToken: string, + artifact: PreviewLookupResult, + notFoundMessage: string +) { + if (artifact.status === "not_found") { + return c.json(notFoundBody(notFoundMessage), 404); + } + if (artifact.status === "unavailable") { + return c.json( + githubErrorBody(artifact.error, "Failed to look up the preview artifact"), + statusFromGithubError(artifact.error) + ); + } + + const redirect = await getArtifactDownloadUrl( + githubToken, + artifact.data.artifact_id + ); + if (redirect.status === "not_found") { + return c.json(notFoundBody("The preview artifact has expired."), 404); + } + if (redirect.status === "unavailable") { + return c.json( + githubErrorBody( + redirect.error, + "Failed to resolve the artifact download URL" + ), + statusFromGithubError(redirect.error) + ); + } + + return c.redirect(redirect.data, 302); +} From a0c427b9a1cc4c8c8df9d4a5611bea68054c001b Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 18:11:12 +0100 Subject: [PATCH 04/17] Cache-share /download routes to cut GitHub API calls per hit GitHub's authenticated rate limit is 5,000 requests/hour, shared with versions/v1 on the same GITHUB_TOKEN. Before this, /commit/{sha}/download and /pr/{number}/download bypassed cachedLookup entirely and re-resolved the artifact lookup from scratch on every single hit - 2 GitHub calls per commit download, 3 per PR download (PR->SHA, SHA->artifact, then the redirect), always, with zero caching. At that rate a few thousand downloads/hour would exhaust the whole token's budget and start failing versions/v1 too. Only the final redirect call is genuinely uncacheable (GitHub's signed URL expires in ~60s) - the artifact lookup that precedes it is exactly what the metadata routes already cache for 60s. Both /download handlers now call cachedLookup with the same cache key the metadata route uses, so a burst of downloads for the same commit/PR costs ~1 GitHub call per 60s window for the lookup, plus the one redirect call that can't be avoided, instead of 2-3 calls per individual hit. Added regression tests asserting the artifacts-list/pulls call counts stay at 1 across a metadata request followed by a download request for the same resource. --- src/services/previews/v1/routes/commit.ts | 15 ++++++--- src/services/previews/v1/routes/pr.ts | 11 +++++-- test/services/previews/v1/commit.test.ts | 31 +++++++++++++++++++ test/services/previews/v1/pr.test.ts | 37 +++++++++++++++++++++++ 4 files changed, 88 insertions(+), 6 deletions(-) diff --git a/src/services/previews/v1/routes/commit.ts b/src/services/previews/v1/routes/commit.ts index e819064..c744d43 100644 --- a/src/services/previews/v1/routes/commit.ts +++ b/src/services/previews/v1/routes/commit.ts @@ -68,10 +68,17 @@ export function registerCommitRoutes(app: PreviewsV1App): void { const { sha } = c.req.valid("param"); const githubToken = c.env.GITHUB_TOKEN; - // Always resolved live - GitHub's artifact download URL is a signed - // link that expires in about a minute, so it can never be served from - // CACHE_KV alongside the longer-lived metadata response. - const artifact = await resolveArtifactPreview(githubToken, sha, null); + // Shares the metadata route's cache entry for which artifact to + // download - only the redirect URL itself (resolved inside + // respondWithDownloadRedirect) has to be live on every hit, since + // that's the part that expires in about a minute. Reusing the cache + // here is what keeps a burst of downloads for the same commit to ~1 + // GitHub API call per cache window instead of 1 per request. + const artifact = await cachedLookup( + c.env.CACHE_KV, + `preview:commit:${sha.toLowerCase()}`, + () => resolveArtifactPreview(githubToken, sha, null) + ); return respondWithDownloadRedirect( c, githubToken, diff --git a/src/services/previews/v1/routes/pr.ts b/src/services/previews/v1/routes/pr.ts index 3eade8c..af9ee04 100644 --- a/src/services/previews/v1/routes/pr.ts +++ b/src/services/previews/v1/routes/pr.ts @@ -80,8 +80,15 @@ export function registerPrRoutes(app: PreviewsV1App): void { const { number } = c.req.valid("param"); const githubToken = c.env.GITHUB_TOKEN; - // Always resolved live, same reasoning as /commit/{sha}/download. - const artifact = await resolvePrPreview(githubToken, number); + // Shares the metadata route's cache entry - see the equivalent comment + // in routes/commit.ts. Without this, every download hit would cost 3 + // GitHub API calls (PR->SHA, SHA->artifact, then the redirect) instead + // of the 1 that's actually unavoidable. + const artifact = await cachedLookup( + c.env.CACHE_KV, + `preview:pr:${number}`, + () => resolvePrPreview(githubToken, number) + ); return respondWithDownloadRedirect( c, githubToken, diff --git a/test/services/previews/v1/commit.test.ts b/test/services/previews/v1/commit.test.ts index de0d6fa..095871e 100644 --- a/test/services/previews/v1/commit.test.ts +++ b/test/services/previews/v1/commit.test.ts @@ -175,4 +175,35 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { "https://example.com/signed-download" ); }); + + it("shares the metadata route's cache instead of re-listing artifacts on every download", async () => { + let artifactsListCalls = 0; + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async (route: string) => { + if (route === "GET /repos/{owner}/{repo}/actions/artifacts") { + artifactsListCalls++; + return { data: SAMPLE_ARTIFACTS }; + } + if ( + route === + "GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/{archive_format}" + ) { + return { + status: 302, + headers: { location: "https://example.com/signed-download" } + }; + } + throw new Error(`Unexpected route: ${route}`); + } + ); + + await get(`/previews/v1/commit/${SHA}`); + const res = await get(`/previews/v1/commit/${SHA}/download`); + + expect(res.status).toBe(302); + // The artifact lookup ran once (warming the cache on the first + // request) - the download request reused it rather than listing + // artifacts again just to find the same artifact_id. + expect(artifactsListCalls).toBe(1); + }); }); diff --git a/test/services/previews/v1/pr.test.ts b/test/services/previews/v1/pr.test.ts index aa0e71a..9e91ea9 100644 --- a/test/services/previews/v1/pr.test.ts +++ b/test/services/previews/v1/pr.test.ts @@ -137,4 +137,41 @@ describe("Previews API v1 - GET /previews/v1/pr/:number", () => { "https://example.com/signed-download" ); }); + + it("shares the metadata route's cache instead of re-resolving the PR on every download", async () => { + let pullsCalls = 0; + let artifactsListCalls = 0; + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async (route: string) => { + if (route === "GET /repos/{owner}/{repo}/pulls/{pull_number}") { + pullsCalls++; + return { data: { head: { sha: SHA } } }; + } + if (route === "GET /repos/{owner}/{repo}/actions/artifacts") { + artifactsListCalls++; + return { data: SAMPLE_ARTIFACTS }; + } + if ( + route === + "GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/{archive_format}" + ) { + return { + status: 302, + headers: { location: "https://example.com/signed-download" } + }; + } + throw new Error(`Unexpected route: ${route}`); + } + ); + + await get(`/previews/v1/pr/${PR_NUMBER}`); + const res = await get(`/previews/v1/pr/${PR_NUMBER}/download`); + + expect(res.status).toBe(302); + // Both the PR->SHA resolution and the artifact lookup ran once, + // warming the cache on the first request - the download request + // reused that instead of re-resolving the PR from scratch. + expect(pullsCalls).toBe(1); + expect(artifactsListCalls).toBe(1); + }); }); From f0bacce8d123ac1c069e0927ee6cbf26d7be769c Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 18:25:18 +0100 Subject: [PATCH 05/17] Fix pre-existing typecheck failure: bump @hono/zod-openapi to 1.5.3 Unrelated to previews/v1 - this was already broken on main, just never caught because ci.yml only runs lint + test, never 'npm run typecheck'. Root cause: @hono/zod-openapi@1.5.2's bundled type declarations (dist/index.d.mts and .d.cts) contained a broken 'import z = zodModule.z;' referencing a namespace never actually imported anywhere in the file. Under this project's skipLibCheck:true, that doesn't error where it's declared - it silently makes every zod schema built through @hono/zod-openapi's re-exported z resolve to an unresolvable type (confirmed empirically: a bare z.string() was assignable to `number` with zero complaint). That's invisible almost everywhere, since TypeScript accepts an unresolvable/any-like type without comment - the only two places it became a visible error were noImplicitAny flagging five .refine()/.transform() callback parameters directly, and a cast in db/revisions.ts choking on one specific field whose type had degraded to the literal unresolved generic 'z.infer' (not simplified to plain any, so it didn't get any's usual free pass on structural overlap). 1.5.3 fixes the export to 'import { z } from "zod"' directly. Verified by reverting speculative return-type annotations added while diagnosing (unneeded once the actual bug is fixed) and confirming typecheck is clean on the dependency bump alone - no source changes required. Already within package.json's existing "^1.5.1" range, so package.json itself doesn't need editing, only the lockfile. Full suite still green: 497 + 47 tests, lint clean. --- package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 03f739b..87beb0e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1477,9 +1477,9 @@ } }, "node_modules/@hono/zod-openapi": { - "version": "1.5.2", - "resolved": "https://registry.npmjs.org/@hono/zod-openapi/-/zod-openapi-1.5.2.tgz", - "integrity": "sha512-FPlspM6+qObGoMfux+0SSE0of0tGO+BVo3havhKzZMxRyz/ql89kuCd/7POd70aW3T4kz/aVL6gNZ6sVd0ItUQ==", + "version": "1.5.3", + "resolved": "https://registry.npmjs.org/@hono/zod-openapi/-/zod-openapi-1.5.3.tgz", + "integrity": "sha512-hG+2wh72WK4z59Cn6UQUb5Ohx9HINzCbZ6oEwqvtAkOe9b2Oc5N9/jPjUzhF49lgSo73s2EoHyJx+gGEmpkv5Q==", "license": "MIT", "dependencies": { "@asteasolutions/zod-to-openapi": "^8.5.0", From 905c673fbacf4fcf37c5b784a3ae85d758ebe01f Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 18:35:01 +0100 Subject: [PATCH 06/17] Longer cache TTL for immutable commit lookups Implements one of the two "further tightenings" from earlier - cache /commit/{sha} (and its /download counterpart, which shares the same cache entry) for 3600s instead of the 60s default. Unlike main/pr, which are moving pointers, a commit's build never changes once it exists, so there's no correctness reason to re-check it every minute - this cuts repeat-download GitHub API calls by 60x for the same commit within an hour, safely within GitHub's 14-day artifact retention. cachedLookup() now takes an optional ttlSeconds parameter (defaults to the existing 60s) rather than a hardcoded constant, so callers can opt into a longer window where the data actually warrants it. The other proposed tightening - caching the resolved signed redirect URL itself for ~45s to collapse a burst of downloads to one GitHub call - turned out not to be safely achievable and was not implemented. Confirmed empirically (not by assumption): Cloudflare KV enforces a hard 60-second minimum TTL ("KV PUT failed: 400 Invalid expiration_ttl of 45. Expiration TTL must be at least 60."), which is not less than GitHub's own ~60s signed-URL expiry. There's no safe margin available through KV - caching at the 60s floor risks handing out a URL that's already expired by the time a client follows the redirect. Doing this safely would need a different caching layer (e.g. the Cache API, which download-worker's original prototype used for exactly this reason) - left alone rather than force a correctness risk into KV for a rate-limit optimization. Full suite: 503 + 47 tests, lint and typecheck clean. --- src/services/previews/v1/cache.ts | 17 +++--- src/services/previews/v1/routes/commit.ts | 26 +++++---- src/services/previews/v1/routes/respond.ts | 7 ++- test/services/previews/v1/cache.test.ts | 61 ++++++++++++++++++++++ test/services/previews/v1/commit.test.ts | 16 ++++++ test/services/previews/v1/pr.test.ts | 19 +++++++ 6 files changed, 129 insertions(+), 17 deletions(-) create mode 100644 test/services/previews/v1/cache.test.ts diff --git a/src/services/previews/v1/cache.ts b/src/services/previews/v1/cache.ts index e2ba679..6c40177 100644 --- a/src/services/previews/v1/cache.ts +++ b/src/services/previews/v1/cache.ts @@ -1,10 +1,12 @@ import { GithubLookupResult } from "./github/artifacts"; -// Previews churn often (a new commit on a PR supersedes the last build -// within minutes), so a short TTL keeps CACHE_KV useful without serving -// meaningfully stale data - matches download-worker's existing choice for -// the same trade-off. -const CACHE_TTL_SECONDS = 60; +// Default for anything that moves (main, pr/{number}) - previews churn +// often (a new commit on a PR supersedes the last build within minutes), +// so a short TTL keeps CACHE_KV useful without serving meaningfully stale +// data - matches download-worker's existing choice for the same trade-off. +// Callers addressing something immutable (a fixed commit/artifact) pass a +// longer ttlSeconds explicitly - see routes/commit.ts and routes/respond.ts. +export const DEFAULT_CACHE_TTL_SECONDS = 60; // Only "found" results are cached. "not_found"/"unavailable" always // re-resolve, so a transient GitHub hiccup or a not-yet-built PR doesn't @@ -12,7 +14,8 @@ const CACHE_TTL_SECONDS = 60; export async function cachedLookup( kv: KVNamespace, key: string, - resolve: () => Promise> + resolve: () => Promise>, + ttlSeconds: number = DEFAULT_CACHE_TTL_SECONDS ): Promise> { const cached = await kv.get(key); if (cached !== null) { @@ -26,7 +29,7 @@ export async function cachedLookup( const result = await resolve(); if (result.status === "found") { await kv.put(key, JSON.stringify(result.data), { - expirationTtl: CACHE_TTL_SECONDS + expirationTtl: ttlSeconds }); } return result; diff --git a/src/services/previews/v1/routes/commit.ts b/src/services/previews/v1/routes/commit.ts index c744d43..de46d2d 100644 --- a/src/services/previews/v1/routes/commit.ts +++ b/src/services/previews/v1/routes/commit.ts @@ -9,6 +9,14 @@ import { cachedLookup } from "../cache"; import { respondWithDownloadRedirect, respondWithLookup } from "./respond"; import { PreviewsV1App } from "./app"; +// A commit's build never changes once it exists, unlike main/pr's moving +// pointers - safe to cache far longer than the 60s default, well within +// GitHub's 14-day artifact retention. Cuts repeat-download GitHub calls by +// 60x for the same commit within an hour. +const COMMIT_CACHE_TTL_SECONDS = 3600; + +const cacheKeyForSha = (sha: string) => `preview:commit:${sha.toLowerCase()}`; + export function registerCommitRoutes(app: PreviewsV1App): void { const commitRoute = createRoute({ method: "get", @@ -37,8 +45,9 @@ export function registerCommitRoutes(app: PreviewsV1App): void { const result = await cachedLookup( c.env.CACHE_KV, - `preview:commit:${sha.toLowerCase()}`, - () => resolveArtifactPreview(githubToken, sha, null) + cacheKeyForSha(sha), + () => resolveArtifactPreview(githubToken, sha, null), + COMMIT_CACHE_TTL_SECONDS ); return respondWithLookup( @@ -69,15 +78,14 @@ export function registerCommitRoutes(app: PreviewsV1App): void { const githubToken = c.env.GITHUB_TOKEN; // Shares the metadata route's cache entry for which artifact to - // download - only the redirect URL itself (resolved inside - // respondWithDownloadRedirect) has to be live on every hit, since - // that's the part that expires in about a minute. Reusing the cache - // here is what keeps a burst of downloads for the same commit to ~1 - // GitHub API call per cache window instead of 1 per request. + // download - only the signed URL itself (resolved inside + // respondWithDownloadRedirect, on its own short-lived cache) has to be + // re-checked often, since that's the part that actually expires. const artifact = await cachedLookup( c.env.CACHE_KV, - `preview:commit:${sha.toLowerCase()}`, - () => resolveArtifactPreview(githubToken, sha, null) + cacheKeyForSha(sha), + () => resolveArtifactPreview(githubToken, sha, null), + COMMIT_CACHE_TTL_SECONDS ); return respondWithDownloadRedirect( c, diff --git a/src/services/previews/v1/routes/respond.ts b/src/services/previews/v1/routes/respond.ts index 7fea326..e1c6c0a 100644 --- a/src/services/previews/v1/routes/respond.ts +++ b/src/services/previews/v1/routes/respond.ts @@ -22,7 +22,12 @@ export function respondWithLookup( ); } -// Shared by /commit/{sha}/download and /pr/{number}/download. +// Shared by /commit/{sha}/download and /pr/{number}/download. Always +// resolved live, never cached - GitHub's signed URL expires in ~60s, and +// KV enforces a hard 60s minimum TTL, so there's no safe margin available +// to cache it without risking handing out an already-expired URL. See +// preview:redirect caching's revert in git history for why that was tried +// and abandoned. export async function respondWithDownloadRedirect( c: Context, githubToken: string, diff --git a/test/services/previews/v1/cache.test.ts b/test/services/previews/v1/cache.test.ts new file mode 100644 index 0000000..86d7fed --- /dev/null +++ b/test/services/previews/v1/cache.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { env } from "cloudflare:workers"; +import { cachedLookup } from "../../../../src/services/previews/v1/cache"; + +describe("previews/v1 cachedLookup", () => { + beforeEach(async () => { + await env.CACHE_KV.delete("test-key"); + }); + + it("defaults to a 60s TTL", async () => { + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + + await cachedLookup(env.CACHE_KV, "test-key", async () => ({ + status: "found", + data: "value" + })); + + expect(putSpy).toHaveBeenCalledWith("test-key", JSON.stringify("value"), { + expirationTtl: 60 + }); + putSpy.mockRestore(); + }); + + it("accepts a longer TTL for immutable data", async () => { + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + + await cachedLookup( + env.CACHE_KV, + "test-key", + async () => ({ status: "found", data: "value" }), + 3600 + ); + + expect(putSpy).toHaveBeenCalledWith("test-key", JSON.stringify("value"), { + expirationTtl: 3600 + }); + putSpy.mockRestore(); + }); + + it("does not cache not_found results", async () => { + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + + const result = await cachedLookup(env.CACHE_KV, "test-key", async () => ({ + status: "not_found" + })); + + expect(result.status).toBe("not_found"); + expect(putSpy).not.toHaveBeenCalled(); + putSpy.mockRestore(); + }); + + it("serves a cache hit without calling resolve again", async () => { + const resolve = vi.fn().mockResolvedValue({ status: "found", data: "v1" }); + + await cachedLookup(env.CACHE_KV, "test-key", resolve); + const second = await cachedLookup(env.CACHE_KV, "test-key", resolve); + + expect(second).toEqual({ status: "found", data: "v1" }); + expect(resolve).toHaveBeenCalledTimes(1); + }); +}); diff --git a/test/services/previews/v1/commit.test.ts b/test/services/previews/v1/commit.test.ts index 095871e..29003ad 100644 --- a/test/services/previews/v1/commit.test.ts +++ b/test/services/previews/v1/commit.test.ts @@ -206,4 +206,20 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { // artifacts again just to find the same artifact_id. expect(artifactsListCalls).toBe(1); }); + + it("caches the commit lookup for longer than the default 60s", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ data: SAMPLE_ARTIFACTS }) + ); + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + + await get(`/previews/v1/commit/${SHA}`); + + expect(putSpy).toHaveBeenCalledWith( + `preview:commit:${SHA.toLowerCase()}`, + expect.any(String), + { expirationTtl: 3600 } + ); + putSpy.mockRestore(); + }); }); diff --git a/test/services/previews/v1/pr.test.ts b/test/services/previews/v1/pr.test.ts index 9e91ea9..c69c115 100644 --- a/test/services/previews/v1/pr.test.ts +++ b/test/services/previews/v1/pr.test.ts @@ -174,4 +174,23 @@ describe("Previews API v1 - GET /previews/v1/pr/:number", () => { expect(pullsCalls).toBe(1); expect(artifactsListCalls).toBe(1); }); + + it("caches the PR lookup at the default 60s, unlike commit's longer TTL", async () => { + mockGithub({ + "GET /repos/{owner}/{repo}/pulls/{pull_number}": { + data: { head: { sha: SHA } } + }, + "GET /repos/{owner}/{repo}/actions/artifacts": { data: SAMPLE_ARTIFACTS } + }); + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + + await get(`/previews/v1/pr/${PR_NUMBER}`); + + expect(putSpy).toHaveBeenCalledWith( + `preview:pr:${PR_NUMBER}`, + expect.any(String), + { expirationTtl: 60 } + ); + putSpy.mockRestore(); + }); }); From d499766e00c9e3957b134c177802a53b60a773d6 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 18:35:24 +0100 Subject: [PATCH 07/17] Update previews/v1 README for the commit-lookup TTL change --- src/services/previews/v1/README.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md index b0dfdb8..b8287ad 100644 --- a/src/services/previews/v1/README.md +++ b/src/services/previews/v1/README.md @@ -113,11 +113,19 @@ etc., surfaced as 429/503/500 depending on severity). ## Notes -- `GET /pr/{number}` and `GET /commit/{sha}` responses are cached in - `CACHE_KV` for 60 seconds (`preview:pr:{number}` / `preview:commit:{sha}`); - `GET /main` for 60 seconds (`preview:main`). Only successful lookups are - cached - a not-yet-built PR or a transient GitHub error always re-resolves - on the next request. +- Responses are cached in `CACHE_KV`, only for successful lookups - a + not-yet-built PR or a transient GitHub error always re-resolves on the + next request. `GET /pr/{number}` (`preview:pr:{number}`) and `GET /main` + (`preview:main`) use the 60s default, matching how often a moving + pointer can realistically change. `GET /commit/{sha}` + (`preview:commit:{sha}`, also used by `/commit/{sha}/download` and + `/pr/{number}/download` to avoid re-resolving what the metadata route + already cached) uses 3600s instead - a commit's build never changes once + it exists, so there's no correctness reason to re-check it every minute. +- `GET /pr/{number}/download` and `GET /commit/{sha}/download` always + resolve GitHub's signed redirect URL live, never cached - it expires in + about a minute, and Cloudflare KV's 60s minimum TTL leaves no safe margin + to cache it without risking handing out an already-expired URL. - `GITHUB_TOKEN` is required for GitHub API access (shared with `versions/v1`). - `PREVIEW_BUCKET` (R2 binding) backs `/main` - see `wrangler.jsonc` for the From e8fee1d67bea3972bde3baa93bead6b3f50fd3cb Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 18:45:13 +0100 Subject: [PATCH 08/17] Add GET /previews/v1/main/download for uniform addressing Every other resource (pr/{number}, commit/{sha}) already has a /download sub-route; main was the one gap, requiring callers to read download_url out of the JSON body instead of hitting a consistent /download path like everything else. Unlike pr/commit's download routes, main's target is a fixed, permanent URL rather than a short-lived signed one, so this doesn't need a live GitHub resolution - it shares /main's existing cached R2 lookup and just redirects to the same download_url GET /main already reports, 404ing the same way if no main preview has been published yet. Refactored the R2 cache-then-fetch logic in routes/main.ts into a shared resolveMainPreview() helper used by both routes rather than duplicating it. --- src/services/previews/v1/README.md | 10 +++ src/services/previews/v1/routes/main.ts | 89 +++++++++++++++++++------ test/services/previews/v1/main.test.ts | 40 ++++++++++- 3 files changed, 116 insertions(+), 23 deletions(-) diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md index b8287ad..57b8752 100644 --- a/src/services/previews/v1/README.md +++ b/src/services/previews/v1/README.md @@ -56,6 +56,16 @@ Current main preview, sourced from an R2 object HEAD (no GitHub API call). both are `null` if that object has no custom metadata (e.g. it predates the CI job setting it). +### GET `/main/download` + +302 redirect to `download_url` - the same permanent URL `GET /main` already +reports. Exists purely for uniform addressing (every resource under +`/previews/v1` has a `/download` sub-route, so callers never need to +special-case main to reach a download link instead of reading one out of a +JSON body). Unlike `/pr/{number}/download` and `/commit/{sha}/download`, +this target URL is fixed rather than short-lived, so it's answered from the +same cache as `GET /main` instead of re-resolving anything live. + ### GET `/pr/{number}` and GET `/commit/{sha}` Preview build for a pull request's current head, or for one exact commit. diff --git a/src/services/previews/v1/routes/main.ts b/src/services/previews/v1/routes/main.ts index 0a5548c..2fe133e 100644 --- a/src/services/previews/v1/routes/main.ts +++ b/src/services/previews/v1/routes/main.ts @@ -1,5 +1,10 @@ import { createRoute } from "@hono/zod-openapi"; -import { MainPreviewResponseSchema, errorResponse } from "../schemas/previews"; +import { Context } from "hono"; +import { + MainPreview, + MainPreviewResponseSchema, + errorResponse +} from "../schemas/previews"; import { getMainPreviewObject } from "../r2"; import { notFoundBody } from "./errors"; import { PreviewsV1App } from "./app"; @@ -7,6 +12,36 @@ import { PreviewsV1App } from "./app"; const MAIN_CACHE_KEY = "preview:main"; const MAIN_CACHE_TTL_SECONDS = 60; +// Shared by /main and /main/download - both need the same cache-then-R2 +// lookup, just to different ends (the full body vs. only download_url). +async function resolveMainPreview( + c: Context<{ Bindings: CloudflareBindings }> +): Promise { + const cached = await c.env.CACHE_KV.get(MAIN_CACHE_KEY); + if (cached) { + return JSON.parse(cached) as MainPreview; + } + + const object = await getMainPreviewObject(c.env.PREVIEW_BUCKET); + if (!object) return null; + + const result: MainPreview = { + commit_sha: object.commitSha, + short_sha: object.commitSha?.slice(0, 7) ?? null, + digest: object.digest, + size_bytes: object.sizeBytes, + last_modified: object.lastModified, + download_url: object.downloadUrl, + source: "r2" + }; + + await c.env.CACHE_KV.put(MAIN_CACHE_KEY, JSON.stringify(result), { + expirationTtl: MAIN_CACHE_TTL_SECONDS + }); + + return result; +} + export function registerMainRoutes(app: PreviewsV1App): void { const mainRoute = createRoute({ method: "get", @@ -26,33 +61,43 @@ export function registerMainRoutes(app: PreviewsV1App): void { }); app.openapi(mainRoute, async (c) => { - const cached = await c.env.CACHE_KV.get(MAIN_CACHE_KEY); - if (cached) { - return c.json({ result: JSON.parse(cached) }, 200); - } - - const object = await getMainPreviewObject(c.env.PREVIEW_BUCKET); - if (!object) { + const result = await resolveMainPreview(c); + if (!result) { return c.json( notFoundBody("No main preview has been published yet"), 404 ); } + return c.json({ result }, 200); + }); - const result = { - commit_sha: object.commitSha, - short_sha: object.commitSha?.slice(0, 7) ?? null, - digest: object.digest, - size_bytes: object.sizeBytes, - last_modified: object.lastModified, - download_url: object.downloadUrl, - source: "r2" as const - }; - - await c.env.CACHE_KV.put(MAIN_CACHE_KEY, JSON.stringify(result), { - expirationTtl: MAIN_CACHE_TTL_SECONDS - }); + // Unlike /pr/{number}/download and /commit/{sha}/download, main's + // download_url is a fixed, permanent path (download.fossbilling.org) + // rather than a live, short-lived signed URL - so this is a plain + // redirect once existence is confirmed, not a fresh resolution on every + // hit. Exists for uniform addressing: every resource under /previews/v1 + // has a /download sub-route, so callers never need to special-case main + // to reach a download link instead of reading it out of the JSON body. + const mainDownloadRoute = createRoute({ + method: "get", + path: "/main/download", + tags: ["Previews"], + summary: "Download the current main preview", + responses: { + 302: { description: "Redirect to the main preview download URL" }, + 404: errorResponse("No main preview has been published yet"), + 500: errorResponse("R2 lookup failed") + } + }); - return c.json({ result }, 200); + app.openapi(mainDownloadRoute, async (c) => { + const result = await resolveMainPreview(c); + if (!result) { + return c.json( + notFoundBody("No main preview has been published yet"), + 404 + ); + } + return c.redirect(result.download_url, 302); }); } diff --git a/test/services/previews/v1/main.test.ts b/test/services/previews/v1/main.test.ts index 5958233..6eb6924 100644 --- a/test/services/previews/v1/main.test.ts +++ b/test/services/previews/v1/main.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, vi } from "vitest"; import { createExecutionContext, waitOnExecutionContext @@ -98,3 +98,41 @@ describe("Previews API v1 - GET /previews/v1/main", () => { expect(secondBody.result.commit_sha).toBe("111"); }); }); + +describe("Previews API v1 - GET /previews/v1/main/download", () => { + beforeEach(async () => { + await env.CACHE_KV.delete("preview:main"); + await env.PREVIEW_BUCKET.delete(MAIN_PREVIEW_KEY); + }); + + it("returns 404 when no main preview has been published", async () => { + const res = await get("/previews/v1/main/download"); + expect(res.status).toBe(404); + const body = (await res.json()) as { error: { code: string } }; + expect(body.error.code).toBe("NOT_FOUND"); + }); + + it("redirects to the permanent main preview download URL", async () => { + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents"); + + const res = await get("/previews/v1/main/download"); + expect(res.status).toBe(302); + expect(res.headers.get("location")).toBe( + "https://download.fossbilling.org/FOSSBilling-preview.zip" + ); + }); + + it("shares the metadata route's cache instead of re-reading R2", async () => { + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents"); + const headSpy = vi.spyOn(env.PREVIEW_BUCKET, "head"); + + await get("/previews/v1/main"); + const res = await get("/previews/v1/main/download"); + + expect(res.status).toBe(302); + // The R2 HEAD ran once (warming the cache on the first request) - the + // download request reused it rather than reading R2 again. + expect(headSpy).toHaveBeenCalledTimes(1); + headSpy.mockRestore(); + }); +}); From d7e29c5eb6fb9ba1c6c96411346ddf3732e86871 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 18:52:03 +0100 Subject: [PATCH 09/17] Enrich /main with GitHub Actions build metadata for shape parity MainPreview gains pr_number (always null - main isn't a PR), run_id, artifact_id, created_at, and expires_at, matching ArtifactPreview's field set so a client doesn't have to special-case which fields are available depending on which endpoint it hit. download_url/digest stay R2-sourced and source stays "r2" - that distinction is kept deliberately, not just left over: main's download link is permanent, pr/commit's are ephemeral signed URLs, and a client needs to be able to tell those apart. The new fields are pulled from that commit's GitHub Actions artifact via the same findPreviewArtifactByCommitSha() /commit/{sha} already uses, purely as enrichment - never a dependency. If the commit has no artifact yet (e.g. it's aged out of GitHub's 14-day retention) or GitHub errors, the five fields are just null and /main still returns 200 with everything R2-sourced intact. Slots into the existing preview:main cache entry, so the GitHub call only happens once per 60s cache window, not per request. Tests cover: successful enrichment, GitHub-unavailable degradation, no-known-artifact degradation, no-commit-sha (skips the GitHub call entirely), and that the enrichment call is cache-shared. Full suite: 509 + 47 tests, lint and typecheck clean. --- src/services/previews/v1/README.md | 38 ++++-- src/services/previews/v1/routes/main.ts | 38 ++++++ src/services/previews/v1/schemas/previews.ts | 12 ++ test/services/previews/v1/main.test.ts | 128 ++++++++++++++++++- 4 files changed, 200 insertions(+), 16 deletions(-) diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md index 57b8752..3d1daba 100644 --- a/src/services/previews/v1/README.md +++ b/src/services/previews/v1/README.md @@ -8,13 +8,16 @@ uploads one artifact per commit, named `FOSSBilling-preview-{short_sha}.zip` (`archive: false`, so the zip itself is the artifact - no extra wrapping), for every PR build, non-main branch push, and main push. This service resolves by querying that exact name rather than listing every preview -artifact and filtering. The `main` preview is answered from R2 instead, -sourced from `digest`/`commit-sha` custom object metadata the same CI job -sets on the R2 upload - kept separate from the GitHub-artifact path because -the R2 zip and the GitHub artifact zip for a given commit are two -independently-built files (a `cp` of the same bytes, in the current CI job, -but not guaranteed to stay that way), so whichever one is reported as the -digest has to match the bytes `main` actually serves. +artifact and filtering. The `main` preview's `download_url`/`digest` are +answered from R2 instead, sourced from `digest`/`commit-sha` custom object +metadata the same CI job sets on the R2 upload - kept separate from the +GitHub-artifact path because the R2 zip and the GitHub artifact zip for a +given commit are two independently-built files (a `cp` of the same bytes, +in the current CI job, but not guaranteed to stay that way), so whichever +one is reported as the digest has to match the bytes `main` actually +serves. `GET /main` does still cross-reference that commit's GitHub Actions +artifact for `run_id`/`artifact_id`/`created_at`/`expires_at` - see its +section below - but only as best-effort enrichment, never as a dependency. There is no publish/write endpoint: nothing pushes data into this service, it only resolves and redirects. @@ -33,7 +36,16 @@ it only resolves and redirects. ### GET `/main` -Current main preview, sourced from an R2 object HEAD (no GitHub API call). +Current main preview. `download_url`, `digest`, `commit_sha`, +`size_bytes`, and `last_modified` are sourced from an R2 object HEAD (no +GitHub API call). `run_id`, `artifact_id`, `created_at`, and `expires_at` +are enrichment: resolved from that commit's GitHub Actions artifact (same +lookup `GET /commit/{sha}` uses) purely for shape parity with +`ArtifactPreview`, so a client reading either response doesn't have to +special-case field availability. That enrichment is best-effort and never +load-bearing - if the commit has no known artifact (e.g. it's aged out of +GitHub's 14-day retention) or GitHub is unavailable, those four fields are +just `null`; the response still succeeds with everything R2-sourced intact. **Response:** @@ -42,6 +54,11 @@ Current main preview, sourced from an R2 object HEAD (no GitHub API call). "result": { "commit_sha": "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", "short_sha": "a1b2c3d", + "pr_number": null, + "run_id": 999999, + "artifact_id": 555555, + "created_at": "2026-08-13T10:00:00Z", + "expires_at": "2026-08-27T10:00:00Z", "digest": "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", "size_bytes": 31229553, "last_modified": "2026-08-13T13:11:41.000Z", @@ -54,7 +71,10 @@ Current main preview, sourced from an R2 object HEAD (no GitHub API call). `commit_sha` and `digest` come straight from the R2 object's `commit-sha`/ `digest` custom metadata (`digest` already carries the `sha256:` prefix) - both are `null` if that object has no custom metadata (e.g. it predates the -CI job setting it). +CI job setting it), which also means the GitHub Actions enrichment above is +skipped entirely (nothing to look up by). `source` stays `"r2"` regardless +of whether the enrichment resolved - it describes where `download_url`/ +`digest` come from, which never changes. ### GET `/main/download` diff --git a/src/services/previews/v1/routes/main.ts b/src/services/previews/v1/routes/main.ts index 2fe133e..299cc27 100644 --- a/src/services/previews/v1/routes/main.ts +++ b/src/services/previews/v1/routes/main.ts @@ -6,12 +6,43 @@ import { errorResponse } from "../schemas/previews"; import { getMainPreviewObject } from "../r2"; +import { findPreviewArtifactByCommitSha } from "../github/artifacts"; import { notFoundBody } from "./errors"; import { PreviewsV1App } from "./app"; const MAIN_CACHE_KEY = "preview:main"; const MAIN_CACHE_TTL_SECONDS = 60; +// Enrichment only - run_id/artifact_id/created_at/expires_at come from +// that commit's GitHub Actions artifact when resolvable. A miss for any +// reason (no commit_sha yet, artifact expired, GitHub unavailable) just +// leaves them null; it never fails or degrades the response, since +// download_url/digest below are R2-sourced and don't depend on this. +async function resolveArtifactFields( + githubToken: string, + commitSha: string | null +): Promise< + Pick +> { + const empty = { + run_id: null, + artifact_id: null, + created_at: null, + expires_at: null + }; + if (!commitSha) return empty; + + const artifact = await findPreviewArtifactByCommitSha(githubToken, commitSha); + if (artifact.status !== "found") return empty; + + return { + run_id: artifact.data.runId, + artifact_id: artifact.data.artifactId, + created_at: artifact.data.createdAt, + expires_at: artifact.data.expiresAt + }; +} + // Shared by /main and /main/download - both need the same cache-then-R2 // lookup, just to different ends (the full body vs. only download_url). async function resolveMainPreview( @@ -25,9 +56,16 @@ async function resolveMainPreview( const object = await getMainPreviewObject(c.env.PREVIEW_BUCKET); if (!object) return null; + const artifactFields = await resolveArtifactFields( + c.env.GITHUB_TOKEN, + object.commitSha + ); + const result: MainPreview = { commit_sha: object.commitSha, short_sha: object.commitSha?.slice(0, 7) ?? null, + pr_number: null, + ...artifactFields, digest: object.digest, size_bytes: object.sizeBytes, last_modified: object.lastModified, diff --git a/src/services/previews/v1/schemas/previews.ts b/src/services/previews/v1/schemas/previews.ts index 492353b..0b287c5 100644 --- a/src/services/previews/v1/schemas/previews.ts +++ b/src/services/previews/v1/schemas/previews.ts @@ -45,6 +45,18 @@ const MainPreviewSchema = z .object({ commit_sha: z.string().nullable(), short_sha: z.string().nullable(), + // Always null - main isn't a PR. Present for shape parity with + // ArtifactPreview. + pr_number: z.number().nullable(), + // Enrichment from that commit's GitHub Actions artifact, when + // resolvable - null if the commit has no known artifact (e.g. expired + // past GitHub's 14-day retention) or GitHub is unavailable. Never + // blocks or degrades the response: download_url/digest below are the + // load-bearing, R2-sourced fields and don't depend on this resolving. + run_id: z.number().nullable(), + artifact_id: z.number().nullable(), + created_at: z.string().nullable(), + expires_at: z.string().nullable(), digest: z.string().nullable(), size_bytes: z.number(), last_modified: z.string(), diff --git a/test/services/previews/v1/main.test.ts b/test/services/previews/v1/main.test.ts index 6eb6924..b268967 100644 --- a/test/services/previews/v1/main.test.ts +++ b/test/services/previews/v1/main.test.ts @@ -1,12 +1,36 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { createExecutionContext, waitOnExecutionContext } from "cloudflare:test"; import { env } from "cloudflare:workers"; import app from "../../../../src/app"; +import { MockGitHubRequest } from "../../../utils/test-types"; +import { suppressConsole } from "../../../utils/mock-helpers"; + +vi.mock("@octokit/request", async () => + (await import("../../../mocks/octokit")).octokitRequestMock() +); + +import { request as ghRequest } from "@octokit/request"; const MAIN_PREVIEW_KEY = "FOSSBilling-preview.zip"; +const COMMIT_SHA = "abc1234567890abc1234567890abc1234567890"; + +const SAMPLE_ARTIFACTS = { + total_count: 1, + artifacts: [ + { + id: 555, + size_in_bytes: 12345, + created_at: "2026-08-13T10:00:00Z", + expires_at: "2026-08-27T10:00:00Z", + expired: false, + digest: "sha256:deadbeef", + workflow_run: { id: 999, head_sha: COMMIT_SHA } + } + ] +}; async function get(path: string) { const ctx = createExecutionContext(); @@ -15,10 +39,19 @@ async function get(path: string) { return res; } +let restoreConsole: (() => void) | null = null; + describe("Previews API v1 - GET /previews/v1/main", () => { beforeEach(async () => { + restoreConsole = suppressConsole(); await env.CACHE_KV.delete("preview:main"); await env.PREVIEW_BUCKET.delete(MAIN_PREVIEW_KEY); + vi.clearAllMocks(); + }); + + afterEach(() => { + restoreConsole?.(); + restoreConsole = null; }); it("returns 404 when no main preview has been published", async () => { @@ -33,7 +66,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { customMetadata: { digest: "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", - "commit-sha": "abc1234567890abc1234567890abc1234567890" + "commit-sha": COMMIT_SHA } }); @@ -50,9 +83,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { }; }; - expect(body.result.commit_sha).toBe( - "abc1234567890abc1234567890abc1234567890" - ); + expect(body.result.commit_sha).toBe(COMMIT_SHA); expect(body.result.short_sha).toBe("abc1234"); expect(body.result.digest).toBe( "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08" @@ -70,16 +101,88 @@ describe("Previews API v1 - GET /previews/v1/main", () => { const res = await get("/previews/v1/main"); expect(res.status).toBe(200); const body = (await res.json()) as { - result: { commit_sha: string | null; digest: string | null }; + result: { + commit_sha: string | null; + digest: string | null; + run_id: number | null; + }; }; expect(body.result.commit_sha).toBeNull(); expect(body.result.digest).toBeNull(); + // No commit_sha means there's nothing to look up an artifact by. + expect(body.result.run_id).toBeNull(); + expect(ghRequest).not.toHaveBeenCalled(); + }); + + it("enriches with that commit's GitHub Actions artifact when resolvable", async () => { + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + customMetadata: { "commit-sha": COMMIT_SHA } + }); + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ data: SAMPLE_ARTIFACTS }) + ); + + const res = await get("/previews/v1/main"); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { + pr_number: number | null; + run_id: number | null; + artifact_id: number | null; + created_at: string | null; + expires_at: string | null; + source: string; + }; + }; + expect(body.result.pr_number).toBeNull(); + expect(body.result.run_id).toBe(999); + expect(body.result.artifact_id).toBe(555); + expect(body.result.created_at).toBe("2026-08-13T10:00:00Z"); + expect(body.result.expires_at).toBe("2026-08-27T10:00:00Z"); + // download_url/digest stay R2-sourced regardless of the enrichment. + expect(body.result.source).toBe("r2"); }); - it("serves the second request from CACHE_KV without re-reading R2", async () => { + it("still succeeds with null enrichment fields when GitHub is unavailable", async () => { + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + customMetadata: { "commit-sha": COMMIT_SHA } + }); + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation(async () => { + throw Object.assign(new Error("Service Unavailable"), { + status: 502 + }); + }); + + const res = await get("/previews/v1/main"); + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { run_id: number | null; digest: string | null }; + }; + expect(body.result.run_id).toBeNull(); + }); + + it("still succeeds with null enrichment fields when the commit has no known artifact", async () => { + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + customMetadata: { "commit-sha": COMMIT_SHA } + }); + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ data: { total_count: 0, artifacts: [] } }) + ); + + const res = await get("/previews/v1/main"); + expect(res.status).toBe(200); + const body = (await res.json()) as { result: { run_id: number | null } }; + expect(body.result.run_id).toBeNull(); + }); + + it("serves the second request from CACHE_KV without re-reading R2 or GitHub", async () => { await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "v1", { customMetadata: { "commit-sha": "111" } }); + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ data: { total_count: 0, artifacts: [] } }) + ); + const first = await get("/previews/v1/main"); expect( ((await first.json()) as { result: { commit_sha: string } }).result @@ -96,13 +199,24 @@ describe("Previews API v1 - GET /previews/v1/main", () => { result: { commit_sha: string }; }; expect(secondBody.result.commit_sha).toBe("111"); + expect(ghRequest).toHaveBeenCalledTimes(1); }); }); describe("Previews API v1 - GET /previews/v1/main/download", () => { beforeEach(async () => { + restoreConsole = suppressConsole(); await env.CACHE_KV.delete("preview:main"); await env.PREVIEW_BUCKET.delete(MAIN_PREVIEW_KEY); + vi.clearAllMocks(); + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ data: { total_count: 0, artifacts: [] } }) + ); + }); + + afterEach(() => { + restoreConsole?.(); + restoreConsole = null; }); it("returns 404 when no main preview has been published", async () => { From 9b25f9ecb27a929e9e1cc838dc31b66ff7b95fb2 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Thu, 13 Aug 2026 19:47:37 +0100 Subject: [PATCH 10/17] Fix review findings: fork-PR resolution, TTL/cache correctness, docs All seven findings verified as real before fixing (one against live GitHub API/PR data, one against GitHub's own docs) rather than taken on faith. In severity order: P1 - pr.ts: /pr/{number} used the PR's real head SHA to look up an artifact named from $GITHUB_SHA, but GitHub's pull_request event sets $GITHUB_SHA to the ephemeral merge commit, not the head SHA (confirmed against GitHub's own docs, then empirically: cross-referenced a real merged PR's head.sha against its actual build artifact's embedded SHA). Since ci.yml's fork-only pull_request job is the only one exposed to this - same-repo PRs take the push-triggered path, which has no merge-commit substitution - this broke exactly the audience preview links exist for: external contributors. Fixed in findPreviewArtifactByCommitSha: try the exact artifact name first (fast, correct for push-triggered builds), fall back to listing every preview artifact and matching by the triggering run's real head_sha - a field GitHub populates accurately regardless of what $GITHUB_SHA the job saw - when that misses. New tests reproduce the actual fork-PR mismatch at both the /commit/{sha} and /pr/{number} layer. P2 - github/artifacts.ts: an uppercase SHA queried an uppercase artifact name that never exists (CI always builds lowercase short SHAs). Fixed by lowercasing once at the top of findPreviewArtifactByCommitSha and using that consistently for both the query and the match. P2 - main.ts: an uncaught JSON.parse on a corrupt preview:main cache entry 500'd both /main endpoints until the entry expired. Now caught and falls through to a fresh R2 lookup, matching cachedLookup()'s existing handling of the same situation. P2 - commit.ts/cache.ts: a lookup resolved shortly before its artifact's GitHub retention expires got the full 3600s TTL, so /commit/{sha} could keep serving stale 200 metadata for up to an hour after the artifact actually expired (while /commit/{sha}/download, resolved live, would already 404). cachedLookup's ttlSeconds can now be a function of the resolved data; commit.ts caps the cache lifetime at min(3600, secondsUntilExpiry). Below KV's 60s minimum TTL, the result is returned but intentionally left uncached rather than rounded up past its real expiry or erroring on an invalid TTL. P2 - schemas/previews.ts: ErrorResponseSchema didn't declare the `details` field the defaultHook actually attaches to 422 bodies, so generated OpenAPI clients saw a different shape than what's served. Added as optional, mirroring extensions/v2's schemas/common.ts. P3 - previews/v1 README: the caching note wrongly attributed /pr/{number}/download to preview:commit:{sha}'s 3600s cache; it's actually keyed on preview:pr:{number} at the 60s default like its own metadata route. Corrected. P3 - root README: "three main services" was stale now that Previews is a fourth. Updated. Verification: 518 + 47 tests (11 new), lint and typecheck clean. --- README.md | 2 +- src/services/previews/v1/README.md | 13 +- src/services/previews/v1/cache.ts | 25 ++- src/services/previews/v1/github/artifacts.ts | 154 ++++++++++++------- src/services/previews/v1/routes/commit.ts | 19 ++- src/services/previews/v1/routes/main.ts | 7 +- src/services/previews/v1/schemas/previews.ts | 11 +- test/services/previews/v1/cache.test.ts | 47 ++++++ test/services/previews/v1/commit.test.ts | 104 +++++++++++++ test/services/previews/v1/main.test.ts | 18 ++- test/services/previews/v1/pr.test.ts | 43 ++++++ 11 files changed, 372 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index 78b2bc1..9cc8109 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Everything is built on [Hono](https://hono.dev), making it lightweight and fast. ## What it does -The worker exposes three main services: +The worker exposes four main services: - **Versions Service** (`/versions/v1`) The source of truth for FOSSBilling updates. It fetches release data from GitHub, caches it for performance, and helps instances decide if they need to update. diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md index 3d1daba..3d5c8fd 100644 --- a/src/services/previews/v1/README.md +++ b/src/services/previews/v1/README.md @@ -145,13 +145,14 @@ etc., surfaced as 429/503/500 depending on severity). - Responses are cached in `CACHE_KV`, only for successful lookups - a not-yet-built PR or a transient GitHub error always re-resolves on the - next request. `GET /pr/{number}` (`preview:pr:{number}`) and `GET /main` - (`preview:main`) use the 60s default, matching how often a moving - pointer can realistically change. `GET /commit/{sha}` - (`preview:commit:{sha}`, also used by `/commit/{sha}/download` and + next request. `GET /pr/{number}` (`preview:pr:{number}`, also used by `/pr/{number}/download` to avoid re-resolving what the metadata route - already cached) uses 3600s instead - a commit's build never changes once - it exists, so there's no correctness reason to re-check it every minute. + already cached) and `GET /main` (`preview:main`) use the 60s default, + matching how often a moving pointer can realistically change. + `GET /commit/{sha}` (`preview:commit:{sha}`, likewise shared with + `/commit/{sha}/download`) uses 3600s instead - a commit's build never + changes once it exists, so there's no correctness reason to re-check it + every minute. - `GET /pr/{number}/download` and `GET /commit/{sha}/download` always resolve GitHub's signed redirect URL live, never cached - it expires in about a minute, and Cloudflare KV's 60s minimum TTL leaves no safe margin diff --git a/src/services/previews/v1/cache.ts b/src/services/previews/v1/cache.ts index 6c40177..fb03e9d 100644 --- a/src/services/previews/v1/cache.ts +++ b/src/services/previews/v1/cache.ts @@ -8,14 +8,27 @@ import { GithubLookupResult } from "./github/artifacts"; // longer ttlSeconds explicitly - see routes/commit.ts and routes/respond.ts. export const DEFAULT_CACHE_TTL_SECONDS = 60; +// Cloudflare KV's own floor - a shorter expirationTtl is a 400 at the API +// level, not just an app-level policy choice. +const KV_MIN_TTL_SECONDS = 60; + // Only "found" results are cached. "not_found"/"unavailable" always // re-resolve, so a transient GitHub hiccup or a not-yet-built PR doesn't // get stuck negative for the TTL window. +// +// ttlSeconds may be a function of the resolved data instead of a fixed +// number - see routes/commit.ts, which caps the cache lifetime at the +// artifact's own remaining GitHub retention so a lookup resolved just +// before expiry doesn't outlive it and keep serving a 200 after GitHub +// itself has started 404ing. If the computed TTL is under KV's 60s floor, +// the result is returned but not cached at all - better to re-resolve +// live for the rest of that final minute than to either violate the floor +// or round up and cache something past its real expiry. export async function cachedLookup( kv: KVNamespace, key: string, resolve: () => Promise>, - ttlSeconds: number = DEFAULT_CACHE_TTL_SECONDS + ttlSeconds: number | ((data: T) => number) = DEFAULT_CACHE_TTL_SECONDS ): Promise> { const cached = await kv.get(key); if (cached !== null) { @@ -28,9 +41,13 @@ export async function cachedLookup( const result = await resolve(); if (result.status === "found") { - await kv.put(key, JSON.stringify(result.data), { - expirationTtl: ttlSeconds - }); + const ttl = + typeof ttlSeconds === "function" ? ttlSeconds(result.data) : ttlSeconds; + if (ttl >= KV_MIN_TTL_SECONDS) { + await kv.put(key, JSON.stringify(result.data), { + expirationTtl: ttl + }); + } } return result; } diff --git a/src/services/previews/v1/github/artifacts.ts b/src/services/previews/v1/github/artifacts.ts index 13fa778..b9b9df2 100644 --- a/src/services/previews/v1/github/artifacts.ts +++ b/src/services/previews/v1/github/artifacts.ts @@ -8,16 +8,15 @@ import { logWarn } from "../../../../lib/logger"; const REPO_OWNER = "FOSSBilling"; const REPO_NAME = "FOSSBilling"; +const ARTIFACT_NAME_PREFIX = "FOSSBilling-preview-"; // FOSSBilling/FOSSBilling's ci.yml uploads one artifact per commit for -// every PR build, non-main branch push, and main push - named after that -// commit's short SHA (archive: false, so the file's own basename becomes -// the artifact name - see upload-preview in that repo's -// .github/workflows/ci.yml). Querying by the exact name this produces -// returns at most the handful of runs that ever targeted this one commit, -// rather than every preview artifact in the retention window. -function artifactNameForSha(sha: string): string { - return `FOSSBilling-preview-${sha.slice(0, 7)}.zip`; +// every PR build, non-main branch push, and main push - named after the +// short form of $GITHUB_SHA at build time (archive: false, so the file's +// own basename becomes the artifact name). Expects an already-lowercased +// sha - see findPreviewArtifactByCommitSha, which is the only caller. +function artifactNameForSha(shaLower: string): string { + return `${ARTIFACT_NAME_PREFIX}${shaLower.slice(0, 7)}.zip`; } export interface PreviewArtifact { @@ -37,6 +36,7 @@ export type GithubLookupResult = interface RawArtifact { id: number; + name?: string; size_in_bytes: number; created_at: string | null; expires_at: string | null; @@ -61,67 +61,111 @@ function unavailable( return { status: "unavailable", error: githubError }; } -// Queries the exact artifact name this commit's build would have produced -// (see artifactNameForSha) and returns the newest non-expired match. The -// head_sha check below is defense against a short-SHA collision, not the -// primary matching mechanism - the name filter already does that. +async function listArtifacts( + githubToken: string, + name: string | undefined +): Promise { + const result = await ghRequest( + "GET /repos/{owner}/{repo}/actions/artifacts", + { + owner: REPO_OWNER, + repo: REPO_NAME, + ...(name ? { name } : {}), + per_page: 100, + headers: { Authorization: `Bearer ${githubToken}` } + } + ); + return result.data.artifacts as RawArtifact[]; +} + +// Newest non-expired artifact whose triggering run's real head commit +// matches shaLower. shaLower may be a short (7+ char) prefix, so this is +// a startsWith rather than an exact match. +function matchArtifact( + artifacts: RawArtifact[], + shaLower: string +): { artifact: RawArtifact; runId: number; headSha: string } | null { + let match: { artifact: RawArtifact; runId: number; headSha: string } | null = + null; + + for (const artifact of artifacts) { + const workflowRun = artifact.workflow_run; + if (artifact.expired || !workflowRun) continue; + if (!workflowRun.head_sha.toLowerCase().startsWith(shaLower)) continue; + if ( + !match || + (artifact.created_at ?? "") > (match.artifact.created_at ?? "") + ) { + match = { + artifact, + runId: workflowRun.id, + headSha: workflowRun.head_sha + }; + } + } + + return match; +} + +function toPreviewArtifact(match: { + artifact: RawArtifact; + runId: number; + headSha: string; +}): PreviewArtifact { + return { + runId: match.runId, + artifactId: match.artifact.id, + commitSha: match.headSha, + digest: match.artifact.digest ?? null, + sizeBytes: match.artifact.size_in_bytes, + createdAt: match.artifact.created_at ?? "", + expiresAt: match.artifact.expires_at ?? "" + }; +} + +// Resolves the preview artifact for a commit. Tries the exact artifact +// name first (artifactNameForSha) - correct and cheap for push-triggered +// builds (main, branch pushes, and same-repo PRs, which take the push +// path since preview-build-pr is fork-only), where $GITHUB_SHA in CI is +// the actual pushed commit. +// +// Falls back to listing every preview artifact and matching by the +// triggering run's real head_sha if that misses. This is what makes fork +// PRs resolve correctly: GitHub's pull_request event makes $GITHUB_SHA the +// ephemeral merge commit rather than the PR's real head commit (see +// https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request), +// so preview-build-pr names its artifact after a SHA this service never +// asks about - only the run's own head_sha metadata (populated by GitHub +// independently of what the job saw as $GITHUB_SHA) still says which +// commit it actually is. export async function findPreviewArtifactByCommitSha( githubToken: string, sha: string ): Promise> { + const shaLower = sha.toLowerCase(); const url = `https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/actions/artifacts`; try { - const result = await ghRequest( - "GET /repos/{owner}/{repo}/actions/artifacts", - { - owner: REPO_OWNER, - repo: REPO_NAME, - name: artifactNameForSha(sha), - per_page: 100, - headers: { Authorization: `Bearer ${githubToken}` } - } + const exact = await listArtifacts( + githubToken, + artifactNameForSha(shaLower) ); + let match = matchArtifact(exact, shaLower); - const artifacts = result.data.artifacts as RawArtifact[]; - const shaLower = sha.toLowerCase(); - let match: { - artifact: RawArtifact; - runId: number; - headSha: string; - } | null = null; - - for (const artifact of artifacts) { - const workflowRun = artifact.workflow_run; - if (artifact.expired || !workflowRun) continue; - if (!workflowRun.head_sha.toLowerCase().startsWith(shaLower)) continue; - if ( - !match || - (artifact.created_at ?? "") > (match.artifact.created_at ?? "") - ) { - match = { - artifact, - runId: workflowRun.id, - headSha: workflowRun.head_sha - }; - } + if (!match) { + const all = await listArtifacts(githubToken, undefined); + match = matchArtifact( + all.filter((artifact) => + artifact.name?.startsWith(ARTIFACT_NAME_PREFIX) + ), + shaLower + ); } if (!match) { return { status: "not_found" }; } - return { - status: "found", - data: { - runId: match.runId, - artifactId: match.artifact.id, - commitSha: match.headSha, - digest: match.artifact.digest ?? null, - sizeBytes: match.artifact.size_in_bytes, - createdAt: match.artifact.created_at ?? "", - expiresAt: match.artifact.expires_at ?? "" - } - }; + return { status: "found", data: toPreviewArtifact(match) }; } catch (error) { return unavailable("Preview artifact lookup", error, url); } diff --git a/src/services/previews/v1/routes/commit.ts b/src/services/previews/v1/routes/commit.ts index de46d2d..4d719c1 100644 --- a/src/services/previews/v1/routes/commit.ts +++ b/src/services/previews/v1/routes/commit.ts @@ -1,5 +1,6 @@ import { createRoute } from "@hono/zod-openapi"; import { + ArtifactPreview, ArtifactPreviewResponseSchema, CommitShaParamSchema, errorResponse @@ -17,6 +18,20 @@ const COMMIT_CACHE_TTL_SECONDS = 3600; const cacheKeyForSha = (sha: string) => `preview:commit:${sha.toLowerCase()}`; +// Caps the cache lifetime at the artifact's own remaining GitHub retention +// - a lookup resolved shortly before an artifact expires must not be +// cached for the full 3600s, or /commit/{sha} would keep serving a 200 +// with stale metadata for up to an hour after GitHub itself starts +// 404ing (which respondWithDownloadRedirect's live resolution already +// would - see cache.ts for what happens when this comes out under KV's +// 60s minimum TTL). +function ttlForArtifact(artifact: ArtifactPreview): number { + const remainingSeconds = Math.floor( + (new Date(artifact.expires_at).getTime() - Date.now()) / 1000 + ); + return Math.min(COMMIT_CACHE_TTL_SECONDS, remainingSeconds); +} + export function registerCommitRoutes(app: PreviewsV1App): void { const commitRoute = createRoute({ method: "get", @@ -47,7 +62,7 @@ export function registerCommitRoutes(app: PreviewsV1App): void { c.env.CACHE_KV, cacheKeyForSha(sha), () => resolveArtifactPreview(githubToken, sha, null), - COMMIT_CACHE_TTL_SECONDS + ttlForArtifact ); return respondWithLookup( @@ -85,7 +100,7 @@ export function registerCommitRoutes(app: PreviewsV1App): void { c.env.CACHE_KV, cacheKeyForSha(sha), () => resolveArtifactPreview(githubToken, sha, null), - COMMIT_CACHE_TTL_SECONDS + ttlForArtifact ); return respondWithDownloadRedirect( c, diff --git a/src/services/previews/v1/routes/main.ts b/src/services/previews/v1/routes/main.ts index 299cc27..7a2b342 100644 --- a/src/services/previews/v1/routes/main.ts +++ b/src/services/previews/v1/routes/main.ts @@ -50,7 +50,12 @@ async function resolveMainPreview( ): Promise { const cached = await c.env.CACHE_KV.get(MAIN_CACHE_KEY); if (cached) { - return JSON.parse(cached) as MainPreview; + try { + return JSON.parse(cached) as MainPreview; + } catch { + // Corrupt cache entry - fall through to a fresh R2 lookup, matching + // cachedLookup()'s handling of the same situation. + } } const object = await getMainPreviewObject(c.env.PREVIEW_BUCKET); diff --git a/src/services/previews/v1/schemas/previews.ts b/src/services/previews/v1/schemas/previews.ts index 0b287c5..c322bbf 100644 --- a/src/services/previews/v1/schemas/previews.ts +++ b/src/services/previews/v1/schemas/previews.ts @@ -4,7 +4,16 @@ export const ErrorResponseSchema = z .object({ error: z.object({ message: z.string(), - code: z.string() + code: z.string(), + // Only present on 422s - index.ts's defaultHook attaches the zod + // validation issues here for VALIDATION_ERROR responses. + details: z + .array( + z.unknown().openapi({ + type: ["string", "number", "boolean", "object", "array", "null"] + }) + ) + .optional() }) }) .openapi("Error"); diff --git a/test/services/previews/v1/cache.test.ts b/test/services/previews/v1/cache.test.ts index 86d7fed..4c6b9ca 100644 --- a/test/services/previews/v1/cache.test.ts +++ b/test/services/previews/v1/cache.test.ts @@ -58,4 +58,51 @@ describe("previews/v1 cachedLookup", () => { expect(second).toEqual({ status: "found", data: "v1" }); expect(resolve).toHaveBeenCalledTimes(1); }); + + it("falls back to a fresh resolve on a corrupt cache entry", async () => { + await env.CACHE_KV.put("test-key", "not valid json{"); + const resolve = vi.fn().mockResolvedValue({ status: "found", data: "v1" }); + + const result = await cachedLookup(env.CACHE_KV, "test-key", resolve); + + expect(result).toEqual({ status: "found", data: "v1" }); + expect(resolve).toHaveBeenCalledTimes(1); + }); + + it("computes the TTL from the resolved data when given a function", async () => { + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + const ttlFor = vi.fn((data: { value: string }) => + data.value === "value" ? 120 : 60 + ); + + await cachedLookup( + env.CACHE_KV, + "test-key", + async () => ({ status: "found", data: { value: "value" } }), + ttlFor + ); + + expect(ttlFor).toHaveBeenCalledWith({ value: "value" }); + expect(putSpy).toHaveBeenCalledWith( + "test-key", + JSON.stringify({ value: "value" }), + { expirationTtl: 120 } + ); + putSpy.mockRestore(); + }); + + it("skips caching entirely when the computed TTL is under KV's 60s floor", async () => { + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + + const result = await cachedLookup( + env.CACHE_KV, + "test-key", + async () => ({ status: "found", data: "value" }), + () => 30 + ); + + expect(result).toEqual({ status: "found", data: "value" }); + expect(putSpy).not.toHaveBeenCalled(); + putSpy.mockRestore(); + }); }); diff --git a/test/services/previews/v1/commit.test.ts b/test/services/previews/v1/commit.test.ts index 29003ad..9b0bbdf 100644 --- a/test/services/previews/v1/commit.test.ts +++ b/test/services/previews/v1/commit.test.ts @@ -222,4 +222,108 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { ); putSpy.mockRestore(); }); + + it("caps the cache TTL at the artifact's own remaining GitHub retention", async () => { + // Expires in ~500s - well under the 3600s default, so the capped + // value (not 3600) must be what's actually written. + const expiresAt = new Date(Date.now() + 500_000).toISOString(); + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ + data: { + total_count: 1, + artifacts: [ + { ...SAMPLE_ARTIFACTS.artifacts[0], expires_at: expiresAt } + ] + } + }) + ); + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + + await get(`/previews/v1/commit/${SHA}`); + + expect(putSpy).toHaveBeenCalledTimes(1); + const ttl = (putSpy.mock.calls[0][2] as { expirationTtl: number }) + .expirationTtl; + expect(ttl).toBeGreaterThan(400); + expect(ttl).toBeLessThanOrEqual(500); + putSpy.mockRestore(); + }); + + it("skips caching when the artifact expires within KV's 60s minimum TTL", async () => { + const expiresAt = new Date(Date.now() + 30_000).toISOString(); + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ + data: { + total_count: 1, + artifacts: [ + { ...SAMPLE_ARTIFACTS.artifacts[0], expires_at: expiresAt } + ] + } + }) + ); + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + + const res = await get(`/previews/v1/commit/${SHA}`); + + expect(res.status).toBe(200); + expect(putSpy).not.toHaveBeenCalled(); + putSpy.mockRestore(); + }); + + it("resolves an uppercase SHA by querying the lowercased artifact name", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ data: SAMPLE_ARTIFACTS }) + ); + + const res = await get(`/previews/v1/commit/${SHA.toUpperCase()}`); + + expect(res.status).toBe(200); + expect(ghRequest).toHaveBeenCalledWith( + "GET /repos/{owner}/{repo}/actions/artifacts", + expect.objectContaining({ + name: `FOSSBilling-preview-${SHA.slice(0, 7)}.zip` + }) + ); + }); + + it("falls back to a broad scan when the exact artifact name misses (fork PR merge-SHA mismatch)", async () => { + // Simulates a fork PR: CI named the artifact after the pull_request + // event's ephemeral merge commit ("deadbeef..."), not the PR's real + // head SHA (SHA) - so the exact-name query for SHA's derived name + // returns nothing, and only a name-less scan (filtered by the run's + // real head_sha) finds it. + const mergeShaArtifact = { + id: 777, + name: "FOSSBilling-preview-deadbee.zip", + size_in_bytes: 99, + created_at: "2026-08-13T11:00:00Z", + expires_at: "2026-08-27T11:00:00Z", + expired: false, + digest: "sha256:fromfork", + workflow_run: { id: 888, head_sha: SHA } + }; + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async (route: string, params?: { name?: string }) => { + if (route !== "GET /repos/{owner}/{repo}/actions/artifacts") { + throw new Error(`Unexpected route: ${route}`); + } + if (params?.name) { + // The exact-name fast path - misses. + return { data: { total_count: 0, artifacts: [] } }; + } + // The fallback broad scan. + return { data: { total_count: 1, artifacts: [mergeShaArtifact] } }; + } + ); + + const res = await get(`/previews/v1/commit/${SHA}`); + + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { artifact_id: number; digest: string | null }; + }; + expect(body.result.artifact_id).toBe(777); + expect(body.result.digest).toBe("sha256:fromfork"); + expect(ghRequest).toHaveBeenCalledTimes(2); + }); }); diff --git a/test/services/previews/v1/main.test.ts b/test/services/previews/v1/main.test.ts index b268967..c1eadb5 100644 --- a/test/services/previews/v1/main.test.ts +++ b/test/services/previews/v1/main.test.ts @@ -199,7 +199,23 @@ describe("Previews API v1 - GET /previews/v1/main", () => { result: { commit_sha: string }; }; expect(secondBody.result.commit_sha).toBe("111"); - expect(ghRequest).toHaveBeenCalledTimes(1); + // 2, not 1: findPreviewArtifactByCommitSha's exact-name query misses + // (no artifact was mocked), so it falls back to a second, broader + // query before giving up - both happen on the first /main request + // only, since the second is served entirely from cache. + expect(ghRequest).toHaveBeenCalledTimes(2); + }); + + it("falls back to R2 instead of erroring on a corrupt cache entry", async () => { + await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + customMetadata: { "commit-sha": COMMIT_SHA } + }); + await env.CACHE_KV.put("preview:main", "not valid json{"); + + const res = await get("/previews/v1/main"); + expect(res.status).toBe(200); + const body = (await res.json()) as { result: { commit_sha: string } }; + expect(body.result.commit_sha).toBe(COMMIT_SHA); }); }); diff --git a/test/services/previews/v1/pr.test.ts b/test/services/previews/v1/pr.test.ts index c69c115..3a987a8 100644 --- a/test/services/previews/v1/pr.test.ts +++ b/test/services/previews/v1/pr.test.ts @@ -193,4 +193,47 @@ describe("Previews API v1 - GET /previews/v1/pr/:number", () => { ); putSpy.mockRestore(); }); + + it("resolves a fork PR whose artifact was named from the merge SHA, not the head SHA", async () => { + // ci.yml's pull_request-triggered job (fork PRs only) names its + // artifact after $GITHUB_SHA, which GitHub sets to the ephemeral + // pull_request merge commit rather than the PR's real head commit - + // see the comment on findPreviewArtifactByCommitSha. The exact-name + // query built from the real head SHA (SHA) therefore misses, and only + // the fallback scan (matched by the run's real head_sha, unaffected by + // what name the artifact was given) finds it. + const mergeShaArtifact = { + id: 777, + name: "FOSSBilling-preview-deadbee.zip", + size_in_bytes: 99, + created_at: "2026-08-13T11:00:00Z", + expires_at: "2026-08-27T11:00:00Z", + expired: false, + digest: "sha256:fromfork", + workflow_run: { id: 888, head_sha: SHA } + }; + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async (route: string, params?: { name?: string }) => { + if (route === "GET /repos/{owner}/{repo}/pulls/{pull_number}") { + return { data: { head: { sha: SHA } } }; + } + if (route === "GET /repos/{owner}/{repo}/actions/artifacts") { + if (params?.name) { + return { data: { total_count: 0, artifacts: [] } }; + } + return { data: { total_count: 1, artifacts: [mergeShaArtifact] } }; + } + throw new Error(`Unexpected route: ${route}`); + } + ); + + const res = await get(`/previews/v1/pr/${PR_NUMBER}`); + + expect(res.status).toBe(200); + const body = (await res.json()) as { + result: { artifact_id: number; pr_number: number | null }; + }; + expect(body.result.artifact_id).toBe(777); + expect(body.result.pr_number).toBe(PR_NUMBER); + }); }); From 9c21738a01ed18ff36fbd64b6cd767a0a794a1d1 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sat, 15 Aug 2026 06:34:59 +0100 Subject: [PATCH 11/17] Fix second review pass: TTL edge cases and fallback pagination Follow-up review of the previous fix commit found 4 real edge cases in that fix itself, not the original bugs. All verified before fixing: P2 - github/artifacts.ts: the fork-PR fallback scan (added in the last commit) queried only one page (per_page: 100, no pagination), so a repo with more than 100 live preview artifacts would silently miss a genuine match sitting past page 1 - exactly defeating the fallback's purpose. findInFallbackPages() now pages through up to 5 pages (500 artifacts, well past what preview artifacts alone reach within GitHub's 14-day retention), stopping as soon as a match is found or the list runs out. P2 - commit.ts/cache.ts: two related gaps in the TTL-capping logic from the last commit: - A lookup resolved with e.g. exactly 60s of real artifact retention left could still write a KV entry that technically outlives the artifact by however long the kv.put() round-trip took, since the TTL was computed before that write, not after. ttlForArtifact now subtracts a 5s safety margin before capping. - toPreviewArtifact falls back to "" when GitHub's own expires_at is null, which made `new Date("").getTime()` (and everything downstream) NaN - cache.ts's `ttl >= 60` check is false for NaN, so caching was silently skipped forever for that entry instead of just this once. ttlForArtifact now detects this and falls back to the normal 3600s ceiling rather than a permanent no-cache state. P3 - previews/v1 README: documented the retention cap and the resulting final-minute-uncached window explicitly, rather than leaving it only inferable from the code. New tests: fallback pagination finding a match on page 2 and stopping there (no page 3 fetch), and expires_at: null falling back to the 3600s ceiling instead of silently disabling caching. Verification: 520 + 47 tests (4 new), lint and typecheck clean. --- src/services/previews/v1/README.md | 9 ++- src/services/previews/v1/github/artifacts.ts | 54 ++++++++++---- src/services/previews/v1/routes/commit.ts | 25 ++++++- test/services/previews/v1/commit.test.ts | 74 ++++++++++++++++++++ 4 files changed, 145 insertions(+), 17 deletions(-) diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md index 3d5c8fd..3c2b0f2 100644 --- a/src/services/previews/v1/README.md +++ b/src/services/previews/v1/README.md @@ -152,7 +152,14 @@ etc., surfaced as 429/503/500 depending on severity). `GET /commit/{sha}` (`preview:commit:{sha}`, likewise shared with `/commit/{sha}/download`) uses 3600s instead - a commit's build never changes once it exists, so there's no correctness reason to re-check it - every minute. + every minute. That 3600s is capped at the artifact's own remaining + GitHub retention (minus a small safety margin for the cache write + itself), so a lookup resolved near the end of an artifact's 14-day life + is never cached longer than the artifact actually exists. Within roughly + the final minute of that life the capped value falls under KV's 60s + minimum TTL, so those requests (and any more before the artifact expires + or a request refreshes it) are just served live instead of cached - a + short burst of extra GitHub calls right at the end, never stale data. - `GET /pr/{number}/download` and `GET /commit/{sha}/download` always resolve GitHub's signed redirect URL live, never cached - it expires in about a minute, and Cloudflare KV's 60s minimum TTL leaves no safe margin diff --git a/src/services/previews/v1/github/artifacts.ts b/src/services/previews/v1/github/artifacts.ts index b9b9df2..c2b49c0 100644 --- a/src/services/previews/v1/github/artifacts.ts +++ b/src/services/previews/v1/github/artifacts.ts @@ -63,7 +63,8 @@ function unavailable( async function listArtifacts( githubToken: string, - name: string | undefined + name: string | undefined, + page: number = 1 ): Promise { const result = await ghRequest( "GET /repos/{owner}/{repo}/actions/artifacts", @@ -72,12 +73,42 @@ async function listArtifacts( repo: REPO_NAME, ...(name ? { name } : {}), per_page: 100, + page, headers: { Authorization: `Bearer ${githubToken}` } } ); return result.data.artifacts as RawArtifact[]; } +// Bounds the fallback scan's worst case to 500 artifacts (5 pages x 100) +// rather than paginating through a repo's entire artifact history. Preview +// artifacts alone rarely approach that within GitHub's 14-day retention, +// even for an active repo. +const MAX_FALLBACK_PAGES = 5; + +// The fallback path (see findPreviewArtifactByCommitSha) can't filter +// server-side by name, so a repo with more than one page of live preview +// artifacts would silently miss a genuine match sitting on page 2+ with a +// single unpaginated call. Pages through until a match is found or the +// list runs out. +async function findInFallbackPages( + githubToken: string, + shaLower: string +): Promise<{ artifact: RawArtifact; runId: number; headSha: string } | null> { + for (let page = 1; page <= MAX_FALLBACK_PAGES; page++) { + const artifacts = await listArtifacts(githubToken, undefined, page); + const match = matchArtifact( + artifacts.filter((artifact) => + artifact.name?.startsWith(ARTIFACT_NAME_PREFIX) + ), + shaLower + ); + if (match) return match; + if (artifacts.length < 100) break; // last page + } + return null; +} + // Newest non-expired artifact whose triggering run's real head commit // matches shaLower. shaLower may be a short (7+ char) prefix, so this is // a startsWith rather than an exact match. @@ -129,15 +160,18 @@ function toPreviewArtifact(match: { // path since preview-build-pr is fork-only), where $GITHUB_SHA in CI is // the actual pushed commit. // -// Falls back to listing every preview artifact and matching by the -// triggering run's real head_sha if that misses. This is what makes fork -// PRs resolve correctly: GitHub's pull_request event makes $GITHUB_SHA the -// ephemeral merge commit rather than the PR's real head commit (see +// Falls back to paging through every preview artifact (findInFallbackPages) +// and matching by the triggering run's real head_sha if that misses. This +// is what makes fork PRs resolve correctly: GitHub's pull_request event +// makes $GITHUB_SHA the ephemeral merge commit rather than the PR's real +// head commit (see // https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#pull_request), // so preview-build-pr names its artifact after a SHA this service never // asks about - only the run's own head_sha metadata (populated by GitHub // independently of what the job saw as $GITHUB_SHA) still says which -// commit it actually is. +// commit it actually is. Can't filter this scan server-side by name (no +// exact name to filter by), so it has to page through results instead of +// trusting a single page holds the match. export async function findPreviewArtifactByCommitSha( githubToken: string, sha: string @@ -152,13 +186,7 @@ export async function findPreviewArtifactByCommitSha( let match = matchArtifact(exact, shaLower); if (!match) { - const all = await listArtifacts(githubToken, undefined); - match = matchArtifact( - all.filter((artifact) => - artifact.name?.startsWith(ARTIFACT_NAME_PREFIX) - ), - shaLower - ); + match = await findInFallbackPages(githubToken, shaLower); } if (!match) { diff --git a/src/services/previews/v1/routes/commit.ts b/src/services/previews/v1/routes/commit.ts index 4d719c1..eaf2667 100644 --- a/src/services/previews/v1/routes/commit.ts +++ b/src/services/previews/v1/routes/commit.ts @@ -18,18 +18,37 @@ const COMMIT_CACHE_TTL_SECONDS = 3600; const cacheKeyForSha = (sha: string) => `preview:commit:${sha.toLowerCase()}`; +// Subtracted from the computed TTL so the value we write already accounts +// for the round-trip between computing it here and cache.ts's kv.put() +// actually landing - without this, a lookup resolved with e.g. exactly +// 60s of real retention left could still get written with a TTL that +// technically outlives the artifact by however long that write took. +const WRITE_SAFETY_MARGIN_SECONDS = 5; + // Caps the cache lifetime at the artifact's own remaining GitHub retention // - a lookup resolved shortly before an artifact expires must not be // cached for the full 3600s, or /commit/{sha} would keep serving a 200 // with stale metadata for up to an hour after GitHub itself starts // 404ing (which respondWithDownloadRedirect's live resolution already -// would - see cache.ts for what happens when this comes out under KV's -// 60s minimum TTL). +// would). Within the final ~65s of an artifact's life this comes out +// under cache.ts's 60s KV floor, so that request (and any others until +// the artifact naturally falls out of GitHub's own list) is served live +// instead of cached - a short burst of extra GitHub calls right at the +// end of an artifact's life, never stale data. function ttlForArtifact(artifact: ArtifactPreview): number { const remainingSeconds = Math.floor( (new Date(artifact.expires_at).getTime() - Date.now()) / 1000 ); - return Math.min(COMMIT_CACHE_TTL_SECONDS, remainingSeconds); + // expires_at was empty/unparseable (toPreviewArtifact falls back to "" + // when GitHub's own value is null) - no real signal to cap against, so + // don't let a NaN here silently defeat caching on every request forever. + if (Number.isNaN(remainingSeconds)) { + return COMMIT_CACHE_TTL_SECONDS; + } + return Math.min( + COMMIT_CACHE_TTL_SECONDS, + remainingSeconds - WRITE_SAFETY_MARGIN_SECONDS + ); } export function registerCommitRoutes(app: PreviewsV1App): void { diff --git a/test/services/previews/v1/commit.test.ts b/test/services/previews/v1/commit.test.ts index 9b0bbdf..f7346d2 100644 --- a/test/services/previews/v1/commit.test.ts +++ b/test/services/previews/v1/commit.test.ts @@ -326,4 +326,78 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { expect(body.result.digest).toBe("sha256:fromfork"); expect(ghRequest).toHaveBeenCalledTimes(2); }); + + it("pages through the fallback scan when the match is past the first page", async () => { + const pageOne = Array.from({ length: 100 }, (_, i) => ({ + id: 1000 + i, + name: `FOSSBilling-preview-other${i}.zip`, + size_in_bytes: 1, + created_at: "2026-08-01T00:00:00Z", + expires_at: "2026-08-15T00:00:00Z", + expired: false, + digest: null, + workflow_run: { id: 1, head_sha: "0000000000000000000000000000000000000" } + })); + const pageTwoMatch = { + id: 2000, + name: "FOSSBilling-preview-deadbee.zip", + size_in_bytes: 99, + created_at: "2026-08-13T11:00:00Z", + expires_at: "2026-08-27T11:00:00Z", + expired: false, + digest: "sha256:page2", + workflow_run: { id: 888, head_sha: SHA } + }; + let fallbackCalls = 0; + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async (route: string, params?: { name?: string; page?: number }) => { + if (route !== "GET /repos/{owner}/{repo}/actions/artifacts") { + throw new Error(`Unexpected route: ${route}`); + } + if (params?.name) { + return { data: { total_count: 0, artifacts: [] } }; + } + fallbackCalls++; + if (params?.page === 1) { + return { data: { total_count: 101, artifacts: pageOne } }; + } + if (params?.page === 2) { + return { data: { total_count: 101, artifacts: [pageTwoMatch] } }; + } + throw new Error(`Unexpected page: ${params?.page}`); + } + ); + + const res = await get(`/previews/v1/commit/${SHA}`); + + expect(res.status).toBe(200); + const body = (await res.json()) as { result: { artifact_id: number } }; + expect(body.result.artifact_id).toBe(2000); + // Exact-name miss (1) + fallback page 1 (2) + fallback page 2 (3) - + // stops as soon as a match is found, no page 3. + expect(fallbackCalls).toBe(2); + expect(ghRequest).toHaveBeenCalledTimes(3); + }); + + it("falls back to the default TTL ceiling when expires_at can't be parsed", async () => { + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async () => ({ + data: { + total_count: 1, + artifacts: [{ ...SAMPLE_ARTIFACTS.artifacts[0], expires_at: null }] + } + }) + ); + const putSpy = vi.spyOn(env.CACHE_KV, "put"); + + const res = await get(`/previews/v1/commit/${SHA}`); + + expect(res.status).toBe(200); + expect(putSpy).toHaveBeenCalledWith( + `preview:commit:${SHA.toLowerCase()}`, + expect.any(String), + { expirationTtl: 3600 } + ); + putSpy.mockRestore(); + }); }); From 48a09fd293f485ccf9771e972caed523af9facff Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sat, 15 Aug 2026 06:41:46 +0100 Subject: [PATCH 12/17] Raise fallback pagination cap from a correctness bound to a circuit breaker The 5-page (500 artifact) cap on the fork-PR fallback scan wasn't just an optimization - that fallback is the source of truth for resolving fork PRs, so any hard cutoff there means a repo with more live preview artifacts than the cap can genuinely miss a valid match, trading the original false-not-found bug for a smaller version of the same bug. Raised to 50 pages (5,000 artifacts) and reframed as a circuit breaker rather than an expected limit: every GitHub Actions artifact expires after 14 days regardless of type, so a repo's total artifact count is inherently finite even for very active repos - this exists only to guarantee termination if the API ever doesn't behave as expected (never returns a short page), not because paging that deep is realistic. New test reproduces the reported scenario directly: a match on page 6, which the previous 5-page cap would have reported not_found for. Verification: 521 + 47 tests (1 new), lint and typecheck clean. --- src/services/previews/v1/github/artifacts.ts | 20 ++++--- test/services/previews/v1/commit.test.ts | 58 ++++++++++++++++++++ 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/services/previews/v1/github/artifacts.ts b/src/services/previews/v1/github/artifacts.ts index c2b49c0..d369ff6 100644 --- a/src/services/previews/v1/github/artifacts.ts +++ b/src/services/previews/v1/github/artifacts.ts @@ -80,17 +80,23 @@ async function listArtifacts( return result.data.artifacts as RawArtifact[]; } -// Bounds the fallback scan's worst case to 500 artifacts (5 pages x 100) -// rather than paginating through a repo's entire artifact history. Preview -// artifacts alone rarely approach that within GitHub's 14-day retention, -// even for an active repo. -const MAX_FALLBACK_PAGES = 5; +// A circuit breaker, not a correctness bound. This is the fallback's +// only source of truth for fork PRs - imposing a real cutoff here would +// just trade the original false-not-found bug for a smaller version of +// itself, missing a genuine match that happens to sit past page N. Every +// GitHub Actions artifact expires after 14 days regardless of type, so a +// repo's total artifact count is inherently finite even for very active +// repos; this exists only to guarantee termination if the API ever +// doesn't behave as expected (e.g. never returns a short page), not +// because 5,000 artifacts is a realistic amount to actually page through. +const MAX_FALLBACK_PAGES = 50; // The fallback path (see findPreviewArtifactByCommitSha) can't filter // server-side by name, so a repo with more than one page of live preview // artifacts would silently miss a genuine match sitting on page 2+ with a -// single unpaginated call. Pages through until a match is found or the -// list runs out. +// single unpaginated call. Pages through until GitHub returns a page +// short of per_page - the real "no more results" signal - or a match is +// found, whichever happens first. async function findInFallbackPages( githubToken: string, shaLower: string diff --git a/test/services/previews/v1/commit.test.ts b/test/services/previews/v1/commit.test.ts index f7346d2..d6071ae 100644 --- a/test/services/previews/v1/commit.test.ts +++ b/test/services/previews/v1/commit.test.ts @@ -379,6 +379,64 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { expect(ghRequest).toHaveBeenCalledTimes(3); }); + it("keeps paging past the old 5-page cap when the match is further back", async () => { + // Regression check: an earlier version of the fallback stopped after + // 5 pages (500 artifacts) as a hard cutoff, which would have reported + // this commit not_found even though its artifact genuinely exists - + // just on page 6. A repo with more than 500 live preview artifacts + // isn't hypothetical for an active project; the fallback is the + // source of truth for fork PRs and can't trade correctness for a + // fixed cutoff the way the fast exact-name path can. + const fullPage = (offset: number) => + Array.from({ length: 100 }, (_, i) => ({ + id: offset + i, + name: `FOSSBilling-preview-other${offset + i}.zip`, + size_in_bytes: 1, + created_at: "2026-08-01T00:00:00Z", + expires_at: "2026-08-15T00:00:00Z", + expired: false, + digest: null, + workflow_run: { + id: 1, + head_sha: "0000000000000000000000000000000000000" + } + })); + const page6Match = { + id: 9000, + name: "FOSSBilling-preview-deadbee.zip", + size_in_bytes: 99, + created_at: "2026-08-13T11:00:00Z", + expires_at: "2026-08-27T11:00:00Z", + expired: false, + digest: "sha256:page6", + workflow_run: { id: 888, head_sha: SHA } + }; + (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( + async (route: string, params?: { name?: string; page?: number }) => { + if (route !== "GET /repos/{owner}/{repo}/actions/artifacts") { + throw new Error(`Unexpected route: ${route}`); + } + if (params?.name) { + return { data: { total_count: 0, artifacts: [] } }; + } + const page = params?.page ?? 1; + if (page <= 5) { + return { data: { artifacts: fullPage(page * 1000) } }; + } + if (page === 6) { + return { data: { artifacts: [page6Match] } }; + } + throw new Error(`Unexpected page: ${page}`); + } + ); + + const res = await get(`/previews/v1/commit/${SHA}`); + + expect(res.status).toBe(200); + const body = (await res.json()) as { result: { artifact_id: number } }; + expect(body.result.artifact_id).toBe(9000); + }); + it("falls back to the default TTL ceiling when expires_at can't be parsed", async () => { (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( async () => ({ From 40116dead7a2dc0cf265722bc96dc1e9eb74c9e3 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sat, 15 Aug 2026 06:51:06 +0100 Subject: [PATCH 13/17] Deslop: extract repeated ArtifactMatch type, merge overlapping tests - github/artifacts.ts: a { artifact: RawArtifact; runId: number; headSha: string } object type was inlined four times (matchArtifact's local var and return type, findInFallbackPages' return type, toPreviewArtifact's param type). Named it ArtifactMatch once. - commit.test.ts: 'pages through the fallback scan when the match is past the first page' and 'keeps paging past the old 5-page cap' had become near-duplicates after the pagination-cap fix - the second is a strict superset of what the first proved (pagination past page 1, finding a match further back, stopping once found), just with a deeper page and the old cap's exact regression scenario. Merged into one test, keeping the stop-on-match call-count assertion the first test had that the second didn't, cutting ~90 lines of duplicated fixture-building code. No behavior change - 520 + 47 tests (41 in previews/v1, one fewer than before since the redundant test merged rather than being duplicated), lint and typecheck clean. --- src/services/previews/v1/github/artifacts.ts | 19 ++++--- test/services/previews/v1/commit.test.ts | 60 +++----------------- 2 files changed, 17 insertions(+), 62 deletions(-) diff --git a/src/services/previews/v1/github/artifacts.ts b/src/services/previews/v1/github/artifacts.ts index d369ff6..8e73f03 100644 --- a/src/services/previews/v1/github/artifacts.ts +++ b/src/services/previews/v1/github/artifacts.ts @@ -48,6 +48,12 @@ interface RawArtifact { } | null; } +interface ArtifactMatch { + artifact: RawArtifact; + runId: number; + headSha: string; +} + function unavailable( context: string, error: unknown, @@ -100,7 +106,7 @@ const MAX_FALLBACK_PAGES = 50; async function findInFallbackPages( githubToken: string, shaLower: string -): Promise<{ artifact: RawArtifact; runId: number; headSha: string } | null> { +): Promise { for (let page = 1; page <= MAX_FALLBACK_PAGES; page++) { const artifacts = await listArtifacts(githubToken, undefined, page); const match = matchArtifact( @@ -121,9 +127,8 @@ async function findInFallbackPages( function matchArtifact( artifacts: RawArtifact[], shaLower: string -): { artifact: RawArtifact; runId: number; headSha: string } | null { - let match: { artifact: RawArtifact; runId: number; headSha: string } | null = - null; +): ArtifactMatch | null { + let match: ArtifactMatch | null = null; for (const artifact of artifacts) { const workflowRun = artifact.workflow_run; @@ -144,11 +149,7 @@ function matchArtifact( return match; } -function toPreviewArtifact(match: { - artifact: RawArtifact; - runId: number; - headSha: string; -}): PreviewArtifact { +function toPreviewArtifact(match: ArtifactMatch): PreviewArtifact { return { runId: match.runId, artifactId: match.artifact.id, diff --git a/test/services/previews/v1/commit.test.ts b/test/services/previews/v1/commit.test.ts index d6071ae..edbdb87 100644 --- a/test/services/previews/v1/commit.test.ts +++ b/test/services/previews/v1/commit.test.ts @@ -327,59 +327,7 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { expect(ghRequest).toHaveBeenCalledTimes(2); }); - it("pages through the fallback scan when the match is past the first page", async () => { - const pageOne = Array.from({ length: 100 }, (_, i) => ({ - id: 1000 + i, - name: `FOSSBilling-preview-other${i}.zip`, - size_in_bytes: 1, - created_at: "2026-08-01T00:00:00Z", - expires_at: "2026-08-15T00:00:00Z", - expired: false, - digest: null, - workflow_run: { id: 1, head_sha: "0000000000000000000000000000000000000" } - })); - const pageTwoMatch = { - id: 2000, - name: "FOSSBilling-preview-deadbee.zip", - size_in_bytes: 99, - created_at: "2026-08-13T11:00:00Z", - expires_at: "2026-08-27T11:00:00Z", - expired: false, - digest: "sha256:page2", - workflow_run: { id: 888, head_sha: SHA } - }; - let fallbackCalls = 0; - (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( - async (route: string, params?: { name?: string; page?: number }) => { - if (route !== "GET /repos/{owner}/{repo}/actions/artifacts") { - throw new Error(`Unexpected route: ${route}`); - } - if (params?.name) { - return { data: { total_count: 0, artifacts: [] } }; - } - fallbackCalls++; - if (params?.page === 1) { - return { data: { total_count: 101, artifacts: pageOne } }; - } - if (params?.page === 2) { - return { data: { total_count: 101, artifacts: [pageTwoMatch] } }; - } - throw new Error(`Unexpected page: ${params?.page}`); - } - ); - - const res = await get(`/previews/v1/commit/${SHA}`); - - expect(res.status).toBe(200); - const body = (await res.json()) as { result: { artifact_id: number } }; - expect(body.result.artifact_id).toBe(2000); - // Exact-name miss (1) + fallback page 1 (2) + fallback page 2 (3) - - // stops as soon as a match is found, no page 3. - expect(fallbackCalls).toBe(2); - expect(ghRequest).toHaveBeenCalledTimes(3); - }); - - it("keeps paging past the old 5-page cap when the match is further back", async () => { + it("pages through the fallback scan past the old 5-page cap, then stops as soon as it finds a match", async () => { // Regression check: an earlier version of the fallback stopped after // 5 pages (500 artifacts) as a hard cutoff, which would have reported // this commit not_found even though its artifact genuinely exists - @@ -411,6 +359,7 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { digest: "sha256:page6", workflow_run: { id: 888, head_sha: SHA } }; + let fallbackCalls = 0; (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( async (route: string, params?: { name?: string; page?: number }) => { if (route !== "GET /repos/{owner}/{repo}/actions/artifacts") { @@ -419,6 +368,7 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { if (params?.name) { return { data: { total_count: 0, artifacts: [] } }; } + fallbackCalls++; const page = params?.page ?? 1; if (page <= 5) { return { data: { artifacts: fullPage(page * 1000) } }; @@ -435,6 +385,10 @@ describe("Previews API v1 - GET /previews/v1/commit/:sha", () => { expect(res.status).toBe(200); const body = (await res.json()) as { result: { artifact_id: number } }; expect(body.result.artifact_id).toBe(9000); + // Exact-name miss + 6 fallback pages - stops on page 6 rather than + // continuing to page 7. + expect(fallbackCalls).toBe(6); + expect(ghRequest).toHaveBeenCalledTimes(7); }); it("falls back to the default TTL ceiling when expires_at can't be parsed", async () => { From 5fbc8dc36ff0c41b7e50fd376cf1ccdf3e7232a5 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sat, 15 Aug 2026 08:29:37 +0100 Subject: [PATCH 14/17] Modify R2 bucket binding and name in wrangler.jsonc Updated R2 bucket configuration and removed comments. --- wrangler.jsonc | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/wrangler.jsonc b/wrangler.jsonc index e3905e0..de21497 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -47,16 +47,10 @@ "id": "0771957093ae481b9cd974ffa83f8263" } ], - // Backs previews/v1's `main` route. Pre-existing bucket (created - // 2024-09-27) that FOSSBilling/FOSSBilling's ci.yml already syncs the - // main-branch preview zip to, served publicly at - // download.fossbilling.org - not previously bound to any Worker. - // Double-check in the Cloudflare dashboard that this is in fact the - // bucket behind that custom domain before relying on it in production. "r2_buckets": [ { - "binding": "PREVIEW_BUCKET", - "bucket_name": "fossbilling" + "binding": "DOWNLOAD_BUCKET", + "bucket_name": "fossbilling-download" } ], "ratelimits": [ From 9d6ca7ea48cd008d041d9e45d7879e75258d6815 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sat, 15 Aug 2026 08:34:40 +0100 Subject: [PATCH 15/17] Rename preview R2 binding to DOWNLOAD_BUCKET --- README.md | 2 +- src/services/previews/v1/README.md | 2 +- src/services/previews/v1/routes/main.ts | 2 +- test/services/previews/v1/main.test.ts | 26 ++++++++++++------------- worker-configuration.d.ts | 4 ++-- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 9cc8109..7a90edd 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ We use [Cloudflare D1](https://developers.cloudflare.com/d1/) and [KV](https://d Migrations are owned by extensions v2 and applied only from this repository — see [its README](src/services/extensions/v2/README.md#database) for the migration and adoption procedure. - **KV Namespace** (`CACHE_KV`): Caches GitHub API responses so we don't hit rate limits. - **KV Namespace** (`AUTH_KV`): Stores the `UPDATE_TOKEN` value for `/versions/v1/update`. -- **R2 Bucket** (`PREVIEW_BUCKET`): Backs `/previews/v1/main` — see [`src/services/previews/v1/README.md`](src/services/previews/v1/README.md) and the comment in `wrangler.jsonc` for which bucket this points at and why. +- **R2 Bucket** (`DOWNLOAD_BUCKET`): Backs `/previews/v1/main` — see [`src/services/previews/v1/README.md`](src/services/previews/v1/README.md) and the comment in `wrangler.jsonc` for which bucket this points at and why. ### Environment Variables diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md index 3c2b0f2..6dbfe19 100644 --- a/src/services/previews/v1/README.md +++ b/src/services/previews/v1/README.md @@ -166,5 +166,5 @@ etc., surfaced as 429/503/500 depending on severity). to cache it without risking handing out an already-expired URL. - `GITHUB_TOKEN` is required for GitHub API access (shared with `versions/v1`). -- `PREVIEW_BUCKET` (R2 binding) backs `/main` - see `wrangler.jsonc` for the +- `DOWNLOAD_BUCKET` (R2 binding) backs `/main` - see `wrangler.jsonc` for the bucket this points at and why. diff --git a/src/services/previews/v1/routes/main.ts b/src/services/previews/v1/routes/main.ts index 7a2b342..292166a 100644 --- a/src/services/previews/v1/routes/main.ts +++ b/src/services/previews/v1/routes/main.ts @@ -58,7 +58,7 @@ async function resolveMainPreview( } } - const object = await getMainPreviewObject(c.env.PREVIEW_BUCKET); + const object = await getMainPreviewObject(c.env.DOWNLOAD_BUCKET); if (!object) return null; const artifactFields = await resolveArtifactFields( diff --git a/test/services/previews/v1/main.test.ts b/test/services/previews/v1/main.test.ts index c1eadb5..d6569dc 100644 --- a/test/services/previews/v1/main.test.ts +++ b/test/services/previews/v1/main.test.ts @@ -45,7 +45,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { beforeEach(async () => { restoreConsole = suppressConsole(); await env.CACHE_KV.delete("preview:main"); - await env.PREVIEW_BUCKET.delete(MAIN_PREVIEW_KEY); + await env.DOWNLOAD_BUCKET.delete(MAIN_PREVIEW_KEY); vi.clearAllMocks(); }); @@ -62,7 +62,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { }); it("returns the R2 object's metadata, including the sha256 digest", async () => { - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { customMetadata: { digest: "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", @@ -96,7 +96,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { }); it("reports a null digest and commit_sha when the object has no custom metadata", async () => { - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents"); + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents"); const res = await get("/previews/v1/main"); expect(res.status).toBe(200); @@ -115,7 +115,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { }); it("enriches with that commit's GitHub Actions artifact when resolvable", async () => { - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { customMetadata: { "commit-sha": COMMIT_SHA } }); (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( @@ -144,7 +144,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { }); it("still succeeds with null enrichment fields when GitHub is unavailable", async () => { - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { customMetadata: { "commit-sha": COMMIT_SHA } }); (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation(async () => { @@ -162,7 +162,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { }); it("still succeeds with null enrichment fields when the commit has no known artifact", async () => { - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { customMetadata: { "commit-sha": COMMIT_SHA } }); (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( @@ -176,7 +176,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { }); it("serves the second request from CACHE_KV without re-reading R2 or GitHub", async () => { - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "v1", { + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "v1", { customMetadata: { "commit-sha": "111" } }); (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( @@ -191,7 +191,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { // Overwrite the R2 object directly - a cache hit should still serve the // first response's data rather than reflecting this change. - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "v2", { + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "v2", { customMetadata: { "commit-sha": "222" } }); const second = await get("/previews/v1/main"); @@ -207,7 +207,7 @@ describe("Previews API v1 - GET /previews/v1/main", () => { }); it("falls back to R2 instead of erroring on a corrupt cache entry", async () => { - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents", { customMetadata: { "commit-sha": COMMIT_SHA } }); await env.CACHE_KV.put("preview:main", "not valid json{"); @@ -223,7 +223,7 @@ describe("Previews API v1 - GET /previews/v1/main/download", () => { beforeEach(async () => { restoreConsole = suppressConsole(); await env.CACHE_KV.delete("preview:main"); - await env.PREVIEW_BUCKET.delete(MAIN_PREVIEW_KEY); + await env.DOWNLOAD_BUCKET.delete(MAIN_PREVIEW_KEY); vi.clearAllMocks(); (vi.mocked(ghRequest) as MockGitHubRequest).mockImplementation( async () => ({ data: { total_count: 0, artifacts: [] } }) @@ -243,7 +243,7 @@ describe("Previews API v1 - GET /previews/v1/main/download", () => { }); it("redirects to the permanent main preview download URL", async () => { - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents"); + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents"); const res = await get("/previews/v1/main/download"); expect(res.status).toBe(302); @@ -253,8 +253,8 @@ describe("Previews API v1 - GET /previews/v1/main/download", () => { }); it("shares the metadata route's cache instead of re-reading R2", async () => { - await env.PREVIEW_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents"); - const headSpy = vi.spyOn(env.PREVIEW_BUCKET, "head"); + await env.DOWNLOAD_BUCKET.put(MAIN_PREVIEW_KEY, "test archive contents"); + const headSpy = vi.spyOn(env.DOWNLOAD_BUCKET, "head"); await get("/previews/v1/main"); const res = await get("/previews/v1/main/download"); diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 4df02c1..a732555 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,10 +1,10 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types --env-interface=CloudflareBindings` (hash: 85d5d01372efbcd1de1d834068043962) +// Generated by Wrangler by running `wrangler types --env-interface=CloudflareBindings` (hash: 3699481541fb6006a52b433d336dc475) // Runtime types generated with workerd@1.20260801.1 2026-06-24 nodejs_compat interface __BaseEnv_CloudflareBindings { AUTH_KV: KVNamespace; CACHE_KV: KVNamespace; - PREVIEW_BUCKET: R2Bucket; + DOWNLOAD_BUCKET: R2Bucket; DB_CENTRAL_ALERTS: D1Database; DB_EXTENSIONS: D1Database; PROFILE_CREATION_RATE_LIMITER: RateLimit; From 886dbd97f23ac2be18b2964a26169e7bbeba2e30 Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sat, 15 Aug 2026 08:42:57 +0100 Subject: [PATCH 16/17] Stop duplicating the generated OpenAPI contract in previews/v1's README Matches the precedent extensions/v2 already established for services with OpenAPI generation: point at the live spec/docs UI instead of hand-copying endpoint shapes that will drift out of sync with the zod schemas that actually generate them. Cut the three '### GET /...' sections' literal JSON request/response examples and the 'Error Responses' section (ErrorResponseSchema already covers that shape completely). Kept and expanded 'Resource Model' with the genuinely non-obvious behavior that OpenAPI can't express on its own - why download_url's shape differs between main and pr/commit, why source stays "r2" regardless of enrichment, and that main's GitHub Actions enrichment is best-effort and never load-bearing. The 'Notes' section (caching/TTL behavior, GITHUB_TOKEN, DOWNLOAD_BUCKET) is operational detail outside the wire contract either way, so it's untouched. Docs-only change, no code touched. --- src/services/previews/v1/README.md | 141 ++++++----------------------- 1 file changed, 29 insertions(+), 112 deletions(-) diff --git a/src/services/previews/v1/README.md b/src/services/previews/v1/README.md index 6dbfe19..5cbb4d1 100644 --- a/src/services/previews/v1/README.md +++ b/src/services/previews/v1/README.md @@ -16,13 +16,20 @@ given commit are two independently-built files (a `cp` of the same bytes, in the current CI job, but not guaranteed to stay that way), so whichever one is reported as the digest has to match the bytes `main` actually serves. `GET /main` does still cross-reference that commit's GitHub Actions -artifact for `run_id`/`artifact_id`/`created_at`/`expires_at` - see its -section below - but only as best-effort enrichment, never as a dependency. +artifact for enrichment - see Resource Model below - but only as +best-effort, never as a dependency. There is no publish/write endpoint: nothing pushes data into this service, it only resolves and redirects. -## Resource model +## Endpoints + +Endpoints are not listed here. The service publishes its own contract: + +- **OpenAPI document:** `GET /previews/v1/openapi.json` +- **Reference UI:** `GET /previews/v1/docs` + +## Resource Model - `GET /main` and `GET /pr/{number}` are **pointers** - they always resolve to whatever is current. @@ -31,115 +38,25 @@ it only resolves and redirects. - `pr/{number}`'s handler resolves the PR to its head SHA (`GET /pulls/{number}`) and delegates to the same resolver `commit/{sha}` uses - one GitHub-facing code path, not two. - -## Endpoints - -### GET `/main` - -Current main preview. `download_url`, `digest`, `commit_sha`, -`size_bytes`, and `last_modified` are sourced from an R2 object HEAD (no -GitHub API call). `run_id`, `artifact_id`, `created_at`, and `expires_at` -are enrichment: resolved from that commit's GitHub Actions artifact (same -lookup `GET /commit/{sha}` uses) purely for shape parity with -`ArtifactPreview`, so a client reading either response doesn't have to -special-case field availability. That enrichment is best-effort and never -load-bearing - if the commit has no known artifact (e.g. it's aged out of -GitHub's 14-day retention) or GitHub is unavailable, those four fields are -just `null`; the response still succeeds with everything R2-sourced intact. - -**Response:** - -```json -{ - "result": { - "commit_sha": "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", - "short_sha": "a1b2c3d", - "pr_number": null, - "run_id": 999999, - "artifact_id": 555555, - "created_at": "2026-08-13T10:00:00Z", - "expires_at": "2026-08-27T10:00:00Z", - "digest": "sha256:9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", - "size_bytes": 31229553, - "last_modified": "2026-08-13T13:11:41.000Z", - "download_url": "https://download.fossbilling.org/FOSSBilling-preview.zip", - "source": "r2" - } -} -``` - -`commit_sha` and `digest` come straight from the R2 object's `commit-sha`/ -`digest` custom metadata (`digest` already carries the `sha256:` prefix) - -both are `null` if that object has no custom metadata (e.g. it predates the -CI job setting it), which also means the GitHub Actions enrichment above is -skipped entirely (nothing to look up by). `source` stays `"r2"` regardless -of whether the enrichment resolved - it describes where `download_url`/ -`digest` come from, which never changes. - -### GET `/main/download` - -302 redirect to `download_url` - the same permanent URL `GET /main` already -reports. Exists purely for uniform addressing (every resource under -`/previews/v1` has a `/download` sub-route, so callers never need to -special-case main to reach a download link instead of reading one out of a -JSON body). Unlike `/pr/{number}/download` and `/commit/{sha}/download`, -this target URL is fixed rather than short-lived, so it's answered from the -same cache as `GET /main` instead of re-resolving anything live. - -### GET `/pr/{number}` and GET `/commit/{sha}` - -Preview build for a pull request's current head, or for one exact commit. -`sha` accepts a full or abbreviated (7+ char) hex SHA. - -**Response:** - -```json -{ - "result": { - "commit_sha": "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", - "short_sha": "a1b2c3d", - "pr_number": 123, - "run_id": 999999, - "artifact_id": 555555, - "digest": "sha256:...", - "size_bytes": 12345, - "created_at": "2026-08-13T10:00:00Z", - "expires_at": "2026-08-27T10:00:00Z", - "download_url": "/previews/v1/commit/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/download", - "source": "actions_artifact" - } -} -``` - -`digest` is GitHub's own artifact digest - the exact bytes served by the -`/download` route. `pr_number` is only set when resolved via `/pr/{number}`; -a direct `/commit/{sha}` lookup has no way to know which PR (if any) built -that commit, and reports `null`. - -`download_url` always points at the canonical `/commit/{sha}/download` -route using the fully-resolved SHA, not `/pr/{number}/download` - a PR's -head SHA moves as new commits land, a specific commit's build does not. - -### GET `/pr/{number}/download` and GET `/commit/{sha}/download` - -302 redirect to GitHub's live, short-lived artifact download URL. Resolved -fresh on every request - never served from cache, since GitHub's signed URL -expires in about a minute. - -## Error Responses - -```json -{ - "error": { - "message": "No pull request #999 was found, or it has no preview build yet.", - "code": "NOT_FOUND" - } -} -``` - -`code` is one of `NOT_FOUND`, `VALIDATION_ERROR` (422, malformed path -param), or GitHub's own `errorCode` (`rate_limit_error`, `auth_error`, -etc., surfaced as 429/503/500 depending on severity). +- `download_url` differs in kind depending on the resource. `main`'s is the + permanent public `download.fossbilling.org` URL, embedded directly, since + it never expires. `pr`/`commit`'s is self-referential - it points back at + their own `/download` sub-route rather than GitHub's actual signed URL, + because that URL expires in ~60s and can't be baked into a response with + any longer cache lifetime; `/download` resolves the real one live on + each hit. +- `source` on `/main` stays `"r2"` regardless of whether the GitHub Actions + enrichment below resolves - it describes where `download_url`/`digest` + come from, which never changes. +- `main`'s `run_id`/`artifact_id`/`created_at`/`expires_at` are enrichment, + resolved from that commit's GitHub Actions artifact (the same lookup + `commit/{sha}` uses) purely for shape parity with the PR/commit response, + so a client reading either doesn't have to special-case field + availability. It's best-effort and never load-bearing: a miss (no known + artifact yet, the artifact aged out of GitHub's 14-day retention, GitHub + unavailable) just leaves those four fields `null` - it's never the reason + a request to `/main` fails, since `download_url`/`digest` are R2-sourced + and don't depend on it. ## Notes From c4434b4efa77143fed34530c3874cb78ac18d87d Mon Sep 17 00:00:00 2001 From: Adam Daley Date: Sat, 15 Aug 2026 08:46:45 +0100 Subject: [PATCH 17/17] Add field-level OpenAPI descriptions to previews/v1's response schemas Follow-up to the README trim: pointing consumers at the generated OpenAPI docs only helps if that spec actually carries the behavior previously explained in prose. Added .openapi({ description }) to the fields with genuinely non-obvious semantics - why download_url's shape differs between main (permanent, embedded directly) and pr/commit (self-referential, resolved live), why source stays fixed regardless of enrichment, that main's run_id/artifact_id/created_at/expires_at are best-effort and never load-bearing, and why pr_number is only set via /pr/{number} - plus top-level descriptions on both MainPreview and ArtifactPreview. Verified by generating the actual OpenAPI document (previewsV1.getOpenAPI31Document(...)) and confirming the descriptions round-trip into components.schemas, not just that the zod chains compile. No behavior change - 520 + 47 tests, lint and typecheck clean. --- src/services/previews/v1/schemas/previews.ts | 87 +++++++++++++++----- 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/src/services/previews/v1/schemas/previews.ts b/src/services/previews/v1/schemas/previews.ts index c322bbf..2a16d33 100644 --- a/src/services/previews/v1/schemas/previews.ts +++ b/src/services/previews/v1/schemas/previews.ts @@ -52,27 +52,55 @@ export const CommitShaParamSchema = z.object({ const MainPreviewSchema = z .object({ - commit_sha: z.string().nullable(), + commit_sha: z.string().nullable().openapi({ + description: + "Commit that produced the current main preview, from R2 object custom metadata. null if the object predates that metadata being set - GitHub Actions enrichment below is skipped in that case too, since there's no commit to look it up by." + }), short_sha: z.string().nullable(), - // Always null - main isn't a PR. Present for shape parity with - // ArtifactPreview. - pr_number: z.number().nullable(), + pr_number: z.number().nullable().openapi({ + description: + "Always null - main is never associated with a pull request. Present only for shape parity with the PR/commit response." + }), // Enrichment from that commit's GitHub Actions artifact, when // resolvable - null if the commit has no known artifact (e.g. expired // past GitHub's 14-day retention) or GitHub is unavailable. Never // blocks or degrades the response: download_url/digest below are the // load-bearing, R2-sourced fields and don't depend on this resolving. - run_id: z.number().nullable(), - artifact_id: z.number().nullable(), - created_at: z.string().nullable(), - expires_at: z.string().nullable(), - digest: z.string().nullable(), + run_id: z.number().nullable().openapi({ + description: + "GitHub Actions run that produced this commit's preview artifact. null if that artifact isn't resolvable (not yet built, aged out of GitHub's 14-day retention, or GitHub unavailable) - this is best-effort enrichment, never required for the response to succeed." + }), + artifact_id: z.number().nullable().openapi({ + description: + "GitHub Actions artifact ID for this commit's build. Same best-effort enrichment as run_id - null under the same conditions." + }), + created_at: z.string().nullable().openapi({ + description: + "When the enrichment artifact was created. Same best-effort enrichment as run_id - null under the same conditions." + }), + expires_at: z.string().nullable().openapi({ + description: + "When the enrichment artifact ages out of GitHub's retention. Same best-effort enrichment as run_id - null under the same conditions." + }), + digest: z.string().nullable().openapi({ + description: + "SHA-256 digest (sha256:) of the R2-hosted zip, from R2 object custom metadata. null if the object predates that metadata being set." + }), size_bytes: z.number(), last_modified: z.string(), - download_url: z.string(), - source: z.literal("r2") + download_url: z.string().openapi({ + description: + "Permanent public download URL (download.fossbilling.org). Unlike the PR/commit equivalent, this never expires and is safe to embed directly rather than resolve through a redirect." + }), + source: z.literal("r2").openapi({ + description: + 'Always "r2" - describes where download_url/digest come from, independent of whether the GitHub Actions enrichment above resolved.' + }) }) - .openapi("MainPreview"); + .openapi("MainPreview", { + description: + "Current main preview. download_url/digest are R2-sourced and always present once main has been published; run_id/artifact_id/created_at/expires_at are best-effort GitHub Actions enrichment that may be null." + }); export const MainPreviewResponseSchema = z .object({ result: MainPreviewSchema }) @@ -82,17 +110,38 @@ const ArtifactPreviewSchema = z .object({ commit_sha: z.string(), short_sha: z.string(), - pr_number: z.number().nullable(), + pr_number: z.number().nullable().openapi({ + description: + "Set only when resolved via /pr/{number} - a direct /commit/{sha} lookup has no way to know which PR (if any) built that commit, and reports null." + }), run_id: z.number(), - artifact_id: z.number(), - digest: z.string().nullable(), + artifact_id: z.number().openapi({ + description: + "GitHub Actions artifact ID - what /download resolves to a live signed URL." + }), + digest: z.string().nullable().openapi({ + description: + "GitHub's own SHA-256 digest (sha256:) for this artifact - the exact bytes served by the /download route." + }), size_bytes: z.number(), created_at: z.string(), - expires_at: z.string(), - download_url: z.string(), - source: z.literal("actions_artifact") + expires_at: z.string().openapi({ + description: + "When this artifact ages out of GitHub's 14-day retention. After this, /download starts returning 404 even if this metadata is still cached." + }), + download_url: z.string().openapi({ + description: + "Self-referential - points at this service's own /commit/{sha}/download, not GitHub's actual signed URL (which expires in ~60s and can't be cached). Always the canonical commit URL, even when resolved via /pr/{number}, since a PR's head SHA moves as new commits land but a commit's build does not." + }), + source: z.literal("actions_artifact").openapi({ + description: + 'Always "actions_artifact" - distinguishes this from main\'s R2-sourced response.' + }) }) - .openapi("ArtifactPreview"); + .openapi("ArtifactPreview", { + description: + "Preview build for a specific commit or pull request, resolved from a GitHub Actions artifact." + }); export const ArtifactPreviewResponseSchema = z .object({ result: ArtifactPreviewSchema })