diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 740731d4a7..c9f892e3d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -138,7 +138,7 @@ jobs: env: CHANGED_FILES: ${{ steps.filter.outputs.base != 'true' && fromJSON(steps.filter.outputs.changed_count) <= 1000 && steps.filter.outputs.changed_files || '' }} run: | - bun test scripts/ci/plan-tests.test.ts + bun test scripts/ci/plan-tests.test.ts scripts/lib/pr-state.test.ts matrix="$(bun scripts/ci/plan-tests.ts)" echo "matrix=$matrix" >> "$GITHUB_OUTPUT" echo "has-tests=$(jq '.include | length > 0' <<< "$matrix")" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/cleanup-preview-orphans.yml b/.github/workflows/cleanup-preview-orphans.yml index 71205360c9..4080683597 100644 --- a/.github/workflows/cleanup-preview-orphans.yml +++ b/.github/workflows/cleanup-preview-orphans.yml @@ -13,6 +13,7 @@ on: permissions: contents: read pull-requests: read # sweep checks each pr- branch's PR state + actions: read # stage sweep skips a PR whose preview deploy is queued or running id-token: write # Infisical OIDC machine-identity auth concurrency: diff --git a/.github/workflows/deploy-pr-preview.yml b/.github/workflows/deploy-pr-preview.yml index 3e45e92053..0c30740b90 100644 --- a/.github/workflows/deploy-pr-preview.yml +++ b/.github/workflows/deploy-pr-preview.yml @@ -15,8 +15,16 @@ on: - unlabeled - closed +# Only runs that deploy or tear down share the PR's group (same test as deploy-pr-preview's +# `if`). A run that skips both gets its own group, so a push to an unlabelled PR can't +# cancel its teardown mid-destroy and leak the stack (#1564). concurrency: - group: pr-preview-${{ github.event.pull_request.number }} + group: >- + ${{ ((github.event.action == 'unlabeled' && github.event.label.name == 'preview') + || (github.event.action != 'unlabeled' + && contains(github.event.pull_request.labels.*.name, 'preview'))) + && format('pr-preview-{0}', github.event.pull_request.number) + || format('pr-preview-noop-{0}', github.run_id) }} cancel-in-progress: true permissions: @@ -57,8 +65,9 @@ jobs: env: # Set `PREVIEW_INFISICAL_ENV_SLUG` once a dedicated preview environment exists. INFISICAL_ENV_SLUG: ${{ vars.PREVIEW_INFISICAL_ENV_SLUG || 'dev' }} - # Each live preview holds one of the account's 25 Hyperdrive configs. - MAX_LIVE_PREVIEWS: 8 + # Each live preview holds a VPC in the prd AWS account, whose us-east-1 quota of 5 + # leaves 3 beside the default and prd VPCs (also 1 of 25 Hyperdrive configs). + MAX_LIVE_PREVIEWS: 3 # `unlabeled` only reaches here for the `preview` label, so both mean teardown. TEARDOWN: ${{ (github.event.action == 'closed' || github.event.action == 'unlabeled') && 'true' || 'false' }} # Opt-in collector (prd-only otherwise, see `profile.deploys.collector`). diff --git a/docs/pr-previews.md b/docs/pr-previews.md index b088fcb94c..fcfaade40f 100644 --- a/docs/pr-previews.md +++ b/docs/pr-previews.md @@ -9,7 +9,7 @@ when the label comes off or the PR closes (`.github/workflows/deploy-pr-preview. | :------------------------------------------------------ | :----------------------------------------------------- | :------------------------------------------------------------------------------------- | | web, api, ai, alerting, chat-bot, electric-sync Workers | `app-pr-`, `api-pr-`, `sync-pr-` `.maple.dev` | Clerk development instance | | Postgres | Neon branch `pr-` of `NEON_PROJECT_ID` | Migrated by the deploy (`declarePreviewDb` in `alchemy.run.ts`), deleted on destroy | -| Hyperdrive | `maple-db-pr-`, shared by every Worker | One of the account's 25 configs, hence the live-preview cap | +| Hyperdrive | `maple-db-pr-`, shared by every Worker | One of the account's 25 configs | | Warehouse | Tinybird branch `pr_` | Empty; `scripts/tinybird-pr-branch.ts` swaps host, token, signing key and workspace id | | Ingest gateway | `ingest-pr-.maple.dev` (ECS on EC2) | Reads ingest keys from the preview's own branch | | Electric | `electric-pr-.maple.dev` (ECS Fargate) | Replicates from the branch; electric-sync points at it | @@ -48,11 +48,15 @@ and fails the job if a Postgres- or warehouse-backed route returns a 5xx. Withou ## Limits and cost -- At most `MAX_LIVE_PREVIEWS` (8) open PRs may carry `preview`; the deploy refuses the next one. +- At most `MAX_LIVE_PREVIEWS` (3) open PRs may carry `preview`; the deploy refuses the next one. + Each preview's ingest needs a VPC in the prd AWS account, and us-east-1 allows 5: the default + VPC and prd hold 2. - A preview runs an EC2 host, two ALBs and a Fargate task while it is live. Neon compute stays awake while Electric holds its replication slot. -- `cleanup-preview-orphans.yml` destroys the stage of any closed PR whose resources are still - around (`scripts/preview-stage-orphan-sweep.ts`), then sweeps Tinybird, Workers and Hyperdrive. +- `cleanup-preview-orphans.yml` sweeps Tinybird, destroys the stage of any PR that is closed or + no longer carries `preview` (`scripts/preview-stage-orphan-sweep.ts`), then sweeps Queues, + Workers and Hyperdrive. That also reaps a stage whose teardown was cancelled. A stage with a + queued or running preview deploy is skipped until the next sweep. ## One-time setup diff --git a/scripts/cloudflare-worker-orphan-sweep.ts b/scripts/cloudflare-worker-orphan-sweep.ts index 34938a91dc..ccbae27544 100644 --- a/scripts/cloudflare-worker-orphan-sweep.ts +++ b/scripts/cloudflare-worker-orphan-sweep.ts @@ -26,7 +26,7 @@ * both binds and consumes its stage's `maple-vcs-sync-pr-` / * `maple-planetscale-webhooks-pr-` queues (apps/api/src/resources/queues.ts), so * the sweep breaks the cycle in three passes: - * 1. detach the consumer registrations of closed-PR queues, + * 1. detach the consumer registrations of retired-PR queues, * 2. delete the workers (their producer bindings die with them), * 3. delete the now-unreferenced queues. * @@ -50,7 +50,7 @@ * GITHUB_TOKEN/GH_TOKEN for the PR-state gate. */ -import { fetchPrState, hasPrStateCredentials, type PrState } from "./lib/pr-state.ts" +import { fetchPreviewState, hasPrStateCredentials, type PreviewState } from "./lib/pr-state.ts" const FAILURE = 1 @@ -122,18 +122,18 @@ const main = async (): Promise => { // One PR-state lookup per PR number, shared across both passes — a stage // leaks ~8 workers + 2 queues and they all share the same verdict. - const stateByPr = new Map() - const prState = async (prNumber: string): Promise => { + const stateByPr = new Map() + const prState = async (prNumber: string): Promise => { let state = stateByPr.get(prNumber) if (!state) { - state = await fetchPrState(prNumber) + state = await fetchPreviewState(prNumber) stateByPr.set(prNumber, state) } return state } const failures: string[] = [] - // Pass 1: detach consumer registrations from closed-PR queues, so the + // Pass 1: detach consumer registrations from retired-PR queues, so the // worker deletions in pass 2 stop failing with 10064. The queues // themselves are deleted in pass 3, after the workers' producer bindings // are gone (deleting them here fails with 11005). @@ -164,8 +164,8 @@ const main = async (): Promise => { for (const { id, name, match } of queueCandidates) { const prNumber = match[2] as string const state = await prState(prNumber) - if (state === "open") { - console.log(`… keeping ${name} (PR #${prNumber} is open)`) + if (state === "live") { + console.log(`… keeping ${name} (PR #${prNumber} has a live preview)`) continue } if (state === "unknown") { @@ -190,7 +190,9 @@ const main = async (): Promise => { failures.push(name) continue } - console.log(`… detaching consumer ${consumerRef} from ${name} (PR #${prNumber} is closed)`) + console.log( + `… detaching consumer ${consumerRef} from ${name} (PR #${prNumber} is closed or unlabelled)`, + ) const detached = await cfRequest( token, `/accounts/${accountId}/queues/${id}/consumers/${consumerRef}`, @@ -227,15 +229,15 @@ const main = async (): Promise => { for (const { name, match } of candidates) { const prNumber = match[2] as string const state = await prState(prNumber) - if (state === "open") { - console.log(`… keeping ${name} (PR #${prNumber} is open)`) + if (state === "live") { + console.log(`… keeping ${name} (PR #${prNumber} has a live preview)`) continue } if (state === "unknown") { console.log(`⚠ skipping ${name} (PR #${prNumber} state unknown)`) continue } - console.log(`… deleting orphan ${name} (PR #${prNumber} is closed)`) + console.log(`… deleting orphan ${name} (PR #${prNumber} is closed or unlabelled)`) const removed = await cfRequest(token, `/accounts/${accountId}/workers/scripts/${name}?force=true`, { method: "DELETE", }) diff --git a/scripts/hyperdrive-orphan-sweep.ts b/scripts/hyperdrive-orphan-sweep.ts index 57671cc1f3..1351f97fba 100644 --- a/scripts/hyperdrive-orphan-sweep.ts +++ b/scripts/hyperdrive-orphan-sweep.ts @@ -31,7 +31,7 @@ * GITHUB_TOKEN/GH_TOKEN for the PR-state gate. */ -import { fetchPrState, hasPrStateCredentials } from "./lib/pr-state.ts" +import { fetchPreviewState, hasPrStateCredentials } from "./lib/pr-state.ts" const FAILURE = 1 @@ -114,16 +114,16 @@ const main = async (): Promise => { const failures: string[] = [] for (const { id, name, match } of candidates) { - const state = await fetchPrState(match[1] as string) - if (state === "open") { - console.log(`… keeping ${name} (PR #${match[1]} is open)`) + const state = await fetchPreviewState(match[1] as string) + if (state === "live") { + console.log(`… keeping ${name} (PR #${match[1]} has a live preview)`) continue } if (state === "unknown") { console.log(`⚠ skipping ${name} (PR #${match[1]} state unknown)`) continue } - console.log(`… deleting orphan ${name} (PR #${match[1]} is closed)`) + console.log(`… deleting orphan ${name} (PR #${match[1]} is closed or unlabelled)`) const removed = await cfRequest(token, `/accounts/${accountId}/hyperdrive/configs/${id}`, { method: "DELETE", }) diff --git a/scripts/lib/pr-state.test.ts b/scripts/lib/pr-state.test.ts new file mode 100644 index 0000000000..03abfb8def --- /dev/null +++ b/scripts/lib/pr-state.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, test } from "bun:test" +import { previewStateOf } from "./pr-state.ts" + +describe("previewStateOf", () => { + test("an open PR keeps its preview only while it carries the label", () => { + expect(previewStateOf({ state: "open", labels: [{ name: "bug" }, { name: "preview" }] })).toBe("live") + expect(previewStateOf({ state: "open", labels: [{ name: "preview:collector" }] })).toBe("retired") + expect(previewStateOf({ state: "open", labels: [] })).toBe("retired") + }) + + test("a closed PR is retired even if the label is still on it", () => { + expect(previewStateOf({ state: "closed", labels: [{ name: "preview" }] })).toBe("retired") + }) + + test("an unrecognised payload is unknown, so nothing is deleted", () => { + expect(previewStateOf({})).toBe("unknown") + expect(previewStateOf({ state: "draft", labels: [{ name: "preview" }] })).toBe("unknown") + }) +}) diff --git a/scripts/lib/pr-state.ts b/scripts/lib/pr-state.ts index f7363311eb..e7dd1ce9a3 100644 --- a/scripts/lib/pr-state.ts +++ b/scripts/lib/pr-state.ts @@ -1,21 +1,74 @@ /** - * The PR-state gate shared by the preview orphan sweeps (cleanup-preview-orphans.yml). + * The preview-state gate shared by the preview orphan sweeps (cleanup-preview-orphans.yml). * * Auth: GITHUB_REPOSITORY + GITHUB_TOKEN/GH_TOKEN. */ -export type PrState = "open" | "closed" | "unknown" +/** `live`: open and labelled `preview`. `retired`: closed, or open without the label. */ +export type PreviewState = "live" | "retired" | "unknown" + +export const PREVIEW_LABEL = "preview" export const hasPrStateCredentials = (): boolean => Boolean( process.env.GITHUB_REPOSITORY?.trim() && (process.env.GITHUB_TOKEN ?? process.env.GH_TOKEN)?.trim(), ) +/** An open PR that lost its label is retired too: a cancelled teardown leaves its stage up. */ +export const previewStateOf = (pull: { + state?: string + labels?: ReadonlyArray<{ name?: string }> +}): PreviewState => { + if (pull.state === "closed") return "retired" + if (pull.state !== "open") return "unknown" + return pull.labels?.some((label) => label.name === PREVIEW_LABEL) ? "live" : "retired" +} + +/** + * Whether a preview deploy run for this PR is queued or running, so a sweep never destroys a + * stage that a restored label is redeploying. "unknown" when the API call fails. + */ +export const hasActivePreviewRun = async (prNumber: string): Promise => { + const repo = process.env.GITHUB_REPOSITORY?.trim() + const token = (process.env.GITHUB_TOKEN ?? process.env.GH_TOKEN)?.trim() + if (!repo || !token) return "unknown" + try { + for (const status of ["queued", "in_progress"]) { + const response = await fetch( + `https://api.github.com/repos/${repo}/actions/workflows/deploy-pr-preview.yml/runs?status=${status}&per_page=100`, + { + headers: { + Authorization: `Bearer ${token}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }, + }, + ) + if (!response.ok) { + console.log(`⚠ Could not list ${status} preview runs (HTTP ${response.status})`) + return "unknown" + } + const { workflow_runs: runs = [] } = (await response.json()) as { + workflow_runs?: ReadonlyArray<{ pull_requests?: ReadonlyArray<{ number?: number }> }> + } + if (runs.some((run) => run.pull_requests?.some((pull) => String(pull.number) === prNumber))) { + return true + } + } + return false + } catch (error) { + console.log( + `⚠ Could not list preview runs (${error instanceof Error ? error.message : String(error)})`, + ) + return "unknown" + } +} + /** - * PR state via the GitHub REST API. Returns "unknown" when the API call - * fails; callers must treat "unknown" as "don't delete", never as "closed". + * Preview state via the GitHub REST API. Returns "unknown" when the API call + * fails; callers must treat "unknown" as "don't delete", never as "retired". */ -export const fetchPrState = async (prNumber: string): Promise => { +export const fetchPreviewState = async (prNumber: string): Promise => { const repo = process.env.GITHUB_REPOSITORY?.trim() const token = (process.env.GITHUB_TOKEN ?? process.env.GH_TOKEN)?.trim() if (!repo || !token) return "unknown" @@ -31,8 +84,9 @@ export const fetchPrState = async (prNumber: string): Promise => { console.log(`⚠ Could not look up PR #${prNumber} state (HTTP ${response.status})`) return "unknown" } - const parsed = (await response.json()) as { state?: string } - return parsed.state === "open" ? "open" : parsed.state === "closed" ? "closed" : "unknown" + return previewStateOf( + (await response.json()) as { state?: string; labels?: ReadonlyArray<{ name?: string }> }, + ) } catch (error) { console.log( `⚠ Could not look up PR #${prNumber} state (${error instanceof Error ? error.message : String(error)})`, diff --git a/scripts/preview-stage-orphan-sweep.ts b/scripts/preview-stage-orphan-sweep.ts index ccb90afe4d..d1288497e7 100644 --- a/scripts/preview-stage-orphan-sweep.ts +++ b/scripts/preview-stage-orphan-sweep.ts @@ -1,6 +1,6 @@ #!/usr/bin/env bun /** - * Tear down whole PR-preview stages whose PR is closed. + * Tear down whole PR-preview stages whose PR is closed or lost its `preview` label. * * bun scripts/preview-stage-orphan-sweep.ts [--dry-run] * @@ -14,7 +14,7 @@ * - Neon branches named `pr-` in NEON_PROJECT_ID (alchemy.run.ts, declarePreviewDb), * - R2 buckets named `maple-replay-blobs-pr-`, which alchemy RETAINS on destroy * (apps/api/src/resources/replay-blobs.ts), so every deployed stage keeps one. - * For each PR the GitHub API affirmatively reports closed (unknown/open → keep), it runs + * For each PR the GitHub API affirmatively reports retired (unknown/live → keep), it runs * `alchemy destroy --yes --stage pr-`, then deletes the Neon branch and empties and * deletes the R2 bucket. If the destroy fails, the branch and bucket stay so the next run * rediscovers the stage and retries. Stages run one at a time; a failure does not stop @@ -31,7 +31,7 @@ import { spawnSync } from "node:child_process" import { join } from "node:path" -import { fetchPrState, hasPrStateCredentials } from "./lib/pr-state.ts" +import { fetchPreviewState, hasActivePreviewRun, hasPrStateCredentials } from "./lib/pr-state.ts" const FAILURE = 1 const DRY_RUN = process.argv.includes("--dry-run") @@ -292,18 +292,27 @@ const main = async (): Promise => { for (const prNumber of prNumbers) { const stage = `pr-${prNumber}` const markers = candidates.get(prNumber) ?? [] - const state = await fetchPrState(prNumber) - if (state === "open") { - console.log(`… keeping ${stage} (PR #${prNumber} is open)`) + const state = await fetchPreviewState(prNumber) + if (state === "live") { + console.log(`… keeping ${stage} (PR #${prNumber} has a live preview)`) continue } if (state === "unknown") { console.log(`⚠ skipping ${stage} (PR #${prNumber} state unknown)`) continue } + const activeRun = await hasActivePreviewRun(prNumber) + if (activeRun !== false) { + console.log( + `⚠ skipping ${stage} (${activeRun === true ? `a preview run for PR #${prNumber} is active` : "preview runs unknown"})`, + ) + continue + } const bucket = markers.find((marker) => REPLAY_BUCKET_PATTERN.test(marker)) const branchId = neonBranchIdByPr.get(prNumber) - console.log(`… tearing down ${stage} (PR #${prNumber} is closed; found ${markers.join(", ")})`) + console.log( + `… tearing down ${stage} (PR #${prNumber} is closed or unlabelled; found ${markers.join(", ")})`, + ) if (DRY_RUN) { console.log( ` would destroy ${stage}${branchId ? `, delete Neon branch ${branchId}` : ""}${bucket ? `, empty and delete ${bucket}` : ""}`, diff --git a/scripts/tinybird-pr-branch.ts b/scripts/tinybird-pr-branch.ts index 1a0b40f47f..55bcbe6d2a 100644 --- a/scripts/tinybird-pr-branch.ts +++ b/scripts/tinybird-pr-branch.ts @@ -33,6 +33,7 @@ */ import { spawnSync } from "node:child_process" import { appendFileSync } from "node:fs" +import { fetchPreviewState, hasPrStateCredentials } from "./lib/pr-state.ts" import { writeTinybirdDatafiles } from "./write-tinybird-datafiles" type Subcommand = "up" | "down" | "sweep" @@ -236,38 +237,7 @@ const down = (branchName: string): void => { } /** - * PR state via the GitHub REST API. Returns "unknown" when no token/repo is - * available (local runs) or the API call fails — callers must treat "unknown" - * as "don't block", never as "closed". - */ -const fetchPrState = async (prNumber: string): Promise<"open" | "closed" | "unknown"> => { - const repo = process.env.GITHUB_REPOSITORY?.trim() - const token = (process.env.GITHUB_TOKEN ?? process.env.GH_TOKEN)?.trim() - if (!repo || !token) return "unknown" - try { - const response = await fetch(`https://api.github.com/repos/${repo}/pulls/${prNumber}`, { - headers: { - Authorization: `Bearer ${token}`, - Accept: "application/vnd.github+json", - "X-GitHub-Api-Version": "2022-11-28", - }, - }) - if (!response.ok) { - console.log(`⚠ Could not look up PR #${prNumber} state (HTTP ${response.status})`) - return "unknown" - } - const parsed = (await response.json()) as { state?: string } - return parsed.state === "open" ? "open" : parsed.state === "closed" ? "closed" : "unknown" - } catch (error) { - console.log( - `⚠ Could not look up PR #${prNumber} state (${error instanceof Error ? error.message : String(error)})`, - ) - return "unknown" - } -} - -/** - * Remove every `pr_` branch whose PR is closed. `tb branch ls` prints a + * Remove every `pr_` branch whose PR is closed or lost its `preview` label. `tb branch ls` prints a * human-readable table (no JSON mode), so candidate names are extracted by the * exact `pr_` token shape — `main` and any other branch name are never * candidates. Branches whose PR state cannot be determined are skipped @@ -277,10 +247,7 @@ const sweep = async (): Promise => { // The PR-state lookup is the sweep's only guard against deleting a LIVE // preview; without a token every branch resolves to "unknown" and the run // green-no-ops forever. Fail loudly instead — this is the safety net. - if ( - !process.env.GITHUB_REPOSITORY?.trim() || - !(process.env.GITHUB_TOKEN ?? process.env.GH_TOKEN)?.trim() - ) { + if (!hasPrStateCredentials()) { fail("sweep requires GITHUB_REPOSITORY and GITHUB_TOKEN (or GH_TOKEN) to check PR state") } const parent = { host: requireEnv("TINYBIRD_HOST"), token: requireEnv("TINYBIRD_TOKEN") } @@ -302,16 +269,16 @@ const sweep = async (): Promise => { const failures: string[] = [] for (const candidate of candidates) { - const state = await fetchPrState(candidate.prNumber) - if (state === "open") { - console.log(`… keeping ${candidate.name} (PR #${candidate.prNumber} is open)`) + const state = await fetchPreviewState(candidate.prNumber) + if (state === "live") { + console.log(`… keeping ${candidate.name} (PR #${candidate.prNumber} has a live preview)`) continue } if (state === "unknown") { console.log(`⚠ skipping ${candidate.name} (PR #${candidate.prNumber} state unknown)`) continue } - console.log(`… deleting orphan ${candidate.name} (PR #${candidate.prNumber} is closed)`) + console.log(`… deleting orphan ${candidate.name} (PR #${candidate.prNumber} is closed or unlabelled)`) const removed = runTb(parent, ["branch", "rm", candidate.name, "--yes"]) if (removed.exitCode !== 0 && !isNotFound(removed)) { failures.push(candidate.name)