From 660a3dad21e0b277f81ed25cba6bc3f6f064d9ae Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 12 Mar 2026 12:13:55 +0200 Subject: [PATCH] feat(scripts/githooks): restore pre-push hook with allowlist (#22980) The pre-push hook was removed in #22956. This restores it with a reduced scope (tests + site build) and an allowlist so it only runs for developers who opt in. Two opt-in mechanisms: - git config coder.pre-push true (local, not committed) - CODER_WORKSPACE_OWNER_NAME allowlist in the hook script git config takes priority and also supports explicit opt-out for allowlisted users (git config coder.pre-push false). Refs #22956 --------- Co-authored-by: Cian Johnston --- AGENTS.md | 10 ++-- Makefile | 32 +++++++++--- docs/about/contributing/CONTRIBUTING.md | 1 + scripts/githooks/pre-commit | 1 + scripts/githooks/pre-push | 65 +++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 11 deletions(-) create mode 100755 scripts/githooks/pre-push diff --git a/AGENTS.md b/AGENTS.md index ece912591f..832f090978 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,6 +50,7 @@ Only pause to ask for confirmation when: | **Format** | `make fmt` | Auto-format code | | **Clean** | `make clean` | Clean build artifacts | | **Pre-commit** | `make pre-commit` | Fast CI checks (gen/fmt/lint/build) | +| **Pre-push** | `make pre-push` | Heavier CI checks (allowlisted) | ### Documentation Commands @@ -118,13 +119,16 @@ no matter how long they take. git config core.hooksPath scripts/githooks ``` -One hook runs automatically: +Two hooks run automatically: - **pre-commit**: `make pre-commit` (gen, fmt, lint, typos, build). Fast checks that catch most CI failures. Allow at least 5 minutes. +- **pre-push**: `make pre-push` (heavier checks including tests). + Allowlisted in `scripts/githooks/pre-push`. Runs only for developers + who opt in. Allow at least 15 minutes. -`git commit` will appear to hang while the hook runs. This is normal. -Do not interrupt, retry, or reduce the timeout. +`git commit` and `git push` will appear to hang while hooks run. +This is normal. Do not interrupt, retry, or reduce the timeout. NEVER run `git config core.hooksPath` to change or disable hooks. diff --git a/Makefile b/Makefile index 24d12c9d8f..8ea62233ec 100644 --- a/Makefile +++ b/Makefile @@ -20,9 +20,9 @@ SHELL := bash .ONESHELL: # When MAKE_TIMED=1, replace SHELL with a wrapper that prints -# elapsed wall-clock time for each recipe. pre-commit sets this on -# its sub-makes so every parallel job reports its duration. Ad-hoc -# usage: make MAKE_TIMED=1 test +# elapsed wall-clock time for each recipe. pre-commit and pre-push +# set this on their sub-makes so every parallel job reports its +# duration. Ad-hoc usage: make MAKE_TIMED=1 test ifdef MAKE_TIMED SHELL := $(CURDIR)/scripts/lib/timed-shell.sh .SHELLFLAGS = $@ -ceu @@ -114,9 +114,9 @@ VERSION := $(shell ./scripts/version.sh) POSTGRES_VERSION ?= 17 POSTGRES_IMAGE ?= us-docker.pkg.dev/coder-v2-images-public/public/postgres:$(POSTGRES_VERSION) -# Limit parallel Make jobs in pre-commit. Defaults to nproc/4 -# (min 2) since lint and build targets have internal parallelism. -# Override: make pre-commit PARALLEL_JOBS=8 +# Limit parallel Make jobs in pre-commit/pre-push. Defaults to +# nproc/4 (min 2) since test, lint, and build targets have internal +# parallelism. Override: make pre-push PARALLEL_JOBS=8 PARALLEL_JOBS ?= $(shell n=$$(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 8); echo $$(( n / 4 > 2 ? n / 4 : 2 ))) # Use the highest ZSTD compression level in release builds to @@ -717,13 +717,16 @@ lint/typos: build/typos-$(TYPOS_VERSION) build/typos-$(TYPOS_VERSION) --config .github/workflows/typos.toml .PHONY: lint/typos -# pre-commit mirrors the fast local CI checks. +# pre-commit and pre-push mirror CI checks locally. # -# It runs checks that don't need external services (Docker, +# pre-commit runs checks that don't need external services (Docker, # Playwright). This is the git pre-commit hook default since Docker # and browser issues in the local environment would otherwise block # all commits. # +# pre-push adds heavier checks: Go tests, JS tests, and site build. +# The pre-push hook is allowlisted, see scripts/githooks/pre-push. +# # pre-commit uses two phases: gen+fmt first, then lint+build. This # avoids races where gen's `go run` creates temporary .go files that # lint's find-based checks pick up. Within each phase, targets run in @@ -770,6 +773,19 @@ pre-commit: echo "$(GREEN)✓ pre-commit passed$(RESET) ($$(( $$(date +%s) - $$start ))s)" .PHONY: pre-commit +pre-push: + start=$$(date +%s) + logdir=$$(mktemp -d "$${TMPDIR:-/tmp}/coder-pre-push.XXXXXX") + echo "$(BOLD)pre-push$(RESET) ($$logdir)" + echo "test + build site:" + $(MAKE) --no-print-directory -j$(PARALLEL_JOBS) MAKE_TIMED=1 MAKE_LOGDIR=$$logdir \ + test \ + test-js \ + site/out/index.html + rm -rf $$logdir + echo "$(GREEN)✓ pre-push passed$(RESET) ($$(( $$(date +%s) - $$start ))s)" +.PHONY: pre-push + offlinedocs/check: offlinedocs/node_modules/.installed cd offlinedocs/ pnpm format:check diff --git a/docs/about/contributing/CONTRIBUTING.md b/docs/about/contributing/CONTRIBUTING.md index 10e7914fa7..09934e6e42 100644 --- a/docs/about/contributing/CONTRIBUTING.md +++ b/docs/about/contributing/CONTRIBUTING.md @@ -71,6 +71,7 @@ Use the following `make` commands and scripts in development: - `make install` installs binaries to `$GOPATH/bin` - `make test` - `make pre-commit` runs gen, fmt, lint, typos, and builds a slim binary +- `make pre-push` runs heavier CI checks including tests (allowlisted) Install the git hooks to run these automatically: diff --git a/scripts/githooks/pre-commit b/scripts/githooks/pre-commit index edc47949ae..5d52dde07f 100755 --- a/scripts/githooks/pre-commit +++ b/scripts/githooks/pre-commit @@ -3,6 +3,7 @@ # Pre-commit hook that runs CI-equivalent checks locally. # Runs `make pre-commit` (gen, fmt, lint, typos, build) which # catches most CI failures without needing Docker or Playwright. +# Heavier checks (tests, site build) run via the pre-push hook. # # Installation (worktree-compatible): # diff --git a/scripts/githooks/pre-push b/scripts/githooks/pre-push new file mode 100755 index 0000000000..eb519065d4 --- /dev/null +++ b/scripts/githooks/pre-push @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# +# Pre-push hook that runs tests and builds the site locally. +# The pre-commit hook handles gen, fmt, lint, typos, and build. +# +# Opt in/out without modifying this file: +# +# git config coder.pre-push true # opt in +# git config coder.pre-push false # opt out (overrides allowlist) +# git config --unset coder.pre-push # default (allowlist decides) +# +# Installation (worktree-compatible): +# +# git config core.hooksPath scripts/githooks +# +# Bypass: git push --no-verify + +set -euo pipefail + +# Allowlist of developers who opt in to pre-push checks by default. +# Matched against CODER_WORKSPACE_OWNER_NAME. +ALLOWLIST=( + mafredri + johnstcn +) + +cd "$(git rev-parse --show-toplevel)" +unset GIT_DIR + +# In linked worktrees, set worktree-scoped hooksPath to override shared config. +if [[ "$(git rev-parse --git-dir)" != "$(git rev-parse --git-common-dir)" ]]; then + git config --worktree core.hooksPath scripts/githooks +fi + +# Explicit opt-in/opt-out via git config (overrides allowlist). +run=false +opt_in=$(git config --type=bool coder.pre-push 2>/dev/null || true) +if [[ $opt_in == true ]]; then + run=true +elif [[ $opt_in == false ]]; then + # Explicit opt-out, skip everything including hint. + exit 0 +fi + +# Check allowlist. +if ! $run; then + owner=${CODER_WORKSPACE_OWNER_NAME:-} + for allowed in "${ALLOWLIST[@]}"; do + if [[ $owner == "$allowed" ]]; then + run=true + break + fi + done +fi + +rc=0 +if $run; then + make pre-push || rc=$? +fi + +# Hint is printed unconditionally so that AI agents that are not +# opted in to pre-push checks are still reminded to monitor CI. +echo "Hint: If you're an AI agent creating a PR, monitor CI and iterate on failures until all checks pass." + +exit "$rc"