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

[miguel-heygen]

What

A gate that fails a branch proposing to delete files the base still ships, plus its tests, registered in test:scripts.

Stacked on #3149.

Why

Written after a scare that turned out to be a measurement error, and the error is the reason it exists rather than an aside.

Comparing tip to tip on a branch a month behind reports every file the base has added since the merge base as a deletion. On the branch that prompted this, that was 1,284 files including an entire skills tree, none of it real. A merge keeps the base's side, and a pull request shows the three-dot diff, which reported zero.

A large branch can also delete something real, and a name-only diff of thousands of files is not where anyone will spot it.

How

Uses the three-dot form, and says so in a comment at the top of the file, so the next person comparing two dots by hand has something to check their result against.

Renames are reported separately rather than folded in. 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, and both failure modes end with the gate being ignored.

The base defaults to origin/main and takes --base for anything else.

Test plan

• Unit tests added/updated
• Manual testing performed
• Documentation updated (if applicable)

6 unit tests cover the argument parsing and the rename/deletion split, and they run in test:scripts. Run against this branch it reports zero deletions.

Not covered. Nothing invokes it in CI yet, so it is a gate you run rather than one that runs for you. Wiring it into a workflow is a follow-up, kept out of here so the script and its tests can be reviewed on their own.

[vanceingalls]

Approving. Small, well-scoped repo-hygiene gate with a good failure-mode analysis baked into the comments.

Correctness

• Uses <base>...HEAD (three-dot), which matches what GitHub's PR view computes and answers the actual question ("what does this branch drop from base?"). The header comment explicitly warns future maintainers off the two-dot form, which is where the original scare came from. scripts/check-no-main-deletions.mjs:5-14.
• -M on git diff --name-status gives git's built-in rename detection, and classify() files R* into its own bucket rather than counting it as a deletion. That's the right call — the two are indistinguishable in a name-only view, and lumping them together would either mask real loss (if allowed) or cry wolf on every legitimate move (if blocked). scripts/check-no-main-deletions.mjs:46-56.
• Unreachable base exits 2, not 0. A misspelled ref reporting "no deletions" would be the exact silent pass this script is meant to prevent. scripts/check-no-main-deletions.mjs:66-72.
• parseBase rejects --base with no value or a following flag, instead of silently defaulting. Tested. scripts/check-no-main-deletions.mjs:26-33, scripts/check-no-main-deletions.test.mjs:35-40.

Blast radius

• Not wired as a CI gate; only the tests run via test:scripts (which ci.yml already invokes). So even a genuine false positive here doesn't block anyone today — worst case is the person who ran it locally sees noise. Wiring is explicitly called out as a follow-up in the PR body.

Nits (non-blocking, for the CI-wiring follow-up)

• P3: --name-status without -z means paths with tabs or unusual characters get quoted rather than tab-delimited, and line.split("\t") would misparse. Extreme edge case in this repo, but -z + null-splitting would be more robust when this eventually gates PRs.
• P3: -M uses git's default rename threshold (typically 50%). A file that's renamed and substantially rewritten will fall out as D+A rather than R, and would trip the gate as a deletion. Consider -M20% (or -B for break-detect) when wiring to CI, or an allowlist mechanism for intentional deletions — right now the only override is "remove the check for that path deliberately," which means a legit deletion PR has to touch this script.

Nothing else surfaced under the standard adversarial lenses (case sensitivity, symlinks, copies, ordering, arg-parse edges). Approve on the merits of the current scope.

— Via

[james-russo-rames-d-jusso]

Reviewed at 1edf729df.

Right shape for the guard. Uses git diff --name-status -M <base>...HEAD — the three-dot form against the merge-base — which is what prevents the false-alarm class this exists for (a two-dot diff on a stale branch reports every file main added since the fork as a "deletion"). The docstring at check-no-main-deletions.mjs:6-14 names the specific incident (1,284 false-alarm deletions including a whole skills tree) and the fix (... form), which is the kind of provenance next-toucher wants.

classify() at :47-58 correctly reads git's --name-status -M output: rename lines carry R<score>\told\tnew and go to renamed[]; delete lines carry D\tpath and go to deleted[]. Everything else silently ignored (add, modify, copy, unmerged — none of which are what this guards). Test at check-no-main-deletions.test.mjs:18-24 locks the rename-is-not-a-deletion distinction with a direct assertion; :26-33 covers the mixed case. Solid.

Base-flag parsing at :22-31: refuses --base with no value or with a following -flag, defaults to origin/main. Test at :36-42 covers both the default and the explicit-empty case. parseBase throws rather than returning null, so the top-level main() doesn't need to try/catch — but the execFileSync call at :71-73 does, and correctly process.exit(2) on unreachable base with a stderr message. Miga's docstring at :66-68 names this ("Reporting 'no deletions' because the ref was misspelled is the exact failure this exists to prevent") — matches the code.

One BLOCKER-tier concern below.

Blocker

The guard is added but never invoked — this PR is scaffolding. test:scripts at package.json:51 adds scripts/check-no-main-deletions.test.mjs so the classify/parseBase logic is tested, but nothing in this PR wires the actual main() entry point of check-no-main-deletions.mjs into CI or lefthook. Grepping the PR head for check-no-main-deletions in .github/, lefthook.yml, or any workflow config comes back with only the .test.mjs reference in package.json. So as landed, this PR:

• Adds a working guard script ✅
• Adds tests for its classify/parseBase logic ✅
• Provides ZERO branch-blocking effect because nothing runs the guard ❌

The PR title ("fail a branch that deletes files main still ships") states an outcome the code doesn't yet deliver. If the wiring lands in a follow-up PR, name that in the PR body so reviewers can decide whether to gate the merge on the follow-up. If the wiring was intended to be in this PR, it's missing — add a step to .github/workflows/ci.yml (probably in the changes / lint group) that runs node scripts/check-no-main-deletions.mjs --base origin/${{ github.base_ref }} and fails the check on non-zero exit. A one-liner.

The concern isn't the guard's correctness — the code is clean and well-motivated. It's that "add the guard" without "invoke the guard" doesn't produce the effect the title claims, and future readers looking for "why did this branch not fail on deletion" will find the guard here and be confused about why it didn't fire.

Nits

• The heuristic at parseBase:26 (value.startsWith("-")) rejects a ref name starting with - as an accidental flag pass-through. Refs almost never start with - (git rejects them for the same reason). Fine.
• main() iterates the first 25 deleted paths at :88-89 and the first 10 renamed at :80-81. Cap counts are hard-coded; different from each other. Small — a MAX_LISTED_DELETIONS = 25 / MAX_LISTED_RENAMES = 10 at the top would make the intent explicit.
• The -M similarity threshold defaults to 50%. If a delete + add of very similar content happens (large file mostly rewritten), git may report it as a rename with a low score. Fine — that's git's built-in behavior and the guard trusts it.

What lands cleanly

• Three-dot diff form (:74) with the docstring naming why (:6-14).
• Rename/deletion separation (:47-58), directly test-locked (:18-24, :26-33).
• Base-flag parsing with explicit "no ref" error (:22-31), test-locked (:36-42).
• Unreachable-base failure mode set to exit 2 with a clear stderr message (:71-73) — pass-because-ref-typo class closed.
• Bounded listing of deleted paths (:88-89) — CI log stays readable even on a large accidental deletion.

Wire the guard before landing — otherwise the PR reads as complete but the effect it names doesn't fire.

— Review by Rames D Jusso

[vanceingalls]

R2 delta-verify at 92d4e3a09.

Miguel's claim verified. .github/workflows/ci.yml:44-49 now adds a Reject accidental file deletions step that runs node scripts/check-no-main-deletions.mjs --base origin/main on every pull_request event, inside the changes job (so it gates before any other work). lefthook.yml mirrors it locally, and scripts/check-no-main-deletions.test.mjs covers the classifier (add / delete / rename / mixed / base-flag parsing / missing-value error). CI wiring is live.

R1 P3-1 (tab / --name-status parsing without -z) — UNCHANGED at the code level, but real-world risk is minimal. classify() at scripts/check-no-main-deletions.mjs:38-48 still splits on \n and \t. git diff --name-status defaults to core.quotePath=true, so paths containing tabs / newlines / non-ASCII / \ / " are C-quoted (e.g. a path foo\tbar.md prints as literal "foo\tbar.md"), which means the tab-separated field parse still yields exactly two tokens. The classified paths are only printed to stderr for the deletion report, never re-used as filesystem paths — so even a c-quoted string is display-only. -z / NUL-delim would be the canonical shape but wouldn't move real behavior on any current Hyperframes tree. Not a blocker post-wiring.

R1 P3-2 (default -M ~50% rename threshold) — UNCHANGED, residual risk on rename-plus-heavy-rewrite PRs. git diff --name-status -M base...HEAD uses git's default 50% similarity, so a genuine rename that also rewrites >50% of the file falls out as D old + A new and gets classified into deleted. With CI now enforcing exit-1 on any deletion, a rename-and-refactor PR will be blocked and the escape hatch is essentially "edit check-no-main-deletions.mjs in the same PR to drop the check" (there's no --allow-delete, no allowlist path, no -B for break-rewrites). I don't see clear evidence this fires on the current Hyperframes rename cadence, so it's a follow-up rather than a blocker; happy to file if useful. Cheap forward moves later: -B for break-rewrites, -M20%, or an explicit allowlist arg.

Positives. Uses execFileSync (no shell); three-dot base…HEAD (the exact shape the commit message defends, avoids the 1,284-file tip-to-tip false alarm on stale branches); explicit exit 2 on unreachable base (a misspelled --base fails loud instead of green-passing); renames reported separately with a truncated list; and the wiring lives in the changes job so the gate runs early. Test file is small but covers the code paths that matter.

Verdict: APPROVE with a P3 follow-up note on the -M threshold. The tab-parse concern is display-only under default git config.

— Via

[james-russo-rames-d-jusso]

Reviewed at 92d4e3a09 (delta from 1edf729df).

R1 blocker closed. The guard is now wired into the PR-critical path at .github/workflows/ci.yml:47-49:

- name: Reject accidental file deletions
  if: github.event_name == 'pull_request'
  run: node scripts/check-no-main-deletions.mjs --base origin/main

Placed inside the changes job (ci.yml:29-97), which every subsequent CI job depends on via needs: changes — Typecheck, Build, Test, Test: runtime contract, Render on windows-latest, Tests on windows-latest, and the regression job (7 of the 8 required checks in ruleset 14211637) all cascade off it. A PR that deletes a file main still ships:

1. Detect changes job fails on the deletion.
2. All 7 dependent required checks skip (not-run).
3. Ruleset blocks merge on the missing required checks.

Effect the PR title claims now fires. The step is gated on pull_request so pushes to main don't false-alarm.

Docstring at check-no-main-deletions.mjs:6-14 still names the specific 1,284-false-alarm incident that motivated the three-dot form — the kind of provenance next-toucher wants.

Nits

• --base origin/main is hardcoded in the workflow at :49 rather than origin/${{ github.base_ref }} — fine for a repo where every PR targets main, but worth naming if the repo ever adopts release branches. Follow-up.
• The changes job's Detect changes name is not itself in the ruleset's required-checks list — merge-blocking comes entirely from the downstream-needs-cascade, not from changes being a required check directly. Worth double-checking that the ruleset semantics for "skipped required check" is "not-passing" here; if a ruleset ever moves to counting skipped-as-passing, the belt-and-suspenders fix is to name a new required check that runs the guard directly. Not blocking today.

Clean; ready from where I sit — stamp routing per standing rule.

— Review by Rames D Jusso

The base branch was changed.