From 0d7b89c86790c84da76843ab9d62eec63a9d8335 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 7 Sep 2026 15:29:37 -0700 Subject: [PATCH 1/7] ci: report a shadow-mode PR risk tier from changed paths, diff size, and test delta --- .github/risk-tiers.yml | 62 ++++ .github/scripts/risk_tier.py | 342 +++++++++++++++++++ .github/workflows/risk-gate.yml | 83 +++++ tests/test_litellm/test_github_risk_tier.py | 351 ++++++++++++++++++++ 4 files changed, 838 insertions(+) create mode 100644 .github/risk-tiers.yml create mode 100644 .github/scripts/risk_tier.py create mode 100644 .github/workflows/risk-gate.yml create mode 100644 tests/test_litellm/test_github_risk_tier.py diff --git a/.github/risk-tiers.yml b/.github/risk-tiers.yml new file mode 100644 index 00000000000..afec788e0aa --- /dev/null +++ b/.github/risk-tiers.yml @@ -0,0 +1,62 @@ +paths: + high: + - "litellm/proxy/auth/**" + - "litellm/proxy/management_endpoints/**" + - "litellm/proxy/spend_tracking/**" + - "litellm/proxy/db/**" + - "**/migrations/**" + - "**/schema.prisma" + - "litellm/caching/**" + - "litellm/router.py" + - "litellm/router_utils/**" + - "litellm/proxy/proxy_server.py" + - "litellm/__init__.py" + - "litellm/utils.py" + - "litellm/proxy/guardrails/**" + - "litellm/proxy/hooks/**" + - "litellm/proxy/custom_hooks/**" + - "enterprise/enterprise_hooks/**" + - "enterprise/litellm_enterprise/proxy/hooks/**" + - "litellm/secret_managers/**" + - "litellm/proxy/pass_through_endpoints/**" + - "litellm/passthrough/**" + - ".github/**" + - ".circleci/**" + - "pyproject.toml" + - "uv.lock" + - "**/package*.json" + - "**/Dockerfile*" + - "scripts/*gate*" + low: + - "cookbook/**" + - "model_prices_and_context_window.json" + - "litellm/model_prices_and_context_window_backup.json" + - "**/*.md" + - "**/*.mdx" + +modules: + medium_from: 2 + high_from: 4 + +size: + low: + lines_under: 100 + files_up_to: 3 + medium: + lines_under: 400 + files_up_to: 10 + ignore: + - "uv.lock" + - "**/package-lock.json" + - "ui/litellm-dashboard/src/lib/http/schema.d.ts" + +tests: + files: + - "tests/**" + - "**/*.test.*" + - "**/*.spec.*" + - "**/__tests__/**" + +authors: + low: + - "devin-ai-integration[bot]" diff --git a/.github/scripts/risk_tier.py b/.github/scripts/risk_tier.py new file mode 100644 index 00000000000..6857d3ff6ca --- /dev/null +++ b/.github/scripts/risk_tier.py @@ -0,0 +1,342 @@ +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +from collections.abc import Sequence +from dataclasses import dataclass +from pathlib import Path +from typing import Final, Literal + +import yaml +from pydantic import BaseModel, ConfigDict, Field + +Tier = Literal["low", "medium", "high"] + +TEST_DEF_RE: Final = re.compile(r"^\s*(?:async\s+)?def\s+test_|^\s*(?:it|test)\(") +SKIP_RE: Final = re.compile(r"pytest\.(?:mark\.)?(?:skip|xfail)|unittest\.skip|\b(?:it|test|describe)\.skip\(|\bxit\(") +ASSERT_RE: Final = re.compile(r"^\s*assert\b|\bexpect\(") +GLOB_TOKEN_RE: Final = re.compile(r"(\*\*/|\*\*|\*|\?)") +DIFF_BLOCK_SEPARATOR: Final = "\ndiff --git a/" +MAX_LISTED_PATHS: Final = 5 + + +class SizeLimit(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + lines_under: int = Field(gt=0) + files_up_to: int = Field(gt=0) + + +class SizeRules(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + low: SizeLimit + medium: SizeLimit + ignore: tuple[str, ...] = () + + +class ModuleRules(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + medium_from: int = Field(gt=1) + high_from: int = Field(gt=1) + + +class PathRules(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + high: tuple[str, ...] + low: tuple[str, ...] + + +class TestRules(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + files: tuple[str, ...] + + +class AuthorRules(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + low: tuple[str, ...] + + +class RiskConfig(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + paths: PathRules + modules: ModuleRules + size: SizeRules + tests: TestRules + authors: AuthorRules + + +@dataclass(frozen=True, slots=True) +class PathMatcher: + patterns: tuple[re.Pattern[str], ...] + + @staticmethod + def from_globs(globs: Sequence[str]) -> PathMatcher: + return PathMatcher(tuple(glob_to_regex(glob) for glob in globs)) + + def matches(self, path: str) -> bool: + return any(pattern.fullmatch(path) for pattern in self.patterns) + + +@dataclass(frozen=True, slots=True) +class Rules: + config: RiskConfig + high_paths: PathMatcher + low_paths: PathMatcher + test_files: PathMatcher + size_ignored: PathMatcher + + @staticmethod + def from_config(config: RiskConfig) -> Rules: + return Rules( + config=config, + high_paths=PathMatcher.from_globs(config.paths.high), + low_paths=PathMatcher.from_globs(config.paths.low), + test_files=PathMatcher.from_globs(config.tests.files), + size_ignored=PathMatcher.from_globs(config.size.ignore), + ) + + def path_tier(self, path: str) -> Tier: + if self.high_paths.matches(path): + return "high" + if self.test_files.matches(path) or self.low_paths.matches(path): + return "low" + return "medium" + + def is_production(self, path: str) -> bool: + return self.path_tier(path) != "low" + + +@dataclass(frozen=True, slots=True) +class FileChange: + path: str + added_lines: tuple[str, ...] + deleted_lines: tuple[str, ...] + + @property + def line_count(self) -> int: + return len(self.added_lines) + len(self.deleted_lines) + + +@dataclass(frozen=True, slots=True) +class Factor: + name: str + tier: Tier + reason: str + + +@dataclass(frozen=True, slots=True) +class Verdict: + tier: Tier + factors: tuple[Factor, ...] + + def summary_markdown(self) -> str: + rows = "\n".join(f"| {factor.name} | {factor.tier} | {factor.reason} |" for factor in self.factors) + return f"risk: {self.tier} (shadow mode, nothing is blocked)\n\n| factor | tier | why |\n| --- | --- | --- |\n{rows}\n" + + def to_json(self) -> str: + payload = { + "tier": self.tier, + "factors": [{"name": f.name, "tier": f.tier, "reason": f.reason} for f in self.factors], + "summary": self.summary_markdown(), + } + return json.dumps(payload, indent=2) + + +def glob_to_regex(pattern: str) -> re.Pattern[str]: + return re.compile("".join(_translate_glob_token(token) for token in GLOB_TOKEN_RE.split(pattern))) + + +def _translate_glob_token(token: str) -> str: + match token: + case "**/": + return "(?:.*/)?" + case "**": + return ".*" + case "*": + return "[^/]*" + case "?": + return "[^/]" + case _: + return re.escape(token) + + +def load_config(path: Path) -> RiskConfig: + return RiskConfig.model_validate(yaml.safe_load(path.read_text(encoding="utf-8"))) + + +def parse_diff(diff_text: str) -> tuple[FileChange, ...]: + blocks = ("\n" + diff_text).split(DIFF_BLOCK_SEPARATOR)[1:] + return tuple(_parse_block(block) for block in blocks) + + +def _parse_block(block: str) -> FileChange: + header, _, body = block.partition("\n") + path = header[: (len(header) - 3) // 2] + lines = body.split("\n") + first_hunk = next((index for index, line in enumerate(lines) if line.startswith("@@")), len(lines)) + hunk_lines = lines[first_hunk:] + return FileChange( + path=path, + added_lines=tuple(line[1:] for line in hunk_lines if line.startswith("+")), + deleted_lines=tuple(line[1:] for line in hunk_lines if line.startswith("-")), + ) + + +def module_key(path: str) -> str: + parts = path.split("/") + if parts[0] != "litellm": + return parts[0] + depth = 3 if len(parts) > 3 else 2 + return "/".join(parts[:depth]) + + +def highest(tiers: Sequence[Tier]) -> Tier: + if "high" in tiers: + return "high" + if "medium" in tiers: + return "medium" + return "low" + + +def _listed(paths: Sequence[str]) -> str: + shown = ", ".join(f"`{path}`" for path in paths[:MAX_LISTED_PATHS]) + rest = len(paths) - MAX_LISTED_PATHS + return f"{shown} and {rest} more" if rest > 0 else shown + + +def paths_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: + tier = highest([rules.path_tier(change.path) for change in changes]) + matching = [change.path for change in changes if rules.path_tier(change.path) == tier] + match tier: + case "high": + return Factor("paths", "high", f"always-human: {_listed(matching)}") + case "medium": + return Factor("paths", "medium", f"{len(matching)} file(s) outside the docs, tests, and model map tiers") + case "low": + return Factor("paths", "low", "docs, tests, cookbook, or model map only") + + +def modules_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: + modules = sorted({module_key(change.path) for change in changes if rules.is_production(change.path)}) + count = len(modules) + tier: Tier = ( + "high" + if count >= rules.config.modules.high_from + else "medium" + if count >= rules.config.modules.medium_from + else "low" + ) + return Factor("modules", tier, f"{count} module(s): {_listed(modules)}" if modules else "no production module") + + +def size_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: + counted = [change for change in changes if not rules.size_ignored.matches(change.path)] + lines = sum(change.line_count for change in counted) + files = len(counted) + limits = rules.config.size + tier: Tier = ( + "low" + if lines < limits.low.lines_under and files <= limits.low.files_up_to + else "medium" + if lines < limits.medium.lines_under and files <= limits.medium.files_up_to + else "high" + ) + return Factor("size", tier, f"{lines} line(s) across {files} file(s)") + + +def _net(changes: Sequence[FileChange], pattern: re.Pattern[str]) -> int: + added = sum(1 for change in changes for line in change.added_lines if pattern.search(line)) + deleted = sum(1 for change in changes for line in change.deleted_lines if pattern.search(line)) + return added - deleted + + +def tests_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: + test_changes = [change for change in changes if rules.test_files.matches(change.path)] + net_tests = _net(test_changes, TEST_DEF_RE) + net_skips = _net(test_changes, SKIP_RE) + net_asserts = _net(test_changes, ASSERT_RE) + production_changed = any(rules.is_production(change.path) for change in changes) + if net_tests < 0: + return Factor("tests", "high", f"{-net_tests} test(s) removed") + if net_skips > 0: + return Factor("tests", "high", f"{net_skips} skip marker(s) added") + if net_asserts < 0: + return Factor("tests", "high", f"{-net_asserts} assertion(s) removed") + if not production_changed: + return Factor("tests", "low", "no production code changed") + if net_tests > 0: + return Factor("tests", "low", f"{net_tests} test(s) added") + if test_changes: + return Factor("tests", "medium", "tests edited, none added") + return Factor("tests", "medium", "production code changed with no test touched") + + +def author_factor(author: str, from_fork: bool, rules: Rules) -> Factor: + if from_fork: + return Factor("author", "high", f"`{author}` from a fork") + if author in rules.config.authors.low: + return Factor("author", "low", f"`{author}` on an internal branch") + return Factor("author", "medium", f"`{author}` on an internal branch, human-opened") + + +def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules: Rules) -> Verdict: + factors = ( + paths_factor(changes, rules), + modules_factor(changes, rules), + size_factor(changes, rules), + tests_factor(changes, rules), + author_factor(author, from_fork, rules), + ) + return Verdict(highest([factor.tier for factor in factors]), factors) + + +def git_diff(repo: Path, base: str, head: str) -> str: + completed = subprocess.run( + ["git", "-c", "core.quotePath=false", "diff", "--no-renames", "--no-ext-diff", "-U0", base, head], + cwd=repo, + capture_output=True, + encoding="utf-8", + errors="replace", + check=True, + ) + return completed.stdout + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser(description="Compute a pull request's floor risk tier from its diff") + parser.add_argument("--config", type=Path, required=True) + parser.add_argument("--base", required=True) + parser.add_argument("--head", required=True) + parser.add_argument("--author", required=True) + parser.add_argument("--from-fork", action="store_true") + parser.add_argument("--repo", type=Path, default=Path.cwd()) + parser.add_argument("--json-out", type=Path) + return parser + + +class CliArgs(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + config: Path + base: str + head: str + author: str + from_fork: bool + repo: Path + json_out: Path | None + + +def main(argv: Sequence[str] | None = None) -> int: + args = CliArgs.model_validate(vars(build_parser().parse_args(argv))) + rules = Rules.from_config(load_config(args.config)) + changes = parse_diff(git_diff(args.repo, args.base, args.head)) + verdict = classify(changes, args.author, args.from_fork, rules) + if args.json_out is not None: + args.json_out.write_text(verdict.to_json(), encoding="utf-8") + sys.stdout.write(verdict.summary_markdown()) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/risk-gate.yml b/.github/workflows/risk-gate.yml new file mode 100644 index 00000000000..ffa246ddbca --- /dev/null +++ b/.github/workflows/risk-gate.yml @@ -0,0 +1,83 @@ +name: risk-gate + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + branches: + - litellm_internal_staging + +permissions: {} + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + floor: + name: risk-gate floor (shadow) + if: github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + pull-requests: write + checks: write + steps: + - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + with: + fetch-depth: 2 + persist-credentials: false + + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + with: + python-version: "3.12" + + - name: Compute the floor tier with the base branch's copy of the gate + shell: bash + env: + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + run: | + python -m pip install "pyyaml==6.0.3" "pydantic==2.13.4" + for gate_file in scripts/risk_tier.py risk-tiers.yml; do + target="${RUNNER_TEMP}/$(basename "${gate_file}")" + git show "HEAD^1:.github/${gate_file}" > "${target}" 2>/dev/null || cp ".github/${gate_file}" "${target}" + done + python "${RUNNER_TEMP}/risk_tier.py" \ + --config "${RUNNER_TEMP}/risk-tiers.yml" \ + --base HEAD^1 \ + --head HEAD \ + --author "${PR_AUTHOR}" \ + --json-out "${RUNNER_TEMP}/risk.json" \ + | tee -a "${GITHUB_STEP_SUMMARY}" + + - name: Publish the risk-gate check run and the risk label + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + env: + RISK_JSON: ${{ runner.temp }}/risk.json + with: + script: | + const fs = require('fs'); + const risk = JSON.parse(fs.readFileSync(process.env.RISK_JSON, 'utf8')); + const pr = context.payload.pull_request; + const repo = { owner: context.repo.owner, repo: context.repo.repo }; + await github.rest.checks.create({ + ...repo, + name: 'risk-gate', + head_sha: pr.head.sha, + status: 'completed', + conclusion: 'neutral', + output: { title: `risk: ${risk.tier} (shadow)`, summary: risk.summary }, + }); + const wanted = `risk:${risk.tier}`; + const { data: labels } = await github.rest.issues.listLabelsOnIssue({ + ...repo, + issue_number: pr.number, + per_page: 100, + }); + const current = labels.map((label) => label.name).filter((name) => name.startsWith('risk:')); + for (const name of current.filter((name) => name !== wanted)) { + await github.rest.issues.removeLabel({ ...repo, issue_number: pr.number, name }); + } + if (!current.includes(wanted)) { + await github.rest.issues.addLabels({ ...repo, issue_number: pr.number, labels: [wanted] }); + } diff --git a/tests/test_litellm/test_github_risk_tier.py b/tests/test_litellm/test_github_risk_tier.py new file mode 100644 index 00000000000..803bf83518a --- /dev/null +++ b/tests/test_litellm/test_github_risk_tier.py @@ -0,0 +1,351 @@ +from __future__ import annotations + +import importlib.util +import json +import subprocess +import sys +from collections.abc import Sequence +from pathlib import Path + +import pytest +from pydantic import ValidationError + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT_PATH = REPO_ROOT / ".github" / "scripts" / "risk_tier.py" +CONFIG_PATH = REPO_ROOT / ".github" / "risk-tiers.yml" +DEVIN = "devin-ai-integration[bot]" + + +@pytest.fixture(scope="module") +def risk_tier(): + spec = importlib.util.spec_from_file_location("risk_tier", SCRIPT_PATH) + assert spec and spec.loader, f"Could not load spec for {SCRIPT_PATH}" + module = importlib.util.module_from_spec(spec) + sys.modules["risk_tier"] = module + spec.loader.exec_module(module) + return module + + +@pytest.fixture(scope="module") +def rules(risk_tier): + return risk_tier.Rules.from_config(risk_tier.load_config(CONFIG_PATH)) + + +def _file_diff(path: str, added: Sequence[str] = (), deleted: Sequence[str] = ()) -> str: + header = ( + f"diff --git a/{path} b/{path}\n" + f"index 0000000..1111111 100644\n" + f"--- a/{path}\n" + f"+++ b/{path}\n" + f"@@ -1,{len(deleted)} +1,{len(added)} @@\n" + ) + body = "".join(f"-{line}\n" for line in deleted) + "".join(f"+{line}\n" for line in added) + return header + body + + +def _lines(count: int, prefix: str = "x = ") -> tuple[str, ...]: + return tuple(f"{prefix}{index}" for index in range(count)) + + +NEW_TEST = ("def test_regression():", " assert True") + + +def _factor(verdict, name: str): + return next(factor for factor in verdict.factors if factor.name == name) + + +def _verdict(risk_tier, rules, diff: str, author: str = DEVIN, from_fork: bool = False): + return risk_tier.classify(risk_tier.parse_diff(diff), author, from_fork, rules) + + +def test_always_human_path_is_high_even_when_tiny_and_tested(risk_tier, rules): + diff = _file_diff("litellm/proxy/auth/user_api_key_auth.py", added=("x = 1",)) + _file_diff( + "tests/test_litellm/proxy/auth/test_user_api_key_auth.py", added=NEW_TEST + ) + verdict = _verdict(risk_tier, rules, diff) + assert verdict.tier == "high" + assert _factor(verdict, "paths").tier == "high" + assert "litellm/proxy/auth/user_api_key_auth.py" in _factor(verdict, "paths").reason + assert _factor(verdict, "size").tier == "low" + assert _factor(verdict, "tests").tier == "low" + + +def test_docs_and_tests_only_change_is_low_on_every_factor(risk_tier, rules): + diff = _file_diff("README.md", added=("hello",)) + _file_diff("tests/test_litellm/test_docs.py", added=NEW_TEST) + verdict = _verdict(risk_tier, rules, diff) + assert verdict.tier == "low" + assert {factor.tier for factor in verdict.factors} == {"low"} + + +def test_provider_change_with_regression_test_is_medium_by_path_only(risk_tier, rules): + diff = _file_diff("litellm/llms/anthropic/chat/transformation.py", added=_lines(20)) + _file_diff( + "tests/test_litellm/llms/anthropic/test_chat_transformation.py", added=NEW_TEST + ) + verdict = _verdict(risk_tier, rules, diff) + assert verdict.tier == "medium" + assert {factor.name: factor.tier for factor in verdict.factors} == { + "paths": "medium", + "modules": "low", + "size": "low", + "tests": "low", + "author": "low", + } + + +@pytest.mark.parametrize( + ("paths", "expected"), + [ + (("litellm/llms/anthropic/chat/x.py",), "low"), + (("litellm/llms/anthropic/chat/x.py", "litellm/types/llms/anthropic.py"), "medium"), + (("litellm/llms/anthropic/chat/x.py", "litellm/llms/openai/chat/x.py", "litellm/types/llms/a.py"), "medium"), + ( + ( + "litellm/llms/anthropic/chat/x.py", + "litellm/llms/openai/chat/x.py", + "litellm/types/llms/a.py", + "ui/litellm-dashboard/src/a.tsx", + ), + "high", + ), + ], +) +def test_modules_factor_counts_distinct_production_modules(risk_tier, rules, paths, expected): + diff = "".join(_file_diff(path, added=("x = 1",)) for path in paths) + diff_with_tests = diff + _file_diff("tests/test_litellm/test_a.py", added=NEW_TEST) + assert _factor(_verdict(risk_tier, rules, diff_with_tests), "modules").tier == expected + + +def test_same_module_touched_twice_counts_once(risk_tier, rules): + diff = _file_diff("litellm/llms/anthropic/chat/a.py", added=("x = 1",)) + _file_diff( + "litellm/llms/anthropic/common_utils.py", added=("x = 1",) + ) + factor = _factor(_verdict(risk_tier, rules, diff), "modules") + assert factor.tier == "low" + assert factor.reason == "1 module(s): `litellm/llms/anthropic`" + + +@pytest.mark.parametrize( + ("line_counts", "expected"), + [ + ((33, 33, 33), "low"), + ((34, 33, 33), "medium"), + ((1, 1, 1, 1), "medium"), + ((100, 100, 100, 99), "medium"), + ((100, 100, 100, 100), "high"), + ((1,) * 11, "high"), + ], +) +def test_size_factor_thresholds(risk_tier, rules, line_counts, expected): + diff = "".join( + _file_diff(f"litellm/llms/provider{index}/chat/x.py", added=_lines(count)) + for index, count in enumerate(line_counts) + ) + assert _factor(_verdict(risk_tier, rules, diff), "size").tier == expected + + +def test_generated_files_do_not_count_toward_size(risk_tier, rules): + diff = _file_diff("ui/litellm-dashboard/src/lib/http/schema.d.ts", added=_lines(5000)) + _file_diff( + "litellm/llms/anthropic/chat/x.py", added=("x = 1",) + ) + factor = _factor(_verdict(risk_tier, rules, diff), "size") + assert factor.tier == "low" + assert factor.reason == "1 line(s) across 1 file(s)" + + +def test_removed_test_function_is_high(risk_tier, rules): + diff = _file_diff("tests/test_litellm/test_a.py", deleted=("def test_gone():", " assert 1 == 1")) + verdict = _verdict(risk_tier, rules, diff) + assert verdict.tier == "high" + assert _factor(verdict, "tests").reason == "1 test(s) removed" + + +def test_added_skip_marker_is_high(risk_tier, rules): + diff = _file_diff("tests/test_litellm/test_a.py", added=('@pytest.mark.skip(reason="flaky")',)) + assert _factor(_verdict(risk_tier, rules, diff), "tests").tier == "high" + + +def test_weakened_assertions_are_high(risk_tier, rules): + diff = _file_diff( + "tests/test_litellm/test_a.py", + added=(" assert result",), + deleted=(" assert result.status == 200", " assert result.body == expected"), + ) + factor = _factor(_verdict(risk_tier, rules, diff), "tests") + assert factor.tier == "high" + assert factor.reason == "1 assertion(s) removed" + + +def test_renamed_test_file_is_not_a_deletion(risk_tier, rules): + body = ("def test_kept():", " assert kept()") + diff = ( + _file_diff("tests/test_litellm/test_old.py", deleted=body) + + _file_diff("tests/test_litellm/test_new.py", added=body) + + _file_diff("litellm/llms/anthropic/chat/x.py", added=("x = 1",)) + ) + factor = _factor(_verdict(risk_tier, rules, diff), "tests") + assert factor.tier == "medium" + assert factor.reason == "tests edited, none added" + + +def test_production_change_without_any_test_is_medium(risk_tier, rules): + diff = _file_diff("litellm/llms/anthropic/chat/x.py", added=("x = 1",)) + factor = _factor(_verdict(risk_tier, rules, diff), "tests") + assert factor.tier == "medium" + assert factor.reason == "production code changed with no test touched" + + +@pytest.mark.parametrize( + ("added", "expected"), + [ + (('it("hides the notice", () => {', " expect(screen.queryByText(notice)).toBeNull();", "});"), "low"), + (('it.skip("hides the notice", () => {', "});"), "high"), + ], +) +def test_typescript_tests_count_like_python_ones(risk_tier, rules, added, expected): + diff = _file_diff("ui/litellm-dashboard/src/app/login/LoginPage.test.tsx", added=added) + _file_diff( + "ui/litellm-dashboard/src/app/login/LoginPage.tsx", added=("const x = 1;",) + ) + assert _factor(_verdict(risk_tier, rules, diff), "tests").tier == expected + + +@pytest.mark.parametrize( + ("author", "from_fork", "expected"), + [ + (DEVIN, False, "low"), + ("mateo-berri", False, "medium"), + (DEVIN, True, "high"), + ("jairandresdiazp", True, "high"), + ], +) +def test_author_factor(risk_tier, rules, author, from_fork, expected): + diff = _file_diff("README.md", added=("hello",)) + verdict = _verdict(risk_tier, rules, diff, author=author, from_fork=from_fork) + assert _factor(verdict, "author").tier == expected + assert verdict.tier == expected + + +@pytest.mark.parametrize( + ("path", "expected"), + [ + ("litellm/proxy/auth/user_api_key_auth.py", "high"), + ("litellm/proxy/schema.prisma", "high"), + ("litellm-proxy-extras/litellm_proxy_extras/migrations/20260901_x/migration.sql", "high"), + ("docker/Dockerfile.database", "high"), + ("ui/litellm-dashboard/package.json", "high"), + ("scripts/type_check_gate.py", "high"), + (".github/risk-tiers.yml", "high"), + ("enterprise/litellm_enterprise/proxy/hooks/x.py", "high"), + ("litellm/proxy/pass_through_endpoints/x.py", "high"), + ("litellm/llms/bedrock/passthrough/x.py", "medium"), + ("ui/litellm-dashboard/src/components/networking.tsx", "medium"), + ("litellm/types/proxy/x.py", "medium"), + ("ui/litellm-dashboard/src/app/login/LoginPage.test.tsx", "low"), + ("tests/e2e/x.py", "low"), + ("cookbook/x.ipynb", "low"), + ("model_prices_and_context_window.json", "low"), + ("litellm/model_prices_and_context_window_backup.json", "low"), + ("litellm/proxy/README.md", "low"), + ], +) +def test_path_tier_from_the_checked_in_config(rules, path, expected): + assert rules.path_tier(path) == expected + + +def test_single_star_does_not_cross_directories(risk_tier): + assert risk_tier.glob_to_regex("Dockerfile*").fullmatch("Dockerfile.database") + assert risk_tier.glob_to_regex("Dockerfile*").fullmatch("docker/Dockerfile.database") is None + assert risk_tier.glob_to_regex("scripts/*gate*").fullmatch("scripts/nested/type_check_gate.py") is None + assert risk_tier.glob_to_regex("**/migrations/**").fullmatch("migrations/x.sql") + + +def test_parse_diff_handles_binary_deleted_and_dash_prefixed_lines(risk_tier): + diff = ( + "diff --git a/img.png b/img.png\n" + "index 0000000..1111111 100644\n" + "Binary files a/img.png and b/img.png differ\n" + "diff --git a/gone.sql b/gone.sql\n" + "deleted file mode 100644\n" + "index 1111111..0000000\n" + "--- a/gone.sql\n" + "+++ /dev/null\n" + "@@ -1,2 +0,0 @@\n" + "--- a sql comment\n" + "-SELECT 1;\n" + ) + changes = risk_tier.parse_diff(diff) + assert [change.path for change in changes] == ["img.png", "gone.sql"] + assert changes[0].line_count == 0 + assert changes[1].deleted_lines == ("-- a sql comment", "SELECT 1;") + assert changes[1].added_lines == () + + +def test_parse_diff_of_an_empty_diff_is_empty(risk_tier): + assert risk_tier.parse_diff("") == () + + +def test_config_rejects_unknown_keys(risk_tier, tmp_path): + bad = tmp_path / "risk-tiers.yml" + bad.write_text(CONFIG_PATH.read_text() + "\nprompt: judge.md\n") + with pytest.raises(ValidationError, match="Extra inputs"): + risk_tier.load_config(bad) + + +def _git(repo: Path, *args: str) -> str: + return subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + env={ + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@x", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@x", + }, + ).stdout.strip() + + +def test_main_end_to_end_against_a_git_repo(risk_tier, tmp_path, capsys): + repo = tmp_path / "repo" + (repo / "litellm" / "caching").mkdir(parents=True) + _git(tmp_path, "init", "-q", "-b", "main", "repo") + (repo / "litellm" / "caching" / "caching.py").write_text("x = 1\n") + _git(repo, "add", ".") + _git(repo, "commit", "-q", "-m", "base") + base = _git(repo, "rev-parse", "HEAD") + (repo / "litellm" / "caching" / "caching.py").write_text("x = 2\n") + _git(repo, "commit", "-q", "-am", "head") + head = _git(repo, "rev-parse", "HEAD") + json_out = tmp_path / "risk.json" + + exit_code = risk_tier.main( + [ + "--config", + str(CONFIG_PATH), + "--base", + base, + "--head", + head, + "--author", + DEVIN, + "--repo", + str(repo), + "--json-out", + str(json_out), + ] + ) + + assert exit_code == 0 + payload = json.loads(json_out.read_text()) + assert payload["tier"] == "high" + assert {factor["name"]: factor["tier"] for factor in payload["factors"]} == { + "paths": "high", + "modules": "low", + "size": "low", + "tests": "medium", + "author": "low", + } + printed = capsys.readouterr().out + assert printed.startswith("risk: high (shadow mode, nothing is blocked)") + assert payload["summary"] == printed From 9d5066df6b9d9e8015772ae23d6940a89fc7601e Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 7 Sep 2026 15:53:50 -0700 Subject: [PATCH 2/7] fix(ci): decode git-quoted paths in the risk tier diff parser --- .github/scripts/risk_tier.py | 22 ++++++++++- tests/test_litellm/test_github_risk_tier.py | 42 +++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/.github/scripts/risk_tier.py b/.github/scripts/risk_tier.py index 6857d3ff6ca..fa448eec704 100644 --- a/.github/scripts/risk_tier.py +++ b/.github/scripts/risk_tier.py @@ -8,6 +8,7 @@ import sys from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path +from types import MappingProxyType from typing import Final, Literal import yaml @@ -19,7 +20,11 @@ TEST_DEF_RE: Final = re.compile(r"^\s*(?:async\s+)?def\s+test_|^\s*(?:it|test)\( SKIP_RE: Final = re.compile(r"pytest\.(?:mark\.)?(?:skip|xfail)|unittest\.skip|\b(?:it|test|describe)\.skip\(|\bxit\(") ASSERT_RE: Final = re.compile(r"^\s*assert\b|\bexpect\(") GLOB_TOKEN_RE: Final = re.compile(r"(\*\*/|\*\*|\*|\?)") -DIFF_BLOCK_SEPARATOR: Final = "\ndiff --git a/" +DIFF_BLOCK_SEPARATOR: Final = "\ndiff --git " +QUOTED_PATH_ESCAPE_RE: Final = re.compile(r'\\(?:([abfnrtv"\\])|([0-7]{3}))') +QUOTED_PATH_ESCAPES: Final = MappingProxyType( + {"a": "\a", "b": "\b", "f": "\f", "n": "\n", "r": "\r", "t": "\t", "v": "\v", '"': '"', "\\": "\\"} +) MAX_LISTED_PATHS: Final = 5 @@ -171,9 +176,22 @@ def parse_diff(diff_text: str) -> tuple[FileChange, ...]: return tuple(_parse_block(block) for block in blocks) +def _unquote_git_path(quoted: str) -> str: + def decode(match: re.Match[str]) -> str: + return QUOTED_PATH_ESCAPES[match[1]] if match[1] else chr(int(match[2], 8)) + + return QUOTED_PATH_ESCAPE_RE.sub(decode, quoted[1:-1]) + + +def _header_path(header: str) -> str: + one_side = header[: (len(header) - 1) // 2] + unquoted = _unquote_git_path(one_side) if one_side.startswith('"') else one_side + return unquoted.removeprefix("a/") + + def _parse_block(block: str) -> FileChange: header, _, body = block.partition("\n") - path = header[: (len(header) - 3) // 2] + path = _header_path(header) lines = body.split("\n") first_hunk = next((index for index, line in enumerate(lines) if line.startswith("@@")), len(lines)) hunk_lines = lines[first_hunk:] diff --git a/tests/test_litellm/test_github_risk_tier.py b/tests/test_litellm/test_github_risk_tier.py index 803bf83518a..c09432df621 100644 --- a/tests/test_litellm/test_github_risk_tier.py +++ b/tests/test_litellm/test_github_risk_tier.py @@ -283,6 +283,26 @@ def test_parse_diff_of_an_empty_diff_is_empty(risk_tier): assert risk_tier.parse_diff("") == () +def test_parse_diff_decodes_git_quoted_paths_instead_of_dropping_them(risk_tier): + diff = ( + "diff --git a/docs/plain.md b/docs/plain.md\n" + "--- a/docs/plain.md\n" + "+++ b/docs/plain.md\n" + "@@ -0,0 +1 @@\n" + "+hello\n" + 'diff --git "a/litellm/proxy/auth/we\\"ird\\ttab\\\\slash\\001.py" "b/litellm/proxy/auth/we\\"ird\\ttab\\\\slash\\001.py"\n' + "new file mode 100644\n" + '--- "a/litellm/proxy/auth/we\\"ird\\ttab\\\\slash\\001.py"\n' + '+++ "b/litellm/proxy/auth/we\\"ird\\ttab\\\\slash\\001.py"\n' + "@@ -0,0 +1,2 @@\n" + "+def f():\n" + "+ return 1\n" + ) + changes = risk_tier.parse_diff(diff) + assert [change.path for change in changes] == ["docs/plain.md", 'litellm/proxy/auth/we"ird\ttab\\slash\x01.py'] + assert [change.line_count for change in changes] == [1, 2] + + def test_config_rejects_unknown_keys(risk_tier, tmp_path): bad = tmp_path / "risk-tiers.yml" bad.write_text(CONFIG_PATH.read_text() + "\nprompt: judge.md\n") @@ -349,3 +369,25 @@ def test_main_end_to_end_against_a_git_repo(risk_tier, tmp_path, capsys): printed = capsys.readouterr().out assert printed.startswith("risk: high (shadow mode, nothing is blocked)") assert payload["summary"] == printed + + +def test_git_quoted_filename_still_reaches_the_paths_factor(risk_tier, rules, tmp_path): + repo = tmp_path / "repo" + (repo / "litellm" / "proxy" / "auth").mkdir(parents=True) + _git(tmp_path, "init", "-q", "-b", "main", "repo") + (repo / "README.md").write_text("base\n") + _git(repo, "add", ".") + _git(repo, "commit", "-q", "-m", "base") + base = _git(repo, "rev-parse", "HEAD") + quoted_name = 'we"ird\ttab\\slash.py' + (repo / "litellm" / "proxy" / "auth" / quoted_name).write_text("def f():\n return 1\n") + _git(repo, "add", ".") + _git(repo, "commit", "-q", "-m", "head") + head = _git(repo, "rev-parse", "HEAD") + + changes = risk_tier.parse_diff(risk_tier.git_diff(repo, base, head)) + + assert [change.path for change in changes] == [f"litellm/proxy/auth/{quoted_name}"] + assert changes[0].added_lines == ("def f():", " return 1") + verdict = risk_tier.classify(changes, DEVIN, False, rules) + assert _factor(verdict, "paths").tier == "high" From fe4504403559f92b00653ec8120a44bea9ece25e Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:10:10 -0700 Subject: [PATCH 3/7] fix(ci): tier the risk floor by fork only and stop counting conditional skips The author factor now only raises a PR to high when it comes from a fork, so an internal human-opened docs or test-only PR can reach the low tier the one-pager's tier 0 needs. Over the last 400 staging merges that moves the low count from 2 to 29 with no reverted merge among them Skip markers only count when they silence a test unconditionally: pytest.mark.skipif, unittest.skipIf/skipUnless, a guarded pytest.skip(), and Playwright's conditional test.skip(cond, reason) no longer rank a PR high. Test directories anywhere in the tree count as test files, and the enterprise auth and management endpoint packages join the always-human paths --- .github/risk-tiers.yml | 8 ++- .github/scripts/risk_tier.py | 18 +++---- tests/test_litellm/test_github_risk_tier.py | 60 +++++++++++++++++++-- 3 files changed, 65 insertions(+), 21 deletions(-) diff --git a/.github/risk-tiers.yml b/.github/risk-tiers.yml index afec788e0aa..51110e058c6 100644 --- a/.github/risk-tiers.yml +++ b/.github/risk-tiers.yml @@ -17,6 +17,8 @@ paths: - "litellm/proxy/custom_hooks/**" - "enterprise/enterprise_hooks/**" - "enterprise/litellm_enterprise/proxy/hooks/**" + - "enterprise/litellm_enterprise/proxy/auth/**" + - "enterprise/litellm_enterprise/proxy/management_endpoints/**" - "litellm/secret_managers/**" - "litellm/proxy/pass_through_endpoints/**" - "litellm/passthrough/**" @@ -52,11 +54,7 @@ size: tests: files: - - "tests/**" + - "**/tests/**" - "**/*.test.*" - "**/*.spec.*" - "**/__tests__/**" - -authors: - low: - - "devin-ai-integration[bot]" diff --git a/.github/scripts/risk_tier.py b/.github/scripts/risk_tier.py index fa448eec704..2996f780664 100644 --- a/.github/scripts/risk_tier.py +++ b/.github/scripts/risk_tier.py @@ -17,7 +17,9 @@ from pydantic import BaseModel, ConfigDict, Field Tier = Literal["low", "medium", "high"] TEST_DEF_RE: Final = re.compile(r"^\s*(?:async\s+)?def\s+test_|^\s*(?:it|test)\(") -SKIP_RE: Final = re.compile(r"pytest\.(?:mark\.)?(?:skip|xfail)|unittest\.skip|\b(?:it|test|describe)\.skip\(|\bxit\(") +SKIP_RE: Final = re.compile( + r"pytest\.mark\.(?:skip(?!if)|xfail)|unittest\.skip\b|\b(?:it|test|describe)\.skip\(\s*[\"'`]|\bx(?:it|test|describe)\(" +) ASSERT_RE: Final = re.compile(r"^\s*assert\b|\bexpect\(") GLOB_TOKEN_RE: Final = re.compile(r"(\*\*/|\*\*|\*|\?)") DIFF_BLOCK_SEPARATOR: Final = "\ndiff --git " @@ -58,18 +60,12 @@ class TestRules(BaseModel): files: tuple[str, ...] -class AuthorRules(BaseModel): - model_config = ConfigDict(extra="forbid", frozen=True) - low: tuple[str, ...] - - class RiskConfig(BaseModel): model_config = ConfigDict(extra="forbid", frozen=True) paths: PathRules modules: ModuleRules size: SizeRules tests: TestRules - authors: AuthorRules @dataclass(frozen=True, slots=True) @@ -291,12 +287,10 @@ def tests_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: return Factor("tests", "medium", "production code changed with no test touched") -def author_factor(author: str, from_fork: bool, rules: Rules) -> Factor: +def author_factor(author: str, from_fork: bool) -> Factor: if from_fork: return Factor("author", "high", f"`{author}` from a fork") - if author in rules.config.authors.low: - return Factor("author", "low", f"`{author}` on an internal branch") - return Factor("author", "medium", f"`{author}` on an internal branch, human-opened") + return Factor("author", "low", f"`{author}` on an internal branch") def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules: Rules) -> Verdict: @@ -305,7 +299,7 @@ def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules: modules_factor(changes, rules), size_factor(changes, rules), tests_factor(changes, rules), - author_factor(author, from_fork, rules), + author_factor(author, from_fork), ) return Verdict(highest([factor.tier for factor in factors]), factors) diff --git a/tests/test_litellm/test_github_risk_tier.py b/tests/test_litellm/test_github_risk_tier.py index c09432df621..722deb0603b 100644 --- a/tests/test_litellm/test_github_risk_tier.py +++ b/tests/test_litellm/test_github_risk_tier.py @@ -159,9 +159,40 @@ def test_removed_test_function_is_high(risk_tier, rules): assert _factor(verdict, "tests").reason == "1 test(s) removed" -def test_added_skip_marker_is_high(risk_tier, rules): - diff = _file_diff("tests/test_litellm/test_a.py", added=('@pytest.mark.skip(reason="flaky")',)) - assert _factor(_verdict(risk_tier, rules, diff), "tests").tier == "high" +@pytest.mark.parametrize( + "marker", + [ + '@pytest.mark.skip(reason="flaky")', + "@pytest.mark.skip", + 'pytestmark = pytest.mark.skip("whole module is flaky")', + '@pytest.mark.xfail(reason="broken since the refactor")', + '@unittest.skip("flaky")', + ], +) +def test_unconditional_skip_marker_is_high(risk_tier, rules, marker): + diff = _file_diff("tests/test_litellm/test_a.py", added=(marker, *NEW_TEST)) + factor = _factor(_verdict(risk_tier, rules, diff), "tests") + assert factor.tier == "high" + assert factor.reason == "1 skip marker(s) added" + + +@pytest.mark.parametrize( + "guard", + [ + '@pytest.mark.skipif(not os.getenv("OPENAI_API_KEY"), reason="needs a real key")', + ' if not os.getenv("OPENAI_API_KEY"):', + ' pytest.skip("needs a real key")', + '@unittest.skipIf(sys.platform == "win32", "posix only")', + '@unittest.skipUnless(HAS_REDIS, "needs redis")', + ], +) +def test_conditional_skip_is_not_a_silenced_test(risk_tier, rules, guard): + diff = _file_diff("tests/test_litellm/test_a.py", added=(guard, *NEW_TEST)) + _file_diff( + "litellm/llms/anthropic/chat/x.py", added=("x = 1",) + ) + factor = _factor(_verdict(risk_tier, rules, diff), "tests") + assert factor.tier == "low" + assert factor.reason == "1 test(s) added" def test_weakened_assertions_are_high(risk_tier, rules): @@ -199,6 +230,20 @@ def test_production_change_without_any_test_is_medium(risk_tier, rules): [ (('it("hides the notice", () => {', " expect(screen.queryByText(notice)).toBeNull();", "});"), "low"), (('it.skip("hides the notice", () => {', "});"), "high"), + (("test.skip('hides the notice', async () => {", "});"), "high"), + (('xit("hides the notice", () => {', "});"), "high"), + ( + ( + 'test("hides the notice", async ({ page }) => {', + " test.skip(!process.env.UI_BASE_URL, 'needs a UI');", + "});", + ), + "low", + ), + ( + ('test("hides the notice", async ({ page }) => {', " if (!process.env.UI_BASE_URL) test.skip();", "});"), + "low", + ), ], ) def test_typescript_tests_count_like_python_ones(risk_tier, rules, added, expected): @@ -212,7 +257,7 @@ def test_typescript_tests_count_like_python_ones(risk_tier, rules, added, expect ("author", "from_fork", "expected"), [ (DEVIN, False, "low"), - ("mateo-berri", False, "medium"), + ("mateo-berri", False, "low"), (DEVIN, True, "high"), ("jairandresdiazp", True, "high"), ], @@ -235,12 +280,19 @@ def test_author_factor(risk_tier, rules, author, from_fork, expected): ("scripts/type_check_gate.py", "high"), (".github/risk-tiers.yml", "high"), ("enterprise/litellm_enterprise/proxy/hooks/x.py", "high"), + ("enterprise/litellm_enterprise/proxy/auth/x.py", "high"), + ("enterprise/litellm_enterprise/proxy/management_endpoints/x.py", "high"), ("litellm/proxy/pass_through_endpoints/x.py", "high"), ("litellm/llms/bedrock/passthrough/x.py", "medium"), ("ui/litellm-dashboard/src/components/networking.tsx", "medium"), ("litellm/types/proxy/x.py", "medium"), ("ui/litellm-dashboard/src/app/login/LoginPage.test.tsx", "low"), ("tests/e2e/x.py", "low"), + ("litellm-proxy-extras/tests/test_x.py", "low"), + ("helm/litellm-helm/tests/x.yaml", "low"), + ("ui/litellm-dashboard/tests/x.spec.ts", "low"), + ("litellm-rust/crates/core/tests/x.rs", "low"), + ("enterprise/litellm_enterprise/proxy/common_utils/x.py", "medium"), ("cookbook/x.ipynb", "low"), ("model_prices_and_context_window.json", "low"), ("litellm/model_prices_and_context_window_backup.json", "low"), From 75b447824214827c77c6554e3759907533cddbcc Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 7 Sep 2026 17:00:03 -0700 Subject: [PATCH 4/7] fix(ci): run the risk gate from the base branch and tier moves, docs, and manifests correctly The workflow now runs on pull_request_target so the base branch's copy of the script and config scores every PR, forks included, and a PR can no longer edit the gate that scores it. The PR head is only ever read as a diff. Forks are passed --from-fork so the author factor is live The diff parser reads git renames, so a pure move counts zero lines and both paths take part in the paths and modules factors. The size factor counts only files outside the docs, tests, and model map tiers, and .only( joins the silenced-test markers Config: every pyproject.toml is always-human, the MCP auth, credential, OAuth, and discovery files are always-human, and the lint budget files are low. The last 400 staging merges now score 217 high, 139 medium, 44 low --- .github/risk-tiers.yml | 9 +- .github/scripts/risk_tier.py | 160 ++++++++++++-------- .github/workflows/risk-gate.yml | 53 ++++--- tests/test_litellm/test_github_risk_tier.py | 112 +++++++++++++- 4 files changed, 253 insertions(+), 81 deletions(-) diff --git a/.github/risk-tiers.yml b/.github/risk-tiers.yml index 51110e058c6..ce7428e7b6b 100644 --- a/.github/risk-tiers.yml +++ b/.github/risk-tiers.yml @@ -22,9 +22,15 @@ paths: - "litellm/secret_managers/**" - "litellm/proxy/pass_through_endpoints/**" - "litellm/passthrough/**" + - "litellm/proxy/_experimental/mcp_server/auth/**" + - "litellm/proxy/_experimental/mcp_server/outbound_credentials/**" + - "litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py" + - "litellm/proxy/_experimental/mcp_server/*oauth*" + - "litellm/proxy/_experimental/mcp_server/*_flow.py" + - "litellm/proxy/_experimental/mcp_server/proxy_api_credentials.py" - ".github/**" - ".circleci/**" - - "pyproject.toml" + - "**/pyproject.toml" - "uv.lock" - "**/package*.json" - "**/Dockerfile*" @@ -35,6 +41,7 @@ paths: - "litellm/model_prices_and_context_window_backup.json" - "**/*.md" - "**/*.mdx" + - "*-budget.json" modules: medium_from: 2 diff --git a/.github/scripts/risk_tier.py b/.github/scripts/risk_tier.py index 2996f780664..b210172ebed 100644 --- a/.github/scripts/risk_tier.py +++ b/.github/scripts/risk_tier.py @@ -9,20 +9,24 @@ from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path from types import MappingProxyType -from typing import Final, Literal +from typing import Final, Literal, TypeAlias import yaml from pydantic import BaseModel, ConfigDict, Field +from typing_extensions import ReadOnly, TypedDict -Tier = Literal["low", "medium", "high"] +Tier: TypeAlias = Literal["low", "medium", "high"] TEST_DEF_RE: Final = re.compile(r"^\s*(?:async\s+)?def\s+test_|^\s*(?:it|test)\(") SKIP_RE: Final = re.compile( - r"pytest\.mark\.(?:skip(?!if)|xfail)|unittest\.skip\b|\b(?:it|test|describe)\.skip\(\s*[\"'`]|\bx(?:it|test|describe)\(" + r"pytest\.mark\.(?:skip(?!if)|xfail)|unittest\.skip\b" + r"|\b(?:it|test|describe)\.(?:skip\(\s*[\"'`]|only\()|\bx(?:it|test|describe)\(" ) ASSERT_RE: Final = re.compile(r"^\s*assert\b|\bexpect\(") GLOB_TOKEN_RE: Final = re.compile(r"(\*\*/|\*\*|\*|\?)") DIFF_BLOCK_SEPARATOR: Final = "\ndiff --git " +RENAME_FROM_RE: Final = re.compile(r"^rename from (.+)$") +RENAME_TO_RE: Final = re.compile(r"^rename to (.+)$") QUOTED_PATH_ESCAPE_RE: Final = re.compile(r'\\(?:([abfnrtv"\\])|([0-7]{3}))') QUOTED_PATH_ESCAPES: Final = MappingProxyType( {"a": "\a", "b": "\b", "f": "\f", "n": "\n", "r": "\r", "t": "\t", "v": "\v", '"': '"', "\\": "\\"} @@ -80,6 +84,22 @@ class PathMatcher: return any(pattern.fullmatch(path) for pattern in self.patterns) +@dataclass(frozen=True, slots=True) +class FileChange: + path: str + added_lines: tuple[str, ...] + deleted_lines: tuple[str, ...] + previous_path: str | None = None + + @property + def paths(self) -> tuple[str, ...]: + return (self.path,) if self.previous_path is None else (self.previous_path, self.path) + + @property + def line_count(self) -> int: + return len(self.added_lines) + len(self.deleted_lines) + + @dataclass(frozen=True, slots=True) class Rules: config: RiskConfig @@ -105,19 +125,11 @@ class Rules: return "low" return "medium" - def is_production(self, path: str) -> bool: - return self.path_tier(path) != "low" + def change_tier(self, change: FileChange) -> Tier: + return highest(tuple(self.path_tier(path) for path in change.paths)) - -@dataclass(frozen=True, slots=True) -class FileChange: - path: str - added_lines: tuple[str, ...] - deleted_lines: tuple[str, ...] - - @property - def line_count(self) -> int: - return len(self.added_lines) + len(self.deleted_lines) + def is_production(self, change: FileChange) -> bool: + return self.change_tier(change) != "low" @dataclass(frozen=True, slots=True) @@ -127,19 +139,31 @@ class Factor: reason: str +class FactorJson(TypedDict): + name: ReadOnly[str] + tier: ReadOnly[Tier] + reason: ReadOnly[str] + + +class VerdictJson(TypedDict): + tier: ReadOnly[Tier] + factors: ReadOnly[tuple[FactorJson, ...]] + summary: ReadOnly[str] + + @dataclass(frozen=True, slots=True) class Verdict: tier: Tier factors: tuple[Factor, ...] def summary_markdown(self) -> str: - rows = "\n".join(f"| {factor.name} | {factor.tier} | {factor.reason} |" for factor in self.factors) + rows: Final = "\n".join(f"| {factor.name} | {factor.tier} | {factor.reason} |" for factor in self.factors) return f"risk: {self.tier} (shadow mode, nothing is blocked)\n\n| factor | tier | why |\n| --- | --- | --- |\n{rows}\n" def to_json(self) -> str: - payload = { + payload: Final[VerdictJson] = { "tier": self.tier, - "factors": [{"name": f.name, "tier": f.tier, "reason": f.reason} for f in self.factors], + "factors": tuple(FactorJson(name=f.name, tier=f.tier, reason=f.reason) for f in self.factors), "summary": self.summary_markdown(), } return json.dumps(payload, indent=2) @@ -168,7 +192,7 @@ def load_config(path: Path) -> RiskConfig: def parse_diff(diff_text: str) -> tuple[FileChange, ...]: - blocks = ("\n" + diff_text).split(DIFF_BLOCK_SEPARATOR)[1:] + blocks: Final = ("\n" + diff_text).split(DIFF_BLOCK_SEPARATOR)[1:] return tuple(_parse_block(block) for block in blocks) @@ -179,30 +203,39 @@ def _unquote_git_path(quoted: str) -> str: return QUOTED_PATH_ESCAPE_RE.sub(decode, quoted[1:-1]) +def _unquote(path: str) -> str: + return _unquote_git_path(path) if path.startswith('"') else path + + def _header_path(header: str) -> str: - one_side = header[: (len(header) - 1) // 2] - unquoted = _unquote_git_path(one_side) if one_side.startswith('"') else one_side - return unquoted.removeprefix("a/") + one_side: Final = header[: (len(header) - 1) // 2] + return _unquote(one_side).removeprefix("a/") + + +def _rename_side(extended_header: Sequence[str], pattern: re.Pattern[str]) -> str | None: + return next((_unquote(match[1]) for line in extended_header if (match := pattern.match(line))), None) def _parse_block(block: str) -> FileChange: header, _, body = block.partition("\n") - path = _header_path(header) - lines = body.split("\n") - first_hunk = next((index for index, line in enumerate(lines) if line.startswith("@@")), len(lines)) - hunk_lines = lines[first_hunk:] + lines: Final = body.split("\n") + first_hunk: Final = next((index for index, line in enumerate(lines) if line.startswith("@@")), len(lines)) + extended_header: Final = lines[:first_hunk] + hunk_lines: Final = lines[first_hunk:] + renamed_to: Final = _rename_side(extended_header, RENAME_TO_RE) return FileChange( - path=path, + path=renamed_to if renamed_to is not None else _header_path(header), added_lines=tuple(line[1:] for line in hunk_lines if line.startswith("+")), deleted_lines=tuple(line[1:] for line in hunk_lines if line.startswith("-")), + previous_path=_rename_side(extended_header, RENAME_FROM_RE) if renamed_to is not None else None, ) def module_key(path: str) -> str: - parts = path.split("/") + parts: Final = path.split("/") if parts[0] != "litellm": return parts[0] - depth = 3 if len(parts) > 3 else 2 + depth: Final = 3 if len(parts) > 3 else 2 return "/".join(parts[:depth]) @@ -215,17 +248,20 @@ def highest(tiers: Sequence[Tier]) -> Tier: def _listed(paths: Sequence[str]) -> str: - shown = ", ".join(f"`{path}`" for path in paths[:MAX_LISTED_PATHS]) - rest = len(paths) - MAX_LISTED_PATHS + shown: Final = ", ".join(f"`{path}`" for path in paths[:MAX_LISTED_PATHS]) + rest: Final = len(paths) - MAX_LISTED_PATHS return f"{shown} and {rest} more" if rest > 0 else shown def paths_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: - tier = highest([rules.path_tier(change.path) for change in changes]) - matching = [change.path for change in changes if rules.path_tier(change.path) == tier] + tier: Final = highest(tuple(rules.change_tier(change) for change in changes)) + matching: Final = tuple(change for change in changes if rules.change_tier(change) == tier) match tier: case "high": - return Factor("paths", "high", f"always-human: {_listed(matching)}") + always_human: Final = tuple( + path for change in matching for path in change.paths if rules.path_tier(path) == "high" + ) + return Factor("paths", "high", f"always-human: {_listed(always_human)}") case "medium": return Factor("paths", "medium", f"{len(matching)} file(s) outside the docs, tests, and model map tiers") case "low": @@ -233,9 +269,13 @@ def paths_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: def modules_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: - modules = sorted({module_key(change.path) for change in changes if rules.is_production(change.path)}) - count = len(modules) - tier: Tier = ( + modules: Final = tuple( + sorted( + frozenset(module_key(path) for change in changes for path in change.paths if rules.path_tier(path) != "low") + ) + ) + count: Final = len(modules) + tier: Final[Tier] = ( "high" if count >= rules.config.modules.high_from else "medium" @@ -246,32 +286,34 @@ def modules_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: def size_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: - counted = [change for change in changes if not rules.size_ignored.matches(change.path)] - lines = sum(change.line_count for change in counted) - files = len(counted) - limits = rules.config.size - tier: Tier = ( + counted: Final = tuple( + change for change in changes if rules.is_production(change) and not rules.size_ignored.matches(change.path) + ) + lines: Final = sum(change.line_count for change in counted) + files: Final = len(counted) + limits: Final = rules.config.size + tier: Final[Tier] = ( "low" if lines < limits.low.lines_under and files <= limits.low.files_up_to else "medium" if lines < limits.medium.lines_under and files <= limits.medium.files_up_to else "high" ) - return Factor("size", tier, f"{lines} line(s) across {files} file(s)") + return Factor("size", tier, f"{lines} line(s) across {files} file(s) outside the docs, tests, and model map tiers") def _net(changes: Sequence[FileChange], pattern: re.Pattern[str]) -> int: - added = sum(1 for change in changes for line in change.added_lines if pattern.search(line)) - deleted = sum(1 for change in changes for line in change.deleted_lines if pattern.search(line)) + added: Final = sum(1 for change in changes for line in change.added_lines if pattern.search(line)) + deleted: Final = sum(1 for change in changes for line in change.deleted_lines if pattern.search(line)) return added - deleted def tests_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: - test_changes = [change for change in changes if rules.test_files.matches(change.path)] - net_tests = _net(test_changes, TEST_DEF_RE) - net_skips = _net(test_changes, SKIP_RE) - net_asserts = _net(test_changes, ASSERT_RE) - production_changed = any(rules.is_production(change.path) for change in changes) + test_changes: Final = tuple(change for change in changes if rules.test_files.matches(change.path)) + net_tests: Final = _net(test_changes, TEST_DEF_RE) + net_skips: Final = _net(test_changes, SKIP_RE) + net_asserts: Final = _net(test_changes, ASSERT_RE) + production_changed: Final = any(rules.is_production(change) for change in changes) if net_tests < 0: return Factor("tests", "high", f"{-net_tests} test(s) removed") if net_skips > 0: @@ -294,19 +336,19 @@ def author_factor(author: str, from_fork: bool) -> Factor: def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules: Rules) -> Verdict: - factors = ( + factors: Final = ( paths_factor(changes, rules), modules_factor(changes, rules), size_factor(changes, rules), tests_factor(changes, rules), author_factor(author, from_fork), ) - return Verdict(highest([factor.tier for factor in factors]), factors) + return Verdict(highest(tuple(factor.tier for factor in factors)), factors) def git_diff(repo: Path, base: str, head: str) -> str: - completed = subprocess.run( - ["git", "-c", "core.quotePath=false", "diff", "--no-renames", "--no-ext-diff", "-U0", base, head], + completed: Final = subprocess.run( + ("git", "-c", "core.quotePath=false", "diff", "--find-renames", "--no-ext-diff", "-U0", base, head), cwd=repo, capture_output=True, encoding="utf-8", @@ -317,7 +359,7 @@ def git_diff(repo: Path, base: str, head: str) -> str: def build_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Compute a pull request's floor risk tier from its diff") + parser: Final = argparse.ArgumentParser(description="Compute a pull request's floor risk tier from its diff") parser.add_argument("--config", type=Path, required=True) parser.add_argument("--base", required=True) parser.add_argument("--head", required=True) @@ -340,10 +382,10 @@ class CliArgs(BaseModel): def main(argv: Sequence[str] | None = None) -> int: - args = CliArgs.model_validate(vars(build_parser().parse_args(argv))) - rules = Rules.from_config(load_config(args.config)) - changes = parse_diff(git_diff(args.repo, args.base, args.head)) - verdict = classify(changes, args.author, args.from_fork, rules) + args: Final = CliArgs.model_validate(vars(build_parser().parse_args(argv))) + rules: Final = Rules.from_config(load_config(args.config)) + changes: Final = parse_diff(git_diff(args.repo, args.base, args.head)) + verdict: Final = classify(changes, args.author, args.from_fork, rules) if args.json_out is not None: args.json_out.write_text(verdict.to_json(), encoding="utf-8") sys.stdout.write(verdict.summary_markdown()) diff --git a/.github/workflows/risk-gate.yml b/.github/workflows/risk-gate.yml index ffa246ddbca..0e3645b83a7 100644 --- a/.github/workflows/risk-gate.yml +++ b/.github/workflows/risk-gate.yml @@ -1,12 +1,13 @@ name: risk-gate -on: - pull_request: +on: # zizmor: ignore[dangerous-triggers] runs the base branch's copy of the gate only; the PR's diff is read as data and never executed + pull_request_target: types: [opened, synchronize, reopened, ready_for_review] branches: - litellm_internal_staging -permissions: {} +permissions: + contents: read concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number }} @@ -15,9 +16,8 @@ concurrency: jobs: floor: name: risk-gate floor (shadow) - if: github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest - timeout-minutes: 5 + timeout-minutes: 10 permissions: contents: read pull-requests: write @@ -25,30 +25,43 @@ jobs: steps: - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 with: - fetch-depth: 2 persist-credentials: false - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + - name: Fetch the pull request head and its merge base + id: revisions + env: + GH_TOKEN: ${{ github.token }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + merge_base="$(gh api "repos/${GITHUB_REPOSITORY}/compare/${BASE_SHA}...${HEAD_SHA}" --jq '.merge_base_commit.sha')" + git fetch --no-tags --depth=1 origin "$merge_base" "$HEAD_SHA" + echo "merge_base=$merge_base" >> "$GITHUB_OUTPUT" + + - name: Set up uv + uses: ./.github/actions/setup-uv-with-retries with: - python-version: "3.12" + version: "0.10.9" - name: Compute the floor tier with the base branch's copy of the gate - shell: bash env: + MERGE_BASE: ${{ steps.revisions.outputs.merge_base }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} PR_AUTHOR: ${{ github.event.pull_request.user.login }} + FROM_FORK: ${{ github.event.pull_request.head.repo.full_name != github.repository }} run: | - python -m pip install "pyyaml==6.0.3" "pydantic==2.13.4" - for gate_file in scripts/risk_tier.py risk-tiers.yml; do - target="${RUNNER_TEMP}/$(basename "${gate_file}")" - git show "HEAD^1:.github/${gate_file}" > "${target}" 2>/dev/null || cp ".github/${gate_file}" "${target}" - done - python "${RUNNER_TEMP}/risk_tier.py" \ - --config "${RUNNER_TEMP}/risk-tiers.yml" \ - --base HEAD^1 \ - --head HEAD \ - --author "${PR_AUTHOR}" \ + fork_flag=() + if [ "$FROM_FORK" = "true" ]; then + fork_flag=(--from-fork) + fi + uv run --frozen python .github/scripts/risk_tier.py \ + --config .github/risk-tiers.yml \ + --base "$MERGE_BASE" \ + --head "$HEAD_SHA" \ + --author "$PR_AUTHOR" \ + "${fork_flag[@]}" \ --json-out "${RUNNER_TEMP}/risk.json" \ - | tee -a "${GITHUB_STEP_SUMMARY}" + | tee -a "$GITHUB_STEP_SUMMARY" - name: Publish the risk-gate check run and the risk label uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 diff --git a/tests/test_litellm/test_github_risk_tier.py b/tests/test_litellm/test_github_risk_tier.py index 722deb0603b..eb905a2e25a 100644 --- a/tests/test_litellm/test_github_risk_tier.py +++ b/tests/test_litellm/test_github_risk_tier.py @@ -2,6 +2,7 @@ from __future__ import annotations import importlib.util import json +import os import subprocess import sys from collections.abc import Sequence @@ -47,6 +48,15 @@ def _lines(count: int, prefix: str = "x = ") -> tuple[str, ...]: return tuple(f"{prefix}{index}" for index in range(count)) +def _rename_diff(old: str, new: str, deleted: Sequence[str] = (), added: Sequence[str] = ()) -> str: + header = f"diff --git a/{old} b/{new}\nsimilarity index 90%\nrename from {old}\nrename to {new}\n" + if not deleted and not added: + return header.replace("90%", "100%") + hunk = f"index 0000000..1111111 100644\n--- a/{old}\n+++ b/{new}\n@@ -1,{len(deleted)} +1,{len(added)} @@\n" + body = "".join(f"-{line}\n" for line in deleted) + "".join(f"+{line}\n" for line in added) + return header + hunk + body + + NEW_TEST = ("def test_regression():", " assert True") @@ -149,7 +159,49 @@ def test_generated_files_do_not_count_toward_size(risk_tier, rules): ) factor = _factor(_verdict(risk_tier, rules, diff), "size") assert factor.tier == "low" - assert factor.reason == "1 line(s) across 1 file(s)" + assert factor.reason == "1 line(s) across 1 file(s) outside the docs, tests, and model map tiers" + + +def test_docs_and_tests_do_not_count_toward_size(risk_tier, rules): + diff = ( + _file_diff("docs/my-website/docs/proxy/guide.md", added=_lines(5000, prefix="line ")) + + _file_diff("tests/test_litellm/test_x.py", added=_lines(500) + NEW_TEST) + + _file_diff("litellm/llms/anthropic/chat/x.py", added=("x = 1",)) + ) + factor = _factor(_verdict(risk_tier, rules, diff), "size") + assert factor.tier == "low" + assert factor.reason == "1 line(s) across 1 file(s) outside the docs, tests, and model map tiers" + + +def test_pure_move_counts_no_lines_toward_size(risk_tier, rules): + diff = _rename_diff("litellm/llms/anthropic/chat/old.py", "litellm/llms/anthropic/chat/new.py") + factor = _factor(_verdict(risk_tier, rules, diff), "size") + assert factor.tier == "low" + assert factor.reason == "0 line(s) across 1 file(s) outside the docs, tests, and model map tiers" + + +def test_move_with_edits_counts_only_the_edited_lines(risk_tier, rules): + diff = _rename_diff( + "litellm/llms/anthropic/chat/old.py", + "litellm/llms/anthropic/chat/new.py", + deleted=_lines(2), + added=_lines(2, prefix="y = "), + ) + assert _factor(_verdict(risk_tier, rules, diff), "size").reason.startswith("4 line(s) across 1 file(s)") + + +def test_move_out_of_an_always_human_path_stays_high(risk_tier, rules): + diff = _rename_diff("litellm/proxy/auth/old.py", "litellm/proxy/common_utils/old.py") + factor = _factor(_verdict(risk_tier, rules, diff), "paths") + assert factor.tier == "high" + assert factor.reason == "always-human: `litellm/proxy/auth/old.py`" + + +def test_move_into_an_always_human_path_is_high(risk_tier, rules): + diff = _rename_diff("litellm/proxy/utils/new.py", "litellm/proxy/auth/new.py") + factor = _factor(_verdict(risk_tier, rules, diff), "paths") + assert factor.tier == "high" + assert factor.reason == "always-human: `litellm/proxy/auth/new.py`" def test_removed_test_function_is_high(risk_tier, rules): @@ -231,6 +283,8 @@ def test_production_change_without_any_test_is_medium(risk_tier, rules): (('it("hides the notice", () => {', " expect(screen.queryByText(notice)).toBeNull();", "});"), "low"), (('it.skip("hides the notice", () => {', "});"), "high"), (("test.skip('hides the notice', async () => {", "});"), "high"), + (('it.only("hides the notice", () => {', "});"), "high"), + (('describe.only("login", () => {', "});"), "high"), (('xit("hides the notice", () => {', "});"), "high"), ( ( @@ -279,6 +333,16 @@ def test_author_factor(risk_tier, rules, author, from_fork, expected): ("ui/litellm-dashboard/package.json", "high"), ("scripts/type_check_gate.py", "high"), (".github/risk-tiers.yml", "high"), + ("enterprise/pyproject.toml", "high"), + ("litellm-proxy-extras/pyproject.toml", "high"), + ("litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py", "high"), + ("litellm/proxy/_experimental/mcp_server/outbound_credentials/x.py", "high"), + ("litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py", "high"), + ("litellm/proxy/_experimental/mcp_server/oauth2_token_cache.py", "high"), + ("litellm/proxy/_experimental/mcp_server/byok_oauth_endpoints.py", "high"), + ("litellm/proxy/_experimental/mcp_server/bridge_token_flow.py", "high"), + ("litellm/proxy/_experimental/mcp_server/proxy_api_credentials.py", "high"), + ("litellm/proxy/_experimental/mcp_server/server.py", "medium"), ("enterprise/litellm_enterprise/proxy/hooks/x.py", "high"), ("enterprise/litellm_enterprise/proxy/auth/x.py", "high"), ("enterprise/litellm_enterprise/proxy/management_endpoints/x.py", "high"), @@ -297,6 +361,8 @@ def test_author_factor(risk_tier, rules, author, from_fork, expected): ("model_prices_and_context_window.json", "low"), ("litellm/model_prices_and_context_window_backup.json", "low"), ("litellm/proxy/README.md", "low"), + ("type-discipline-budget.json", "low"), + ("ruff-strict-budget.json", "low"), ], ) def test_path_tier_from_the_checked_in_config(rules, path, expected): @@ -335,6 +401,27 @@ def test_parse_diff_of_an_empty_diff_is_empty(risk_tier): assert risk_tier.parse_diff("") == () +def test_parse_diff_reads_a_rename_as_one_change_with_both_paths(risk_tier): + diff = _rename_diff("litellm/a/very_long_old_name.py", "litellm/b/new.py", deleted=("x = 1",), added=("x = 2",)) + changes = risk_tier.parse_diff(diff) + assert len(changes) == 1 + assert changes[0].path == "litellm/b/new.py" + assert changes[0].previous_path == "litellm/a/very_long_old_name.py" + assert changes[0].paths == ("litellm/a/very_long_old_name.py", "litellm/b/new.py") + assert changes[0].line_count == 2 + + +def test_parse_diff_unquotes_renamed_paths(risk_tier): + diff = ( + 'diff --git "a/docs/we\\"ird.md" b/docs/plain.md\n' + "similarity index 100%\n" + 'rename from "docs/we\\"ird.md"\n' + "rename to docs/plain.md\n" + ) + changes = risk_tier.parse_diff(diff) + assert changes[0].paths == ('docs/we"ird.md', "docs/plain.md") + + def test_parse_diff_decodes_git_quoted_paths_instead_of_dropping_them(risk_tier): diff = ( "diff --git a/docs/plain.md b/docs/plain.md\n" @@ -370,6 +457,7 @@ def _git(repo: Path, *args: str) -> str: capture_output=True, text=True, env={ + "PATH": os.environ["PATH"], "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@x", "GIT_COMMITTER_NAME": "t", @@ -443,3 +531,25 @@ def test_git_quoted_filename_still_reaches_the_paths_factor(risk_tier, rules, tm assert changes[0].added_lines == ("def f():", " return 1") verdict = risk_tier.classify(changes, DEVIN, False, rules) assert _factor(verdict, "paths").tier == "high" + + +def test_git_pure_move_is_one_zero_line_change(risk_tier, rules, tmp_path): + repo = tmp_path / "repo" + (repo / "litellm" / "proxy" / "auth").mkdir(parents=True) + _git(tmp_path, "init", "-q", "-b", "main", "repo") + (repo / "litellm" / "proxy" / "auth" / "checks.py").write_text("".join(f"x{i} = {i}\n" for i in range(300))) + _git(repo, "add", ".") + _git(repo, "commit", "-q", "-m", "base") + base = _git(repo, "rev-parse", "HEAD") + (repo / "litellm" / "proxy" / "utils").mkdir() + _git(repo, "mv", "litellm/proxy/auth/checks.py", "litellm/proxy/utils/checks.py") + _git(repo, "commit", "-q", "-m", "move") + head = _git(repo, "rev-parse", "HEAD") + + changes = risk_tier.parse_diff(risk_tier.git_diff(repo, base, head)) + + assert [change.paths for change in changes] == [("litellm/proxy/auth/checks.py", "litellm/proxy/utils/checks.py")] + assert changes[0].line_count == 0 + verdict = risk_tier.classify(changes, DEVIN, False, rules) + assert _factor(verdict, "paths").tier == "high" + assert _factor(verdict, "size").tier == "low" From 6edc2c16116f474a386666179e7200fb23022ca9 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 7 Sep 2026 17:13:39 -0700 Subject: [PATCH 5/7] fix(ci): tier nested MCP OAuth and flow files as always-human The two MCP globs only matched files directly under mcp_server, so faults/render_oauth.py, which renders the OAuth and DCR fault responses, scored medium. Both globs now use **/ so nested files match too. The last 400 staging merges still score 217 high, 139 medium, 44 low --- .github/risk-tiers.yml | 4 ++-- tests/test_litellm/test_github_risk_tier.py | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/risk-tiers.yml b/.github/risk-tiers.yml index ce7428e7b6b..63d61a69f22 100644 --- a/.github/risk-tiers.yml +++ b/.github/risk-tiers.yml @@ -25,8 +25,8 @@ paths: - "litellm/proxy/_experimental/mcp_server/auth/**" - "litellm/proxy/_experimental/mcp_server/outbound_credentials/**" - "litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py" - - "litellm/proxy/_experimental/mcp_server/*oauth*" - - "litellm/proxy/_experimental/mcp_server/*_flow.py" + - "litellm/proxy/_experimental/mcp_server/**/*oauth*" + - "litellm/proxy/_experimental/mcp_server/**/*_flow.py" - "litellm/proxy/_experimental/mcp_server/proxy_api_credentials.py" - ".github/**" - ".circleci/**" diff --git a/tests/test_litellm/test_github_risk_tier.py b/tests/test_litellm/test_github_risk_tier.py index eb905a2e25a..696f5464d78 100644 --- a/tests/test_litellm/test_github_risk_tier.py +++ b/tests/test_litellm/test_github_risk_tier.py @@ -342,6 +342,8 @@ def test_author_factor(risk_tier, rules, author, from_fork, expected): ("litellm/proxy/_experimental/mcp_server/byok_oauth_endpoints.py", "high"), ("litellm/proxy/_experimental/mcp_server/bridge_token_flow.py", "high"), ("litellm/proxy/_experimental/mcp_server/proxy_api_credentials.py", "high"), + ("litellm/proxy/_experimental/mcp_server/faults/render_oauth.py", "high"), + ("litellm/proxy/_experimental/mcp_server/faults/classify.py", "medium"), ("litellm/proxy/_experimental/mcp_server/server.py", "medium"), ("enterprise/litellm_enterprise/proxy/hooks/x.py", "high"), ("enterprise/litellm_enterprise/proxy/auth/x.py", "high"), From 7064b2bf094c8ff9012d6b9a679c7ff3390974ba Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 7 Sep 2026 17:57:01 -0700 Subject: [PATCH 6/7] fix(ci): run the risk gate on pull_request and follow the one-pager rubric The workflow now triggers on pull_request instead of pull_request_target, which the auto-approve one-pager and the 2026-06-18 CI exposure post-mortem both forbid, and the job skips fork PRs since the read-only token cannot label them. The floor now follows the rubric's author row (Devin-opened low, human-opened medium, fork high) through a new authors.low list, and adds a paths.medium list for agent instruction files, conftest, helm, and docker. Two guards compare both sides of a file through git show: model map rows that change or disappear stop being low, and a budget file that raises a limit stops being low. A skip or importorskip call added to an existing test file with no net new test scores high. The last 400 staging merges score 218 high, 179 medium, 3 low; the author row alone moves 37 human-opened docs and test PRs from low to medium --- .github/risk-tiers.yml | 18 +++ .github/scripts/risk_tier.py | 150 ++++++++++++++++++-- .github/workflows/risk-gate.yml | 13 +- tests/test_litellm/test_github_risk_tier.py | 130 ++++++++++++++++- 4 files changed, 291 insertions(+), 20 deletions(-) diff --git a/.github/risk-tiers.yml b/.github/risk-tiers.yml index 63d61a69f22..512fa9d6761 100644 --- a/.github/risk-tiers.yml +++ b/.github/risk-tiers.yml @@ -35,6 +35,13 @@ paths: - "**/package*.json" - "**/Dockerfile*" - "scripts/*gate*" + medium: + - "**/CLAUDE.md" + - "**/AGENTS.md" + - "**/GEMINI.md" + - "**/conftest.py" + - "helm/**" + - "docker/**" low: - "cookbook/**" - "model_prices_and_context_window.json" @@ -65,3 +72,14 @@ tests: - "**/*.test.*" - "**/*.spec.*" - "**/__tests__/**" + +authors: + low: + - "devin-ai-integration[bot]" + +guards: + additive_rows: + - "model_prices_and_context_window.json" + - "litellm/model_prices_and_context_window_backup.json" + lowered_limits: + - "*-budget.json" diff --git a/.github/scripts/risk_tier.py b/.github/scripts/risk_tier.py index b210172ebed..6f6d6089ace 100644 --- a/.github/scripts/risk_tier.py +++ b/.github/scripts/risk_tier.py @@ -5,23 +5,26 @@ import json import re import subprocess import sys -from collections.abc import Sequence -from dataclasses import dataclass +from collections.abc import Callable, Mapping, Sequence +from dataclasses import dataclass, replace from pathlib import Path from types import MappingProxyType from typing import Final, Literal, TypeAlias import yaml -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, TypeAdapter, ValidationError from typing_extensions import ReadOnly, TypedDict Tier: TypeAlias = Literal["low", "medium", "high"] +ReadFile: TypeAlias = Callable[[str, str], str | None] +RewriteCheck: TypeAlias = Callable[[str | None, str | None], str | None] TEST_DEF_RE: Final = re.compile(r"^\s*(?:async\s+)?def\s+test_|^\s*(?:it|test)\(") SKIP_RE: Final = re.compile( r"pytest\.mark\.(?:skip(?!if)|xfail)|unittest\.skip\b" r"|\b(?:it|test|describe)\.(?:skip\(\s*[\"'`]|only\()|\bx(?:it|test|describe)\(" ) +SKIP_CALL_RE: Final = re.compile(r"\bpytest\.(?:skip|importorskip)\(|\b(?:it|test|describe)\.fixme\(") ASSERT_RE: Final = re.compile(r"^\s*assert\b|\bexpect\(") GLOB_TOKEN_RE: Final = re.compile(r"(\*\*/|\*\*|\*|\?)") DIFF_BLOCK_SEPARATOR: Final = "\ndiff --git " @@ -32,6 +35,7 @@ QUOTED_PATH_ESCAPES: Final = MappingProxyType( {"a": "\a", "b": "\b", "f": "\f", "n": "\n", "r": "\r", "t": "\t", "v": "\v", '"': '"', "\\": "\\"} ) MAX_LISTED_PATHS: Final = 5 +JSON_OBJECT: Final = TypeAdapter(dict[str, object]) class SizeLimit(BaseModel): @@ -56,6 +60,7 @@ class ModuleRules(BaseModel): class PathRules(BaseModel): model_config = ConfigDict(extra="forbid", frozen=True) high: tuple[str, ...] + medium: tuple[str, ...] = () low: tuple[str, ...] @@ -64,12 +69,25 @@ class TestRules(BaseModel): files: tuple[str, ...] +class AuthorRules(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + low: tuple[str, ...] + + +class GuardRules(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + additive_rows: tuple[str, ...] = () + lowered_limits: tuple[str, ...] = () + + class RiskConfig(BaseModel): model_config = ConfigDict(extra="forbid", frozen=True) paths: PathRules modules: ModuleRules size: SizeRules tests: TestRules + authors: AuthorRules + guards: GuardRules = GuardRules() @dataclass(frozen=True, slots=True) @@ -90,6 +108,7 @@ class FileChange: added_lines: tuple[str, ...] deleted_lines: tuple[str, ...] previous_path: str | None = None + guarded_rewrite: str | None = None @property def paths(self) -> tuple[str, ...]: @@ -104,29 +123,45 @@ class FileChange: class Rules: config: RiskConfig high_paths: PathMatcher + medium_paths: PathMatcher low_paths: PathMatcher test_files: PathMatcher size_ignored: PathMatcher + additive_rows: PathMatcher + lowered_limits: PathMatcher @staticmethod def from_config(config: RiskConfig) -> Rules: return Rules( config=config, high_paths=PathMatcher.from_globs(config.paths.high), + medium_paths=PathMatcher.from_globs(config.paths.medium), low_paths=PathMatcher.from_globs(config.paths.low), test_files=PathMatcher.from_globs(config.tests.files), size_ignored=PathMatcher.from_globs(config.size.ignore), + additive_rows=PathMatcher.from_globs(config.guards.additive_rows), + lowered_limits=PathMatcher.from_globs(config.guards.lowered_limits), ) def path_tier(self, path: str) -> Tier: if self.high_paths.matches(path): return "high" + if self.medium_paths.matches(path): + return "medium" if self.test_files.matches(path) or self.low_paths.matches(path): return "low" return "medium" def change_tier(self, change: FileChange) -> Tier: - return highest(tuple(self.path_tier(path) for path in change.paths)) + rewrite_tier: Final[Tier] = "low" if change.guarded_rewrite is None else "medium" + return highest((rewrite_tier, *(self.path_tier(path) for path in change.paths))) + + def rewrite_check(self, path: str) -> RewriteCheck | None: + if self.additive_rows.matches(path): + return rewritten_rows + if self.lowered_limits.matches(path): + return raised_limits + return None def is_production(self, change: FileChange) -> bool: return self.change_tier(change) != "low" @@ -263,7 +298,14 @@ def paths_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: ) return Factor("paths", "high", f"always-human: {_listed(always_human)}") case "medium": - return Factor("paths", "medium", f"{len(matching)} file(s) outside the docs, tests, and model map tiers") + rewritten: Final = tuple( + f"{change.guarded_rewrite} in `{change.path}`" for change in matching if change.guarded_rewrite + ) + outside: Final = len(matching) - len(rewritten) + outside_note: Final = ( + (f"{outside} file(s) outside the docs, tests, and model map tiers",) if outside else () + ) + return Factor("paths", "medium", "; ".join((*rewritten, *outside_note))) case "low": return Factor("paths", "low", "docs, tests, cookbook, or model map only") @@ -312,12 +354,17 @@ def tests_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: test_changes: Final = tuple(change for change in changes if rules.test_files.matches(change.path)) net_tests: Final = _net(test_changes, TEST_DEF_RE) net_skips: Final = _net(test_changes, SKIP_RE) + silenced: Final = sum( + max(_net((change,), SKIP_CALL_RE), 0) for change in test_changes if _net((change,), TEST_DEF_RE) <= 0 + ) net_asserts: Final = _net(test_changes, ASSERT_RE) production_changed: Final = any(rules.is_production(change) for change in changes) if net_tests < 0: return Factor("tests", "high", f"{-net_tests} test(s) removed") if net_skips > 0: return Factor("tests", "high", f"{net_skips} skip marker(s) added") + if silenced > 0: + return Factor("tests", "high", f"{silenced} skip call(s) added to existing tests") if net_asserts < 0: return Factor("tests", "high", f"{-net_asserts} assertion(s) removed") if not production_changed: @@ -329,10 +376,12 @@ def tests_factor(changes: Sequence[FileChange], rules: Rules) -> Factor: return Factor("tests", "medium", "production code changed with no test touched") -def author_factor(author: str, from_fork: bool) -> Factor: +def author_factor(author: str, from_fork: bool, rules: Rules) -> Factor: if from_fork: return Factor("author", "high", f"`{author}` from a fork") - return Factor("author", "low", f"`{author}` on an internal branch") + if author in rules.config.authors.low: + return Factor("author", "low", f"`{author}` opened it on an internal branch") + return Factor("author", "medium", f"`{author}` opened it by hand on an internal branch") def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules: Rules) -> Verdict: @@ -341,11 +390,88 @@ def classify(changes: Sequence[FileChange], author: str, from_fork: bool, rules: modules_factor(changes, rules), size_factor(changes, rules), tests_factor(changes, rules), - author_factor(author, from_fork), + author_factor(author, from_fork, rules), ) return Verdict(highest(tuple(factor.tier for factor in factors)), factors) +def _json_object(text: str | None) -> Mapping[str, object] | None: + if text is None: + return None + try: + return MappingProxyType(JSON_OBJECT.validate_json(text)) + except ValidationError: + return None + + +def _as_object(value: object) -> Mapping[str, object] | None: + try: + return MappingProxyType(JSON_OBJECT.validate_python(value)) + except ValidationError: + return None + + +def rewritten_rows(base_text: str | None, head_text: str | None) -> str | None: + if base_text is None: + return None + base: Final = _json_object(base_text) + head: Final = _json_object(head_text) + if base is None or head is None: + return "not a JSON object on both sides" + rewritten: Final = sum(1 for key, value in base.items() if key not in head or head[key] != value) + return f"{rewritten} existing row(s) changed or removed" if rewritten else None + + +def _limits(value: object, prefix: str = "") -> tuple[tuple[str, float], ...]: + node: Final = _as_object(value) + if node is None: + return () + limit: Final = node.get("limit") + own: Final = ((prefix, float(limit)),) if isinstance(limit, int | float) and not isinstance(limit, bool) else () + nested: Final = tuple( + pair for key, child in node.items() if key != "limit" for pair in _limits(child, f"{prefix}/{key}") + ) + return (*own, *nested) + + +def raised_limits(base_text: str | None, head_text: str | None) -> str | None: + if base_text is None: + return None + base: Final = _json_object(base_text) + head: Final = _json_object(head_text) + if base is None or head is None: + return "not a JSON object on both sides" + base_limits: Final = MappingProxyType(dict(_limits(base))) + raised: Final = sum(1 for key, limit in _limits(head) if key not in base_limits or limit > base_limits[key]) + return f"{raised} limit(s) raised" if raised else None + + +def with_guards( + changes: Sequence[FileChange], rules: Rules, base: str, head: str, read_file: ReadFile +) -> tuple[FileChange, ...]: + return tuple(_guarded(change, rules, base, head, read_file) for change in changes) + + +def _guarded(change: FileChange, rules: Rules, base: str, head: str, read_file: ReadFile) -> FileChange: + check: Final = rules.rewrite_check(change.path) + if check is None: + return change + reason: Final = check(read_file(base, change.previous_path or change.path), read_file(head, change.path)) + return change if reason is None else replace(change, guarded_rewrite=reason) + + +def git_show(repo: Path, rev: str, path: str) -> str | None: + completed: Final = subprocess.run( + ("git", "show", f"{rev}:{path}"), + cwd=repo, + capture_output=True, + encoding="utf-8", + errors="replace", + check=False, + ) + return completed.stdout if completed.returncode == 0 else None + + def git_diff(repo: Path, base: str, head: str) -> str: completed: Final = subprocess.run( ("git", "-c", "core.quotePath=false", "diff", "--find-renames", "--no-ext-diff", "-U0", base, head), @@ -384,7 +510,13 @@ class CliArgs(BaseModel): def main(argv: Sequence[str] | None = None) -> int: args: Final = CliArgs.model_validate(vars(build_parser().parse_args(argv))) rules: Final = Rules.from_config(load_config(args.config)) - changes: Final = parse_diff(git_diff(args.repo, args.base, args.head)) + changes: Final = with_guards( + parse_diff(git_diff(args.repo, args.base, args.head)), + rules, + args.base, + args.head, + lambda rev, path: git_show(args.repo, rev, path), + ) verdict: Final = classify(changes, args.author, args.from_fork, rules) if args.json_out is not None: args.json_out.write_text(verdict.to_json(), encoding="utf-8") diff --git a/.github/workflows/risk-gate.yml b/.github/workflows/risk-gate.yml index 0e3645b83a7..654df5225bd 100644 --- a/.github/workflows/risk-gate.yml +++ b/.github/workflows/risk-gate.yml @@ -1,7 +1,7 @@ name: risk-gate -on: # zizmor: ignore[dangerous-triggers] runs the base branch's copy of the gate only; the PR's diff is read as data and never executed - pull_request_target: +on: + pull_request: types: [opened, synchronize, reopened, ready_for_review] branches: - litellm_internal_staging @@ -16,6 +16,7 @@ concurrency: jobs: floor: name: risk-gate floor (shadow) + if: github.event.pull_request.head.repo.full_name == github.repository runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -43,23 +44,17 @@ jobs: with: version: "0.10.9" - - name: Compute the floor tier with the base branch's copy of the gate + - name: Compute the floor tier env: MERGE_BASE: ${{ steps.revisions.outputs.merge_base }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} PR_AUTHOR: ${{ github.event.pull_request.user.login }} - FROM_FORK: ${{ github.event.pull_request.head.repo.full_name != github.repository }} run: | - fork_flag=() - if [ "$FROM_FORK" = "true" ]; then - fork_flag=(--from-fork) - fi uv run --frozen python .github/scripts/risk_tier.py \ --config .github/risk-tiers.yml \ --base "$MERGE_BASE" \ --head "$HEAD_SHA" \ --author "$PR_AUTHOR" \ - "${fork_flag[@]}" \ --json-out "${RUNNER_TEMP}/risk.json" \ | tee -a "$GITHUB_STEP_SUMMARY" diff --git a/tests/test_litellm/test_github_risk_tier.py b/tests/test_litellm/test_github_risk_tier.py index 696f5464d78..a7a23c609f7 100644 --- a/tests/test_litellm/test_github_risk_tier.py +++ b/tests/test_litellm/test_github_risk_tier.py @@ -236,6 +236,7 @@ def test_unconditional_skip_marker_is_high(risk_tier, rules, marker): ' pytest.skip("needs a real key")', '@unittest.skipIf(sys.platform == "win32", "posix only")', '@unittest.skipUnless(HAS_REDIS, "needs redis")', + 'redis = pytest.importorskip("redis")', ], ) def test_conditional_skip_is_not_a_silenced_test(risk_tier, rules, guard): @@ -247,6 +248,29 @@ def test_conditional_skip_is_not_a_silenced_test(risk_tier, rules, guard): assert factor.reason == "1 test(s) added" +@pytest.mark.parametrize( + ("path", "line"), + [ + ("tests/test_litellm/test_a.py", ' pytest.skip("flaky since the refactor, see LIT-0000")'), + ("tests/test_litellm/test_a.py", 'redis = pytest.importorskip("redis")'), + ("ui/litellm-dashboard/src/app/login/LoginPage.test.tsx", " test.fixme(true, 'broken after the redesign');"), + ], +) +def test_skip_call_added_to_an_existing_test_is_high_even_without_production_code(risk_tier, rules, path, line): + verdict = _verdict(risk_tier, rules, _file_diff(path, added=(line,))) + assert verdict.tier == "high" + assert _factor(verdict, "tests").reason == "1 skip call(s) added to existing tests" + + +def test_removing_a_skip_call_is_not_silencing(risk_tier, rules): + diff = _file_diff("tests/test_litellm/test_a.py", deleted=(' pytest.skip("flaky")',)) + _file_diff( + "litellm/llms/anthropic/chat/x.py", added=("x = 1",) + ) + factor = _factor(_verdict(risk_tier, rules, diff), "tests") + assert factor.tier == "medium" + assert factor.reason == "tests edited, none added" + + def test_weakened_assertions_are_high(risk_tier, rules): diff = _file_diff( "tests/test_litellm/test_a.py", @@ -311,7 +335,7 @@ def test_typescript_tests_count_like_python_ones(risk_tier, rules, added, expect ("author", "from_fork", "expected"), [ (DEVIN, False, "low"), - ("mateo-berri", False, "low"), + ("mateo-berri", False, "medium"), (DEVIN, True, "high"), ("jairandresdiazp", True, "high"), ], @@ -355,7 +379,17 @@ def test_author_factor(risk_tier, rules, author, from_fork, expected): ("ui/litellm-dashboard/src/app/login/LoginPage.test.tsx", "low"), ("tests/e2e/x.py", "low"), ("litellm-proxy-extras/tests/test_x.py", "low"), - ("helm/litellm-helm/tests/x.yaml", "low"), + ("helm/litellm-helm/tests/x.yaml", "medium"), + ("helm/litellm-helm/templates/tests/test-connection.yaml", "medium"), + ("docker/tests/nonroot.yaml", "medium"), + ("docker/entrypoint.sh", "medium"), + ("CLAUDE.md", "medium"), + ("AGENTS.md", "medium"), + ("GEMINI.md", "medium"), + ("litellm/proxy/_experimental/mcp_server/CLAUDE.md", "medium"), + ("tests/conftest.py", "medium"), + ("tests/test_litellm/conftest.py", "medium"), + ("tests/e2e/junit_properties.py", "low"), ("ui/litellm-dashboard/tests/x.spec.ts", "low"), ("litellm-rust/crates/core/tests/x.rs", "low"), ("enterprise/litellm_enterprise/proxy/common_utils/x.py", "medium"), @@ -371,6 +405,73 @@ def test_path_tier_from_the_checked_in_config(rules, path, expected): assert rules.path_tier(path) == expected +def test_human_opened_docs_change_is_medium_on_the_author_factor_only(risk_tier, rules): + verdict = _verdict(risk_tier, rules, _file_diff("README.md", added=("hello",)), author="mateo-berri") + assert verdict.tier == "medium" + assert {factor.name: factor.tier for factor in verdict.factors} == { + "paths": "low", + "modules": "low", + "size": "low", + "tests": "low", + "author": "medium", + } + + +MODEL_MAP = "model_prices_and_context_window.json" +BUDGET = "ruff-strict-budget.json" + + +def _guarded_verdict(risk_tier, rules, path: str, base: str | None, head: str | None): + contents = {("base", path): base, ("head", path): head} + diff = _file_diff(path, added=("changed",)) + changes = risk_tier.with_guards(risk_tier.parse_diff(diff), rules, "base", "head", lambda rev, p: contents[(rev, p)]) + return risk_tier.classify(changes, DEVIN, False, rules) + + +def test_additive_model_map_rows_stay_low(risk_tier, rules): + base = json.dumps({"gpt-x": {"input_cost_per_token": 1e-6}}) + head = json.dumps({"gpt-x": {"input_cost_per_token": 1e-6}, "gpt-y": {"input_cost_per_token": 2e-6}}) + verdict = _guarded_verdict(risk_tier, rules, MODEL_MAP, base, head) + assert verdict.tier == "low" + assert _factor(verdict, "paths").reason == "docs, tests, cookbook, or model map only" + + +@pytest.mark.parametrize( + "head", + [ + {"gpt-x": {"input_cost_per_token": 3e-6}}, + {"gpt-x-renamed": {"input_cost_per_token": 1e-6}}, + {}, + ], +) +def test_changed_or_removed_model_map_rows_are_medium(risk_tier, rules, head): + base = json.dumps({"gpt-x": {"input_cost_per_token": 1e-6}}) + verdict = _guarded_verdict(risk_tier, rules, MODEL_MAP, base, json.dumps(head)) + assert verdict.tier == "medium" + assert _factor(verdict, "paths").tier == "medium" + assert _factor(verdict, "paths").reason == f"1 existing row(s) changed or removed in `{MODEL_MAP}`" + + +def test_new_model_map_file_or_broken_json_is_never_low(risk_tier, rules): + assert _guarded_verdict(risk_tier, rules, MODEL_MAP, None, "{}").tier == "low" + assert _factor(_guarded_verdict(risk_tier, rules, MODEL_MAP, "{}", "not json"), "paths").tier == "medium" + assert _factor(_guarded_verdict(risk_tier, rules, MODEL_MAP, "{}", None), "paths").tier == "medium" + + +def test_lowered_budget_limits_stay_low_and_raised_ones_are_medium(risk_tier, rules): + base = json.dumps({"ANN001": {"limit": 10}, "B006": {"limit": 5}}) + lowered = json.dumps({"ANN001": {"limit": 9}, "B006": {"limit": 5}}) + raised = json.dumps({"ANN001": {"limit": 10}, "B006": {"limit": 6}}) + new_rule = json.dumps({"ANN001": {"limit": 10}, "B006": {"limit": 5}, "B999": {"limit": 1}}) + dropped_rule = json.dumps({"ANN001": {"limit": 10}}) + assert _guarded_verdict(risk_tier, rules, BUDGET, base, lowered).tier == "low" + assert _guarded_verdict(risk_tier, rules, BUDGET, base, dropped_rule).tier == "low" + for head in (raised, new_rule): + verdict = _guarded_verdict(risk_tier, rules, BUDGET, base, head) + assert verdict.tier == "medium" + assert _factor(verdict, "paths").reason == f"1 limit(s) raised in `{BUDGET}`" + + def test_single_star_does_not_cross_directories(risk_tier): assert risk_tier.glob_to_regex("Dockerfile*").fullmatch("Dockerfile.database") assert risk_tier.glob_to_regex("Dockerfile*").fullmatch("docker/Dockerfile.database") is None @@ -513,6 +614,31 @@ def test_main_end_to_end_against_a_git_repo(risk_tier, tmp_path, capsys): assert payload["summary"] == printed +def test_main_reads_both_sides_of_the_model_map_from_git(risk_tier, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + _git(tmp_path, "init", "-q", "-b", "main", "repo") + model_map = repo / "model_prices_and_context_window.json" + model_map.write_text(json.dumps({"gpt-x": {"input_cost_per_token": 1e-6}}, indent=1)) + _git(repo, "add", ".") + _git(repo, "commit", "-q", "-m", "base") + base = _git(repo, "rev-parse", "HEAD") + model_map.write_text(json.dumps({"gpt-x": {"input_cost_per_token": 2e-6}}, indent=1)) + _git(repo, "commit", "-q", "-am", "reprice") + head = _git(repo, "rev-parse", "HEAD") + json_out = tmp_path / "risk.json" + + exit_code = risk_tier.main( + ["--config", str(CONFIG_PATH), "--base", base, "--head", head, "--author", DEVIN, "--repo", str(repo), "--json-out", str(json_out)] + ) + + assert exit_code == 0 + payload = json.loads(json_out.read_text()) + assert payload["tier"] == "medium" + paths = next(factor for factor in payload["factors"] if factor["name"] == "paths") + assert paths["reason"] == "1 existing row(s) changed or removed in `model_prices_and_context_window.json`" + + def test_git_quoted_filename_still_reaches_the_paths_factor(risk_tier, rules, tmp_path): repo = tmp_path / "repo" (repo / "litellm" / "proxy" / "auth").mkdir(parents=True) From 472d1dce435bb86580d6d0a013f4ef7677afdfdd Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 7 Sep 2026 18:05:53 -0700 Subject: [PATCH 7/7] fix(ci): keep the write token out of the job that runs pull request code The floor job now runs with a read-only token and hands the tier and summary to a second job through job outputs. Only that report job holds pull-requests and checks write, and it never checks out code, so a compromised dependency or setup action in the compute step cannot forge labels or check runs. A PR that edits the workflow file itself still runs its own copy, which the ruleset's two-approval .github/** path covers --- .github/workflows/risk-gate.yml | 36 ++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/.github/workflows/risk-gate.yml b/.github/workflows/risk-gate.yml index 654df5225bd..bc3f0712ff2 100644 --- a/.github/workflows/risk-gate.yml +++ b/.github/workflows/risk-gate.yml @@ -21,8 +21,9 @@ jobs: timeout-minutes: 10 permissions: contents: read - pull-requests: write - checks: write + outputs: + tier: ${{ steps.floor.outputs.tier }} + summary: ${{ steps.floor.outputs.summary }} steps: - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 with: @@ -45,6 +46,8 @@ jobs: version: "0.10.9" - name: Compute the floor tier + id: floor + shell: bash env: MERGE_BASE: ${{ steps.revisions.outputs.merge_base }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} @@ -57,15 +60,34 @@ jobs: --author "$PR_AUTHOR" \ --json-out "${RUNNER_TEMP}/risk.json" \ | tee -a "$GITHUB_STEP_SUMMARY" + { + echo "tier=$(jq -r '.tier' "${RUNNER_TEMP}/risk.json")" + echo "summary<> "$GITHUB_OUTPUT" + report: + name: risk-gate report (shadow) + needs: floor + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + pull-requests: write + checks: write + steps: - name: Publish the risk-gate check run and the risk label uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 env: - RISK_JSON: ${{ runner.temp }}/risk.json + TIER: ${{ needs.floor.outputs.tier }} + SUMMARY: ${{ needs.floor.outputs.summary }} with: script: | - const fs = require('fs'); - const risk = JSON.parse(fs.readFileSync(process.env.RISK_JSON, 'utf8')); + const tier = process.env.TIER; + if (!['low', 'medium', 'high'].includes(tier)) { + core.setFailed(`unexpected tier ${JSON.stringify(tier)} from the floor job`); + return; + } const pr = context.payload.pull_request; const repo = { owner: context.repo.owner, repo: context.repo.repo }; await github.rest.checks.create({ @@ -74,9 +96,9 @@ jobs: head_sha: pr.head.sha, status: 'completed', conclusion: 'neutral', - output: { title: `risk: ${risk.tier} (shadow)`, summary: risk.summary }, + output: { title: `risk: ${tier} (shadow)`, summary: process.env.SUMMARY }, }); - const wanted = `risk:${risk.tier}`; + const wanted = `risk:${tier}`; const { data: labels } = await github.rest.issues.listLabelsOnIssue({ ...repo, issue_number: pr.number,