Skip to content

ci(prek): protect main and versions/ branches from direct commits - #929

Merged
nstarman merged 10 commits into
mainfrom
add-no-commit-to-branch-prek
Sep 15, 2026
Merged

nstarman merged 10 commits into
mainfrom
add-no-commit-to-branch-prek

Conversation

@nstarman

Copy link
Copy Markdown
Contributor

Summary

  • Adds the no-commit-to-branch hook from pre-commit-hooks to .pre-commit-config.yaml
  • Protects main (--branch main) and any versions/* maintenance branch (--pattern ^versions/.*) from direct commits

Test plan

  • Verified locally with prek run no-commit-to-branch --all-files on branches named main and versions/vTest (fails as expected) and on a feature branch (passes)

🤖 Generated with Claude Code

Adds the no-commit-to-branch hook from pre-commit-hooks, blocking
direct commits to main and any versions/* branch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 15, 2026 14:13
@nstarman nstarman added this to the v2.1.0 milestone Sep 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new hook will also run on pre-push due to default_stages, which is a behavioral mismatch with the PR’s stated “direct commits” scope unless stages are pinned explicitly.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR adds a pre-commit guardrail to prevent direct commits to protected branches in the unxt repository by enabling the no-commit-to-branch hook.

Changes:

  • Add no-commit-to-branch from pre-commit-hooks to block commits on main and versions/*.
File summaries
File Description
.pre-commit-config.yaml Adds a branch-protection hook to prevent direct commits to main and versions/*.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .pre-commit-config.yaml
@codecov

codecov Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.96%. Comparing base (b91fed8) to head (d5b49d1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #929      +/-   ##
==========================================
+ Coverage   99.80%   99.96%   +0.15%     
==========================================
  Files          86       47      -39     
  Lines        4067     2710    -1357     
  Branches      318      165     -153     
==========================================
- Hits         4059     2709    -1350     
+ Misses          4        0       -4     
+ Partials        4        1       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This was referenced Sep 15, 2026
always_run is already the hook's shipped default, but setting it
explicitly documents that this hook intentionally ignores any
files/exclude/types filtering (and would still allow --allow-empty
commits through).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
no-commit-to-branch would otherwise fail every push to main once this
merges: CI checks out a real local branch literally named `main` for
push events, so the hook would always fire. It's a client-side guard
for a human running `git commit`/`git push` locally (or via installed
git hooks) -- not something a full "run every hook" CI invocation
should re-evaluate after the fact. Skips it there via
SKIP=no-commit-to-branch; the hook itself is untouched and still fully
active locally.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@nstarman
nstarman force-pushed the add-no-commit-to-branch-prek branch from ea7fba9 to b080e18 Compare September 15, 2026 15:28
@github-actions github-actions Bot added the 🔧 Add / update configuration Add or update configuration files. label Sep 15, 2026
nstarman and others added 7 commits September 15, 2026 11:40
…anch

Appends no-commit-to-branch to any SKIP a developer already has set
(e.g. via their shell) rather than overwriting it wholesale, matching
the same fix applied in response to Copilot review feedback on
GalacticDynamics/coordinax#885.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
With default_stages: [pre-commit, pre-push] set repo-wide, this hook
would also run on pre-push. It checks the currently checked-out
branch, not the ref being pushed, so a maintainer sitting on main and
pushing a release tag (or anything else) would be blocked even though
they aren't committing to main. Restricting to stages: [pre-commit]
keeps the guard at the point it's meant for -- a local `git commit` --
without touching pre-push at all.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses Copilot review feedback on GalacticDynamics/galax#847: the
comment said "CI checks out the real main branch," but the skip
applies unconditionally, including local `nox -s lint` runs -- which
is correct (a CI-only skip would leave the same false failure for any
local dev running the full suite while on `main`). Fixes the wording
to match the actual, intended behavior instead of narrowing it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses Copilot review feedback on GalacticDynamics/dataclassish#94:
the docstring "Run prek." on a session still named `precommit` could
read as though the session itself was renamed. Spells out that it
runs the pre-commit hooks, now via prek.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses Copilot review feedback on GalacticDynamics/galax#847: this
comment still said no-commit-to-branch guards `git push`, but the
earlier stages: [pre-commit] fix means it no longer runs on push at
all. Clarifies that explicitly instead of leaving stale wording.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… trigger

Addresses Copilot review feedback on GalacticDynamics/galax#847: "it
never fires on push" reads as a claim about this workflow's own
`on: push:` trigger (which is false -- that's why the SKIP exists at
all), when it actually means the git pre-push hook stage. Spells that
out explicitly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The archaeology (why pre-commit's nodeenv/pyyaml floors mattered, why
--skip clobbers, the full CI-checkout explanation) belongs in commit
history, not permanently inline. Keeps just enough to orient a future
reader without re-litigating the whole investigation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@nstarman
nstarman merged commit a9a299d into main Sep 15, 2026
37 checks passed
@nstarman
nstarman deleted the add-no-commit-to-branch-prek branch September 15, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔧 Add / update configuration Add or update configuration files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants