From dd739dfc50ec8eb9189943601f7cbeb7a9c84807 Mon Sep 17 00:00:00 2001 From: Greg Gardner Date: Sat, 29 Aug 2026 02:11:32 -0700 Subject: [PATCH 1/2] feat(cli): add commit message normalizer Add an internal/commitmsg package that parses, validates and normalizes commit messages against one Conventional Commits format, and expose it three ways from a single canonical spec: a nightshift commit-msg subcommand, a commit-msg git hook installed by make install-hooks, and the orchestrator's plan and implement prompts. Scope note: this makes the repository enforce one format going forward. Rewriting historical commits is destructive and deliberately out of scope. Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- CHANGELOG.md | 11 + Makefile | 6 +- README.md | 29 + cmd/nightshift/commands/commitmsg.go | 150 ++++ cmd/nightshift/commands/commitmsg_test.go | 157 +++++ internal/commitmsg/commitmsg.go | 649 ++++++++++++++++++ internal/commitmsg/commitmsg_test.go | 503 ++++++++++++++ internal/commitmsg/spec.go | 103 +++ internal/orchestrator/orchestrator.go | 15 +- .../orchestrator_commitmsg_test.go | 81 +++ scripts/commit-msg.sh | 38 + website/docs/cli-reference.md | 26 + 12 files changed, 1758 insertions(+), 10 deletions(-) create mode 100644 cmd/nightshift/commands/commitmsg.go create mode 100644 cmd/nightshift/commands/commitmsg_test.go create mode 100644 internal/commitmsg/commitmsg.go create mode 100644 internal/commitmsg/commitmsg_test.go create mode 100644 internal/commitmsg/spec.go create mode 100644 internal/orchestrator/orchestrator_commitmsg_test.go create mode 100755 scripts/commit-msg.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 59afa7d..daca4be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ All notable changes to nightshift are documented in this file. +## [Unreleased] + +### Features +- **Commit message normalizer** — new `internal/commitmsg` package plus a + `nightshift commit-msg` subcommand (`--check`, `--fix`, `--print-spec`, + `--quiet`) that validates and rewrites commit messages against one + Conventional Commits format. `make install-hooks` now also installs a + `commit-msg` git hook, and the orchestrator's plan and implement prompts + quote the same canonical spec so autonomous agents write messages the hook + accepts. + ## [v0.3.3] - 2026-02-19 ### Features diff --git a/Makefile b/Makefile index 088be01..5bf99a2 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 git hooks install-hooks: @ln -sf ../../scripts/pre-commit.sh .git/hooks/pre-commit @echo "✓ pre-commit hook installed (.git/hooks/pre-commit → scripts/pre-commit.sh)" + @ln -sf ../../scripts/commit-msg.sh .git/hooks/commit-msg + @echo "✓ commit-msg hook installed (.git/hooks/commit-msg → scripts/commit-msg.sh)" diff --git a/README.md b/README.md index 84f92cd..81db2d1 100644 --- a/README.md +++ b/README.md @@ -273,6 +273,35 @@ This symlinks `scripts/pre-commit.sh` into `.git/hooks/pre-commit`. The hook run To bypass in a pinch: `git commit --no-verify` +### Commit message format + +Commits follow [Conventional Commits](https://www.conventionalcommits.org/): + +``` +type(scope)!: subject + +Body, wrapped at 72 columns. + +Trailer-Key: value +``` + +- `type` is lowercase and one of `feat`, `fix`, `docs`, `style`, `refactor`, + `perf`, `test`, `build`, `ci`, `chore`, `revert` +- the whole header line is at most 72 characters, imperative mood, no + trailing period +- trailers form one block at the very end + +`make install-hooks` also installs a `commit-msg` hook that enforces this via +`nightshift commit-msg --check`. You can run the checker yourself: + +```bash +nightshift commit-msg --print-spec # Show the full format +nightshift commit-msg --check .git/COMMIT_EDITMSG +nightshift commit-msg --fix .git/COMMIT_EDITMSG # Rewrite it for you +``` + +To bypass in a pinch: `git commit --no-verify` + ## Uninstalling ```bash diff --git a/cmd/nightshift/commands/commitmsg.go b/cmd/nightshift/commands/commitmsg.go new file mode 100644 index 0000000..3ee6db9 --- /dev/null +++ b/cmd/nightshift/commands/commitmsg.go @@ -0,0 +1,150 @@ +package commands + +import ( + "fmt" + "io" + "os" + + "github.com/spf13/cobra" + + "github.com/marcus/nightshift/internal/commitmsg" +) + +// commitMsgOptions holds the parsed flags for `nightshift commit-msg`. +type commitMsgOptions struct { + check bool + fix bool + printSpec bool + quiet bool +} + +func newCommitMsgCmd() *cobra.Command { + opts := &commitMsgOptions{} + + cmd := &cobra.Command{ + Use: "commit-msg [file|-]", + Short: "Check or normalize a commit message", + Long: `Check a commit message against the repository's commit format, or +rewrite it into that format. + +Reads the message from FILE, or from stdin when FILE is "-" or omitted. +This is what the commit-msg git hook runs; install it with: + + make install-hooks + +Examples: + nightshift commit-msg --print-spec + nightshift commit-msg --check .git/COMMIT_EDITMSG + nightshift commit-msg --fix .git/COMMIT_EDITMSG + printf 'Fixed the thing.' | nightshift commit-msg --fix -`, + Args: cobra.MaximumNArgs(1), + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + path := "-" + if len(args) == 1 { + path = args[0] + } + return runCommitMsg(path, opts, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr()) + }, + } + + cmd.Flags().BoolVar(&opts.check, "check", false, "Validate only; exit non-zero when the message has errors") + cmd.Flags().BoolVar(&opts.fix, "fix", false, "Rewrite the message into the canonical format") + cmd.Flags().BoolVar(&opts.printSpec, "print-spec", false, "Print the commit message format and exit") + cmd.Flags().BoolVar(&opts.quiet, "quiet", false, "Suppress issue output; rely on the exit code") + + return cmd +} + +func init() { + rootCmd.AddCommand(newCommitMsgCmd()) +} + +func runCommitMsg(path string, opts *commitMsgOptions, stdin io.Reader, stdout, stderr io.Writer) error { + if opts.printSpec { + fmt.Fprint(stdout, commitmsg.Spec()) + return nil + } + if opts.check && opts.fix { + return fmt.Errorf("--check and --fix are mutually exclusive") + } + + raw, err := readCommitMsg(path, stdin) + if err != nil { + return err + } + + label := path + if label == "-" { + label = "" + } + + if opts.fix { + fixed, issues, err := commitmsg.Normalize(raw, commitMsgValidationOptions()) + if err != nil { + return fmt.Errorf("%s: %w\n\n%s", label, err, commitmsg.Spec()) + } + if path == "-" { + fmt.Fprint(stdout, fixed) + } else if fixed != raw { + if err := os.WriteFile(path, []byte(fixed), 0o644); err != nil { + return fmt.Errorf("writing %s: %w", path, err) + } + } + reportCommitMsgIssues(stderr, label, issues, opts.quiet) + if commitmsg.HasErrors(issues) { + return fmt.Errorf("%s: commit message still has errors after --fix", label) + } + return nil + } + + // Default behaviour, with or without an explicit --check, is to validate. + msg, err := commitmsg.Parse(raw) + if err != nil { + return fmt.Errorf("%s: %w", label, err) + } + issues := commitmsg.ValidateWithOptions(msg, commitMsgValidationOptions()) + reportCommitMsgIssues(stderr, label, issues, opts.quiet) + if commitmsg.HasErrors(issues) { + if !opts.quiet { + fmt.Fprintf(stderr, "\n%s", commitmsg.Spec()) + } + return fmt.Errorf("%s: commit message does not match the required format", label) + } + return nil +} + +// commitMsgValidationOptions returns the validation options the CLI enforces. Nightshift +// trailers are not required of humans, so the defaults are used as-is. +func commitMsgValidationOptions() commitmsg.Options { + return commitmsg.DefaultOptions() +} + +func readCommitMsg(path string, stdin io.Reader) (string, error) { + if path == "-" { + data, err := io.ReadAll(stdin) + if err != nil { + return "", fmt.Errorf("reading stdin: %w", err) + } + return string(data), nil + } + data, err := os.ReadFile(path) + if err != nil { + return "", fmt.Errorf("reading %s: %w", path, err) + } + return string(data), nil +} + +// reportCommitMsgIssues prints issues as "file:line: severity: message (rule)". +func reportCommitMsgIssues(w io.Writer, label string, issues []commitmsg.Issue, quiet bool) { + if quiet { + return + } + for _, i := range issues { + line := "-" + if i.Line > 0 { + line = fmt.Sprintf("%d", i.Line) + } + fmt.Fprintf(w, "%s:%s: %s: %s (%s)\n", label, line, i.Severity, i.Message, i.Rule) + } +} diff --git a/cmd/nightshift/commands/commitmsg_test.go b/cmd/nightshift/commands/commitmsg_test.go new file mode 100644 index 0000000..9130232 --- /dev/null +++ b/cmd/nightshift/commands/commitmsg_test.go @@ -0,0 +1,157 @@ +package commands + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func writeTempMsg(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "COMMIT_EDITMSG") + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("writing temp message: %v", err) + } + return path +} + +func TestRunCommitMsg_PrintSpec(t *testing.T) { + var out, errBuf bytes.Buffer + if err := runCommitMsg("-", &commitMsgOptions{printSpec: true}, strings.NewReader(""), &out, &errBuf); err != nil { + t.Fatalf("runCommitMsg: %v", err) + } + if !strings.Contains(out.String(), "type(scope)") { + t.Errorf("--print-spec output missing the format:\n%s", out.String()) + } +} + +func TestRunCommitMsg_CheckPassesOnGoodFile(t *testing.T) { + path := writeTempMsg(t, "feat(cli): add commit message normalizer\n\nA short body.\n") + var out, errBuf bytes.Buffer + if err := runCommitMsg(path, &commitMsgOptions{check: true}, strings.NewReader(""), &out, &errBuf); err != nil { + t.Fatalf("runCommitMsg: %v\nstderr: %s", err, errBuf.String()) + } + if errBuf.Len() != 0 { + t.Errorf("unexpected diagnostics: %s", errBuf.String()) + } +} + +func TestRunCommitMsg_CheckFailsOnBadFile(t *testing.T) { + path := writeTempMsg(t, "Fixed the thing.\n") + var out, errBuf bytes.Buffer + err := runCommitMsg(path, &commitMsgOptions{check: true}, strings.NewReader(""), &out, &errBuf) + if err == nil { + t.Fatalf("expected an error for a malformed message") + } + if !strings.Contains(errBuf.String(), "header-format") { + t.Errorf("expected header-format diagnostic, got:\n%s", errBuf.String()) + } + if !strings.Contains(errBuf.String(), path+":1:") { + t.Errorf("expected file:line prefixed diagnostics, got:\n%s", errBuf.String()) + } + if !strings.Contains(errBuf.String(), "type(scope)") { + t.Errorf("expected the spec to be printed on failure, got:\n%s", errBuf.String()) + } +} + +func TestRunCommitMsg_CheckQuietSuppressesOutput(t *testing.T) { + path := writeTempMsg(t, "Fixed the thing.\n") + var out, errBuf bytes.Buffer + if err := runCommitMsg(path, &commitMsgOptions{check: true, quiet: true}, strings.NewReader(""), &out, &errBuf); err == nil { + t.Fatalf("expected an error for a malformed message") + } + if errBuf.Len() != 0 { + t.Errorf("--quiet should suppress diagnostics, got:\n%s", errBuf.String()) + } +} + +func TestRunCommitMsg_FixRewritesFileInPlace(t *testing.T) { + path := writeTempMsg(t, "Fixed bug in the parser.\n# a comment git added\n") + var out, errBuf bytes.Buffer + if err := runCommitMsg(path, &commitMsgOptions{fix: true}, strings.NewReader(""), &out, &errBuf); err != nil { + t.Fatalf("runCommitMsg: %v\nstderr: %s", err, errBuf.String()) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading fixed file: %v", err) + } + if string(got) != "fix: fixed bug in the parser\n" { + t.Errorf("fixed file = %q", string(got)) + } + // The rewritten message must now pass --check. + if err := runCommitMsg(path, &commitMsgOptions{check: true, quiet: true}, strings.NewReader(""), &out, &errBuf); err != nil { + t.Errorf("fixed message still fails --check: %v", err) + } +} + +func TestRunCommitMsg_FixFromStdinWritesToStdout(t *testing.T) { + var out, errBuf bytes.Buffer + err := runCommitMsg("-", &commitMsgOptions{fix: true}, strings.NewReader("Fixed bug in the parser."), &out, &errBuf) + if err != nil { + t.Fatalf("runCommitMsg: %v\nstderr: %s", err, errBuf.String()) + } + if out.String() != "fix: fixed bug in the parser\n" { + t.Errorf("stdout = %q", out.String()) + } +} + +func TestRunCommitMsg_CheckFromStdin(t *testing.T) { + var out, errBuf bytes.Buffer + err := runCommitMsg("-", &commitMsgOptions{check: true}, strings.NewReader("Fixed the thing."), &out, &errBuf) + if err == nil { + t.Fatalf("expected an error for a malformed message") + } + if !strings.Contains(errBuf.String(), ":1:") { + t.Errorf("expected label, got:\n%s", errBuf.String()) + } +} + +func TestRunCommitMsg_FixReportsUninferrableType(t *testing.T) { + var out, errBuf bytes.Buffer + err := runCommitMsg("-", &commitMsgOptions{fix: true}, strings.NewReader("zzzqqq wibble frobnicate"), &out, &errBuf) + if err == nil { + t.Fatalf("expected an error when no type can be inferred") + } + if !strings.Contains(err.Error(), "cannot infer") { + t.Errorf("error = %v, want it to explain the inference failure", err) + } +} + +func TestRunCommitMsg_CheckAndFixAreExclusive(t *testing.T) { + var out, errBuf bytes.Buffer + err := runCommitMsg("-", &commitMsgOptions{check: true, fix: true}, strings.NewReader("fix: a thing"), &out, &errBuf) + if err == nil || !strings.Contains(err.Error(), "mutually exclusive") { + t.Errorf("err = %v, want a mutual-exclusion error", err) + } +} + +func TestRunCommitMsg_MissingFile(t *testing.T) { + var out, errBuf bytes.Buffer + err := runCommitMsg(filepath.Join(t.TempDir(), "nope"), &commitMsgOptions{check: true}, strings.NewReader(""), &out, &errBuf) + if err == nil { + t.Fatalf("expected an error for a missing file") + } +} + +func TestCommitMsgCmd_EndToEnd(t *testing.T) { + path := writeTempMsg(t, "Fixed bug in the parser.\n") + + cmd := newCommitMsgCmd() + var out, errBuf bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&errBuf) + cmd.SetArgs([]string{"--fix", path}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v\nstderr: %s", err, errBuf.String()) + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading fixed file: %v", err) + } + if string(got) != "fix: fixed bug in the parser\n" { + t.Errorf("fixed file = %q", string(got)) + } +} diff --git a/internal/commitmsg/commitmsg.go b/internal/commitmsg/commitmsg.go new file mode 100644 index 0000000..b7048f4 --- /dev/null +++ b/internal/commitmsg/commitmsg.go @@ -0,0 +1,649 @@ +// Package commitmsg parses, validates and normalizes git commit messages +// against a single Conventional-Commits-style format. +// +// The canonical, human-readable description of that format lives in spec.go +// so the CLI, the git hook, the docs and the agent prompts all quote one +// source of truth. This package uses the standard library only. +package commitmsg + +import ( + "errors" + "fmt" + "regexp" + "strings" + "unicode" +) + +// Severity classifies how badly an issue breaks the format. +type Severity int + +const ( + // SeverityWarning marks a stylistic problem that does not fail --check. + SeverityWarning Severity = iota + // SeverityError marks a problem that fails --check. + SeverityError +) + +// String renders the severity as a lowercase word. +func (s Severity) String() string { + if s == SeverityError { + return "error" + } + return "warning" +} + +// Issue is a single validation finding. +type Issue struct { + // Line is the 1-based line of the normalized message the issue refers to. + // Zero means "the message as a whole". + Line int + Severity Severity + Rule string + Message string +} + +// String renders the issue as "line N: severity: message (rule)". +func (i Issue) String() string { + return fmt.Sprintf("line %d: %s: %s (%s)", i.Line, i.Severity, i.Message, i.Rule) +} + +// Trailer is a single git trailer, e.g. "Nightshift-Task: commit-normalize". +type Trailer struct { + Key string + Value string +} + +// String renders the trailer as "Key: Value". +func (t Trailer) String() string { return t.Key + ": " + t.Value } + +// Message is a parsed commit message. +type Message struct { + Type string + Scope string + Breaking bool + Subject string + Body string + Trailers []Trailer + + // rawHeader is the header line exactly as written, used to report issues + // about headers that do not match the conventional format at all. + rawHeader string + // noBlankAfterSubject records that the source message ran the body + // straight into the subject line. + noBlankAfterSubject bool +} + +// Options controls validation and normalization. +type Options struct { + // AllowedTypes are the accepted commit types, in canonical order. + AllowedTypes []string + // MaxSubjectLen caps the length of the whole header line. + MaxSubjectLen int + // WrapBody is the column body paragraphs are wrapped at. + WrapBody int + // RequiredTrailers must be present; Normalize appends missing ones. + RequiredTrailers []Trailer +} + +// DefaultOptions returns the format Nightshift enforces on itself. +func DefaultOptions() Options { + return Options{ + AllowedTypes: append([]string(nil), AllowedTypes...), + MaxSubjectLen: MaxSubjectLen, + WrapBody: WrapBody, + } +} + +var ( + headerRe = regexp.MustCompile(`^([A-Za-z][A-Za-z0-9]*)(?:\(([^()]+)\))?(!)?:[ \t]*(.*)$`) + // Git trailer syntax: "Token: value", token may contain letters, digits + // and hyphens. "BREAKING CHANGE" is allowed as a special case. + trailerRe = regexp.MustCompile(`^(BREAKING CHANGE|[A-Za-z][A-Za-z0-9-]*):[ \t]+(.*\S)[ \t]*$`) +) + +// knownTrailerKeys are hoisted out of the body even when they appear in the +// middle of a message, so every message ends with one trailer block. +var knownTrailerKeys = map[string]bool{ + "nightshift-task": true, + "nightshift-ref": true, + "co-authored-by": true, + "signed-off-by": true, + "reviewed-by": true, + "acked-by": true, + "tested-by": true, + "reported-by": true, + "refs": true, + "fixes": true, + "closes": true, + "breaking change": true, +} + +// ErrEmptyMessage is returned when nothing but comments or whitespace remains. +var ErrEmptyMessage = errors.New("commit message is empty") + +// Parse reads a raw commit message — including one straight out of a +// COMMIT_EDITMSG file, with git's comment lines and verbose diff attached — +// into a Message. A header that does not follow the conventional format is +// not an error: it parses with an empty Type so Validate can report it. +func Parse(raw string) (*Message, error) { + lines := cleanLines(raw) + if len(lines) == 0 { + return nil, ErrEmptyMessage + } + + m := &Message{rawHeader: lines[0]} + if match := headerRe.FindStringSubmatch(lines[0]); match != nil { + m.Type = match[1] + m.Scope = match[2] + m.Breaking = match[3] == "!" + m.Subject = strings.TrimSpace(match[4]) + } else { + m.Subject = strings.TrimSpace(lines[0]) + } + + rest := lines[1:] + if len(rest) > 0 && strings.TrimSpace(rest[0]) != "" { + m.noBlankAfterSubject = true + } + for len(rest) > 0 && strings.TrimSpace(rest[0]) == "" { + rest = rest[1:] + } + + bodyLines, trailers := splitTrailers(rest) + m.Body = strings.Join(bodyLines, "\n") + m.Body = strings.Trim(m.Body, "\n") + m.Trailers = trailers + return m, nil +} + +// cleanLines strips git's comment lines and any verbose diff, and trims +// trailing whitespace and leading/trailing blank lines. +func cleanLines(raw string) []string { + raw = strings.ReplaceAll(raw, "\r\n", "\n") + var out []string + for _, line := range strings.Split(raw, "\n") { + trimmed := strings.TrimRight(line, " \t") + // git --verbose puts an uncommented diff below a scissors line. + if isScissors(trimmed) || strings.HasPrefix(trimmed, "diff --git ") { + break + } + if strings.HasPrefix(strings.TrimSpace(trimmed), "#") { + continue + } + out = append(out, trimmed) + } + // Trim 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 +} + +func isScissors(line string) bool { + t := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(line), "#")) + return strings.Contains(t, ">8") && strings.Contains(t, "--") +} + +// splitTrailers separates body lines from trailers. It takes the trailing +// block when it is made entirely of trailers, and additionally hoists any +// well-known trailer that got stranded in the middle of the body. +func splitTrailers(lines []string) ([]string, []Trailer) { + var trailers []Trailer + + // Trailing block: walk back over the last contiguous non-blank run. + end := len(lines) + start := end + for start > 0 && strings.TrimSpace(lines[start-1]) != "" { + start-- + } + if start < end { + allTrailers := true + for _, line := range lines[start:end] { + if trailerRe.FindStringSubmatch(line) == nil { + allTrailers = false + break + } + } + if allTrailers { + for _, line := range lines[start:end] { + match := trailerRe.FindStringSubmatch(line) + trailers = append(trailers, Trailer{Key: match[1], Value: match[2]}) + } + lines = lines[:start] + } + } + + // Hoist stranded well-known trailers, keeping document order ahead of the + // trailing block's. + var body []string + var hoisted []Trailer + for _, line := range lines { + if match := trailerRe.FindStringSubmatch(line); match != nil && knownTrailerKeys[strings.ToLower(match[1])] { + hoisted = append(hoisted, Trailer{Key: match[1], Value: match[2]}) + continue + } + body = append(body, line) + } + return body, append(hoisted, trailers...) +} + +// Header renders just the first line of the message. +func (m *Message) Header() string { + if m.Type == "" { + return m.Subject + } + head := m.Type + if m.Scope != "" { + head += "(" + m.Scope + ")" + } + if m.Breaking { + head += "!" + } + return head + ": " + m.Subject +} + +// String renders the message in canonical layout, without a trailing newline. +func (m *Message) String() string { + var b strings.Builder + b.WriteString(m.Header()) + if body := strings.Trim(m.Body, "\n"); body != "" { + b.WriteString("\n\n") + b.WriteString(body) + } + if len(m.Trailers) > 0 { + b.WriteString("\n\n") + for i, t := range m.Trailers { + if i > 0 { + b.WriteString("\n") + } + b.WriteString(t.String()) + } + } + return b.String() +} + +// Validate checks the message against DefaultOptions. +func Validate(m *Message) []Issue { return ValidateWithOptions(m, DefaultOptions()) } + +// ValidateWithOptions checks the message against opts and returns every +// issue found, errors and warnings alike. +func ValidateWithOptions(m *Message, opts Options) []Issue { + var issues []Issue + add := func(line int, sev Severity, rule, format string, args ...interface{}) { + issues = append(issues, Issue{Line: line, Severity: sev, Rule: rule, Message: fmt.Sprintf(format, args...)}) + } + + switch { + case m.Type == "": + add(1, SeverityError, "header-format", + "header must be %q, got %q", "type(scope): subject", m.rawHeader) + default: + if strings.ToLower(m.Type) != m.Type { + add(1, SeverityError, "type-lowercase", "type %q must be lowercase", m.Type) + } + if !allowed(strings.ToLower(m.Type), opts.AllowedTypes) { + add(1, SeverityError, "type-allowed", + "unknown type %q; allowed: %s", m.Type, strings.Join(opts.AllowedTypes, ", ")) + } + } + + if strings.TrimSpace(m.Subject) == "" { + add(1, SeverityError, "subject-empty", "subject must not be empty") + } else { + if n := len(m.Header()); n > opts.MaxSubjectLen { + add(1, SeverityError, "subject-length", + "header is %d characters, limit is %d", n, opts.MaxSubjectLen) + } + if strings.HasSuffix(m.Subject, ".") { + add(1, SeverityError, "subject-trailing-period", "subject must not end with a period") + } + if word := firstWord(m.Subject); !isImperative(word) { + add(1, SeverityWarning, "subject-mood", + "subject should use the imperative mood (%q reads as past tense or third person)", word) + } + } + + if m.noBlankAfterSubject { + add(2, SeverityError, "body-blank-line", "subject and body must be separated by a blank line") + } + + for i, line := range strings.Split(m.Body, "\n") { + if !protectedLine(line) && len(line) > opts.WrapBody { + add(bodyLineNumber(i), SeverityWarning, "body-wrap", + "body line is %d characters, wrap at %d", len(line), opts.WrapBody) + } + } + + for _, want := range opts.RequiredTrailers { + if !hasTrailerKey(m.Trailers, want.Key) { + add(0, SeverityError, "trailer-required", "missing required trailer %q", want.Key) + } + } + + return issues +} + +// bodyLineNumber maps a 0-based body line index onto the rendered message, +// where line 1 is the header and line 2 is the blank separator. +func bodyLineNumber(i int) int { return i + 3 } + +func allowed(t string, allowedTypes []string) bool { + for _, a := range allowedTypes { + if a == t { + return true + } + } + return false +} + +func hasTrailerKey(trailers []Trailer, key string) bool { + for _, t := range trailers { + if strings.EqualFold(t.Key, key) { + return true + } + } + return false +} + +// HasErrors reports whether any issue is error severity. +func HasErrors(issues []Issue) bool { + for _, i := range issues { + if i.Severity == SeverityError { + return true + } + } + return false +} + +// Normalize rewrites raw into the canonical format, returning the normalized +// message (with a trailing newline), the issues that remain after everything +// fixable has been fixed, and an error only when the message cannot be +// normalized at all — empty, or with a subject no type can be inferred from. +func Normalize(raw string, opts Options) (string, []Issue, error) { + m, err := Parse(raw) + if err != nil { + return "", nil, err + } + if strings.TrimSpace(m.Subject) == "" { + return "", nil, errors.New("commit message has no subject") + } + + if m.Type == "" { + inferred := inferType(m.Subject) + if inferred == "" { + return "", nil, fmt.Errorf("cannot infer a commit type from subject %q; "+ + "prefix it explicitly, e.g. \"fix: %s\"", m.Subject, m.Subject) + } + m.Type = inferred + } + m.Type = strings.ToLower(m.Type) + m.Subject = normalizeSubject(m.Subject) + m.noBlankAfterSubject = false + m.Body = wrapBody(m.Body, opts.WrapBody) + m.Trailers = mergeTrailers(m.Trailers, opts.RequiredTrailers) + + out := m.String() + "\n" + return out, ValidateWithOptions(m, opts), nil +} + +// normalizeSubject strips trailing periods and lowercases the leading word +// unless it looks like an acronym or identifier. +func normalizeSubject(s string) string { + s = strings.TrimSpace(s) + s = strings.TrimRight(s, ".") + s = strings.TrimSpace(s) + if s == "" { + return s + } + word := firstWord(s) + if isAcronymish(word) { + return s + } + runes := []rune(s) + runes[0] = unicode.ToLower(runes[0]) + return string(runes) +} + +// isAcronymish reports whether a word should keep its capitalization, e.g. +// "API", "HTTPClient", "GitHub". +func isAcronymish(word string) bool { + upper := 0 + for i, r := range word { + if unicode.IsUpper(r) && i > 0 { + return true + } + if unicode.IsUpper(r) { + upper++ + } + } + // A single-letter uppercase word, or a word already lowercase. + return upper > 0 && len([]rune(word)) == 1 +} + +func firstWord(s string) string { + s = strings.TrimSpace(s) + if i := strings.IndexAny(s, " \t"); i >= 0 { + return s[:i] + } + return s +} + +// nonImperative lists common past-tense and third-person subject openers. +var nonImperative = map[string]bool{ + "fixed": true, "fixes": true, "added": true, "adds": true, + "updated": true, "updates": true, "removed": true, "removes": true, + "changed": true, "changes": true, "refactored": true, "refactors": true, + "created": true, "creates": true, "implemented": true, "implements": true, + "bumped": true, "bumps": true, "moved": true, "moves": true, + "renamed": true, "renames": true, "documented": true, "documents": true, + "improved": true, "improves": true, "deleted": true, "deletes": true, + "wrote": true, "made": true, "makes": true, +} + +func isImperative(word string) bool { + w := strings.ToLower(strings.Trim(word, `"'`)) + if nonImperative[w] { + return false + } + // "…ing" openers ("adding a thing") are gerunds, not imperatives. + return !strings.HasSuffix(w, "ing") +} + +// typeByVerb maps a leading verb to a commit type. +var typeByVerb = map[string]string{ + "fix": "fix", "fixed": "fix", "fixes": "fix", "resolve": "fix", + "resolved": "fix", "correct": "fix", "corrected": "fix", "patch": "fix", + "repair": "fix", "handle": "fix", "prevent": "fix", "guard": "fix", + + "add": "feat", "added": "feat", "adds": "feat", "create": "feat", + "created": "feat", "implement": "feat", "implemented": "feat", + "introduce": "feat", "introduced": "feat", "support": "feat", + + "refactor": "refactor", "refactored": "refactor", "rename": "refactor", + "renamed": "refactor", "move": "refactor", "moved": "refactor", + "simplify": "refactor", "simplified": "refactor", "extract": "refactor", + "remove": "refactor", "removed": "refactor", "delete": "refactor", + "deleted": "refactor", "drop": "refactor", "update": "refactor", + "updated": "refactor", "change": "refactor", "changed": "refactor", + + "document": "docs", "documented": "docs", "clarify": "docs", + + "test": "test", "tested": "test", "cover": "test", + + "optimize": "perf", "optimized": "perf", "speed": "perf", + + "bump": "chore", "upgrade": "chore", "upgraded": "chore", "pin": "chore", + "clean": "chore", "cleanup": "chore", "chore": "chore", + + "revert": "revert", "reverted": "revert", +} + +// typeByKeyword maps a distinctive word anywhere in the subject to a type. +// These win over the leading verb: "Update the README" is docs, not refactor. +var typeByKeyword = []struct { + word string + typ string +}{ + {"readme", "docs"}, + {"changelog", "docs"}, + {"documentation", "docs"}, + {"docs", "docs"}, + {"godoc", "docs"}, + {"typo", "docs"}, +} + +// inferType guesses a commit type from a free-form subject, returning "" when +// it cannot make a confident guess. +func inferType(subject string) string { + lower := strings.ToLower(subject) + words := strings.FieldsFunc(lower, func(r rune) bool { return !unicode.IsLetter(r) && !unicode.IsNumber(r) }) + wordSet := make(map[string]bool, len(words)) + for _, w := range words { + wordSet[w] = true + } + + for _, kw := range typeByKeyword { + if wordSet[kw.word] { + return kw.typ + } + } + if len(words) > 0 { + if t, ok := typeByVerb[words[0]]; ok { + return t + } + } + if wordSet["bug"] || wordSet["crash"] || wordSet["panic"] || wordSet["regression"] { + return "fix" + } + if wordSet["test"] || wordSet["tests"] { + return "test" + } + return "" +} + +// mergeTrailers dedupes trailers (case-insensitively on the key, exactly on +// the value) and appends any required trailer that is missing. +func mergeTrailers(existing, required []Trailer) []Trailer { + var out []Trailer + seen := make(map[string]bool) + for _, t := range existing { + key := strings.ToLower(t.Key) + "\x00" + t.Value + if seen[key] { + continue + } + seen[key] = true + out = append(out, t) + } + for _, want := range required { + if !hasTrailerKey(out, want.Key) { + out = append(out, want) + } + } + return out +} + +// wrapBody reflows prose paragraphs at width columns, leaving code blocks, +// indented text, lists, quotes and unbreakable lines (long URLs) alone. +func wrapBody(body string, width int) string { + body = strings.Trim(body, "\n") + if body == "" { + return "" + } + var out []string + var para []string + inFence := false + + flush := func() { + if len(para) == 0 { + return + } + if paragraphProtected(para) { + out = append(out, para...) + } else { + out = append(out, wrapParagraph(strings.Join(para, " "), width)...) + } + para = nil + } + + for _, line := range strings.Split(body, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "```") { + flush() + inFence = !inFence + out = append(out, line) + continue + } + if inFence { + out = append(out, line) + continue + } + if strings.TrimSpace(line) == "" { + flush() + out = append(out, "") + continue + } + para = append(para, line) + } + flush() + + // Collapse runs of blank lines and trim the edges. + var collapsed []string + for i, line := range out { + if line == "" && i > 0 && out[i-1] == "" { + continue + } + collapsed = append(collapsed, line) + } + return strings.Trim(strings.Join(collapsed, "\n"), "\n") +} + +func paragraphProtected(para []string) bool { + for _, line := range para { + if protectedLine(line) { + return true + } + } + return false +} + +// protectedLine reports whether a line must be preserved verbatim. +func protectedLine(line string) bool { + if line == "" { + return true + } + if strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t") { + return true + } + switch line[0] { + case '-', '*', '+', '>', '|', '#': + return true + } + if strings.HasPrefix(line, "```") { + return true + } + // A single unbreakable token, e.g. a long URL. + if !strings.ContainsAny(line, " \t") { + return true + } + return false +} + +func wrapParagraph(text string, width int) []string { + words := strings.Fields(text) + if len(words) == 0 { + return nil + } + var lines []string + current := words[0] + for _, w := range words[1:] { + if len(current)+1+len(w) > width { + lines = append(lines, current) + current = w + continue + } + current += " " + w + } + return append(lines, current) +} diff --git a/internal/commitmsg/commitmsg_test.go b/internal/commitmsg/commitmsg_test.go new file mode 100644 index 0000000..b5769e7 --- /dev/null +++ b/internal/commitmsg/commitmsg_test.go @@ -0,0 +1,503 @@ +package commitmsg + +import ( + "strings" + "testing" +) + +func hasRule(issues []Issue, rule string) bool { + for _, i := range issues { + if i.Rule == rule { + return true + } + } + return false +} + +func errorCount(issues []Issue) int { + n := 0 + for _, i := range issues { + if i.Severity == SeverityError { + n++ + } + } + return n +} + +// --- Parsing --- + +func TestParse_FullMessage(t *testing.T) { + raw := `feat(cli): add commit message normalizer + +This adds a normalizer that enforces a single commit format +across the repository. + +Nightshift-Task: commit-normalize +Co-Authored-By: Someone +` + m, err := Parse(raw) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if m.Type != "feat" { + t.Errorf("Type = %q, want feat", m.Type) + } + if m.Scope != "cli" { + t.Errorf("Scope = %q, want cli", m.Scope) + } + if m.Breaking { + t.Errorf("Breaking = true, want false") + } + if m.Subject != "add commit message normalizer" { + t.Errorf("Subject = %q", m.Subject) + } + if !strings.HasPrefix(m.Body, "This adds a normalizer") { + t.Errorf("Body = %q", m.Body) + } + if strings.Contains(m.Body, "Nightshift-Task") { + t.Errorf("body should not contain trailers: %q", m.Body) + } + if len(m.Trailers) != 2 { + t.Fatalf("Trailers = %v, want 2", m.Trailers) + } + if m.Trailers[0].Key != "Nightshift-Task" || m.Trailers[0].Value != "commit-normalize" { + t.Errorf("Trailers[0] = %+v", m.Trailers[0]) + } + if m.Trailers[1].Key != "Co-Authored-By" { + t.Errorf("Trailers[1] = %+v", m.Trailers[1]) + } +} + +func TestParse_BreakingMarker(t *testing.T) { + m, err := Parse("feat(api)!: drop v1 endpoints") + if err != nil { + t.Fatalf("Parse: %v", err) + } + if !m.Breaking { + t.Errorf("Breaking = false, want true") + } + if m.Type != "feat" || m.Scope != "api" { + t.Errorf("got type=%q scope=%q", m.Type, m.Scope) + } + if m.Subject != "drop v1 endpoints" { + t.Errorf("Subject = %q", m.Subject) + } +} + +func TestParse_NoConventionalHeader(t *testing.T) { + m, err := Parse("Fixed the flaky test") + if err != nil { + t.Fatalf("Parse should not fail on a non-conventional header: %v", err) + } + if m.Type != "" { + t.Errorf("Type = %q, want empty", m.Type) + } + if m.Subject != "Fixed the flaky test" { + t.Errorf("Subject = %q", m.Subject) + } +} + +func TestParse_EmptyMessage(t *testing.T) { + for _, raw := range []string{"", " \n\n ", "# just a comment\n# another\n"} { + if _, err := Parse(raw); err == nil { + t.Errorf("Parse(%q) = nil error, want error", raw) + } + } +} + +func TestParse_StripsCommentsAndVerboseDiff(t *testing.T) { + raw := `fix(db): close rows on scan error + +# Please enter the commit message for your changes. Lines starting +# with '#' will be ignored, and an empty message aborts the commit. +# +# ------------------------ >8 ------------------------ +# Do not modify or remove the line above. +diff --git a/internal/db/db.go b/internal/db/db.go +index 1234567..89abcde 100644 +--- a/internal/db/db.go ++++ b/internal/db/db.go +@@ -1,3 +1,4 @@ ++// a change that must never leak into the message +` + m, err := Parse(raw) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if m.Body != "" { + t.Errorf("Body = %q, want empty (comments and diff stripped)", m.Body) + } + if strings.Contains(m.String(), "diff --git") || strings.Contains(m.String(), "#") { + t.Errorf("String() leaked comments/diff:\n%s", m.String()) + } +} + +func TestMessage_String_RoundTrip(t *testing.T) { + raw := "feat(cli): add thing\n\nSome body text.\n\nNightshift-Task: commit-normalize\n" + m, err := Parse(raw) + if err != nil { + t.Fatalf("Parse: %v", err) + } + want := "feat(cli): add thing\n\nSome body text.\n\nNightshift-Task: commit-normalize" + if got := m.String(); got != want { + t.Errorf("String() =\n%q\nwant\n%q", got, want) + } +} + +// --- Validation --- + +func TestValidate_Clean(t *testing.T) { + m, err := Parse("feat(cli): add commit message normalizer\n\nA short body.\n") + if err != nil { + t.Fatalf("Parse: %v", err) + } + if issues := Validate(m); len(issues) != 0 { + t.Errorf("Validate() = %v, want none", issues) + } +} + +func TestValidate_MissingType(t *testing.T) { + m, _ := Parse("Fixed the flaky test") + issues := Validate(m) + if !hasRule(issues, "header-format") { + t.Errorf("expected header-format issue, got %v", issues) + } + if errorCount(issues) == 0 { + t.Errorf("expected at least one error-severity issue, got %v", issues) + } +} + +func TestValidate_UnknownType(t *testing.T) { + m, _ := Parse("feet(cli): add a thing") + if !hasRule(Validate(m), "type-allowed") { + t.Errorf("expected type-allowed issue, got %v", Validate(m)) + } +} + +func TestValidate_UppercaseType(t *testing.T) { + m, _ := Parse("Fix(cli): add a thing") + if !hasRule(Validate(m), "type-lowercase") { + t.Errorf("expected type-lowercase issue, got %v", Validate(m)) + } +} + +func TestValidate_EmptySubject(t *testing.T) { + m, _ := Parse("fix(cli):") + if !hasRule(Validate(m), "subject-empty") && !hasRule(Validate(m), "header-format") { + t.Errorf("expected an issue for an empty subject, got %v", Validate(m)) + } +} + +func TestValidate_SubjectTooLong(t *testing.T) { + long := "feat(cli): " + strings.Repeat("a", 80) + m, _ := Parse(long) + issues := Validate(m) + if !hasRule(issues, "subject-length") { + t.Errorf("expected subject-length issue, got %v", issues) + } + for _, i := range issues { + if i.Rule == "subject-length" && i.Severity != SeverityError { + t.Errorf("subject-length should be an error, got %v", i.Severity) + } + } +} + +func TestValidate_TrailingPeriod(t *testing.T) { + m, _ := Parse("fix(cli): stop panicking on empty input.") + if !hasRule(Validate(m), "subject-trailing-period") { + t.Errorf("expected subject-trailing-period issue, got %v", Validate(m)) + } +} + +func TestValidate_ImperativeMoodWarning(t *testing.T) { + m, _ := Parse("fix(cli): fixed the parser") + issues := Validate(m) + if !hasRule(issues, "subject-mood") { + t.Fatalf("expected subject-mood issue, got %v", issues) + } + for _, i := range issues { + if i.Rule == "subject-mood" && i.Severity != SeverityWarning { + t.Errorf("subject-mood should be a warning, got %v", i.Severity) + } + } +} + +func TestValidate_MissingBlankLineAfterSubject(t *testing.T) { + m, _ := Parse("fix(cli): stop panicking\nthe body starts immediately\n") + if !hasRule(Validate(m), "body-blank-line") { + t.Errorf("expected body-blank-line issue, got %v", Validate(m)) + } +} + +func TestValidate_BodyWrapWarning(t *testing.T) { + m, _ := Parse("fix(cli): wrap it\n\n" + strings.Repeat("word ", 30) + "\n") + issues := Validate(m) + if !hasRule(issues, "body-wrap") { + t.Fatalf("expected body-wrap issue, got %v", issues) + } + for _, i := range issues { + if i.Rule == "body-wrap" && i.Severity != SeverityWarning { + t.Errorf("body-wrap should be a warning, got %v", i.Severity) + } + } +} + +func TestValidateWithOptions_RequiredTrailers(t *testing.T) { + opts := DefaultOptions() + opts.RequiredTrailers = []Trailer{{Key: "Nightshift-Task", Value: "commit-normalize"}} + + m, _ := Parse("fix(cli): stop panicking") + if !hasRule(ValidateWithOptions(m, opts), "trailer-required") { + t.Errorf("expected trailer-required issue, got %v", ValidateWithOptions(m, opts)) + } + + m2, _ := Parse("fix(cli): stop panicking\n\nNightshift-Task: commit-normalize\n") + if hasRule(ValidateWithOptions(m2, opts), "trailer-required") { + t.Errorf("unexpected trailer-required issue: %v", ValidateWithOptions(m2, opts)) + } +} + +func TestIssue_String(t *testing.T) { + i := Issue{Line: 3, Severity: SeverityError, Rule: "subject-length", Message: "too long"} + got := i.String() + if !strings.Contains(got, "3") || !strings.Contains(got, "error") || !strings.Contains(got, "too long") { + t.Errorf("Issue.String() = %q", got) + } +} + +// --- Normalization --- + +func TestNormalize_InfersTypeFromSubject(t *testing.T) { + cases := []struct { + raw string + want string + }{ + {"Fixed bug in X.", "fix: fixed bug in X"}, + {"Added a new report command", "feat: added a new report command"}, + {"Update the README", "docs: update the README"}, + {"Refactored the orchestrator", "refactor: refactored the orchestrator"}, + {"Bump go version", "chore: bump go version"}, + } + for _, tc := range cases { + got, _, err := Normalize(tc.raw, DefaultOptions()) + if err != nil { + t.Errorf("Normalize(%q): %v", tc.raw, err) + continue + } + if strings.TrimRight(got, "\n") != tc.want { + t.Errorf("Normalize(%q) = %q, want %q", tc.raw, strings.TrimRight(got, "\n"), tc.want) + } + } +} + +func TestNormalize_UninferrableTypeIsAnError(t *testing.T) { + if _, issues, err := Normalize("zzzqqq wibble frobnicate", DefaultOptions()); err == nil { + t.Errorf("expected error for uninferrable type, got issues %v", issues) + } +} + +func TestNormalize_LowercasesTypeAndStripsPeriod(t *testing.T) { + got, _, err := Normalize("Fix(CLI): stop panicking on empty input.", DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + want := "fix(CLI): stop panicking on empty input\n" + if got != want { + t.Errorf("Normalize() = %q, want %q", got, want) + } +} + +func TestNormalize_PreservesAcronymInSubject(t *testing.T) { + got, _, err := Normalize("Fixed API timeouts", DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + if !strings.Contains(got, "API timeouts") { + t.Errorf("Normalize() = %q, want the acronym preserved", got) + } +} + +func TestNormalize_InsertsBlankLineBeforeBody(t *testing.T) { + got, _, err := Normalize("fix(cli): stop panicking\nthe body starts immediately\n", DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + want := "fix(cli): stop panicking\n\nthe body starts immediately\n" + if got != want { + t.Errorf("Normalize() = %q, want %q", got, want) + } +} + +func TestNormalize_WrapsBody(t *testing.T) { + body := strings.Repeat("word ", 40) + got, _, err := Normalize("fix(cli): wrap it\n\n"+body+"\n", DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + for _, line := range strings.Split(got, "\n") { + if len(line) > DefaultOptions().WrapBody { + t.Errorf("line exceeds wrap width (%d): %q", len(line), line) + } + } +} + +func TestNormalize_DoesNotWrapCodeOrURLs(t *testing.T) { + url := "https://example.com/" + strings.Repeat("a", 90) + raw := "docs: add link\n\n indented code line that is quite long and should not be rewrapped at all\n\n" + url + "\n" + got, _, err := Normalize(raw, DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + if !strings.Contains(got, url) { + t.Errorf("URL was rewrapped:\n%s", got) + } + if !strings.Contains(got, " indented code line that is quite long and should not be rewrapped at all") { + t.Errorf("indented line was rewrapped:\n%s", got) + } +} + +func TestNormalize_AppendsRequiredTrailers(t *testing.T) { + opts := DefaultOptions() + opts.RequiredTrailers = []Trailer{ + {Key: "Nightshift-Task", Value: "commit-normalize"}, + {Key: "Nightshift-Ref", Value: "https://github.com/marcus/nightshift"}, + } + got, _, err := Normalize("fix(cli): stop panicking\n\nA body.\n", opts) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + want := "fix(cli): stop panicking\n\nA body.\n\nNightshift-Task: commit-normalize\nNightshift-Ref: https://github.com/marcus/nightshift\n" + if got != want { + t.Errorf("Normalize() =\n%q\nwant\n%q", got, want) + } +} + +func TestNormalize_DedupesTrailersAndPreservesCoAuthors(t *testing.T) { + opts := DefaultOptions() + opts.RequiredTrailers = []Trailer{{Key: "Nightshift-Task", Value: "commit-normalize"}} + raw := `fix(cli): stop panicking + +Nightshift-Task: commit-normalize +Nightshift-Task: commit-normalize +Co-Authored-By: A +Co-Authored-By: B +Co-Authored-By: A +` + got, _, err := Normalize(raw, opts) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + if n := strings.Count(got, "Nightshift-Task:"); n != 1 { + t.Errorf("Nightshift-Task appears %d times:\n%s", n, got) + } + if n := strings.Count(got, "Co-Authored-By: A "); n != 1 { + t.Errorf("duplicate co-author not deduped:\n%s", got) + } + if !strings.Contains(got, "Co-Authored-By: B ") { + t.Errorf("distinct co-author dropped:\n%s", got) + } +} + +func TestNormalize_TrailersEndUpInASingleTrailingBlock(t *testing.T) { + raw := `fix(cli): stop panicking + +Nightshift-Task: commit-normalize + +Some body text that came after a trailer. + +Co-Authored-By: A +` + got, _, err := Normalize(raw, DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + lines := strings.Split(strings.TrimRight(got, "\n"), "\n") + // The last block must be contiguous trailers. + if lines[len(lines)-1] != "Co-Authored-By: A " || + lines[len(lines)-2] != "Nightshift-Task: commit-normalize" { + t.Errorf("trailers not collected into one trailing block:\n%s", got) + } + if lines[len(lines)-3] != "" { + t.Errorf("missing blank line before trailer block:\n%s", got) + } +} + +func TestNormalize_Idempotent(t *testing.T) { + opts := DefaultOptions() + opts.RequiredTrailers = []Trailer{{Key: "Nightshift-Task", Value: "commit-normalize"}} + inputs := []string{ + "Fixed bug in X.", + "fix(cli): stop panicking\nbody right here\n", + "feat(api)!: drop v1\n\n" + strings.Repeat("word ", 40) + "\n\nCo-Authored-By: A \n", + } + for _, raw := range inputs { + once, _, err := Normalize(raw, opts) + if err != nil { + t.Fatalf("Normalize(%q): %v", raw, err) + } + twice, _, err := Normalize(once, opts) + if err != nil { + t.Fatalf("Normalize(normalized): %v", err) + } + if once != twice { + t.Errorf("not idempotent for %q:\nfirst:\n%q\nsecond:\n%q", raw, once, twice) + } + } +} + +func TestNormalize_AlreadyNormalIsNoOp(t *testing.T) { + raw := "feat(cli): add commit message normalizer\n\nA tidy body that is already short enough.\n\nNightshift-Task: commit-normalize\n" + got, issues, err := Normalize(raw, DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + if got != raw { + t.Errorf("Normalize() changed an already-normal message:\ngot %q\nwant %q", got, raw) + } + if errorCount(issues) != 0 { + t.Errorf("unexpected residual errors: %v", issues) + } +} + +func TestNormalize_ReportsResidualErrorsItCannotFix(t *testing.T) { + long := "feat(cli): " + strings.Repeat("a", 90) + _, issues, err := Normalize(long, DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + if !hasRule(issues, "subject-length") { + t.Errorf("expected residual subject-length issue, got %v", issues) + } +} + +func TestNormalize_StripsCommentsAndDiff(t *testing.T) { + raw := "fix(db): close rows\n# a comment\n# ------------------------ >8 ------------------------\ndiff --git a/x b/x\n+leak\n" + got, _, err := Normalize(raw, DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + if got != "fix(db): close rows\n" { + t.Errorf("Normalize() = %q", got) + } +} + +// --- Spec --- + +func TestSpec_MentionsCoreRules(t *testing.T) { + s := Spec() + for _, want := range []string{"type(scope)", "72", "feat", "fix"} { + if !strings.Contains(s, want) { + t.Errorf("Spec() missing %q:\n%s", want, s) + } + } +} + +func TestPromptSpec_IncludesTrailers(t *testing.T) { + s := PromptSpec("commit-normalize") + if !strings.Contains(s, "Nightshift-Task: commit-normalize") { + t.Errorf("PromptSpec missing task trailer:\n%s", s) + } + if !strings.Contains(s, "Nightshift-Ref: https://github.com/marcus/nightshift") { + t.Errorf("PromptSpec missing ref trailer:\n%s", s) + } +} diff --git a/internal/commitmsg/spec.go b/internal/commitmsg/spec.go new file mode 100644 index 0000000..f2461db --- /dev/null +++ b/internal/commitmsg/spec.go @@ -0,0 +1,103 @@ +package commitmsg + +import ( + "fmt" + "strings" +) + +// The tunable limits of the format. These are the single source of truth for +// the validator, the CLI, the git hook and the agent prompts. +const ( + // MaxSubjectLen caps the whole header line, "type(scope): subject". + MaxSubjectLen = 72 + // WrapBody is the column body paragraphs are wrapped at. + WrapBody = 72 + + // TaskTrailerKey identifies the Nightshift task a commit came from. + TaskTrailerKey = "Nightshift-Task" + // RefTrailerKey points back at the Nightshift project. + RefTrailerKey = "Nightshift-Ref" + // RefTrailerValue is the canonical value for RefTrailerKey. + RefTrailerValue = "https://github.com/marcus/nightshift" +) + +// AllowedTypes are the commit types this repository accepts. +var AllowedTypes = []string{ + "feat", + "fix", + "docs", + "style", + "refactor", + "perf", + "test", + "build", + "ci", + "chore", + "revert", +} + +// NightshiftTrailers returns the trailers an autonomous run must attach to +// every commit it makes for the given task type. +func NightshiftTrailers(taskType string) []Trailer { + return []Trailer{ + {Key: TaskTrailerKey, Value: taskType}, + {Key: RefTrailerKey, Value: RefTrailerValue}, + } +} + +// Spec returns the canonical, human-readable description of the commit +// message format. Everything that explains the format — `nightshift +// commit-msg --print-spec`, the commit-msg hook's failure output, the docs +// and the agent prompts — quotes this text. +func Spec() string { + return fmt.Sprintf(`Commit message format + + type(scope)!: subject + + body, wrapped at %d columns + + Trailer-Key: value + +Rules + - type is required and lowercase, one of: %s + - scope is optional and describes the area touched, e.g. (cli), (db) + - "!" after the type/scope marks a breaking change + - the whole header line "type(scope): subject" is at most %d characters + - the subject is imperative mood ("add x", not "added x") and has no + trailing period + - a blank line separates the subject from the body and the body from the + trailers + - body paragraphs wrap at %d columns; code blocks, indented text, lists + and URLs are left alone + - trailers form one block at the very end, one "Key: value" per line + +Example + feat(cli): add commit message normalizer + + Adds a nightshift commit-msg subcommand that checks and rewrites commit + messages so every commit in the repository shares one format. + + Nightshift-Task: commit-normalize + Nightshift-Ref: %s +`, WrapBody, strings.Join(AllowedTypes, ", "), MaxSubjectLen, WrapBody, RefTrailerValue) +} + +// PromptSpec returns the commit instructions handed to autonomous agents for +// a task of the given type: the shared Spec plus the exact trailers the run +// must attach. It is the same format `nightshift commit-msg --check` +// enforces, so agents are told precisely what the hook will accept. +func PromptSpec(taskType string) string { + var b strings.Builder + for _, line := range strings.Split(strings.TrimRight(Spec(), "\n"), "\n") { + if line == "" { + b.WriteString("\n") + continue + } + b.WriteString(" " + line + "\n") + } + b.WriteString("\n Every commit you make must also carry these trailers:\n") + for _, t := range NightshiftTrailers(taskType) { + b.WriteString(" " + t.String() + "\n") + } + return strings.TrimRight(b.String(), "\n") +} diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 6141c95..2eb1511 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -16,6 +16,7 @@ import ( "github.com/marcus/nightshift/internal/agents" "github.com/marcus/nightshift/internal/budget" + "github.com/marcus/nightshift/internal/commitmsg" "github.com/marcus/nightshift/internal/logging" "github.com/marcus/nightshift/internal/tasks" ) @@ -728,9 +729,8 @@ Description: %s 0. You are running autonomously. If the task is broad or ambiguous, choose a concrete, minimal scope that delivers value and state any assumptions in the description. 1. Work on a new branch and plan to submit a PR. Never work directly on the primary branch.%s 2. Before creating your branch, record the current branch name and plan to switch back after the PR is opened. -3. If you create commits, include a concise message with these git trailers: - Nightshift-Task: %s - Nightshift-Ref: https://github.com/marcus/nightshift +3. If you create commits, use this commit message format: +%s 4. Analyze the task requirements 5. Identify files that need to be modified 6. Create step-by-step implementation plan @@ -741,7 +741,7 @@ Description: %s "files": ["file1.go", "file2.go", ...], "description": "overall approach" } -`, task.ID, task.Title, task.Description, branchInstruction, task.Type) +`, task.ID, task.Title, task.Description, branchInstruction, commitmsg.PromptSpec(string(task.Type))) } func (o *Orchestrator) buildImplementPrompt(task *tasks.Task, plan *PlanOutput, iteration int) string { @@ -771,9 +771,8 @@ Description: %s ## Instructions 0. Before creating your branch, record the current branch name. Create and work on a new branch. Never modify or commit directly to the primary branch.%s When finished, open a PR. After the PR is submitted, switch back to the original branch. If you cannot open a PR, leave the branch and explain next steps. -1. If you create commits, include a concise message with these git trailers: - Nightshift-Task: %s - Nightshift-Ref: https://github.com/marcus/nightshift +1. If you create commits, use this commit message format: +%s 2. Implement the plan step by step 3. Make all necessary code changes 4. Ensure tests pass @@ -783,7 +782,7 @@ Description: %s "files_modified": ["file1.go", ...], "summary": "what was done" } -`, task.ID, task.Title, task.Description, plan.Description, plan.Steps, iterationNote, branchInstruction, task.Type) +`, task.ID, task.Title, task.Description, plan.Description, plan.Steps, iterationNote, branchInstruction, commitmsg.PromptSpec(string(task.Type))) } func (o *Orchestrator) buildReviewPrompt(task *tasks.Task, impl *ImplementOutput) string { diff --git a/internal/orchestrator/orchestrator_commitmsg_test.go b/internal/orchestrator/orchestrator_commitmsg_test.go new file mode 100644 index 0000000..7770ea9 --- /dev/null +++ b/internal/orchestrator/orchestrator_commitmsg_test.go @@ -0,0 +1,81 @@ +package orchestrator + +import ( + "strings" + "testing" + + "github.com/marcus/nightshift/internal/commitmsg" + "github.com/marcus/nightshift/internal/tasks" +) + +func commitMsgTestTask() *tasks.Task { + return &tasks.Task{ + ID: "commit-normalize", + Title: "Commit Message Normalizer", + Description: "Standardize commit message format", + Type: tasks.TaskType("commit-normalize"), + } +} + +// The prompts must keep instructing agents with the exact trailers the +// orchestrator has always required, now sourced from internal/commitmsg. +func TestPrompts_ContainNightshiftTrailers(t *testing.T) { + o := New() + task := commitMsgTestTask() + plan := &PlanOutput{Steps: []string{"step1"}, Description: "test plan"} + + prompts := map[string]string{ + "plan": o.buildPlanPrompt(task), + "implement": o.buildImplementPrompt(task, plan, 1), + } + for name, prompt := range prompts { + if !strings.Contains(prompt, "Nightshift-Task: commit-normalize") { + t.Errorf("%s prompt missing task trailer\nGot:\n%s", name, prompt) + } + if !strings.Contains(prompt, "Nightshift-Ref: https://github.com/marcus/nightshift") { + t.Errorf("%s prompt missing ref trailer\nGot:\n%s", name, prompt) + } + } +} + +// Both prompts must embed the same canonical spec the validator enforces, so +// agents are never told a format the commit-msg hook would reject. +func TestPrompts_EmbedCanonicalSpec(t *testing.T) { + o := New() + task := commitMsgTestTask() + plan := &PlanOutput{Steps: []string{"step1"}, Description: "test plan"} + + spec := commitmsg.PromptSpec("commit-normalize") + for name, prompt := range map[string]string{ + "plan": o.buildPlanPrompt(task), + "implement": o.buildImplementPrompt(task, plan, 1), + } { + if !strings.Contains(prompt, spec) { + t.Errorf("%s prompt does not embed commitmsg.PromptSpec\nGot:\n%s", name, prompt) + } + if !strings.Contains(prompt, "type(scope)") { + t.Errorf("%s prompt missing the header format\nGot:\n%s", name, prompt) + } + } +} + +// The spec handed to agents must itself describe a message the validator +// accepts: the example in the spec is checked against the real rules. +func TestPromptSpecExampleValidates(t *testing.T) { + example := "feat(cli): add commit message normalizer\n\n" + + "Adds a nightshift commit-msg subcommand that checks and rewrites commit\n" + + "messages so every commit in the repository shares one format.\n\n" + + "Nightshift-Task: commit-normalize\n" + + "Nightshift-Ref: https://github.com/marcus/nightshift\n" + + if !strings.Contains(commitmsg.Spec(), strings.TrimRight(example, "\n")[:40]) { + t.Fatalf("spec example drifted; update this test alongside commitmsg.Spec()") + } + m, err := commitmsg.Parse(example) + if err != nil { + t.Fatalf("parsing the spec example: %v", err) + } + if issues := commitmsg.Validate(m); commitmsg.HasErrors(issues) { + t.Errorf("the spec's own example fails validation: %v", issues) + } +} diff --git a/scripts/commit-msg.sh b/scripts/commit-msg.sh new file mode 100755 index 0000000..dffe2aa --- /dev/null +++ b/scripts/commit-msg.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# commit-msg hook for nightshift +# Install: make install-hooks (or: ln -sf ../../scripts/commit-msg.sh .git/hooks/commit-msg) +# +# Validates the commit message against the format defined in +# internal/commitmsg. Skip with: git commit --no-verify +set -euo pipefail + +MSG_FILE="${1:?usage: commit-msg.sh }" + +REPO_ROOT="$(git rev-parse --show-toplevel)" + +echo "🪡 commit-msg check" + +# Prefer the built binary; fall back to `go run` so the hook works in a fresh +# clone before `make build`. +if [[ -x "$REPO_ROOT/nightshift" ]]; then + RUNNER=("$REPO_ROOT/nightshift") +elif command -v nightshift >/dev/null 2>&1; then + RUNNER=(nightshift) +elif command -v go >/dev/null 2>&1; then + RUNNER=(go run "$REPO_ROOT/cmd/nightshift") +else + echo " – skipped (no nightshift binary and no go toolchain)" + exit 0 +fi + +if "${RUNNER[@]}" commit-msg --check "$MSG_FILE"; then + echo "✅ commit message OK" + exit 0 +fi + +echo "" +echo "❌ Commit message rejected." +echo " Rewrite it automatically with:" +echo " ${RUNNER[*]} commit-msg --fix \"$MSG_FILE\"" +echo " Or skip this check with: git commit --no-verify" +exit 1 diff --git a/website/docs/cli-reference.md b/website/docs/cli-reference.md index d5a2cd4..08c2fa6 100644 --- a/website/docs/cli-reference.md +++ b/website/docs/cli-reference.md @@ -73,6 +73,32 @@ nightshift task run lint-fix --provider claude nightshift task run lint-fix --provider codex --dry-run ``` +## Commit Message Commands + +```bash +nightshift commit-msg --print-spec # Print the required format +nightshift commit-msg --check .git/COMMIT_EDITMSG +nightshift commit-msg --fix .git/COMMIT_EDITMSG # Rewrite the file in place +printf 'Fixed the thing.' | nightshift commit-msg --fix - +``` + +`commit-msg` reads the message from the given file, or from stdin when the +argument is `-` or omitted. Without `--fix` it validates, printing issues as +`file:line: severity: message (rule)` and exiting non-zero if any are errors. + +| Flag | Description | +|------|-------------| +| `--check` | Validate only; exit non-zero when the message has errors (the default) | +| `--fix` | Rewrite the message into the canonical format | +| `--print-spec` | Print the commit message format and exit | +| `--quiet` | Suppress issue output; rely on the exit code | + +The expected format is Conventional Commits: `type(scope)!: subject`, a blank +line, a body wrapped at 72 columns, and a single trailing block of git +trailers. `make install-hooks` installs a `commit-msg` hook that runs +`nightshift commit-msg --check` on every commit; bypass it with +`git commit --no-verify`. + ## Budget Commands ```bash From 86eab7a2bb845dbf1da8410a6763580468aadab1 Mon Sep 17 00:00:00 2001 From: Greg Gardner Date: Sat, 29 Aug 2026 02:21:53 -0700 Subject: [PATCH 2/2] fix(commitmsg): exempt git-generated commit messages Git writes merge, revert and fixup!/squash!/amend! headers itself, and the user cannot choose their format. Validating them made the installed commit-msg hook reject `git merge --no-ff` (leaving the repo mid-merge) and every `git commit --fixup`, which in turn broke `git rebase --autosquash`. Parse now flags those headers as GitGenerated, and Validate and Normalize pass them straight through, so the CLI, the hook and any future CI check all agree. The exemption is documented in the canonical spec, so --print-spec, the hook's failure output, the README, the CLI reference and the agent prompts all describe it too. Nightshift-Task: commit-normalize Nightshift-Ref: https://github.com/marcus/nightshift --- CHANGELOG.md | 3 +- README.md | 3 + cmd/nightshift/commands/commitmsg_test.go | 43 ++++++++ internal/commitmsg/commitmsg.go | 29 +++++- internal/commitmsg/commitmsg_test.go | 121 ++++++++++++++++++++++ internal/commitmsg/spec.go | 3 + website/docs/cli-reference.md | 6 +- 7 files changed, 205 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index daca4be..b177bfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,8 @@ All notable changes to nightshift are documented in this file. Conventional Commits format. `make install-hooks` now also installs a `commit-msg` git hook, and the orchestrator's plan and implement prompts quote the same canonical spec so autonomous agents write messages the hook - accepts. + accepts. Messages git generates itself — merges, reverts and + `fixup!`/`squash!`/`amend!` commits — are exempt from the format. ## [v0.3.3] - 2026-02-19 diff --git a/README.md b/README.md index 81db2d1..74b07f2 100644 --- a/README.md +++ b/README.md @@ -290,6 +290,9 @@ Trailer-Key: value - the whole header line is at most 72 characters, imperative mood, no trailing period - trailers form one block at the very end +- messages git writes itself are exempt — headers starting with `Merge `, + `Revert "`, `fixup! `, `squash! ` or `amend! ` always pass, so `git merge` + and `git rebase --autosquash` are unaffected `make install-hooks` also installs a `commit-msg` hook that enforces this via `nightshift commit-msg --check`. You can run the checker yourself: diff --git a/cmd/nightshift/commands/commitmsg_test.go b/cmd/nightshift/commands/commitmsg_test.go index 9130232..ce46fee 100644 --- a/cmd/nightshift/commands/commitmsg_test.go +++ b/cmd/nightshift/commands/commitmsg_test.go @@ -155,3 +155,46 @@ func TestCommitMsgCmd_EndToEnd(t *testing.T) { t.Errorf("fixed file = %q", string(got)) } } + +func TestRunCommitMsg_AcceptsGitGeneratedMessages(t *testing.T) { + for _, raw := range []string{ + "Merge branch 'side'\n", + "Merge branch 'side'\n\n# Conflicts:\n#\tfile.go\n", + "Revert \"feat(cli): add commit message normalizer\"\n\nThis reverts commit deadbeef.\n", + "fixup! feat(cli): add commit message normalizer\n", + "squash! feat(cli): add commit message normalizer\n", + "amend! feat(cli): add commit message normalizer\n", + } { + path := writeTempMsg(t, raw) + var out, errBuf bytes.Buffer + if err := runCommitMsg(path, &commitMsgOptions{check: true}, strings.NewReader(""), &out, &errBuf); err != nil { + t.Errorf("check %q: %v\nstderr: %s", raw, err, errBuf.String()) + } + } +} + +func TestRunCommitMsg_FixLeavesGitGeneratedAlone(t *testing.T) { + raw := "Merge branch 'side' into main\n" + path := writeTempMsg(t, raw) + var out, errBuf bytes.Buffer + if err := runCommitMsg(path, &commitMsgOptions{fix: true}, strings.NewReader(""), &out, &errBuf); err != nil { + t.Fatalf("fix: %v\nstderr: %s", err, errBuf.String()) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading file: %v", err) + } + if string(got) != raw { + t.Errorf("fixed file = %q, want it unchanged", string(got)) + } +} + +func TestRunCommitMsg_PrintSpecMentionsGitGeneratedExemption(t *testing.T) { + var out, errBuf bytes.Buffer + if err := runCommitMsg("-", &commitMsgOptions{printSpec: true}, strings.NewReader(""), &out, &errBuf); err != nil { + t.Fatalf("print-spec: %v", err) + } + if !strings.Contains(out.String(), "fixup! ") || !strings.Contains(out.String(), "Merge ") { + t.Errorf("spec does not document the git-generated exemption:\n%s", out.String()) + } +} diff --git a/internal/commitmsg/commitmsg.go b/internal/commitmsg/commitmsg.go index b7048f4..3f3ea69 100644 --- a/internal/commitmsg/commitmsg.go +++ b/internal/commitmsg/commitmsg.go @@ -65,6 +65,11 @@ type Message struct { Body string Trailers []Trailer + // GitGenerated marks a message git wrote itself — a merge, a revert, or a + // fixup!/squash!/amend! commit. Those headers are fixed by git and are + // exempt from every rule below. + GitGenerated bool + // rawHeader is the header line exactly as written, used to report issues // about headers that do not match the conventional format at all. rawHeader string @@ -99,8 +104,20 @@ var ( // Git trailer syntax: "Token: value", token may contain letters, digits // and hyphens. "BREAKING CHANGE" is allowed as a special case. trailerRe = regexp.MustCompile(`^(BREAKING CHANGE|[A-Za-z][A-Za-z0-9-]*):[ \t]+(.*\S)[ \t]*$`) + // Headers git generates on the user's behalf. Enforcing the conventional + // format on these would break `git merge --no-ff` (which aborts mid-merge + // when commit-msg fails) and every `git commit --fixup`/`--squash`, and so + // `git rebase --autosquash`. + gitGeneratedRe = regexp.MustCompile(`^(Merge |Revert "|fixup! |squash! |amend! )`) ) +// IsGitGenerated reports whether a header line was written by git itself — +// a merge, a revert, or a fixup!/squash!/amend! commit — and is therefore +// exempt from the commit format. +func IsGitGenerated(header string) bool { + return gitGeneratedRe.MatchString(header) +} + // knownTrailerKeys are hoisted out of the body even when they appear in the // middle of a message, so every message ends with one trailer block. var knownTrailerKeys = map[string]bool{ @@ -131,7 +148,7 @@ func Parse(raw string) (*Message, error) { return nil, ErrEmptyMessage } - m := &Message{rawHeader: lines[0]} + m := &Message{rawHeader: lines[0], GitGenerated: IsGitGenerated(lines[0])} if match := headerRe.FindStringSubmatch(lines[0]); match != nil { m.Type = match[1] m.Scope = match[2] @@ -271,6 +288,12 @@ func Validate(m *Message) []Issue { return ValidateWithOptions(m, DefaultOptions // ValidateWithOptions checks the message against opts and returns every // issue found, errors and warnings alike. func ValidateWithOptions(m *Message, opts Options) []Issue { + // Git writes merge, revert and fixup!/squash!/amend! headers itself; the + // user cannot choose their format, so there is nothing to enforce. + if m.GitGenerated { + return nil + } + var issues []Issue add := func(line int, sev Severity, rule, format string, args ...interface{}) { issues = append(issues, Issue{Line: line, Severity: sev, Rule: rule, Message: fmt.Sprintf(format, args...)}) @@ -367,6 +390,10 @@ func Normalize(raw string, opts Options) (string, []Issue, error) { if err != nil { return "", nil, err } + if m.GitGenerated { + // Nothing to rewrite: only git's comments and verbose diff are dropped. + return strings.Join(cleanLines(raw), "\n") + "\n", nil, nil + } if strings.TrimSpace(m.Subject) == "" { return "", nil, errors.New("commit message has no subject") } diff --git a/internal/commitmsg/commitmsg_test.go b/internal/commitmsg/commitmsg_test.go index b5769e7..5d8f562 100644 --- a/internal/commitmsg/commitmsg_test.go +++ b/internal/commitmsg/commitmsg_test.go @@ -501,3 +501,124 @@ func TestPromptSpec_IncludesTrailers(t *testing.T) { t.Errorf("PromptSpec missing ref trailer:\n%s", s) } } + +// --- Git-generated messages --- + +// gitGenerated are headers git itself writes, which the hook must let through +// untouched: rejecting them breaks `git merge --no-ff` and `--autosquash`. +var gitGenerated = []string{ + "Merge branch 'side'", + "Merge branch 'side' into main", + "Merge pull request #186 from marcus/chore/commit-message-normalizer", + "Merge remote-tracking branch 'origin/main'", + `Revert "feat(cli): add commit message normalizer"`, + "fixup! feat(cli): add commit message normalizer", + "squash! feat(cli): add commit message normalizer", + "amend! feat(cli): add commit message normalizer", +} + +func TestIsGitGenerated(t *testing.T) { + for _, header := range gitGenerated { + if !IsGitGenerated(header) { + t.Errorf("IsGitGenerated(%q) = false, want true", header) + } + } + notGenerated := []string{ + "feat: add merge support", + "Merged the branches", + "merge branch 'side'", + "Revert the thing", + "fixup the thing", + "fix: revert \"a change\"", + "Mergebranch 'side'", + } + for _, header := range notGenerated { + if IsGitGenerated(header) { + t.Errorf("IsGitGenerated(%q) = true, want false", header) + } + } +} + +func TestParse_MarksGitGenerated(t *testing.T) { + for _, header := range gitGenerated { + m, err := Parse(header + "\n") + if err != nil { + t.Fatalf("Parse(%q): %v", header, err) + } + if !m.GitGenerated { + t.Errorf("Parse(%q).GitGenerated = false, want true", header) + } + } + m, err := Parse("feat: add a thing\n") + if err != nil { + t.Fatalf("Parse: %v", err) + } + if m.GitGenerated { + t.Error("GitGenerated = true for a conventional header, want false") + } +} + +func TestValidate_SkipsGitGenerated(t *testing.T) { + for _, header := range gitGenerated { + m, err := Parse(header + "\n") + if err != nil { + t.Fatalf("Parse(%q): %v", header, err) + } + if issues := Validate(m); len(issues) != 0 { + t.Errorf("Validate(%q) = %v, want no issues", header, issues) + } + } +} + +func TestValidate_GitGeneratedIgnoresRequiredTrailers(t *testing.T) { + opts := DefaultOptions() + opts.RequiredTrailers = []Trailer{{Key: "Nightshift-Task", Value: "commit-normalize"}} + m, err := Parse("Merge branch 'side'\n") + if err != nil { + t.Fatalf("Parse: %v", err) + } + if issues := ValidateWithOptions(m, opts); len(issues) != 0 { + t.Errorf("ValidateWithOptions = %v, want no issues", issues) + } +} + +func TestValidate_MergeWithConflictBodyIsClean(t *testing.T) { + // What git actually hands the hook for a conflicted merge resolution. + raw := "Merge branch 'side'\n\n# Conflicts:\n#\tfile.go\n" + m, err := Parse(raw) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if issues := Validate(m); len(issues) != 0 { + t.Errorf("Validate = %v, want no issues", issues) + } +} + +func TestNormalize_LeavesGitGeneratedUntouched(t *testing.T) { + opts := DefaultOptions() + opts.RequiredTrailers = []Trailer{{Key: "Nightshift-Task", Value: "commit-normalize"}} + for _, header := range gitGenerated { + raw := header + "\n" + got, issues, err := Normalize(raw, opts) + if err != nil { + t.Fatalf("Normalize(%q): %v", header, err) + } + if got != raw { + t.Errorf("Normalize(%q) = %q, want it unchanged", header, got) + } + if len(issues) != 0 { + t.Errorf("Normalize(%q) issues = %v, want none", header, issues) + } + } +} + +func TestNormalize_PreservesMergeBody(t *testing.T) { + raw := "Merge branch 'side' into main\n\nA long line in a merge body that nobody should be rewrapping at all.\n" + got, _, err := Normalize(raw, DefaultOptions()) + if err != nil { + t.Fatalf("Normalize: %v", err) + } + if got != raw { + t.Errorf("Normalize = %q, want %q", got, raw) + } +} diff --git a/internal/commitmsg/spec.go b/internal/commitmsg/spec.go index f2461db..b892212 100644 --- a/internal/commitmsg/spec.go +++ b/internal/commitmsg/spec.go @@ -70,6 +70,9 @@ Rules - body paragraphs wrap at %d columns; code blocks, indented text, lists and URLs are left alone - trailers form one block at the very end, one "Key: value" per line + - messages git writes itself are exempt: headers starting with 'Merge ', + 'Revert "', 'fixup! ', 'squash! ' or 'amend! ' are always accepted, so + merges and rebase --autosquash are never blocked Example feat(cli): add commit message normalizer diff --git a/website/docs/cli-reference.md b/website/docs/cli-reference.md index 08c2fa6..e1ad936 100644 --- a/website/docs/cli-reference.md +++ b/website/docs/cli-reference.md @@ -95,7 +95,11 @@ argument is `-` or omitted. Without `--fix` it validates, printing issues as The expected format is Conventional Commits: `type(scope)!: subject`, a blank line, a body wrapped at 72 columns, and a single trailing block of git -trailers. `make install-hooks` installs a `commit-msg` hook that runs +trailers. Messages git generates itself — headers starting with `Merge `, +`Revert "`, `fixup! `, `squash! ` or `amend! ` — are exempt, so merges and +`git rebase --autosquash` are never blocked. + +`make install-hooks` installs a `commit-msg` hook that runs `nightshift commit-msg --check` on every commit; bypass it with `git commit --no-verify`.