From 23542cb6afdf056027dc583d23dd21cc8b5ca5f3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 20 Mar 2026 17:05:44 +0200 Subject: [PATCH] feat: smart file-based target selection for scripts/githooks (#23358) Pre-commit classifies staged files and runs make pre-commit-light when no Go, TypeScript, or Makefile changes are present. This skips gen, lint/go, lint/ts, fmt/go, fmt/ts, and the binary build. A markdown-only commit takes seconds instead of minutes. Pre-push uses the same heuristic: if only light files changed (docs, shell, terraform, etc.), tests are skipped entirely. Falls back to the full make targets when Go/TS/Makefile changes are detected, CODER_HOOK_RUN_ALL=1 is set, or the diff range can't be determined. Also adds test-storybook to make pre-push (vitest with the storybook project in Playwright browser mode). --- AGENTS.md | 19 ++++++++++---- Makefile | 33 +++++++++++++++++++++++++ scripts/githooks/pre-commit | 25 ++++++++++++++++--- scripts/githooks/pre-push | 49 ++++++++++++++++++++++++++++++++++++- 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 51699b93e9..5898c3b8c8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -146,11 +146,20 @@ git config core.hooksPath scripts/githooks 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. +- **pre-commit**: Classifies staged files by type and runs either + the full `make pre-commit` or the lightweight `make pre-commit-light` + depending on whether Go, TypeScript, SQL, proto, or Makefile + changes are present. Falls back to the full target when + `CODER_HOOK_RUN_ALL=1` is set. A markdown-only commit takes + seconds; a Go change takes several minutes. +- **pre-push**: Classifies changed files (vs remote branch or + merge-base) and runs `make pre-push` when Go, TypeScript, SQL, + proto, or Makefile changes are detected. Skips tests entirely + for lightweight changes. Allowlisted in + `scripts/githooks/pre-push`. Runs only for developers who opt + in. Falls back to `make pre-push` when the diff range can't + be determined or `CODER_HOOK_RUN_ALL=1` is set. Allow at least + 15 minutes for a full run. `git commit` and `git push` will appear to hang while hooks run. This is normal. Do not interrupt, retry, or reduce the timeout. diff --git a/Makefile b/Makefile index ca4a4ed4a6..1b62cd31a3 100644 --- a/Makefile +++ b/Makefile @@ -522,6 +522,10 @@ RESET := $(shell tput sgr0 2>/dev/null) fmt: fmt/ts fmt/go fmt/terraform fmt/shfmt fmt/biome fmt/markdown .PHONY: fmt +# Subset of fmt that does not require Go or Node toolchains. +fmt-light: fmt/shfmt fmt/terraform fmt/markdown +.PHONY: fmt-light + fmt/go: ifdef FILE # Format single file @@ -629,6 +633,10 @@ LINT_ACTIONS_TARGETS := $(if $(CI),,lint/actions/actionlint) lint: lint/shellcheck lint/go lint/ts lint/examples lint/helm lint/site-icons lint/markdown lint/check-scopes lint/migrations lint/bootstrap $(LINT_ACTIONS_TARGETS) .PHONY: lint +# Subset of lint that does not require Go or Node toolchains. +lint-light: lint/shellcheck lint/markdown lint/helm lint/bootstrap lint/migrations lint/actions/actionlint lint/typos +.PHONY: lint-light + lint/site-icons: ./scripts/check_site_icons.sh .PHONY: lint/site-icons @@ -771,6 +779,25 @@ pre-commit: echo "$(GREEN)✓ pre-commit passed$(RESET) ($$(( $$(date +%s) - $$start ))s)" .PHONY: pre-commit +# Lightweight pre-commit for changes that don't touch Go or +# TypeScript. Skips gen, lint/go, lint/ts, fmt/go, fmt/ts, and +# the binary build. Used by the pre-commit hook when only docs, +# shell, terraform, helm, or other fast-to-check files changed. +pre-commit-light: + start=$$(date +%s) + logdir=$$(mktemp -d "$${TMPDIR:-/tmp}/coder-pre-commit-light.XXXXXX") + echo "$(BOLD)pre-commit-light$(RESET) ($$logdir)" + echo "fmt:" + $(MAKE) --no-print-directory -j$(PARALLEL_JOBS) MAKE_TIMED=1 MAKE_LOGDIR=$$logdir fmt-light + $(check-unstaged) + echo "lint:" + $(MAKE) --no-print-directory -j$(PARALLEL_JOBS) MAKE_TIMED=1 MAKE_LOGDIR=$$logdir lint-light + $(check-unstaged) + $(check-untracked) + rm -rf $$logdir + echo "$(GREEN)✓ pre-commit-light passed$(RESET) ($$(( $$(date +%s) - $$start ))s)" +.PHONY: pre-commit-light + pre-push: start=$$(date +%s) logdir=$$(mktemp -d "$${TMPDIR:-/tmp}/coder-pre-push.XXXXXX") @@ -779,6 +806,7 @@ pre-push: $(MAKE) --no-print-directory -j$(PARALLEL_JOBS) MAKE_TIMED=1 MAKE_LOGDIR=$$logdir \ test \ test-js \ + test-storybook \ site/out/index.html rm -rf $$logdir echo "$(GREEN)✓ pre-push passed$(RESET) ($$(( $$(date +%s) - $$start ))s)" @@ -1313,6 +1341,11 @@ test-js: site/node_modules/.installed pnpm test:ci .PHONY: test-js +test-storybook: site/node_modules/.installed + cd site/ + pnpm exec vitest run --project=storybook +.PHONY: test-storybook + # sqlc-cloud-is-setup will fail if no SQLc auth token is set. Use this as a # dependency for any sqlc-cloud related targets. sqlc-cloud-is-setup: diff --git a/scripts/githooks/pre-commit b/scripts/githooks/pre-commit index 5d52dde07f..d04ba81479 100755 --- a/scripts/githooks/pre-commit +++ b/scripts/githooks/pre-commit @@ -1,9 +1,9 @@ #!/usr/bin/env bash # # 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. +# Classifies staged files by type and only runs relevant make +# targets. Falls back to the full `make pre-commit` when the +# Makefile changed or CODER_HOOK_RUN_ALL=1 is set. # # Installation (worktree-compatible): # @@ -20,4 +20,21 @@ unset GIT_DIR if [[ "$(git rev-parse --git-dir)" != "$(git rev-parse --git-common-dir)" ]]; then git config --worktree core.hooksPath scripts/githooks fi -exec make pre-commit + +if [[ ${CODER_HOOK_RUN_ALL:-} == 1 ]]; then + exec make pre-commit +fi + +staged=$(git diff --cached --name-only --diff-filter=d) +if [[ -z $staged ]]; then + echo "pre-commit: no staged changes, skipping" + exit 0 +fi + +# If Go, TS, or build-system files changed, run the full +# pre-commit. Otherwise run the lightweight target that +# covers everything except gen, Go/TS fmt+lint, and binary build. +if echo "$staged" | grep -qE '\.(go|ts|tsx|sql|proto)$|^go\.(mod|sum)$|^site/|^Makefile$'; then + exec make pre-commit +fi +exec make pre-commit-light diff --git a/scripts/githooks/pre-push b/scripts/githooks/pre-push index eb519065d4..6042f4898c 100755 --- a/scripts/githooks/pre-push +++ b/scripts/githooks/pre-push @@ -1,6 +1,11 @@ #!/usr/bin/env bash # # Pre-push hook that runs tests and builds the site locally. +# Classifies changed files (vs remote branch or merge-base) +# and only runs relevant test targets. Falls back to the full +# `make pre-push` when the Makefile changed, the diff range +# can't be determined, or CODER_HOOK_RUN_ALL=1 is set. +# # The pre-commit hook handles gen, fmt, lint, typos, and build. # # Opt in/out without modifying this file: @@ -32,6 +37,13 @@ if [[ "$(git rev-parse --git-dir)" != "$(git rev-parse --git-common-dir)" ]]; th git config --worktree core.hooksPath scripts/githooks fi +# Drain stdin before any early exits so git doesn't see a +# broken pipe. The push refs are used later for classification. +push_refs=() +while read -r local_ref local_oid remote_ref remote_oid; do + push_refs+=("$local_ref $local_oid $remote_ref $remote_oid") +done + # 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) @@ -55,7 +67,42 @@ fi rc=0 if $run; then - make pre-push || rc=$? + if [[ ${CODER_HOOK_RUN_ALL:-} == 1 ]]; then + make pre-push || rc=$? + else + # Determine changed files from push refs. + zero="0000000000000000000000000000000000000000" + changed="" + fallback=false + + for entry in "${push_refs[@]}"; do + read -r _local_ref local_oid _remote_ref remote_oid <<< "$entry" + if [[ $local_oid == "$zero" ]]; then + continue + fi + if [[ $remote_oid == "$zero" ]]; then + base=$(git merge-base "$local_oid" origin/main 2>/dev/null || true) + if [[ -z $base ]]; then + fallback=true + break + fi + else + base="$remote_oid" + fi + files=$(git diff --name-only "$base" "$local_oid" 2>/dev/null || true) + if [[ -n $files ]]; then + changed+=$'\n'"$files" + fi + done + + if $fallback || [[ -z $changed ]]; then + make pre-push || rc=$? + elif echo "$changed" | grep -qE '\.(go|ts|tsx|sql|proto)$|^go\.(mod|sum)$|^site/|^Makefile$'; then + make pre-push || rc=$? + else + echo "pre-push: no Go/TS changes, skipping tests" + fi + fi fi # Hint is printed unconditionally so that AI agents that are not