From 2e3a9bc813723a1f975c4640b4d512062266cfac Mon Sep 17 00:00:00 2001 From: Lasse Larsen Date: Wed, 26 Aug 2026 02:16:21 +0200 Subject: [PATCH 1/2] feat(commits): add Conventional Commits message normalizer Add internal/commits with a pure Normalize function enforcing the project's Conventional Commits rules: known type set (feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert), optional scope, lowercase imperative subject limited to 72 chars with no trailing period, comment stripping, body wrapping at 72 columns, and verbatim preservation of git trailer blocks. Wire it into the CLI as 'nightshift commit normalize' (positional, --file, and stdin sources; --check validates only), ship a commit-msg hook under scripts/, install it via 'make install-hooks', and add a CI job that validates every commit in a pull request. Document the format in docs/commit-messages.md. Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- .github/workflows/ci.yml | 30 +++ Makefile | 6 +- README.md | 8 +- cmd/nightshift/commands/commit.go | 89 +++++++++ docs/commit-messages.md | 64 +++++++ internal/commits/normalizer.go | 281 ++++++++++++++++++++++++++++ internal/commits/normalizer_test.go | 149 +++++++++++++++ scripts/commit-msg.sh | 48 +++++ 8 files changed, 671 insertions(+), 4 deletions(-) create mode 100644 cmd/nightshift/commands/commit.go create mode 100644 docs/commit-messages.md create mode 100644 internal/commits/normalizer.go create mode 100644 internal/commits/normalizer_test.go create mode 100755 scripts/commit-msg.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8fc5686..ee4c2b1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,3 +50,33 @@ jobs: uses: golangci/golangci-lint-action@v6 with: version: latest + + commit-msg: + name: Commit messages + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: '1.23' + + - name: Validate commit messages (Conventional Commits) + run: | + if [[ "$GITHUB_EVENT_NAME" == "pull_request" ]]; then + RANGE="${{ github.event.pull_request.base.sha }}..HEAD" + else + RANGE="HEAD^..HEAD" + fi + status=0 + for sha in $(git rev-list "$RANGE"); do + if ! git log -1 --format=%B "$sha" | go run ./cmd/nightshift commit normalize --check; then + echo "::error title=Invalid commit message::$(git log -1 --format='%h %s' "$sha") does not follow Conventional Commits (docs/commit-messages.md)" + status=1 + fi + done + exit $status diff --git a/Makefile b/Makefile index 088be01..c63be74 100644 --- a/Makefile +++ b/Makefile @@ -75,10 +75,12 @@ help: @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 (pre-commit, commit-msg)" @echo " help - Show this help" -# Install git pre-commit hook +# Install git hooks (pre-commit + commit-msg) install-hooks: @ln -sf ../../scripts/pre-commit.sh .git/hooks/pre-commit + @ln -sf ../../scripts/commit-msg.sh .git/hooks/commit-msg @echo "✓ pre-commit hook installed (.git/hooks/pre-commit → scripts/pre-commit.sh)" + @echo "✓ commit-msg hook installed (.git/hooks/commit-msg → scripts/commit-msg.sh)" diff --git a/README.md b/README.md index 84f92cd..df63447 100644 --- a/README.md +++ b/README.md @@ -258,9 +258,9 @@ 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 to catch formatting/vet issues and enforce commit message style before pushing: ```bash make install-hooks @@ -271,6 +271,10 @@ This symlinks `scripts/pre-commit.sh` into `.git/hooks/pre-commit`. The hook run - **go vet** — catches common correctness issues - **go build** — ensures the project compiles +It also symlinks `scripts/commit-msg.sh` into `.git/hooks/commit-msg`, which enforces +[Conventional Commits](docs/commit-messages.md) (`(): `) and +normalizes the message before the commit is created. + To bypass in a pinch: `git commit --no-verify` ## Uninstalling diff --git a/cmd/nightshift/commands/commit.go b/cmd/nightshift/commands/commit.go new file mode 100644 index 0000000..df8652c --- /dev/null +++ b/cmd/nightshift/commands/commit.go @@ -0,0 +1,89 @@ +package commands + +import ( + "fmt" + "io" + "os" + + "github.com/spf13/cobra" + + "github.com/marcus/nightshift/internal/commits" +) + +var commitCmd = &cobra.Command{ + Use: "commit", + Short: "Conventional Commits helpers", + Long: `Tools for working with Conventional Commits messages. + +Use "commit normalize" to validate and reformat a commit message so it +follows the project's rules (type prefix, lowercase type, subject length, +and wrapped body).`, +} + +var commitNormalizeCmd = &cobra.Command{ + Use: "normalize [MESSAGE]", + Short: "Normalize a commit message to Conventional Commits format", + Long: `Validate and rewrite a commit message into canonical Conventional +Commits form. + +The message is read from a positional argument, from a file passed via +--file (typically .git/COMMIT_EDITMSG by a commit-msg hook), or from stdin +when no argument and no --file are given. + + nightshift commit normalize "feat: add login" + nightshift commit normalize --file .git/COMMIT_EDITMSG + git log -1 --pretty=%B | nightshift commit normalize + +With --check the message is only validated: nothing is printed on success +and the exit code is non-zero when the message does not conform. Without +--check the normalized message is printed to stdout.`, + Args: cobra.MaximumNArgs(1), + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + check, _ := cmd.Flags().GetBool("check") + file, _ := cmd.Flags().GetString("file") + + raw, err := readCommitMessage(args, file) + if err != nil { + return err + } + + normalized, err := commits.Normalize(raw) + if err != nil { + return fmt.Errorf("invalid commit message: %w", err) + } + + if check { + return nil + } + fmt.Fprintln(cmd.OutOrStdout(), normalized) + return nil + }, +} + +func init() { + commitNormalizeCmd.Flags().BoolP("check", "c", false, "Only validate; do not print the normalized message") + commitNormalizeCmd.Flags().StringP("file", "f", "", "Read the message from this file (used by the commit-msg hook)") + commitCmd.AddCommand(commitNormalizeCmd) + rootCmd.AddCommand(commitCmd) +} + +// readCommitMessage resolves the message source in order: positional arg, +// --file, then stdin. +func readCommitMessage(args []string, file string) (string, error) { + if len(args) == 1 { + return args[0], nil + } + if file != "" { + b, err := os.ReadFile(file) + if err != nil { + return "", fmt.Errorf("read %s: %w", file, err) + } + return string(b), nil + } + b, err := io.ReadAll(os.Stdin) + if err != nil { + return "", fmt.Errorf("read stdin: %w", err) + } + return string(b), nil +} diff --git a/docs/commit-messages.md b/docs/commit-messages.md new file mode 100644 index 0000000..c02aa4d --- /dev/null +++ b/docs/commit-messages.md @@ -0,0 +1,64 @@ +# Commit Messages + +Nightshift uses [Conventional Commits](https://www.conventionalcommits.org/) +for all commit messages. This keeps the history readable and lets tooling +derive changelogs automatically. + +## Format + +``` +(): + + + + +``` + +- **type** — one of `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, + `test`, `build`, `ci`, `chore`, `revert`. +- **scope** — optional, e.g. `fix(api): ...`. +- **subject** — lowercase, imperative mood, no trailing period, max 72 chars. +- **body** — optional, wrapped at 72 columns, separated from the subject by a + blank line. +- **trailers** — optional `Token: value` lines (e.g. `Signed-off-by:`); kept + verbatim, one per line. + +Examples: + +``` +feat(api): add retry with exponential backoff +fix: apply --max-projects limit after processed-today filter +docs: document the commit-msg hook +``` + +## The `commit normalize` command + +Validate and reformat a message: + +```sh +nightshift commit normalize "feat: add login screen" +nightshift commit normalize --file .git/COMMIT_EDITMSG +git log -1 --pretty=%B | nightshift commit normalize +``` + +The command fixes the trivially fixable (whitespace, type casing, trailing +period, body wrapping) and exits non-zero when a message cannot be normalized +(missing/unknown type, capitalized or overlong subject). Add `--check` to +validate only without printing the normalized message. + +## commit-msg hook + +To enforce the rules locally, install the hook: + +```sh +make install-hooks +``` + +This also symlinks `scripts/commit-msg.sh` into `.git/hooks/commit-msg` (and +`scripts/pre-commit.sh` into `.git/hooks/pre-commit`). The commit-msg hook +normalizes your message file in place before the commit is created and rejects +messages that cannot be fixed automatically. Bypass it with +`git commit --no-verify`. + +CI runs the same validation over every commit in a pull request, so a +non-conforming message fails the build even without the local hook. diff --git a/internal/commits/normalizer.go b/internal/commits/normalizer.go new file mode 100644 index 0000000..cf9eaa4 --- /dev/null +++ b/internal/commits/normalizer.go @@ -0,0 +1,281 @@ +// Package commits implements Conventional Commits message normalization and +// validation. It exposes pure, well-tested functions used by the CLI and by the +// commit-msg git hook to keep the project's history consistent. +// +// The supported format follows the Conventional Commits 1.0.0 specification: +// +// (): +// +// +// +// The normalizer is intentionally strict but constructive: rather than silently +// accepting malformed input it fixes the trivially fixable (whitespace, type +// casing, trailing punctuation, body wrapping) and rejects anything that needs +// a human decision (missing type, unknown type, missing subject). +package commits + +import ( + "errors" + "fmt" + "strings" + "unicode/utf8" +) + +// MaxSubjectLength is the maximum number of runes allowed in a commit subject. +const MaxSubjectLength = 72 + +// BodyWrapWidth is the column at which the commit body is wrapped. +const BodyWrapWidth = 72 + +// allowedTypes is the set of Conventional Commit types this project accepts. +var allowedTypes = map[string]struct{}{ + "feat": {}, + "fix": {}, + "docs": {}, + "style": {}, + "refactor": {}, + "perf": {}, + "test": {}, + "build": {}, + "ci": {}, + "chore": {}, + "revert": {}, +} + +// Errors returned by the normalizer. They are wrapped so callers can match on +// the underlying cause with errors.Is. +var ( + // ErrEmptyMessage is returned when the message contains no non-comment, + // non-whitespace content. + ErrEmptyMessage = errors.New("commit message is empty") + // ErrMissingType is returned when the subject line is not a Conventional + // Commit (no type prefix before the colon). + ErrMissingType = errors.New("commit message must start with a conventional commit type") + // ErrUnknownType is returned when the type prefix is not in the allowed set. + ErrUnknownType = errors.New("commit type is not in the allowed set") + // ErrMissingSubject is returned when the type prefix is present but no + // subject text follows the colon. + ErrMissingSubject = errors.New("commit subject is missing") + // ErrSubjectTooLong is returned when the subject exceeds MaxSubjectLength. + ErrSubjectTooLong = fmt.Errorf("commit subject exceeds %d characters", MaxSubjectLength) + // ErrSubjectCapitalized is returned when the subject starts with an + // uppercase letter (the rule is "do not capitalize the subject"). + ErrSubjectCapitalized = errors.New("commit subject must not be capitalized") +) + +// Normalize parses, validates, and rewrites a raw commit message so that it +// conforms to the project's Conventional Commits rules. It returns the +// canonical form and a non-nil error describing the first unrecoverable +// problem when the message cannot be normalized. +// +// Normalization is idempotent: Normalize(Normalize(m)) == Normalize(m). +func Normalize(msg string) (string, error) { + lines := stripComments(msg) + if len(lines) == 0 { + return "", ErrEmptyMessage + } + + header := lines[0] + body := lines[1:] + + typ, scope, subject, err := parseHeader(header) + if err != nil { + return "", err + } + + subject = cleanSubject(subject) + + var b strings.Builder + b.WriteString(formatHeader(typ, scope, subject)) + + wrapped := wrapBody(body, BodyWrapWidth) + if wrapped != "" { + b.WriteString("\n\n") + b.WriteString(wrapped) + } + + return b.String(), nil +} + +// stripComments removes git's commented-out lines (those beginning with "#"), +// trims trailing whitespace from every line, and drops leading/trailing blank +// lines. It returns the meaningful lines of the message. +func stripComments(msg string) []string { + rawLines := strings.Split(msg, "\n") + out := make([]string, 0, len(rawLines)) + for _, l := range rawLines { + l = strings.TrimRight(l, " \t\r") + if strings.HasPrefix(strings.TrimSpace(l), "#") { + continue + } + out = append(out, l) + } + // Drop leading and trailing blank lines. + for len(out) > 0 && strings.TrimSpace(out[0]) == "" { + out = out[1:] + } + for len(out) > 0 && strings.TrimSpace(out[len(out)-1]) == "" { + out = out[:len(out)-1] + } + return out +} + +// parseHeader splits the first line into its Conventional Commit components and +// validates them. The returned type is lower-cased to match the allowed set. +func parseHeader(header string) (typ, scope, subject string, err error) { + header = strings.TrimSpace(header) + colon := strings.Index(header, ":") + if colon <= 0 { + return "", "", "", ErrMissingType + } + prefix := header[:colon] + subject = strings.TrimSpace(header[colon+1:]) + + // Split an optional "(scope)" from the type. + prefix = strings.TrimSpace(prefix) + if strings.HasPrefix(prefix, "(") { + // A leading "(" with no type is not a valid conventional header. + return "", "", "", ErrMissingType + } + if open := strings.Index(prefix, "("); open > 0 && strings.HasSuffix(prefix, ")") { + typ = prefix[:open] + scope = prefix[open+1 : len(prefix)-1] + } else { + typ = prefix + } + typ = strings.ToLower(strings.TrimSpace(typ)) + scope = strings.TrimSpace(scope) + + if typ == "" { + return "", "", "", ErrMissingType + } + if !isAllowedType(typ) { + return "", "", "", fmt.Errorf("%w: %q", ErrUnknownType, typ) + } + if strings.TrimSpace(subject) == "" { + return "", "", "", ErrMissingSubject + } + if utf8.RuneCountInString(subject) > MaxSubjectLength { + return "", "", "", ErrSubjectTooLong + } + if startsUpper(subject) { + return "", "", "", ErrSubjectCapitalized + } + return typ, scope, subject, nil +} + +// cleanSubject normalizes the subject text: surrounding whitespace and a +// trailing period are removed. Capitalization is a hard error, not a fix, so +// it is deliberately left alone. +func cleanSubject(subject string) string { + s := strings.TrimSpace(subject) + s = strings.TrimRight(s, ".") + return s +} + +// formatHeader reassembles a canonical header line from its components. +func formatHeader(typ, scope, subject string) string { + if scope != "" { + return typ + "(" + scope + "): " + subject + } + return typ + ": " + subject +} + +// wrapBody collapses runs of blank lines, preserves paragraph breaks (a single +// blank line), and hard-wraps each paragraph to width. Git trailer lines +// ("Token: value" such as "Signed-off-by:" or "Nightshift-Task:") are kept +// verbatim and unwrapped; a run of consecutive trailers stays a single +// newline-separated block, as git's own trailer convention requires. +func wrapBody(body []string, width int) string { + var paragraphs []string + var cur []string + var trailers []string + flush := func() { + if len(cur) > 0 { + paragraphs = append(paragraphs, wrapParagraph(strings.Join(cur, " "), width)) + cur = nil + } + if len(trailers) > 0 { + paragraphs = append(paragraphs, strings.Join(trailers, "\n")) + trailers = nil + } + } + for _, l := range body { + if strings.TrimSpace(l) == "" { + flush() + continue + } + if isTrailerLine(l) { + trailers = append(trailers, strings.TrimSpace(l)) + continue + } + cur = append(cur, strings.TrimSpace(l)) + } + flush() + + return strings.Join(paragraphs, "\n\n") +} + +// isTrailerLine reports whether l looks like a git trailer: a bare token +// followed by a colon and a value (or an end-of-line colon), e.g. +// "Signed-off-by: Jane " or "Nightshift-Task: lint-fix". +func isTrailerLine(l string) bool { + s := strings.TrimSpace(l) + colon := strings.Index(s, ":") + if colon <= 0 { + return false + } + token := s[:colon] + for _, r := range token { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '-', r == '_': + default: + return false + } + } + return true +} + +// wrapParagraph hard-wraps a single-line paragraph at width, breaking on word +// boundaries. A word longer than width is left intact rather than split. +func wrapParagraph(text string, width int) string { + words := strings.Fields(text) + if len(words) == 0 { + return "" + } + var b strings.Builder + lineLen := 0 + for i, w := range words { + if i == 0 { + b.WriteString(w) + lineLen = len(w) + continue + } + if lineLen+1+len(w) <= width { + b.WriteByte(' ') + b.WriteString(w) + lineLen += 1 + len(w) + } else { + b.WriteByte('\n') + b.WriteString(w) + lineLen = len(w) + } + } + return b.String() +} + +// isAllowedType reports whether typ is one of the accepted Conventional Commit +// types. +func isAllowedType(typ string) bool { + _, ok := allowedTypes[typ] + return ok +} + +// startsUpper reports whether the first rune of s is an ASCII uppercase letter. +func startsUpper(s string) bool { + if s == "" { + return false + } + r, _ := utf8.DecodeRuneInString(s) + return r >= 'A' && r <= 'Z' +} diff --git a/internal/commits/normalizer_test.go b/internal/commits/normalizer_test.go new file mode 100644 index 0000000..a9cacd8 --- /dev/null +++ b/internal/commits/normalizer_test.go @@ -0,0 +1,149 @@ +package commits + +import ( + "errors" + "strings" + "testing" +) + +func TestNormalize(t *testing.T) { + tests := []struct { + name string + in string + want string + wantErr error + }{ + { + name: "valid simple feat", + in: "feat: add login screen", + want: "feat: add login screen", + }, + { + name: "valid with scope", + in: "fix(api): handle nil response", + want: "fix(api): handle nil response", + }, + { + name: "revert type accepted", + in: "revert: feat(api): drop v2 endpoint", + want: "revert: feat(api): drop v2 endpoint", + }, + { + name: "trims surrounding whitespace and trailing period", + in: " docs: update README. ", + want: "docs: update README", + }, + { + name: "lowercases an uppercased type", + in: "FEAT(ui): render button", + want: "feat(ui): render button", + }, + { + name: "preserves body and wraps long lines", + in: "feat: add thing\n\nthis is a body paragraph that is intentionally far longer than the configured wrap width so it must be hard wrapped onto multiple lines by the normalizer function", + want: "feat: add thing\n\n" + + "this is a body paragraph that is intentionally far longer than the\n" + + "configured wrap width so it must be hard wrapped onto multiple lines by\n" + + "the normalizer function", + }, + { + name: "keeps trailer lines verbatim and unwrapped", + in: "chore: bump deps\n\nupdate everything to latest and here is a long explanatory paragraph that goes past the wrap width so it has to be wrapped by the normalizer\n\nSigned-off-by: Jane \nNightshift-Task: commit-normalize", + want: "chore: bump deps\n\n" + + "update everything to latest and here is a long explanatory paragraph\n" + + "that goes past the wrap width so it has to be wrapped by the normalizer" + + "\n\n" + + "Signed-off-by: Jane \n" + + "Nightshift-Task: commit-normalize", + }, + { + name: "strips git comment lines", + in: "chore: tidy\n# please enter the commit message\n\nbody here", + want: "chore: tidy\n\nbody here", + }, + { + name: "missing type rejected", + in: "just a plain message", + wantErr: ErrMissingType, + }, + { + name: "unknown type rejected", + in: "wip: halfway done", + wantErr: ErrUnknownType, + }, + { + name: "missing subject rejected", + in: "feat:", + wantErr: ErrMissingSubject, + }, + { + name: "capitalized subject rejected", + in: "feat: Add login screen", + wantErr: ErrSubjectCapitalized, + }, + { + name: "overlong subject rejected", + in: "feat: " + strings.Repeat("a", MaxSubjectLength+1), + wantErr: ErrSubjectTooLong, + }, + { + name: "empty message rejected", + in: "\n\n# only comments\n \n", + wantErr: ErrEmptyMessage, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := Normalize(tc.in) + if tc.wantErr != nil { + if err == nil { + t.Fatalf("Normalize(%q): expected error %v, got nil (result %q)", tc.in, tc.wantErr, got) + } + if !errors.Is(err, tc.wantErr) { + t.Fatalf("Normalize(%q): expected error to wrap %v, got %v", tc.in, tc.wantErr, err) + } + return + } + if err != nil { + t.Fatalf("Normalize(%q): unexpected error: %v", tc.in, err) + } + if got != tc.want { + t.Errorf("Normalize(%q):\n got: %q\nwant: %q", tc.in, got, tc.want) + } + }) + } +} + +func TestNormalizeIdempotent(t *testing.T) { + cases := []string{ + "feat: add login screen", + "fix(api): handle nil response\n\nLong body that explains the fix in more detail than the subject alone can manage so that we exercise the wrapping path too and then some more words here.", + "docs: update README\n\nfirst paragraph\n\nsecond paragraph stays separate", + "chore: tidy\n\nsome prose that wraps because it is long enough to need it across columns\n\nSigned-off-by: Jane ", + } + for _, in := range cases { + once, err := Normalize(in) + if err != nil { + t.Fatalf("first Normalize(%q) errored: %v", in, err) + } + twice, err := Normalize(once) + if err != nil { + t.Fatalf("second Normalize(%q) errored: %v", once, err) + } + if once != twice { + t.Errorf("not idempotent for %q\n once: %q\n twice: %q", in, once, twice) + } + } +} + +func TestAllowedTypes(t *testing.T) { + for _, typ := range []string{"feat", "fix", "docs", "style", "refactor", "perf", "test", "build", "ci", "chore", "revert"} { + if !isAllowedType(typ) { + t.Errorf("expected %q to be an allowed type", typ) + } + } + if isAllowedType("wip") { + t.Error("did not expect wip to be allowed") + } +} diff --git a/scripts/commit-msg.sh b/scripts/commit-msg.sh new file mode 100755 index 0000000..c493c8f --- /dev/null +++ b/scripts/commit-msg.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +# commit-msg hook for nightshift +# +# Enforces Conventional Commits on every commit message and rewrites the +# message file into canonical form before the commit is created. Messages that +# cannot be normalized (missing/unknown type, capitalized or overlong subject) +# are rejected with a non-zero exit so the commit is aborted. +# +# Install: +# make install-hooks +# # or manually: +# ln -sf ../../scripts/commit-msg.sh .git/hooks/commit-msg +set -u + +if [[ $# -lt 1 ]]; then + echo "usage: commit-msg " >&2 + exit 1 +fi + +MSG_FILE="$1" + +# Resolve the nightshift binary: prefer the one on $PATH, fall back to +# running the current source tree. +NIGHTSHIFT="$(command -v nightshift || true)" +if [[ -z "$NIGHTSHIFT" ]]; then + NIGHTSHIFT="go run github.com/marcus/nightshift/cmd/nightshift" +fi + +ERR_FILE="$(mktemp -t nightshift-commit-msg)" +trap 'rm -f "$ERR_FILE"' EXIT + +NORMALIZED="$($NIGHTSHIFT commit normalize --file "$MSG_FILE" 2>"$ERR_FILE")" +STATUS=$? + +if [[ $STATUS -ne 0 ]]; then + echo "🪡 commit-msg: message does not follow Conventional Commits" >&2 + sed 's/^/ /' "$ERR_FILE" >&2 + echo "" >&2 + echo " Expected format: (): " >&2 + echo " Types: feat fix docs style refactor perf test build ci chore revert" >&2 + echo " (rewrite your message, or bypass with: git commit --no-verify)" >&2 + exit 1 +fi + +# Rewrite the message file into canonical form. +printf '%s\n' "$NORMALIZED" > "$MSG_FILE" +echo "🪡 commit-msg: normalized" +exit 0 From 5f8aaef219e5d3d59559e5ab07a6760835c1e156 Mon Sep 17 00:00:00 2001 From: Lasse Larsen Date: Wed, 26 Aug 2026 02:22:57 +0200 Subject: [PATCH 2/2] fix(commits): restrict trailer handling to the final body block Address review feedback on the commit message normalizer: - wrapBody no longer hoists Token-shaped lines out of mid-body prose; git only recognizes trailers in the last block, so only the final block is kept verbatim as trailers (and only when every line in it is trailer-shaped); anything else is wrapped in place, preserving order - the CI commit-messages job uses git rev-list --no-merges so GitHub merge commits do not fail validation - the 72-char subject limit is checked after the trailing period is trimmed, matching the documented auto-fix - malformed scopes such as feat(a)b) or feat() are rejected with a new ErrInvalidScope sentinel Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- .github/workflows/ci.yml | 4 +- docs/commit-messages.md | 20 ++++--- internal/commits/normalizer.go | 88 ++++++++++++++++++++--------- internal/commits/normalizer_test.go | 33 +++++++++++ 4 files changed, 111 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee4c2b1..28cdc82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,7 +73,9 @@ jobs: RANGE="HEAD^..HEAD" fi status=0 - for sha in $(git rev-list "$RANGE"); do + # Merge commits (e.g. "Merge pull request #N") are excluded: they are + # generated by GitHub and never follow Conventional Commits. + for sha in $(git rev-list --no-merges "$RANGE"); do if ! git log -1 --format=%B "$sha" | go run ./cmd/nightshift commit normalize --check; then echo "::error title=Invalid commit message::$(git log -1 --format='%h %s' "$sha") does not follow Conventional Commits (docs/commit-messages.md)" status=1 diff --git a/docs/commit-messages.md b/docs/commit-messages.md index c02aa4d..66a2139 100644 --- a/docs/commit-messages.md +++ b/docs/commit-messages.md @@ -16,12 +16,14 @@ derive changelogs automatically. - **type** — one of `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert`. -- **scope** — optional, e.g. `fix(api): ...`. +- **scope** — optional, e.g. `fix(api): ...`; balanced parentheses and no + whitespace inside. - **subject** — lowercase, imperative mood, no trailing period, max 72 chars. - **body** — optional, wrapped at 72 columns, separated from the subject by a blank line. -- **trailers** — optional `Token: value` lines (e.g. `Signed-off-by:`); kept - verbatim, one per line. +- **trailers** — optional `Token: value` lines (e.g. `Signed-off-by:`) as the + final block of the message; kept verbatim, one per line. A `Token: value`- + shaped line elsewhere in the body is treated as prose. Examples: @@ -43,8 +45,10 @@ git log -1 --pretty=%B | nightshift commit normalize The command fixes the trivially fixable (whitespace, type casing, trailing period, body wrapping) and exits non-zero when a message cannot be normalized -(missing/unknown type, capitalized or overlong subject). Add `--check` to -validate only without printing the normalized message. +(missing/unknown type, malformed scope, capitalized or overlong subject). The +72-character subject limit is applied after the trailing period is trimmed, so +a 73-character subject ending in `.` is accepted. Add `--check` to validate +only without printing the normalized message. ## commit-msg hook @@ -60,5 +64,7 @@ normalizes your message file in place before the commit is created and rejects messages that cannot be fixed automatically. Bypass it with `git commit --no-verify`. -CI runs the same validation over every commit in a pull request, so a -non-conforming message fails the build even without the local hook. +CI runs the same validation over every non-merge commit in a pull request, so +a non-conforming message fails the build even without the local hook. Merge +commits are skipped: they are generated by GitHub and do not follow +Conventional Commits. diff --git a/internal/commits/normalizer.go b/internal/commits/normalizer.go index cf9eaa4..78e0d11 100644 --- a/internal/commits/normalizer.go +++ b/internal/commits/normalizer.go @@ -11,7 +11,8 @@ // The normalizer is intentionally strict but constructive: rather than silently // accepting malformed input it fixes the trivially fixable (whitespace, type // casing, trailing punctuation, body wrapping) and rejects anything that needs -// a human decision (missing type, unknown type, missing subject). +// a human decision (missing type, unknown type, malformed scope, missing +// subject). package commits import ( @@ -51,6 +52,9 @@ var ( // ErrMissingType is returned when the subject line is not a Conventional // Commit (no type prefix before the colon). ErrMissingType = errors.New("commit message must start with a conventional commit type") + // ErrInvalidScope is returned when a scope is present but malformed + // (empty, unbalanced parentheses, or containing whitespace). + ErrInvalidScope = errors.New("commit scope is malformed") // ErrUnknownType is returned when the type prefix is not in the allowed set. ErrUnknownType = errors.New("commit type is not in the allowed set") // ErrMissingSubject is returned when the type prefix is present but no @@ -83,8 +87,6 @@ func Normalize(msg string) (string, error) { return "", err } - subject = cleanSubject(subject) - var b strings.Builder b.WriteString(formatHeader(typ, scope, subject)) @@ -121,7 +123,10 @@ func stripComments(msg string) []string { } // parseHeader splits the first line into its Conventional Commit components and -// validates them. The returned type is lower-cased to match the allowed set. +// validates them. The returned type is lower-cased to match the allowed set and +// the subject is returned in cleaned form, so the length and capitalization +// checks apply to the subject as it will actually be written (e.g. a 73-rune +// subject ending in '.' trims to a conforming 72 rather than being rejected). func parseHeader(header string) (typ, scope, subject string, err error) { header = strings.TrimSpace(header) colon := strings.Index(header, ":") @@ -140,11 +145,13 @@ func parseHeader(header string) (typ, scope, subject string, err error) { if open := strings.Index(prefix, "("); open > 0 && strings.HasSuffix(prefix, ")") { typ = prefix[:open] scope = prefix[open+1 : len(prefix)-1] + if scope == "" || strings.ContainsAny(scope, "() \t") { + return "", "", "", fmt.Errorf("%w: %q", ErrInvalidScope, scope) + } } else { typ = prefix } typ = strings.ToLower(strings.TrimSpace(typ)) - scope = strings.TrimSpace(scope) if typ == "" { return "", "", "", ErrMissingType @@ -152,7 +159,8 @@ func parseHeader(header string) (typ, scope, subject string, err error) { if !isAllowedType(typ) { return "", "", "", fmt.Errorf("%w: %q", ErrUnknownType, typ) } - if strings.TrimSpace(subject) == "" { + subject = cleanSubject(subject) + if subject == "" { return "", "", "", ErrMissingSubject } if utf8.RuneCountInString(subject) > MaxSubjectLength { @@ -184,36 +192,64 @@ func formatHeader(typ, scope, subject string) string { // wrapBody collapses runs of blank lines, preserves paragraph breaks (a single // blank line), and hard-wraps each paragraph to width. Git trailer lines // ("Token: value" such as "Signed-off-by:" or "Nightshift-Task:") are kept -// verbatim and unwrapped; a run of consecutive trailers stays a single -// newline-separated block, as git's own trailer convention requires. +// verbatim and unwrapped. Git only recognizes trailers in the last block of a +// message, so only the final block is treated as a trailer block — and only +// when every line in it is trailer-shaped; a "Token: value"-shaped line +// anywhere else is ordinary prose and is wrapped in place, never reordered. func wrapBody(body []string, width int) string { - var paragraphs []string - var cur []string - var trailers []string - flush := func() { - if len(cur) > 0 { - paragraphs = append(paragraphs, wrapParagraph(strings.Join(cur, " "), width)) - cur = nil + blocks := splitBlocks(body) + if len(blocks) == 0 { + return "" + } + + // The last block is a trailer block only when all of its lines look like + // trailers; a mixed block is prose and must not be reordered. + last := blocks[len(blocks)-1] + isTrailerBlock := true + for _, l := range last { + if !isTrailerLine(l) { + isTrailerBlock = false + break } - if len(trailers) > 0 { - paragraphs = append(paragraphs, strings.Join(trailers, "\n")) - trailers = nil + } + + prose := blocks + if isTrailerBlock { + prose = blocks[:len(blocks)-1] + } + var paragraphs []string + for _, b := range prose { + paragraphs = append(paragraphs, wrapParagraph(strings.Join(b, " "), width)) + } + if isTrailerBlock { + var trailers []string + for _, l := range last { + trailers = append(trailers, strings.TrimSpace(l)) } + paragraphs = append(paragraphs, strings.Join(trailers, "\n")) } + return strings.Join(paragraphs, "\n\n") +} + +// splitBlocks splits body lines into blocks separated by blank lines, trimming +// each line and dropping empty blocks. +func splitBlocks(body []string) [][]string { + var blocks [][]string + var cur []string for _, l := range body { if strings.TrimSpace(l) == "" { - flush() - continue - } - if isTrailerLine(l) { - trailers = append(trailers, strings.TrimSpace(l)) + if len(cur) > 0 { + blocks = append(blocks, cur) + cur = nil + } continue } cur = append(cur, strings.TrimSpace(l)) } - flush() - - return strings.Join(paragraphs, "\n\n") + if len(cur) > 0 { + blocks = append(blocks, cur) + } + return blocks } // isTrailerLine reports whether l looks like a git trailer: a bare token diff --git a/internal/commits/normalizer_test.go b/internal/commits/normalizer_test.go index a9cacd8..06511da 100644 --- a/internal/commits/normalizer_test.go +++ b/internal/commits/normalizer_test.go @@ -56,6 +56,27 @@ func TestNormalize(t *testing.T) { "Signed-off-by: Jane \n" + "Nightshift-Task: commit-normalize", }, + { + name: "trailer-shaped line mid-body stays prose in place", + in: "fix(api): retry on 429\n\nbefore the note this paragraph has some words\nNote: this line looks like a trailer but is mid-body prose\nand the paragraph continues after it unchanged in order", + want: "fix(api): retry on 429\n\n" + + "before the note this paragraph has some words Note: this line looks like\n" + + "a trailer but is mid-body prose and the paragraph continues after it\n" + + "unchanged in order", + }, + { + name: "trailer-shaped block followed by prose is wrapped as prose", + in: "chore: tidy\n\nSigned-off-by: Jane \n\none closing paragraph of prose", + want: "chore: tidy\n\n" + + "Signed-off-by: Jane " + + "\n\n" + + "one closing paragraph of prose", + }, + { + name: "trailing period trimmed before subject length is checked", + in: "feat: " + strings.Repeat("a", MaxSubjectLength) + ".", + want: "feat: " + strings.Repeat("a", MaxSubjectLength), + }, { name: "strips git comment lines", in: "chore: tidy\n# please enter the commit message\n\nbody here", @@ -86,6 +107,16 @@ func TestNormalize(t *testing.T) { in: "feat: " + strings.Repeat("a", MaxSubjectLength+1), wantErr: ErrSubjectTooLong, }, + { + name: "malformed scope with unbalanced parens rejected", + in: "feat(a)b): add thing", + wantErr: ErrInvalidScope, + }, + { + name: "empty scope rejected", + in: "feat(): add thing", + wantErr: ErrInvalidScope, + }, { name: "empty message rejected", in: "\n\n# only comments\n \n", @@ -121,6 +152,8 @@ func TestNormalizeIdempotent(t *testing.T) { "fix(api): handle nil response\n\nLong body that explains the fix in more detail than the subject alone can manage so that we exercise the wrapping path too and then some more words here.", "docs: update README\n\nfirst paragraph\n\nsecond paragraph stays separate", "chore: tidy\n\nsome prose that wraps because it is long enough to need it across columns\n\nSigned-off-by: Jane ", + "fix: keep prose order\n\na paragraph mentioning Note: inline that is long enough that the wrapping code has to run over it and fold it across several lines", + "chore: trailers last\n\nprose paragraph\n\nSigned-off-by: Jane \nReviewed-by: Bob ", } for _, in := range cases { once, err := Normalize(in)