From 1b7e0590b0e2ea64dd109ce2691244c4430d1d2a Mon Sep 17 00:00:00 2001 From: "Michael K (Pear)" Date: Sun, 6 Sep 2026 20:25:27 -0400 Subject: [PATCH 1/2] fix(pr-package): keep PR previews until the pull request closes TTL expiry now checks GitHub when a tarball is tagged with Alchemy-Pull-Request and renews while the PR is open. CI binds that header after publish, deletes the preview on close, and updates the sticky install comment. --- .github/workflows/pr-package.yml | 168 +++++++++++- .../cloudflare-pr-package/test/integ.test.ts | 242 +++++++++++++++++- packages/pr-package/README.md | 16 +- packages/pr-package/src/PackageStore.ts | 99 ++++++- packages/pr-package/src/PullRequest.ts | 95 +++++++ packages/pr-package/src/Worker.ts | 59 ++++- packages/pr-package/test/PullRequest.test.ts | 43 ++++ scripts/bind-pr-packages.ts | 82 ++++++ 8 files changed, 780 insertions(+), 24 deletions(-) create mode 100644 packages/pr-package/src/PullRequest.ts create mode 100644 packages/pr-package/test/PullRequest.test.ts create mode 100644 scripts/bind-pr-packages.ts diff --git a/.github/workflows/pr-package.yml b/.github/workflows/pr-package.yml index ded1c04142..4771a7572f 100644 --- a/.github/workflows/pr-package.yml +++ b/.github/workflows/pr-package.yml @@ -14,10 +14,11 @@ on: - "tsconfig.json" - ".github/workflows/pr-package.yml" pull_request: - # `labeled` so adding the `force-ci` label re-triggers the run. We - # intentionally do NOT listen to `closed` — pr-package tags persist - # past PR close so existing install URLs keep resolving. - types: [opened, synchronize, reopened, labeled] + # `labeled` so adding the `force-ci` label re-triggers the run. + # `closed` tears down PR-tied preview tags and updates the sticky + # install comment. While a PR is open, pkg.ing renews TTL instead + # of deleting. + types: [opened, synchronize, reopened, labeled, closed] paths: - "submodules/distilled" - "packages/**" @@ -30,7 +31,7 @@ on: - ".github/workflows/pr-package.yml" # The action can't widen permissions, so the job must grant write access for -# the sticky install-instructions comment. +# the sticky install-instructions comment and the teardown comment on close. permissions: contents: read pull-requests: write @@ -105,6 +106,18 @@ jobs: ] pr-package-token: ${{ secrets.PR_PACKAGE_TOKEN }} + - name: Tie preview packages to this PR + if: >- + github.event_name == 'pull_request' && + fromJson(steps.publish.outputs.plan).packages[0] != null + env: + PLAN: ${{ steps.publish.outputs.plan }} + TOKEN: ${{ secrets.PR_PACKAGE_TOKEN }} + PR_PACKAGE_HOST: pkg.ing + PULL_REQUEST: ${{ github.repository }}#${{ github.event.pull_request.number }} + TTL: 1 week + run: bun scripts/bind-pr-packages.ts + - name: Generate bot token if: >- github.event_name == 'pull_request' && @@ -123,3 +136,148 @@ jobs: with: plan: ${{ steps.publish.outputs.plan }} token: ${{ steps.bot-token.outputs.token }} + + pr-package-teardown: + name: Tear down PR preview packages + if: >- + github.event_name == 'pull_request' && github.event.action == 'closed' && + github.event.pull_request.head.repo.full_name == github.repository + runs-on: blacksmith-8vcpu-ubuntu-2404 + steps: + - name: Remove preview packages + env: + PR_PACKAGE_TOKEN: ${{ secrets.PR_PACKAGE_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + BRANCH: ${{ github.event.pull_request.head.ref }} + SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + host=pkg.ing + packages=( + alchemy + @alchemy.run/better-auth + @alchemy.run/cloudflare-runtime + @alchemy.run/frontend-frameworks + @alchemy.run/node-utils + @alchemy.run/pr-package + @alchemy.run/floci + @distilled.cloud/core + @distilled.cloud/aws + @distilled.cloud/axiom + @distilled.cloud/cloudflare + @distilled.cloud/hetzner + @distilled.cloud/neon + @distilled.cloud/planetscale + ) + encode_project() { + python3 -c 'import urllib.parse,sys; print("/".join(urllib.parse.quote(p, safe="") for p in sys.argv[1].split("/")))' "$1" + } + encode_tag() { + python3 -c 'import urllib.parse,sys; print(urllib.parse.quote(sys.argv[1], safe=""))' "$1" + } + delete() { + local url="$1" + local code + code=$(curl -sS -o /tmp/pr-pkg-body -w "%{http_code}" -X DELETE \ + -H "Authorization: Bearer ${PR_PACKAGE_TOKEN}" \ + "$url") + if [[ "$code" != "200" && "$code" != "404" ]]; then + echo "Failed DELETE $url ($code): $(cat /tmp/pr-pkg-body)" + return 1 + fi + echo "DELETE $url ($code)" + } + failed=0 + short="${SHA:0:7}" + for pkg in "${packages[@]}"; do + path=$(encode_project "$pkg") + base="https://${host}/projects/${path}" + delete "${base}/pull-requests/${PR_NUMBER}" || failed=1 + delete "${base}/tags/$(encode_tag "pr-${PR_NUMBER}")" || failed=1 + if [[ "$BRANCH" != "main" && "$BRANCH" != "master" ]]; then + delete "${base}/tags/$(encode_tag "$BRANCH")" || failed=1 + fi + delete "${base}/tags/$(encode_tag "$SHA")" || failed=1 + delete "${base}/tags/$(encode_tag "$short")" || failed=1 + done + exit "$failed" + + - name: Generate bot token + id: bot-token + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + app-id: ${{ secrets.ALCHEMY_VERSION_BOT_ID }} + private-key: ${{ secrets.ALCHEMY_VERSION_BOT_PRIVATE_KEY }} + + - name: Comment that preview packages were removed + env: + GH_TOKEN: ${{ steps.bot-token.outputs.token }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + python3 <<'PY' + import json + import os + import urllib.request + + token = os.environ["GH_TOKEN"] + repo = os.environ["REPO"] + pr = os.environ["PR_NUMBER"] + marker = "" + body = "\n".join( + [ + marker, + "", + "Preview packages for this PR have been removed.", + "", + "Install URLs from earlier comments no longer resolve.", + "", + ] + ) + + def api(path, method="GET", payload=None): + headers = { + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {token}", + "User-Agent": "alchemy-pr-package-teardown", + "X-GitHub-Api-Version": "2022-11-28", + } + data = None + if payload is not None: + headers["Content-Type"] = "application/json" + data = json.dumps(payload).encode() + req = urllib.request.Request( + f"https://api.github.com{path}", + data=data, + method=method, + headers=headers, + ) + with urllib.request.urlopen(req) as res: + if res.status == 204: + return None + return json.load(res) + + existing = None + page = 1 + while True: + comments = api( + f"/repos/{repo}/issues/{pr}/comments?per_page=100&page={page}" + ) + existing = next( + (c for c in comments if str(c.get("body", "")).startswith(marker)), + None, + ) + if existing or len(comments) < 100: + break + page += 1 + + if existing is None: + print("No PR-package comment to update") + else: + api( + f"/repos/{repo}/issues/comments/{existing['id']}", + "PATCH", + {"body": body}, + ) + print("Updated PR-package comment") + PY diff --git a/examples/cloudflare-pr-package/test/integ.test.ts b/examples/cloudflare-pr-package/test/integ.test.ts index fb0960e157..7440a6b55d 100644 --- a/examples/cloudflare-pr-package/test/integ.test.ts +++ b/examples/cloudflare-pr-package/test/integ.test.ts @@ -1,6 +1,7 @@ import * as Cloudflare from "alchemy/Cloudflare"; import * as Test from "alchemy/Test/Bun"; import { expect } from "bun:test"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Schedule from "effect/Schedule"; import * as HttpBody from "effect/unstable/http/HttpBody"; @@ -80,14 +81,18 @@ const assignTags = ( project: string, hash: string, tags: string[], -) => - client.execute( - HttpClientRequest.put(`${url}/projects/${project}/tags`).pipe( - HttpClientRequest.bearerToken(token), - HttpClientRequest.setHeader("Alchemy-Tags", JSON.stringify(tags)), - HttpClientRequest.setHeader("Alchemy-Tarball-Hash", hash), - ), + extraHeaders: Record = {}, +) => { + let req = HttpClientRequest.put(`${url}/projects/${project}/tags`).pipe( + HttpClientRequest.bearerToken(token), + HttpClientRequest.setHeader("Alchemy-Tags", JSON.stringify(tags)), + HttpClientRequest.setHeader("Alchemy-Tarball-Hash", hash), ); + for (const [name, value] of Object.entries(extraHeaders)) { + req = req.pipe(HttpClientRequest.setHeader(name, value)); + } + return client.execute(req); +}; const getPackage = ( client: Client, @@ -112,6 +117,19 @@ const deleteTag = ( ).pipe(HttpClientRequest.bearerToken(token)), ); +const deletePullRequest = ( + client: Client, + url: string, + token: string, + project: string, + number: number, +) => + client.execute( + HttpClientRequest.make("DELETE")( + `${url}/projects/${project}/pull-requests/${number}`, + ).pipe(HttpClientRequest.bearerToken(token)), + ); + // Tag lookups go through Workers KV, which is eventually consistent, and the // edge can return a transient 5xx. Poll the request until it reaches the // expected status (or give up after ~30s) so assertions test the converged @@ -119,12 +137,13 @@ const deleteTag = ( const pollUntilStatus = ( request: Effect.Effect, status: number, + times = 30, ): Effect.Effect => request.pipe( Effect.repeat({ schedule: Schedule.spaced("1 second"), until: (res) => res.status === status, - times: 30, + times, }), ); @@ -325,3 +344,210 @@ test( }), { timeout: 180_000 }, ); + +test( + "Alchemy-Pull-Request must be a GitHub owner/repo#number", + Effect.gen(function* () { + const { url, authToken } = yield* stack; + const client = yield* HttpClient.HttpClient; + expect((yield* warmUp(client, url)).status).toBe(401); + + const project = "pr-header-test"; + const content = "pr-header-bundle"; + const hash = sha256(content); + + expect( + (yield* pollUntilStatus( + upload(client, url, authToken, project, content), + 200, + )).status, + ).toBe(200); + + const invalid = yield* assignTags( + client, + url, + authToken, + project, + hash, + ["pr-1"], + { "Alchemy-Pull-Request": "not-a-pr" }, + ); + expect(invalid.status).toBe(400); + + const tagged = yield* pollUntilStatus( + assignTags(client, url, authToken, project, hash, ["pr-1", "v1"], { + "Alchemy-Pull-Request": "alchemy-run/alchemy#550", + }), + 200, + ); + expect(tagged.status).toBe(200); + const body = (yield* tagged.json) as { pullRequest?: string }; + expect(body.pullRequest).toBe("alchemy-run/alchemy#550"); + + expect( + (yield* deleteTag(client, url, authToken, project, "pr-1")).status, + ).toBe(200); + expect( + (yield* deleteTag(client, url, authToken, project, "v1")).status, + ).toBe(200); + }), + { timeout: 180_000 }, +); + +test( + "DELETE /pull-requests/:n drops PR tags and keeps unrelated ones", + Effect.gen(function* () { + const { url, authToken } = yield* stack; + const client = yield* HttpClient.HttpClient; + expect((yield* warmUp(client, url)).status).toBe(401); + + const project = "pr-teardown-test"; + const content = "pr-teardown-bundle"; + const hash = sha256(content); + + expect( + (yield* pollUntilStatus( + upload(client, url, authToken, project, content), + 200, + )).status, + ).toBe(200); + + expect( + (yield* pollUntilStatus( + assignTags(client, url, authToken, project, hash, ["main"]), + 200, + )).status, + ).toBe(200); + + expect( + (yield* pollUntilStatus( + assignTags( + client, + url, + authToken, + project, + hash, + ["pr-99", "deadbeef"], + { "Alchemy-Pull-Request": "alchemy-run/alchemy#99" }, + ), + 200, + )).status, + ).toBe(200); + + expect( + (yield* deletePullRequest(client, url, authToken, project, 99)).status, + ).toBe(200); + + expect( + (yield* pollUntilStatus(getTag(client, url, project, "pr-99"), 404)) + .status, + ).toBe(404); + expect( + (yield* pollUntilStatus(getTag(client, url, project, "deadbeef"), 404)) + .status, + ).toBe(404); + expect( + (yield* pollUntilStatus(getTag(client, url, project, "main"), 200)) + .status, + ).toBe(200); + expect((yield* getPackage(client, url, project, hash)).status).toBe(200); + + expect( + (yield* deleteTag(client, url, authToken, project, "main")).status, + ).toBe(200); + }), + { timeout: 180_000 }, +); + +test( + "TTL expiry deletes a tarball that is not tied to a pull request", + Effect.gen(function* () { + const { url, authToken } = yield* stack; + const client = yield* HttpClient.HttpClient; + expect((yield* warmUp(client, url)).status).toBe(401); + + const project = "ttl-no-pr"; + const content = "ttl-no-pr-bundle"; + const hash = sha256(content); + + expect( + (yield* pollUntilStatus( + upload(client, url, authToken, project, content), + 200, + )).status, + ).toBe(200); + expect( + (yield* pollUntilStatus( + assignTags(client, url, authToken, project, hash, ["ephemeral"], { + "Alchemy-TTL": "5 seconds", + }), + 200, + )).status, + ).toBe(200); + + const gone = yield* pollUntilStatus( + getPackage(client, url, project, hash), + 404, + 90, + ); + expect(gone.status).toBe(404); + }), + { timeout: 180_000 }, +); + +test( + "TTL expiry renews while the tied pull request is still open", + Effect.gen(function* () { + const { url, authToken } = yield* stack; + const client = yield* HttpClient.HttpClient; + expect((yield* warmUp(client, url)).status).toBe(401); + + const github = yield* client.get( + "https://api.github.com/repos/nodejs/node/pulls?state=open&per_page=1", + ); + expect(github.status).toBe(200); + const pulls = (yield* github.json) as Array<{ + number?: number; + html_url?: string; + }>; + const open = pulls[0]; + expect(open?.number).toBeGreaterThan(0); + + const project = "ttl-open-pr"; + const content = "ttl-open-pr-bundle"; + const hash = sha256(content); + + expect( + (yield* pollUntilStatus( + upload(client, url, authToken, project, content), + 200, + )).status, + ).toBe(200); + expect( + (yield* pollUntilStatus( + assignTags( + client, + url, + authToken, + project, + hash, + [`pr-${open!.number}`], + { + "Alchemy-TTL": "5 seconds", + "Alchemy-Pull-Request": `nodejs/node#${open!.number}`, + }, + ), + 200, + )).status, + ).toBe(200); + + yield* Effect.sleep(Duration.millis(20_000)); + expect((yield* getPackage(client, url, project, hash)).status).toBe(200); + + expect( + (yield* deletePullRequest(client, url, authToken, project, open!.number!)) + .status, + ).toBe(200); + }), + { timeout: 180_000 }, +); diff --git a/packages/pr-package/README.md b/packages/pr-package/README.md index 7cba55b5ca..c6a8a77a8e 100644 --- a/packages/pr-package/README.md +++ b/packages/pr-package/README.md @@ -117,10 +117,16 @@ Headers: - `Alchemy-Tarball-Hash: ` (required) - `Alchemy-Tags: ` (required) — e.g. `["main","abc1234","abc1234abc1234..."]` - `Alchemy-TTL: ` (optional) — e.g. `"7 hours"`, `"3 weeks"`. Effect `Duration` syntax. +- `Alchemy-Pull-Request: ` (optional) — e.g. `alchemy-run/alchemy#123` or `https://github.com/alchemy-run/alchemy/pull/123`. Ties this tarball to a GitHub pull request. If a tag already points elsewhere, it moves to the new tarball. A tarball is deleted after its final tag is removed. -Assigning tags schedules a named Durable Object expiration event. When it fires, the service removes every KV tag that still points to that tarball, deletes the R2 blob, and clears the tarball state. Reassigning the tarball before expiry reschedules the event. +Assigning tags schedules a named Durable Object expiration event. When it fires: + +- If the tarball is **not** tied to a pull request, every KV tag that still points at it is removed, the R2 blob is deleted, and state is cleared. +- If it **is** tied to a pull request, the service checks GitHub. An open (or unreadable) PR renews the TTL. A closed PR drops only the tags that were assigned with that PR — other tags on the same content-addressed tarball, such as `main`, are left alone. + +Reassigning the tarball before expiry reschedules the event. ### `GET /` — pretty install URL → 301 @@ -138,6 +144,12 @@ Returns the `.tgz` with `cache-control: public, max-age=31536000, immutable`. No Auth required. If the tag was the tarball's last one, the backing blob is also deleted. +### `DELETE /projects/:pkgName/pull-requests/:number` — tear down a PR preview + +Auth required. Looks up the `pr-` tag and removes every tag that was assigned together with that pull request (commit, branch, and `pr-N` aliases). Tags that were pointed at the same tarball without the PR (for example `main`) are kept. If no tags remain, the backing blob is deleted. + +Use this from CI on `pull_request` closed so preview install URLs stop resolving immediately instead of waiting for the next TTL. + ### `GET /projects/:pkgName/packages/:sha256/stats` — download stats Auth required. Returns `{ downloads: { [tag]: number }, totalDownloads: number }`. @@ -171,7 +183,7 @@ bun add https://pkg.example.com/projects/my-pkg/tags/abc1234 bun add https://pkg.example.com/my-pkg/abc1234 ``` -See `.github/workflows/pr-package.yaml` in this repo for the full pipeline (publish on push/PR sync, sticky comment with install URLs, tag cleanup on PR close). +See `.github/workflows/pr-package.yml` in this repo for the full pipeline (publish on push/PR sync, sticky comment with install URLs, PR-tied TTL renewal while the PR is open, tag cleanup and a teardown comment on PR close). ## Cleaning up state diff --git a/packages/pr-package/src/PackageStore.ts b/packages/pr-package/src/PackageStore.ts index 8ff1910812..eb84edaa4e 100644 --- a/packages/pr-package/src/PackageStore.ts +++ b/packages/pr-package/src/PackageStore.ts @@ -2,6 +2,11 @@ import * as Cloudflare from "alchemy/Cloudflare"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import { Bucket } from "./Bucket.ts"; +import { + pullRequestState, + shouldRenewOnTtl, + type PullRequestRef, +} from "./PullRequest.ts"; import { TagIndex } from "./TagIndex.ts"; import { tarballId, tarballKey, tarballRef } from "./Tarball.ts"; @@ -12,6 +17,14 @@ interface PackageState { expiresAt: number; downloads: Record; totalDownloads: number; + pullRequest?: PullRequestRef; + ttlMillis?: number; + prTags?: string[]; +} + +export interface InitOptions { + ttlMillis?: number; + pullRequest?: PullRequestRef; } const emptyState: PackageState = { @@ -25,6 +38,7 @@ const emptyState: PackageState = { const EXPIRATION_EVENT = "expire"; const RETRY_DELAY_MS = 60_000; +const WEEK_MS = 7 * 24 * 60 * 60 * 1000; export default class PackageStore extends Cloudflare.DurableObject()( "PackageStore", @@ -47,20 +61,70 @@ export default class PackageStore extends Cloudflare.DurableObject new Date(expiresAt), null, ).pipe(Effect.provideService(Cloudflare.DurableObjectState, doState)); + const cancelExpiration = Cloudflare.Workers.cancelEvent( + EXPIRATION_EVENT, + ).pipe(Effect.provideService(Cloudflare.DurableObjectState, doState)); const processExpirations = Cloudflare.Workers.processScheduledEvents.pipe( Effect.provideService(Cloudflare.DurableObjectState, doState), ); + const expireTags = ( + current: PackageState, + tagsToRemove: Iterable, + ) => + Effect.gen(function* () { + if (!current.packageName || !current.hash) return; + const ref = tarballRef(current.packageName, current.hash); + const id = tarballId(ref); + const removing = new Set(tagsToRemove); + for (const tag of removing) { + const key = `tag:${current.packageName}:${tag}`; + if ((yield* kv.get(key)) === id) { + yield* kv.delete(key); + } + } + + const remaining = current.tags.filter((tag) => !removing.has(tag)); + if (remaining.length === 0) { + yield* r2.delete(tarballKey(ref)).pipe(Effect.orDie); + yield* doState.storage.delete("state"); + return; + } + + yield* setState({ + packageName: current.packageName, + hash: current.hash, + tags: remaining, + expiresAt: 0, + downloads: current.downloads, + totalDownloads: current.totalDownloads, + }); + yield* cancelExpiration; + }); + + const tagsForPullRequest = (current: PackageState, number: number) => { + const tags = new Set([`pr-${number}`]); + if (current.pullRequest?.number === number) { + for (const tag of current.prTags ?? []) tags.add(tag); + } + return tags; + }; + return { init: ( packageName: string, hash: string, tags: string[], expiresAt: number, + options?: InitOptions, ) => Effect.gen(function* () { const current = yield* getState; const merged = new Set([...current.tags, ...tags]); + const pullRequest = options?.pullRequest ?? current.pullRequest; + const prTags = options?.pullRequest + ? [...new Set([...(current.prTags ?? []), ...tags])] + : current.prTags; const newState: PackageState = { packageName, hash, @@ -69,6 +133,10 @@ export default class PackageStore extends Cloudflare.DurableObject downloads: current.downloads, totalDownloads: current.totalDownloads, }; + if (pullRequest) newState.pullRequest = pullRequest; + const ttlMillis = options?.ttlMillis ?? current.ttlMillis; + if (ttlMillis) newState.ttlMillis = ttlMillis; + if (prTags && prTags.length > 0) newState.prTags = prTags; yield* setState(newState); yield* scheduleExpiration(expiresAt); }), @@ -81,6 +149,12 @@ export default class PackageStore extends Cloudflare.DurableObject return { orphaned: tags.length === 0 }; }), + expirePullRequest: (number: number) => + Effect.gen(function* () { + const current = yield* getState; + yield* expireTags(current, tagsForPullRequest(current, number)); + }), + recordDownload: (tag: string) => Effect.gen(function* () { const current = yield* getState; @@ -113,17 +187,26 @@ export default class PackageStore extends Cloudflare.DurableObject const current = yield* getState; if (!current.packageName || !current.hash) return; - const ref = tarballRef(current.packageName, current.hash); - const id = tarballId(ref); - for (const tag of current.tags) { - const key = `tag:${current.packageName}:${tag}`; - if ((yield* kv.get(key)) === id) { - yield* kv.delete(key); + if (current.pullRequest) { + const state = yield* pullRequestState(current.pullRequest); + if (shouldRenewOnTtl(true, state)) { + const ttl = + current.ttlMillis && current.ttlMillis > 0 + ? current.ttlMillis + : WEEK_MS; + const expiresAt = Date.now() + ttl; + yield* setState({ ...current, expiresAt }); + yield* scheduleExpiration(expiresAt); + return; } + yield* expireTags( + current, + tagsForPullRequest(current, current.pullRequest.number), + ); + return; } - yield* r2.delete(tarballKey(ref)).pipe(Effect.orDie); - yield* doState.storage.delete("state"); + yield* expireTags(current, current.tags); }).pipe( Effect.catchCause((cause) => scheduleExpiration(Date.now() + RETRY_DELAY_MS).pipe( diff --git a/packages/pr-package/src/PullRequest.ts b/packages/pr-package/src/PullRequest.ts new file mode 100644 index 0000000000..5c37a808f9 --- /dev/null +++ b/packages/pr-package/src/PullRequest.ts @@ -0,0 +1,95 @@ +import * as Effect from "effect/Effect"; +import * as FetchHttpClient from "effect/unstable/http/FetchHttpClient"; +import * as HttpClient from "effect/unstable/http/HttpClient"; +import * as HttpClientRequest from "effect/unstable/http/HttpClientRequest"; + +export interface PullRequestRef { + owner: string; + repo: string; + number: number; +} + +export type PullRequestState = "open" | "closed" | "unknown"; + +const OWNER_REPO = /^[A-Za-z0-9_.-]+$/; + +/** + * Parse `Alchemy-Pull-Request`. Returns `undefined` when the header is + * absent, `"invalid"` when it is present but malformed. + * + * Accepted forms: `owner/repo#123`, `owner/repo/123`, + * `https://github.com/owner/repo/pull/123`. + */ +export const parsePullRequest = ( + raw: string | undefined, +): PullRequestRef | undefined | "invalid" => { + if (raw === undefined) return undefined; + const value = raw.trim(); + if (value.length === 0) return undefined; + + const match = + value.match( + /^https?:\/\/github\.com\/([^/]+)\/([^/]+)\/pull\/(\d+)\/?$/i, + ) ?? + value.match(/^([^/#]+)\/([^/#]+)#(\d+)$/) ?? + value.match(/^([^/#]+)\/([^/#]+)\/(\d+)$/); + if (!match) return "invalid"; + + const owner = match[1]!; + const repo = match[2]!; + const number = Number(match[3]); + if ( + !OWNER_REPO.test(owner) || + !OWNER_REPO.test(repo) || + !Number.isSafeInteger(number) || + number <= 0 + ) { + return "invalid"; + } + return { owner, repo, number }; +}; + +export const formatPullRequest = (pr: PullRequestRef) => + `${pr.owner}/${pr.repo}#${pr.number}`; + +/** Renew TTL when a tarball is PR-tied and GitHub has not confirmed closed. */ +export const shouldRenewOnTtl = ( + tiedToPullRequest: boolean, + state: PullRequestState, +): boolean => tiedToPullRequest && state !== "closed"; + +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null; + +/** + * Best-effort GitHub pull-request state. Network errors, rate limits, and + * private/missing repos return `"unknown"` so a TTL handler can fail closed + * (renew) instead of deleting a still-open preview. + */ +export const pullRequestState = ( + pr: PullRequestRef, +): Effect.Effect => + Effect.gen(function* () { + const client = yield* HttpClient.HttpClient; + const response = yield* client.execute( + HttpClientRequest.get( + `https://api.github.com/repos/${pr.owner}/${pr.repo}/pulls/${pr.number}`, + ).pipe( + HttpClientRequest.setHeader( + "User-Agent", + "alchemy-pr-package (https://github.com/alchemy-run/alchemy)", + ), + HttpClientRequest.setHeader("Accept", "application/vnd.github+json"), + HttpClientRequest.setHeader("X-GitHub-Api-Version", "2022-11-28"), + ), + ); + if (response.status !== 200) return "unknown" as const; + const body: unknown = yield* response.json; + if (!isRecord(body)) return "unknown" as const; + if (body.state === "closed") return "closed" as const; + if (body.state === "open") return "open" as const; + return "unknown" as const; + }).pipe( + Effect.provide(FetchHttpClient.layer), + Effect.catchCause(() => Effect.succeed("unknown" as const)), + ); diff --git a/packages/pr-package/src/Worker.ts b/packages/pr-package/src/Worker.ts index a4aa25a51c..1d22fd6173 100644 --- a/packages/pr-package/src/Worker.ts +++ b/packages/pr-package/src/Worker.ts @@ -13,6 +13,7 @@ import { import { AuthToken } from "./AuthToken.ts"; import { Bucket } from "./Bucket.ts"; import PackageStore from "./PackageStore.ts"; +import { formatPullRequest, parsePullRequest } from "./PullRequest.ts"; import { TagIndex } from "./TagIndex.ts"; import { tarballId, @@ -67,6 +68,10 @@ const parseTags = (raw: string | undefined) => { * * The user's stack file must be the worker entry (`main: import.meta.url`) * because `parseAliasUrl` is a closure that has to live in the bundle. + * + * Tag assignment accepts an optional `Alchemy-Pull-Request` header + * (`owner/repo#123`). When a tarball is tied to a pull request, TTL expiry + * checks GitHub and renews while the PR is open instead of deleting. */ export const handler = (options: HandlerOptions = {}) => Effect.gen(function* () { @@ -242,6 +247,19 @@ export const handler = (options: HandlerOptions = {}) => } const expiresAt = Date.now() + ttlMillis; + const pullRequest = parsePullRequest( + request.headers["alchemy-pull-request"], + ); + if (pullRequest === "invalid") { + return yield* HttpServerResponse.json( + { + error: + "Alchemy-Pull-Request must be owner/repo#number or a GitHub pull URL", + }, + { status: 400 }, + ); + } + for (const tag of tags) { const oldId = yield* kv.get(`tag:${project}:${tag}`); if (oldId && oldId !== id) { @@ -262,7 +280,10 @@ export const handler = (options: HandlerOptions = {}) => const store = packages.getByName(id); yield* store - .init(project, hash, tags, expiresAt) + .init(project, hash, tags, expiresAt, { + ttlMillis, + ...(pullRequest ? { pullRequest } : {}), + }) .pipe(Effect.orDie); return yield* HttpServerResponse.json({ @@ -272,6 +293,9 @@ export const handler = (options: HandlerOptions = {}) => tags, ttl: ttlStr, expiresAt, + pullRequest: pullRequest + ? formatPullRequest(pullRequest) + : undefined, }); }).pipe( Effect.catchTag("Unauthorized", () => @@ -338,6 +362,39 @@ export const handler = (options: HandlerOptions = {}) => ); } + const pullRequestMatch = subPath.match(/^\/pull-requests\/(\d+)$/); + if (method === "DELETE" && pullRequestMatch) { + return yield* Effect.gen(function* () { + yield* requireAuth; + + const number = Number(pullRequestMatch[1]); + const tag = `pr-${number}`; + const id = yield* kv.get(`tag:${project}:${tag}`); + if (!id) { + return yield* HttpServerResponse.json( + { error: "pull request preview not found" }, + { status: 404 }, + ); + } + + const store = packages.getByName(id); + yield* store.expirePullRequest(number).pipe(Effect.orDie); + + if ((yield* kv.get(`tag:${project}:${tag}`)) === id) { + yield* kv.delete(`tag:${project}:${tag}`); + } + + return yield* HttpServerResponse.json({ ok: true }); + }).pipe( + Effect.catchTag("Unauthorized", () => + HttpServerResponse.json( + { error: "unauthorized" }, + { status: 401 }, + ), + ), + ); + } + if (method === "DELETE" && subPath.startsWith("/tags/")) { return yield* Effect.gen(function* () { yield* requireAuth; diff --git a/packages/pr-package/test/PullRequest.test.ts b/packages/pr-package/test/PullRequest.test.ts new file mode 100644 index 0000000000..63a06ac726 --- /dev/null +++ b/packages/pr-package/test/PullRequest.test.ts @@ -0,0 +1,43 @@ +import { expect, test } from "bun:test"; +import { + formatPullRequest, + parsePullRequest, + shouldRenewOnTtl, +} from "../src/PullRequest.ts"; + +test("parsePullRequest accepts owner/repo#number and GitHub URLs", () => { + expect(parsePullRequest(undefined)).toBeUndefined(); + expect(parsePullRequest("")).toBeUndefined(); + expect(parsePullRequest(" ")).toBeUndefined(); + expect(parsePullRequest("not-a-pr")).toBe("invalid"); + expect(parsePullRequest("alchemy-run/alchemy#550")).toEqual({ + owner: "alchemy-run", + repo: "alchemy", + number: 550, + }); + expect( + parsePullRequest("https://github.com/alchemy-run/alchemy/pull/550"), + ).toEqual({ + owner: "alchemy-run", + repo: "alchemy", + number: 550, + }); + expect(parsePullRequest("alchemy-run/alchemy/550")).toEqual({ + owner: "alchemy-run", + repo: "alchemy", + number: 550, + }); + expect(parsePullRequest("acme/widgets#0")).toBe("invalid"); + expect( + formatPullRequest({ owner: "alchemy-run", repo: "alchemy", number: 550 }), + ).toBe("alchemy-run/alchemy#550"); +}); + +test("shouldRenewOnTtl keeps PR-tied tarballs unless GitHub says closed", () => { + expect(shouldRenewOnTtl(false, "closed")).toBe(false); + expect(shouldRenewOnTtl(false, "open")).toBe(false); + expect(shouldRenewOnTtl(false, "unknown")).toBe(false); + expect(shouldRenewOnTtl(true, "closed")).toBe(false); + expect(shouldRenewOnTtl(true, "open")).toBe(true); + expect(shouldRenewOnTtl(true, "unknown")).toBe(true); +}); diff --git a/scripts/bind-pr-packages.ts b/scripts/bind-pr-packages.ts new file mode 100644 index 0000000000..4074040359 --- /dev/null +++ b/scripts/bind-pr-packages.ts @@ -0,0 +1,82 @@ +#!/usr/bin/env bun +/** + * Re-point a PR publish's tags with Alchemy-Pull-Request so the pr-package + * worker renews TTL while the PR stays open. + * + * The publish action may not send that header yet; this step is the + * repo-local contract until it does. + */ +type Package = { + project: string; + tags: string[]; +}; + +type Plan = { + packages: Package[]; +}; + +const plan = JSON.parse(required("PLAN")) as Plan; +const host = process.env.PR_PACKAGE_HOST?.trim() || "pkg.ing"; +const token = required("TOKEN"); +const pullRequest = required("PULL_REQUEST"); +const ttl = process.env.TTL?.trim() || "1 week"; + +function required(name: string): string { + const value = process.env[name]?.trim(); + if (!value) { + throw new Error(`${name} is required`); + } + return value; +} + +function projectUrl(project: string): string { + const path = project.split("/").map(encodeURIComponent).join("/"); + return `https://${host}/projects/${path}`; +} + +async function tarballHash( + project: string, + tag: string, +): Promise { + const url = `${projectUrl(project)}/tags/${encodeURIComponent(tag)}`; + for (let attempt = 0; attempt < 15; attempt++) { + const response = await fetch(url, { redirect: "manual" }); + if (response.status === 302) { + const location = response.headers.get("location") ?? ""; + const match = location.match(/\/packages\/([a-f0-9]{64})\/?$/); + if (match) return match[1]; + } + await Bun.sleep(1000); + } + return undefined; +} + +for (const pkg of plan.packages) { + const prTag = pkg.tags.find((tag) => /^pr-\d+$/.test(tag)); + if (!prTag) continue; + + const hash = await tarballHash(pkg.project, prTag); + if (!hash) { + throw new Error( + `Could not resolve ${pkg.project} tag ${prTag} to a tarball`, + ); + } + + const response = await fetch(`${projectUrl(pkg.project)}/tags`, { + method: "PUT", + headers: { + Authorization: `Bearer ${token}`, + "Alchemy-Tags": JSON.stringify(pkg.tags), + "Alchemy-Tarball-Hash": hash, + "Alchemy-TTL": ttl, + "Alchemy-Pull-Request": pullRequest, + }, + }); + if (!response.ok) { + const details = await response.text(); + throw new Error( + `Failed to bind ${pkg.project} to ${pullRequest}: ${response.status} ${response.statusText}${details ? `\n${details}` : ""}`, + ); + } + console.log(`Bound ${pkg.project} ${prTag} to ${pullRequest}`); +} From 4b7bd3d2a9dda6a0df20df1a15d65912e095d4e6 Mon Sep 17 00:00:00 2001 From: "Michael K (Pear)" Date: Mon, 7 Sep 2026 05:57:39 -0400 Subject: [PATCH 2/2] fix(pr-package): track preview tags per pull request Closing one PR could release tags another open PR still claimed on the same content-addressed tarball. Tags are now recorded per PR; a closed PR releases only tags no other open PR claims. GitHub lookups accept a bound GITHUB_TOKEN and unverifiable PRs stop renewing after 28 days. Use actions#12 so the publish action sends Alchemy-Pull-Request itself. --- .github/workflows/check.yml | 3 + .github/workflows/pr-package.yml | 19 +- .../cloudflare-pr-package/test/integ.test.ts | 115 ++++++++--- packages/pr-package/README.md | 7 +- packages/pr-package/package.json | 3 +- packages/pr-package/src/PackageState.ts | 113 +++++++++++ packages/pr-package/src/PackageStore.ts | 181 ++++++++++-------- packages/pr-package/src/PullRequest.ts | 70 +++++-- packages/pr-package/test/PullRequest.test.ts | 120 +++++++++++- scripts/bind-pr-packages.ts | 82 -------- 10 files changed, 490 insertions(+), 223 deletions(-) create mode 100644 packages/pr-package/src/PackageState.ts delete mode 100644 scripts/bind-pr-packages.ts diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index f2b323fc83..1e48e00bf7 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -84,6 +84,9 @@ jobs: - name: Run Node Utils tests run: pnpm -C packages/node-utils test + - name: Run PR Package tests + run: pnpm -C packages/pr-package test + - name: Run Typecheck run: pnpm tsc -b diff --git a/.github/workflows/pr-package.yml b/.github/workflows/pr-package.yml index 4771a7572f..ffd8302dbf 100644 --- a/.github/workflows/pr-package.yml +++ b/.github/workflows/pr-package.yml @@ -84,7 +84,7 @@ jobs: - name: Publish packages id: publish - uses: alchemy-run/actions/actions/pr-package@098beeda4d3c2ce8850638c4466783893b8f0c23 # actions#11 + uses: alchemy-run/actions/actions/pr-package@148b694161fe91bf90767385ad34667145593394 # actions#12 with: rebuild-all-paths: submodules/distilled/** packages: >- @@ -106,18 +106,6 @@ jobs: ] pr-package-token: ${{ secrets.PR_PACKAGE_TOKEN }} - - name: Tie preview packages to this PR - if: >- - github.event_name == 'pull_request' && - fromJson(steps.publish.outputs.plan).packages[0] != null - env: - PLAN: ${{ steps.publish.outputs.plan }} - TOKEN: ${{ secrets.PR_PACKAGE_TOKEN }} - PR_PACKAGE_HOST: pkg.ing - PULL_REQUEST: ${{ github.repository }}#${{ github.event.pull_request.number }} - TTL: 1 week - run: bun scripts/bind-pr-packages.ts - - name: Generate bot token if: >- github.event_name == 'pull_request' && @@ -132,7 +120,7 @@ jobs: if: >- github.event_name == 'pull_request' && fromJson(steps.publish.outputs.plan).packages[0] != null - uses: alchemy-run/actions/actions/pr-package-comment@098beeda4d3c2ce8850638c4466783893b8f0c23 # actions#11 + uses: alchemy-run/actions/actions/pr-package-comment@148b694161fe91bf90767385ad34667145593394 # actions#12 with: plan: ${{ steps.publish.outputs.plan }} token: ${{ steps.bot-token.outputs.token }} @@ -230,7 +218,8 @@ jobs: "", "Preview packages for this PR have been removed.", "", - "Install URLs from earlier comments no longer resolve.", + "The install URLs from this comment no longer resolve. " + "Older per-commit URLs expire with their TTL.", "", ] ) diff --git a/examples/cloudflare-pr-package/test/integ.test.ts b/examples/cloudflare-pr-package/test/integ.test.ts index 7440a6b55d..2a959927cc 100644 --- a/examples/cloudflare-pr-package/test/integ.test.ts +++ b/examples/cloudflare-pr-package/test/integ.test.ts @@ -434,6 +434,23 @@ test( )).status, ).toBe(200); + // A second PR pins the same content: it shares `deadbeef` and adds its + // own `pr-100`. Closing PR 99 must not take `deadbeef` away from PR 100. + expect( + (yield* pollUntilStatus( + assignTags( + client, + url, + authToken, + project, + hash, + ["pr-100", "deadbeef"], + { "Alchemy-Pull-Request": "alchemy-run/alchemy#100" }, + ), + 200, + )).status, + ).toBe(200); + expect( (yield* deletePullRequest(client, url, authToken, project, 99)).status, ).toBe(200); @@ -442,6 +459,24 @@ test( (yield* pollUntilStatus(getTag(client, url, project, "pr-99"), 404)) .status, ).toBe(404); + expect( + (yield* pollUntilStatus(getTag(client, url, project, "deadbeef"), 200)) + .status, + ).toBe(200); + expect( + (yield* pollUntilStatus(getTag(client, url, project, "pr-100"), 200)) + .status, + ).toBe(200); + + // Closing the last PR releases the shared tag; `main` still holds the + // tarball. + expect( + (yield* deletePullRequest(client, url, authToken, project, 100)).status, + ).toBe(200); + expect( + (yield* pollUntilStatus(getTag(client, url, project, "pr-100"), 404)) + .status, + ).toBe(404); expect( (yield* pollUntilStatus(getTag(client, url, project, "deadbeef"), 404)) .status, @@ -455,6 +490,10 @@ test( expect( (yield* deleteTag(client, url, authToken, project, "main")).status, ).toBe(200); + expect( + (yield* pollUntilStatus(getPackage(client, url, project, hash), 404)) + .status, + ).toBe(404); }), { timeout: 180_000 }, ); @@ -496,23 +535,15 @@ test( ); test( - "TTL expiry renews while the tied pull request is still open", + "TTL expiry renews while a tied pull request cannot be confirmed closed", Effect.gen(function* () { const { url, authToken } = yield* stack; const client = yield* HttpClient.HttpClient; expect((yield* warmUp(client, url)).status).toBe(401); - const github = yield* client.get( - "https://api.github.com/repos/nodejs/node/pulls?state=open&per_page=1", - ); - expect(github.status).toBe(200); - const pulls = (yield* github.json) as Array<{ - number?: number; - html_url?: string; - }>; - const open = pulls[0]; - expect(open?.number).toBeGreaterThan(0); - + // A PR number GitHub will never resolve: the worker sees "unknown" and + // must fail closed (renew) since the PR was tied less than 28 days ago. + const number = 999_999_999; const project = "ttl-open-pr"; const content = "ttl-open-pr-bundle"; const hash = sha256(content); @@ -525,18 +556,10 @@ test( ).toBe(200); expect( (yield* pollUntilStatus( - assignTags( - client, - url, - authToken, - project, - hash, - [`pr-${open!.number}`], - { - "Alchemy-TTL": "5 seconds", - "Alchemy-Pull-Request": `nodejs/node#${open!.number}`, - }, - ), + assignTags(client, url, authToken, project, hash, [`pr-${number}`], { + "Alchemy-TTL": "5 seconds", + "Alchemy-Pull-Request": `alchemy-run/alchemy#${number}`, + }), 200, )).status, ).toBe(200); @@ -545,9 +568,51 @@ test( expect((yield* getPackage(client, url, project, hash)).status).toBe(200); expect( - (yield* deletePullRequest(client, url, authToken, project, open!.number!)) + (yield* deletePullRequest(client, url, authToken, project, number)) .status, ).toBe(200); + expect( + (yield* pollUntilStatus(getPackage(client, url, project, hash), 404)) + .status, + ).toBe(404); + }), + { timeout: 180_000 }, +); + +// The worker reads `GITHUB_TOKEN` at deploy time; without it the lookup may be +// rate-limited into "unknown" and this test would only prove the renew path. +test.skipIf(!process.env.GITHUB_TOKEN)( + "TTL expiry deletes a tarball whose only tied pull request is closed", + Effect.gen(function* () { + const { url, authToken } = yield* stack; + const client = yield* HttpClient.HttpClient; + expect((yield* warmUp(client, url)).status).toBe(401); + + // alchemy-run/alchemy#1 is long closed. + const project = "ttl-closed-pr"; + const content = "ttl-closed-pr-bundle"; + const hash = sha256(content); + + expect( + (yield* pollUntilStatus( + upload(client, url, authToken, project, content), + 200, + )).status, + ).toBe(200); + expect( + (yield* pollUntilStatus( + assignTags(client, url, authToken, project, hash, ["pr-1"], { + "Alchemy-TTL": "5 seconds", + "Alchemy-Pull-Request": "alchemy-run/alchemy#1", + }), + 200, + )).status, + ).toBe(200); + + expect( + (yield* pollUntilStatus(getPackage(client, url, project, hash), 404, 90)) + .status, + ).toBe(404); }), { timeout: 180_000 }, ); diff --git a/packages/pr-package/README.md b/packages/pr-package/README.md index c6a8a77a8e..d19e3d759d 100644 --- a/packages/pr-package/README.md +++ b/packages/pr-package/README.md @@ -124,10 +124,13 @@ If a tag already points elsewhere, it moves to the new tarball. A tarball is del Assigning tags schedules a named Durable Object expiration event. When it fires: - If the tarball is **not** tied to a pull request, every KV tag that still points at it is removed, the R2 blob is deleted, and state is cleared. -- If it **is** tied to a pull request, the service checks GitHub. An open (or unreadable) PR renews the TTL. A closed PR drops only the tags that were assigned with that PR — other tags on the same content-addressed tarball, such as `main`, are left alone. +- If it **is** tied to one or more pull requests, the service checks each on GitHub. While any tied PR is open the TTL renews. A closed PR releases only the tags that were assigned with that PR and that no other open PR also claims — other tags on the same content-addressed tarball, such as `main` or another PR's commit tag, are left alone. Once no tied PR remains open, the whole tarball expires. +- A PR GitHub cannot confirm (rate limit, outage, private repo) keeps renewing for up to 28 days after it was last seen open, then is treated as closed. Reassigning the tarball before expiry reschedules the event. +Set `GITHUB_TOKEN` in the deploy environment to bind a token for these lookups; unauthenticated GitHub requests share a 60/hour limit per egress IP, which is not enough for a busy registry. + ### `GET /` — pretty install URL → 301 Whenever the path doesn't start with `/projects/`, the request URL is handed to `parseAliasUrl(url)`. If it returns a match, the worker 301s to `/projects/:pkgName/tags/:tag`. Otherwise 404. @@ -146,7 +149,7 @@ Auth required. If the tag was the tarball's last one, the backing blob is also d ### `DELETE /projects/:pkgName/pull-requests/:number` — tear down a PR preview -Auth required. Looks up the `pr-` tag and removes every tag that was assigned together with that pull request (commit, branch, and `pr-N` aliases). Tags that were pointed at the same tarball without the PR (for example `main`) are kept. If no tags remain, the backing blob is deleted. +Auth required. Looks up the `pr-` tag and removes every tag that was assigned together with that pull request (commit, branch, and `pr-N` aliases). Tags that were pointed at the same tarball without the PR (for example `main`) or that another open PR also claims are kept. If no tags remain, the backing blob is deleted. Use this from CI on `pull_request` closed so preview install URLs stop resolving immediately instead of waiting for the next TTL. diff --git a/packages/pr-package/package.json b/packages/pr-package/package.json index dffdc99a94..adb5c6559c 100644 --- a/packages/pr-package/package.json +++ b/packages/pr-package/package.json @@ -27,7 +27,8 @@ "NOTICE" ], "scripts": { - "build": "tsc -b && bun ../../scripts/copy-package-files.ts pr-package" + "build": "tsc -b && bun ../../scripts/copy-package-files.ts pr-package", + "test": "bun test ./test" }, "exports": { ".": { diff --git a/packages/pr-package/src/PackageState.ts b/packages/pr-package/src/PackageState.ts new file mode 100644 index 0000000000..bad3c12e6f --- /dev/null +++ b/packages/pr-package/src/PackageState.ts @@ -0,0 +1,113 @@ +import { formatPullRequest, type PullRequestRef } from "./PullRequest.ts"; + +/** + * Tags a pull request assigned to a tarball. A content-addressed tarball + * can be tied to several PRs at once (e.g. distilled packages pinned to the + * same commit), so each PR owns its own tag list. + */ +export interface PullRequestBinding { + ref: PullRequestRef; + tags: string[]; + /** + * Last time the PR was known to be open: set when CI ties it (a publish + * proves the PR is open) and refreshed whenever GitHub reports `open`. + * Bounds renewals when GitHub is unreachable. + */ + verifiedAt: number; +} + +export interface PackageState { + packageName: string; + hash: string; + tags: string[]; + expiresAt: number; + downloads: Record; + totalDownloads: number; + ttlMillis?: number; + pullRequests?: Record; +} + +export const emptyState: PackageState = { + packageName: "", + hash: "", + tags: [], + expiresAt: 0, + downloads: {}, + totalDownloads: 0, +}; + +export const pullRequestBindings = ( + state: PackageState, +): PullRequestBinding[] => Object.values(state.pullRequests ?? {}); + +/** Record that `tags` were assigned to this tarball on behalf of `ref`. */ +export const tiePullRequest = ( + state: PackageState, + ref: PullRequestRef, + tags: string[], + now: number, +): Record => { + const key = formatPullRequest(ref); + const existing = state.pullRequests?.[key]; + return { + ...state.pullRequests, + [key]: { + ref, + tags: [...new Set([...(existing?.tags ?? []), ...tags])], + verifiedAt: now, + }, + }; +}; + +/** + * Remove `tags` from the tarball and from every PR binding. A binding left + * with no tags is dropped, so a tarball only renews while a tied PR still + * owns at least one of its tags. + */ +export const withoutTags = ( + state: PackageState, + tags: Iterable, +): PackageState => { + const removing = new Set(tags); + const next: PackageState = { + ...state, + tags: state.tags.filter((tag) => !removing.has(tag)), + }; + delete next.pullRequests; + const pullRequests: Record = {}; + for (const [key, binding] of Object.entries(state.pullRequests ?? {})) { + const remaining = binding.tags.filter((tag) => !removing.has(tag)); + if (remaining.length > 0) { + pullRequests[key] = { ...binding, tags: remaining }; + } + } + if (Object.keys(pullRequests).length > 0) next.pullRequests = pullRequests; + return next; +}; + +/** + * Release the PR bindings matching `release`. Returns the tags that no + * remaining PR still claims (those are safe to delete) and the state with + * the released bindings removed. `pr-` is always released with its PR. + */ +export const releasePullRequests = ( + state: PackageState, + release: (binding: PullRequestBinding) => boolean, +): { tags: Set; state: PackageState } => { + const kept: Record = {}; + const owned = new Set(); + for (const [key, binding] of Object.entries(state.pullRequests ?? {})) { + if (release(binding)) { + for (const tag of binding.tags) owned.add(tag); + owned.add(`pr-${binding.ref.number}`); + } else { + kept[key] = binding; + } + } + const claimed = new Set(Object.values(kept).flatMap((b) => b.tags)); + const tags = new Set([...owned].filter((tag) => !claimed.has(tag))); + const next: PackageState = { ...state }; + delete next.pullRequests; + if (Object.keys(kept).length > 0) next.pullRequests = kept; + return { tags, state: next }; +}; diff --git a/packages/pr-package/src/PackageStore.ts b/packages/pr-package/src/PackageStore.ts index eb84edaa4e..857d619b39 100644 --- a/packages/pr-package/src/PackageStore.ts +++ b/packages/pr-package/src/PackageStore.ts @@ -1,50 +1,50 @@ import * as Cloudflare from "alchemy/Cloudflare"; +import * as Config from "effect/Config"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; import { Bucket } from "./Bucket.ts"; import { + emptyState, + pullRequestBindings, + releasePullRequests, + tiePullRequest, + withoutTags, + type PackageState, +} from "./PackageState.ts"; +import { + formatPullRequest, + isStillOpen, pullRequestState, - shouldRenewOnTtl, type PullRequestRef, } from "./PullRequest.ts"; import { TagIndex } from "./TagIndex.ts"; import { tarballId, tarballKey, tarballRef } from "./Tarball.ts"; -interface PackageState { - packageName: string; - hash: string; - tags: string[]; - expiresAt: number; - downloads: Record; - totalDownloads: number; - pullRequest?: PullRequestRef; - ttlMillis?: number; - prTags?: string[]; -} - export interface InitOptions { ttlMillis?: number; pullRequest?: PullRequestRef; } -const emptyState: PackageState = { - packageName: "", - hash: "", - tags: [], - expiresAt: 0, - downloads: {}, - totalDownloads: 0, -}; - const EXPIRATION_EVENT = "expire"; const RETRY_DELAY_MS = 60_000; const WEEK_MS = 7 * 24 * 60 * 60 * 1000; +/** + * Optional GitHub token for PR state lookups on TTL expiry. Bound as a + * Worker secret when `GITHUB_TOKEN` is set at deploy time. + */ +export const GitHubToken = Config.redacted("GITHUB_TOKEN").pipe(Config.option); + export default class PackageStore extends Cloudflare.DurableObject()( "PackageStore", Effect.gen(function* () { const r2 = yield* Cloudflare.R2.ReadWriteBucket(yield* Bucket); const kv = yield* Cloudflare.KV.ReadWriteNamespace(yield* TagIndex); + const githubToken = yield* GitHubToken.pipe( + Effect.map(Option.getOrUndefined), + Effect.orElseSucceed(() => undefined), + ); return Effect.gen(function* () { const doState = yield* Cloudflare.DurableObjectState; @@ -68,6 +68,11 @@ export default class PackageStore extends Cloudflare.DurableObject Effect.provideService(Cloudflare.DurableObjectState, doState), ); + /** + * Drop `tagsToRemove` from KV (only where they still point here) and + * from state. Deletes the blob when no tags remain; otherwise the + * tarball keeps its remaining tags and its existing expiry. + */ const expireTags = ( current: PackageState, tagsToRemove: Iterable, @@ -84,32 +89,16 @@ export default class PackageStore extends Cloudflare.DurableObject } } - const remaining = current.tags.filter((tag) => !removing.has(tag)); - if (remaining.length === 0) { + const next = withoutTags(current, removing); + if (next.tags.length === 0) { yield* r2.delete(tarballKey(ref)).pipe(Effect.orDie); yield* doState.storage.delete("state"); + yield* cancelExpiration; return; } - - yield* setState({ - packageName: current.packageName, - hash: current.hash, - tags: remaining, - expiresAt: 0, - downloads: current.downloads, - totalDownloads: current.totalDownloads, - }); - yield* cancelExpiration; + yield* setState(next); }); - const tagsForPullRequest = (current: PackageState, number: number) => { - const tags = new Set([`pr-${number}`]); - if (current.pullRequest?.number === number) { - for (const tag of current.prTags ?? []) tags.add(tag); - } - return tags; - }; - return { init: ( packageName: string, @@ -120,39 +109,44 @@ export default class PackageStore extends Cloudflare.DurableObject ) => Effect.gen(function* () { const current = yield* getState; - const merged = new Set([...current.tags, ...tags]); - const pullRequest = options?.pullRequest ?? current.pullRequest; - const prTags = options?.pullRequest - ? [...new Set([...(current.prTags ?? []), ...tags])] - : current.prTags; - const newState: PackageState = { + const next: PackageState = { + ...current, packageName, hash, - tags: [...merged], + tags: [...new Set([...current.tags, ...tags])], expiresAt, - downloads: current.downloads, - totalDownloads: current.totalDownloads, }; - if (pullRequest) newState.pullRequest = pullRequest; const ttlMillis = options?.ttlMillis ?? current.ttlMillis; - if (ttlMillis) newState.ttlMillis = ttlMillis; - if (prTags && prTags.length > 0) newState.prTags = prTags; - yield* setState(newState); + if (ttlMillis) next.ttlMillis = ttlMillis; + if (options?.pullRequest) { + next.pullRequests = tiePullRequest( + current, + options.pullRequest, + tags, + Date.now(), + ); + } + yield* setState(next); yield* scheduleExpiration(expiresAt); }), removeTag: (tag: string) => Effect.gen(function* () { const current = yield* getState; - const tags = current.tags.filter((t) => t !== tag); - yield* setState({ ...current, tags }); - return { orphaned: tags.length === 0 }; + const next = withoutTags(current, [tag]); + yield* setState(next); + return { orphaned: next.tags.length === 0 }; }), + /** Tear down one PR's tags; tags another tied PR still claims stay. */ expirePullRequest: (number: number) => Effect.gen(function* () { const current = yield* getState; - yield* expireTags(current, tagsForPullRequest(current, number)); + const released = releasePullRequests( + current, + (binding) => binding.ref.number === number, + ); + yield* expireTags(released.state, released.tags); }), recordDownload: (tag: string) => @@ -186,27 +180,64 @@ export default class PackageStore extends Cloudflare.DurableObject yield* Effect.gen(function* () { const current = yield* getState; if (!current.packageName || !current.hash) return; + const now = Date.now(); - if (current.pullRequest) { - const state = yield* pullRequestState(current.pullRequest); - if (shouldRenewOnTtl(true, state)) { - const ttl = - current.ttlMillis && current.ttlMillis > 0 - ? current.ttlMillis - : WEEK_MS; - const expiresAt = Date.now() + ttl; - yield* setState({ ...current, expiresAt }); - yield* scheduleExpiration(expiresAt); - return; - } - yield* expireTags( - current, - tagsForPullRequest(current, current.pullRequest.number), + const bindings = pullRequestBindings(current); + if (bindings.length === 0) { + yield* expireTags(current, current.tags); + return; + } + + const states = new Map( + yield* Effect.forEach(bindings, (binding) => + pullRequestState(binding.ref, githubToken).pipe( + Effect.map( + (state) => + [formatPullRequest(binding.ref), state] as const, + ), + ), + ), + ); + const stateOf = (key: string) => states.get(key) ?? "unknown"; + + // Refresh `verifiedAt` for PRs GitHub confirmed open, then + // release every PR that is closed or unverified for too long. + const refreshed: PackageState = { + ...current, + pullRequests: Object.fromEntries( + Object.entries(current.pullRequests ?? {}).map( + ([key, binding]) => [ + key, + stateOf(key) === "open" + ? { ...binding, verifiedAt: now } + : binding, + ], + ), + ), + }; + const released = releasePullRequests(refreshed, (binding) => { + const key = formatPullRequest(binding.ref); + return !isStillOpen( + { state: stateOf(key), verifiedAt: binding.verifiedAt }, + now, ); + }); + + if (!released.state.pullRequests) { + yield* expireTags(released.state, released.state.tags); return; } - yield* expireTags(current, current.tags); + yield* expireTags(released.state, released.tags); + const ttl = + current.ttlMillis && current.ttlMillis > 0 + ? current.ttlMillis + : WEEK_MS; + const expiresAt = now + ttl; + const latest = yield* getState; + if (!latest.packageName) return; + yield* setState({ ...latest, expiresAt }); + yield* scheduleExpiration(expiresAt); }).pipe( Effect.catchCause((cause) => scheduleExpiration(Date.now() + RETRY_DELAY_MS).pipe( diff --git a/packages/pr-package/src/PullRequest.ts b/packages/pr-package/src/PullRequest.ts index 5c37a808f9..899ae1a2fc 100644 --- a/packages/pr-package/src/PullRequest.ts +++ b/packages/pr-package/src/PullRequest.ts @@ -1,4 +1,5 @@ import * as Effect from "effect/Effect"; +import * as Redacted from "effect/Redacted"; import * as FetchHttpClient from "effect/unstable/http/FetchHttpClient"; import * as HttpClient from "effect/unstable/http/HttpClient"; import * as HttpClientRequest from "effect/unstable/http/HttpClientRequest"; @@ -52,37 +53,74 @@ export const parsePullRequest = ( export const formatPullRequest = (pr: PullRequestRef) => `${pr.owner}/${pr.repo}#${pr.number}`; -/** Renew TTL when a tarball is PR-tied and GitHub has not confirmed closed. */ +/** + * How long a PR-tied tarball keeps renewing after the PR was last confirmed + * open when GitHub cannot be reached (no token, rate limited, outage). Past + * this, an unconfirmed PR is treated as closed so previews cannot leak + * forever on weekly renewals. + */ +export const MAX_UNVERIFIED_MS = 28 * 24 * 60 * 60 * 1000; + +export interface RenewalCandidate { + state: PullRequestState; + /** Last time the PR was known to be open. */ + verifiedAt: number; +} + +/** + * Renew while any tied PR is open. A PR GitHub could not confirm counts as + * open until `MAX_UNVERIFIED_MS` after it was last verified. + */ +export const isStillOpen = ( + candidate: RenewalCandidate, + now: number, +): boolean => + candidate.state === "open" || + (candidate.state === "unknown" && + now - candidate.verifiedAt < MAX_UNVERIFIED_MS); + export const shouldRenewOnTtl = ( - tiedToPullRequest: boolean, - state: PullRequestState, -): boolean => tiedToPullRequest && state !== "closed"; + candidates: Iterable, + now: number, +): boolean => { + for (const candidate of candidates) { + if (isStillOpen(candidate, now)) return true; + } + return false; +}; const isRecord = (value: unknown): value is Record => typeof value === "object" && value !== null; /** * Best-effort GitHub pull-request state. Network errors, rate limits, and - * private/missing repos return `"unknown"` so a TTL handler can fail closed - * (renew) instead of deleting a still-open preview. + * private/missing repos return `"unknown"` so the TTL handler can keep + * renewing (bounded by `MAX_UNVERIFIED_MS`) instead of deleting a still-open + * preview. Unauthenticated requests share a 60/hour limit per egress IP, so + * pass a `token` in production. */ export const pullRequestState = ( pr: PullRequestRef, + token?: Redacted.Redacted, ): Effect.Effect => Effect.gen(function* () { const client = yield* HttpClient.HttpClient; - const response = yield* client.execute( - HttpClientRequest.get( - `https://api.github.com/repos/${pr.owner}/${pr.repo}/pulls/${pr.number}`, - ).pipe( - HttpClientRequest.setHeader( - "User-Agent", - "alchemy-pr-package (https://github.com/alchemy-run/alchemy)", - ), - HttpClientRequest.setHeader("Accept", "application/vnd.github+json"), - HttpClientRequest.setHeader("X-GitHub-Api-Version", "2022-11-28"), + let request = HttpClientRequest.get( + `https://api.github.com/repos/${pr.owner}/${pr.repo}/pulls/${pr.number}`, + ).pipe( + HttpClientRequest.setHeader( + "User-Agent", + "alchemy-pr-package (https://github.com/alchemy-run/alchemy)", ), + HttpClientRequest.setHeader("Accept", "application/vnd.github+json"), + HttpClientRequest.setHeader("X-GitHub-Api-Version", "2022-11-28"), ); + if (token) { + request = request.pipe( + HttpClientRequest.bearerToken(Redacted.value(token)), + ); + } + const response = yield* client.execute(request); if (response.status !== 200) return "unknown" as const; const body: unknown = yield* response.json; if (!isRecord(body)) return "unknown" as const; diff --git a/packages/pr-package/test/PullRequest.test.ts b/packages/pr-package/test/PullRequest.test.ts index 63a06ac726..1ffc58df69 100644 --- a/packages/pr-package/test/PullRequest.test.ts +++ b/packages/pr-package/test/PullRequest.test.ts @@ -1,6 +1,15 @@ import { expect, test } from "bun:test"; +import { + emptyState, + releasePullRequests, + tiePullRequest, + withoutTags, + type PackageState, +} from "../src/PackageState.ts"; import { formatPullRequest, + isStillOpen, + MAX_UNVERIFIED_MS, parsePullRequest, shouldRenewOnTtl, } from "../src/PullRequest.ts"; @@ -33,11 +42,108 @@ test("parsePullRequest accepts owner/repo#number and GitHub URLs", () => { ).toBe("alchemy-run/alchemy#550"); }); -test("shouldRenewOnTtl keeps PR-tied tarballs unless GitHub says closed", () => { - expect(shouldRenewOnTtl(false, "closed")).toBe(false); - expect(shouldRenewOnTtl(false, "open")).toBe(false); - expect(shouldRenewOnTtl(false, "unknown")).toBe(false); - expect(shouldRenewOnTtl(true, "closed")).toBe(false); - expect(shouldRenewOnTtl(true, "open")).toBe(true); - expect(shouldRenewOnTtl(true, "unknown")).toBe(true); +test("isStillOpen treats unknown as open only within the verification window", () => { + const now = 1_000_000_000_000; + expect(isStillOpen({ state: "open", verifiedAt: 0 }, now)).toBe(true); + expect(isStillOpen({ state: "closed", verifiedAt: now }, now)).toBe(false); + expect(isStillOpen({ state: "unknown", verifiedAt: now - 1 }, now)).toBe( + true, + ); + expect( + isStillOpen({ state: "unknown", verifiedAt: now - MAX_UNVERIFIED_MS }, now), + ).toBe(false); +}); + +test("shouldRenewOnTtl renews while any tied PR is open", () => { + const now = 1_000_000_000_000; + expect(shouldRenewOnTtl([], now)).toBe(false); + expect( + shouldRenewOnTtl( + [ + { state: "closed", verifiedAt: now }, + { state: "open", verifiedAt: now }, + ], + now, + ), + ).toBe(true); + expect(shouldRenewOnTtl([{ state: "closed", verifiedAt: now }], now)).toBe( + false, + ); +}); + +const prC = { owner: "alchemy-run", repo: "alchemy", number: 3 }; +const prB = { owner: "alchemy-run", repo: "alchemy", number: 2 }; + +const sharedTarball = (): PackageState => { + // A distilled tarball published by `main`, then tied to two PRs that pin + // the same distilled commit. + let state: PackageState = { + ...emptyState, + packageName: "@distilled.cloud/core", + hash: "a".repeat(64), + tags: ["main", "abc1234", "commit-c", "branch-c", "pr-3"], + }; + state = { + ...state, + pullRequests: tiePullRequest( + state, + prC, + ["abc1234", "commit-c", "branch-c", "pr-3"], + 1, + ), + }; + state = { + ...state, + tags: [...state.tags, "branch-b", "pr-2"], + pullRequests: tiePullRequest( + state, + prB, + ["abc1234", "commit-c", "branch-b", "pr-2"], + 2, + ), + }; + return state; +}; + +test("closing one PR keeps tags another open PR still claims", () => { + const state = sharedTarball(); + const released = releasePullRequests( + state, + (binding) => binding.ref.number === prB.number, + ); + // pr-2 and branch-b belong only to PR B; the shared commit tags stay for + // PR C and `main` was never PR-owned. + expect([...released.tags].sort()).toEqual(["branch-b", "pr-2"]); + expect(Object.keys(released.state.pullRequests ?? {})).toEqual([ + "alchemy-run/alchemy#3", + ]); +}); + +test("releasing every PR frees all PR-owned tags but not main", () => { + const state = sharedTarball(); + const released = releasePullRequests(state, () => true); + expect([...released.tags].sort()).toEqual([ + "abc1234", + "branch-b", + "branch-c", + "commit-c", + "pr-2", + "pr-3", + ]); + expect(released.state.pullRequests).toBeUndefined(); + const next = withoutTags(released.state, released.tags); + expect(next.tags).toEqual(["main"]); +}); + +test("withoutTags drops PR bindings that lose their last tag", () => { + const state = sharedTarball(); + const next = withoutTags(state, ["branch-b", "pr-2", "abc1234", "commit-c"]); + expect(Object.keys(next.pullRequests ?? {})).toEqual([ + "alchemy-run/alchemy#3", + ]); + expect(next.pullRequests?.["alchemy-run/alchemy#3"]?.tags).toEqual([ + "branch-c", + "pr-3", + ]); + expect(withoutTags(next, next.tags).pullRequests).toBeUndefined(); }); diff --git a/scripts/bind-pr-packages.ts b/scripts/bind-pr-packages.ts deleted file mode 100644 index 4074040359..0000000000 --- a/scripts/bind-pr-packages.ts +++ /dev/null @@ -1,82 +0,0 @@ -#!/usr/bin/env bun -/** - * Re-point a PR publish's tags with Alchemy-Pull-Request so the pr-package - * worker renews TTL while the PR stays open. - * - * The publish action may not send that header yet; this step is the - * repo-local contract until it does. - */ -type Package = { - project: string; - tags: string[]; -}; - -type Plan = { - packages: Package[]; -}; - -const plan = JSON.parse(required("PLAN")) as Plan; -const host = process.env.PR_PACKAGE_HOST?.trim() || "pkg.ing"; -const token = required("TOKEN"); -const pullRequest = required("PULL_REQUEST"); -const ttl = process.env.TTL?.trim() || "1 week"; - -function required(name: string): string { - const value = process.env[name]?.trim(); - if (!value) { - throw new Error(`${name} is required`); - } - return value; -} - -function projectUrl(project: string): string { - const path = project.split("/").map(encodeURIComponent).join("/"); - return `https://${host}/projects/${path}`; -} - -async function tarballHash( - project: string, - tag: string, -): Promise { - const url = `${projectUrl(project)}/tags/${encodeURIComponent(tag)}`; - for (let attempt = 0; attempt < 15; attempt++) { - const response = await fetch(url, { redirect: "manual" }); - if (response.status === 302) { - const location = response.headers.get("location") ?? ""; - const match = location.match(/\/packages\/([a-f0-9]{64})\/?$/); - if (match) return match[1]; - } - await Bun.sleep(1000); - } - return undefined; -} - -for (const pkg of plan.packages) { - const prTag = pkg.tags.find((tag) => /^pr-\d+$/.test(tag)); - if (!prTag) continue; - - const hash = await tarballHash(pkg.project, prTag); - if (!hash) { - throw new Error( - `Could not resolve ${pkg.project} tag ${prTag} to a tarball`, - ); - } - - const response = await fetch(`${projectUrl(pkg.project)}/tags`, { - method: "PUT", - headers: { - Authorization: `Bearer ${token}`, - "Alchemy-Tags": JSON.stringify(pkg.tags), - "Alchemy-Tarball-Hash": hash, - "Alchemy-TTL": ttl, - "Alchemy-Pull-Request": pullRequest, - }, - }); - if (!response.ok) { - const details = await response.text(); - throw new Error( - `Failed to bind ${pkg.project} to ${pullRequest}: ${response.status} ${response.statusText}${details ? `\n${details}` : ""}`, - ); - } - console.log(`Bound ${pkg.project} ${prTag} to ${pullRequest}`); -}