diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8fc5686..fe23550 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,3 +50,41 @@ jobs: uses: golangci/golangci-lint-action@v6 with: version: latest + + commit-lint: + name: Commit messages + runs-on: ubuntu-latest + # PR-range only: history on main predates this convention and is grandfathered. + if: github.event_name == 'pull_request' + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Test the validator + run: bash scripts/commit-msg-test.sh + + - name: Validate commit messages in this PR + env: + BASE_REF: ${{ github.base_ref }} + run: | + git fetch --no-tags origin "$BASE_REF" + RANGE="origin/$BASE_REF..HEAD" + FAIL=0 + COUNT=0 + for sha in $(git rev-list "$RANGE"); do + COUNT=$((COUNT+1)) + git log -1 --format=%B "$sha" > /tmp/commit-msg + if ! bash scripts/commit-msg.sh /tmp/commit-msg; then + echo " ↳ commit $(git log -1 --format='%h %s' "$sha")" + FAIL=$((FAIL+1)) + fi + done + echo "" + if [ "$FAIL" -gt 0 ]; then + echo "❌ $FAIL of $COUNT commit message(s) in $RANGE are invalid." + echo " See CONTRIBUTING.md, or run: make install-hooks" + exit 1 + fi + echo "✅ All $COUNT commit message(s) in $RANGE are valid." diff --git a/.gitmessage b/.gitmessage new file mode 100644 index 0000000..e584238 --- /dev/null +++ b/.gitmessage @@ -0,0 +1,34 @@ + + +# ── Nightshift commit message ──────────────────────────────────────────────── +# +# Subject (line 1): [()][!]: +# +# type one of: feat fix docs chore test refactor perf build ci style +# revert +# scope optional, lowercase: config tasks providers budget scheduler +# orchestrator daemon web docs ci +# ! optional, marks a breaking change (also add the footer below) +# description imperative mood, not sentence-cased, no trailing period +# (leading digits and acronyms are fine: "2x faster lookups", +# "HTTP retry support" — but not "Add retry support") +# +# Hard limits: subject ≤ 72 chars, body wrapped at 100 chars. +# Line 2 must be blank when a body follows. +# +# Body (optional): what changed and why, not how. Wrap at 100 chars. +# +# Footers (optional): +# BREAKING CHANGE: +# Fixes #123 +# Nightshift-Task: +# Nightshift-Ref: https://github.com/marcus/nightshift +# +# Examples: +# feat(budget): add codex daily token calibration +# fix(config): guard nil provider map on merge +# docs: document the commit message convention +# feat(api)!: drop legacy budget fields +# +# The commit-msg hook enforces the subject rules. Bypass with --no-verify. +# ───────────────────────────────────────────────────────────────────────────── diff --git a/AGENTS.md b/AGENTS.md index 1fd3ae1..64a6f71 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,3 +54,9 @@ 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. +- **Commit messages**: Conventional Commits - `[()][!]: `. + Types: feat, fix, docs, chore, test, refactor, perf, build, ci, style, revert. + Subject imperative, not sentence-cased, no trailing period, <= 72 chars; blank line before + any body. Agent-authored commits must carry `Nightshift-Task:` and + `Nightshift-Ref:` trailers. Enforced by `scripts/commit-msg.sh`; see + [CONTRIBUTING.md](CONTRIBUTING.md#commit-messages). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..e79894e --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,165 @@ +# Contributing to Nightshift + +Thanks for helping out. This document covers the local setup and the +conventions the project enforces. + +## Setup + +```bash +git clone https://github.com/marcus/nightshift.git +cd nightshift +make deps +make install-hooks # one-command setup: git hooks + commit template +make build +make check # go test + golangci-lint + commit-msg validator tests +``` + +`make install-hooks` wires up three things: + +| What | Where | Purpose | +|---|---|---| +| `scripts/pre-commit.sh` | `.git/hooks/pre-commit` | gofmt, `go vet`, `go build` on staged Go files | +| `scripts/commit-msg.sh` | `.git/hooks/commit-msg` | validates the commit subject (see below) | +| `.gitmessage` | `git config commit.template` | prefills `git commit` with the format guide | + +Hooks are opt-in and local — they are never installed automatically. Bypass +either hook in a pinch with `git commit --no-verify`. + +## Commit messages + +Nightshift uses [Conventional Commits](https://www.conventionalcommits.org/). +This is not a new invention: it is the format the repository already used for +the majority of its history, now written down and enforced going forward. + +### Format + +``` +[()][!]: + +[optional body, wrapped at 100 chars] + +[optional footers] +``` + +Subject rules, all enforced by `scripts/commit-msg.sh`: + +- `` is required and must be one of the types below. +- `` is optional, in parentheses, lowercase `[a-z0-9._/-]`. +- `!` before the colon marks a breaking change. +- Exactly one space after the colon. +- `` is imperative mood ("add", not "added"/"adds"), is not + sentence-cased, and does not end with a period. It must start with a letter + or digit; leading digits and acronyms are fine (`2x faster lookups`, + `HTTP retry support`, `OAuth token refresh`). What the hook rejects is a + capital immediately followed by a lowercase letter — `Add budget calibration`. +- The whole subject line is **72 characters or fewer**. +- If a body follows, line 2 must be blank. + +### Types + +| Type | Use for | +|---|---| +| `feat` | a new user-visible capability | +| `fix` | a bug fix | +| `docs` | documentation only, including `website/` | +| `refactor` | code change that neither fixes a bug nor adds a feature | +| `perf` | a change made to improve performance | +| `test` | adding or correcting tests | +| `build` | build system, `go.mod`, goreleaser, Makefile | +| `ci` | GitHub Actions and other CI configuration | +| `chore` | maintenance that fits nothing else (version bumps, tidying) | +| `style` | formatting only, no behaviour change | +| `revert` | reverting an earlier commit | + +### Scopes + +Scopes are optional but encouraged. Use the package or area the change lands +in — the names that already appear in the tree: + +`config`, `budget`, `scheduler`, `providers`, `tasks`, `orchestrator`, +`commands`, `daemon`, `web`, `docs`, `hooks`, `ci` + +### Breaking changes + +Mark them both ways — `!` in the subject for scanability, and a +`BREAKING CHANGE:` footer explaining the migration: + +``` +feat(config)!: drop v1 provider keys + +BREAKING CHANGE: `providers.claude.path` is now `providers.claude.data_path`. +Run `nightshift config validate` after upgrading. +``` + +### Agent-authored commits + +Commits produced by a Nightshift agent run must carry these trailers in the +footer so the run can be traced back: + +``` +Nightshift-Task: +Nightshift-Ref: https://github.com/marcus/nightshift +``` + +### Examples + +``` +feat(budget): add codex daily token calibration +fix(config): guard nil provider map on merge +docs: document the commit message convention +ci: lint pull request commit messages +feat(api)!: drop legacy budget fields +``` + +Commits git itself writes or rewrites are exempt from validation: merge +commits, `Revert "..."`, and `fixup!` / `squash!` / `amend!` commits. + +### Grandfathered history + +History is **not** rewritten. Of the 171 commits on `main` at the time this +convention was written down, 129 (75%) already used a Conventional Commits +prefix and 110 pass the validator as-is; the remaining 61 predate the rules — +mostly merge commits, `Bump version to ...` subjects, and otherwise-valid +subjects that run past 72 characters because a `(#42)` or `(td-abc123)` ref was +appended. Those stay as they are. + +To reproduce those numbers: + +```bash +git rev-list --count main # 171 +git log --pretty=%s main | grep -cE \ + '^(feat|fix|docs|chore|test|refactor|perf|build|ci|style|revert)(\([^)]+\))?!?: ' # 129 +git log --pretty=%s main | while read -r s; do \ + printf '%s\n' "$s" > /tmp/m && scripts/commit-msg.sh /tmp/m >/dev/null 2>&1 \ + || echo "$s"; done | wc -l # 61 rejected -> 110 pass +``` + +The 129 figure counts *any* parenthesised scope. Exactly one of those commits +(`fix(#19): ...`) uses a scope the validator rejects, so a stricter count that +requires a `[a-z0-9._/-]+` scope gives 128. Either way the 110-pass and +61-reject figures are unchanged, since the validator is what produced them. + +Because of this, CI validates **only the commits in a pull request's range** +(`origin/..HEAD`), never the full history. A `git log` on `main` will +still show non-conforming subjects, and that is expected. + +Note that GitHub appends ` (#N)` to the subject on squash merge. That happens +server-side, after the hook has run, so a subject that is legal locally can end +up slightly over 72 characters on `main`. Leave a little headroom. + +## Pull requests + +- PR titles follow the same format as commit subjects — the squash-merge + subject comes from the PR title. +- Describe *why* in the body, not just what. +- `make check` must pass locally before you push. +- Everything lands on a branch; nothing is committed directly to `main`. + +## Code conventions + +See [AGENTS.md](AGENTS.md) for the short version: + +- **Style**: standard Go — gofmt, `go vet`. Explicit over clever. +- **Errors**: wrap with context, never swallow. +- **Tests**: table-driven, in `_test.go` alongside the code. +- **Logging**: hyper-concise. Include what is needed, minimize words. diff --git a/Makefile b/Makefile index 088be01..ca2a3f2 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-verbose test-race coverage coverage-html lint clean deps check install calibrate-providers install-hooks test-hooks help # Binary name BINARY=nightshift @@ -57,8 +57,12 @@ deps: go mod download go mod tidy +# Run the commit-msg validator fixture tests +test-hooks: + @bash scripts/commit-msg-test.sh + # Run all checks (test + lint) -check: test lint +check: test lint test-hooks # Show help help: @@ -73,12 +77,24 @@ help: @echo " clean - Clean build artifacts" @echo " deps - Download and tidy dependencies" @echo " check - Run tests and lint" + @echo " test-hooks - Run the commit-msg validator test suite" @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 (pre-commit, commit-msg) and commit template" @echo " help - Show this help" -# Install git pre-commit hook +# Install git hooks and the commit message template +# The hooks directory is shared by every worktree, so link targets must resolve +# to the MAIN worktree (--git-common-dir/..), not the current one +# (--show-toplevel). Linking a throwaway worktree's path would leave a dangling +# symlink once that worktree is removed, and git skips broken hooks silently. 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)" + @hooks="$$(git rev-parse --git-path hooks)"; \ + root="$$(cd "$$(git rev-parse --path-format=absolute --git-common-dir)/.." && pwd)"; \ + mkdir -p "$$hooks"; \ + ln -sf "$$root/scripts/pre-commit.sh" "$$hooks/pre-commit"; \ + echo "✓ pre-commit hook installed ($$hooks/pre-commit → scripts/pre-commit.sh)"; \ + ln -sf "$$root/scripts/commit-msg.sh" "$$hooks/commit-msg"; \ + echo "✓ commit-msg hook installed ($$hooks/commit-msg → scripts/commit-msg.sh)" + @git config commit.template .gitmessage + @echo "✓ commit template configured (commit.template → .gitmessage)" diff --git a/README.md b/README.md index 84f92cd..5aee8d5 100644 --- a/README.md +++ b/README.md @@ -258,21 +258,33 @@ Each task has a default cooldown interval to prevent the same task from running ## Development -### Pre-commit hooks +See [CONTRIBUTING.md](CONTRIBUTING.md) for the full contributor guide, including the commit message convention. -Install the git pre-commit hook to catch formatting and vet issues before pushing: +### Git hooks + +Install the git hooks and commit template in one step: ```bash make install-hooks ``` -This symlinks `scripts/pre-commit.sh` into `.git/hooks/pre-commit`. The hook runs: -- **gofmt** — flags any staged `.go` files that need formatting -- **go vet** — catches common correctness issues -- **go build** — ensures the project compiles +This wires up: +- **pre-commit** (`scripts/pre-commit.sh`) — gofmt, `go vet`, and `go build` on staged Go files +- **commit-msg** (`scripts/commit-msg.sh`) — validates the commit subject against the convention +- **commit template** (`.gitmessage`) — prefills `git commit` with the format guide To bypass in a pinch: `git commit --no-verify` +### Commit messages + +Nightshift uses [Conventional Commits](https://www.conventionalcommits.org/): + +``` +[()][!]: +``` + +Types: `feat`, `fix`, `docs`, `chore`, `test`, `refactor`, `perf`, `build`, `ci`, `style`, `revert`. Subjects are imperative and not sentence-cased (acronyms and digits are fine), carry no trailing period, and stay at 72 characters or fewer. Existing history predates this convention and is grandfathered — CI only checks the commits in a pull request. Full rules: [CONTRIBUTING.md](CONTRIBUTING.md#commit-messages). + ## Uninstalling ```bash diff --git a/scripts/commit-msg-test.sh b/scripts/commit-msg-test.sh new file mode 100755 index 0000000..35b024d --- /dev/null +++ b/scripts/commit-msg-test.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +# Fixture harness for scripts/commit-msg.sh +# Run: bash scripts/commit-msg-test.sh +set -uo pipefail + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HOOK="$HERE/commit-msg.sh" +TMPDIR_="$(mktemp -d)" +trap 'rm -rf "$TMPDIR_"' EXIT + +PASS=0 +FAIL=0 + +echo "🪡 commit-msg validator tests" + +# expect +expect() { + local want="$1" name="$2"; shift 2 + local file="$TMPDIR_/msg" + printf '%s\n' "$@" > "$file" + + local out rc + out=$(bash "$HOOK" "$file" 2>&1); rc=$? + + local got="accept" + [[ $rc -ne 0 ]] && got="reject" + + printf " %-8s %-46s" "$want" "$name" + if [[ "$got" == "$want" ]]; then + echo "✓" + PASS=$((PASS+1)) + else + echo "✗ FAILED (exit $rc, wanted $want)" + echo "$out" | sed 's/^/ /' + FAIL=$((FAIL+1)) + fi +} + +long_subject="feat: $(printf 'x%.0s' {1..80})" +max_subject="feat: $(printf 'x%.0s' {1..66})" # exactly 72 chars +over_subject="feat: $(printf 'x%.0s' {1..67})" # exactly 73 chars + +# --- accepted --- +expect accept "plain type" "feat: add budget calibration" +expect accept "scoped" "fix(config): guard nil provider map" +expect accept "breaking scoped" "feat(api)!: drop legacy budget fields" +expect accept "breaking unscoped" "feat!: require go 1.23" +expect accept "single-char subject" "feat: x" +expect accept "nested scope" "docs(website/docs): clarify install" +expect accept "subject exactly 72" "$max_subject" +expect accept "body after blank line" "fix: correct day boundary" "" "The daily window rolled over in UTC instead of local time." +expect accept "trailers" "chore: normalize commit messages" "" "Nightshift-Task: commit-normalize" "Nightshift-Ref: https://github.com/marcus/nightshift" +expect accept "breaking change footer" "feat: drop v1 config" "" "BREAKING CHANGE: config.v1 keys are no longer read." +expect accept "merge commit" "Merge pull request #17 from cedricfarinazzo/nightshift-lint-fixes" +expect accept "revert commit" "Revert \"feat: add budget calibration\"" "" "This reverts commit deadbeef." +expect accept "fixup!" "fixup! feat: add budget calibration" +expect accept "squash!" "squash! feat: add budget calibration" +expect accept "amend!" "amend! feat: add budget calibration" +expect accept "comments stripped" "# please enter the commit message" "feat: add doctor command" "# Changes to be committed:" +expect accept "commit -v diff ignored" "fix: drop stale lock file" "" "# ------------------------ >8 ------------------------" "diff --git a/x.go b/x.go" "+FEAT: NOT A SUBJECT." +expect accept "empty message" "" "# nothing staged" +expect accept "whitespace-only (git aborts)" " " +expect accept "all allowed types" "perf: hoist regex out of loop" +expect accept "ci type" "ci: lint pull request commit messages" +expect accept "revert type prefix" "revert: feat add budget calibration" +expect accept "leading digit" "fix: 2x faster budget lookups" +expect accept "leading acronym" "feat: HTTP retry support" +expect accept "short acronym" "feat: URL parsing for provider hosts" +expect accept "mixed-case acronym" "feat: OAuth token refresh" +expect accept "trailing whitespace" "feat: add budget calibration " + +# --- rejected --- +expect reject "unknown type" "foo: add a thing" +expect reject "missing colon" "add budget calibration" +expect reject "capitalized subject" "feat: Add budget calibration" +expect reject "trailing period" "feat: add budget calibration." +expect reject "subject over 72" "$over_subject" +expect reject "very long subject" "$long_subject" +expect reject "empty subject text" "feat:" +expect reject "no space after colon" "feat:add budget calibration" +expect reject "uppercase type" "Feat: add budget calibration" +expect reject "uppercase scope" "fix(Config): guard nil provider map" +expect reject "empty scope" "fix(): guard nil provider map" +expect reject "body without blank line" "fix: correct day boundary" "The window rolled over in UTC." +expect reject "leading whitespace" " feat: add budget calibration" +expect reject "trailing period past space" "feat: add budget calibration. " +expect reject "sentence case" "feat: Add budget calibration" +expect reject "over 72 once trimmed" "$over_subject " + +echo "" +if [[ $FAIL -gt 0 ]]; then + echo "❌ $FAIL of $((PASS+FAIL)) test(s) failed" + exit 1 +fi +echo "✅ All $PASS tests passed" diff --git a/scripts/commit-msg.sh b/scripts/commit-msg.sh new file mode 100755 index 0000000..dc08ff6 --- /dev/null +++ b/scripts/commit-msg.sh @@ -0,0 +1,128 @@ +#!/usr/bin/env bash +# commit-msg hook for nightshift — enforces Conventional Commits. +# Install: make install-hooks (or: ln -sf ../../scripts/commit-msg.sh .git/hooks/commit-msg) +# Bypass: git commit --no-verify +# +# Format: [()][!]: +# See CONTRIBUTING.md for the full convention. +set -uo pipefail + +TYPES="feat|fix|docs|chore|test|refactor|perf|build|ci|style|revert" +MAX_SUBJECT=72 + +MSG_FILE="${1:-}" +if [[ -z "$MSG_FILE" || ! -f "$MSG_FILE" ]]; then + echo "commit-msg: no message file given" >&2 + exit 1 +fi + +# Git strips comment lines using core.commentChar (a single char, or "auto", +# which still emits "#" in the common case). core.commentString supersedes it in +# newer git. Honor whichever is configured so custom setups are not mis-parsed. +COMMENT="$(git config --get core.commentString 2>/dev/null || true)" +[[ -z "$COMMENT" ]] && COMMENT="$(git config --get core.commentChar 2>/dev/null || true)" +[[ -z "$COMMENT" || "$COMMENT" == "auto" ]] && COMMENT="#" + +# --- strip comments and the `commit -v` / scissors diff --- +LINES=() +while IFS= read -r line || [[ -n "$line" ]]; do + # everything from the scissors marker onward is not part of the message + if [[ "$line" == *"------------------------ >8 ------------------------"* ]]; then + break + fi + [[ "$line" == "$COMMENT"* ]] && continue + # git's default cleanup strips trailing whitespace *after* this hook runs, so + # trim it here too — otherwise "feat: x. " sneaks past the no-period rule and + # lands on the branch as "feat: x." + LINES+=("${line%"${line##*[![:space:]]}"}") +done < "$MSG_FILE" + +# --- locate the subject: first non-empty line --- +SUBJECT="" +SUBJECT_IDX=-1 +for i in "${!LINES[@]}"; do + if [[ -n "${LINES[$i]}" ]]; then + SUBJECT="${LINES[$i]}" + SUBJECT_IDX=$i + break + fi +done + +# Empty message: git aborts the commit on its own. +[[ $SUBJECT_IDX -lt 0 ]] && exit 0 + +# --- commits git generates or rewrites later are exempt --- +case "$SUBJECT" in + Merge\ *|Revert\ \"*|fixup!\ *|squash!\ *|amend!\ *) exit 0 ;; +esac + +reject() { + local rule="$1" fix="$2" + echo "🪡 commit-msg check" + printf " %-20s %s\n" "subject" "✗ $rule" + echo "" + echo " got: $SUBJECT" + echo " expected: [()][!]: " + echo " example: $fix" + echo "" + echo " types: ${TYPES//|/, }" + echo " limits: subject ≤ ${MAX_SUBJECT} chars, not sentence-cased, no trailing period" + echo "" + echo "❌ Commit message rejected. See CONTRIBUTING.md, or bypass with --no-verify." + exit 1 +} + +# --- subject rules --- +if [[ "$SUBJECT" =~ ^[[:space:]] ]]; then + reject "leading whitespace" "feat(budget): add daily calibration" +fi + +if [[ ${#SUBJECT} -gt $MAX_SUBJECT ]]; then + reject "too long (${#SUBJECT} chars, max ${MAX_SUBJECT})" "feat(budget): add daily calibration" +fi + +if [[ ! "$SUBJECT" =~ ^($TYPES)(\([a-z0-9._/-]+\))?!?:\ ]]; then + if [[ "$SUBJECT" != *:* ]]; then + reject "missing ': ' prefix" "feat(budget): add daily calibration" + fi + prefix="${SUBJECT%%:*}" + reject "bad type or scope in '${prefix}:' (or missing space after the colon)" \ + "feat(budget): add daily calibration" +fi + +DESC="${SUBJECT#*: }" + +if [[ -z "$DESC" ]]; then + reject "empty description" "feat(budget): add daily calibration" +fi + +if [[ ! "$DESC" =~ ^[A-Za-z0-9] ]]; then + reject "description must start with a letter or digit" "feat: add daily calibration" +fi + +# Reject Sentence case ("Add x") but allow digits ("2x faster") and acronyms +# ("HTTP retry", "OAuth refresh") — a capital is only wrong when a lowercase +# letter follows it immediately. +if [[ "$DESC" =~ ^[A-Z][a-z] ]]; then + reject "description must not be sentence-cased (write 'add x', not 'Add x')" \ + "feat: add daily calibration" +fi + +if [[ "$DESC" == *. ]]; then + reject "description must not end with a period" "feat: add daily calibration" +fi + +# --- body rules: line 2 must be blank when a body follows --- +NEXT_IDX=$((SUBJECT_IDX + 1)) +if [[ $NEXT_IDX -lt ${#LINES[@]} && -n "${LINES[$NEXT_IDX]}" ]]; then + echo "🪡 commit-msg check" + printf " %-20s %s\n" "body" "✗ line 2 must be blank when a body follows the subject" + echo "" + echo " got: ${LINES[$NEXT_IDX]}" + echo " expected: an empty line between the subject and the body" + echo "" + echo "❌ Commit message rejected. See CONTRIBUTING.md, or bypass with --no-verify." + exit 1 +fi + +exit 0