Skip to content

Add previews/v1 service for preview build lookup - #195

Open
admdly wants to merge 10 commits into
mainfrom
previews-v1-api
Open

Add previews/v1 service for preview build lookup#195
admdly wants to merge 10 commits into
mainfrom
previews-v1-api

Conversation

@admdly

@admdly admdly commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

What

Adds previews/v1 — a read-only lookup service for FOSSBilling preview builds (current main, a PR's current head, or one exact commit), plus a small unrelated typecheck fix found along the way.

GET /previews/v1/main               GET /previews/v1/main/download
GET /previews/v1/pr/{number}        GET /previews/v1/pr/{number}/download
GET /previews/v1/commit/{sha}       GET /previews/v1/commit/{sha}/download

OpenAPI docs at /previews/v1/openapi.json and /previews/v1/docs.

Design

  • GitHub Actions is the source of truth for pr/commitFOSSBilling/FOSSBilling's ci.yml (see FOSSBilling/FOSSBilling#4157) uploads one artifact per commit, named FOSSBilling-preview-{short_sha}.zip (archive: false, so the zip itself is the artifact). This service resolves by querying that exact name rather than listing every preview artifact and filtering.
  • main is R2-backed for download_url/digest — 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, so whichever one is reported as the digest has to match the bytes actually served. main additionally cross-references that commit's GitHub Actions artifact to populate run_id/artifact_id/created_at/expires_at, purely as best-effort enrichment — never a dependency; a GitHub hiccup never fails /main, those fields just come back null.
  • No publish/write endpoint — nothing pushes data into this service, it only resolves and redirects.
  • Pointers vs fixed pointsmain and pr/{number} always resolve to whatever's current; commit/{sha} is permanently addressable to one exact build. pr/{number} resolves the PR to its head SHA and delegates to the same resolver commit/{sha} uses.
  • GitHub API call budget/download routes share the same cached lookup the metadata routes use (only the final signed-redirect resolution is ever live, since it expires in ~60s and can't be cached — Cloudflare KV enforces a hard 60s minimum TTL, leaving no safe margin). commit/{sha} caches for 3600s rather than the 60s default, since a commit's build never changes once it exists.

admdly added 9 commits August 13, 2026 15:22
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.
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.
- 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.
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.
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<any>' (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.
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.
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.
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.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 13, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
api 9b25f9e Commit Preview URL

Branch Preview URL
Aug 13 2026, 06:48 PM

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 22 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/services/previews/v1/routes/pr.ts
Comment thread src/services/previews/v1/routes/main.ts Outdated
Comment thread src/services/previews/v1/github/artifacts.ts Outdated
Comment thread src/services/previews/v1/routes/commit.ts
Comment thread src/services/previews/v1/schemas/previews.ts Outdated
Comment thread src/services/previews/v1/README.md Outdated
Comment thread README.md
@admdly admdly self-assigned this Aug 13, 2026
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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 11 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/services/previews/v1/routes/commit.ts">

<violation number="1" location="src/services/previews/v1/routes/commit.ts:32">
P3: When an artifact expires within the next hour, this callback returns less than 3600 seconds, and the final minute is uncached. Update the caching documentation to describe this retention cap and final-minute behavior.</violation>

<violation number="2" location="src/services/previews/v1/routes/commit.ts:32">
P2: When `artifact.expires_at` is an empty string or otherwise unparseable, `new Date(expires_at).getTime()` returns `NaN`, so `remainingSeconds` and `Math.min(...)` are `NaN`. `NaN >= KV_MIN_TTL_SECONDS` is `false`, so in cache.ts the commit result is never cached and the lookup silently performs 2 GitHub API calls on every request instead of the intended 3600s caching. This is reachable in practice because `toPreviewArtifact` (github/artifacts.ts) fills `expiresAt` from `artifact.expires_at ?? ""`, so a null GitHub `expires_at` lands here as "".</violation>
</file>

<file name="src/services/previews/v1/cache.ts">

<violation number="1" location="src/services/previews/v1/cache.ts:48">
P2: When an artifact has only 60.x seconds of retention left, this writes a full 60-second KV TTL calculated before the write completes. Recompute against an absolute expiration or skip near-floor TTLs with a write-time safety margin so `/commit/{sha}` cannot remain cached after the artifact expires.</violation>
</file>

<file name="src/services/previews/v1/github/artifacts.ts">

<violation number="1" location="src/services/previews/v1/github/artifacts.ts:155">
P2: The new fork-PR fallback only scans a single page of the artifacts API (`per_page: 100`, no pagination), so it never looks past the 100 most recent artifacts in the repo. For an active repo whose retention window holds more than 100 preview artifacts, valid fork-PR builds older than the newest ~100 produce a false NOT_FOUND — exactly the case this fallback was added to fix. Paginate through the artifact list (following the link header or “next” pages) until a match is found or pages are exhausted, and stop as soon as `matchArtifact` returns a result.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

typeof ttlSeconds === "function" ? ttlSeconds(result.data) : ttlSeconds;
if (ttl >= KV_MIN_TTL_SECONDS) {
await kv.put(key, JSON.stringify(result.data), {
expirationTtl: ttl

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When an artifact has only 60.x seconds of retention left, this writes a full 60-second KV TTL calculated before the write completes. Recompute against an absolute expiration or skip near-floor TTLs with a write-time safety margin so /commit/{sha} cannot remain cached after the artifact expires.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/services/previews/v1/cache.ts, line 48:

<comment>When an artifact has only 60.x seconds of retention left, this writes a full 60-second KV TTL calculated before the write completes. Recompute against an absolute expiration or skip near-floor TTLs with a write-time safety margin so `/commit/{sha}` cannot remain cached after the artifact expires.</comment>

<file context>
@@ -28,9 +41,13 @@ export async function cachedLookup<T>(
+      typeof ttlSeconds === "function" ? ttlSeconds(result.data) : ttlSeconds;
+    if (ttl >= KV_MIN_TTL_SECONDS) {
+      await kv.put(key, JSON.stringify(result.data), {
+        expirationTtl: ttl
+      });
+    }
</file context>

let match = matchArtifact(exact, shaLower);

if (!match) {
const all = await listArtifacts(githubToken, undefined);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The new fork-PR fallback only scans a single page of the artifacts API (per_page: 100, no pagination), so it never looks past the 100 most recent artifacts in the repo. For an active repo whose retention window holds more than 100 preview artifacts, valid fork-PR builds older than the newest ~100 produce a false NOT_FOUND — exactly the case this fallback was added to fix. Paginate through the artifact list (following the link header or “next” pages) until a match is found or pages are exhausted, and stop as soon as matchArtifact returns a result.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/services/previews/v1/github/artifacts.ts, line 155:

<comment>The new fork-PR fallback only scans a single page of the artifacts API (`per_page: 100`, no pagination), so it never looks past the 100 most recent artifacts in the repo. For an active repo whose retention window holds more than 100 preview artifacts, valid fork-PR builds older than the newest ~100 produce a false NOT_FOUND — exactly the case this fallback was added to fix. Paginate through the artifact list (following the link header or “next” pages) until a match is found or pages are exhausted, and stop as soon as `matchArtifact` returns a result.</comment>

<file context>
@@ -61,67 +61,111 @@ function unavailable<T>(
-        };
-      }
+    if (!match) {
+      const all = await listArtifacts(githubToken, undefined);
+      match = matchArtifact(
+        all.filter((artifact) =>
</file context>

const remainingSeconds = Math.floor(
(new Date(artifact.expires_at).getTime() - Date.now()) / 1000
);
return Math.min(COMMIT_CACHE_TTL_SECONDS, remainingSeconds);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When artifact.expires_at is an empty string or otherwise unparseable, new Date(expires_at).getTime() returns NaN, so remainingSeconds and Math.min(...) are NaN. NaN >= KV_MIN_TTL_SECONDS is false, so in cache.ts the commit result is never cached and the lookup silently performs 2 GitHub API calls on every request instead of the intended 3600s caching. This is reachable in practice because toPreviewArtifact (github/artifacts.ts) fills expiresAt from artifact.expires_at ?? "", so a null GitHub expires_at lands here as "".

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/services/previews/v1/routes/commit.ts, line 32:

<comment>When `artifact.expires_at` is an empty string or otherwise unparseable, `new Date(expires_at).getTime()` returns `NaN`, so `remainingSeconds` and `Math.min(...)` are `NaN`. `NaN >= KV_MIN_TTL_SECONDS` is `false`, so in cache.ts the commit result is never cached and the lookup silently performs 2 GitHub API calls on every request instead of the intended 3600s caching. This is reachable in practice because `toPreviewArtifact` (github/artifacts.ts) fills `expiresAt` from `artifact.expires_at ?? ""`, so a null GitHub `expires_at` lands here as "".</comment>

<file context>
@@ -17,6 +18,20 @@ const COMMIT_CACHE_TTL_SECONDS = 3600;
+  const remainingSeconds = Math.floor(
+    (new Date(artifact.expires_at).getTime() - Date.now()) / 1000
+  );
+  return Math.min(COMMIT_CACHE_TTL_SECONDS, remainingSeconds);
+}
+
</file context>

const remainingSeconds = Math.floor(
(new Date(artifact.expires_at).getTime() - Date.now()) / 1000
);
return Math.min(COMMIT_CACHE_TTL_SECONDS, remainingSeconds);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: When an artifact expires within the next hour, this callback returns less than 3600 seconds, and the final minute is uncached. Update the caching documentation to describe this retention cap and final-minute behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/services/previews/v1/routes/commit.ts, line 32:

<comment>When an artifact expires within the next hour, this callback returns less than 3600 seconds, and the final minute is uncached. Update the caching documentation to describe this retention cap and final-minute behavior.</comment>

<file context>
@@ -17,6 +18,20 @@ const COMMIT_CACHE_TTL_SECONDS = 3600;
+  const remainingSeconds = Math.floor(
+    (new Date(artifact.expires_at).getTime() - Date.now()) / 1000
+  );
+  return Math.min(COMMIT_CACHE_TTL_SECONDS, remainingSeconds);
+}
+
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant