From 064636723a4e511edc899ed8a9c3feeb45a17fb5 Mon Sep 17 00:00:00 2001 From: Greg Gardner Date: Sat, 22 Aug 2026 02:08:05 -0700 Subject: [PATCH 1/2] docs: standardize commit message format Codify Conventional Commits as the repository standard: a convention guide, a commit template, a commit-msg hook validator, a range validator reused by a commit-lint CI job, and shell tests for the validator. Enforcement covers new commits only. Existing history is not rewritten, so merged pull requests and released tags keep their current SHAs. Agent prompts now request a conforming subject while preserving the Nightshift-Task and Nightshift-Ref trailers. Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- .github/workflows/ci.yml | 21 +++ .gitmessage.txt | 18 +++ AGENTS.md | 4 + CHANGELOG.md | 11 ++ Makefile | 20 ++- README.md | 38 +++++- docs/guides/commit-messages.md | 118 +++++++++++++++++ internal/orchestrator/orchestrator.go | 10 +- internal/orchestrator/orchestrator_test.go | 37 ++++++ scripts/check-commit-range.sh | 83 ++++++++++++ scripts/commit-msg.sh | 141 +++++++++++++++++++++ scripts/commit-msg_test.sh | 121 ++++++++++++++++++ 12 files changed, 614 insertions(+), 8 deletions(-) create mode 100644 .gitmessage.txt create mode 100644 docs/guides/commit-messages.md create mode 100755 scripts/check-commit-range.sh create mode 100755 scripts/commit-msg.sh create mode 100755 scripts/commit-msg_test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8fc5686..018e8fa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,3 +50,24 @@ jobs: uses: golangci/golangci-lint-action@v6 with: version: latest + + commit-lint: + name: Commit messages + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Validate pull request commit messages + env: + BASE_REF: ${{ github.event.pull_request.base.ref }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + git fetch --no-tags origin "$BASE_REF" + # Only the commits unique to this pull request are checked; history + # already on the base branch is deliberately left alone. + base=$(git merge-base "origin/$BASE_REF" "$HEAD_SHA") + ./scripts/check-commit-range.sh --base "$base" "$HEAD_SHA" diff --git a/.gitmessage.txt b/.gitmessage.txt new file mode 100644 index 0000000..489edd7 --- /dev/null +++ b/.gitmessage.txt @@ -0,0 +1,18 @@ +# (): +# +# Subject rules: +# - one of: feat fix docs refactor test chore build ci perf style revert +# - lowercase, imperative mood ("add", not "adds" or "Added") +# - 72 characters or fewer, no trailing period +# - add ! before the colon for a breaking change: feat(config)!: ... +# +# Body (optional): explain what and why, not how. Wrap at 72 columns. +# Leave a blank line between the subject and the body. +# +# Footers (optional): +# fixes #123 +# BREAKING CHANGE: describe the migration +# Nightshift-Task: (agent-authored commits) +# Nightshift-Ref: https://github.com/marcus/nightshift +# +# Full convention: docs/guides/commit-messages.md diff --git a/AGENTS.md b/AGENTS.md index 1fd3ae1..94e23f2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,3 +54,7 @@ go test ./... - **Style**: Standard Go (gofmt, govet). No magic, explicit is better. - **Errors**: Wrap with context, don't swallow. - **Tests**: Table-driven, in `_test.go` files alongside code. +- **Commits**: Conventional Commits — `(): `, + 72 chars or fewer, no trailing period. See [docs/guides/commit-messages.md](docs/guides/commit-messages.md). + Agent-authored commits keep their `Nightshift-Task:` and `Nightshift-Ref:` trailers. + Enforced by the `commit-msg` hook (`make install-hooks`) and CI for pull request commits. diff --git a/CHANGELOG.md b/CHANGELOG.md index 59afa7d..d20b2ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ All notable changes to nightshift are documented in this file. +## [Unreleased] + +### Other +- **Commit message convention** — commits now follow Conventional Commits, documented in + [docs/guides/commit-messages.md](docs/guides/commit-messages.md). A `commit-msg` hook + (installed by `make install-hooks`, alongside a `.gitmessage.txt` template) validates + messages locally, and a `commit-lint` CI job validates the commits in a pull request. + Existing history is not rewritten — enforcement applies to new commits only. Agent + prompts now request conforming subjects while keeping the `Nightshift-Task` and + `Nightshift-Ref` trailers. + ## [v0.3.3] - 2026-02-19 ### Features diff --git a/Makefile b/Makefile index 088be01..773c8c3 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build test test-verbose test-race coverage coverage-html lint clean deps check install calibrate-providers install-hooks help +.PHONY: build test test-scripts test-verbose test-race coverage coverage-html lint lint-commits clean deps check install calibrate-providers install-hooks help # Binary name BINARY=nightshift @@ -21,6 +21,10 @@ calibrate-providers: test: go test ./... +# Run shell script test suites +test-scripts: + @./scripts/commit-msg_test.sh + # Run tests with verbose output test-verbose: go test -v ./... @@ -46,6 +50,10 @@ lint: @which golangci-lint > /dev/null || (echo "golangci-lint not installed. Run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest" && exit 1) golangci-lint run +# Validate commit messages on this branch against the default branch +lint-commits: + @./scripts/check-commit-range.sh --base origin/main + # Clean build artifacts clean: rm -f $(BINARY) @@ -65,20 +73,26 @@ help: @echo "Available targets:" @echo " build - Build the binary" @echo " test - Run all tests" + @echo " test-scripts - Run shell script test suites" @echo " test-verbose - Run tests with verbose output" @echo " test-race - Run tests with race detection" @echo " coverage - Run tests with coverage report" @echo " coverage-html - Generate HTML coverage report" @echo " lint - Run golangci-lint" + @echo " lint-commits - Check commit messages against origin/main" @echo " clean - Clean build artifacts" @echo " deps - Download and tidy dependencies" @echo " check - Run tests and lint" @echo " install - Build and install to Go bin directory" @echo " calibrate-providers - Compare local Claude/Codex session usage for calibration" - @echo " install-hooks - Install git pre-commit hook" + @echo " install-hooks - Install git hooks and commit message template" @echo " help - Show this help" -# Install git pre-commit hook +# Install git hooks and the commit message template install-hooks: @ln -sf ../../scripts/pre-commit.sh .git/hooks/pre-commit @echo "✓ pre-commit hook installed (.git/hooks/pre-commit → scripts/pre-commit.sh)" + @ln -sf ../../scripts/commit-msg.sh .git/hooks/commit-msg + @echo "✓ commit-msg hook installed (.git/hooks/commit-msg → scripts/commit-msg.sh)" + @git config commit.template .gitmessage.txt + @echo "✓ commit template set (commit.template → .gitmessage.txt)" diff --git a/README.md b/README.md index 84f92cd..5d4b1a6 100644 --- a/README.md +++ b/README.md @@ -258,21 +258,53 @@ Each task has a default cooldown interval to prevent the same task from running ## Development -### Pre-commit hooks +### Git hooks -Install the git pre-commit hook to catch formatting and vet issues before pushing: +Install the git hooks and the commit message template: ```bash make install-hooks ``` -This symlinks `scripts/pre-commit.sh` into `.git/hooks/pre-commit`. The hook runs: +This symlinks `scripts/pre-commit.sh` into `.git/hooks/pre-commit` and +`scripts/commit-msg.sh` into `.git/hooks/commit-msg`, and points `commit.template` at +`.gitmessage.txt`. + +The pre-commit hook runs: - **gofmt** — flags any staged `.go` files that need formatting - **go vet** — catches common correctness issues - **go build** — ensures the project compiles +The commit-msg hook validates the commit message against the convention below. + To bypass in a pinch: `git commit --no-verify` +### Commit messages + +Commits follow [Conventional Commits](https://www.conventionalcommits.org/): + +``` +(): +``` + +Subjects are 72 characters or fewer with no trailing period, and the type is one of +`feat`, `fix`, `docs`, `refactor`, `test`, `chore`, `build`, `ci`, `perf`, `style`, or +`revert`. Commits authored by a nightshift run also carry `Nightshift-Task:` and +`Nightshift-Ref:` trailers. + +See [docs/guides/commit-messages.md](docs/guides/commit-messages.md) for the full +convention, including examples and the rules the validator applies. + +Check your branch before opening a pull request: + +```bash +make lint-commits # validates commits against origin/main +make test-scripts # runs the validator's own test suite +``` + +Existing history is left as-is — enforcement applies to new commits only. CI validates +the commits in a pull request, not the commits already on `main`. + ## Uninstalling ```bash diff --git a/docs/guides/commit-messages.md b/docs/guides/commit-messages.md new file mode 100644 index 0000000..7615cb9 --- /dev/null +++ b/docs/guides/commit-messages.md @@ -0,0 +1,118 @@ +# Commit message convention + +nightshift uses [Conventional Commits](https://www.conventionalcommits.org/). Roughly +three quarters of the existing history already follows this shape — this guide makes it +explicit and enforceable for new work. + +**History is not rewritten.** Enforcement applies to new commits only: the `commit-msg` +hook checks what you write locally, and the `commit-lint` CI job checks the commits in a +pull request. Commits already on `main` are left alone. + +## Format + +``` +(): + + + + +``` + +### Subject + +| Rule | Detail | +| --- | --- | +| Type | One of the types below, lowercase | +| Scope | Optional, lowercase, in parentheses — a package or area (`orchestrator`, `internal/tasks`, `#19`) | +| Breaking | Append `!` before the colon (`feat(config)!: ...`) | +| Separator | A colon and a single space | +| Mood | Imperative — "add", not "adds", "added", or "Add" | +| Case | Starts lowercase; acronyms such as `JSONL` or `PR` are fine | +| Length | 72 characters or fewer, including the type and scope | +| Punctuation | No trailing period | + +### Types + +| Type | Use for | +| --- | --- | +| `feat` | A new user-facing capability | +| `fix` | A bug fix | +| `docs` | Documentation only | +| `refactor` | A change that neither fixes a bug nor adds a feature | +| `test` | Adding or correcting tests | +| `chore` | Maintenance that does not fit elsewhere (version bumps, housekeeping) | +| `build` | Build system, `go.mod`, release packaging | +| `ci` | CI configuration and workflows | +| `perf` | A performance improvement | +| `style` | Formatting only, no behavior change (`gofmt`) | +| `revert` | Reverting a previous commit | + +### Body + +Optional. Separate it from the subject with a blank line and wrap at 72 columns. +Explain *what* changed and *why*; the diff already shows *how*. + +### Footers + +``` +fixes #21 +BREAKING CHANGE: budget.daily is now budget.per_day +``` + +Agent-authored commits produced by a nightshift run must keep their trailers, which the +validator accepts as footers: + +``` +Nightshift-Task: commit-normalize +Nightshift-Ref: https://github.com/marcus/nightshift +``` + +## Examples + +Good — all drawn from the existing history: + +``` +feat(orchestrator): implement plan-implement-review loop (td-d50819) +fix: use week-aware avg daily usage in budget projection (td-f08410) +fix(providers): compute billable tokens excluding cached input +test(commands): add task CLI unit tests (td-190775) +refactor: replace WriteString(fmt.Sprintf) with fmt.Fprintf (#41) +fix: serialize provider config with correct YAML key names (fixes #20) (#43) +``` + +Avoid — also drawn from the existing history, with the conforming rewrite: + +| Instead of | Write | +| --- | --- | +| `Bump version to v0.3.4` | `chore: bump version to v0.3.4` | +| `Add pre-commit hook for gofmt/vet/build (#44)` | `build: add pre-commit hook for gofmt/vet/build (#44)` | +| `Fix gofmt formatting issues across codebase` | `style: fix gofmt formatting across codebase` | +| `Update readme` | `docs: update readme` | +| `fix(task)` | `fix(task): guard nil task lookup` | +| `Initial plan` | `chore: add initial plan` | + +Merge, revert, `fixup!`, and `squash!` subjects that git writes itself are passed +through unchanged. + +## Tooling + +Install the hooks and the message template once per clone: + +```sh +make install-hooks +``` + +That symlinks `.git/hooks/pre-commit` and `.git/hooks/commit-msg` to the scripts in +`scripts/`, and points `commit.template` at `.gitmessage.txt` so `git commit` opens with +the format inline. + +| Command | Does | +| --- | --- | +| `make install-hooks` | Install both hooks and the commit template | +| `make test-scripts` | Run the validator's own test suite | +| `make lint-commits` | Validate commits on your branch against `origin/main` | +| `scripts/commit-msg.sh ` | Validate a single message file | +| `scripts/check-commit-range.sh --base []` | Validate a range | + +If the hook rejects a message you are confident about, `git commit --no-verify` bypasses +it. CI still checks the commits in the pull request. diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 6141c95..2266b10 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -728,7 +728,10 @@ Description: %s 0. You are running autonomously. If the task is broad or ambiguous, choose a concrete, minimal scope that delivers value and state any assumptions in the description. 1. Work on a new branch and plan to submit a PR. Never work directly on the primary branch.%s 2. Before creating your branch, record the current branch name and plan to switch back after the PR is opened. -3. If you create commits, include a concise message with these git trailers: +3. If you create commits, use a Conventional Commits subject — + (): — 72 characters or + fewer, no trailing period, where type is one of feat, fix, docs, refactor, + test, chore, build, ci, perf, style, revert. Include these git trailers: Nightshift-Task: %s Nightshift-Ref: https://github.com/marcus/nightshift 4. Analyze the task requirements @@ -771,7 +774,10 @@ Description: %s ## Instructions 0. Before creating your branch, record the current branch name. Create and work on a new branch. Never modify or commit directly to the primary branch.%s When finished, open a PR. After the PR is submitted, switch back to the original branch. If you cannot open a PR, leave the branch and explain next steps. -1. If you create commits, include a concise message with these git trailers: +1. If you create commits, use a Conventional Commits subject — + (): — 72 characters or + fewer, no trailing period, where type is one of feat, fix, docs, refactor, + test, chore, build, ci, perf, style, revert. Include these git trailers: Nightshift-Task: %s Nightshift-Ref: https://github.com/marcus/nightshift 2. Implement the plan step by step diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index 45bff5c..8e8e27b 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -930,3 +930,40 @@ func TestRunTaskNoPRURL(t *testing.T) { t.Errorf("OutputRef = %q, want empty", result.OutputRef) } } + +func TestBuildPrompts_CommitConvention(t *testing.T) { + o := New() + + task := &tasks.Task{ + ID: "commit-convention", + Title: "Commit Convention", + Description: "Test commit instructions", + Type: tasks.TaskType("commit-normalize"), + } + plan := &PlanOutput{ + Steps: []string{"step1"}, + Description: "test plan", + } + + // Both prompts must state the Conventional Commits subject format and keep + // the Nightshift trailers, so agent-authored commits pass the commit-msg + // hook. See docs/guides/commit-messages.md. + want := []string{ + "Conventional Commits subject", + "(): ", + "feat, fix, docs, refactor", + "Nightshift-Task: commit-normalize", + "Nightshift-Ref: https://github.com/marcus/nightshift", + } + + for _, prompt := range map[string]string{ + "plan": o.buildPlanPrompt(task), + "implement": o.buildImplementPrompt(task, plan, 1), + } { + for _, substr := range want { + if !strings.Contains(prompt, substr) { + t.Errorf("prompt missing %q\nGot:\n%s", substr, prompt) + } + } + } +} diff --git a/scripts/check-commit-range.sh b/scripts/check-commit-range.sh new file mode 100755 index 0000000..dc2baae --- /dev/null +++ b/scripts/check-commit-range.sh @@ -0,0 +1,83 @@ +#!/bin/sh +# Validate every commit in a range against the nightshift commit convention. +# +# Usage: scripts/check-commit-range.sh [--base ] [] +# +# Validates .., defaulting to origin/main..HEAD. Used by the +# commit-lint CI job so pull request commits are checked without touching +# historical commits already on the default branch. +set -eu + +script_dir=$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd) +validator="$script_dir/commit-msg.sh" + +base="origin/main" +head="HEAD" + +while [ "$#" -gt 0 ]; do + case "$1" in + --base) + if [ "$#" -lt 2 ]; then + echo "check-commit-range: --base requires a value" >&2 + exit 2 + fi + base=$2 + shift 2 + ;; + --base=*) + base=${1#--base=} + shift + ;; + -h | --help) + echo "usage: $0 [--base ] []" + exit 0 + ;; + -*) + echo "check-commit-range: unknown option: $1" >&2 + exit 2 + ;; + *) + head=$1 + shift + ;; + esac +done + +if [ ! -x "$validator" ]; then + echo "check-commit-range: validator not executable: $validator" >&2 + exit 2 +fi + +# Only inspect commits unique to ; anything reachable from is +# existing history and deliberately left alone. +revs=$(git rev-list --no-merges "$base".."$head") + +if [ -z "$revs" ]; then + echo "✓ no commits to check in $base..$head" + exit 0 +fi + +tmp_file=$(mktemp "${TMPDIR:-/tmp}/nightshift-commit-range.XXXXXX") +cleanup() { rm -f "$tmp_file"; } +trap cleanup EXIT HUP INT TERM + +checked=0 +failed=0 + +for rev in $revs; do + checked=$((checked + 1)) + git log -1 --format=%B "$rev" >"$tmp_file" + if ! "$validator" "$tmp_file"; then + echo " ↳ commit $(git log -1 --format='%h' "$rev")" >&2 + echo "" >&2 + failed=$((failed + 1)) + fi +done + +if [ "$failed" -gt 0 ]; then + echo "✗ $failed of $checked commit(s) do not follow the commit convention" >&2 + echo " see docs/guides/commit-messages.md" >&2 + exit 1 +fi + +echo "✓ all $checked commit(s) follow the commit convention" diff --git a/scripts/commit-msg.sh b/scripts/commit-msg.sh new file mode 100755 index 0000000..88fb2cb --- /dev/null +++ b/scripts/commit-msg.sh @@ -0,0 +1,141 @@ +#!/bin/sh +# Validate a commit message against the nightshift commit convention. +# +# Usage: scripts/commit-msg.sh +# +# Installed as the `commit-msg` git hook by `make install-hooks`. The same +# validation is reused over a commit range by scripts/check-commit-range.sh. +# +# See docs/guides/commit-messages.md for the full convention. +set -eu + +DOC_URL="docs/guides/commit-messages.md" +TYPES='feat|fix|docs|refactor|test|chore|build|ci|perf|style|revert' +MAX_SUBJECT=72 +MAX_BODY=100 + +usage() { + echo "usage: $0 " >&2 +} + +if [ "$#" -ne 1 ]; then + usage + exit 2 +fi + +message_file=$1 +if [ ! -r "$message_file" ]; then + echo "commit-msg: not a readable file: $message_file" >&2 + exit 2 +fi + +# Git may be configured to use a comment character other than '#'. +comment_char=$(git config --get core.commentChar 2>/dev/null || true) +case "$comment_char" in +"" | auto) comment_char='#' ;; +esac + +# Strip comment lines and the scissors section git adds under --verbose. +stripped=$( + awk -v c="$comment_char" ' + index($0, "------------------------ >8 ------------------------") { exit } + substr($0, 1, length(c)) == c { next } + { sub(/\r$/, ""); print } + ' "$message_file" +) + +# The subject is the first non-blank line. +subject=$(printf '%s\n' "$stripped" | awk 'NF { print; exit }') + +if [ -z "$subject" ]; then + echo "commit-msg: aborting due to empty commit message" >&2 + exit 1 +fi + +# Git authors these subjects itself. Rejecting them would break merges, +# reverts, and rebase autosquash, so let them through untouched. +case "$subject" in +"Merge "* | "Revert "* | "fixup!"* | "squash!"* | "amend!"* | "WIP on "* | "index on "*) + exit 0 + ;; +esac + +errors="" +add_error() { + errors="${errors} - $1 +" +} + +# --- subject: structure ----------------------------------------------------- +if ! printf '%s' "$subject" | grep -qE "^($TYPES)(\([a-z0-9._/#-]+\))?!?: .+"; then + case "$subject" in + *": "*) + add_error "subject type is not one of: $(printf '%s' "$TYPES" | tr '|' ' ')" + ;; + *) + add_error "subject is missing the ': ' prefix" + ;; + esac +else + description=${subject#*: } + + # --- subject: length --- + length=$(printf '%s' "$subject" | wc -c | tr -d ' ') + if [ "$length" -gt "$MAX_SUBJECT" ]; then + add_error "subject is $length characters; keep it to $MAX_SUBJECT or fewer" + fi + + # --- subject: no trailing period --- + case "$description" in + *.) + add_error "subject must not end with a period" + ;; + esac + + # --- subject: imperative, lowercase start --- + # Reject "Add"/"Fix" style capitalization but allow acronyms (JSONL, PR, API). + if printf '%s' "$description" | grep -qE '^[A-Z][a-z]'; then + add_error "subject must start lowercase and be imperative (e.g. 'add', not 'Adds')" + fi +fi + +# --- body: blank line after subject ---------------------------------------- +subject_line=$(printf '%s\n' "$stripped" | awk 'NF { print NR; exit }') +next_line=$(printf '%s\n' "$stripped" | awk -v s="$subject_line" 'NR == s + 1 { print }') +has_more=$(printf '%s\n' "$stripped" | awk -v s="$subject_line" 'NR > s + 1 && NF { print "yes"; exit }') + +if [ -n "$next_line" ]; then + add_error "separate the subject from the body with a blank line" +elif [ -n "$has_more" ]; then + # --- body: line length --- + long=$( + printf '%s\n' "$stripped" | awk -v s="$subject_line" -v max="$MAX_BODY" ' + NR <= s + 1 { next } + /^[A-Za-z][A-Za-z0-9-]*: / { next } # trailers (Nightshift-Task:, etc.) + $0 ~ /^[[:space:]]*$/ { next } + NF == 1 { next } # unbreakable tokens such as URLs + length($0) > max { print NR; exit } + ' + ) + if [ -n "$long" ]; then + add_error "body line $long exceeds $MAX_BODY characters; wrap the body at 72" + fi +fi + +if [ -n "$errors" ]; then + { + echo "commit-msg: rejected commit message" + echo "" + echo " subject: $subject" + echo "" + printf '%s' "$errors" + echo "" + echo " expected: (): " + echo " see $DOC_URL" + echo "" + echo " bypass with --no-verify if you are certain." + } >&2 + exit 1 +fi + +exit 0 diff --git a/scripts/commit-msg_test.sh b/scripts/commit-msg_test.sh new file mode 100755 index 0000000..9527efa --- /dev/null +++ b/scripts/commit-msg_test.sh @@ -0,0 +1,121 @@ +#!/bin/sh +# Table tests for scripts/commit-msg.sh. +# +# Usage: make test-scripts (or: ./scripts/commit-msg_test.sh) +set -eu + +script_dir=$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd) +validator="$script_dir/commit-msg.sh" + +tmp_dir=$(mktemp -d "${TMPDIR:-/tmp}/nightshift-commit-msg-test.XXXXXX") +cleanup() { rm -rf "$tmp_dir"; } +trap cleanup EXIT HUP INT TERM + +pass=0 +fail=0 + +# check +check() { + expected=$1 + name=$2 + message=$3 + + file="$tmp_dir/msg" + printf '%s\n' "$message" >"$file" + + set +e + output=$("$validator" "$file" 2>&1) + actual=$? + set -e + + if [ "$actual" -eq "$expected" ]; then + pass=$((pass + 1)) + printf ' ✓ %s\n' "$name" + else + fail=$((fail + 1)) + printf ' ✗ %s (want exit %s, got %s)\n' "$name" "$expected" "$actual" + printf '%s\n' "$output" | sed 's/^/ /' + fi +} + +echo "commit-msg validator" + +# --- accepted: one per allowed type --- +for type in feat fix docs refactor test chore build ci perf style revert; do + check 0 "accepts type $type" "$type: add a thing" +done + +# --- accepted: shapes --- +check 0 "accepts scope" "feat(orchestrator): add plan retry" +check 0 "accepts scope with slash" "fix(internal/tasks): guard nil task" +check 0 "accepts issue-style scope" "fix(#19): honor max_projects" +check 0 "accepts breaking-change marker" "feat(config)!: drop legacy provider keys" +check 0 "accepts acronym subject" "fix: JSONL parsing reads nested usage" +check 0 "accepts subject at 72 chars" "feat: $(printf 'a%.0s' $(seq 1 66))" +check 0 "accepts body after blank line" "feat: add thing + +Explains why the thing was added." +check 0 "accepts breaking-change footer" "feat(api)!: rename budget field + +BREAKING CHANGE: budget.daily is now budget.per_day." +check 0 "accepts issue reference" "fix: correct budget rounding + +fixes #21" +check 0 "accepts nightshift trailers" "docs: standardize commit message format + +Nightshift-Task: commit-normalize +Nightshift-Ref: https://github.com/marcus/nightshift" +check 0 "accepts long URL in body" "docs: add reference + +https://example.com/$(printf 'a%.0s' $(seq 1 120))" + +# --- accepted: git-authored subjects pass through --- +check 0 "passes through merge commit" "Merge pull request #17 from user/branch" +check 0 "passes through revert commit" "Revert \"feat: add thing\"" +check 0 "passes through fixup" "fixup! feat: add thing" +check 0 "passes through squash" "squash! feat: add thing" + +# --- accepted: comments are stripped --- +check 0 "ignores comment lines" "feat: add thing +# Please enter the commit message for your changes." + +# --- rejected --- +check 1 "rejects unknown type" "feature: add thing" +check 1 "rejects missing colon" "feat add thing" +check 1 "rejects bare type without subject" "fix(task)" +check 1 "rejects capitalized subject" "feat: Add thing" +check 1 "rejects non-conventional capitalized subject" "Bump version to v0.3.4" +check 1 "rejects trailing period" "feat: add thing." +check 1 "rejects over-length subject" "feat: $(printf 'a%.0s' $(seq 1 100))" +check 1 "rejects missing blank line before body" "feat: add thing +body starts immediately" +check 1 "rejects empty message" "" +check 1 "rejects comment-only message" "# nothing here" +check 1 "rejects over-length body line" "feat: add thing + +$(printf 'word %.0s' $(seq 1 40))" + +# --- usage errors --- +set +e +"$validator" >/dev/null 2>&1 +usage_status=$? +"$validator" "$tmp_dir/does-not-exist" >/dev/null 2>&1 +missing_status=$? +set -e +if [ "$usage_status" -eq 2 ]; then + pass=$((pass + 1)); printf ' ✓ %s\n' "exits 2 without arguments" +else + fail=$((fail + 1)); printf ' ✗ %s (want 2, got %s)\n' "exits 2 without arguments" "$usage_status" +fi +if [ "$missing_status" -eq 2 ]; then + pass=$((pass + 1)); printf ' ✓ %s\n' "exits 2 on missing file" +else + fail=$((fail + 1)); printf ' ✗ %s (want 2, got %s)\n' "exits 2 on missing file" "$missing_status" +fi + +echo "" +if [ "$fail" -gt 0 ]; then + echo "❌ $fail failed, $pass passed" + exit 1 +fi +echo "✅ all $pass checks passed" From fa7b59248a54a66b55a06865dba813e218d224e1 Mon Sep 17 00:00:00 2001 From: Greg Gardner Date: Sat, 22 Aug 2026 02:18:55 -0700 Subject: [PATCH 2/2] fix(scripts): measure commit message length in characters The commit-msg hook sized subjects with `wc -c` and body lines with awk's length(), both of which count bytes. A 71-character subject containing accents, an em-dash, smart quotes, or emoji was rejected as over-long, so the effective limit for non-ASCII text was well under 72. False rejections in a hook that blocks every commit push people toward --no-verify, which erodes the convention this hook exists to enforce. Measure with `wc -m` under a UTF-8 locale instead, probing for a locale the machine actually has. macOS awk reports length() in bytes even in a UTF-8 locale, so awk cannot be used for this. Body-length errors also reported awk's line number from the internally stripped message. git does not strip the comment block before running commit-msg, and `make install-hooks` installs an 18-line comment template, so "body line 3" pointed at line 21 of the author's editor buffer. Tag each line with its original number while stripping and report that. Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- docs/guides/commit-messages.md | 6 ++- scripts/commit-msg.sh | 84 +++++++++++++++++++++++++++------- scripts/commit-msg_test.sh | 53 +++++++++++++++++++++ 3 files changed, 126 insertions(+), 17 deletions(-) diff --git a/docs/guides/commit-messages.md b/docs/guides/commit-messages.md index 7615cb9..0d07f2e 100644 --- a/docs/guides/commit-messages.md +++ b/docs/guides/commit-messages.md @@ -28,7 +28,7 @@ pull request. Commits already on `main` are left alone. | Separator | A colon and a single space | | Mood | Imperative — "add", not "adds", "added", or "Add" | | Case | Starts lowercase; acronyms such as `JSONL` or `PR` are fine | -| Length | 72 characters or fewer, including the type and scope | +| Length | 72 characters or fewer, including the type and scope (characters, not bytes — accents and emoji count as one) | | Punctuation | No trailing period | ### Types @@ -52,6 +52,10 @@ pull request. Commits already on `main` are left alone. Optional. Separate it from the subject with a blank line and wrap at 72 columns. Explain *what* changed and *why*; the diff already shows *how*. +The hook wraps at 72 by convention but only rejects body lines longer than 100 +characters, so prose you deliberately keep on one line is not blocked. Trailers +and unbreakable single tokens such as URLs are exempt from the limit entirely. + ### Footers ``` diff --git a/scripts/commit-msg.sh b/scripts/commit-msg.sh index 88fb2cb..4d2dbd0 100755 --- a/scripts/commit-msg.sh +++ b/scripts/commit-msg.sh @@ -14,6 +14,31 @@ TYPES='feat|fix|docs|refactor|test|chore|build|ci|perf|style|revert' MAX_SUBJECT=72 MAX_BODY=100 +# Limits are character counts, not byte counts: a subject with accents, an +# em-dash, smart quotes, or emoji must not get a smaller budget than an ASCII +# one. macOS awk measures length() in bytes even under a UTF-8 locale, so use +# `wc -m`, which is multibyte-aware as long as the locale is. Probe for a UTF-8 +# locale this machine actually has rather than assuming a particular name. +utf8_locale="" +for candidate in "${LC_ALL:-}" "${LC_CTYPE:-}" "${LANG:-}" C.UTF-8 en_US.UTF-8; do + [ -n "$candidate" ] || continue + # U+00E9 is two bytes in UTF-8; a locale that reports 1 counts characters. + if [ "$(printf '\303\251' | LC_ALL="$candidate" wc -m 2>/dev/null | tr -d ' ')" = "1" ]; then + utf8_locale=$candidate + break + fi +done + +# char_len -- length in characters, falling back to bytes only when no +# UTF-8 locale is available at all. +char_len() { + if [ -n "$utf8_locale" ]; then + printf '%s' "$1" | LC_ALL="$utf8_locale" wc -m | tr -d ' ' + else + printf '%s' "$1" | wc -c | tr -d ' ' + fi +} + usage() { echo "usage: $0 " >&2 } @@ -35,14 +60,20 @@ case "$comment_char" in "" | auto) comment_char='#' ;; esac -# Strip comment lines and the scissors section git adds under --verbose. -stripped=$( +# Strip comment lines and the scissors section git adds under --verbose, +# tagging each surviving line with its original line number so errors point at +# the line the author is looking at in their editor. `git commit` does not strip +# the comment block before running this hook, and the template installed by +# `make install-hooks` is 18 comment lines, so the two numberings differ in the +# normal workflow. +numbered=$( awk -v c="$comment_char" ' index($0, "------------------------ >8 ------------------------") { exit } substr($0, 1, length(c)) == c { next } - { sub(/\r$/, ""); print } + { sub(/\r$/, ""); print NR "\t" $0 } ' "$message_file" ) +stripped=$(printf '%s\n' "$numbered" | awk '{ print substr($0, index($0, "\t") + 1) }') # The subject is the first non-blank line. subject=$(printf '%s\n' "$stripped" | awk 'NF { print; exit }') @@ -80,7 +111,7 @@ else description=${subject#*: } # --- subject: length --- - length=$(printf '%s' "$subject" | wc -c | tr -d ' ') + length=$(char_len "$subject") if [ "$length" -gt "$MAX_SUBJECT" ]; then add_error "subject is $length characters; keep it to $MAX_SUBJECT or fewer" fi @@ -108,18 +139,39 @@ if [ -n "$next_line" ]; then add_error "separate the subject from the body with a blank line" elif [ -n "$has_more" ]; then # --- body: line length --- - long=$( - printf '%s\n' "$stripped" | awk -v s="$subject_line" -v max="$MAX_BODY" ' - NR <= s + 1 { next } - /^[A-Za-z][A-Za-z0-9-]*: / { next } # trailers (Nightshift-Task:, etc.) - $0 ~ /^[[:space:]]*$/ { next } - NF == 1 { next } # unbreakable tokens such as URLs - length($0) > max { print NR; exit } - ' - ) - if [ -n "$long" ]; then - add_error "body line $long exceeds $MAX_BODY characters; wrap the body at 72" - fi + # Walked in the shell rather than awk so the reported number is the original + # file line and the measurement counts characters, not bytes. + tab=$(printf '\t') + line_index=0 + while IFS= read -r entry; do + line_index=$((line_index + 1)) + [ "$line_index" -gt "$((subject_line + 1))" ] || continue + + orig_line=${entry%%"$tab"*} + text=${entry#*"$tab"} + + # Skip blank lines. + case "$text" in + *[![:space:]]*) ;; + *) continue ;; + esac + # Skip trailers (Nightshift-Task:, BREAKING CHANGE:, etc.). + if printf '%s' "$text" | grep -qE '^[A-Za-z][A-Za-z0-9-]*: '; then + continue + fi + # Skip unbreakable single tokens such as URLs. + if [ "$(printf '%s\n' "$text" | awk '{ print NF }')" -le 1 ]; then + continue + fi + + text_length=$(char_len "$text") + if [ "$text_length" -gt "$MAX_BODY" ]; then + add_error "body line $orig_line is $text_length characters; keep body lines to $MAX_BODY or fewer (wrap at 72)" + break + fi + done <<-BODY + $numbered + BODY fi if [ -n "$errors" ]; then diff --git a/scripts/commit-msg_test.sh b/scripts/commit-msg_test.sh index 9527efa..2517d73 100755 --- a/scripts/commit-msg_test.sh +++ b/scripts/commit-msg_test.sh @@ -38,6 +38,32 @@ check() { fi } +# check_output +check_output() { + expected=$1 + name=$2 + message=$3 + want=$4 + + file="$tmp_dir/msg" + printf '%s\n' "$message" >"$file" + + set +e + output=$("$validator" "$file" 2>&1) + actual=$? + set -e + + if [ "$actual" -eq "$expected" ] && printf '%s' "$output" | grep -qF "$want"; then + pass=$((pass + 1)) + printf ' ✓ %s\n' "$name" + else + fail=$((fail + 1)) + printf ' ✗ %s (want exit %s containing "%s", got exit %s)\n' \ + "$name" "$expected" "$want" "$actual" + printf '%s\n' "$output" | sed 's/^/ /' + fi +} + echo "commit-msg validator" # --- accepted: one per allowed type --- @@ -95,6 +121,33 @@ check 1 "rejects over-length body line" "feat: add thing $(printf 'word %.0s' $(seq 1 40))" +# --- lengths are counted in characters, not bytes --- +# A subject full of accents, em-dashes, smart quotes, or emoji must get the +# same 72-character budget as an ASCII one. +check 0 "accepts 72-char subject with accents" \ + "fix(core): café résumé naïve — handle accented input in the parser path" +check 0 "accepts 72-char subject with emoji and smart quotes" \ + "feat(ui): add 🎉 banner with “smart quotes” and – dashes in the subject" +check_output 1 "reports non-ASCII subject length in characters" \ + "fix(core): café résumé naïve — handle accented input in the parser pathss" \ + "subject is 73 characters" +check 0 "accepts body line of 100 accented characters" "docs: add note + +$(printf 'é%.0s' $(seq 1 91)) end word" + +# --- body errors point at the original file line --- +# git does not strip the comment block before running commit-msg, and +# `make install-hooks` installs an 18-line comment template, so a body line +# number computed after stripping would not match the author's editor. +check_output 1 "reports the original file line for a long body line" \ + "# comment one +# comment two +# comment three +feat: add thing + +$(printf 'word %.0s' $(seq 1 40))" \ + "body line 6" + # --- usage errors --- set +e "$validator" >/dev/null 2>&1