diff --git a/.githooks/commit-msg b/.githooks/commit-msg new file mode 100755 index 0000000000..b64e38a228 --- /dev/null +++ b/.githooks/commit-msg @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# +# commit-msg — enforce Conventional Commits 1.0.0 +# https://www.conventionalcommits.org/en/v1.0.0/ +# +# Subject format: ()!: +# - must be one of the angular types (feat, fix, ...) +# - () is optional +# - ! is optional and marks a breaking change +# - is mandatory and must be non-empty +# +# Bypass: commit with --no-verify. +# Merge, revert, fixup!, squash!, and amend! messages are passed through. + +set -eu + +COMMIT_MSG_FILE="${1:-}" +if [ -z "$COMMIT_MSG_FILE" ] || [ ! -f "$COMMIT_MSG_FILE" ]; then + echo "commit-msg: missing commit message file" >&2 + exit 1 +fi + +# First non-comment, non-empty line is the subject. +subject="" +while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + ''|'#'*) continue ;; + esac + subject="$line" + break +done < "$COMMIT_MSG_FILE" + +if [ -z "$subject" ]; then + echo "commit-msg: empty commit message" >&2 + exit 1 +fi + +# Pass-through commits generated by git itself. +case "$subject" in + "Merge "*|"Revert \""*|"fixup! "*|"squash! "*|"amend! "*) + exit 0 + ;; +esac + +ALLOWED_TYPES="feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert" +# Description must not start with an uppercase letter — kept in sync with the +# subjectPattern in .github/workflows/conventional-commits.yml so the local +# hook is the strictly tighter of the two gates. (Without this guard, a commit +# like "feat: Add thing" passes locally but fails the PR-title CI check.) +PATTERN="^(${ALLOWED_TYPES})(\([^)]+\))?!?: [^A-Z].*" + +if printf '%s' "$subject" | grep -Eq "$PATTERN"; then + exit 0 +fi + +cat >&2 <()!: + (description must start with a lowercase letter) + + Allowed types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert + Examples: + feat(router): add weighted round-robin strategy + fix(bedrock): decouple STS region from aws_region_name + chore(deps): bump black to 26.3.1 + refactor!: drop Python 3.8 support + +See https://www.conventionalcommits.org/en/v1.0.0/ + +To bypass (use sparingly): git commit --no-verify +EOF +exit 1 diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 0000000000..c2267c8501 --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,92 @@ +#!/usr/bin/env bash +# +# pre-push — enforce Conventional Branches +# https://conventional-branch.github.io/ +# +# Branch format: / +# must be one of: feature, bugfix, hotfix, release, chore +# +# Protected branches (always allowed): +# - main +# - litellm_internal_staging +# - dependabot/* +# - gh-readonly-queue/* +# +# Tag pushes and branch deletions are skipped. +# Bypass: git push --no-verify. + +set -eu + +ZERO_OID="0000000000000000000000000000000000000000" +ZERO_OID_SHA256="0000000000000000000000000000000000000000000000000000000000000000" +ALLOWED_TYPES="feature|bugfix|hotfix|release|chore" +BRANCH_PATTERN="^(${ALLOWED_TYPES})/.+" + +PROTECTED_NAMES="main litellm_internal_staging" +PROTECTED_PREFIXES="dependabot/ gh-readonly-queue/" + +is_protected() { + branch="$1" + for name in $PROTECTED_NAMES; do + if [ "$branch" = "$name" ]; then + return 0 + fi + done + for prefix in $PROTECTED_PREFIXES; do + case "$branch" in "$prefix"*) return 0 ;; esac + done + return 1 +} + +invalid="" + +while read -r local_ref local_oid remote_ref remote_oid; do + # Branch deletion (no local commit being pushed). + if [ "$local_oid" = "$ZERO_OID" ] || [ "$local_oid" = "$ZERO_OID_SHA256" ]; then + continue + fi + + # Only validate branch pushes; ignore tags and other ref namespaces. + case "$remote_ref" in + refs/heads/*) ;; + *) continue ;; + esac + + branch="${remote_ref#refs/heads/}" + + if is_protected "$branch"; then + continue + fi + + if ! printf '%s' "$branch" | grep -Eq "$BRANCH_PATTERN"; then + invalid="$invalid $branch" + fi +done + +if [ -n "$invalid" ]; then + cat >&2 </ + + Allowed types: feature, bugfix, hotfix, release, chore + Examples: + feature/weighted-round-robin + bugfix/streaming-empty-chunks + chore/bump-deps + hotfix/auth-bypass + + Protected (always allowed): main, litellm_internal_staging, + dependabot/*, gh-readonly-queue/*. + +See https://conventional-branch.github.io/ + +Rename with: git branch -m +To bypass (use sparingly): git push --no-verify +EOF + exit 1 +fi + +exit 0 diff --git a/.github/workflows/conventional-commits.yml b/.github/workflows/conventional-commits.yml new file mode 100644 index 0000000000..69ade24d02 --- /dev/null +++ b/.github/workflows/conventional-commits.yml @@ -0,0 +1,46 @@ +name: Conventional PR Title + +# Squash-merge replaces the merge commit subject with the PR title, so +# enforcing Conventional Commits at the PR-title level is what actually gates +# the commits that land on the default branch. The local commit-msg hook +# (.githooks/commit-msg) is a best-effort assist; this workflow is the gate. +# +# See https://www.conventionalcommits.org/en/v1.0.0/ + +on: + pull_request: + types: [opened, edited, reopened, synchronize, labeled, unlabeled] + +permissions: + pull-requests: read + +jobs: + lint-pr-title: + name: Validate PR title + runs-on: ubuntu-latest + steps: + - name: Check title against Conventional Commits + uses: amannn/action-semantic-pull-request@48f256284bd46cdaab1048c3721360e808335d50 # v6.1.1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + # Must mirror the type list in .githooks/commit-msg. + types: | + feat + fix + docs + style + refactor + perf + test + build + ci + chore + revert + requireScope: false + subjectPattern: ^(?![A-Z]).+$ + subjectPatternError: | + The subject "{subject}" must start with a lowercase character. + # Allow merges/reverts that GitHub generates automatically. + ignoreLabels: | + ignore-semantic-pull-request diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8ac83341f6..2177c76480 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,18 +38,25 @@ Before contributing code to LiteLLM, you must sign our [Contributor License Agre git clone https://github.com/YOUR_USERNAME/litellm.git cd litellm -# Create a new branch for your feature -git checkout -b your-feature-branch +# Create a new branch for your feature (see "Commit and Branch Conventions" below) +git checkout -b feature/your-feature # Install development dependencies make install-dev +# Install git hooks that enforce commit + branch conventions (one-time, opt-in) +make install-hooks + # Verify your setup works make help ``` That's it! Your local development environment is ready. +## Commit and Branch Conventions + +Commits follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) and branches follow [Conventional Branches](https://conventional-branch.github.io/). Run `make install-hooks` once per clone to enable the local git hooks that enforce these — see the [contributor docs](https://docs.litellm.ai/docs/extras/contributing_code#commit-and-branch-conventions) for the full type list, examples, the protected-branch bypass list, and how to opt out. + ### 2. Development Workflow Here's the recommended workflow for making changes: @@ -67,12 +74,12 @@ make lint # Run unit tests to ensure nothing is broken make test-unit -# Commit your changes +# Commit your changes (must follow Conventional Commits — see above) git add . -git commit -m "Your descriptive commit message" +git commit -m "feat(scope): your descriptive commit message" -# Push and create a PR -git push origin your-feature-branch +# Push and create a PR (branch must follow Conventional Branches — see above) +git push origin feature/your-feature ``` ## Adding Testing diff --git a/Makefile b/Makefile index a00a90da60..3d7b51bc74 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ test-unit-integrations test-unit-core-utils test-unit-other test-unit-root \ test-proxy-unit-a test-proxy-unit-b test-integration test-unit-helm \ info lint lint-dev format \ - install-dev install-proxy-dev install-test-deps \ + install-dev install-proxy-dev install-test-deps install-hooks \ install-helm-unittest check-circular-imports check-import-safety # Default target @@ -17,6 +17,7 @@ help: @echo " make install-proxy-dev-ci - Install proxy dev dependencies (CI-compatible)" @echo " make install-test-deps - Install the full local test environment" @echo " make install-helm-unittest - Install helm unittest plugin" + @echo " make install-hooks - Install git hooks (Conventional Commits + Branches)" @echo " make format - Apply Black code formatting" @echo " make format-check - Check Black code formatting (matches CI)" @echo " make lint - Run all linting (Ruff, MyPy, Black check, circular imports, import safety)" @@ -68,6 +69,11 @@ install-test-deps: install-proxy-dev install-helm-unittest: helm plugin install https://github.com/helm-unittest/helm-unittest --version v0.4.4 || echo "ignore error if plugin exists" +# Install git hooks that enforce Conventional Commits and Conventional Branches. +# Opt-in: not chained into install-dev. +install-hooks: + ./scripts/install_git_hooks.sh + # Formatting format: install-dev cd litellm && $(UV_RUN) black . && cd .. diff --git a/scripts/install_git_hooks.sh b/scripts/install_git_hooks.sh new file mode 100755 index 0000000000..1e4e3c6de1 --- /dev/null +++ b/scripts/install_git_hooks.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# +# Install the repo's git hooks by pointing core.hooksPath at .githooks. +# +# Idempotent: re-running just reaffirms the config and refreshes chmod bits. +# Run from anywhere inside the repo. + +set -euo pipefail + +if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then + echo "install_git_hooks: not inside a git working tree" >&2 + exit 1 +fi + +repo_root=$(git rev-parse --show-toplevel) +hooks_dir="$repo_root/.githooks" + +if [ ! -d "$hooks_dir" ]; then + echo "install_git_hooks: $hooks_dir does not exist" >&2 + exit 1 +fi + +# Ensure the hook scripts are executable. New clones on case-preserving +# filesystems sometimes lose the exec bit; this normalizes it. +chmod +x "$hooks_dir"/* 2>/dev/null || true + +git config core.hooksPath .githooks + +cat < subprocess.CompletedProcess: + msg_file = tmp_path / "COMMIT_EDITMSG" + msg_file.write_text(subject + "\n", encoding="utf-8") + return subprocess.run( + ["bash", str(_COMMIT_MSG_HOOK), str(msg_file)], + capture_output=True, + text=True, + check=False, + ) + + +def _run_pre_push(stdin: str) -> subprocess.CompletedProcess: + return subprocess.run( + ["bash", str(_PRE_PUSH_HOOK)], + input=stdin, + capture_output=True, + text=True, + check=False, + ) + + +def _ref_line(branch: str, local_oid: str = _NONZERO_OID, remote_oid: str = _ZERO_OID) -> str: + ref = f"refs/heads/{branch}" + return f"{ref} {local_oid} {ref} {remote_oid}\n" + + +# ----- commit-msg ----------------------------------------------------------- + + +@pytest.mark.parametrize( + "subject", + [ + "feat(router): add weighted round-robin strategy", + "fix(bedrock): decouple STS region from aws_region_name", + "chore(deps): bump black to 26.3.1", + "docs: rewrite contributing guide", + "refactor!: drop Python 3.8 support", + "feat(api,proxy)!: rename endpoint", + "test: cover hook bypass list", + "perf(streaming): avoid extra json parse", + "revert: feat(router): add weighted round-robin", + ], +) +def test_commit_msg_accepts_conventional_subjects(tmp_path, subject): + result = _run_commit_msg(subject, tmp_path) + assert result.returncode == 0, ( + f"hook rejected a valid subject:\n subject: {subject!r}\n" + f" stderr: {result.stderr}" + ) + + +@pytest.mark.parametrize( + "subject", + [ + "add stuff", # no type + "feat add router strategy", # missing colon + "feat:add router strategy", # missing space after colon + "feat():", # empty description + "ux: thing", # unknown type + "Feat(router): capital type", # types are lowercase + "feat(router):", # empty description + # Description must start with a lowercase letter — kept in sync with + # the CI workflow's subjectPattern so the local hook never accepts a + # subject that CI will later reject. + "feat: Add thing", + "fix(router): Decouple something", + "chore: BUMP deps", + "feat: A", + ], +) +def test_commit_msg_rejects_invalid_subjects(tmp_path, subject): + result = _run_commit_msg(subject, tmp_path) + assert result.returncode == 1, ( + f"hook accepted an invalid subject:\n subject: {subject!r}\n" + f" stderr: {result.stderr}" + ) + assert "Conventional Commits" in result.stderr + + +@pytest.mark.parametrize( + "subject", + [ + # Lowercase letter — the common case. + "feat: lowercase start is fine", + # The CI's `^(?![A-Z]).+$` rejects only uppercase A-Z, so digits and + # symbols are still allowed; mirror that behavior here. + "feat: 1-based indexing now works", + "fix(deps): @types/node bump", + ], +) +def test_commit_msg_accepts_non_uppercase_starts(tmp_path, subject): + result = _run_commit_msg(subject, tmp_path) + assert result.returncode == 0, ( + f"hook rejected a valid non-uppercase-start subject:\n" + f" subject: {subject!r}\n stderr: {result.stderr}" + ) + + +@pytest.mark.parametrize( + "subject", + [ + "Merge branch 'main' into feature/foo", + 'Revert "feat(router): add weighted round-robin strategy"', + "fixup! feat(router): add weighted round-robin strategy", + "squash! feat(router): add weighted round-robin strategy", + "amend! feat(router): add weighted round-robin strategy", + ], +) +def test_commit_msg_passes_git_generated_messages(tmp_path, subject): + result = _run_commit_msg(subject, tmp_path) + assert result.returncode == 0, ( + f"hook should pass git-generated subject:\n subject: {subject!r}\n" + f" stderr: {result.stderr}" + ) + + +def test_commit_msg_rejects_empty_message(tmp_path): + result = _run_commit_msg("", tmp_path) + assert result.returncode == 1 + assert "empty commit message" in result.stderr + + +def test_commit_msg_skips_comment_only_lines(tmp_path): + # An all-comments file has no subject — should be rejected. + msg_file = tmp_path / "COMMIT_EDITMSG" + msg_file.write_text("# please enter a commit message\n# above this line\n", encoding="utf-8") + result = subprocess.run( + ["bash", str(_COMMIT_MSG_HOOK), str(msg_file)], + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 1 + assert "empty commit message" in result.stderr + + +def test_commit_msg_uses_first_non_comment_line(tmp_path): + # Real git-generated COMMIT_EDITMSG has a status block prefixed with '#' + # below the subject. Make sure leading comment lines are skipped too. + msg_file = tmp_path / "COMMIT_EDITMSG" + msg_file.write_text( + "# On branch feature/foo\n" + "\n" + "feat(router): add weighted round-robin\n" + "\n" + "# Please enter the commit message...\n", + encoding="utf-8", + ) + result = subprocess.run( + ["bash", str(_COMMIT_MSG_HOOK), str(msg_file)], + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + + +# ----- pre-push ------------------------------------------------------------- + + +@pytest.mark.parametrize( + "branch", + [ + "feature/weighted-round-robin", + "bugfix/streaming-empty-chunks", + "hotfix/auth-bypass", + "release/v1.45.0", + "chore/bump-deps", + "feature/nested/path/ok", # nested slashes after type are fine + ], +) +def test_pre_push_accepts_conventional_branches(branch): + result = _run_pre_push(_ref_line(branch)) + assert result.returncode == 0, ( + f"hook rejected a valid branch:\n branch: {branch!r}\n" + f" stderr: {result.stderr}" + ) + + +@pytest.mark.parametrize( + "branch", + [ + "random-branch-name", + "litellm_fix/optimize-streaming", # legacy pattern is now rejected + "ui/navbar-notifications", # not in the allow list + "feature/", # empty description + "Feature/foo", # type is case-sensitive + "feat/foo", # angular commit type, not branch type + ], +) +def test_pre_push_rejects_non_conventional_branches(branch): + result = _run_pre_push(_ref_line(branch)) + assert result.returncode == 1, ( + f"hook accepted an invalid branch:\n branch: {branch!r}\n" + f" stderr: {result.stderr}" + ) + assert "Conventional Branches" in result.stderr + + +@pytest.mark.parametrize( + "branch", + [ + "main", + "litellm_internal_staging", + "dependabot/github_actions/foo", + "gh-readonly-queue/main/abc123", + ], +) +def test_pre_push_bypasses_protected_branches(branch): + result = _run_pre_push(_ref_line(branch)) + assert result.returncode == 0, ( + f"protected branch was rejected:\n branch: {branch!r}\n" + f" stderr: {result.stderr}" + ) + + +def test_pre_push_skips_tag_pushes(): + line = f"refs/tags/v1 {_NONZERO_OID} refs/tags/v1 {_ZERO_OID}\n" + result = _run_pre_push(line) + assert result.returncode == 0, result.stderr + + +def test_pre_push_skips_branch_deletions(): + # local oid all zeros = deletion + line = f"refs/heads/whatever {_ZERO_OID} refs/heads/whatever {_NONZERO_OID}\n" + result = _run_pre_push(line) + assert result.returncode == 0, result.stderr + + +def test_pre_push_fails_if_any_ref_is_invalid(): + # Mixed batch: one valid, one invalid — entire push should fail. + stdin = _ref_line("feature/ok") + _ref_line("random-bad") + result = _run_pre_push(stdin) + assert result.returncode == 1 + assert "random-bad" in result.stderr + + +def test_pre_push_no_refs_passes(): + # Empty stdin (no refs being pushed) should pass. + result = _run_pre_push("") + assert result.returncode == 0, result.stderr