From 9c0d7ee8189f596632b269f6a5818e140e3954cc Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:56:53 -0500 Subject: [PATCH 1/2] fix(release): let two prunes of the same build tags both succeed Two post-green runs can name the same build tags for deletion. When the rival's deletion lands after this run's delete push has read origin's refs, the server refuses the deletion of a ref that is already gone, git push exits non-zero, and the job failed with its build tag minted and latest not moved. A refused delete push is now judged by the prune's postcondition: origin's build tags are read back, and the prune is done when none of the tags it named is left. A refusal that leaves a named tag on origin is thrown as before, in git's words. A push git accepts is trusted without a read-back. The fixture gains origin-side hooks, which play a rival landing inside a push, after it read origin's refs and before origin writes any: the gap a push plan cannot reach. One test has the rival take every named tag there and expects the prune to succeed; another has it take one while origin declines the other and expects the refusal. --- .github/scripts/release-pipeline/git.ts | 5 ++- .github/scripts/release-pipeline/tags.ts | 37 ++++++++++++----- test/scripts/release-pipeline-build.test.ts | 45 +++++++++++++++++++++ test/scripts/release-pipeline-fixture.ts | 19 ++++++++- 4 files changed, 93 insertions(+), 13 deletions(-) diff --git a/.github/scripts/release-pipeline/git.ts b/.github/scripts/release-pipeline/git.ts index 9e50f330..5d0a1334 100644 --- a/.github/scripts/release-pipeline/git.ts +++ b/.github/scripts/release-pipeline/git.ts @@ -63,8 +63,9 @@ export function tryGit(cwd: string, ...args: string[]): string | null { } } -/** A push as git ran it. A refusal is never read from git's words: the caller re-reads origin, and a ref that - * moved since it was observed is a lost compare-and-set to retry, an unmoved one a refusal to throw. */ +/** A push as git ran it. A refusal is never read from git's words: the caller re-reads origin. A ref that moved + * since it was observed is a lost compare-and-set to retry, an unmoved one a refusal to throw; a refused deletion + * whose refs are all gone is done. */ export function push(cwd: string, ...pushArgs: string[]): Error | null { try { execFileSync("git", ["push", ...pushArgs], { cwd, encoding: "utf8" }); diff --git a/.github/scripts/release-pipeline/tags.ts b/.github/scripts/release-pipeline/tags.ts index 68261489..bd4a52c0 100644 --- a/.github/scripts/release-pipeline/tags.ts +++ b/.github/scripts/release-pipeline/tags.ts @@ -1,7 +1,8 @@ /** * The packaged commits and the refs that name them: a main commit's package under its build tag, the version tag * minted once on the release's package, and the pointers (latest, vX) that move forward along main alone. Every - * push is a create-once or a lease, judged by re-reading origin, never by git's words. + * push is a create-once, a lease, or the prune's deletion, and a refused one is judged by re-reading origin, never + * by git's words. */ import { execFileSync } from "node:child_process"; @@ -310,14 +311,24 @@ function ensurePackaged(cwd: string, sourceSha: string, tree: string, runUrl?: s ); } -/** The build tags beyond the `keep` newest by position, deleted in one push. Two runs pruning at once commute: the - * server answers the deletion of a ref a rival deleted first with a warning, not a refusal. A ref under build/ this - * pipeline would not name stops the prune: sorted in, it could push a genuine tag out of the window. */ -export function pruneBuildTags(cwd: string, keep = KEPT_BUILD_TAGS): string[] { - const refs = git(cwd, "ls-remote", "origin", `${BUILD_TAG_PREFIX}*`) +/** Every ref origin holds under build/, by name. */ +function remoteBuildRefs(cwd: string): string[] { + return git(cwd, "ls-remote", "origin", `${BUILD_TAG_PREFIX}*`) .split("\n") .map((line) => line.split("\t")[1] ?? "") - .filter((ref) => ref !== "" && !ref.endsWith("^{}")) + .filter((ref) => ref !== "" && !ref.endsWith("^{}")); +} + +/** + * The build tags beyond the `keep` newest by position, deleted in one push. A ref under build/ this pipeline would + * not name stops the prune: sorted in, it could push a genuine tag out of the window. + * + * Two runs pruning at once commute. The server answers a deletion a rival made before this push with a warning, and + * one that lands after the push read origin's refs with a refusal, so a refused push counts as done when origin + * holds none of the tags it named. + */ +export function pruneBuildTags(cwd: string, keep = KEPT_BUILD_TAGS): string[] { + const refs = remoteBuildRefs(cwd) .map((ref) => { const position = Number(ref.match(BUILD_TAG)?.[1]); if (Number.isNaN(position)) { @@ -331,7 +342,13 @@ export function pruneBuildTags(cwd: string, keep = KEPT_BUILD_TAGS): string[] { .slice(keep) .map((tag) => tag.ref); if (refs.length > 0) { - git(cwd, "push", "origin", ...refs.map((ref) => `:${ref}`)); + const refused = push(cwd, "origin", ...refs.map((ref) => `:${ref}`)); + if (refused !== null) { + const held = new Set(remoteBuildRefs(cwd)); + if (refs.some((ref) => held.has(ref))) { + throw refused; + } + } } return refs; } @@ -443,7 +460,7 @@ interface PackageCommitOptions { } interface PackageCommitResult extends EnsuredTag { - /** The build tags this run deleted, beyond the kept window. */ + /** The build tags beyond the kept window: deleted by this run, or by a rival pruning at once. */ pruned: string[]; /** latest's move: forward to this package, or left where it is. */ @@ -486,7 +503,7 @@ interface PackagedRelease { /** The packaged commit the version tag names. */ packagedSha: string; - /** The build tags this run deleted, beyond the kept window. */ + /** The build tags beyond the kept window: deleted by this run, or by a rival pruning at once. */ pruned: string[]; /** latest's move: forward to this package, or left where it is. */ diff --git a/test/scripts/release-pipeline-build.test.ts b/test/scripts/release-pipeline-build.test.ts index c99e4756..f8636fbd 100644 --- a/test/scripts/release-pipeline-build.test.ts +++ b/test/scripts/release-pipeline-build.test.ts @@ -36,6 +36,7 @@ import { manifestJson, moveOf, originHolds, + originHooks, PERMANENT, PLANTED_PACKAGES, packagedOf, @@ -871,6 +872,50 @@ describe("pruneBuildTags", () => { expect(buildTags(fx).sort()).toEqual(refs.slice(2).sort()); }); + test("a rival deleting every candidate inside the delete push, after the push read origin and before origin locks the refs, gets the push refused, and the prune succeeds on its postcondition: the tags it named are gone", () => { + const fx = seedFixture(); + const { refs, first, second } = plantWindow(fx); + // The marker proves the rival ran inside the push; the next test pins that git refuses a ref it lost that way. + const marker = join(fx.root, "rival-deleted-inside"); + originHooks(fx, { + "pre-receive": `git update-ref -d ${first} && git update-ref -d ${second} && : > "${marker}"\n`, + }); + let deleted: string[] | undefined; + const pushes = withPushPlans(fx, [], () => { + deleted = pruneBuildTags(fx.work); + }); + expect(existsSync(marker)).toBe(true); + expect(deleted).toEqual([second, first]); + expect(pushes).toEqual([deleteOf(second, first)]); + expect(buildTags(fx).sort()).toEqual(refs.slice(2).sort()); + }); + + test("a refused delete push that leaves one candidate on origin fails with git's own words, though a rival took the other inside it: the postcondition is every tag gone, not some", () => { + const fx = seedFixture(); + const { refs, first, second } = plantWindow(fx); + originHooks(fx, { + "pre-receive": `git update-ref -d ${second}\n`, + update: `test "$1" != ${first}\n`, + }); + let error: unknown; + const pushes = withPushPlans(fx, [], () => { + try { + pruneBuildTags(fx.work); + } catch (thrown) { + error = thrown; + } + }); + expect(pushes).toEqual([deleteOf(second, first)]); + const message = error instanceof Error ? error.message : ""; + expect(message).toStartWith(`git push origin :${second} :${first} failed: `); + // git's reason for the ref the rival took has changed across versions; that it rejected the ref has not. + expect(message).toContain(` ! [remote rejected] ${second.replace("refs/tags/", "")} (`); + expect(message).toContain( + ` ! [remote rejected] ${first.replace("refs/tags/", "")} (hook declined)`, + ); + expect(buildTags(fx).sort()).toEqual(refs.filter((ref) => ref !== second).sort()); + }); + test("a ref under build/ this pipeline would not name stops the prune", () => { const fx = seedFixture(); plantWindow(fx, 3); diff --git a/test/scripts/release-pipeline-fixture.ts b/test/scripts/release-pipeline-fixture.ts index eb34b3d1..054506ce 100644 --- a/test/scripts/release-pipeline-fixture.ts +++ b/test/scripts/release-pipeline-fixture.ts @@ -1,7 +1,8 @@ /** * The fixture harness behind the release pipeline tests: a bare origin plus clones playing the CI checkouts, a git * shim first on PATH that refuses a push outside the fixture area and plays scripted remotes (rivals, refusals, - * a landing between a read and the write it informs), and the helpers the test files share. + * a landing between a read and the write it informs), origin's hooks for what lands inside a push, and the helpers + * the test files share. */ import { afterAll, beforeAll, expect } from "bun:test"; @@ -658,6 +659,22 @@ export function withPushPlans( .map((line) => line.split(" ")); } +/** + * What origin runs inside every later push, after the push read origin's refs and before origin writes any: the gap + * a push plan, which runs before the push starts, cannot reach. `pre-receive` plays a rival landing there, and + * `update` declines the ref in $1 by exiting non-zero. + * + * Origin's own hooksPath, so a developer's global one cannot answer instead. + */ +export function originHooks(fx: Fixture, hooks: { "pre-receive"?: string; update?: string }): void { + const dir = join(fx.root, "origin-hooks"); + mkdirSync(dir); + for (const [name, script] of Object.entries(hooks)) { + writeFileSync(join(dir, name), `#!/bin/sh\n${script}`, { mode: 0o755 }); + } + git(fx.origin, "config", "core.hooksPath", dir); +} + /** The moving pointer's ref. */ export const LATEST = "refs/tags/latest"; From f6d3121a7f6e795863fffb1e13d3db76fd264968 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Wed, 11 Nov 2026 21:37:59 -0500 Subject: [PATCH 2/2] test(release): seal the release fixture's git from the caller's environment Every git the suites run, the fixture's own calls and the pipeline under test, inherits the process environment. With GIT_DIR or GIT_CONFIG in it (git exports GIT_DIR to a hook), a fixture command such as the origin's config write landed in the file or repository those variables name: the repository the tests were launched from. The fixture now takes every GIT_* variable out of the environment where it already puts its shim on PATH, points global and system config at /dev/null, and puts the caller's variables back when the file's tests end. Identity needs nothing new: each clone carries the fixture's in its own config, which an exported author or committer would outrank. One test runs the fixture with a decoy GIT_DIR and GIT_CONFIG in the environment. Both decoys stay byte-identical, origin holds its own config, the pipeline's tag lands on origin, and the caller's variables are back once the fixture ended. --- test/scripts/release-pipeline-fixture.test.ts | 111 ++++++++++++++++++ test/scripts/release-pipeline-fixture.ts | 33 +++++- 2 files changed, 141 insertions(+), 3 deletions(-) create mode 100644 test/scripts/release-pipeline-fixture.test.ts diff --git a/test/scripts/release-pipeline-fixture.test.ts b/test/scripts/release-pipeline-fixture.test.ts new file mode 100644 index 00000000..2bea2e71 --- /dev/null +++ b/test/scripts/release-pipeline-fixture.test.ts @@ -0,0 +1,111 @@ +import { afterAll, beforeAll, expect, setDefaultTimeout, test } from "bun:test"; +import { + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { packageCommit } from "../../.github/scripts/release-pipeline/tags.js"; +import { + buildTagOf, + buildTags, + type Fixture, + installReleasePipelineFixture, + originHooks, + seedFixture, +} from "./release-pipeline-fixture.js"; + +setDefaultTimeout(60_000); + +/** Two variables a launcher can leave in the environment (git exports GIT_DIR to a hook); each redirects every git + * run below it. */ +const LAUNCHER = ["GIT_CONFIG", "GIT_DIR"] as const; + +/** What the suite itself was launched with, put back when the file ends. */ +const launchedWith = new Map(); + +/** The launcher's side: a config file and a repository, hand-written so no git runs before the fixture's seal. */ +let launcher = ""; +const launcherConfig = (): string => join(launcher, "launcher.gitconfig"); +const launcherRepo = (): string => join(launcher, "launched-from.git"); + +/** The launcher's side as it was written. */ +let written: Record = {}; + +/** Every path under `dir` with its bytes, a directory as null. */ +function snapshot(dir: string): Record { + return Object.fromEntries( + readdirSync(dir, { recursive: true, encoding: "utf8" }) + .sort() + .map((entry) => [ + entry, + statSync(join(dir, entry)).isDirectory() ? null : readFileSync(join(dir, entry), "latin1"), + ]), + ); +} + +// Registered ahead of the fixture's own hooks, so the fixture starts in an environment that already redirects git. +beforeAll(() => { + launcher = mkdtempSync(join(tmpdir(), "release-pipeline-launcher-")); + writeFileSync(launcherConfig(), "[launcher]\n\tkept = true\n"); + mkdirSync(join(launcherRepo(), "objects"), { recursive: true }); + mkdirSync(join(launcherRepo(), "refs")); + writeFileSync(join(launcherRepo(), "HEAD"), "ref: refs/heads/main\n"); + writeFileSync( + join(launcherRepo(), "config"), + "[core]\n\trepositoryformatversion = 0\n\tbare = true\n", + ); + written = snapshot(launcher); + for (const name of LAUNCHER) { + launchedWith.set(name, process.env[name]); + } + process.env.GIT_CONFIG = launcherConfig(); + process.env.GIT_DIR = launcherRepo(); +}); + +installReleasePipelineFixture(); + +// Registered after the fixture's hooks, so it runs once the fixture ended: the launcher's variables are back. +afterAll(() => { + try { + expect({ config: process.env.GIT_CONFIG, dir: process.env.GIT_DIR }).toEqual({ + config: launcherConfig(), + dir: launcherRepo(), + }); + } finally { + for (const [name, value] of launchedWith) { + if (value === undefined) { + delete process.env[name]; + } else { + process.env[name] = value; + } + } + rmSync(launcher, { recursive: true, force: true }); + } +}); + +test("a fixture git command never writes into the repository the tests were launched from: with the launcher's GIT_DIR and GIT_CONFIG in the environment, both stay byte-identical while origin takes its own config and the pipeline's tag", () => { + let fx: Fixture | undefined; + let failure: unknown; + try { + fx = seedFixture(); + originHooks(fx, { "pre-receive": "exit 0\n" }); + packageCommit({ cwd: fx.work, sourceSha: fx.mergeSha }); + } catch (thrown) { + failure = thrown; + } + expect(snapshot(launcher)).toEqual(written); + expect(failure).toBeUndefined(); + if (fx === undefined) { + return; + } + const originConfig = readFileSync(join(fx.origin, "config"), "utf8"); + expect(originConfig).toContain("[maintenance]\n\tauto = false\n"); + expect(originConfig).toContain(`\thooksPath = ${join(fx.root, "origin-hooks")}\n`); + expect(buildTags(fx)).toEqual([buildTagOf(fx, fx.mergeSha)]); +}); diff --git a/test/scripts/release-pipeline-fixture.ts b/test/scripts/release-pipeline-fixture.ts index 054506ce..d3152008 100644 --- a/test/scripts/release-pipeline-fixture.ts +++ b/test/scripts/release-pipeline-fixture.ts @@ -17,7 +17,7 @@ import { symlinkSync, writeFileSync, } from "node:fs"; -import { tmpdir } from "node:os"; +import { devNull, tmpdir } from "node:os"; import { dirname, join } from "node:path"; export function git(cwd: string, ...args: string[]): string { @@ -47,8 +47,22 @@ let realPath: string | undefined; /** The real git, which the shim execs. */ let realGit = ""; -/** The per-file hooks: the shim goes first on PATH before the file's tests and comes off after them, and the - * fixture roots the file created are removed; a test file calls this once at module scope. */ +/** The caller's GIT_* variables, out of the environment while a file's tests run; undefined until they were. */ +let callerGit: Record | undefined; + +/** What stands in for them: no global and no system config, so a git the suite runs reads its own repository's + * config alone. Identity stays in each clone's config, which GIT_AUTHOR_* and GIT_COMMITTER_* would outrank. */ +const SEALED_GIT = { GIT_CONFIG_GLOBAL: devNull, GIT_CONFIG_SYSTEM: devNull }; + +/** Every GIT_* variable the environment holds now. */ +const gitVariables = (): [string, string][] => + Object.entries(process.env).filter( + (entry): entry is [string, string] => entry[0].startsWith("GIT_") && entry[1] !== undefined, + ); + +/** The per-file hooks: before the file's tests the shim goes first on PATH and the caller's GIT_* variables leave + * the environment, after them both are put back, and the fixture roots the file created are removed; a test file + * calls this once at module scope. */ export function installReleasePipelineFixture(): void { afterAll(() => { for (const root of roots.splice(0)) { @@ -99,9 +113,22 @@ export function installReleasePipelineFixture(): void { writeFileSync(join(shimDir, "git"), script, { mode: 0o755 }); realPath = process.env.PATH ?? ""; process.env.PATH = `${shimDir}:${realPath}`; + // The fixture's git calls and the pipeline's both inherit this environment: a GIT_DIR or GIT_CONFIG left in it + // (git exports GIT_DIR to a hook) would send them into the repository the tests were launched from. + callerGit = Object.fromEntries(gitVariables()); + for (const name of Object.keys(callerGit)) { + delete process.env[name]; + } + Object.assign(process.env, SEALED_GIT); }); // Undo only what beforeAll got to, so a setup failure surfaces as itself. afterAll(() => { + if (callerGit !== undefined) { + for (const [name] of gitVariables()) { + delete process.env[name]; + } + Object.assign(process.env, callerGit); + } if (realPath !== undefined) { process.env.PATH = realPath; }