From 6994fea0bd08c38fa7c037ae0c1846624dff7c4f Mon Sep 17 00:00:00 2001 From: Marcus Vorwaller Date: Wed, 1 Apr 2026 02:35:06 -0700 Subject: [PATCH] chore(git): add commit message normalizer Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- .goreleaser.yml | 8 +- Makefile | 17 +++- README.md | 23 ++++- scripts/commit-msg.sh | 199 +++++++++++++++++++++++++++++++++++++ scripts/loop-prompt.md | 10 +- scripts/pre-commit.sh | 2 +- scripts/test-commit-msg.sh | 182 +++++++++++++++++++++++++++++++++ 7 files changed, 428 insertions(+), 13 deletions(-) create mode 100755 scripts/commit-msg.sh create mode 100755 scripts/test-commit-msg.sh diff --git a/.goreleaser.yml b/.goreleaser.yml index 926d8d4f..0a4b4413 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -30,10 +30,10 @@ changelog: sort: asc filters: exclude: - - "^docs:" - - "^test:" - - "^ci:" - - "^chore:" + - "^docs(\\([^)]+\\))?:" + - "^test(\\([^)]+\\))?:" + - "^ci(\\([^)]+\\))?:" + - "^chore(\\([^)]+\\))?:" # Note: Homebrew formula is manually maintained in marcus/homebrew-tap # to build from source (avoids Gatekeeper warnings on macOS) diff --git a/Makefile b/Makefile index 18da511f..28e7a27a 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help fmt test install tag release check-clean check-version install-hooks +.PHONY: help fmt test test-commit-msg install tag release check-clean check-version install-hooks SHELL := /bin/sh @@ -13,8 +13,9 @@ help: @printf "%s\n" \ "Targets:" \ " make fmt # gofmt -w ." \ - " make install-hooks # install git pre-commit hook" \ + " make install-hooks # install git pre-commit + commit-msg hooks" \ " make test # go test ./..." \ + " make test-commit-msg # run commit-msg hook regression tests" \ " make install # build and install with version from git" \ " make tag VERSION=vX.Y.Z # create annotated git tag (requires clean tree)" \ " make release VERSION=vX.Y.Z # tag + push (triggers GoReleaser via GitHub Actions)" @@ -25,6 +26,9 @@ fmt: test: go test ./... +test-commit-msg: + ./scripts/test-commit-msg.sh + install: @V="$(GIT_DESCRIBE)"; V=$${V:-dev}; \ echo "Installing td $$V"; \ @@ -52,6 +56,9 @@ release: tag git push origin "$(VERSION)" install-hooks: - @echo "Installing git pre-commit hook..." - @ln -sf ../../scripts/pre-commit.sh .git/hooks/pre-commit - @echo "Done. Hook installed at .git/hooks/pre-commit" + @hooks_dir="$$(git rev-parse --git-path hooks)"; \ + mkdir -p "$$hooks_dir"; \ + echo "Installing git hooks into $$hooks_dir..."; \ + install -m 0755 scripts/pre-commit.sh "$$hooks_dir/pre-commit"; \ + install -m 0755 scripts/commit-msg.sh "$$hooks_dir/commit-msg"; \ + echo "Done. Hooks installed at $$hooks_dir/pre-commit and $$hooks_dir/commit-msg" diff --git a/README.md b/README.md index 684416ad..56337c0c 100644 --- a/README.md +++ b/README.md @@ -189,16 +189,37 @@ make install-dev # Format code make fmt -# Install git pre-commit hook (gofmt, go vet, go build on staged files) +# Install git hooks (pre-commit checks + commit-msg normalizer) make install-hooks ``` +## Commit Messages + +New commits should use `type(scope)?: summary`. + +- Supported types: `feat`, `fix`, `docs`, `test`, `chore`, `ci`, `perf`, `refactor`, `style`, `release` +- Add `(td-)` for task work when you have one; it is recommended, not required for every commit +- `(#123)` PR references are also fine at the end of the subject +- Merge, revert, fixup, squash, and release/version flows are exempt from the local normalizer + +Accepted examples: + +```text +feat(cli): add commit message normalizer (td-a1b2c3) +fix(review): preserve Nightshift trailers (#91) +docs: document install-hooks behavior +release: v0.44.0 +``` + ## Tests & Quality Checks ```bash # Run all tests (114 tests across cmd/, internal/db/, internal/models/, etc.) make test +# Verify the commit-msg normalizer +make test-commit-msg + # Expected output: ok for each package, ~2s total runtime # Example: # ok github.com/marcus/td/cmd 1.994s diff --git a/scripts/commit-msg.sh b/scripts/commit-msg.sh new file mode 100755 index 00000000..d020888d --- /dev/null +++ b/scripts/commit-msg.sh @@ -0,0 +1,199 @@ +#!/usr/bin/env bash +# commit-msg hook for td +# Install: make install-hooks +set -euo pipefail + +MSG_FILE="${1:-}" +ALLOWED_TYPES_DISPLAY='feat, fix, docs, test, chore, ci, perf, refactor, style, release' + +if [[ -z "$MSG_FILE" || ! -f "$MSG_FILE" ]]; then + echo "Usage: $0 " >&2 + exit 1 +fi + +trim_and_collapse() { + printf '%s' "$1" | tr '\t' ' ' | sed -E 's/[[:space:]]+/ /g; s/^ //; s/ $//' +} + +lowercase() { + printf '%s' "$1" | tr '[:upper:]' '[:lower:]' +} + +is_allowed_type() { + case "$1" in + feat|fix|docs|test|chore|ci|perf|refactor|style|release) + return 0 + ;; + esac + + return 1 +} + +parse_subject() { + local subject="$1" + local pattern="$2" + + printf '%s\n' "$subject" | sed -En "s/$pattern/\\1|\\2|\\3/p" | head -n 1 +} + +is_exempt_subject() { + local subject="$1" + + case "$subject" in + Merge*|Merged*|merge*) + return 0 + ;; + Revert*|revert*) + return 0 + ;; + fixup!\ *|squash!\ *) + return 0 + ;; + release:*|release\(*\):*|Release:*|Release\(*\):*) + return 0 + ;; + Version*|Bump\ version*) + return 0 + ;; + esac + + printf '%s\n' "$subject" | grep -Eq '^v[0-9]+\.[0-9]+\.[0-9]+([.-][0-9A-Za-z]+)?$' && return 0 + + return 1 +} + +normalize_subject() { + local subject="$1" + local cleaned type scope summary + local with_colon_regex='^([A-Za-z]+)(\([^)]+\))?[[:space:]]*:[[:space:]]*(.+)$' + local scoped_missing_colon_regex='^([A-Za-z]+)(\([^)]+\))[[:space:]]+(.+)$' + local unscoped_missing_colon_regex='^([A-Za-z]+)[[:space:]]+(.+)$' + local malformed_scope_summary_regex='^\([^)]+\)(:|[[:space:]])' + + cleaned="$(trim_and_collapse "$subject")" + + if [[ "$cleaned" =~ $with_colon_regex ]]; then + type="$(lowercase "${BASH_REMATCH[1]}")" + scope="${BASH_REMATCH[2]}" + summary="${BASH_REMATCH[3]}" + if is_allowed_type "$type"; then + printf '%s' "$(trim_and_collapse "$type$scope: $summary")" + return 0 + fi + fi + + if [[ "$cleaned" =~ $scoped_missing_colon_regex ]]; then + type="$(lowercase "${BASH_REMATCH[1]}")" + scope="${BASH_REMATCH[2]}" + summary="${BASH_REMATCH[3]}" + if is_allowed_type "$type"; then + printf '%s' "$(trim_and_collapse "$type$scope: $summary")" + return 0 + fi + fi + + if [[ "$cleaned" =~ $unscoped_missing_colon_regex ]]; then + type="$(lowercase "${BASH_REMATCH[1]}")" + summary="$(trim_and_collapse "${BASH_REMATCH[2]}")" + if is_allowed_type "$type"; then + if [[ "$summary" =~ $malformed_scope_summary_regex ]]; then + printf '%s' "$cleaned" + return 0 + fi + printf '%s' "$(trim_and_collapse "$type: $summary")" + return 0 + fi + fi + + printf '%s' "$cleaned" +} + +strip_trailing_refs() { + local subject="$1" + local stripped + local trailing_ref_regex='^(.+)[[:space:]]+\((td-[[:alnum:]]+|#[0-9]+)\)$' + local ref_only_regex='^\((td-[[:alnum:]]+|#[0-9]+)\)$' + + stripped="$(trim_and_collapse "$subject")" + + while [[ "$stripped" =~ $trailing_ref_regex ]]; do + stripped="$(trim_and_collapse "${BASH_REMATCH[1]}")" + done + + if [[ "$stripped" =~ $ref_only_regex ]]; then + stripped="" + fi + + printf '%s' "$stripped" +} + +is_valid_subject() { + local subject="$1" + local parsed type scope summary + + [[ -n "$subject" ]] || return 1 + + parsed="$(parse_subject "$subject" '^([a-z]+)(\([^)]+\))?:[[:space:]]+(.+)$')" + if [[ -n "$parsed" ]]; then + IFS='|' read -r type scope summary <"$tmp_file" + mv "$tmp_file" "$MSG_FILE" +} + +subject="$(sed -n '1p' "$MSG_FILE")" +trimmed_subject="$(trim_and_collapse "$subject")" + +if is_exempt_subject "$trimmed_subject"; then + exit 0 +fi + +normalized_subject="$(normalize_subject "$subject")" + +if is_valid_subject "$normalized_subject"; then + if [[ "$normalized_subject" != "$subject" ]]; then + rewrite_subject_line "$normalized_subject" + echo "Normalized commit subject: $normalized_subject" + fi + exit 0 +fi + +cat >&2 <) and/or (#123) + +Examples: + feat(cli): add commit message normalizer (td-a1b2c3) + fix(review): preserve Nightshift trailers (#91) + docs: document install-hooks behavior + +Exempt flows: + Merge..., Revert..., fixup!, squash!, release/version commits + +Scopes must be attached directly to the type: + fix(cli): preserve trailers + not: fix (cli): preserve trailers + +To fix the last commit message: + git commit --amend +EOF +exit 1 diff --git a/scripts/loop-prompt.md b/scripts/loop-prompt.md index 7f84db2e..6ffb4ff4 100644 --- a/scripts/loop-prompt.md +++ b/scripts/loop-prompt.md @@ -158,12 +158,18 @@ Batch review loops: ```bash git add -git commit -m "feat: (td-)" +git commit -m "feat(cli): (td-)" td review ``` Use `td review`, not `td close` — self-closing is blocked. +Commit subjects should use `type(scope)?: summary`. + +- Include `(td-)` for task work when you have it; it is recommended, not required for every commit. +- `(#123)` references are also accepted at the end of the subject. +- Examples: `feat(cli): add JSON output (td-a1b2c3)`, `fix(review): preserve trailers (#91)`, `docs: clarify install-hooks behavior` + ## Rules - **ONE task per iteration.** Complete it, verify it, commit it, mark it done, then exit. @@ -174,4 +180,4 @@ Use `td review`, not `td close` — self-closing is blocked. - **Don't break sync.** Deterministic IDs, proper event logging, no hard deletes. - **Session isolation is sacred.** Don't bypass review guards. - **If stuck, log and skip.** `td log "Blocked: "` then `td block `. -- **Commit messages reference td.** Format: `feat|fix|chore: (td-)` +- **Commit messages follow `type(scope)?: summary`.** `(td-)` is recommended for task work, not required for every commit. diff --git a/scripts/pre-commit.sh b/scripts/pre-commit.sh index 2b563f26..b1ec33cf 100755 --- a/scripts/pre-commit.sh +++ b/scripts/pre-commit.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # pre-commit hook for td -# Install: make install-hooks (or: ln -sf ../../scripts/pre-commit.sh .git/hooks/pre-commit) +# Install: make install-hooks set -euo pipefail PASS=0 diff --git a/scripts/test-commit-msg.sh b/scripts/test-commit-msg.sh new file mode 100755 index 00000000..d4d916ce --- /dev/null +++ b/scripts/test-commit-msg.sh @@ -0,0 +1,182 @@ +#!/usr/bin/env bash +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HOOK="$SCRIPT_DIR/commit-msg.sh" + +PASS=0 +FAIL=0 + +cleanup() { + if [[ -n "${TEST_TMPDIR:-}" && -d "${TEST_TMPDIR:-}" ]]; then + rm -rf "$TEST_TMPDIR" + fi +} + +trap cleanup EXIT + +TEST_TMPDIR="$(mktemp -d "${TMPDIR:-/tmp}/td-commit-msg-tests-XXXXXX")" + +run_hook() { + local input="$1" + local message_file output_file status + + message_file="$(mktemp "$TEST_TMPDIR/message-XXXXXX")" + output_file="$(mktemp "$TEST_TMPDIR/output-XXXXXX")" + + printf '%s' "$input" >"$message_file" + + if "$HOOK" "$message_file" >"$output_file" 2>&1; then + status=0 + else + status=$? + fi + + LAST_STATUS="$status" + LAST_MESSAGE_FILE="$message_file" + LAST_OUTPUT="$(cat "$output_file")" +} + +pass_case() { + local name="$1" + echo "ok $name" + PASS=$((PASS + 1)) +} + +fail_case() { + local name="$1" + local details="$2" + echo "not ok $name" + echo "$details" | sed 's/^/ /' + FAIL=$((FAIL + 1)) +} + +expect_pass() { + local name="$1" + local input="$2" + local expected_message="$3" + local expected_file diff_output + + run_hook "$input" + + if [[ "$LAST_STATUS" -ne 0 ]]; then + fail_case "$name" "expected success, got exit $LAST_STATUS\n$LAST_OUTPUT" + return + fi + + expected_file="$(mktemp "$TEST_TMPDIR/expected-XXXXXX")" + printf '%s' "$expected_message" >"$expected_file" + + if ! cmp -s "$expected_file" "$LAST_MESSAGE_FILE"; then + diff_output="$(diff -u "$expected_file" "$LAST_MESSAGE_FILE" || true)" + fail_case "$name" "message mismatch\n$diff_output" + return + fi + + pass_case "$name" +} + +expect_reject() { + local name="$1" + local input="$2" + local expected_output_fragment="$3" + + run_hook "$input" + + if [[ "$LAST_STATUS" -eq 0 ]]; then + fail_case "$name" "expected rejection, but hook succeeded" + return + fi + + if [[ "$LAST_OUTPUT" != *"$expected_output_fragment"* ]]; then + fail_case "$name" "expected output to contain: $expected_output_fragment\nactual:\n$LAST_OUTPUT" + return + fi + + pass_case "$name" +} + +expect_pass "valid pass-through subject" \ + $'fix: keep commit bodies unchanged\n\nNightshift-Task: commit-normalize\n' \ + $'fix: keep commit bodies unchanged\n\nNightshift-Task: commit-normalize\n' + +expect_pass "normalizes capitalization and missing colon" \ + $'Fix add commit message hook\n' \ + $'fix: add commit message hook\n' + +expect_pass "normalizes scoped subject missing colon" \ + $'Fix(cli) add commit message hook\n' \ + $'fix(cli): add commit message hook\n' + +expect_pass "normalizes scoped subject and whitespace" \ + $'Docs(api) : add usage examples (#12)\n' \ + $'docs(api): add usage examples (#12)\n' + +expect_pass "accepts scoped commit with td and PR suffixes" \ + $'feat(cli): add commit normalizer (td-a1b2c3) (#91)\n' \ + $'feat(cli): add commit normalizer (td-a1b2c3) (#91)\n' + +expect_pass "exempts merge commits" \ + $'Merge pull request #91 from marcus/dispatch/td-527bd4-0006\n' \ + $'Merge pull request #91 from marcus/dispatch/td-527bd4-0006\n' + +expect_pass "exempts revert commits" \ + $'Revert "feat(sync): add encryption tables and key_id column"\n\nThis reverts commit deadbeef.\n' \ + $'Revert "feat(sync): add encryption tables and key_id column"\n\nThis reverts commit deadbeef.\n' + +expect_pass "exempts fixup commits" \ + $'fixup! feat(cli): add commit normalizer\n' \ + $'fixup! feat(cli): add commit normalizer\n' + +expect_pass "exempts squash commits" \ + $'squash! fix(cli): preserve trailers\n' \ + $'squash! fix(cli): preserve trailers\n' + +expect_pass "exempts release subjects" \ + $'release: v0.44.0\n' \ + $'release: v0.44.0\n' + +expect_pass "exempts bare version subjects" \ + $'v0.44.0\n' \ + $'v0.44.0\n' + +expect_pass "preserves body and trailers when normalizing" \ + $'Test(sync) preserve trailers (td-a1b2c3)\n\nBody line\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n' \ + $'test(sync): preserve trailers (td-a1b2c3)\n\nBody line\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n' + +expect_reject "rejects unknown subjects" \ + $'Add pre-commit hook and make install-hooks target\n' \ + 'type(scope)?: summary' + +expect_reject "rejects malformed spaced scope with colon" \ + $'fix (cli): broken parser\n' \ + 'fix(cli): preserve trailers' + +expect_reject "rejects malformed spaced scope without colon" \ + $'docs (api) typo fix\n' \ + 'fix(cli): preserve trailers' + +expect_reject "rejects missing summary" \ + $'fix:\n' \ + 'Examples:' + +expect_reject "rejects pr-only summary" \ + $'docs(scope): (#12)\n' \ + 'Examples:' + +expect_reject "rejects td-only summary" \ + $'docs(scope): (td-a1b2c3)\n' \ + 'Examples:' + +expect_reject "rejects ref-only summary chain" \ + $'docs(scope): (td-a1b2c3) (#12)\n' \ + 'Examples:' + +if [[ "$FAIL" -gt 0 ]]; then + echo "" + echo "Failed $FAIL commit-msg hook test(s); passed $PASS." + exit 1 +fi + +echo "" +echo "Passed $PASS commit-msg hook tests."