diff --git a/.github/scripts/closed-pr-branch-cleanup.cjs b/.github/scripts/closed-pr-branch-cleanup.cjs index 57142f19fe2..3f62bd50dfb 100644 --- a/.github/scripts/closed-pr-branch-cleanup.cjs +++ b/.github/scripts/closed-pr-branch-cleanup.cjs @@ -16,11 +16,17 @@ /** Branches that may never be deleted regardless of pull-request state. */ const PROTECTED_BRANCHES = Object.freeze(["main", "dev", "preview", "gh-pages"]); +/** Branch namespaces explicitly reserved for disposable pull-request work. */ +const DISPOSABLE_BRANCH_PREFIXES = Object.freeze(["codex/", "ingw/"]); + /** Default grace period before a closed PR's head branch becomes eligible. */ const DEFAULT_GRACE_DAYS = 14; function normalizeBranchName(value) { - return String(value || "").trim(); + // Git permits non-ASCII whitespace in ref names, while String#trim removes + // it. Preserve API-provided branch identity byte-for-byte so two distinct + // refs cannot collapse into one deletion candidate. + return typeof value === "string" ? value : ""; } /** @@ -59,6 +65,7 @@ const KEEP_REASONS = Object.freeze({ CROSS_REPOSITORY: "cross-repository-head", MISSING_CLOSED_AT: "missing-closed-at", WITHIN_GRACE: "within-grace-period", + OUTSIDE_DISPOSABLE_NAMESPACE: "outside-disposable-namespace", MOVED_SINCE_CLOSE: "branch-moved-since-close", UNKNOWN_HEAD_SHA: "unknown-head-sha", }); @@ -79,6 +86,11 @@ const KEEP_REASONS = Object.freeze({ * contributor's repository and this token has no business there. * - A grace period after `closed_at` leaves room to reopen a PR that was * closed by mistake. + * - Only branches under namespaces explicitly reserved for disposable pull- + * request work are eligible. Pull-request history alone must not authorize + * deletion of an unrelated persistent branch. + * - Branch names are compared and emitted byte-for-byte. Normalizing Unicode + * whitespace can merge distinct valid refs and delete the wrong branch. * - The branch must still POINT AT a commit one of those closed pull requests * had as its head. Matching by NAME alone deletes reused work: `codex/`-style * names get picked up again all the time, and a branch recreated for new work @@ -178,6 +190,11 @@ function planClosedPrBranchDeletions({ continue; } + if (!DISPOSABLE_BRANCH_PREFIXES.some((prefix) => branch.startsWith(prefix))) { + keeps.push({ branch, reason: KEEP_REASONS.OUTSIDE_DISPOSABLE_NAMESPACE }); + continue; + } + // The tip check, last because it is the most expensive claim to satisfy and // the cheaper rules above have already excluded most branches. // @@ -219,6 +236,7 @@ function planClosedPrBranchDeletions({ module.exports = { DEFAULT_GRACE_DAYS, + DISPOSABLE_BRANCH_PREFIXES, KEEP_REASONS, PROTECTED_BRANCHES, isProtectedBranch, diff --git a/.github/scripts/closed-pr-branch-cleanup.test.cjs b/.github/scripts/closed-pr-branch-cleanup.test.cjs index 644cb0b7aa0..1dac5c7ce68 100644 --- a/.github/scripts/closed-pr-branch-cleanup.test.cjs +++ b/.github/scripts/closed-pr-branch-cleanup.test.cjs @@ -4,6 +4,7 @@ const { describe, it } = require("node:test"); const assert = require("node:assert/strict"); const { DEFAULT_GRACE_DAYS, + DISPOSABLE_BRANCH_PREFIXES, KEEP_REASONS, isProtectedBranch, planClosedPrBranchDeletions, @@ -11,6 +12,9 @@ const { const NOW = Date.parse("2026-08-26T00:00:00Z"); const DAY = 24 * 60 * 60 * 1000; +const HEAD_OID = "a".repeat(40); +const OTHER_OID = "b".repeat(40); +const NBSP = "\u00a0"; const longAgo = new Date(NOW - 60 * DAY).toISOString(); function closedPr(overrides) { @@ -20,6 +24,7 @@ function closedPr(overrides) { merged: false, isCrossRepository: false, headRefName: "codex/example", + headRefOid: HEAD_OID, baseRefName: "dev", closedAt: longAgo, ...overrides, @@ -42,13 +47,17 @@ describe("isProtectedBranch", () => { } assert.equal(isProtectedBranch("codex/dev"), false); }); + + it("declares the disposable pull-request branch namespaces", () => { + assert.deepEqual(DISPOSABLE_BRANCH_PREFIXES, ["codex/", "ingw/"]); + }); }); describe("planClosedPrBranchDeletions", () => { it("deletes a branch whose only pull request closed unmerged past the grace period", () => { const result = planClosedPrBranchDeletions({ pullRequests: [closedPr({ number: 42, headRefName: "codex/stale" })], - branches: ["codex/stale", "dev"], + branches: [{ name: "codex/stale", oid: HEAD_OID }, "dev"], now: NOW, }); assert.deepEqual(deletedBranches(result), ["codex/stale"]); @@ -147,13 +156,58 @@ describe("planClosedPrBranchDeletions", () => { it("ignores branches that no pull request ever used", () => { const result = planClosedPrBranchDeletions({ pullRequests: [closedPr({ number: 80, headRefName: "codex/known" })], - branches: ["codex/known", "codex/never-a-pr"], + branches: [ + { name: "codex/known", oid: HEAD_OID }, + { name: "codex/never-a-pr", oid: HEAD_OID }, + ], now: NOW, }); assert.deepEqual(deletedBranches(result), ["codex/known"]); assert.equal(keepReason(result, "codex/never-a-pr"), null); }); + it("keeps a persistent branch even when a closed pull request still matches its tip", () => { + const branch = "release/maintenance"; + const result = planClosedPrBranchDeletions({ + pullRequests: [closedPr({ number: 85, headRefName: branch })], + branches: [{ name: branch, oid: HEAD_OID }], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), []); + assert.equal( + keepReason(result, branch), + KEEP_REASONS.OUTSIDE_DISPOSABLE_NAMESPACE, + ); + }); + + it("preserves Unicode whitespace so distinct valid refs never collapse", () => { + const disposable = `codex/live${NBSP}`; + const result = planClosedPrBranchDeletions({ + pullRequests: [closedPr({ number: 86, headRefName: disposable })], + branches: [ + { name: "codex/live", oid: OTHER_OID }, + { name: disposable, oid: HEAD_OID }, + ], + now: NOW, + }); + assert.deepEqual(result.deletions, [{ branch: disposable, pullRequests: [86] }]); + assert.equal(keepReason(result, "codex/live"), null); + }); + + it("does not trim leading Unicode whitespace into a disposable namespace", () => { + const branch = `${NBSP}codex/persistent`; + const result = planClosedPrBranchDeletions({ + pullRequests: [closedPr({ number: 87, headRefName: branch })], + branches: [{ name: branch, oid: HEAD_OID }], + now: NOW, + }); + assert.deepEqual(deletedBranches(result), []); + assert.equal( + keepReason(result, branch), + KEEP_REASONS.OUTSIDE_DISPOSABLE_NAMESPACE, + ); + }); + it("only plans deletions for branches that still exist", () => { const result = planClosedPrBranchDeletions({ pullRequests: [closedPr({ number: 90, headRefName: "codex/already-gone" })], diff --git a/tests/closed-pr-branch-cleanup.test.ts b/tests/closed-pr-branch-cleanup.test.ts index b18fdd7016a..4fb0f3261ad 100644 --- a/tests/closed-pr-branch-cleanup.test.ts +++ b/tests/closed-pr-branch-cleanup.test.ts @@ -75,6 +75,55 @@ describe("closed-PR branch cleanup planning", () => { expect(result.deletions).toEqual([{ branch: "codex/some-work", pullRequests: [42] }]); }); + test("an abandoned ingw branch still at the closed PR tip is deleted", () => { + const result = plan( + [closedPr({ headRefName: "ingw/some-work" })], + [{ name: "ingw/some-work", oid: OLD_TIP }], + ); + expect(result.deletions).toEqual([{ branch: "ingw/some-work", pullRequests: [42] }]); + }); + + test("a persistent branch outside disposable namespaces is kept", () => { + const branch = "release/maintenance"; + const result = plan( + [closedPr({ headRefName: branch })], + [{ name: branch, oid: OLD_TIP }], + ); + expect(result.deletions).toEqual([]); + expect(keepReason(result, branch)).toBe(KEEP_REASONS.OUTSIDE_DISPOSABLE_NAMESPACE); + }); + + test("near-miss prefixes do not enter disposable namespaces", () => { + for (const branch of ["codexx/some-work", "ingw2/some-work"]) { + const result = plan( + [closedPr({ headRefName: branch })], + [{ name: branch, oid: OLD_TIP }], + ); + expect(result.deletions).toEqual([]); + expect(keepReason(result, branch)).toBe(KEEP_REASONS.OUTSIDE_DISPOSABLE_NAMESPACE); + } + }); + + test("Unicode whitespace preserves ref identity and cannot create a namespace match", () => { + const trailing = "codex/some-work\u00a0"; + const collision = plan( + [closedPr({ headRefName: trailing })], + [ + { name: "codex/some-work", oid: NEW_TIP }, + { name: trailing, oid: OLD_TIP }, + ], + ); + expect(collision.deletions).toEqual([{ branch: trailing, pullRequests: [42] }]); + + const leading = "\u00a0codex/some-work"; + const outside = plan( + [closedPr({ headRefName: leading })], + [{ name: leading, oid: OLD_TIP }], + ); + expect(outside.deletions).toEqual([]); + expect(keepReason(outside, leading)).toBe(KEEP_REASONS.OUTSIDE_DISPOSABLE_NAMESPACE); + }); + test("BUG-R4: a branch reused for new work is kept, not deleted", () => { // Same NAME, different tip. Before the SHA guard this returned a deletion // for a branch carrying commits that had never been in any pull request.