From 1694f73643d6ccccc5511571c4438d455581bd1e Mon Sep 17 00:00:00 2001 From: hachej Date: Fri, 31 Jul 2026 12:49:35 +0000 Subject: [PATCH 1/4] #391 ci(release): gate cuts on built-dist smoke --- .github/workflows/release-candidate.yml | 60 +++++++++ .../release-candidate-golden-route.spec.ts | 87 ++++++++++++ apps/workspace-playground/package.json | 1 + .../workspace-playground/playwright.config.ts | 3 +- .../src/__tests__/build-chain.test.ts | 6 + .../__tests__/release-candidate-css.test.ts | 23 ++++ .../src/release-candidate-css.ts | 23 ++++ apps/workspace-playground/vite.config.ts | 47 ++++++- scripts/cut-release.sh | 10 +- scripts/require-release-candidate-check.mjs | 124 ++++++++++++++++++ .../require-release-candidate-check.test.mjs | 122 +++++++++++++++++ 11 files changed, 502 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/release-candidate.yml create mode 100644 apps/workspace-playground/e2e/release-candidate-golden-route.spec.ts create mode 100644 apps/workspace-playground/src/__tests__/release-candidate-css.test.ts create mode 100644 apps/workspace-playground/src/release-candidate-css.ts create mode 100644 scripts/require-release-candidate-check.mjs create mode 100644 scripts/require-release-candidate-check.test.mjs diff --git a/.github/workflows/release-candidate.yml b/.github/workflows/release-candidate.yml new file mode 100644 index 000000000..743b24e03 --- /dev/null +++ b/.github/workflows/release-candidate.yml @@ -0,0 +1,60 @@ +name: Release Candidate Built-Dist + +on: + pull_request: + types: [opened, synchronize, reopened, labeled] + push: + branches: + - main + - "release/**" + +permissions: + contents: read + +concurrency: + group: release-candidate-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.sha }} + cancel-in-progress: true + +jobs: + built-dist: + name: Release Candidate Built-Dist + if: github.event_name == 'push' || contains(github.event.pull_request.labels.*.name, 'release-candidate') || startsWith(github.head_ref, 'release/') + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Setup pnpm + uses: pnpm/action-setup@0e279bb959325dab635dd2c09392533439d90093 # v6.0.8 + + - name: Setup Node + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 22.19.0 + cache: pnpm + + - name: Install + run: pnpm install --frozen-lockfile + + - name: Build package and plugin release artifacts + run: pnpm build:packages + env: + NODE_OPTIONS: --max-old-space-size=4096 + + - name: Cache Playwright browsers + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ~/.cache/ms-playwright + key: ${{ runner.os }}-playwright-${{ hashFiles('pnpm-lock.yaml') }} + restore-keys: | + ${{ runner.os }}-playwright- + + - name: Install Playwright browser + run: pnpm exec playwright install --with-deps chromium + + - name: Run built-dist release-candidate smoke + run: pnpm --filter workspace-playground run test:e2e:release-candidate + env: + BORING_PLAYGROUND_DIST_ONLY: "1" + BORING_AGENT_WORKSPACE_ROOT: ${{ runner.temp }}/workspace-playground-release-candidate diff --git a/apps/workspace-playground/e2e/release-candidate-golden-route.spec.ts b/apps/workspace-playground/e2e/release-candidate-golden-route.spec.ts new file mode 100644 index 000000000..eee4e8fb5 --- /dev/null +++ b/apps/workspace-playground/e2e/release-candidate-golden-route.spec.ts @@ -0,0 +1,87 @@ +import { expect, test } from "@playwright/test" +import { assertReleaseCandidateAgentCss } from "../src/release-candidate-css" + +const agentDistCssPath = "/packages/agent/dist/front/styles.css" + +function isAgentDistStylesheet(url: string): boolean { + const parsed = new URL(url) + return parsed.searchParams.get("boring-rc-agent-css") === "1" + && decodeURIComponent(parsed.pathname).replaceAll("\\", "/").includes(agentDistCssPath) +} + +test("boots built dist and completes one Alpha first send", async ({ page }) => { + test.setTimeout(120_000) + expect(process.env.BORING_PLAYGROUND_DIST_ONLY, "RC smoke requires explicit dist-only mode").toBe("1") + + const cssFault = process.env.BORING_RC_BREAK_CSS + if (cssFault === "small" || cssFault === "mime") { + await page.route(/boring-rc-agent-css=1/, async (route) => { + const response = await route.fetch() + const body = cssFault === "small" ? Buffer.from("/* deliberately small RC fixture */") : await response.body() + await route.fulfill({ + response, + body, + headers: { + ...response.headers(), + "content-type": cssFault === "mime" ? "application/javascript" : "text/css; charset=utf-8", + }, + }) + }) + } + + let breakFirstSend = process.env.BORING_RC_BREAK_FIRST_SEND === "1" + if (breakFirstSend) { + await page.route("**/api/v1/agents/alpha/sessions", async (route) => { + if (breakFirstSend && route.request().method() === "POST") { + breakFirstSend = false + await route.fulfill({ + status: 500, + contentType: "application/json", + body: JSON.stringify({ error: { code: "RC_EXPECTED_FIRST_SEND_FAILURE" } }), + }) + return + } + await route.continue() + }) + } + + const agentCssResponse = page.waitForResponse( + (response) => isAgentDistStylesheet(response.url()), + { timeout: 30_000 }, + ) + await page.goto("/?fresh=1") + const cssResponse = await agentCssResponse + const cssBody = await cssResponse.body() + console.log( + `[release-candidate] Agent stylesheet ${cssResponse.url()} ${cssResponse.headers()["content-type"] ?? "missing"} ${cssBody.byteLength} bytes`, + ) + assertReleaseCandidateAgentCss( + cssResponse.url(), + cssResponse.headers()["content-type"], + cssBody.byteLength, + ) + await expect(page.locator('aside[aria-label="App navigation"]')).toBeVisible({ timeout: 30_000 }) + + await page.getByRole("button", { name: "New chat with Alpha", exact: true }).click() + const chat = page.locator('[data-boring-agent-part="chat"][data-agent-type-id="alpha"]').last() + await expect(chat).toHaveAttribute("data-pi-chat-session-id", /^local-/, { timeout: 15_000 }) + const localSessionId = await chat.getAttribute("data-pi-chat-session-id") + + const created = page.waitForResponse((response) => { + const url = new URL(response.url()) + return response.request().method() === "POST" + && url.pathname === "/api/v1/agents/alpha/sessions" + }) + await chat.getByRole("textbox", { name: "Agent prompt" }).fill(`release candidate ${Date.now()}`) + await chat.locator('[data-boring-agent-part="composer-submit"]').click() + expect((await created).status(), "first send must create an addressed session").toBe(201) + + let adoptedSessionId = "" + await expect.poll(async () => { + adoptedSessionId = await chat.getAttribute("data-pi-chat-session-id") ?? "" + return adoptedSessionId + }, { timeout: 15_000 }).not.toBe(localSessionId) + expect(adoptedSessionId).not.toMatch(/^local-/) + await expect(chat).toHaveAttribute("data-pi-chat-connection", "connected", { timeout: 15_000 }) + await expect(chat.getByText("PI_NATIVE_ASSISTANT_DONE:alpha", { exact: true })).toBeVisible({ timeout: 30_000 }) +}) diff --git a/apps/workspace-playground/package.json b/apps/workspace-playground/package.json index 11a7ef9ca..b34baefa2 100644 --- a/apps/workspace-playground/package.json +++ b/apps/workspace-playground/package.json @@ -11,6 +11,7 @@ "test": "vitest run", "test:e2e": "pnpm run build:deps && playwright test", "test:e2e:addressed-agent": "pnpm run build:deps && playwright test apps/workspace-playground/e2e/agent-host-golden-route.spec.ts apps/workspace-playground/e2e/native-session-regressions.spec.ts", + "test:e2e:release-candidate": "playwright test apps/workspace-playground/e2e/release-candidate-golden-route.spec.ts", "smoke:bridge": "pnpm --filter @hachej/boring-sandbox build && pnpm --filter @hachej/boring-bash build && pnpm --filter @hachej/boring-agent build && pnpm --filter @hachej/boring-workspace build && pnpm --filter @hachej/boring-ask-user build && tsx scripts/bridge-e2e.ts", "eval": "AGENT_API_PORT=5350 vite-node src/eval/run.ts", "eval:slash-command": "AGENT_API_PORT=5350 vite-node src/eval/run.ts src/eval/plugin-slash-command.yaml", diff --git a/apps/workspace-playground/playwright.config.ts b/apps/workspace-playground/playwright.config.ts index de8c6ecdb..3c4e04ebe 100644 --- a/apps/workspace-playground/playwright.config.ts +++ b/apps/workspace-playground/playwright.config.ts @@ -59,6 +59,7 @@ export default defineConfig({ "BORING_AGENT_E2E_SCRIPTED_PI=1", "BORING_AGENT_E2E_SCRIPTED_PI_TICK_MS=300", "BORING_AGENT_E2E_SCRIPTED_PI_TOOL_DELAY_TICKS=20", + `BORING_PLAYGROUND_DIST_ONLY=${shell(process.env.BORING_PLAYGROUND_DIST_ONLY || "")}`, "BORING_AGENT_CUSTOM_MODEL_PROVIDER=scripted-e2e", "BORING_AGENT_CUSTOM_MODEL_ID=scripted-model", "BORING_AGENT_CUSTOM_MODEL_BASE_URL=http://127.0.0.1", @@ -67,7 +68,7 @@ export default defineConfig({ "pnpm exec vite", ].join(" ")}`, port: VITE_PORT, - reuseExistingServer: !process.env.CI, + reuseExistingServer: !process.env.CI && !process.env.BORING_PLAYGROUND_DIST_ONLY, timeout: 300_000, }, }) diff --git a/apps/workspace-playground/src/__tests__/build-chain.test.ts b/apps/workspace-playground/src/__tests__/build-chain.test.ts index f68153130..d5a36a0da 100644 --- a/apps/workspace-playground/src/__tests__/build-chain.test.ts +++ b/apps/workspace-playground/src/__tests__/build-chain.test.ts @@ -80,4 +80,10 @@ describe("workspace-playground build chain", () => { it.each(["dev", "build", "test:e2e"])("%s runs build:deps before serving", (name) => { expect(scripts[name] ?? "").toContain("build:deps") }) + + it("keeps the release-candidate smoke build-free", () => { + const script = scripts["test:e2e:release-candidate"] ?? "" + expect(script).toContain("release-candidate-golden-route.spec.ts") + expect(script).not.toContain("build:deps") + }) }) diff --git a/apps/workspace-playground/src/__tests__/release-candidate-css.test.ts b/apps/workspace-playground/src/__tests__/release-candidate-css.test.ts new file mode 100644 index 000000000..61284e2d0 --- /dev/null +++ b/apps/workspace-playground/src/__tests__/release-candidate-css.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from "vitest" +import { assertReleaseCandidateAgentCss } from "../release-candidate-css" + +const distUrl = "http://127.0.0.1:5380/@fs/repo/packages/agent/dist/front/styles.css" + +describe("release-candidate Agent stylesheet assertion", () => { + it("accepts dist CSS with a CSS MIME and more than 100,000 bytes", () => { + expect(() => assertReleaseCandidateAgentCss(distUrl, "text/css; charset=utf-8", 100_001)).not.toThrow() + }) + + it("rejects CSS at the 100,000-byte boundary", () => { + expect(() => assertReleaseCandidateAgentCss(distUrl, "text/css", 100_000)).toThrow(/too small/) + }) + + it("rejects a wrong MIME", () => { + expect(() => assertReleaseCandidateAgentCss(distUrl, "application/javascript", 100_001)).toThrow(/wrong MIME/) + }) + + it("rejects a source stylesheet path", () => { + const sourceUrl = "http://127.0.0.1:5380/@fs/repo/packages/agent/src/front/styles.css" + expect(() => assertReleaseCandidateAgentCss(sourceUrl, "text/css", 100_001)).toThrow(/did not load from dist/) + }) +}) diff --git a/apps/workspace-playground/src/release-candidate-css.ts b/apps/workspace-playground/src/release-candidate-css.ts new file mode 100644 index 000000000..383d9cd1c --- /dev/null +++ b/apps/workspace-playground/src/release-candidate-css.ts @@ -0,0 +1,23 @@ +export const RELEASE_CANDIDATE_AGENT_CSS_MIN_BYTES = 100_000 + +export function assertReleaseCandidateAgentCss( + url: string, + contentType: string | undefined, + byteLength: number, +): void { + const pathname = decodeURIComponent(new URL(url).pathname).replaceAll("\\", "/") + if (!pathname.includes("/packages/agent/dist/front/styles.css")) { + throw new Error(`release-candidate Agent stylesheet did not load from dist: ${pathname}`) + } + if (pathname.includes("/packages/agent/src/")) { + throw new Error(`release-candidate Agent stylesheet loaded from source: ${pathname}`) + } + if (!/^text\/css(?:;|$)/i.test(contentType ?? "")) { + throw new Error(`release-candidate Agent stylesheet has wrong MIME: ${contentType ?? "missing"}`) + } + if (byteLength <= RELEASE_CANDIDATE_AGENT_CSS_MIN_BYTES) { + throw new Error( + `release-candidate Agent stylesheet is too small: ${byteLength} bytes (must be > ${RELEASE_CANDIDATE_AGENT_CSS_MIN_BYTES})`, + ) + } +} diff --git a/apps/workspace-playground/vite.config.ts b/apps/workspace-playground/vite.config.ts index 4ff351720..b972c2801 100644 --- a/apps/workspace-playground/vite.config.ts +++ b/apps/workspace-playground/vite.config.ts @@ -1,4 +1,4 @@ -import { defineConfig } from "vite" +import { defineConfig, type Plugin } from "vite" import react from "@vitejs/plugin-react" import tailwindcss from "@tailwindcss/vite" import { dirname, resolve } from "node:path" @@ -7,6 +7,44 @@ import { AGENT_API_PORT, VITE_PORT, startPlaygroundServer } from "./src/server/d const baseResolve = createBoringAppViteAliases({ appRoot: __dirname }) const repoRoot = resolve(__dirname, "../..") +const releaseCandidateDistOnly = process.env.BORING_PLAYGROUND_DIST_ONLY === "1" + +if (releaseCandidateDistOnly) { + console.log("[workspace-playground] release-candidate dist-only package resolution enabled") +} + +function releaseCandidateDistOnlyGuard(): Plugin { + return { + name: "boring-release-candidate-dist-only", + enforce: "pre", + async resolveId(source, importer, options) { + if (!source.startsWith("@hachej/boring-")) return null + + const resolved = await this.resolve(source, importer, { ...options, skipSelf: true }) + if (!resolved || resolved.external) return resolved + + const normalized = resolved.id.split("?", 1)[0].replaceAll("\\", "/") + const sourcePackage = /\/(packages|plugins)\/[^/]+\/src(?:\/|$)/.test(normalized) + if (sourcePackage) { + throw new Error( + `release-candidate dist-only resolution violation: ${source} resolved to ${normalized}`, + ) + } + return resolved + }, + transformIndexHtml() { + const agentCss = resolve(repoRoot, "packages/agent/dist/front/styles.css").replaceAll("\\", "/") + return [{ + tag: "link", + attrs: { + rel: "stylesheet", + href: `/@fs${agentCss}?direct&boring-rc-agent-css=1`, + }, + injectTo: "head", + }] + }, + } +} const externalWorkspaceRoot = process.env.BORING_AGENT_WORKSPACE_ROOT?.trim() const externalRuntimeExtensionsRoot = externalWorkspaceRoot ? resolve(externalWorkspaceRoot, ".pi", "extensions") @@ -90,6 +128,7 @@ const pollingInterval = Number(process.env.CHOKIDAR_INTERVAL ?? process.env.BORI export default defineConfig({ plugins: [ + ...(releaseCandidateDistOnly ? [releaseCandidateDistOnlyGuard()] : []), react({ exclude: dynamicPluginReactRefreshExclude, }), @@ -113,7 +152,11 @@ export default defineConfig({ }, ], resolve: { - alias: [...baseResolve.alias, ...playgroundOnlyAliases], + // RC smoke must consume package exports as shipped. Normal playground dev + // keeps its source/HMR aliases unchanged. + alias: releaseCandidateDistOnly + ? baseResolve.alias + : [...baseResolve.alias, ...playgroundOnlyAliases], dedupe: baseResolve.dedupe, }, server: { diff --git a/scripts/cut-release.sh b/scripts/cut-release.sh index 7c3bf0465..32791bc26 100755 --- a/scripts/cut-release.sh +++ b/scripts/cut-release.sh @@ -102,11 +102,19 @@ done <<< "$status" git commit -m "chore(release): bump packages to $after" git push origin main +release_sha=$(git rev-parse HEAD) +repository=$(gh repo view --json nameWithOwner --jq .nameWithOwner) +if ! GH_REPOSITORY="$repository" node scripts/require-release-candidate-check.mjs "$release_sha"; then + echo "Release-candidate gate did not pass for pushed release commit $release_sha." >&2 + echo "No GitHub release or tag was created. Fix or revert through normal git, then rerun." >&2 + exit 1 +fi + tag="v$after" echo "Creating GitHub release $tag (this also creates the git tag)…" gh release create "$tag" \ --title "$tag" \ - --target "$(git rev-parse HEAD)" \ + --target "$release_sha" \ --generate-notes echo diff --git a/scripts/require-release-candidate-check.mjs b/scripts/require-release-candidate-check.mjs new file mode 100644 index 000000000..305cbd664 --- /dev/null +++ b/scripts/require-release-candidate-check.mjs @@ -0,0 +1,124 @@ +#!/usr/bin/env node + +import { execFileSync } from "node:child_process" +import { fileURLToPath } from "node:url" +import { resolve } from "node:path" + +export const RELEASE_CANDIDATE_CHECK_NAME = "Release Candidate Built-Dist" + +function checkId(check) { + try { + return BigInt(check.id) + } catch { + throw new Error(`Release-candidate check has an invalid id: ${String(check.id)}`) + } +} + +export function selectLatestReleaseCandidateCheck(payload, sha) { + if (!payload || !Array.isArray(payload.check_runs)) { + throw new Error("GitHub check-runs response is missing check_runs") + } + + const matches = payload.check_runs.filter((check) => + check?.name === RELEASE_CANDIDATE_CHECK_NAME + && check?.head_sha === sha + && check?.app?.slug === "github-actions" + ) + if (matches.length === 0) return null + + return matches.reduce((latest, check) => checkId(check) > checkId(latest) ? check : latest) +} + +export function classifyReleaseCandidateCheck(check) { + if (!check) return { state: "waiting", description: "check has not been created" } + if (check.status !== "completed") { + return { state: "waiting", description: `latest check ${check.id} is ${check.status}` } + } + if (check.conclusion === "success") { + return { state: "success", description: `check ${check.id} completed successfully` } + } + return { + state: "failure", + description: `latest check ${check.id} completed with ${check.conclusion ?? "no conclusion"}`, + } +} + +export async function waitForReleaseCandidateCheck({ + sha, + loadChecks, + timeoutMs = 30 * 60 * 1_000, + pollIntervalMs = 15_000, + now = Date.now, + sleep = (milliseconds) => new Promise((resolveSleep) => setTimeout(resolveSleep, milliseconds)), + log = console.log, +}) { + const startedAt = now() + let lastDescription = "check has not been queried" + + while (true) { + const payload = await loadChecks() + const check = selectLatestReleaseCandidateCheck(payload, sha) + const classification = classifyReleaseCandidateCheck(check) + lastDescription = classification.description + + if (classification.state === "success") return check + if (classification.state === "failure") { + throw new Error(`Release-candidate gate failed: ${classification.description}`) + } + + const elapsed = now() - startedAt + if (elapsed >= timeoutMs) { + throw new Error(`Timed out waiting for ${RELEASE_CANDIDATE_CHECK_NAME} on ${sha}: ${lastDescription}`) + } + log(`Waiting for ${RELEASE_CANDIDATE_CHECK_NAME} on ${sha}: ${lastDescription}`) + await sleep(Math.min(pollIntervalMs, timeoutMs - elapsed)) + } +} + +function positiveNumber(value, fallback, name) { + if (value === undefined || value === "") return fallback + const parsed = Number(value) + if (!Number.isFinite(parsed) || parsed <= 0) throw new Error(`${name} must be a positive number`) + return parsed +} + +async function main() { + const sha = process.argv[2] + const repository = process.env.GH_REPOSITORY + if (!/^[0-9a-f]{40}$/i.test(sha ?? "")) { + throw new Error("Usage: require-release-candidate-check.mjs <40-character-release-sha>") + } + if (!/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repository ?? "")) { + throw new Error("GH_REPOSITORY must be set to owner/repository") + } + + const timeoutMs = positiveNumber(process.env.RELEASE_CHECK_TIMEOUT_SECONDS, 30 * 60, "RELEASE_CHECK_TIMEOUT_SECONDS") * 1_000 + const pollIntervalMs = positiveNumber(process.env.RELEASE_CHECK_POLL_SECONDS, 15, "RELEASE_CHECK_POLL_SECONDS") * 1_000 + const endpoint = `repos/${repository}/commits/${sha}/check-runs?check_name=${encodeURIComponent(RELEASE_CANDIDATE_CHECK_NAME)}&filter=all&per_page=100` + const actionsUrl = `https://github.com/${repository}/actions?query=${encodeURIComponent(`head_sha:${sha}`)}` + console.log(`Requiring exact check "${RELEASE_CANDIDATE_CHECK_NAME}" for ${repository}@${sha}`) + console.log(`Actions: ${actionsUrl}`) + + const check = await waitForReleaseCandidateCheck({ + sha, + timeoutMs, + pollIntervalMs, + loadChecks: () => { + const output = execFileSync("gh", ["api", "--method", "GET", endpoint], { + encoding: "utf8", + stdio: ["ignore", "pipe", "inherit"], + }) + return JSON.parse(output) + }, + }) + console.log(`Release-candidate gate passed: ${check.html_url ?? actionsUrl}`) +} + +const isDirectInvocation = process.argv[1] + && fileURLToPath(import.meta.url) === resolve(process.argv[1]) +if (isDirectInvocation) { + main().catch((error) => { + console.error(error instanceof Error ? error.message : error) + process.exitCode = 1 + }) +} diff --git a/scripts/require-release-candidate-check.test.mjs b/scripts/require-release-candidate-check.test.mjs new file mode 100644 index 000000000..ccb5b9735 --- /dev/null +++ b/scripts/require-release-candidate-check.test.mjs @@ -0,0 +1,122 @@ +import assert from "node:assert/strict" +import { describe, test } from "node:test" +import { + classifyReleaseCandidateCheck, + selectLatestReleaseCandidateCheck, + waitForReleaseCandidateCheck, +} from "./require-release-candidate-check.mjs" + +const sha = "a".repeat(40) +const otherSha = "b".repeat(40) + +function check(overrides = {}) { + return { + id: 100, + name: "Release Candidate Built-Dist", + head_sha: sha, + status: "completed", + conclusion: "success", + app: { slug: "github-actions" }, + ...overrides, + } +} + +function payload(...checks) { + return { check_runs: checks } +} + +describe("release-candidate check selection", () => { + test("treats a missing exact check as waiting", () => { + assert.deepEqual(classifyReleaseCandidateCheck(selectLatestReleaseCandidateCheck(payload(), sha)), { + state: "waiting", + description: "check has not been created", + }) + }) + + test("ignores checks with the wrong SHA, name, or app", () => { + const selected = selectLatestReleaseCandidateCheck(payload( + check({ id: 101, head_sha: otherSha }), + check({ id: 102, name: "Main Green Summary" }), + check({ id: 103, app: { slug: "external-ci" } }), + ), sha) + assert.equal(selected, null) + }) + + test("classifies queued and in-progress checks as waiting", () => { + for (const status of ["queued", "in_progress"]) { + assert.equal(classifyReleaseCandidateCheck(check({ status, conclusion: null })).state, "waiting") + } + }) + + test("accepts only completed success", () => { + assert.equal(classifyReleaseCandidateCheck(check()).state, "success") + }) + + test("fails completed failure and cancellation", () => { + for (const conclusion of ["failure", "cancelled"]) { + assert.equal(classifyReleaseCandidateCheck(check({ conclusion })).state, "failure") + } + }) + + test("uses the latest exact-name rerun rather than an older success", () => { + const selected = selectLatestReleaseCandidateCheck(payload( + check({ id: 200, conclusion: "success" }), + check({ id: 201, conclusion: "failure" }), + ), sha) + assert.equal(selected.id, 201) + assert.equal(classifyReleaseCandidateCheck(selected).state, "failure") + }) +}) + +describe("release-candidate check polling", () => { + test("polls missing and in-progress fixtures until success", async () => { + const fixtures = [ + payload(), + payload(check({ id: 201, status: "in_progress", conclusion: null })), + payload(check({ id: 201 })), + ] + let now = 0 + const result = await waitForReleaseCandidateCheck({ + sha, + timeoutMs: 10, + pollIntervalMs: 1, + now: () => now, + sleep: async (milliseconds) => { now += milliseconds }, + loadChecks: async () => fixtures.shift(), + log: () => {}, + }) + assert.equal(result.id, 201) + }) + + test("fails immediately on the latest failure or cancellation fixture", async () => { + for (const conclusion of ["failure", "cancelled"]) { + await assert.rejects( + waitForReleaseCandidateCheck({ + sha, + loadChecks: async () => payload(check({ conclusion })), + log: () => {}, + }), + new RegExp(conclusion), + ) + } + }) + + test("times out fail-closed when only wrong SHA/name fixtures exist", async () => { + let now = 0 + await assert.rejects( + waitForReleaseCandidateCheck({ + sha, + timeoutMs: 2, + pollIntervalMs: 1, + now: () => now, + sleep: async (milliseconds) => { now += milliseconds }, + loadChecks: async () => payload( + check({ head_sha: otherSha }), + check({ name: "Release Candidate" }), + ), + log: () => {}, + }), + /Timed out.*check has not been created/, + ) + }) +}) From 069d21715b4bb682ea1f213ab5e3a39399a4ac3c Mon Sep 17 00:00:00 2001 From: hachej Date: Fri, 31 Jul 2026 13:11:51 +0000 Subject: [PATCH 2/4] #391 fix(release): close RC gate safety gaps --- .github/workflows/ci.yml | 1 + .github/workflows/release-candidate.yml | 9 +- .../release-candidate-golden-route.spec.ts | 13 +- .../__tests__/release-candidate-dist.test.ts | 21 +++ apps/workspace-playground/src/front/main.tsx | 8 +- .../src/release-candidate-dist.ts | 8 + apps/workspace-playground/src/vite-env.d.ts | 1 + apps/workspace-playground/vite.config.ts | 26 ++- scripts/cut-release.sh | 150 ++++++++++++------ scripts/require-release-candidate-check.mjs | 72 ++++++--- .../require-release-candidate-check.test.mjs | 89 +++++++---- scripts/validate-release-resume.mjs | 72 +++++++++ scripts/validate-release-resume.test.mjs | 40 +++++ 13 files changed, 383 insertions(+), 127 deletions(-) create mode 100644 apps/workspace-playground/src/__tests__/release-candidate-dist.test.ts create mode 100644 apps/workspace-playground/src/release-candidate-dist.ts create mode 100644 apps/workspace-playground/src/vite-env.d.ts create mode 100644 scripts/validate-release-resume.mjs create mode 100644 scripts/validate-release-resume.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f8b088f6..335d69ea0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,6 +2,7 @@ name: CI on: pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] push: branches: - main diff --git a/.github/workflows/release-candidate.yml b/.github/workflows/release-candidate.yml index 743b24e03..b8f1a21a2 100644 --- a/.github/workflows/release-candidate.yml +++ b/.github/workflows/release-candidate.yml @@ -31,12 +31,15 @@ jobs: - name: Setup Node uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 with: - node-version: 22.19.0 + node-version: 24 cache: pnpm - name: Install run: pnpm install --frozen-lockfile + - name: Test release controls + run: node --test scripts/require-release-candidate-check.test.mjs scripts/validate-release-resume.test.mjs + - name: Build package and plugin release artifacts run: pnpm build:packages env: @@ -54,7 +57,9 @@ jobs: run: pnpm exec playwright install --with-deps chromium - name: Run built-dist release-candidate smoke - run: pnpm --filter workspace-playground run test:e2e:release-candidate + run: | + env -u BORING_RC_BREAK_CSS -u BORING_RC_BREAK_FIRST_SEND \ + pnpm --filter workspace-playground run test:e2e:release-candidate env: BORING_PLAYGROUND_DIST_ONLY: "1" BORING_AGENT_WORKSPACE_ROOT: ${{ runner.temp }}/workspace-playground-release-candidate diff --git a/apps/workspace-playground/e2e/release-candidate-golden-route.spec.ts b/apps/workspace-playground/e2e/release-candidate-golden-route.spec.ts index eee4e8fb5..21750e9fc 100644 --- a/apps/workspace-playground/e2e/release-candidate-golden-route.spec.ts +++ b/apps/workspace-playground/e2e/release-candidate-golden-route.spec.ts @@ -4,9 +4,7 @@ import { assertReleaseCandidateAgentCss } from "../src/release-candidate-css" const agentDistCssPath = "/packages/agent/dist/front/styles.css" function isAgentDistStylesheet(url: string): boolean { - const parsed = new URL(url) - return parsed.searchParams.get("boring-rc-agent-css") === "1" - && decodeURIComponent(parsed.pathname).replaceAll("\\", "/").includes(agentDistCssPath) + return decodeURIComponent(new URL(url).pathname).replaceAll("\\", "/").includes(agentDistCssPath) } test("boots built dist and completes one Alpha first send", async ({ page }) => { @@ -15,7 +13,11 @@ test("boots built dist and completes one Alpha first send", async ({ page }) => const cssFault = process.env.BORING_RC_BREAK_CSS if (cssFault === "small" || cssFault === "mime") { - await page.route(/boring-rc-agent-css=1/, async (route) => { + await page.route(`**${agentDistCssPath}*`, async (route) => { + if (route.request().resourceType() !== "stylesheet") { + await route.continue() + return + } const response = await route.fetch() const body = cssFault === "small" ? Buffer.from("/* deliberately small RC fixture */") : await response.body() await route.fulfill({ @@ -46,7 +48,8 @@ test("boots built dist and completes one Alpha first send", async ({ page }) => } const agentCssResponse = page.waitForResponse( - (response) => isAgentDistStylesheet(response.url()), + (response) => response.request().resourceType() === "stylesheet" + && isAgentDistStylesheet(response.url()), { timeout: 30_000 }, ) await page.goto("/?fresh=1") diff --git a/apps/workspace-playground/src/__tests__/release-candidate-dist.test.ts b/apps/workspace-playground/src/__tests__/release-candidate-dist.test.ts new file mode 100644 index 000000000..1d345658a --- /dev/null +++ b/apps/workspace-playground/src/__tests__/release-candidate-dist.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from "vitest" +import { assertReleaseCandidateDistModule } from "../release-candidate-dist" + +describe("release-candidate dist-only module guard", () => { + it.each([ + "/repo/packages/agent/src/front/index.ts", + "/repo/plugins/tasks/src/front/index.tsx?import", + ])("rejects package source loaded through any Vite hook: %s", (id) => { + expect(() => assertReleaseCandidateDistModule(id, "transform")).toThrow( + /release-candidate dist-only resolution violation/, + ) + }) + + it.each([ + "/repo/packages/agent/dist/front/index.js", + "/repo/plugins/tasks/dist/front/index.js", + "/repo/apps/workspace-playground/src/front/main.tsx", + ])("allows dist packages and playground fixture source: %s", (id) => { + expect(() => assertReleaseCandidateDistModule(id, "load")).not.toThrow() + }) +}) diff --git a/apps/workspace-playground/src/front/main.tsx b/apps/workspace-playground/src/front/main.tsx index 3eaadf783..e39732b71 100644 --- a/apps/workspace-playground/src/front/main.tsx +++ b/apps/workspace-playground/src/front/main.tsx @@ -2,9 +2,15 @@ import { StrictMode } from "react" import { createRoot } from "react-dom/client" import { WorkspaceShell } from "./App" import "@hachej/boring-workspace/globals.css" -import "@hachej/boring-agent/front/styles.css" +import agentStylesheetUrl from "@hachej/boring-agent/front/styles.css?url" import "./app.css" +const agentStylesheet = document.createElement("link") +agentStylesheet.rel = "stylesheet" +agentStylesheet.href = agentStylesheetUrl +agentStylesheet.dataset.boringAgentStylesheet = "package-import" +document.head.append(agentStylesheet) + // The playground is the standalone dev surface for @hachej/boring-workspace. // Auth, DB, user management, config — all of that belongs to @hachej/boring-core // and is exercised separately by apps/full-app. This app still starts the diff --git a/apps/workspace-playground/src/release-candidate-dist.ts b/apps/workspace-playground/src/release-candidate-dist.ts new file mode 100644 index 000000000..bfec19e4b --- /dev/null +++ b/apps/workspace-playground/src/release-candidate-dist.ts @@ -0,0 +1,8 @@ +export function assertReleaseCandidateDistModule(id: string, hook: string): void { + const normalized = id.split("?", 1)[0].replaceAll("\\", "/") + if (/\/(packages|plugins)\/[^/]+\/src(?:\/|$)/.test(normalized)) { + throw new Error( + `release-candidate dist-only resolution violation: ${hook} loaded ${normalized}`, + ) + } +} diff --git a/apps/workspace-playground/src/vite-env.d.ts b/apps/workspace-playground/src/vite-env.d.ts new file mode 100644 index 000000000..11f02fe2a --- /dev/null +++ b/apps/workspace-playground/src/vite-env.d.ts @@ -0,0 +1 @@ +/// diff --git a/apps/workspace-playground/vite.config.ts b/apps/workspace-playground/vite.config.ts index b972c2801..f627b2fc8 100644 --- a/apps/workspace-playground/vite.config.ts +++ b/apps/workspace-playground/vite.config.ts @@ -4,6 +4,7 @@ import tailwindcss from "@tailwindcss/vite" import { dirname, resolve } from "node:path" import { createBoringAppViteAliases } from "@hachej/boring-core/app/vite" import { AGENT_API_PORT, VITE_PORT, startPlaygroundServer } from "./src/server/dev" +import { assertReleaseCandidateDistModule } from "./src/release-candidate-dist" const baseResolve = createBoringAppViteAliases({ appRoot: __dirname }) const repoRoot = resolve(__dirname, "../..") @@ -23,25 +24,16 @@ function releaseCandidateDistOnlyGuard(): Plugin { const resolved = await this.resolve(source, importer, { ...options, skipSelf: true }) if (!resolved || resolved.external) return resolved - const normalized = resolved.id.split("?", 1)[0].replaceAll("\\", "/") - const sourcePackage = /\/(packages|plugins)\/[^/]+\/src(?:\/|$)/.test(normalized) - if (sourcePackage) { - throw new Error( - `release-candidate dist-only resolution violation: ${source} resolved to ${normalized}`, - ) - } + assertReleaseCandidateDistModule(resolved.id, `resolved ${source}`) return resolved }, - transformIndexHtml() { - const agentCss = resolve(repoRoot, "packages/agent/dist/front/styles.css").replaceAll("\\", "/") - return [{ - tag: "link", - attrs: { - rel: "stylesheet", - href: `/@fs${agentCss}?direct&boring-rc-agent-css=1`, - }, - injectTo: "head", - }] + load(id) { + assertReleaseCandidateDistModule(id, "load") + return null + }, + transform(_code, id) { + assertReleaseCandidateDistModule(id, "transform") + return null }, } } diff --git a/scripts/cut-release.sh b/scripts/cut-release.sh index 32791bc26..9fd8d2a56 100755 --- a/scripts/cut-release.sh +++ b/scripts/cut-release.sh @@ -1,32 +1,47 @@ #!/usr/bin/env bash -# Cut a release: bump all publishable package versions, commit, push, and -# create a GitHub release. The Release workflow auto-fires on the -# `release: published` event and publishes to npm. +# Cut a release: bump all publishable package versions, commit, push, wait for +# exact-SHA release gates, and create a GitHub release. Use --resume after a +# post-push gate failure to reuse the existing untagged bump commit. # # Usage: # ./scripts/cut-release.sh # patch bump (default) # ./scripts/cut-release.sh minor # ./scripts/cut-release.sh major +# ./scripts/cut-release.sh --resume set -euo pipefail cd "$(dirname "$0")/.." +resume=false bump="${1:-patch}" -case "$bump" in patch|minor|major) ;; *) - echo "Usage: $0 [patch|minor|major]" >&2 - exit 2 +case "$bump" in + patch|minor|major) + if [ "$#" -ne 0 ] && [ "$#" -ne 1 ]; then + echo "Usage: $0 [patch|minor|major|--resume]" >&2 + exit 2 + fi + ;; + --resume) + if [ "$#" -ne 1 ]; then + echo "Usage: $0 [patch|minor|major|--resume]" >&2 + exit 2 + fi + resume=true + ;; + *) + echo "Usage: $0 [patch|minor|major|--resume]" >&2 + exit 2 + ;; esac -# Refuse to bump from a dirty tree — the release commit must contain only -# version markers and required generated release evidence. -if ! git diff --quiet || ! git diff --cached --quiet; then +# Refuse any dirty or untracked state. A release commit and a resumed release +# must both be fully reproducible from synchronized main. +if [ -n "$(git status --porcelain)" ]; then echo "Working tree is dirty. Commit or stash first." >&2 exit 1 fi -# Refuse to release from anything other than main, and make sure we're -# in sync with origin so the tag we cut points at the same SHA people see. branch=$(git branch --show-current) if [ "$branch" != "main" ]; then echo "Release must run on main; got '$branch'." >&2 @@ -38,14 +53,6 @@ if [ "$(git rev-parse HEAD)" != "$(git rev-parse origin/main)" ]; then exit 1 fi -before=$(node -p "require('./package.json').version") -node scripts/version.mjs "$bump" -after=$(node -p "require('./package.json').version") -node scripts/version.mjs --check -pnpm golden-path:timing -pnpm check:golden-path -pnpm audit:publish-manifests - release_files=( package.json docs/issues/391/runtime-refactor/golden-path.json @@ -74,43 +81,92 @@ if [ -f pnpm-lock.yaml ]; then release_files+=(pnpm-lock.yaml) fi -git add "${release_files[@]}" -node scripts/check-release-staging.mjs +assert_remote_tag_absent() { + local tag_name=$1 + local status + set +e + git ls-remote --exit-code --tags origin "refs/tags/$tag_name" >/dev/null 2>&1 + status=$? + set -e + if [ "$status" -eq 0 ]; then + echo "Release tag $tag_name already exists on origin; refusing to recreate it." >&2 + exit 1 + fi + if [ "$status" -ne 2 ]; then + echo "Could not verify whether release tag $tag_name exists on origin." >&2 + exit 1 + fi +} -status=$(git status --short) -if [ -z "$status" ]; then - echo "No release changes staged." >&2 - exit 1 -fi -while IFS= read -r line; do - [ -z "$line" ] && continue - path=${line:3} - allowed=false - for release_file in "${release_files[@]}"; do - if [ "$path" = "$release_file" ]; then - allowed=true - break - fi - done - if [ "$allowed" != true ]; then - echo "Unexpected release tree change: $line" >&2 - echo "$status" >&2 +if [ "$resume" = true ]; then + after=$(node -p "require('./package.json').version") + before=$(git show HEAD^:package.json | node -e "let s=''; process.stdin.on('data', c => s += c); process.stdin.on('end', () => console.log(JSON.parse(s).version))") + node scripts/version.mjs --check + node scripts/validate-release-resume.mjs "${release_files[@]}" + release_sha=$(git rev-parse HEAD) + tag="v$after" + assert_remote_tag_absent "$tag" + echo "Resuming release $tag from existing bump commit $release_sha." +else + before=$(node -p "require('./package.json').version") + node scripts/version.mjs "$bump" + after=$(node -p "require('./package.json').version") + node scripts/version.mjs --check + pnpm golden-path:timing + pnpm check:golden-path + pnpm audit:publish-manifests + + git add "${release_files[@]}" + node scripts/check-release-staging.mjs + + status=$(git status --short) + if [ -z "$status" ]; then + echo "No release changes staged." >&2 exit 1 fi -done <<< "$status" + while IFS= read -r line; do + [ -z "$line" ] && continue + path=${line:3} + allowed=false + for release_file in "${release_files[@]}"; do + if [ "$path" = "$release_file" ]; then + allowed=true + break + fi + done + if [ "$allowed" != true ]; then + echo "Unexpected release tree change: $line" >&2 + echo "$status" >&2 + exit 1 + fi + done <<< "$status" -git commit -m "chore(release): bump packages to $after" -git push origin main + git commit -m "chore(release): bump packages to $after" + git push origin main + release_sha=$(git rev-parse HEAD) + tag="v$after" +fi -release_sha=$(git rev-parse HEAD) repository=$(gh repo view --json nameWithOwner --jq .nameWithOwner) -if ! GH_REPOSITORY="$repository" node scripts/require-release-candidate-check.mjs "$release_sha"; then - echo "Release-candidate gate did not pass for pushed release commit $release_sha." >&2 - echo "No GitHub release or tag was created. Fix or revert through normal git, then rerun." >&2 +if ! GH_REPOSITORY="$repository" node scripts/require-release-candidate-check.mjs \ + "$release_sha" \ + "Release Candidate Built-Dist" \ + "Main Green Summary"; then + echo "Required release gates did not pass for pushed release commit $release_sha." >&2 + echo "No GitHub release or tag was created. After the checks are fixed/green, run:" >&2 + echo " ./scripts/cut-release.sh --resume" >&2 + exit 1 +fi + +# Revalidate the branch and tag after the potentially long polling window. A +# later main push must never cause this invocation to publish a stale target. +git fetch origin main +if [ "$(git rev-parse HEAD)" != "$release_sha" ] || [ "$(git rev-parse origin/main)" != "$release_sha" ]; then + echo "origin/main moved while release gates were running; refusing to release $release_sha." >&2 exit 1 fi +assert_remote_tag_absent "$tag" -tag="v$after" echo "Creating GitHub release $tag (this also creates the git tag)…" gh release create "$tag" \ --title "$tag" \ diff --git a/scripts/require-release-candidate-check.mjs b/scripts/require-release-candidate-check.mjs index 305cbd664..1dc562516 100644 --- a/scripts/require-release-candidate-check.mjs +++ b/scripts/require-release-candidate-check.mjs @@ -5,22 +5,27 @@ import { fileURLToPath } from "node:url" import { resolve } from "node:path" export const RELEASE_CANDIDATE_CHECK_NAME = "Release Candidate Built-Dist" +export const MAIN_GREEN_CHECK_NAME = "Main Green Summary" +export const DEFAULT_REQUIRED_CHECK_NAMES = [ + RELEASE_CANDIDATE_CHECK_NAME, + MAIN_GREEN_CHECK_NAME, +] function checkId(check) { try { return BigInt(check.id) } catch { - throw new Error(`Release-candidate check has an invalid id: ${String(check.id)}`) + throw new Error(`Required check has an invalid id: ${String(check.id)}`) } } -export function selectLatestReleaseCandidateCheck(payload, sha) { +export function selectLatestRequiredCheck(payload, sha, name) { if (!payload || !Array.isArray(payload.check_runs)) { throw new Error("GitHub check-runs response is missing check_runs") } const matches = payload.check_runs.filter((check) => - check?.name === RELEASE_CANDIDATE_CHECK_NAME + check?.name === name && check?.head_sha === sha && check?.app?.slug === "github-actions" ) @@ -29,7 +34,7 @@ export function selectLatestReleaseCandidateCheck(payload, sha) { return matches.reduce((latest, check) => checkId(check) > checkId(latest) ? check : latest) } -export function classifyReleaseCandidateCheck(check) { +export function classifyRequiredCheck(check) { if (!check) return { state: "waiting", description: "check has not been created" } if (check.status !== "completed") { return { state: "waiting", description: `latest check ${check.id} is ${check.status}` } @@ -43,34 +48,47 @@ export function classifyReleaseCandidateCheck(check) { } } -export async function waitForReleaseCandidateCheck({ +export async function waitForRequiredChecks({ sha, + names, loadChecks, - timeoutMs = 30 * 60 * 1_000, + timeoutMs = 45 * 60 * 1_000, pollIntervalMs = 15_000, now = Date.now, sleep = (milliseconds) => new Promise((resolveSleep) => setTimeout(resolveSleep, milliseconds)), log = console.log, }) { + if (!Array.isArray(names) || names.length === 0 || names.some((name) => typeof name !== "string" || !name.trim())) { + throw new Error("At least one exact required check name is required") + } + if (new Set(names).size !== names.length) throw new Error("Required check names must be unique") + const startedAt = now() - let lastDescription = "check has not been queried" + let lastWaiting = "checks have not been queried" while (true) { - const payload = await loadChecks() - const check = selectLatestReleaseCandidateCheck(payload, sha) - const classification = classifyReleaseCandidateCheck(check) - lastDescription = classification.description - - if (classification.state === "success") return check - if (classification.state === "failure") { - throw new Error(`Release-candidate gate failed: ${classification.description}`) + const currentSuccesses = new Map() + const waiting = [] + for (const name of names) { + const payload = await loadChecks(name) + const check = selectLatestRequiredCheck(payload, sha, name) + const classification = classifyRequiredCheck(check) + if (classification.state === "success") { + currentSuccesses.set(name, check) + } else if (classification.state === "failure") { + throw new Error(`Required check "${name}" failed: ${classification.description}`) + } else { + waiting.push(`${name}: ${classification.description}`) + } } + if (currentSuccesses.size === names.length) return currentSuccesses + lastWaiting = waiting.join("; ") || lastWaiting const elapsed = now() - startedAt if (elapsed >= timeoutMs) { - throw new Error(`Timed out waiting for ${RELEASE_CANDIDATE_CHECK_NAME} on ${sha}: ${lastDescription}`) + throw new Error(`Timed out waiting for required checks on ${sha}: ${lastWaiting}`) } - log(`Waiting for ${RELEASE_CANDIDATE_CHECK_NAME} on ${sha}: ${lastDescription}`) + log(`Waiting for required checks on ${sha}: ${lastWaiting}`) await sleep(Math.min(pollIntervalMs, timeoutMs - elapsed)) } } @@ -84,26 +102,28 @@ function positiveNumber(value, fallback, name) { async function main() { const sha = process.argv[2] + const names = process.argv.slice(3) const repository = process.env.GH_REPOSITORY - if (!/^[0-9a-f]{40}$/i.test(sha ?? "")) { - throw new Error("Usage: require-release-candidate-check.mjs <40-character-release-sha>") + if (!/^[0-9a-f]{40}$/i.test(sha ?? "") || names.length === 0) { + throw new Error("Usage: require-release-candidate-check.mjs <40-character-release-sha> [...]") } if (!/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repository ?? "")) { throw new Error("GH_REPOSITORY must be set to owner/repository") } - const timeoutMs = positiveNumber(process.env.RELEASE_CHECK_TIMEOUT_SECONDS, 30 * 60, "RELEASE_CHECK_TIMEOUT_SECONDS") * 1_000 + const timeoutMs = positiveNumber(process.env.RELEASE_CHECK_TIMEOUT_SECONDS, 45 * 60, "RELEASE_CHECK_TIMEOUT_SECONDS") * 1_000 const pollIntervalMs = positiveNumber(process.env.RELEASE_CHECK_POLL_SECONDS, 15, "RELEASE_CHECK_POLL_SECONDS") * 1_000 - const endpoint = `repos/${repository}/commits/${sha}/check-runs?check_name=${encodeURIComponent(RELEASE_CANDIDATE_CHECK_NAME)}&filter=all&per_page=100` const actionsUrl = `https://github.com/${repository}/actions?query=${encodeURIComponent(`head_sha:${sha}`)}` - console.log(`Requiring exact check "${RELEASE_CANDIDATE_CHECK_NAME}" for ${repository}@${sha}`) + console.log(`Requiring exact checks for ${repository}@${sha}: ${names.join(", ")}`) console.log(`Actions: ${actionsUrl}`) - const check = await waitForReleaseCandidateCheck({ + const passed = await waitForRequiredChecks({ sha, + names, timeoutMs, pollIntervalMs, - loadChecks: () => { + loadChecks: (name) => { + const endpoint = `repos/${repository}/commits/${sha}/check-runs?check_name=${encodeURIComponent(name)}&filter=all&per_page=100` const output = execFileSync("gh", ["api", "--method", "GET", endpoint], { encoding: "utf8", stdio: ["ignore", "pipe", "inherit"], @@ -111,7 +131,9 @@ async function main() { return JSON.parse(output) }, }) - console.log(`Release-candidate gate passed: ${check.html_url ?? actionsUrl}`) + for (const name of names) { + console.log(`Required check green: ${name}: ${passed.get(name)?.html_url ?? actionsUrl}`) + } } const isDirectInvocation = process.argv[1] diff --git a/scripts/require-release-candidate-check.test.mjs b/scripts/require-release-candidate-check.test.mjs index ccb5b9735..81f63b526 100644 --- a/scripts/require-release-candidate-check.test.mjs +++ b/scripts/require-release-candidate-check.test.mjs @@ -1,9 +1,11 @@ import assert from "node:assert/strict" import { describe, test } from "node:test" import { - classifyReleaseCandidateCheck, - selectLatestReleaseCandidateCheck, - waitForReleaseCandidateCheck, + MAIN_GREEN_CHECK_NAME, + RELEASE_CANDIDATE_CHECK_NAME, + classifyRequiredCheck, + selectLatestRequiredCheck, + waitForRequiredChecks, } from "./require-release-candidate-check.mjs" const sha = "a".repeat(40) @@ -12,7 +14,7 @@ const otherSha = "b".repeat(40) function check(overrides = {}) { return { id: 100, - name: "Release Candidate Built-Dist", + name: RELEASE_CANDIDATE_CHECK_NAME, head_sha: sha, status: "completed", conclusion: "success", @@ -25,78 +27,98 @@ function payload(...checks) { return { check_runs: checks } } -describe("release-candidate check selection", () => { +describe("required check selection", () => { test("treats a missing exact check as waiting", () => { - assert.deepEqual(classifyReleaseCandidateCheck(selectLatestReleaseCandidateCheck(payload(), sha)), { + assert.deepEqual(classifyRequiredCheck(selectLatestRequiredCheck(payload(), sha, RELEASE_CANDIDATE_CHECK_NAME)), { state: "waiting", description: "check has not been created", }) }) test("ignores checks with the wrong SHA, name, or app", () => { - const selected = selectLatestReleaseCandidateCheck(payload( + const selected = selectLatestRequiredCheck(payload( check({ id: 101, head_sha: otherSha }), - check({ id: 102, name: "Main Green Summary" }), + check({ id: 102, name: MAIN_GREEN_CHECK_NAME }), check({ id: 103, app: { slug: "external-ci" } }), - ), sha) + ), sha, RELEASE_CANDIDATE_CHECK_NAME) assert.equal(selected, null) }) test("classifies queued and in-progress checks as waiting", () => { for (const status of ["queued", "in_progress"]) { - assert.equal(classifyReleaseCandidateCheck(check({ status, conclusion: null })).state, "waiting") + assert.equal(classifyRequiredCheck(check({ status, conclusion: null })).state, "waiting") } }) test("accepts only completed success", () => { - assert.equal(classifyReleaseCandidateCheck(check()).state, "success") + assert.equal(classifyRequiredCheck(check()).state, "success") }) test("fails completed failure and cancellation", () => { for (const conclusion of ["failure", "cancelled"]) { - assert.equal(classifyReleaseCandidateCheck(check({ conclusion })).state, "failure") + assert.equal(classifyRequiredCheck(check({ conclusion })).state, "failure") } }) test("uses the latest exact-name rerun rather than an older success", () => { - const selected = selectLatestReleaseCandidateCheck(payload( + const selected = selectLatestRequiredCheck(payload( check({ id: 200, conclusion: "success" }), check({ id: 201, conclusion: "failure" }), - ), sha) + ), sha, RELEASE_CANDIDATE_CHECK_NAME) assert.equal(selected.id, 201) - assert.equal(classifyReleaseCandidateCheck(selected).state, "failure") + assert.equal(classifyRequiredCheck(selected).state, "failure") }) }) -describe("release-candidate check polling", () => { - test("polls missing and in-progress fixtures until success", async () => { - const fixtures = [ - payload(), - payload(check({ id: 201, status: "in_progress", conclusion: null })), - payload(check({ id: 201 })), - ] +describe("required check polling", () => { + test("waits for both exact required names on the same SHA", async () => { + const fixtures = new Map([ + [RELEASE_CANDIDATE_CHECK_NAME, [payload(), payload(check({ id: 201 }))]], + [MAIN_GREEN_CHECK_NAME, [payload(check({ id: 301, name: MAIN_GREEN_CHECK_NAME, status: "in_progress", conclusion: null })), payload(check({ id: 301, name: MAIN_GREEN_CHECK_NAME }))]], + ]) let now = 0 - const result = await waitForReleaseCandidateCheck({ + const result = await waitForRequiredChecks({ sha, + names: [RELEASE_CANDIDATE_CHECK_NAME, MAIN_GREEN_CHECK_NAME], timeoutMs: 10, pollIntervalMs: 1, now: () => now, sleep: async (milliseconds) => { now += milliseconds }, - loadChecks: async () => fixtures.shift(), + loadChecks: async (name) => fixtures.get(name).shift(), log: () => {}, }) - assert.equal(result.id, 201) + assert.equal(result.get(RELEASE_CANDIDATE_CHECK_NAME).id, 201) + assert.equal(result.get(MAIN_GREEN_CHECK_NAME).id, 301) }) - test("fails immediately on the latest failure or cancellation fixture", async () => { + test("revalidates an earlier success while another required check is pending", async () => { + const fixtures = new Map([ + [RELEASE_CANDIDATE_CHECK_NAME, [payload(check({ id: 201 })), payload(check({ id: 202, conclusion: "failure" }))]], + [MAIN_GREEN_CHECK_NAME, [payload(check({ id: 301, name: MAIN_GREEN_CHECK_NAME, status: "in_progress", conclusion: null }))]], + ]) + let now = 0 + await assert.rejects(waitForRequiredChecks({ + sha, + names: [RELEASE_CANDIDATE_CHECK_NAME, MAIN_GREEN_CHECK_NAME], + timeoutMs: 10, + pollIntervalMs: 1, + now: () => now, + sleep: async (milliseconds) => { now += milliseconds }, + loadChecks: async (name) => fixtures.get(name).shift(), + log: () => {}, + }), /Release Candidate Built-Dist.*failure/) + }) + + test("fails immediately when either latest required check fails or is cancelled", async () => { for (const conclusion of ["failure", "cancelled"]) { await assert.rejects( - waitForReleaseCandidateCheck({ + waitForRequiredChecks({ sha, - loadChecks: async () => payload(check({ conclusion })), + names: [RELEASE_CANDIDATE_CHECK_NAME, MAIN_GREEN_CHECK_NAME], + loadChecks: async (name) => payload(check({ name, conclusion: name === MAIN_GREEN_CHECK_NAME ? conclusion : "success" })), log: () => {}, }), - new RegExp(conclusion), + new RegExp(`${MAIN_GREEN_CHECK_NAME}.*${conclusion}`), ) } }) @@ -104,8 +126,9 @@ describe("release-candidate check polling", () => { test("times out fail-closed when only wrong SHA/name fixtures exist", async () => { let now = 0 await assert.rejects( - waitForReleaseCandidateCheck({ + waitForRequiredChecks({ sha, + names: [RELEASE_CANDIDATE_CHECK_NAME], timeoutMs: 2, pollIntervalMs: 1, now: () => now, @@ -119,4 +142,10 @@ describe("release-candidate check polling", () => { /Timed out.*check has not been created/, ) }) + + test("rejects missing or duplicate required names", async () => { + for (const names of [[], [MAIN_GREEN_CHECK_NAME, MAIN_GREEN_CHECK_NAME]]) { + await assert.rejects(waitForRequiredChecks({ sha, names, loadChecks: async () => payload() })) + } + }) }) diff --git a/scripts/validate-release-resume.mjs b/scripts/validate-release-resume.mjs new file mode 100644 index 000000000..8d0b40b6a --- /dev/null +++ b/scripts/validate-release-resume.mjs @@ -0,0 +1,72 @@ +#!/usr/bin/env node + +import { execFileSync } from "node:child_process" +import { readFileSync } from "node:fs" +import { fileURLToPath } from "node:url" +import { resolve } from "node:path" + +export function validateReleaseResumeState({ + version, + parentVersion, + commitSubject, + changedFiles, + allowedFiles, + tagsAtHead, +}) { + const errors = [] + if (!/^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/.test(version)) { + errors.push(`current package version is invalid: ${version}`) + } + if (parentVersion === version) errors.push("current commit does not change the root package version") + if (commitSubject !== `chore(release): bump packages to ${version}`) { + errors.push(`current commit is not the expected release bump for ${version}`) + } + if (!Array.isArray(changedFiles) || changedFiles.length === 0) { + errors.push("current release bump commit has no changed files") + } else { + const allowed = new Set(allowedFiles) + for (const file of changedFiles) { + if (!allowed.has(file)) errors.push(`release bump commit changed unexpected file: ${file}`) + } + } + if (tagsAtHead.length > 0) errors.push(`current release bump is already tagged: ${tagsAtHead.join(", ")}`) + if (errors.length > 0) throw new Error(errors.join("\n")) +} + +function git(...args) { + return execFileSync("git", args, { encoding: "utf8" }).trim() +} + +function packageVersion(contents) { + return JSON.parse(contents).version +} + +function main() { + const allowedFiles = process.argv.slice(2) + if (allowedFiles.length === 0) throw new Error("validate-release-resume requires allowed release file paths") + const version = packageVersion(readFileSync("package.json", "utf8")) + const parentVersion = packageVersion(git("show", "HEAD^:package.json")) + const commitSubject = git("log", "-1", "--pretty=%s") + const changedFiles = git("diff-tree", "--no-commit-id", "--name-only", "-r", "HEAD").split("\n").filter(Boolean) + const tagsAtHead = git("tag", "--points-at", "HEAD").split("\n").filter(Boolean) + validateReleaseResumeState({ + version, + parentVersion, + commitSubject, + changedFiles, + allowedFiles, + tagsAtHead, + }) + console.log(`Release resume state is valid for untagged v${version} bump at ${git("rev-parse", "HEAD")}.`) +} + +const isDirectInvocation = process.argv[1] + && fileURLToPath(import.meta.url) === resolve(process.argv[1]) +if (isDirectInvocation) { + try { + main() + } catch (error) { + console.error(error instanceof Error ? error.message : error) + process.exitCode = 1 + } +} diff --git a/scripts/validate-release-resume.test.mjs b/scripts/validate-release-resume.test.mjs new file mode 100644 index 000000000..a5b542421 --- /dev/null +++ b/scripts/validate-release-resume.test.mjs @@ -0,0 +1,40 @@ +import assert from "node:assert/strict" +import { describe, test } from "node:test" +import { validateReleaseResumeState } from "./validate-release-resume.mjs" + +const valid = { + version: "0.1.94", + parentVersion: "0.1.93", + commitSubject: "chore(release): bump packages to 0.1.94", + changedFiles: ["package.json", "packages/agent/package.json"], + allowedFiles: ["package.json", "packages/agent/package.json", "pnpm-lock.yaml"], + tagsAtHead: [], +} + +describe("release resume validation", () => { + test("accepts a cleanly shaped untagged version bump commit", () => { + assert.doesNotThrow(() => validateReleaseResumeState(valid)) + }) + + test("rejects a non-bump commit or unchanged version", () => { + assert.throws(() => validateReleaseResumeState({ + ...valid, + parentVersion: valid.version, + commitSubject: "fix: unrelated", + }), /does not change.*not the expected release bump/s) + }) + + test("rejects unexpected files in the bump commit", () => { + assert.throws(() => validateReleaseResumeState({ + ...valid, + changedFiles: [...valid.changedFiles, "src/product.ts"], + }), /unexpected file: src\/product.ts/) + }) + + test("rejects an already tagged bump", () => { + assert.throws(() => validateReleaseResumeState({ + ...valid, + tagsAtHead: ["v0.1.94"], + }), /already tagged/) + }) +}) From 6e8c1219f890a86c06b3cf65f86cc68c34626d9b Mon Sep 17 00:00:00 2001 From: hachej Date: Fri, 31 Jul 2026 13:31:38 +0000 Subject: [PATCH 3/4] #391 fix(release): atomically bind release tags --- .github/workflows/release-candidate.yml | 2 +- scripts/atomic-release-tag.mjs | 91 ++++++++++++++++++++++++ scripts/atomic-release-tag.test.mjs | 71 ++++++++++++++++++ scripts/cut-release.sh | 81 +++++++++++++++++---- scripts/validate-release-resume.mjs | 31 ++++++-- scripts/validate-release-resume.test.mjs | 43 ++++++++--- 6 files changed, 291 insertions(+), 28 deletions(-) create mode 100644 scripts/atomic-release-tag.mjs create mode 100644 scripts/atomic-release-tag.test.mjs diff --git a/.github/workflows/release-candidate.yml b/.github/workflows/release-candidate.yml index b8f1a21a2..6a20f6452 100644 --- a/.github/workflows/release-candidate.yml +++ b/.github/workflows/release-candidate.yml @@ -38,7 +38,7 @@ jobs: run: pnpm install --frozen-lockfile - name: Test release controls - run: node --test scripts/require-release-candidate-check.test.mjs scripts/validate-release-resume.test.mjs + run: node --test scripts/atomic-release-tag.test.mjs scripts/require-release-candidate-check.test.mjs scripts/validate-release-resume.test.mjs - name: Build package and plugin release artifacts run: pnpm build:packages diff --git a/scripts/atomic-release-tag.mjs b/scripts/atomic-release-tag.mjs new file mode 100644 index 000000000..d20d368e8 --- /dev/null +++ b/scripts/atomic-release-tag.mjs @@ -0,0 +1,91 @@ +#!/usr/bin/env node + +import { spawnSync } from "node:child_process" +import { fileURLToPath } from "node:url" +import { resolve } from "node:path" + +function defaultRunGit(args) { + const result = spawnSync("git", args, { encoding: "utf8" }) + return { + status: result.status ?? 1, + stdout: result.stdout ?? "", + stderr: result.stderr ?? "", + } +} + +function firstSha(output) { + return output.trim().split(/\s+/, 1)[0] || null +} + +export function atomicReleaseRefspecs(releaseSha, tag) { + return [ + "push", + "--atomic", + `--force-with-lease=refs/heads/main:${releaseSha}`, + "origin", + `${releaseSha}:refs/heads/main`, + `refs/tags/${tag}:refs/tags/${tag}`, + ] +} + +export function pushAnnotatedReleaseTagAtomically({ releaseSha, tag, runGit = defaultRunGit }) { + const created = runGit(["tag", "-a", tag, releaseSha, "-m", tag]) + if (created.status !== 0) { + throw new Error(`Could not create local annotated release tag ${tag}: ${created.stderr.trim()}`) + } + + const pushed = runGit(atomicReleaseRefspecs(releaseSha, tag)) + if (pushed.status === 0) return + + const remoteTag = runGit([ + "ls-remote", + "--exit-code", + "--tags", + "origin", + `refs/tags/${tag}`, + `refs/tags/${tag}^{}`, + ]) + const remoteMain = runGit(["ls-remote", "--exit-code", "--heads", "origin", "refs/heads/main"]) + + // Delete only the local tag created by this invocation when the remote can + // authoritatively prove the atomic push created no tag. + if (remoteTag.status === 2) { + const localTag = runGit(["rev-parse", `refs/tags/${tag}^{}`]) + if (localTag.status === 0 && firstSha(localTag.stdout) === releaseSha) { + runGit(["tag", "-d", tag]) + } + } + + const remoteMainSha = remoteMain.status === 0 ? firstSha(remoteMain.stdout) : null + if (remoteMainSha && remoteMainSha !== releaseSha) { + throw new Error( + `Atomic release tag push failed because origin/main advanced from ${releaseSha} to ${remoteMainSha}; no release was created.`, + ) + } + if (remoteTag.status === 0) { + throw new Error( + `Atomic release tag push reported failure but ${tag} exists remotely; keep the local tag and run ./scripts/cut-release.sh --resume.`, + ) + } + throw new Error(`Atomic release tag push failed: ${pushed.stderr.trim()}`) +} + +function main() { + const [releaseSha, tag] = process.argv.slice(2) + if (!/^[0-9a-f]{40}$/i.test(releaseSha ?? "") || !/^v[0-9A-Za-z.+-]+$/.test(tag ?? "")) { + throw new Error("Usage: atomic-release-tag.mjs <40-character-release-sha> ") + } + pushAnnotatedReleaseTagAtomically({ releaseSha, tag }) + console.log(`Atomically asserted main and pushed annotated tag ${tag} at ${releaseSha}.`) +} + +const isDirectInvocation = process.argv[1] + && fileURLToPath(import.meta.url) === resolve(process.argv[1]) +if (isDirectInvocation) { + try { + main() + } catch (error) { + console.error(error instanceof Error ? error.message : error) + process.exitCode = 1 + } +} diff --git a/scripts/atomic-release-tag.test.mjs b/scripts/atomic-release-tag.test.mjs new file mode 100644 index 000000000..0898293c2 --- /dev/null +++ b/scripts/atomic-release-tag.test.mjs @@ -0,0 +1,71 @@ +import assert from "node:assert/strict" +import { describe, test } from "node:test" +import { + atomicReleaseRefspecs, + pushAnnotatedReleaseTagAtomically, +} from "./atomic-release-tag.mjs" + +const releaseSha = "a".repeat(40) +const advancedSha = "b".repeat(40) +const tag = "v0.1.94" + +function fixtureRunner(fixtures, calls) { + return (args) => { + calls.push(args) + const fixture = fixtures.shift() + assert.ok(fixture, `unexpected git call: ${args.join(" ")}`) + return { stdout: "", stderr: "", ...fixture } + } +} + +describe("atomic release tag push", () => { + test("pushes a main assertion and annotated tag in one atomic transaction", () => { + assert.deepEqual(atomicReleaseRefspecs(releaseSha, tag), [ + "push", + "--atomic", + `--force-with-lease=refs/heads/main:${releaseSha}`, + "origin", + `${releaseSha}:refs/heads/main`, + `refs/tags/${tag}:refs/tags/${tag}`, + ]) + const calls = [] + pushAnnotatedReleaseTagAtomically({ + releaseSha, + tag, + runGit: fixtureRunner([{ status: 0 }, { status: 0 }], calls), + }) + assert.deepEqual(calls[0], ["tag", "-a", tag, releaseSha, "-m", tag]) + }) + + test("fails when main advanced and safely removes an unpushed local tag", () => { + const calls = [] + const runGit = fixtureRunner([ + { status: 0 }, + { status: 1, stderr: "atomic push rejected" }, + { status: 2 }, + { status: 0, stdout: `${advancedSha}\trefs/heads/main\n` }, + { status: 0, stdout: `${releaseSha}\n` }, + { status: 0 }, + ], calls) + assert.throws( + () => pushAnnotatedReleaseTagAtomically({ releaseSha, tag, runGit }), + /origin\/main advanced/, + ) + assert.deepEqual(calls.at(-1), ["tag", "-d", tag]) + }) + + test("keeps the local tag when remote tag state cannot be proven absent", () => { + const calls = [] + const runGit = fixtureRunner([ + { status: 0 }, + { status: 1, stderr: "connection lost" }, + { status: 1, stderr: "connection lost" }, + { status: 1, stderr: "connection lost" }, + ], calls) + assert.throws( + () => pushAnnotatedReleaseTagAtomically({ releaseSha, tag, runGit }), + /Atomic release tag push failed/, + ) + assert.equal(calls.some((args) => args[0] === "tag" && args[1] === "-d"), false) + }) +}) diff --git a/scripts/cut-release.sh b/scripts/cut-release.sh index 9fd8d2a56..e7c91311b 100755 --- a/scripts/cut-release.sh +++ b/scripts/cut-release.sh @@ -81,19 +81,43 @@ if [ -f pnpm-lock.yaml ]; then release_files+=(pnpm-lock.yaml) fi -assert_remote_tag_absent() { +read_remote_tag_sha() { local tag_name=$1 - local status + local output status peeled set +e - git ls-remote --exit-code --tags origin "refs/tags/$tag_name" >/dev/null 2>&1 + output=$(git ls-remote --exit-code --tags origin \ + "refs/tags/$tag_name" "refs/tags/$tag_name^{}" 2>/dev/null) status=$? set -e - if [ "$status" -eq 0 ]; then - echo "Release tag $tag_name already exists on origin; refusing to recreate it." >&2 + if [ "$status" -eq 2 ]; then + echo "" + return + fi + if [ "$status" -ne 0 ]; then + echo "Could not read release tag $tag_name from origin." >&2 exit 1 fi - if [ "$status" -ne 2 ]; then - echo "Could not verify whether release tag $tag_name exists on origin." >&2 + peeled=$(awk '$2 ~ /\^\{\}$/ { print $1; exit }' <<< "$output") + if [ -n "$peeled" ]; then + echo "$peeled" + else + awk 'NR == 1 { print $1 }' <<< "$output" + fi +} + +github_release_exists() { + local tag_name=$1 + local output status + set +e + output=$(gh api --silent "repos/$repository/releases/tags/$tag_name" 2>&1) + status=$? + set -e + if [ "$status" -eq 0 ]; then + echo "true" + elif [[ "$output" == *"(HTTP 404)"* ]]; then + echo "false" + else + echo "Could not verify GitHub release state for $tag_name: $output" >&2 exit 1 fi } @@ -105,8 +129,6 @@ if [ "$resume" = true ]; then node scripts/validate-release-resume.mjs "${release_files[@]}" release_sha=$(git rev-parse HEAD) tag="v$after" - assert_remote_tag_absent "$tag" - echo "Resuming release $tag from existing bump commit $release_sha." else before=$(node -p "require('./package.json').version") node scripts/version.mjs "$bump" @@ -148,6 +170,21 @@ else fi repository=$(gh repo view --json nameWithOwner --jq .nameWithOwner) +local_tag_sha="" +if git show-ref --verify --quiet "refs/tags/$tag"; then + local_tag_sha=$(git rev-parse "refs/tags/$tag^{}") +fi +remote_tag_sha=$(read_remote_tag_sha "$tag") +release_exists=$(github_release_exists "$tag") +tag_state=$(node scripts/validate-release-resume.mjs --tag-state \ + "$release_sha" "${local_tag_sha:--}" "${remote_tag_sha:--}" "$release_exists") +if [ "$resume" = true ]; then + echo "Resuming release $tag from existing bump commit $release_sha ($tag_state tag state)." +elif [ "$tag_state" != "untagged" ]; then + echo "New release unexpectedly found an existing tag state for $tag." >&2 + exit 1 +fi + if ! GH_REPOSITORY="$repository" node scripts/require-release-candidate-check.mjs \ "$release_sha" \ "Release Candidate Built-Dist" \ @@ -158,19 +195,35 @@ if ! GH_REPOSITORY="$repository" node scripts/require-release-candidate-check.mj exit 1 fi -# Revalidate the branch and tag after the potentially long polling window. A -# later main push must never cause this invocation to publish a stale target. +# Revalidate after the potentially long polling window, then close the remaining +# race by atomically asserting main's exact SHA while pushing the annotated tag. git fetch origin main if [ "$(git rev-parse HEAD)" != "$release_sha" ] || [ "$(git rev-parse origin/main)" != "$release_sha" ]; then echo "origin/main moved while release gates were running; refusing to release $release_sha." >&2 exit 1 fi -assert_remote_tag_absent "$tag" +if [ "$tag_state" = "untagged" ]; then + if ! node scripts/atomic-release-tag.mjs "$release_sha" "$tag"; then + echo "No GitHub release was created. Resolve main/tag state, then run:" >&2 + echo " ./scripts/cut-release.sh --resume" >&2 + exit 1 + fi +else + echo "Verified annotated tag $tag was already pushed; skipping tag creation." +fi + +# Whether this is the first attempt or a post-tag resume, require both tag refs +# to resolve to the release commit and require that no GitHub release exists. +local_tag_sha=$(git rev-parse "refs/tags/$tag^{}") +remote_tag_sha=$(read_remote_tag_sha "$tag") +release_exists=$(github_release_exists "$tag") +node scripts/validate-release-resume.mjs --tag-state \ + "$release_sha" "$local_tag_sha" "$remote_tag_sha" "$release_exists" >/dev/null -echo "Creating GitHub release $tag (this also creates the git tag)…" +echo "Creating GitHub release $tag from pre-existing verified tag…" gh release create "$tag" \ + --verify-tag \ --title "$tag" \ - --target "$release_sha" \ --generate-notes echo diff --git a/scripts/validate-release-resume.mjs b/scripts/validate-release-resume.mjs index 8d0b40b6a..5f98935c2 100644 --- a/scripts/validate-release-resume.mjs +++ b/scripts/validate-release-resume.mjs @@ -5,13 +5,26 @@ import { readFileSync } from "node:fs" import { fileURLToPath } from "node:url" import { resolve } from "node:path" +export function validateReleaseTagState({ releaseSha, localTagSha, remoteTagSha, releaseExists }) { + if (releaseExists) throw new Error("GitHub release already exists for this tag") + if (!localTagSha && !remoteTagSha) return "untagged" + if (!localTagSha || !remoteTagSha) { + throw new Error("release tag must either be absent locally and remotely or present in both places") + } + if (localTagSha !== releaseSha || remoteTagSha !== releaseSha) { + throw new Error( + `release tag target mismatch: expected ${releaseSha}, local=${localTagSha}, remote=${remoteTagSha}`, + ) + } + return "tagged" +} + export function validateReleaseResumeState({ version, parentVersion, commitSubject, changedFiles, allowedFiles, - tagsAtHead, }) { const errors = [] if (!/^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/.test(version)) { @@ -29,7 +42,6 @@ export function validateReleaseResumeState({ if (!allowed.has(file)) errors.push(`release bump commit changed unexpected file: ${file}`) } } - if (tagsAtHead.length > 0) errors.push(`current release bump is already tagged: ${tagsAtHead.join(", ")}`) if (errors.length > 0) throw new Error(errors.join("\n")) } @@ -42,20 +54,31 @@ function packageVersion(contents) { } function main() { + if (process.argv[2] === "--tag-state") { + const [, releaseSha, localArg, remoteArg, releaseExistsArg] = process.argv.slice(2) + if (!/^[0-9a-f]{40}$/i.test(releaseSha ?? "")) throw new Error("invalid release SHA for tag-state validation") + const state = validateReleaseTagState({ + releaseSha, + localTagSha: localArg === "-" ? null : localArg, + remoteTagSha: remoteArg === "-" ? null : remoteArg, + releaseExists: releaseExistsArg === "true", + }) + console.log(state) + return + } + const allowedFiles = process.argv.slice(2) if (allowedFiles.length === 0) throw new Error("validate-release-resume requires allowed release file paths") const version = packageVersion(readFileSync("package.json", "utf8")) const parentVersion = packageVersion(git("show", "HEAD^:package.json")) const commitSubject = git("log", "-1", "--pretty=%s") const changedFiles = git("diff-tree", "--no-commit-id", "--name-only", "-r", "HEAD").split("\n").filter(Boolean) - const tagsAtHead = git("tag", "--points-at", "HEAD").split("\n").filter(Boolean) validateReleaseResumeState({ version, parentVersion, commitSubject, changedFiles, allowedFiles, - tagsAtHead, }) console.log(`Release resume state is valid for untagged v${version} bump at ${git("rev-parse", "HEAD")}.`) } diff --git a/scripts/validate-release-resume.test.mjs b/scripts/validate-release-resume.test.mjs index a5b542421..3bf47eb2e 100644 --- a/scripts/validate-release-resume.test.mjs +++ b/scripts/validate-release-resume.test.mjs @@ -1,6 +1,9 @@ import assert from "node:assert/strict" import { describe, test } from "node:test" -import { validateReleaseResumeState } from "./validate-release-resume.mjs" +import { + validateReleaseResumeState, + validateReleaseTagState, +} from "./validate-release-resume.mjs" const valid = { version: "0.1.94", @@ -8,9 +11,38 @@ const valid = { commitSubject: "chore(release): bump packages to 0.1.94", changedFiles: ["package.json", "packages/agent/package.json"], allowedFiles: ["package.json", "packages/agent/package.json", "pnpm-lock.yaml"], - tagsAtHead: [], } +describe("post-tag release resume validation", () => { + const releaseSha = "a".repeat(40) + + test("accepts either a fully untagged state or matching local and remote tags", () => { + assert.equal(validateReleaseTagState({ + releaseSha, + localTagSha: null, + remoteTagSha: null, + releaseExists: false, + }), "untagged") + assert.equal(validateReleaseTagState({ + releaseSha, + localTagSha: releaseSha, + remoteTagSha: releaseSha, + releaseExists: false, + }), "tagged") + }) + + test("rejects one-sided tags, mismatched targets, or an existing GitHub release", () => { + const invalid = [ + { localTagSha: releaseSha, remoteTagSha: null, releaseExists: false }, + { localTagSha: releaseSha, remoteTagSha: "b".repeat(40), releaseExists: false }, + { localTagSha: releaseSha, remoteTagSha: releaseSha, releaseExists: true }, + ] + for (const state of invalid) { + assert.throws(() => validateReleaseTagState({ releaseSha, ...state })) + } + }) +}) + describe("release resume validation", () => { test("accepts a cleanly shaped untagged version bump commit", () => { assert.doesNotThrow(() => validateReleaseResumeState(valid)) @@ -30,11 +62,4 @@ describe("release resume validation", () => { changedFiles: [...valid.changedFiles, "src/product.ts"], }), /unexpected file: src\/product.ts/) }) - - test("rejects an already tagged bump", () => { - assert.throws(() => validateReleaseResumeState({ - ...valid, - tagsAtHead: ["v0.1.94"], - }), /already tagged/) - }) }) From d5db6679426e95093616ecb65bc01485d882e670 Mon Sep 17 00:00:00 2001 From: hachej Date: Fri, 31 Jul 2026 13:43:36 +0000 Subject: [PATCH 4/4] #391 fix(release): compare-and-delete local candidate tag --- scripts/atomic-release-tag.mjs | 17 +++++++---- scripts/atomic-release-tag.test.mjs | 45 ++++++++++++++++++++++++++--- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/scripts/atomic-release-tag.mjs b/scripts/atomic-release-tag.mjs index d20d368e8..6b6e5667a 100644 --- a/scripts/atomic-release-tag.mjs +++ b/scripts/atomic-release-tag.mjs @@ -34,6 +34,12 @@ export function pushAnnotatedReleaseTagAtomically({ releaseSha, tag, runGit = de throw new Error(`Could not create local annotated release tag ${tag}: ${created.stderr.trim()}`) } + const localTagObject = runGit(["rev-parse", `refs/tags/${tag}`]) + const createdTagObjectSha = localTagObject.status === 0 ? firstSha(localTagObject.stdout) : null + if (!createdTagObjectSha) { + throw new Error(`Could not capture the annotated tag object for ${tag}; keeping the local tag for investigation.`) + } + const pushed = runGit(atomicReleaseRefspecs(releaseSha, tag)) if (pushed.status === 0) return @@ -47,13 +53,12 @@ export function pushAnnotatedReleaseTagAtomically({ releaseSha, tag, runGit = de ]) const remoteMain = runGit(["ls-remote", "--exit-code", "--heads", "origin", "refs/heads/main"]) - // Delete only the local tag created by this invocation when the remote can - // authoritatively prove the atomic push created no tag. + // Compare-and-delete only the exact tag object created by this invocation + // when the remote authoritatively proves the atomic push created no tag. + // A concurrent same-target replacement has a different tag object SHA and + // is therefore retained for investigation. if (remoteTag.status === 2) { - const localTag = runGit(["rev-parse", `refs/tags/${tag}^{}`]) - if (localTag.status === 0 && firstSha(localTag.stdout) === releaseSha) { - runGit(["tag", "-d", tag]) - } + runGit(["update-ref", "-d", `refs/tags/${tag}`, createdTagObjectSha]) } const remoteMainSha = remoteMain.status === 0 ? firstSha(remoteMain.stdout) : null diff --git a/scripts/atomic-release-tag.test.mjs b/scripts/atomic-release-tag.test.mjs index 0898293c2..912ebed86 100644 --- a/scripts/atomic-release-tag.test.mjs +++ b/scripts/atomic-release-tag.test.mjs @@ -7,6 +7,7 @@ import { const releaseSha = "a".repeat(40) const advancedSha = "b".repeat(40) +const createdTagObjectSha = "c".repeat(40) const tag = "v0.1.94" function fixtureRunner(fixtures, calls) { @@ -32,32 +33,68 @@ describe("atomic release tag push", () => { pushAnnotatedReleaseTagAtomically({ releaseSha, tag, - runGit: fixtureRunner([{ status: 0 }, { status: 0 }], calls), + runGit: fixtureRunner([ + { status: 0 }, + { status: 0, stdout: `${createdTagObjectSha}\n` }, + { status: 0 }, + ], calls), }) assert.deepEqual(calls[0], ["tag", "-a", tag, releaseSha, "-m", tag]) + assert.deepEqual(calls[1], ["rev-parse", `refs/tags/${tag}`]) }) test("fails when main advanced and safely removes an unpushed local tag", () => { const calls = [] const runGit = fixtureRunner([ { status: 0 }, + { status: 0, stdout: `${createdTagObjectSha}\n` }, { status: 1, stderr: "atomic push rejected" }, { status: 2 }, { status: 0, stdout: `${advancedSha}\trefs/heads/main\n` }, - { status: 0, stdout: `${releaseSha}\n` }, { status: 0 }, ], calls) assert.throws( () => pushAnnotatedReleaseTagAtomically({ releaseSha, tag, runGit }), /origin\/main advanced/, ) - assert.deepEqual(calls.at(-1), ["tag", "-d", tag]) + assert.deepEqual(calls.at(-1), [ + "update-ref", + "-d", + `refs/tags/${tag}`, + createdTagObjectSha, + ]) + }) + + test("retains a concurrent same-target replacement through compare-and-delete", () => { + const calls = [] + const runGit = fixtureRunner([ + { status: 0 }, + { status: 0, stdout: `${createdTagObjectSha}\n` }, + { status: 1, stderr: "atomic push rejected" }, + { status: 2 }, + { status: 0, stdout: `${advancedSha}\trefs/heads/main\n` }, + // Simulates update-ref rejecting deletion because another tag object now + // occupies the ref, even though it peels to the same release commit. + { status: 1, stderr: "cannot lock ref: is at replacement object" }, + ], calls) + assert.throws( + () => pushAnnotatedReleaseTagAtomically({ releaseSha, tag, runGit }), + /origin\/main advanced/, + ) + assert.deepEqual(calls.at(-1), [ + "update-ref", + "-d", + `refs/tags/${tag}`, + createdTagObjectSha, + ]) + assert.equal(calls.some((args) => args[0] === "tag" && args[1] === "-d"), false) }) test("keeps the local tag when remote tag state cannot be proven absent", () => { const calls = [] const runGit = fixtureRunner([ { status: 0 }, + { status: 0, stdout: `${createdTagObjectSha}\n` }, { status: 1, stderr: "connection lost" }, { status: 1, stderr: "connection lost" }, { status: 1, stderr: "connection lost" }, @@ -66,6 +103,6 @@ describe("atomic release tag push", () => { () => pushAnnotatedReleaseTagAtomically({ releaseSha, tag, runGit }), /Atomic release tag push failed/, ) - assert.equal(calls.some((args) => args[0] === "tag" && args[1] === "-d"), false) + assert.equal(calls.some((args) => args[0] === "update-ref" && args[1] === "-d"), false) }) })