feat: add max-header-length for header length check

[aj3sh]

Description

• add max-header-length for header length check
• disable default header length check
• update dependency actions version
• add release details on contributing file

Related Issue

Fixes #67

Type of Change

Please mark the appropriate option below to describe the type of change your pull request introduces:

• Bug fix
• New feature
• Enhancement
• Documentation update
• Refactor
• Other (please specify)

Checklist

• My pull request has a clear title and description.
• I have used semantic commit messages.
  Examples: "fix: Fixed foobar bug", "feat(accounts): Added foobar feature".
• I have added/updated the necessary documentation on README.md.
• I have added appropriate test cases (if applicable) to ensure the changes are functioning correctly.

Additional Notes

[Add any additional notes or context that you think the reviewers should know about.]

By submitting this pull request, I confirm that I have read and complied with the contribution guidelines of this project.

[sugat009]

Supply chain security: please keep SHA pins for write-privileged actions. See inline comments.

[sugat009]

Suggestion: Keep SHA pin for this write-privileged action

This action has contents: write and creates GitHub releases/tags. The current main branch correctly SHA-pins it:

uses: googleapis/release-please-action@7987652d64b4581673a76e33ad5e98e3dd56832f # v4.1.3

Switching to a mutable tag (@v4) means anyone with write access to that repo can silently move the tag to point at different code — a known supply chain vector (ref: March 2025 tj-actions hijack).

Suggested change:

uses: googleapis/release-please-action@16a9c90856f42705d54a6fda1823352bdc62cf38 # v4

The # v4 comment lets Dependabot auto-update the SHA when new versions release.

[sugat009]

Suggestion: Keep SHA pin for PyPI publish action

This action publishes packages to PyPI — the highest-privilege action in the repo. The current main branch correctly SHA-pins it:

uses: pypa/gh-action-pypi-publish@ed0c53931b1dc9bd32cbe73a98c7f6766f8a527e # v1.13.0

v1.13.0 resolves to the exact same commit SHA already pinned on main, so this line should simply remain unchanged.

For ongoing maintenance, consider adding a .github/dependabot.yml to auto-update SHA pins:

version: 2
updates:
  - package-ecosystem: "github-actions"
    directory: "/"
    schedule:
      interval: "weekly"

[sugat009]

Bug: --max-header-length falsy check allows invalid values. See inline comment with reproduction steps.

[sugat009]

Bug: not max_header_length falsy check silently mishandles 0 and negative values

if not max_header_length: uses Python truthiness — 0 is falsy, so it skips validation entirely. Negative values pass the guard and produce nonsensical behavior.

Reproduction:

# Bug 1: --max-header-length 0 silently passes (should reject or error)
$ commitlint --max-header-length 0 "feat: this is a very long commit message that should definitely fail any reasonable header length check but it wont"
Commit validation: successful!

# Bug 2: --max-header-length -5 rejects everything with a nonsensical message
$ commitlint --max-header-length -5 "feat: short"
✖ Found 1 error(s).
- Header length cannot exceed -5 characters.

Suggested fix — two changes:

1. Use explicit None check here instead of falsy:

if max_header_length is None:
    return

2. Add input validation in cli.py to reject non-positive values at parse time:

def positive_int(value):
    ivalue = int(value)
    if ivalue < 1:
        raise argparse.ArgumentTypeError(f"{value} is not a positive integer")
    return ivalue

parser.add_argument("--max-header-length", type=positive_int, ...)

The same falsy issue exists in github_actions/action/run.py where if max_header_length: should be if max_header_length is not None:.

[sugat009]

Question on the architectural direction for configurable validator options. See inline comment.

[sugat009]

Question: should validators read from the global config singleton, or receive parameters explicitly?

With this change, HeaderLengthValidator now imports and reads config.max_header_length directly, making it the only validator with a hidden dependency on global state. The other validators are pure functions of commit_message alone, which keeps them easy to test and reason about.

A couple of things I'm wondering:

1. Is this the pattern we want going forward? If more options become configurable (e.g., --max-body-line-length, --allowed-types), each validator would import config and read its own values. The config singleton would grow from an infrastructure concern (verbose/quiet) into a bag of validator-specific settings, and every validator test would need to mock it.

2. Would it be cleaner to pass max_header_length explicitly through the call chain? For example, adding it as a parameter to lint_commit_message() (similar to how skip_detail and strip_comments are already passed), and then threading it through to the validator via a class attribute or run_validators(). That way config stays small and validators remain testable without patching globals.

3. On the default value: the existing COMMIT_HEADER_MAX_LENGTH = 72 constant served as a sensible default. With this PR, header length checking is off unless explicitly opted in. Was that intentional, or would it make more sense to default to 72 and let --max-header-length override it? Keeping the default preserves backward compatibility for existing users.

Not blocking, just want to align on the direction before this sets the pattern for future configurable rules.

[sugat009]

Additional review comments using conventional comments labels.

[sugat009]

issue (blocking): get_int_input silently swallows invalid input

get_boolean_input raises TypeError on invalid values, but get_int_input returns None and silently disables the check. If a user writes max_header_length: "abc" in their workflow YAML, there is no warning, no error, and no indication the configuration was ignored.

except ValueError:
    return None  # silently treated as "not configured"

This creates an inconsistency in the contract between the two input helpers and makes misconfiguration invisible to the user.

Suggested fix:

except ValueError:
    raise ValueError(
        f"Input '{key}' must be a valid integer, got: {val!r}"
    )

[sugat009]

suggestion (non-blocking): add a @property/setter for max_header_length to match the existing pattern

verbose and quiet both use @property with setters that enforce invariants. Adding max_header_length as a bare class attribute breaks that consistency and skips runtime validation entirely.

For example, config.max_header_length = "not-an-int" or config.max_header_length = -5 would be accepted silently.

_max_header_length: Optional[int] = None

@property
def max_header_length(self) -> Optional[int]:
    return self._max_header_length

@max_header_length.setter
def max_header_length(self, value: Optional[int]) -> None:
    if value is not None and value < 1:
        raise ValueError("max_header_length must be a positive integer")
    self._max_header_length = value

nitpick (non-blocking): int | None here vs Optional["_CommitlintConfig"] on line 13 is a style inconsistency within the same file. Both are valid for Python 3.10+, but picking one convention would keep things uniform.

[sugat009]

nitpick (non-blocking): help text could describe the default behavior and units

Currently:

help="Maximum header length to check"

A user reading --help won't know what happens when this is omitted (no length check) or what "length" means (characters).

Suggestion:

help="Maximum number of characters allowed in the commit header. "
     "If not specified, header length is not checked."

[sugat009]

todo (non-blocking): reset config.max_header_length in test teardown to prevent state leaks

The existing test fixtures reset config.verbose and config.quiet after each test class, but max_header_length is never cleaned up. Tests that call main() will set config.max_header_length on the singleton, and that value persists into subsequent tests.

This could cause flaky failures if test execution order changes or new tests are added that depend on the default None value.