Repository navigation
ci(prek): protect main and versions/ branches from direct commits - #929
Conversation
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>
There was a problem hiding this comment.
🟡 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-branchfrompre-commit-hooksto block commits onmainandversions/*.
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.
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
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>
ea7fba9 to
b080e18
Compare
…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>
Summary
no-commit-to-branchhook from pre-commit-hooks to.pre-commit-config.yamlmain(--branch main) and anyversions/*maintenance branch (--pattern ^versions/.*) from direct commitsTest plan
prek run no-commit-to-branch --all-fileson branches namedmainandversions/vTest(fails as expected) and on a feature branch (passes)🤖 Generated with Claude Code