From de1aeba2406e2ef7e14132dc46a69ac410183bbf Mon Sep 17 00:00:00 2001 From: ssmanji89 Date: Thu, 24 Sep 2026 03:18:03 -0500 Subject: [PATCH 1/3] feat(commitmsg): normalize commit messages Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- Makefile | 4 +- README.md | 4 +- cmd/nightshift/commands/commitmsg.go | 90 ++++++ docs/COMMIT_CONVENTION.md | 56 ++++ internal/commitmsg/commitmsg.go | 425 +++++++++++++++++++++++++++ internal/commitmsg/commitmsg_test.go | 93 ++++++ scripts/commit-msg.sh | 21 ++ 7 files changed, 691 insertions(+), 2 deletions(-) create mode 100644 cmd/nightshift/commands/commitmsg.go create mode 100644 docs/COMMIT_CONVENTION.md create mode 100644 internal/commitmsg/commitmsg.go create mode 100644 internal/commitmsg/commitmsg_test.go create mode 100755 scripts/commit-msg.sh diff --git a/Makefile b/Makefile index 088be01..f34b481 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 pre-commit and commit-msg hooks" @echo " help - Show this help" # Install git pre-commit hook 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..54e2443 100644 --- a/README.md +++ b/README.md @@ -260,7 +260,7 @@ Each task has a default cooldown interval to prevent the same task from running ### Pre-commit hooks -Install the git pre-commit hook to catch formatting and vet issues before pushing: +Install the git pre-commit and commit-msg hooks to catch formatting, vet, and commit message issues: ```bash make install-hooks @@ -273,6 +273,8 @@ This symlinks `scripts/pre-commit.sh` into `.git/hooks/pre-commit`. The hook run To bypass in a pinch: `git commit --no-verify` +See [docs/COMMIT_CONVENTION.md](docs/COMMIT_CONVENTION.md) for the commit message format, trailer rules, and the `commit-msg` hook. + ## Uninstalling ```bash diff --git a/cmd/nightshift/commands/commitmsg.go b/cmd/nightshift/commands/commitmsg.go new file mode 100644 index 0000000..9aa0e1e --- /dev/null +++ b/cmd/nightshift/commands/commitmsg.go @@ -0,0 +1,90 @@ +package commands + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/marcus/nightshift/internal/commitmsg" + "github.com/spf13/cobra" +) + +var commitMsgCheck bool + +var commitMsgCmd = &cobra.Command{ + Use: "commit-msg [--check] ", + Short: "Normalize a Git commit message file", + Args: cobra.ExactArgs(1), + RunE: runCommitMsg, +} + +func init() { + commitMsgCmd.Flags().BoolVar(&commitMsgCheck, "check", false, "validate without changing the file") + rootCmd.AddCommand(commitMsgCmd) +} + +func runCommitMsg(cmd *cobra.Command, args []string) error { + path := args[0] + input, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("read commit message %s: %w", path, err) + } + if commitMsgCheck { + if err := commitmsg.Validate(string(input)); err != nil { + return fmt.Errorf("validate commit message: %w", err) + } + return nil + } + + normalized, err := commitmsg.Normalize(string(input)) + if err != nil { + return fmt.Errorf("normalize commit message: %w", err) + } + if normalized == string(input) { + return nil + } + if err := writeCommitMessageAtomically(path, []byte(normalized)); err != nil { + return fmt.Errorf("write commit message %s: %w", path, err) + } + return nil +} + +func writeCommitMessageAtomically(path string, content []byte) (err error) { + info, err := os.Stat(path) + if err != nil { + return err + } + temp, err := os.CreateTemp(filepath.Dir(path), ".commit-msg-*") + if err != nil { + return err + } + tempName := temp.Name() + closed := false + defer func() { + if !closed { + if closeErr := temp.Close(); err == nil && closeErr != nil { + err = closeErr + } + } + if err != nil { + _ = os.Remove(tempName) + } + }() + if err := temp.Chmod(info.Mode().Perm()); err != nil { + return err + } + if _, err := temp.Write(content); err != nil { + return err + } + if err := temp.Sync(); err != nil { + return err + } + if err := temp.Close(); err != nil { + return err + } + closed = true + if err := os.Rename(tempName, path); err != nil { + return err + } + return nil +} diff --git a/docs/COMMIT_CONVENTION.md b/docs/COMMIT_CONVENTION.md new file mode 100644 index 0000000..71130e5 --- /dev/null +++ b/docs/COMMIT_CONVENTION.md @@ -0,0 +1,56 @@ +# Commit Convention + +Nightshift uses a normalized Conventional Commits format. + +## Format + +```text +type(scope)!: lowercase description + +wrapped body text + +Trailer: value +``` + +The type is required. The scope is optional. The `!` marker is optional and marks a breaking change. The description is lowercase, has no terminal punctuation, and the complete subject is at most 72 characters. + +Allowed types are `build`, `chore`, `ci`, `docs`, `feat`, `fix`, `perf`, `refactor`, `revert`, `style`, and `test`. + +The normalizer infers `feat` from `add`, `implement`, `introduce`, or `support` subjects. It infers `fix` from `fix`, `bug`, `repair`, `resolve`, or `handle` subjects. It maps common first words to the other allowed types and uses `chore` by default. + +The body starts after one blank line. Body paragraphs wrap at 72 characters. The normalizer preserves body content that it can repair without data loss. + +## Trailers + +Trailers remain at the end of the message. The normalizer preserves the first trailer for each case-insensitive token and removes later duplicates. It preserves trailer values, including `Nightshift-Task` and `Nightshift-Ref`. + +Breaking changes can use either `!` in the subject or a `BREAKING CHANGE` trailer. The normalizer adds `!` when a breaking trailer exists. + +## Examples + +```text +feat(cli): add commit message normalization + +Normalize Git commit messages before they enter project history. + +Nightshift-Task: commit-normalize +Nightshift-Ref: https://github.com/marcus/nightshift +``` + +```text +fix!: remove the legacy config format + +BREAKING CHANGE: migrate existing config files before upgrading. +``` + +## Installation and bypass + +Install both the `pre-commit` and `commit-msg` hooks with: + +```bash +make install-hooks +``` + +The `commit-msg` hook normalizes the message file atomically. Use `nightshift commit-msg --check ` to validate a message without changing it. + +Use `git commit --no-verify` to bypass both hooks for an exceptional commit. diff --git a/internal/commitmsg/commitmsg.go b/internal/commitmsg/commitmsg.go new file mode 100644 index 0000000..21871ba --- /dev/null +++ b/internal/commitmsg/commitmsg.go @@ -0,0 +1,425 @@ +// Package commitmsg parses, normalizes, and validates commit messages. +package commitmsg + +import ( + "errors" + "fmt" + "regexp" + "strings" + "unicode" +) + +const ( + // SubjectLimit is the maximum length of a formatted commit subject. + SubjectLimit = 72 +) + +var ( + ErrEmptyMessage = errors.New("commit message is empty") + ErrInvalidMessage = errors.New("commit message is invalid") + ErrSubjectTooLong = errors.New("commit subject exceeds 72 characters") + + conventionalPattern = regexp.MustCompile(`^([A-Za-z][A-Za-z0-9-]*)(?:\(([^()\r\n]+)\))?(!)?:[ \t]*(.*)$`) + trailerPattern = regexp.MustCompile(`^([A-Za-z][A-Za-z0-9-]*(?: [A-Za-z][A-Za-z0-9-]*)*):[ \t]*(.+)$`) +) + +var allowedTypes = map[string]struct{}{ + "build": {}, + "chore": {}, + "ci": {}, + "docs": {}, + "feat": {}, + "fix": {}, + "perf": {}, + "refactor": {}, + "revert": {}, + "style": {}, + "test": {}, +} + +// Trailer is a Git trailer at the end of a commit message. +type Trailer struct { + Token string + Value string +} + +// Message is the structured form of a commit message. +type Message struct { + Type string + Scope string + Breaking bool + Description string + Body []string + Trailers []Trailer +} + +// Parse parses a commit message and repairs fields that can be normalized safely. +func Parse(input string) (Message, error) { + input = strings.ReplaceAll(input, "\r\n", "\n") + input = strings.ReplaceAll(input, "\r", "\n") + lines := strings.Split(input, "\n") + for len(lines) > 0 && strings.TrimSpace(lines[0]) == "" { + lines = lines[1:] + } + for len(lines) > 0 && strings.TrimSpace(lines[len(lines)-1]) == "" { + lines = lines[:len(lines)-1] + } + if len(lines) == 0 { + return Message{}, ErrEmptyMessage + } + + subject := strings.TrimSpace(lines[0]) + message := Message{} + if matches := conventionalPattern.FindStringSubmatch(subject); matches != nil { + message.Type = strings.ToLower(matches[1]) + if _, ok := allowedTypes[message.Type]; !ok { + return Message{}, fmt.Errorf("%w: unsupported type %q", ErrInvalidMessage, matches[1]) + } + message.Scope = normalizeScope(matches[2]) + message.Breaking = matches[3] == "!" + message.Description = normalizeDescription(matches[4]) + if message.Description == "" { + return Message{}, fmt.Errorf("%w: subject description is empty", ErrInvalidMessage) + } + } else { + if strings.Contains(subject, ":") && looksLikeConventionalSubject(subject) { + return Message{}, fmt.Errorf("%w: malformed conventional subject", ErrInvalidMessage) + } + message.Type = inferType(subject) + message.Description = normalizeDescription(inferredDescription(subject, message.Type)) + if message.Description == "" { + return Message{}, fmt.Errorf("%w: subject description is empty", ErrInvalidMessage) + } + } + + bodyLines := append([]string(nil), lines[1:]...) + message.Trailers, bodyLines = parseTrailers(bodyLines) + message.Body = normalizeBody(bodyLines) + for _, trailer := range message.Trailers { + if strings.EqualFold(trailer.Token, "BREAKING CHANGE") { + message.Breaking = true + break + } + } + return message, nil +} + +// Normalize returns the canonical commit message, including a final newline. +func Normalize(input string) (string, error) { + message, err := Parse(input) + if err != nil { + return "", err + } + return Format(message) +} + +// Validate checks that input already uses the canonical format. +func Validate(input string) error { + normalized, err := Normalize(input) + if err != nil { + return err + } + if input != normalized { + return fmt.Errorf("%w: run the commit-msg normalizer", ErrInvalidMessage) + } + return nil +} + +// Format formats a parsed message with the canonical subject, body, and trailers. +func Format(message Message) (string, error) { + message.Type = strings.ToLower(strings.TrimSpace(message.Type)) + if _, ok := allowedTypes[message.Type]; !ok { + return "", fmt.Errorf("%w: unsupported type %q", ErrInvalidMessage, message.Type) + } + message.Scope = normalizeScope(message.Scope) + message.Description = normalizeDescription(message.Description) + if message.Description == "" { + return "", fmt.Errorf("%w: subject description is empty", ErrInvalidMessage) + } + + subject := message.Type + if message.Scope != "" { + subject += "(" + message.Scope + ")" + } + if message.Breaking { + subject += "!" + } + subject += ": " + message.Description + if runeLen(subject) > SubjectLimit { + return "", ErrSubjectTooLong + } + + trailers := deduplicateTrailers(message.Trailers) + for _, trailer := range trailers { + if trailer.Token == "" || trailer.Value == "" || !trailerPattern.MatchString(trailer.Token+": value") { + return "", fmt.Errorf("%w: malformed trailer %q", ErrInvalidMessage, trailer.Token) + } + } + + sections := []string{subject} + body := normalizeBody(message.Body) + if len(body) > 0 { + sections = append(sections, strings.Join(body, "\n")) + } + if len(trailers) > 0 { + trailerLines := make([]string, 0, len(trailers)) + for _, trailer := range trailers { + trailerLines = append(trailerLines, trailer.Token+": "+trailer.Value) + } + if len(sections) == 1 { + sections = append(sections, strings.Join(trailerLines, "\n")) + } else { + sections[len(sections)-1] += "\n" + strings.Join(trailerLines, "\n") + } + } + return strings.Join(sections, "\n\n") + "\n", nil +} + +func looksLikeConventionalSubject(subject string) bool { + colon := strings.IndexByte(subject, ':') + if colon <= 0 { + return false + } + prefix := strings.TrimSpace(subject[:colon]) + if prefix == "" { + return false + } + return !strings.ContainsAny(prefix, " \t") +} + +func normalizeScope(scope string) string { + scope = strings.ToLower(strings.Join(strings.Fields(scope), "-")) + return strings.Trim(scope, "-_") +} + +func normalizeDescription(description string) string { + description = strings.ToLower(strings.Join(strings.Fields(description), " ")) + description = strings.TrimLeftFunc(description, unicode.IsPunct) + description = strings.TrimRightFunc(description, func(r rune) bool { + return unicode.IsPunct(r) || unicode.IsSpace(r) + }) + return description +} + +func inferType(subject string) string { + lower := strings.ToLower(strings.TrimSpace(subject)) + first := lower + if fields := strings.Fields(lower); len(fields) > 0 { + first = strings.Trim(fields[0], "[]():,;") + } + switch { + case first == "test" || first == "tests" || strings.HasPrefix(lower, "add test"): + return "test" + case first == "fix" || first == "bug" || first == "repair" || first == "resolve" || first == "handle": + return "fix" + case first == "add" || first == "implement" || first == "introduce" || first == "support": + return "feat" + case first == "doc" || first == "docs" || first == "document" || first == "readme": + return "docs" + case first == "refactor" || first == "rework": + return "refactor" + case first == "perf" || first == "optimize": + return "perf" + case first == "style" || first == "format": + return "style" + case first == "build" || first == "compile": + return "build" + case first == "ci" || first == "pipeline": + return "ci" + case first == "revert": + return "revert" + default: + return "chore" + } +} + +func inferredDescription(subject, messageType string) string { + if messageType == "chore" { + return subject + } + fields := strings.Fields(subject) + if len(fields) < 2 { + return subject + } + first := strings.ToLower(strings.Trim(fields[0], "[]():,;")) + if messageType == "test" && strings.HasPrefix(strings.ToLower(strings.TrimSpace(subject)), "add test") { + return subject + } + if _, ok := map[string]string{ + "add": "feat", "implement": "feat", "introduce": "feat", "support": "feat", + "fix": "fix", "bug": "fix", "repair": "fix", "resolve": "fix", "handle": "fix", + "doc": "docs", "docs": "docs", "document": "docs", "readme": "docs", + "refactor": "refactor", "rework": "refactor", "perf": "perf", "optimize": "perf", + "style": "style", "format": "style", "build": "build", "compile": "build", + "ci": "ci", "pipeline": "ci", "revert": "revert", "test": "test", "tests": "test", + }[first]; ok { + return strings.Join(fields[1:], " ") + } + return subject +} + +func parseTrailers(lines []string) ([]Trailer, []string) { + end := len(lines) + for end > 0 && strings.TrimSpace(lines[end-1]) == "" { + end-- + } + start := end + foundTrailer := false + separatorSeen := false + for start > 0 { + if trailerPattern.MatchString(lines[start-1]) { + start-- + foundTrailer = true + separatorSeen = false + continue + } + if foundTrailer && !separatorSeen && strings.TrimSpace(lines[start-1]) == "" { + start-- + separatorSeen = true + continue + } + if foundTrailer && (strings.HasPrefix(lines[start-1], " ") || strings.HasPrefix(lines[start-1], "\t")) { + start-- + continue + } + break + } + if start == end { + return nil, lines + } + + trailers := make([]Trailer, 0, end-start) + for _, line := range lines[start:end] { + if matches := trailerPattern.FindStringSubmatch(line); matches != nil { + trailers = append(trailers, Trailer{Token: matches[1], Value: strings.TrimSpace(matches[2])}) + } else if len(trailers) > 0 { + trailers[len(trailers)-1].Value += "\n" + strings.TrimSpace(line) + } + } + return deduplicateTrailers(trailers), lines[:start] +} + +func deduplicateTrailers(trailers []Trailer) []Trailer { + result := make([]Trailer, 0, len(trailers)) + seen := make(map[string]struct{}, len(trailers)) + for _, trailer := range trailers { + trailer.Token = strings.TrimSpace(trailer.Token) + trailer.Value = strings.TrimSpace(trailer.Value) + key := strings.ToLower(trailer.Token) + if trailer.Token == "" || trailer.Value == "" { + continue + } + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + result = append(result, trailer) + } + return result +} + +func normalizeBody(lines []string) []string { + for len(lines) > 0 && strings.TrimSpace(lines[0]) == "" { + lines = lines[1:] + } + for len(lines) > 0 && strings.TrimSpace(lines[len(lines)-1]) == "" { + lines = lines[:len(lines)-1] + } + if len(lines) == 0 { + return nil + } + + result := make([]string, 0, len(lines)) + paragraph := make([]string, 0) + flush := func() { + if len(paragraph) == 0 { + return + } + result = append(result, wrapParagraph(paragraph)...) + paragraph = paragraph[:0] + } + for _, line := range lines { + line = strings.TrimRightFunc(line, unicode.IsSpace) + if strings.TrimSpace(line) == "" { + flush() + if len(result) > 0 && result[len(result)-1] != "" { + result = append(result, "") + } + continue + } + paragraph = append(paragraph, line) + } + flush() + for len(result) > 0 && result[len(result)-1] == "" { + result = result[:len(result)-1] + } + return result +} + +func wrapParagraph(lines []string) []string { + joined := strings.Join(lines, " ") + joined = strings.Join(strings.Fields(joined), " ") + if joined == "" { + return nil + } + prefix := "" + content := joined + if len(lines) == 1 { + original := strings.TrimSpace(lines[0]) + if len(original) >= 2 && (strings.HasPrefix(original, "- ") || strings.HasPrefix(original, "* ") || strings.HasPrefix(original, "+ ")) { + prefix, content = original[:2], original[2:] + } else { + for i, r := range original { + if r == '.' || r == ')' { + if i+1 < len(original) && original[i+1] == ' ' { + prefix, content = original[:i+2], original[i+2:] + } + break + } + } + } + } + words := strings.Fields(content) + if len(words) == 0 { + return nil + } + firstWidth := SubjectLimit - runeLen(prefix) + if firstWidth < 1 { + firstWidth = SubjectLimit + } + wrapped := wrapWords(words, firstWidth) + if prefix == "" || len(wrapped) == 0 { + return wrapped + } + wrapped[0] = prefix + wrapped[0] + indent := strings.Repeat(" ", runeLen(prefix)) + for i := 1; i < len(wrapped); i++ { + wrapped[i] = indent + wrapped[i] + } + return wrapped +} + +func wrapWords(words []string, width int) []string { + result := make([]string, 0, len(words)) + line := "" + for _, word := range words { + if line == "" { + line = word + continue + } + if runeLen(line)+1+runeLen(word) <= width { + line += " " + word + continue + } + result = append(result, line) + line = word + } + if line != "" { + result = append(result, line) + } + return result +} + +func runeLen(value string) int { + return len([]rune(value)) +} diff --git a/internal/commitmsg/commitmsg_test.go b/internal/commitmsg/commitmsg_test.go new file mode 100644 index 0000000..9a77c29 --- /dev/null +++ b/internal/commitmsg/commitmsg_test.go @@ -0,0 +1,93 @@ +package commitmsg + +import ( + "errors" + "strings" + "testing" +) + +func TestNormalize(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {"conventional input is idempotent", "feat(parser): add support\n", "feat(parser): add support\n"}, + {"bare subject", "Improve the setup flow\n", "chore: improve the setup flow\n"}, + {"inferred types", "Fix broken login\n", "fix: broken login\n"}, + {"punctuation cleanup", "feat: Add the API!\n", "feat: add the api\n"}, + {"body spacing and wrapping", "fix: repair login\nbody line with extra spaces\n\n\n", "fix: repair login\n\nbody line with extra spaces\n"}, + {"breaking changes", "feat!: replace the config format\n\nBREAKING CHANGE: migrate existing files\n", "feat!: replace the config format\n\nBREAKING CHANGE: migrate existing files\n"}, + {"trailer preservation", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n", "fix: repair login\n\nbody\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n"}, + {"duplicate trailer removal", "fix: repair login\n\nSigned-off-by: A\nSigned-off-by: B\n", "fix: repair login\n\nSigned-off-by: A\n"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := Normalize(test.input) + if err != nil { + t.Fatalf("Normalize() error = %v", err) + } + if got != test.want { + t.Errorf("Normalize() = %q, want %q", got, test.want) + } + again, err := Normalize(got) + if err != nil { + t.Fatalf("second Normalize() error = %v", err) + } + if again != got { + t.Errorf("Normalize() is not idempotent: second result = %q", again) + } + }) + } +} + +func TestNormalizeRejectsInvalidInput(t *testing.T) { + tests := []struct { + name string + input string + err error + }{ + {"empty input", "\n\t\n", ErrEmptyMessage}, + {"long subject", "feat: " + strings.Repeat("x", 70) + "\n", ErrSubjectTooLong}, + {"unknown conventional type", "unknown: do the thing\n", ErrInvalidMessage}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, err := Normalize(test.input) + if !errors.Is(err, test.err) { + t.Fatalf("Normalize() error = %v, want errors.Is(..., %v)", err, test.err) + } + }) + } +} + +func TestValidate(t *testing.T) { + if err := Validate("fix: repair login\n"); err != nil { + t.Fatalf("Validate() error = %v", err) + } + if err := Validate("Fix broken login\n"); !errors.Is(err, ErrInvalidMessage) { + t.Fatalf("Validate() error = %v, want ErrInvalidMessage", err) + } +} + +func TestFormat(t *testing.T) { + formatted, err := Format(Message{ + Type: "feat", + Scope: "CLI", + Breaking: true, + Description: "Add commit message support", + Body: []string{"This body is wrapped when it contains enough words to exceed the configured line length for commit messages."}, + Trailers: []Trailer{{Token: "Nightshift-Task", Value: "commit-normalize"}}, + }) + if err != nil { + t.Fatalf("Format() error = %v", err) + } + if !strings.HasPrefix(formatted, "feat(cli)!: add commit message support\n\n") { + t.Fatalf("Format() = %q, missing canonical subject", formatted) + } + for _, line := range strings.Split(strings.TrimSuffix(formatted, "\n"), "\n") { + if !strings.Contains(line, "Nightshift-Task:") && len([]rune(line)) > SubjectLimit { + t.Errorf("formatted line exceeds limit: %q", line) + } + } +} diff --git a/scripts/commit-msg.sh b/scripts/commit-msg.sh new file mode 100755 index 0000000..5c9adc6 --- /dev/null +++ b/scripts/commit-msg.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +# commit-msg hook for nightshift +# Install: make install-hooks +set -euo pipefail + +if [[ $# -ne 1 ]]; then + printf 'commit-msg: expected the commit message file path\n' >&2 + exit 2 +fi + +repo_root=$(git rev-parse --show-toplevel) + +if [[ -x "$repo_root/nightshift" ]]; then + exec "$repo_root/nightshift" commit-msg "$1" +fi + +if command -v nightshift >/dev/null 2>&1 && nightshift commit-msg --help >/dev/null 2>&1; then + exec nightshift commit-msg "$1" +fi + +exec go run "$repo_root/cmd/nightshift" commit-msg "$1" From dc4a7855960a697552bd7f9722d0afe1069eb3a2 Mon Sep 17 00:00:00 2001 From: ssmanji89 Date: Thu, 24 Sep 2026 03:35:47 -0500 Subject: [PATCH 2/3] fix(commitmsg): repair trailer formatting and hook fallback Keep trailers in a separate canonical block and run the fallback CLI from the repository module. Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- cmd/nightshift/commands/commitmsg_test.go | 60 +++++++++++++++++++++++ docs/COMMIT_CONVENTION.md | 2 +- internal/commitmsg/commitmsg.go | 6 +-- internal/commitmsg/commitmsg_test.go | 3 +- scripts/commit-msg.sh | 3 +- 5 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 cmd/nightshift/commands/commitmsg_test.go diff --git a/cmd/nightshift/commands/commitmsg_test.go b/cmd/nightshift/commands/commitmsg_test.go new file mode 100644 index 0000000..ca7142a --- /dev/null +++ b/cmd/nightshift/commands/commitmsg_test.go @@ -0,0 +1,60 @@ +package commands + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/marcus/nightshift/internal/commitmsg" +) + +func TestRunCommitMsgNormalizesFileAtomically(t *testing.T) { + messagePath := filepath.Join(t.TempDir(), "COMMIT_EDITMSG") + if err := os.WriteFile(messagePath, []byte("Improve setup\n"), 0o600); err != nil { + t.Fatal(err) + } + + commitMsgCheck = false + t.Cleanup(func() { commitMsgCheck = false }) + if err := runCommitMsg(nil, []string{messagePath}); err != nil { + t.Fatalf("runCommitMsg() error = %v", err) + } + + content, err := os.ReadFile(messagePath) + if err != nil { + t.Fatal(err) + } + if got, want := string(content), "chore: improve setup\n"; got != want { + t.Errorf("normalized content = %q, want %q", got, want) + } + info, err := os.Stat(messagePath) + if err != nil { + t.Fatal(err) + } + if got, want := info.Mode().Perm(), os.FileMode(0o600); got != want { + t.Errorf("file mode = %o, want %o", got, want) + } +} + +func TestRunCommitMsgCheckDoesNotChangeFile(t *testing.T) { + messagePath := filepath.Join(t.TempDir(), "COMMIT_EDITMSG") + original := []byte("Improve setup\n") + if err := os.WriteFile(messagePath, original, 0o600); err != nil { + t.Fatal(err) + } + + commitMsgCheck = true + t.Cleanup(func() { commitMsgCheck = false }) + err := runCommitMsg(nil, []string{messagePath}) + if !errors.Is(err, commitmsg.ErrInvalidMessage) { + t.Fatalf("runCommitMsg() error = %v, want ErrInvalidMessage", err) + } + content, err := os.ReadFile(messagePath) + if err != nil { + t.Fatal(err) + } + if string(content) != string(original) { + t.Errorf("check mode changed content to %q", content) + } +} diff --git a/docs/COMMIT_CONVENTION.md b/docs/COMMIT_CONVENTION.md index 71130e5..91251ff 100644 --- a/docs/COMMIT_CONVENTION.md +++ b/docs/COMMIT_CONVENTION.md @@ -18,7 +18,7 @@ Allowed types are `build`, `chore`, `ci`, `docs`, `feat`, `fix`, `perf`, `refact The normalizer infers `feat` from `add`, `implement`, `introduce`, or `support` subjects. It infers `fix` from `fix`, `bug`, `repair`, `resolve`, or `handle` subjects. It maps common first words to the other allowed types and uses `chore` by default. -The body starts after one blank line. Body paragraphs wrap at 72 characters. The normalizer preserves body content that it can repair without data loss. +The body starts after one blank line. Body paragraphs wrap at 72 characters. Trailers start after one blank line and remain at the end. The normalizer preserves body content that it can repair without data loss. ## Trailers diff --git a/internal/commitmsg/commitmsg.go b/internal/commitmsg/commitmsg.go index 21871ba..82dc2ab 100644 --- a/internal/commitmsg/commitmsg.go +++ b/internal/commitmsg/commitmsg.go @@ -166,11 +166,7 @@ func Format(message Message) (string, error) { for _, trailer := range trailers { trailerLines = append(trailerLines, trailer.Token+": "+trailer.Value) } - if len(sections) == 1 { - sections = append(sections, strings.Join(trailerLines, "\n")) - } else { - sections[len(sections)-1] += "\n" + strings.Join(trailerLines, "\n") - } + sections = append(sections, strings.Join(trailerLines, "\n")) } return strings.Join(sections, "\n\n") + "\n", nil } diff --git a/internal/commitmsg/commitmsg_test.go b/internal/commitmsg/commitmsg_test.go index 9a77c29..68f7932 100644 --- a/internal/commitmsg/commitmsg_test.go +++ b/internal/commitmsg/commitmsg_test.go @@ -18,7 +18,8 @@ func TestNormalize(t *testing.T) { {"punctuation cleanup", "feat: Add the API!\n", "feat: add the api\n"}, {"body spacing and wrapping", "fix: repair login\nbody line with extra spaces\n\n\n", "fix: repair login\n\nbody line with extra spaces\n"}, {"breaking changes", "feat!: replace the config format\n\nBREAKING CHANGE: migrate existing files\n", "feat!: replace the config format\n\nBREAKING CHANGE: migrate existing files\n"}, - {"trailer preservation", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n", "fix: repair login\n\nbody\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n"}, + {"trailer preservation", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n"}, + {"trailer block has a blank line after the body", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\n", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\n"}, {"duplicate trailer removal", "fix: repair login\n\nSigned-off-by: A\nSigned-off-by: B\n", "fix: repair login\n\nSigned-off-by: A\n"}, } for _, test := range tests { diff --git a/scripts/commit-msg.sh b/scripts/commit-msg.sh index 5c9adc6..caa3de5 100755 --- a/scripts/commit-msg.sh +++ b/scripts/commit-msg.sh @@ -18,4 +18,5 @@ if command -v nightshift >/dev/null 2>&1 && nightshift commit-msg --help >/dev/n exec nightshift commit-msg "$1" fi -exec go run "$repo_root/cmd/nightshift" commit-msg "$1" +cd "$repo_root" +exec go run ./cmd/nightshift commit-msg "$1" From ef96288a6495b03c71912edff5431af9caa9e3c9 Mon Sep 17 00:00:00 2001 From: ssmanji89 Date: Thu, 24 Sep 2026 03:53:56 -0500 Subject: [PATCH 3/3] fix(commitmsg): preserve valid message content Keep punctuation, body formatting, and distinct trailer values intact during normalization. Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- docs/COMMIT_CONVENTION.md | 2 +- internal/commitmsg/commitmsg.go | 44 ++++++++++++++++++---------- internal/commitmsg/commitmsg_test.go | 8 +++-- 3 files changed, 35 insertions(+), 19 deletions(-) diff --git a/docs/COMMIT_CONVENTION.md b/docs/COMMIT_CONVENTION.md index 91251ff..91cb8df 100644 --- a/docs/COMMIT_CONVENTION.md +++ b/docs/COMMIT_CONVENTION.md @@ -22,7 +22,7 @@ The body starts after one blank line. Body paragraphs wrap at 72 characters. Tra ## Trailers -Trailers remain at the end of the message. The normalizer preserves the first trailer for each case-insensitive token and removes later duplicates. It preserves trailer values, including `Nightshift-Task` and `Nightshift-Ref`. +Trailers remain at the end of the message. The normalizer removes later exact duplicates for each case-insensitive token and preserves distinct values. It preserves trailer values, including `Nightshift-Task` and `Nightshift-Ref`. Breaking changes can use either `!` in the subject or a `BREAKING CHANGE` trailer. The normalizer adds `!` when a breaking trailer exists. diff --git a/internal/commitmsg/commitmsg.go b/internal/commitmsg/commitmsg.go index 82dc2ab..7cebbbb 100644 --- a/internal/commitmsg/commitmsg.go +++ b/internal/commitmsg/commitmsg.go @@ -70,7 +70,7 @@ func Parse(input string) (Message, error) { subject := strings.TrimSpace(lines[0]) message := Message{} - if matches := conventionalPattern.FindStringSubmatch(subject); matches != nil { + if matches := conventionalPattern.FindStringSubmatch(subject); matches != nil && isConventionalType(matches[1]) { message.Type = strings.ToLower(matches[1]) if _, ok := allowedTypes[message.Type]; !ok { return Message{}, fmt.Errorf("%w: unsupported type %q", ErrInvalidMessage, matches[1]) @@ -180,7 +180,27 @@ func looksLikeConventionalSubject(subject string) bool { if prefix == "" { return false } - return !strings.ContainsAny(prefix, " \t") + if strings.ContainsAny(prefix, " \t") { + return false + } + lower := strings.ToLower(prefix) + if _, ok := allowedTypes[lower]; ok { + return true + } + if strings.HasSuffix(lower, "!") { + _, ok := allowedTypes[strings.TrimSuffix(lower, "!")] + return ok + } + if open := strings.IndexByte(lower, '('); open > 0 && strings.HasSuffix(lower, ")") { + _, ok := allowedTypes[lower[:open]] + return ok + } + return false +} + +func isConventionalType(typeName string) bool { + _, ok := allowedTypes[strings.ToLower(typeName)] + return ok || typeName == strings.ToLower(typeName) } func normalizeScope(scope string) string { @@ -190,7 +210,6 @@ func normalizeScope(scope string) string { func normalizeDescription(description string) string { description = strings.ToLower(strings.Join(strings.Fields(description), " ")) - description = strings.TrimLeftFunc(description, unicode.IsPunct) description = strings.TrimRightFunc(description, func(r rune) bool { return unicode.IsPunct(r) || unicode.IsSpace(r) }) @@ -301,7 +320,7 @@ func deduplicateTrailers(trailers []Trailer) []Trailer { for _, trailer := range trailers { trailer.Token = strings.TrimSpace(trailer.Token) trailer.Value = strings.TrimSpace(trailer.Value) - key := strings.ToLower(trailer.Token) + key := strings.ToLower(trailer.Token) + "\x00" + trailer.Value if trailer.Token == "" || trailer.Value == "" { continue } @@ -326,26 +345,19 @@ func normalizeBody(lines []string) []string { } result := make([]string, 0, len(lines)) - paragraph := make([]string, 0) - flush := func() { - if len(paragraph) == 0 { - return - } - result = append(result, wrapParagraph(paragraph)...) - paragraph = paragraph[:0] - } for _, line := range lines { - line = strings.TrimRightFunc(line, unicode.IsSpace) if strings.TrimSpace(line) == "" { - flush() if len(result) > 0 && result[len(result)-1] != "" { result = append(result, "") } continue } - paragraph = append(paragraph, line) + if runeLen(line) <= SubjectLimit { + result = append(result, line) + continue + } + result = append(result, wrapParagraph([]string{line})...) } - flush() for len(result) > 0 && result[len(result)-1] == "" { result = result[:len(result)-1] } diff --git a/internal/commitmsg/commitmsg_test.go b/internal/commitmsg/commitmsg_test.go index 68f7932..63ef252 100644 --- a/internal/commitmsg/commitmsg_test.go +++ b/internal/commitmsg/commitmsg_test.go @@ -14,13 +14,17 @@ func TestNormalize(t *testing.T) { }{ {"conventional input is idempotent", "feat(parser): add support\n", "feat(parser): add support\n"}, {"bare subject", "Improve the setup flow\n", "chore: improve the setup flow\n"}, + {"bare subject with colon", "URL: preserve the endpoint\n", "chore: url: preserve the endpoint\n"}, {"inferred types", "Fix broken login\n", "fix: broken login\n"}, {"punctuation cleanup", "feat: Add the API!\n", "feat: add the api\n"}, - {"body spacing and wrapping", "fix: repair login\nbody line with extra spaces\n\n\n", "fix: repair login\n\nbody line with extra spaces\n"}, + {"body spacing and wrapping", "fix: repair login\nbody line with extra spaces\n\n\n", "fix: repair login\n\nbody line with extra spaces\n"}, {"breaking changes", "feat!: replace the config format\n\nBREAKING CHANGE: migrate existing files\n", "feat!: replace the config format\n\nBREAKING CHANGE: migrate existing files\n"}, {"trailer preservation", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n"}, {"trailer block has a blank line after the body", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\n", "fix: repair login\n\nbody\n\nNightshift-Task: commit-normalize\n"}, - {"duplicate trailer removal", "fix: repair login\n\nSigned-off-by: A\nSigned-off-by: B\n", "fix: repair login\n\nSigned-off-by: A\n"}, + {"duplicate trailer removal", "fix: repair login\n\nSigned-off-by: A\nSigned-off-by: A\n", "fix: repair login\n\nSigned-off-by: A\n"}, + {"preserve leading description punctuation", "fix: #123 crash!\n", "fix: #123 crash\n"}, + {"preserve distinct repeated trailers", "feat: add authors\n\nCo-authored-by: A \nCo-authored-by: B \n", "feat: add authors\n\nCo-authored-by: A \nCo-authored-by: B \n"}, + {"preserve body formatting", "fix: repair login\n\nUse two spaces:\n code: value\n", "fix: repair login\n\nUse two spaces:\n code: value\n"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) {