ci(lint): reject layer allowlist entries added since the merge base

Part of LIT-9423

.github/workflows/test-linting.yml

@@ -193,7 +193,7 @@ jobs:
 
       - name: Check layer imports
         if: steps.changes.outputs.decision != 'skip'
-        run: uv run --no-sync python scripts/check_layer_imports.py
+        run: uv run --no-sync python scripts/check_layer_imports.py --base "$GATE_BASE_SHA"
 
       - name: Check import safety
         if: steps.changes.outputs.decision != 'skip'

Makefile

@@ -230,7 +230,7 @@ lint-gate: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
 	$(UV_RUN) python scripts/ruff_strict_gate.py --base "$(BASE_REF)"
 
 check-layer-imports: $(LINT_DEP_INSTALL)
-	$(UV_RUN) python scripts/check_layer_imports.py
+	$(UV_RUN) python scripts/check_layer_imports.py --base "$$($(RESOLVE_BASE))"
 
 check-import-safety: $(LINT_DEP_INSTALL)
 	@$(UV_RUN) python -c "from litellm import *; print('[from litellm import *] OK! no issues!');" || (echo '🚨 import failed, this means you introduced unprotected imports! 🚨'; exit 1)

scripts/check_layer_imports.py

@@ -1,6 +1,8 @@
+import argparse
 import ast
+import subprocess
 import sys
-from collections.abc import Callable, Iterable, Iterator
+from collections.abc import Callable, Iterable, Iterator, Sequence
 from dataclasses import dataclass
 from itertools import chain, product
 from pathlib import Path
@@ -140,12 +142,14 @@ def violations(paths: tuple[Path, ...]) -> tuple[Violation, ...]:
     )
 
 
-def read_allowlist(path: Path) -> frozenset[str]:
-    lines: Final = (" ".join(line.split()) for line in path.read_text(encoding="utf-8").splitlines())
+def parse_allowlist(text: str) -> frozenset[str]:
+    lines: Final = (" ".join(line.split()) for line in text.splitlines())
     return frozenset(line for line in lines if line and not line.startswith("#"))
 
 
-def report(found: tuple[Violation, ...], allowed: frozenset[str]) -> tuple[str, ...]:
+def report(
+    found: tuple[Violation, ...], allowed: frozenset[str], base_allowed: frozenset[str] | None
+) -> tuple[str, ...]:
     first_seen: Final = MappingProxyType({violation.key: violation for violation in reversed(found)})
     new: Final = tuple(
         f"{violation.edge.path}:{violation.edge.line}: {violation.rule} forbids "
@@ -157,12 +161,41 @@ def report(found: tuple[Violation, ...], allowed: frozenset[str]) -> tuple[str,
         f"{ALLOWLIST}: stale entry '{key}' no longer matches an import, delete it"
         for key in sorted(allowed - first_seen.keys())
     )
-    return new + stale
+    added: Final = (
+        tuple(
+            f"{ALLOWLIST}: entry '{key}' is not in the base allowlist, fix the import instead of allowlisting it"
+            for key in sorted(allowed - base_allowed)
+        )
+        if base_allowed is not None
+        else ()
+    )
+    return new + stale + added
 
 
-def main() -> int:
+def git_base_allowlist(ref: str) -> frozenset[str] | None:
+    merge_base: Final = subprocess.run(
+        ["git", "merge-base", ref, "HEAD"], capture_output=True, text=True, check=True
+    ).stdout.strip()
+    shown: Final = subprocess.run(
+        ["git", "show", f"{merge_base}:{ALLOWLIST.as_posix()}"], capture_output=True, text=True, check=False
+    )
+    return parse_allowlist(shown.stdout) if shown.returncode == 0 else None
+
+
+class Arguments(argparse.Namespace):
+    base: str | None = None
+
+
+def main(
+    argv: Sequence[str] | None = None,
+    base_allowlist: Callable[[str], frozenset[str] | None] = git_base_allowlist,
+) -> int:
+    parser: Final = argparse.ArgumentParser(description="Check SDK/proxy layer import rules")
+    parser.add_argument("--base", help="Also reject allowlist entries missing at the merge base with this ref")
+    args: Final = parser.parse_args(argv, namespace=Arguments())
     paths: Final = tuple(sorted(PACKAGE_ROOT.rglob("*.py")))
-    problems: Final = report(violations(paths), read_allowlist(ALLOWLIST))
+    base_allowed: Final = base_allowlist(args.base) if args.base else None
+    problems: Final = report(violations(paths), parse_allowlist(ALLOWLIST.read_text(encoding="utf-8")), base_allowed)
     if problems:
         print("\n".join(problems), file=sys.stderr)
         return 1

scripts/pre_commit_lint.sh

@@ -107,7 +107,7 @@ if [ -n "$(scope_match '^(litellm/proxy/_experimental/mcp_server/|scripts/check_
     uv run --no-sync python scripts/check_mcp_operation_boundary.py || exit 1
 fi
 if [ -n "$(scope_match '^(litellm/.*\.py|scripts/check_layer_imports\.py|scripts/layer_imports_allowlist\.txt)$')" ]; then
-    uv run --no-sync python scripts/check_layer_imports.py || exit 1
+    uv run --no-sync python scripts/check_layer_imports.py ${base_ref:+--base "$base_ref"} || exit 1
 fi
 e2e_py_files=$(scope_match "$e2e_py_pattern")
 test_tree_files=$(scope_match "$test_tree_pattern")

tests/unit/test_check_layer_imports.py

@@ -1,13 +1,9 @@
-import subprocess
-import sys
 from pathlib import Path
 
 import pytest
 
 from scripts.check_layer_imports import main
 
-SCRIPT = Path(__file__).resolve().parents[2] / "scripts/check_layer_imports.py"
-
 
 def build_tree(root: Path, files: dict[str, str], allowlist: str = "") -> None:
     for relative, source in {
@@ -23,9 +19,14 @@ def build_tree(root: Path, files: dict[str, str], allowlist: str = "") -> None:
     (root / "scripts/layer_imports_allowlist.txt").write_text(allowlist)
 
 
-def run_check(root: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> tuple[int, str]:
+def run_check(
+    root: Path,
+    monkeypatch: pytest.MonkeyPatch,
+    capsys: pytest.CaptureFixture[str],
+    base_allowed: frozenset[str] | None = None,
+) -> tuple[int, str]:
     monkeypatch.chdir(root)
-    code = main()
+    code = main(["--base", "origin/main"], lambda _ref: base_allowed)
     return code, capsys.readouterr().err
 
 
@@ -162,17 +163,34 @@ def test_allowlisted_violation_passes_and_stale_entry_fails(
     ]
 
 
-def test_command_exit_code(tmp_path: Path) -> None:
-    build_tree(tmp_path, {"litellm/types/foo.py": "from litellm.router import Router\n"})
-    rejected = subprocess.run(
-        [sys.executable, "-I", str(SCRIPT)], cwd=tmp_path, capture_output=True, text=True, check=False
+def test_allowlist_entry_added_since_base_fails(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
+) -> None:
+    build_tree(
+        tmp_path,
+        {
+            "litellm/types/foo.py": "from litellm.router import Router\n",
+            "litellm/types/bar.py": "import litellm.main\n",
+        },
+        "L3 module litellm/types/foo.py litellm.router\nL3 module litellm/types/bar.py litellm.main\n",
     )
-    assert rejected.returncode == 1
-    assert "litellm/types/foo.py:1: L3 forbids" in rejected.stderr
+    base = frozenset({"L3 module litellm/types/foo.py litellm.router"})
 
-    (tmp_path / "scripts/layer_imports_allowlist.txt").write_text("L3 module litellm/types/foo.py litellm.router\n")
-    accepted = subprocess.run(
-        [sys.executable, "-I", str(SCRIPT)], cwd=tmp_path, capture_output=True, text=True, check=False
+    assert run_check(tmp_path, monkeypatch, capsys, base) == (
+        1,
+        "scripts/layer_imports_allowlist.txt: entry 'L3 module litellm/types/bar.py litellm.main' is not in the "
+        "base allowlist, fix the import instead of allowlisting it\n",
     )
-    assert accepted.returncode == 0
-    assert accepted.stdout == "Layer imports: passed\n"
+    assert run_check(tmp_path, monkeypatch, capsys, base | {"L3 module litellm/types/bar.py litellm.main"}) == (0, "")
+
+
+def test_missing_base_allowlist_skips_growth_check(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
+) -> None:
+    build_tree(
+        tmp_path,
+        {"litellm/types/foo.py": "from litellm.router import Router\n"},
+        "L3 module litellm/types/foo.py litellm.router\n",
+    )
+
+    assert run_check(tmp_path, monkeypatch, capsys, None) == (0, "")