feat(scripts): fail a branch that deletes files main still ships (#3150)

* feat(scripts): fail a branch that deletes files main still ships

Written after a scare that turned out to be a measurement error, and the error is the reason it exists. Comparing tip to tip on a branch a month behind reports every file main has added since the merge base as a deletion: 1,284 of them, an entire skills tree among them, none of it real. A merge keeps mains side and a pull request shows the three-dot diff, which reported zero.

So the gate uses the three-dot form and reports renames separately, because in a name-only diff a rename is indistinguishable from a deletion and treating them alike would either mask real loss or block every legitimate move.

* ci: enforce the no-deletions guard

.github/workflows/ci.yml

@@ -44,6 +44,9 @@ jobs:
       - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
         with:
           fetch-depth: 0
+      - name: Reject accidental file deletions
+        if: github.event_name == 'pull_request'
+        run: node scripts/check-no-main-deletions.mjs --base origin/main
       - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4
         id: filter
         with:

package.json

@@ -48,7 +48,7 @@
     "player:perf": "bun run --filter @hyperframes/player perf",
     "format:check": "oxfmt --check .",
     "knip": "knip",
-    "test:scripts": "node --import tsx --test scripts/check-tracked-artifacts.test.mjs scripts/check-docs-snippet-motion.test.mjs scripts/registry-target-paths.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-package-cycles.test.mjs scripts/check-cli-process-ownership.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/publish-workflow.test.mjs scripts/install-workspace-dependencies.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs scripts/lint-skills.test.mjs packages/gcp-cloud-run/check-dockerfile-workspaces.test.mjs",
+    "test:scripts": "node --import tsx --test scripts/check-tracked-artifacts.test.mjs scripts/check-no-main-deletions.test.mjs scripts/check-docs-snippet-motion.test.mjs scripts/registry-target-paths.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-package-cycles.test.mjs scripts/check-cli-process-ownership.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/publish-workflow.test.mjs scripts/install-workspace-dependencies.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs scripts/lint-skills.test.mjs packages/gcp-cloud-run/check-dockerfile-workspaces.test.mjs",
     "typecheck:scripts": "tsc --noEmit -p scripts/tsconfig.json",
     "test:skills": "node --test 'skills/**/*.test.mjs'",
     "generate:previews": "tsx scripts/generate-template-previews.ts",

scripts/check-no-main-deletions.mjs

@@ -0,0 +1,93 @@
+#!/usr/bin/env node
+/**
+ * Fail if this branch would delete files that exist on the base.
+ *
+ * Written after a scare that turned out to be a measurement error, which is
+ * the reason it uses the three-dot form. Comparing tip to tip (two dots) on a
+ * branch that is a month behind reports every file main added since the merge
+ * base as a deletion: 1,284 of them, including an entire skills tree. None of
+ * that is real. A merge keeps main's side, and a pull request shows the
+ * three-dot diff, which reported zero.
+ *
+ * So this exists to catch deletions a branch genuinely proposes, and to make
+ * the distinction hard to get wrong again. If it ever disagrees with a manual
+ * git diff, check which form the manual one used before believing it.
+ *
+ * Renames are reported separately. In a name-only diff a rename is
+ * indistinguishable from a deletion, so treating them alike would either mask
+ * real loss or block every legitimate move.
+ *
+ *   node scripts/check-no-main-deletions.mjs [--base origin/main]
+ */
+
+import { execFileSync } from "node:child_process";
+
+const BASE_FLAG = "--base";
+
+export function parseBase(argv, fallback = "origin/main") {
+  const index = argv.indexOf(BASE_FLAG);
+  if (index === -1) return fallback;
+  const value = argv[index + 1];
+  if (!value || value.startsWith("-")) {
+    throw new Error(`${BASE_FLAG} needs a ref, for example ${BASE_FLAG} origin/main`);
+  }
+  return value;
+}
+
+/**
+ * Split a `--name-status` diff into deletions and renames.
+ *
+ * Git reports a rename as `R<score>\told\tnew`. Reading only the first column
+ * would file that under "deleted", which is the false alarm this guards
+ * against being noisy enough to ignore.
+ */
+// one branch per git status code
+// fallow-ignore-next-line complexity
+export function classify(nameStatus) {
+  const deleted = [];
+  const renamed = [];
+  for (const line of nameStatus.split("\n")) {
+    if (!line.trim()) continue;
+    const [status, ...paths] = line.split("\t");
+    if (status.startsWith("R")) renamed.push({ from: paths[0], to: paths[1] });
+    else if (status === "D") deleted.push(paths[0]);
+  }
+  return { deleted, renamed };
+}
+
+// a script entry point
+// fallow-ignore-next-line complexity
+function main() {
+  const base = parseBase(process.argv.slice(2));
+  let diff;
+  try {
+    diff = execFileSync("git", ["diff", "--name-status", "-M", `${base}...HEAD`], {
+      encoding: "utf8",
+    });
+  } catch (error) {
+    // An unreachable base is not a pass. Reporting "no deletions" because the
+    // ref was misspelled is the exact failure this exists to prevent.
+    console.error(`cannot diff against ${base}: ${error.message.trim()}`);
+    process.exit(2);
+  }
+
+  const { deleted, renamed } = classify(diff);
+  if (renamed.length > 0) {
+    console.log(`${renamed.length} renamed (allowed):`);
+    for (const { from, to } of renamed.slice(0, 10)) console.log(`  ${from} -> ${to}`);
+    if (renamed.length > 10) console.log(`  … and ${renamed.length - 10} more`);
+  }
+
+  if (deleted.length === 0) {
+    console.log(`No files from ${base} are deleted by this branch.`);
+    return;
+  }
+
+  console.error(`This branch deletes ${deleted.length} files that exist on ${base}:`);
+  for (const path of deleted.slice(0, 25)) console.error(`  ${path}`);
+  if (deleted.length > 25) console.error(`  … and ${deleted.length - 25} more`);
+  console.error("\nIf a deletion is intended, remove this check for that path deliberately.");
+  process.exit(1);
+}
+
+if (import.meta.url === `file://${process.argv[1]}`) main();

scripts/check-no-main-deletions.test.mjs

@@ -0,0 +1,42 @@
+import { strict as assert } from "node:assert";
+import { test } from "node:test";
+
+import { classify, parseBase } from "./check-no-main-deletions.mjs";
+
+test("a branch that only adds reports nothing", () => {
+  const { deleted, renamed } = classify("A\tpackages/cli/src/new.ts\nM\tpackages/cli/src/old.ts\n");
+  assert.deepEqual(deleted, []);
+  assert.deepEqual(renamed, []);
+});
+
+test("a deleted file is named", () => {
+  const { deleted } = classify("D\t.agents/skills/README.md\nA\tsrc/new.ts\n");
+  assert.deepEqual(deleted, [".agents/skills/README.md"]);
+});
+
+test("a rename is not reported as a deletion", () => {
+  // The false alarm worth avoiding: in a name-only diff a rename looks exactly
+  // like loss, and a check that cried wolf on every move would be turned off.
+  const { deleted, renamed } = classify("R096\tsrc/old/name.ts\tsrc/new/name.ts\n");
+  assert.deepEqual(deleted, []);
+  assert.deepEqual(renamed, [{ from: "src/old/name.ts", to: "src/new/name.ts" }]);
+});
+
+test("deletions and renames are separated in one diff", () => {
+  const { deleted, renamed } = classify(
+    "D\tdocs/gone.md\nR100\ta.ts\tb.ts\nM\tc.ts\nD\tdocs/also-gone.md\n",
+  );
+  assert.deepEqual(deleted, ["docs/gone.md", "docs/also-gone.md"]);
+  assert.equal(renamed.length, 1);
+});
+
+test("the base ref defaults, and an explicit one is honoured", () => {
+  assert.equal(parseBase([]), "origin/main");
+  assert.equal(parseBase(["--base", "origin/release"]), "origin/release");
+});
+
+test("a --base with no value fails rather than silently defaulting", () => {
+  // Silently falling back would diff against the wrong ref and report a pass.
+  assert.throws(() => parseBase(["--base"]), /needs a ref/);
+  assert.throws(() => parseBase(["--base", "--other"]), /needs a ref/);
+});