mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(hooks): scope pre-push lanes to branch merge-base diff (#6423)
Lefthook selected pre-push lanes from the file set `git diff --name-only
HEAD @{push}` returns, which is overinclusive two different ways:
- **Rebase/merge poisoning.** `@{push}` is a two-dot diff against the
branch's last-pushed tip. After a rebase onto newer main — or the `git
merge origin/main` that `check-branch-skew.sh` itself instructs agents
to run — that diff includes everything `main` changed since the last
push, so `mobile-checks`, the `desktop-*` lanes, and `rust-tests` fire
on branches that never touched those trees (a recent relay-only push
discovered 266 files, 26 of them `mobile/`).
- **Brand-new branch in a worktree.** When `@{push}` doesn't resolve,
lefthook reads `$GIT_DIR/refs/remotes/origin/HEAD` — which doesn't exist
in a linked worktree's git-dir — then falls back to the
alphabetically-first `HEAD ->` remote. In a clone with a stale
third-party remote sorted before `origin`, that diffs against a frozen
tip and runs the full suite. All agent work happens in worktrees.
## Fix
Add `files: git diff --name-only origin/main...HEAD` to the six globbed
pre-push lanes (`rust-tests`, `desktop-check`, `desktop-typecheck`,
`desktop-test`, `desktop-tauri-checks`, `mobile-checks`). The three-dot
form is the branch's merge-base diff against `origin/main` — the same
base resolution `scripts/check-file-sizes-core.mjs` (`resolveBaseRef`)
and CI's dorny/paths-filter already use. A lane now fires only when this
branch actually changed a file it covers; `main`-side changes never
re-trigger it, and a new branch diffs against `origin/main` (empty diff
→ all globbed lanes skip) rather than a stale remote. `branch-skew` and
`file-size-check` stay unfiltered by design.
Also pin hook dispatch to the Hermit-managed lefthook. The generated
`.git/hooks/*` dispatchers prefer `$LEFTHOOK_BIN`, then a `lefthook` on
`PATH` — so a Homebrew lefthook shadowed the pinned 2.1.3. A `rc:
bin/.lefthookrc` (a lefthook config key `lefthook install` bakes into
every dispatcher) exports `LEFTHOOK_BIN` to `bin/lefthook`, so a push
runs the pinned version regardless of `PATH`. `just hooks` already runs
`lefthook install`, so re-running it picks up the `rc` line — no recipe
change needed.
`AGENTS.md` documents that lanes are scoped by merge-base diff and that
agent shells should lead `PATH` with `./bin` so pinned tools win.
## Verification
Reproduced both bugs in a scratch repo mirroring the worktree +
stale-remote topology, with lanes stubbed to echo on selection:
- **Bug 1:** relay-only branch, `git merge origin/main` (main carried
`mobile/` + `desktop/` changes). Without the fix, `desktop-check` and
`mobile-checks` fired; with it, three-dot diff = only the relay file,
all globbed lanes skip.
- **Bug 2:** brand-new branch, no upstream, in a linked worktree with an
alphabetically-first stale remote. Confirmed `@{push}` is `fatal`,
`refs/remotes/origin/HEAD` is absent from the worktree git-dir, and the
stale remote's `HEAD ->` sorts first. Without the fix it fired lanes off
the stale tip; with it, lanes scope to `origin/main` merge-base and
skip.
- **Positive controls:** a `mobile/**` commit still fires
`mobile-checks`; a `crates/**` commit still fires `rust-tests` +
`desktop-tauri-checks`.
- **Dispatch:** with Homebrew lefthook 2.1.10 on `PATH`, running the
real generated `.git/hooks/pre-push` under `LEFTHOOK_VERBOSE=1` shows it
sourcing `bin/.lefthookrc`, exporting `LEFTHOOK_BIN`, and running
`lefthook v2.1.3`.
- This PR's own push (a brand-new branch in a linked worktree — the bug
2 case) ran only `branch-skew` and `file-size-check`; no
mobile/desktop/rust lanes fired, since the branch touches only
`lefthook.yml`, `AGENTS.md`, and `bin/.lefthookrc`.
---------
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
Co-authored-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
This commit is contained in:
19
AGENTS.md
19
AGENTS.md
@@ -125,9 +125,22 @@ clippy (workspace + Tauri), desktop TypeScript typechecking (`tsc --noEmit`),
|
||||
and fast unit tests in parallel (Rust, desktop JS, Tauri Rust, mobile Flutter)
|
||||
— no overlap with pre-commit. Builds are CI-only. Run `just fix-all` to auto-fix
|
||||
all formatting in one shot. Run `just ci` for the full local gate. Run `just
|
||||
hooks` to re-install hooks after env changes. Before agents run Git or hooks,
|
||||
activate the repo's Hermit environment (`. ./bin/activate-hermit`); do not
|
||||
rewrite hook commands to compensate for an unconfigured shell `PATH`.
|
||||
hooks` to re-install hooks after env changes. Each globbed pre-push lane is
|
||||
scoped to the branch's merge-base diff against `origin/main` (`git diff
|
||||
origin/main...HEAD`), matching CI's paths-filter — so a lane only fires when this
|
||||
branch actually changed a file it covers, never because `origin/main` moved.
|
||||
These lanes validate the checked-out HEAD; pushing a non-HEAD ref (explicit
|
||||
refspec, `--all`) gets a non-fatal `push-head-scope` warning and relies on CI for
|
||||
its path-scoped checks.
|
||||
Before agents run Git or hooks, activate the repo's Hermit environment
|
||||
(`. ./bin/activate-hermit`) so `./bin` leads `PATH` and the pinned toolchain
|
||||
(flutter, dart, lefthook) wins over any Homebrew version; do not
|
||||
rewrite hook commands to compensate for an unconfigured shell `PATH`. The
|
||||
pre-push hook self-pins regardless: `bin/.lefthookrc` (sourced by the generated
|
||||
`.git/hooks/*`) prepends the Hermit `bin/` to `PATH` and pins `LEFTHOOK_BIN`, so
|
||||
lane subprocesses resolve the pinned flutter/dart/lefthook even when an
|
||||
unactivated shell has Homebrew first. Activating Hermit remains recommended for
|
||||
non-hook commands.
|
||||
|
||||
**Commit with `git commit -s`.** The required **DCO Check** fails any PR with a commit missing a `Signed-off-by` trailer, and `just hooks` installs a `commit-msg` hook that adds it to commits you create locally (`git rebase` and `git cherry-pick` still need `--signoff`) — if you build commit commands programmatically, include `-s` every time. To repair a branch that already has unsigned commits: `git rebase --signoff main`, then force-push.
|
||||
|
||||
|
||||
21
bin/.lefthookrc
Executable file
21
bin/.lefthookrc
Executable file
@@ -0,0 +1,21 @@
|
||||
# Sourced by the generated .git/hooks/* dispatchers (see `rc:` in lefthook.yml)
|
||||
# before their $LEFTHOOK_BIN-first lookup. Two jobs, both anchored on the repo
|
||||
# root so they hold regardless of the hook's working dir:
|
||||
# 1. Pin dispatch to the Hermit-managed lefthook (bin/lefthook ->
|
||||
# .lefthook-2.1.3.pkg) so a push from any worktree runs the pinned version
|
||||
# even when a newer lefthook is on PATH (e.g. Homebrew).
|
||||
# 2. Prepend the Hermit bin/ to PATH so every lane subprocess (just mobile-check
|
||||
# -> flutter/dart, etc.) resolves the repo's pinned toolchain, not whatever
|
||||
# the invoking shell had first (e.g. Homebrew flutter). This is the safe
|
||||
# subset of `activate-hermit`: a plain PATH prepend, no interactive-shell
|
||||
# machinery. It makes the hook self-pinning regardless of shell setup.
|
||||
_lefthook_root="$(git rev-parse --show-toplevel 2>/dev/null)"
|
||||
if [ -n "$_lefthook_root" ] && [ -d "$_lefthook_root/bin" ]; then
|
||||
PATH="$_lefthook_root/bin:$PATH"
|
||||
export PATH
|
||||
if [ -x "$_lefthook_root/bin/lefthook" ]; then
|
||||
LEFTHOOK_BIN="$_lefthook_root/bin/lefthook"
|
||||
export LEFTHOOK_BIN
|
||||
fi
|
||||
fi
|
||||
unset _lefthook_root
|
||||
39
lefthook.yml
39
lefthook.yml
@@ -1,5 +1,18 @@
|
||||
# Glob patterns below mirror the `changes` job's dorny/paths-filter groups in
|
||||
# .github/workflows/ci.yml — keep the two in sync. Deliberate deviations:
|
||||
# .github/workflows/ci.yml — keep the two in sync. Each globbed pre-push lane
|
||||
# also sets `files: git diff --name-only origin/main...HEAD` so its file set is
|
||||
# the branch's merge-base diff against origin/main — the same semantics as CI's
|
||||
# dorny/paths-filter and `scripts/check-file-sizes-core.mjs` `resolveBaseRef`.
|
||||
# Without it lefthook derives files from `git diff HEAD @{push}`, which after a
|
||||
# rebase or `git merge origin/main` includes everything main changed since the
|
||||
# branch's last push (firing mobile/desktop/rust lanes on unrelated pushes), and
|
||||
# on a brand-new branch falls back to a stale remote's tip (a full-suite run).
|
||||
# The three-dot diff scopes every lane to what this branch actually changed.
|
||||
# Both the three-dot diff and lefthook's stock discovery inspect the checked-out
|
||||
# HEAD, so pushing a non-HEAD ref (explicit refspec, `--all`) validates HEAD's
|
||||
# tree, not the pushed commit — the always-on `push-head-scope` lane warns when
|
||||
# they diverge and CI remains the authoritative gate for that ref.
|
||||
# Deliberate deviations:
|
||||
# - The `.github/workflows/ci.yml` path CI adds to its `rust`/`mobile` filters
|
||||
# is omitted; a CI-workflow-only edit doesn't need a local test run.
|
||||
# - `desktop-check`/`desktop-typecheck`/`desktop-test` don't trigger on `rust`
|
||||
@@ -16,6 +29,17 @@
|
||||
# - `commit-msg` has no glob: it rewrites the commit message, not files. Note
|
||||
# Git only runs it for `git commit` and `git merge` — other flows need their own
|
||||
# flag: `git rebase --signoff`, `git cherry-pick -s`.
|
||||
#
|
||||
# `rc` points `lefthook install` at `bin/.lefthookrc`, which the generated
|
||||
# `.git/hooks/*` dispatchers source before their `$LEFTHOOK_BIN`-first lookup. It
|
||||
# (1) sets `LEFTHOOK_BIN` to the Hermit-pinned `bin/lefthook` (2.1.3) so a push
|
||||
# from any worktree runs the pinned binary even when a different lefthook is on
|
||||
# PATH, and (2) prepends the repo's Hermit `bin/` to `PATH` so every lane
|
||||
# subprocess (`just mobile-check` -> flutter/dart, etc.) resolves the pinned
|
||||
# toolchain regardless of the invoking shell — the hook self-pins, so a Homebrew
|
||||
# flutter/dart ahead on PATH can no longer cause false lane failures.
|
||||
rc: bin/.lefthookrc
|
||||
|
||||
pre-commit:
|
||||
parallel: true
|
||||
commands:
|
||||
@@ -56,22 +80,33 @@ pre-push:
|
||||
commands:
|
||||
branch-skew:
|
||||
run: ./scripts/check-branch-skew.sh
|
||||
push-head-scope:
|
||||
# Warn-only: the scoped lanes below validate the checked-out HEAD, so a
|
||||
# non-HEAD ref push (explicit refspec, --all) relies on CI for its
|
||||
# path-scoped checks. This lane reads the pushed refs from stdin and
|
||||
# warns when any pushed commit is not HEAD. It never fails the push.
|
||||
use_stdin: true
|
||||
run: ./scripts/check-push-head-scope.sh
|
||||
file-size-check:
|
||||
# The ratchet computes its own merge-base diff, so path filtering here
|
||||
# would only duplicate policy and create another place for coverage drift.
|
||||
run: just file-size-check
|
||||
rust-tests:
|
||||
files: git diff --name-only origin/main...HEAD
|
||||
glob: ["crates/**", "migrations/**", "schema/**", "Cargo.toml", "Cargo.lock", "rust-toolchain.toml", "deny.toml", "scripts/run-tests.sh", "Justfile"]
|
||||
run: just test-unit
|
||||
desktop-check:
|
||||
files: git diff --name-only origin/main...HEAD
|
||||
glob: ["desktop/**", "pnpm-lock.yaml"]
|
||||
exclude: ["desktop/src-tauri/**"]
|
||||
run: just desktop-check
|
||||
desktop-typecheck:
|
||||
files: git diff --name-only origin/main...HEAD
|
||||
glob: ["desktop/**", "pnpm-lock.yaml"]
|
||||
exclude: ["desktop/src-tauri/**"]
|
||||
run: just desktop-typecheck
|
||||
desktop-test:
|
||||
files: git diff --name-only origin/main...HEAD
|
||||
glob: ["desktop/**", "pnpm-lock.yaml"]
|
||||
exclude: ["desktop/src-tauri/**"]
|
||||
run: just desktop-test
|
||||
@@ -79,10 +114,12 @@ pre-push:
|
||||
# Keep local lint parity with Desktop Core CI for every path that can
|
||||
# affect the Tauri crate or its path dependencies. Run clippy and tests
|
||||
# serially so parallel pre-push hooks do not contend for Cargo's lock.
|
||||
files: git diff --name-only origin/main...HEAD
|
||||
glob: ["desktop/src-tauri/**", "crates/**", "migrations/**", "schema/**", "Cargo.toml", "Cargo.lock", "rust-toolchain.toml", "deny.toml", "scripts/run-tests.sh", "Justfile"]
|
||||
run: just desktop-tauri-clippy && just desktop-tauri-test
|
||||
mobile-checks:
|
||||
# Run analysis and tests serially so parallel pre-push hooks do not
|
||||
# contend for shared Flutter state (.dart_tool, shader bundle).
|
||||
files: git diff --name-only origin/main...HEAD
|
||||
glob: ["mobile/**"]
|
||||
run: just mobile-check && just mobile-test
|
||||
|
||||
42
scripts/check-push-head-scope.sh
Executable file
42
scripts/check-push-head-scope.sh
Executable file
@@ -0,0 +1,42 @@
|
||||
#!/usr/bin/env bash
|
||||
# Pre-push scope signal (warn-only, never fails the push).
|
||||
#
|
||||
# The scoped pre-push lanes derive their file set from `git diff origin/main...HEAD`,
|
||||
# and lefthook's own stock discovery is `git diff HEAD @{push}` — both inspect the
|
||||
# checked-out HEAD, not the SHAs actually being pushed. Pushing a non-checked-out
|
||||
# ref (e.g. `git push origin other-branch`, an explicit refspec, or `--all`) can
|
||||
# therefore run the scoped lanes against HEAD while pushing a different commit, so
|
||||
# the pushed ref's path-scoped checks are only exercised by CI.
|
||||
#
|
||||
# This lane reads the pre-push stdin refspecs and prints a loud, non-fatal warning
|
||||
# naming each pushed ref whose commit is not the checked-out HEAD. It never blocks
|
||||
# the push; CI remains the authoritative gate for a non-HEAD ref.
|
||||
#
|
||||
# Stdin format (one line per ref, git-provided):
|
||||
# <local_ref> <local_sha> <remote_ref> <remote_sha>
|
||||
set -euo pipefail
|
||||
|
||||
head_sha=$(git rev-parse HEAD 2>/dev/null) || exit 0
|
||||
|
||||
divergent=""
|
||||
while read -r local_ref local_sha remote_ref remote_sha; do
|
||||
# Empty stdin (up-to-date push with no refs) or malformed line: skip.
|
||||
[ -n "${local_sha:-}" ] || continue
|
||||
# Deletion push (local sha is all-zeros): nothing built locally to validate.
|
||||
case "$local_sha" in *[!0]*) : ;; *) continue ;; esac
|
||||
# Pushed commit equals the checked-out HEAD: scoped lanes validated it.
|
||||
[ "$local_sha" = "$head_sha" ] && continue
|
||||
divergent="${divergent} ${local_ref:-?} -> ${remote_ref:-?} ($(git rev-parse --short "$local_sha" 2>/dev/null || echo "$local_sha"))
|
||||
"
|
||||
done
|
||||
|
||||
if [ -n "$divergent" ]; then
|
||||
{
|
||||
echo "Pushing commits that are not the checked-out HEAD:"
|
||||
printf '%s' "$divergent"
|
||||
echo "Local scoped lanes validated HEAD only; these refs rely on CI for"
|
||||
echo "path-scoped checks. This is a warning, not a failure."
|
||||
} >&2
|
||||
fi
|
||||
|
||||
exit 0
|
||||
Reference in New Issue
Block a user