fetch_pr_author_association already wraps gh api in a try/except
CalledProcessError to keep the daily sweep moving when the lookup for
one PR transiently fails. fetch_pr_comments lacked the same guard — a
single 5xx mid-loop would crash the run before later PRs were
evaluated. Mirror the fail-safe pattern: return [] on CalledProcessError
or malformed paginated JSON; downstream that becomes a conservative
skip-no-greptile-score for the affected PR and the sweep continues to
the next one (which will be re-evaluated tomorrow).
Pinned with two new TestFetchPrComments tests.
Reconsider previously treated any historical Agent Shin auto-close
comment as sufficient proof that Agent Shin owns the current closure.
If the PR was reopened and later re-closed by a maintainer (e.g. as
a duplicate or out-of-scope), the contributor could override that
maintainer-initiated closure by commenting `@agent-shin reconsider`.
`was_auto_closed_by_agent_shin` now fetches the issue events, finds
the most recent `closed` event, and requires its actor login to (a)
end with [bot] and (b) match the author of a comment containing the
Agent Shin marker. Closures by maintainers or unrelated bots (stale,
cla-assistant, etc.) no longer satisfy provenance.
The close comment in close_low_quality_prs.py directs users to comment
'@agent-shin reconsider', but lacked the AGENT_SHIN_AUTO_CLOSE_MARKER
('I'm **Agent Shin**') string. Without that marker, the reconsider
provenance check in was_auto_closed_by_agent_shin() rejected the
request, so following the instructions was a silent no-op.
Co-authored-by: Yassin Kortam <yassin@berri.ai>
Addresses greptile-apps P2 review note: 'gh pr list --limit 1000' is
the CLI's hard ceiling. If BerriAI/litellm ever exceeds 1000 open PRs,
fetch_open_prs silently returns a truncated list and the daily sweep
quietly misses everything beyond the cap.
Extract the cap value as GH_PR_LIST_LIMIT and emit a GitHub Actions
':⚠️:' annotation (to stderr, where Actions surfaces it in the
job log) whenever the result count reaches the cap. This is the minimal
defensive change — a real fix (paginated 'gh api' calls) is left as a
follow-up because the repo is nowhere near the cap today and an
unnecessary pagination loop would slow every run.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Address two related concerns raised in PR review on the reconsider flow:
1. **Dry-run support for --reconsider** (P1, greptile-apps):
The previous --reconsider branch unconditionally called post_comment +
reopen_* regardless of --close. The docstring claimed 'close is forced
True implicitly', but the workflow's only kill switch was
AGENT_SHIN_ENABLED — invoking the script directly without --close was
still destructive, the opposite of the conventional dry-run
expectation.
triage() now honors close=False in reconsider mode: a passing verdict
returns action='would-reopen' (with the comment body it WOULD post in
result['comment']) and a failing verdict returns
action='would-leave-closed-still-failing'. The reconsider workflow now
appends --close iff AGENT_SHIN_ENABLED == 'true', mirroring the
pattern used by close_low_quality_prs.yml.
2. **Provenance gate for reopen** (P2, both greptile-apps and veria-ai):
The reconsider flow could be used to silently override a maintainer's
close decision — an external author edits the closed PR to include a
closing keyword, comments '@agent-shin reconsider', and the bot
reopens it. There was no check that Agent Shin was the actor that
originally closed it.
triage() now requires was_auto_closed_by_agent_shin() to be true
before any reopen path can fire. The check looks for a bot-authored
comment (login ends with '[bot]') containing the auto-close marker
'I'm **Agent Shin**'. Filtering by bot author makes the marker
unspoofable: a contributor pasting the phrase into a manual comment
cannot satisfy the check. When the provenance gate fails, triage
returns action='skip-not-bot-closed' without burning LLM tokens or
posting anything.
Unit tests cover both behaviors plus the failure modes of
was_auto_closed_by_agent_shin (no comments, non-bot author with marker,
bot comment without marker, marker found anywhere in the comment list).
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Adds regression tests covering the bugbot high-severity finding that
str.format() would crash on user-supplied content containing { or }.
Empirically str.format() does NOT re-parse interpolated values — only
the template literal is scanned for replacement fields — so the bug
does not exist in the current code, but pinning the safe behavior
prevents a future templating change from silently reintroducing it.
Also pins the dedented prompt shape (no leading 8-space indentation on
template lines) so a future change to the build_*_prompt functions can't
silently regress the LLM judge prompt format on multi-line bodies.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Follow-up to PR #28117. Three behavior changes + one new workflow,
addressing the team's concerns on the original review:
1) Apply auto-close to ALL open PRs, not just those over a week old.
- close_low_quality_prs.py: --min-age-days default flipped from 7 to
0. The flag is preserved as an opt-in safety net for one-off
backfill runs that want to spare very-young PRs, but the daily
scheduled sweep now closes external-author PRs as soon as Greptile
scores them <4/5.
- close_low_quality_prs.yml: workflow_dispatch input default also
flipped to 0; doc comments updated.
2) Apply auto-close to draft PRs too.
- close_low_quality_prs.py: removed the skip-draft branch in
evaluate_pr. Drafts are NOT a free pass — the team's intent is
'open PR count == PRs internal collaborators need to action on',
so a draft Greptile scored 2/5 still belongs in the closed bucket.
Authors who genuinely need a long-lived draft can attach the 'wip'
opt-out label, which is unchanged.
- The 'skip-draft' action is gone; the 'wip' label still skips.
3) Address the 'OSS contributors cannot reopen a bot-closed PR' wrinkle.
GitHub does NOT let an external (non-write-access) contributor
reopen a PR that was closed by a bot or maintainer (long-standing
limitation). The original PR's close-comments told contributors to
'Reopen the PR — I'll re-evaluate automatically', which is broken
for the very audience this triage targets. Two changes:
a) Reword every close-comment (Greptile sweep + Agent Shin PR
close + Agent Shin issue close + PR template) to recommend:
- Open a new PR with the updated branch (primary path).
- Or comment '@agent-shin reconsider' on the closed PR for a
re-evaluation that, on pass, reopens the PR via the bot's
GH_TOKEN write access.
b) Add the @agent-shin reconsider workflow:
- .github/workflows/triage_reconsider.yml: new
'issue_comment'-triggered workflow. Authorizes only the
PR/issue author or an internal collaborator
(OWNER/MEMBER/COLLABORATOR), gated via a step output so
unauthorized commenters never reach the destructive steps.
Globally gated on AGENT_SHIN_ENABLED='true' (positive form,
matching the test_github_triage_workflows guardrail
patterns).
- triage_with_llm.py: --reconsider mode. On a closed PR/issue,
re-runs the LLM judge (or linked-issue regex short-circuit)
and:
- on pass: reopens via reopen_pr/reopen_issue + posts a
'Re-evaluated and reopened' comment.
- on fail: leaves closed and posts a 'still missing X'
comment so the contributor can iterate again.
Reconsider-on-open is a no-op ('skip-not-closed').
Internal-author + bot-account skips still take priority over
reconsider.
4) Greptile-on-closed-PRs question: the team asked whether Greptile can
re-review a closed PR. Greptile's docs don't address this and we
shouldn't promise behavior we can't verify, so the new close-comment
wording does NOT instruct contributors to 're-request greptile on
the closed PR'. Instead it points them at the new-PR path (which
Greptile definitely reviews) or the @agent-shin reconsider trigger
(which re-runs the LiteLLM-side rubric judge, not Greptile).
Tests: 93 passing (was 59).
- test_github_close_low_quality_prs.py: replaced 'skip drafts' test
with 'closes drafts when score is low' + 'closes brand-new PR when
min_age=0' + 'no skip when min_age=0'. The 'skip too young'
assertion is preserved as opt-in.
- test_github_triage_with_llm.py: 6 new TestTriageOrchestration cases
for reconsider mode (skip-not-closed on open, reopen on pass,
still-failing comment on fail, linked-issue short-circuit reopen,
skip internal author in reconsider, reopen-issue on pass) + a new
TestCloseCommentText class that pins the user-facing 'open a new
PR' + '@agent-shin reconsider' wording.
- test_github_triage_workflows.py: added triage_reconsider.yml to
the destructive-gate guardrail table; AGENT_SHIN_ENABLED is its
own destructive gate (no separate per-run flag needed).
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
The PR and issue Agent Shin workflows gated the destructive --close
flag with [ "${DISPATCH_CLOSE:-false}" != "false" ]. That pattern
treats anything other than the literal string "false" as enabling
closure — "True", "yes", "1", typos, accidental whitespace, etc.
The workflow_dispatch input UI is a 'true'/'false' choice dropdown so
the form is constrained, but the API (`gh workflow run -f close=...`)
accepts any string, and a CI cron / external invoker passing a
non-canonical truthy value would have silently enabled real
contributor PR closures.
Mirror the sibling Greptile closer's [ "${CLOSE_FLAG}" = "true" ]
pattern: only the EXACT string "true" enables --close; every other
value (including the unset/empty default) resolves to dry-run. This is
the fail-safe philosophy applied everywhere else in this PR.
Added tests/test_litellm/test_github_triage_workflows.py with two
parametrized invariants:
1. The destructive gate uses '= "true"' for its env-var
comparison (either bare '${ENV}' or '${ENV:-false}' form
accepted), and never the fail-open '!= "false"' pattern.
2. Every destructive gate is also gated on AGENT_SHIN_ENABLED being
"true" — either by entering the close branch on '=' or by
bailing out early on '!=' — so flipping the repo variable off is
a true kill switch regardless of per-run inputs.
Manually verified the test fails on the buggy '!= "false"' pattern and
passes on the fix, so it would have caught the regression at PR time.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Three independent bugbot findings against triage_with_llm.py:
1. LINKED_ISSUE_PATTERN included weak keywords (`see`, `ref`,
`addresses`) so casual mentions like "See #1234 for context" were
short-circuited to pass-linked-issue without ever calling the LLM —
contradicting the prompt's own "a bare issue number without a closing
keyword counts only if it's clearly the related issue (not a passing
mention)" rubric. Limit the regex to GitHub's documented PR-closing
keywords (fixes/fix/fixed/closes/close/closed/resolves/resolve/resolved).
2. is_internal_contributor() treated an empty/missing author_association
as external (eligible for the destructive close path), while the sibling
is_external_pr_author() in close_low_quality_prs.py fail-safes the same
case as internal. Align the two so a partial/unknown GitHub response can
never make a PR eligible for auto-close.
3. argparse `default=os.environ.get("TRIAGE_MODEL", DEFAULT_MODEL)` returns
the empty string when GitHub Actions exposes an unset repo variable as
an empty-string env var (the optional vars.TRIAGE_MODEL case in the
workflow). Use `os.environ.get(...) or DEFAULT_MODEL` so empty -> default,
matching the existing OPENAI_BASE_URL pattern.
Tests:
- Casual mentions now must fall through to the LLM (parametrized);
added an orchestration test ensuring "See #1234" reaches the judge.
- Empty/missing author_association now fails safe (parametrized).
- Empty TRIAGE_MODEL env var falls back to DEFAULT_MODEL; explicit
TRIAGE_MODEL is still honored.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
- Removed the unused gh_json helper (bugbot low-severity dead code).
- Replaced argparse `action="append", default=[...]` with default=None
+ DEFAULT_OPTOUT_LABELS fallback. The mutable-default + append combo
silently APPENDS to the canonical defaults instead of replacing them,
so --optout-label could not actually scope the opt-out list.
- Added tests covering both the canonical default and the
flag-replaces-defaults behavior.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
- Bump DEFAULT_MODEL from gpt-4o-mini to gpt-5.4-mini (more modern;
4M total context window per OpenAI catalog, JSON-schema response
format, function calling all supported).
- For gpt-5.x family models, pass reasoning_effort="none" via
extra_body. gpt-5.x rejects temperature != 1 unless reasoning_effort
is explicitly "none"; setting it lets us keep temperature=0 for
deterministic JSON rubric judgments. extra_body works across openai
SDK versions regardless of whether they natively type the kwarg.
- For non-gpt5 overrides (TRIAGE_MODEL=gpt-4o-mini etc.), reasoning_effort
is not sent.
- 4 new unit tests cover: gpt-5.4-mini -> reasoning_effort=none,
capitalized/dated gpt-5 variants -> reasoning_effort=none,
gpt-4o-mini -> no extra_body, base_url passthrough.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
- close_low_quality_prs.py: Treat author_association API lookup failures
as internal (fail-safe) so transient errors don't cause internal
contributors' PRs to be auto-closed.
- triage_with_llm.py: Update summary heading from 'Would post comment:'
to 'Posted comment:' since this branch only runs after the comment
has already been posted.
Co-authored-by: Yassin Kortam <yassin@berri.ai>
- close_low_quality_prs.yml: only workflow_dispatch with close=true (and
AGENT_SHIN_ENABLED=true) actually closes PRs. Scheduled runs are always
dry-run, matching the safety invariant documented for triage_pr/issue.
- triage_with_llm.py: textwrap.dedent on an f-string with multi-line
interpolated bodies fails because the body's 2nd+ lines start at column 0,
making the common-indent zero. Dedent the static template first, then
.format() the title/body in.
Co-authored-by: Yassin Kortam <yassin@berri.ai>
- close_low_quality_prs.py now filters by GitHub author_association via
the REST API: PRs from OWNER / MEMBER / COLLABORATOR (and bot accounts)
are skipped with a new 'skip-internal' summary bucket.
- close_low_quality_prs.yml now defaults workflow_dispatch close=false,
and ignores 'close=true' unless the new repo variable
AGENT_SHIN_ENABLED is set to 'true'. Scheduled runs are dry-run only
until the team flips that switch.
- Updated unit tests: one new test asserting internal authors are
skipped, and an autouse fixture treats unspecified test PRs as
external so the rest of the suite still exercises the close path.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Adds a new triage flow that evaluates external pull requests and issues
against the project's contribution rubric and, when configured to do so,
auto-closes non-conforming ones with an explanatory comment. Contributors
can update + reopen to be re-evaluated.
Scope:
- Internal BerriAI contributors (author_association OWNER/MEMBER/COLLABORATOR)
and bot accounts are skipped entirely.
- 'Fixes #1234' / 'Resolves https://github.com/.../issues/N' in the PR body
short-circuits to PASS without burning LLM tokens.
- LLM judge returns structured JSON (verdict, missing[], explanation);
parser tolerates markdown fences and embedded JSON.
- LLM errors NEVER close PRs/issues — failure surfaces as 'skip-llm-error'.
Safety:
- pull_request_target / issues triggers are FORCED dry-run in the workflow;
only manual workflow_dispatch with close=true (and AGENT_SHIN_ENABLED=true)
takes destructive action.
- Default mode writes verdicts to GITHUB_STEP_SUMMARY only — no public
comments until the team flips the AGENT_SHIN_ENABLED repo variable.
- LLM uses an OpenAI-compatible endpoint (model and base URL configurable
via repo variables; key via OPENAI_API_KEY secret).
Files:
- .github/scripts/triage_with_llm.py - judge orchestrator + CLI
- .github/workflows/triage_pr_with_llm.yml
- .github/workflows/triage_issue_with_llm.yml
- tests/test_litellm/test_github_triage_with_llm.py - 33 unit tests
End-to-end validated against four real PRs (#28117 internal collaborator,
#28108 bot, #28129 'Fixes #28128', #28116 no linked issue) and issue
#28132 with a stubbed LLM judge: each path produces the expected action.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
PR template:
- Make the rubric explicit at the top: link an issue, OR provide a clear
problem description + expected vs. actual + visual QA proof.
- Add dedicated sections for each piece so the bot has a deterministic
shape to read.
- Keep the existing 'Linear ticket' section for internal contributors
(they're exempt from the auto-triage rubric).
Bug report template:
- Split 'What happened?' into 'Actual behavior' + 'Expected behavior'.
- Make logs/screenshot a required textarea.
- Warning banner at the top tells external contributors that incomplete
reports will be auto-closed (with re-evaluation on reopen).
Feature request template:
- Require a concrete use case + example in the motivation field, not just
a one-liner pitch.
- Same auto-triage warning banner.
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
Adds .github/scripts/close_low_quality_prs.py and a daily workflow that
closes PRs which:
- are open for at least 7 days, and
- carry a most-recent greptile-apps review with Confidence Score <4/5,
- and are not drafts or opt-out-labeled ('do not close', 'wip', etc.).
Each closure posts an explanatory comment telling the contributor how to
bring the PR back (rebase, re-request greptile, reopen at 4+/5). The
4/5 bar is already documented in the PR template
(.github/pull_request_template.md), so this just enforces it.
Tested with a dry run against the live BerriAI/litellm backlog of 1000
open PRs: 100 candidates identified, 598 PRs pass the bar (4+/5), 186
are too young, 97 are drafts, 19 lack any Greptile review and are left
alone.
Workflow defaults to closing 25 PRs/run as a safety net and supports
workflow_dispatch with overrides (close=false for a dry run, custom
min_age_days/min_score/limit).
18 unit tests cover score extraction (HTML/markdown/plain text, login
variants, multi-review picks latest) and per-PR evaluation (drafts,
opt-out labels, age, missing/passing/failing scores).
Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
* fix(mcp-oauth): add PROXY_BASE_URL escape hatch + diagnostic logging for invalid_request
Customers hitting "{"detail":"invalid_request"}" on the MCP /authorize
endpoint had no way to recover when their ingress mangles X-Forwarded-*
headers (the same-origin check in validate_trusted_redirect_uri compares
the browser-supplied redirect_uri against get_request_base_url, which is
reconstructed from those headers).
Two contained changes:
1. get_request_base_url now honours PROXY_BASE_URL as the canonical
public origin when set, bypassing the X-Forwarded-* trust gate
entirely. Operators who know their public URL can set it once
instead of debugging ingress header rewrites.
2. The rejection path in validate_trusted_redirect_uri emits a WARN
log carrying the redirect_uri, computed proxy base, and the
X-Forwarded-* / Host headers seen. A bare 400 was undiagnosable;
this turns it into a one-line root-cause.
* test(mcp-oauth): capture warnings from correct logger ("LiteLLM")
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* fix(mcp-oauth): reject malformed PROXY_BASE_URL with one-shot diagnostic
A scheme-less PROXY_BASE_URL (e.g. "litellm.example.com" instead of
"https://litellm.example.com") would sail through urlparse with empty
scheme + netloc, silently breaking every same-origin compare in
validate_trusted_redirect_uri and leaving the operator staring at the
same opaque 400 the env var was meant to fix.
Validate it once at read time: only honour values that parse as
http(s) URLs with a non-empty netloc; otherwise log a one-shot WARN
naming the bad value and fall through to the request-derived origin
so the proxy still serves traffic.
* fix(mcp/oauth): normalize PROXY_BASE_URL to strip query/fragment
Match the X-Forwarded-* path's normalization so a configured
PROXY_BASE_URL containing a query string or fragment does not break
downstream f-string concatenation like f"{base_url}/callback".
Co-authored-by: Yassin Kortam <yassin@berri.ai>
* refactor(mcp-oauth): drop non-essential comments from PROXY_BASE_URL changes
Strip narrative comments and verbose docstrings added in this PR; the
code is intuitive enough on its own and the log messages already carry
their own diagnostic context. Pre-existing comments are left untouched.
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Yassin Kortam <yassin@berri.ai>
- Add AWS ECS Fargate stack with Aurora Postgres (IAM auth), ElastiCache Redis, S3, ALB with path-based routing to gateway/backend/ui components, Application Auto Scaling, and automated DB bootstrap + prisma migration via local-exec provisioners
- Add GCP Cloud Run stack with Cloud SQL Postgres (password auth), Memorystore Redis, GCS, external HTTPS load balancer with serverless NEGs and URL map routing, and automated prisma migration via Cloud Run Job
- Both stacks support typed proxy_config input mirroring the helm chart's gateway.config.proxy_config, per-component extra env vars, and Secret Manager references for provider API keys
- Gateway/backend services depend on terraform_data.migration so they never start before the schema is in place, eliminating crash-loop windows on first apply
- AWS stack uses IAM database authentication with a one-shot Fargate bootstrap task that creates and grants the rds_iam role to the application user; GCP stack uses password auth assembled at container startup to avoid Cloud SQL Auth Proxy sidecar complexity
- Add .gitignore rules for Terraform state files, plan files, tfvars inputs, provider binaries, and crash logs while explicitly keeping .terraform.lock.hcl for provider version pinning
- Include terraform.tfvars.example files, provider lock files, and comprehensive README documentation covering architecture, TLS setup, image pull strategies, and quick-start instructions for both stacks
Co-authored-by: Yassin Kortam <yassinkortam@g.ucla.edu>
* fix(proxy): sort BYOK models by team_public_model_name in /v2/model/info
Team BYOK rows persist an internal `model_name` like
`model_name_{team_id}_{uuid}` and expose the user-facing name via
`model_info.team_public_model_name`. The UI's `getDisplayModelName`
and the search filter already fall back to that field, but
`_sort_models` was keying off the raw `model_name` — so BYOK rows
ranked by their opaque IDs and clumped at the end of the alphabetized
list instead of interleaving with non-BYOK rows.
Match the UI/search behavior: prefer `team_public_model_name` when
present, fall back to `model_name` otherwise.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(proxy): case-insensitive DB-side search for BYOK models
`_apply_search_filter_to_models` used Prisma's JSON path
`string_contains` to match the BYOK `team_public_model_name` field, but
that operator is case-sensitive in Postgres (no `mode: insensitive`
flag like column-level string filters have). So a search for "claude"
missed a stored "Claude Sonnet" via the DB branch even though the
router-side path matched it case-insensitively.
Widen the JSON branch to "row has a team_public_model_name set" and
filter case-insensitively in Python so DB-only BYOK rows match the
same terms users see in the UI. This also drops the now-unused
DB-level page-size optimization and `sort_by` knob — the in-Python
filter is the source of truth for `db_models_total_count` now.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(proxy): scope BYOK search results to caller's accessible teams
`_apply_search_filter_to_models` was widened to fetch every row with a
`team_public_model_name` set so case-insensitive search could match
mixed-case stored names. `/v2/model/info` is reachable by non-admin
keys though, and the helper ran before `include_team_models` / `teamId`
filtering — so a non-admin caller could search a common substring like
"claude" and see BYOK rows belonging to teams they're not a member of.
Resolve the caller's team membership once (admin → no scoping, else
their `user_row.teams`) and drop BYOK rows (those with
`model_info.team_id` set) outside that scope on both the router-side
matches and the over-broad DB query, before display-name matching.
Non-team rows are unaffected and remain gated by the existing
`include_team_models` / `direct_access` paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(proxy): search by team_public_model_name and scope teamId queries
- /v2/model/info search now matches both `model_name` and
`model_info.team_public_model_name`, so team BYOK rows (which persist
an internal `model_name_{team_id}_{uuid}`) are findable by the public
name shown in the UI. DB query OR-includes a JSON-path match on
`team_public_model_name` for rows that exist only in the DB.
- `_filter_models_by_team_id` no longer short-circuits on the viewer's
`direct_access` flag — that describes the admin viewer's own
permissions and would leak every public model into a team-scoped view.
Models are kept only when they belong to the team (own BYOK, in
access_via_team_ids, or reachable via team.models / access groups).
- Added `_authorize_team_id_query`: the untrusted `teamId` query
parameter now requires the caller to be a proxy admin or a member of
the requested team, otherwise returns 403. Without this, any
authenticated user could enumerate another team's BYOK metadata by
guessing the team id.
- `_get_caller_byok_team_scope` now treats `PROXY_ADMIN_VIEW_ONLY` the
same as `PROXY_ADMIN` (both are admin roles); previously VIEW_ONLY
admins fell through to a user-id team lookup and saw only their own
teams' BYOK rows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(proxy): bound BYOK search DB fetch in /v2/model/info
Previously the DB-side search OR'd a JSON-path predicate
`{model_info: {path: [team_public_model_name], string_contains: ""}}`
to compensate for Prisma's case-sensitive JSON `string_contains` on
Postgres. That predicate matches every row that has any
`team_public_model_name` set, so any authenticated caller could force a
full BYOK-table read with `/v2/model/info?search=x` regardless of page
size.
Drop the JSON-path branch. The DB query now does a bounded
`model_name contains <search>` lookup. BYOK rows that are loaded into
the router are still searchable by their `team_public_model_name` via
the router-side filter; only the rare edge case of a BYOK row that
exists only in the DB (router sync failed) loses display-name search,
which is an acceptable trade-off given the DoS surface.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(proxy): bound DB find_many in /v2/model/info search
The previous bounding patch dropped the page-aware `take=N` on
`find_many`, so a broad `?search=model` would load and decrypt every
matching DB row on each request even though the response only returns
one page.
Restore bounded fetches in `_apply_search_filter_to_models`:
* Unsorted searches use `take = max(0, page * size - router_count)`,
i.e. exactly one page worth of remaining DB rows.
* Sorted searches need ordering across the full match set, so they cap
at `_SORTED_SEARCH_DB_FETCH_CAP = 500` instead of fetching everything.
* Total count comes from a cheap `count(...)` query so pagination stays
accurate without materializing every row.
Wired `page`, `size`, and `sortBy` through from the endpoint and added
a regression test covering both `take` values.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(proxy): extract DB-fetch helper to satisfy PLR0915
_apply_search_filter_to_models tripped Ruff's "too many statements"
(51 > 50) after the bounded-fetch fix. Move the DB-side block into
`_fetch_db_models_for_search`, which keeps the same behavior:
* Bounded `take` via page math (unsorted) or `_SORTED_SEARCH_DB_FETCH_CAP`
(sorted)
* Cheap `count(...)` for accurate pagination totals
* Caller-team scope applied to fetched rows before decrypt
Pure refactor; no behavior change. All 8 BYOK/team tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style: apply black formatting to _fetch_db_models_for_search
CI's "Check Black formatting" step flagged one line in the helper added
in d55eecf6af. No behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The proxy SERVER span ("Received Proxy Server Request") only carried
http.response.status_code on failures (set in _record_exception_on_span),
so success traces had no 2xx bucket — error-ratio and status-breakdown
dashboards were missing their denominator and the span violated the HTTP
semconv (the attribute is required whenever a response is sent). Add a
set_response_status_code_attribute helper and call it from
async_post_call_success_hook with 200, symmetric with the failure path
and the existing route/preprocessing-duration SERVER-span attributes.
* feat(model_catalog): add Azure AI Foundry GPT-5.4 model metadata
Register azure_ai GPT-5.4 variants with pricing, context limits from
Foundry catalog, and capability flags for cost routing and tooling.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(model_catalog): tighten Azure AI GPT-5.4 cost and capability metadata
Add supports_web_search for base GPT-5.4 aliases, priority-tier Pro rates,
and mini/nano above-272k plus priority pricing for correct spend math.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(model_catalog): sync web_search flag on Azure AI GPT-5.4 dated backup row
Mirror supports_web_search for azure_ai/gpt-5.4-2026-03-05 in the backup
catalog so it matches model_prices_and_context_window.json.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
openai 2.34.0 began rejecting an explicitly-passed empty-string api_key
at client construction (raises OpenAIError before any request), which
broke tests/local_testing/test_exceptions.py::test_exception_with_headers
and related cases after uv.lock floated openai 2.33.0 -> 2.36.0.
Pin back to 2.33.0 (within the existing pyproject >=2.20.0,<3.0.0 range)
as a temporary stopgap; longer-term fix to follow.
* feat(otel): expose http.response.status_code on failure spans
Set the OTel-standard http.response.status_code (integer) on failure
spans alongside the existing OpenInference error.code (kept for
back-compat). error.type is already emitted via ERROR_TYPE.
Crucially, also record structured error attributes on the proxy SERVER
span ('Received Proxy Server Request') from async_post_call_failure_hook
- the only place the SERVER span is in hand. _handle_failure records on
the litellm_request child span (the parent span is not propagated into
its kwargs), so prior to this change the SERVER span that dashboards
query carried only span status, never error.code/error.type. Reuses
_record_exception_on_span + StandardLoggingPayloadSetup.get_error_information
so values match the child span.
Tests: recorder unit coverage + a hook-driven test asserting the SERVER
span is stamped (the gap recorder-only tests missed). Full
test_opentelemetry.py suite: 197 passed.
* feat(otel): set http.route + url.path on the proxy SERVER span
Add the OTel-standard http.route (low-cardinality route template, e.g.
/v1/threads/{thread_id}/runs) and url.path (literal path) to the SERVER
span ('Received Proxy Server Request') so dashboards can group traffic
by endpoint instead of seeing every path param as a unique value.
Same architectural gap as the status-code commit: the success/failure
logging handlers write the litellm_request CHILD span, and
_handle_success explicitly refuses to copy to the SERVER span. Verified
with a console-exporter run that the SERVER span was bare on success.
Unlike error info, route/path are known at request time, so set them
directly on the freshly-created SERVER span in user_api_key_auth (one
edit point, works for success and failure, no hook-ordering risk):
- http.route from the matched FastAPI route (scope['route'].path),
empirically confirmed populated at auth-dependency time.
- url.path from the existing literal-path variable.
New get_request_route_template helper + set_proxy_request_route_attributes
(no-op on None span, so the Langfuse override stays safe).
Tests: route-attribute setter + route-template helper edges. Full
test_opentelemetry.py and test_auth_utils.py green.
* feat(otel): set litellm.preprocessing.duration_ms on the proxy SERVER span
Expose the total time LiteLLM spends before the upstream provider
request begins (auth + parsing + pre-call hooks) as a single number on
the SERVER span ('Received Proxy Server Request'). Window:
proxy-receive -> FIRST provider handoff.
Retry semantics: first attempt only (pure preprocessing, excludes
retry loops + backoff). api_call_start_time is overwritten on every
attempt, so a set-once first_api_call_start_time pins the first handoff.
Same architectural gap as the prior two commits: the success/failure
logging handlers write the litellm_request CHILD span, not the SERVER
span. Set it instead from the post-call hooks on
user_api_key_dict.parent_otel_span.
Failure-path subtlety: request_data.pop('litellm_logging_obj') runs
before the failure-hook loop, so the failure hook can't read the
logging object. litellm_received_at is propagated via the existing
request->metadata channel, and first_api_call_start_time is mirrored
onto litellm_params.metadata, so both anchors survive into request_data
and the OTel helper reads them uniformly for success and failure.
Edits: user_api_key_auth (stash receive instant), litellm_pre_call_utils
(propagate it), litellm_logging (set-once first handoff + metadata
mirror), opentelemetry (constant + set_preprocessing_duration_attribute,
called from both post-call hooks).
Tests: duration helper (both container shapes, missing/negative/None
edges) + set-once invariant (retry doesn't overwrite, metadata mirror).
test_opentelemetry.py + test_auth_utils.py + test_litellm_logging.py:
447 passed. Verified live: SERVER span carries the attribute on success
and failure, coexisting with the status-code and route attributes.
* fix(otel): MyPy type-narrowing for status-code + preprocessing-duration
No behavior change. MyPy (CI lint) flagged:
- error_information["error_code"] is str|None: narrow via a None-checked
local before int().
- _to_timestamp returns Optional[float]: resolve both anchors and return
early if either is None instead of subtracting possibly-None floats.
* fix(otel): stop polluting user request metadata with first_api_call_start_time
The PR3 set-once preprocessing anchor was mirrored into
litellm_params["metadata"] from core litellm_logging.py. That dict is
the caller's request metadata, mutated in place and shared across every
call path including pure SDK (litellm.acreate_batch). It got echoed into
LiteLLMBatch(metadata=...), which the OpenAI batch schema types as
Dict[str, str] -> pydantic ValidationError on a datetime value.
- litellm_logging.py: set first_api_call_start_time only on
model_call_details (success path reads it there directly).
- proxy/utils.py: post_call_failure_hook lifts it off the logging object
into request_data (internal top-level key, same convention as the
other proxy-internal request_data keys) right before the existing
litellm_logging_obj pop. Never touches user metadata.
- opentelemetry.py: read the anchor from the container top level
(model_call_details on success, request_data on failure).
- Tests updated; add TestPostCallFailureHookLiftsFirstApiCallStartTime.
Fixes the batches_testing regression introduced on this branch.
* chore(otel): trim verbose comments to concise rationale
Collapse multi-line why-blocks to one or two lines and drop process/plan references (PR-numbering, "the plan") from test comments. No behavior change.
* fix(ci): flag codecov uploads and enable carryforward
Coverage uploads from GHA and CircleCI were unflagged. Commits that
receive the push-triggered workflows more than once (re-runs, or branches
cut at the same SHA) accumulated many overlapping flagless sessions, and
Codecov's per-commit merge dropped the largest, ubiquitously-imported
files (router.py, proxy_server.py, main.py, utils.py, cost_calculator.py)
from the report even though the uploaded XMLs contained them.
- codecov.yaml: flag_management.default_rules.carryforward: true
- GHA reusable bases: tag each upload with its workflow/shard name
- CircleCI: tag the combined upload "circleci"; also combine the
agent / google_generate_content_endpoint / litellm_utils datafiles
that were produced and required but missing from the combine list
* fix(ci): close coverage gaps in proxy-legacy, router-unit, auth-ui, caching-redis
- test-unit-proxy-legacy: route through _test-unit-base so the full
proxy_unit_tests suite (incl. comprehensive test_proxy_server*.py) is
measured and uploaded with per-group flags (was plain pytest, no --cov)
- _test-unit-services-base: declare the enable-redis input + the six
secrets test-unit-caching-redis passes; that workflow had a workflow_call
signature mismatch and startup_failed on every push (never ran).
Changes are additive/optional - proxy-db and security callers unchanged
- circleci: add --cov + persist + combine + upload-coverage requires for
litellm_router_unit_testing (tests/router_unit_tests) and
auth_ui_unit_tests (tests/proxy_admin_ui_tests); neither was covered
anywhere. Redundant -k subset jobs left as-is (local_testing covers them)
* fix(ci): remove dead GHA Redis workflow; keep Redis on CircleCI only
CircleCI redis_caching_unit_tests already runs the exact same files
(tests/local_testing/test_dual_cache.py, test_redis_batch_optimizations.py,
test_router_utils.py) with --cov, and that datafile is already combined
and uploaded. The GHA test-unit-caching-redis workflow was redundant and
had never run (workflow_call signature mismatch -> startup_failure on
every push).
- Delete .github/workflows/test-unit-caching-redis.yml
- Revert _test-unit-services-base.yml to the flag-fix state (drop the
enable-redis input / secrets / env wiring added only to prop up the
GHA Redis workflow); the verified per-upload flags line is kept
- The only single-star "litellm_*" branch glob lived in the deleted
file; no other single-star globs exist, so none remain to widen
* fix(ci): keep proxy-legacy as a standalone job to preserve required check names
Routing proxy-legacy through the reusable workflow renamed each check from
the bare matrix name (e.g. "proxy-response-and-misc") to
"proxy-response-and-misc / Run tests". Those bare names are required status
checks in branch protection, so the old contexts never reported and PRs sat
"Expected — Waiting for status to be reported" indefinitely.
Restore the original standalone matrix job (job name == matrix name, so the
required contexts report again) and add coverage in place: --cov on pytest
plus an OIDC Codecov upload flagged proxy-legacy-<group>. Net effect of the
gap-#2 fix is preserved (flagged coverage for tests/proxy_unit_tests/**)
without changing any check name.
* revert(ci): drop all proxy-legacy changes from this PR
tests/proxy_unit_tests/** is already fully covered by test-unit-proxy-db
(its shard-coverage guard fails CI if any file in that dir is unassigned),
which this PR already flags + carryforwards. Adding --cov and id-token:write
to the legacy pull_request job was redundant and put OIDC on a job that runs
untrusted PR code. Restore the file to the base version verbatim so this PR
no longer touches proxy-legacy at all (also restores its original required
check names). Retiring proxy-legacy in favor of proxy-db on pull_request is
a separate effort that needs a branch-protection change.
Split the monolithic LiteLLM proxy into independently scalable Kubernetes components to allow separate horizontal scaling of the LLM data plane and management API surfaces
- Add DatabaseURLSettings pydantic-settings model that assembles DATABASE_URL (and optional DATABASE_URL_READ_REPLICA) from discrete DATABASE_* env vars before Prisma initializes, supporting both IAM token auth (minting short-lived RDS tokens) and password auth; replaces the CLI-only path that componentized entrypoints bypass
- Add gateway component (port 4000) that trims the proxy route table to the LLM data-plane surface (chat, embeddings, completions, audio, realtime, provider passthroughs, health/metrics) via an allowlist applied inside the lifespan context so plugin-registered routes are captured
- Add backend component (port 4001) that exposes the management/admin surface (keys, users, teams, orgs, spend analytics, model management, SSO, audit logs) with a complementary allowlist
- Add ui component — Next.js static export served by nginx (port 3000) with RSC payload routing, asset prefix aliasing, and SPA fallback for dashboard routes
- Add migrations component with dedicated Dockerfile that runs prisma migrate deploy via a Helm pre-install/pre-upgrade Job, eliminating per-pod schema contention on the Prisma advisory lock
- Add Helm chart (helm/litellm) with separate Deployments, Services, HPAs, and ConfigMap for each component; shared _helpers.tpl emits DATABASE_*, IAM_TOKEN_DB_AUTH, REDIS_*, and DISABLE_SCHEMA_UPDATE env vars from chart values; ingress template routes traffic to the correct component by path prefix
- Add comprehensive tests for DatabaseURLSettings covering IAM auth, password auth, read replica fallbacks, operator-pinned URL preservation, and percent-encoding; add coverage test asserting gateway + backend allowlist union equals the full proxy route set
- Add pydantic-settings>=2.14.1 as a proxy extra dependency and update liccheck allowlist
Co-authored-by: Yassin Kortam <yassinkortam@g.ucla.edu>
Resolve conflicts in the five unrelated CI-flake fixes I previously landed
on this branch -- staging shipped stronger versions (mocked HTTP for the
Fireworks tests, mocked image-fetch for the Gemini size-limit test, switched
the openapi-compliance test to the Interaction response schema instead of
dropping the assertion). Take staging's version of all five files and drop
my now-unreachable 429-skip lines from the Gemini test that the auto-merge
left behind.
Two P2 nits flagged by Greptile on PR 28036:
1. _build_completion_kwargs() defaulted vertex_project to "vertex-check-481318"
when VERTEX_PROJECT was unset. That value is a specific GCP project that
doesn't belong to this repo, so if the env-var skip guard were ever
bypassed (misconfig, direct helper call), the test would silently issue
calls to a foreign project rather than failing loudly. Drop the fallback
and read os.environ["VERTEX_PROJECT"] directly, mirroring how
AZURE_FOUNDRY_* are handled.
2. _build_messages_kwargs() was a one-liner that returned the result of
_build_completion_kwargs() unchanged -- a dead abstraction with one
caller. Inline at the _call_messages call site and delete the helper.
litellm.ImageFetchError is a subclass of BadRequestError, so when
Wikimedia returns 429 the pytest.raises(ImageFetchError) block matches
and swallows the exception -- the outer try/except never fires. Drop the
try/except and check the captured error message for "Status code: 429"
after the raises block, calling pytest.skip in that case. Same intent,
right control flow.
Four pre-existing flakes on main that gate this branch's workflow even
though they're unrelated to the reasoning_effort_grid suite:
1. tests/local_testing/test_completion.py::test_completion_fireworks_ai
2. tests/local_testing/test_completion_cost.py::test_completion_cost_fireworks_ai[fireworks_ai/llama-v3p3-70b-instruct]
3. tests/llm_translation/test_fireworks_ai_translation.py::test_document_inlining_example[False]
The Fireworks-hosted `llama-v3p3-70b-instruct` deployment is currently
returning 404 "Model not found, inaccessible, and/or not deployed".
These tests pass when the model is deployed; the issue is upstream
capacity, not our code path. Wrap the live call in a try/except that
pytest.skip's on litellm.NotFoundError so a Fireworks deployment hiccup
no longer fails CI for unrelated PRs.
4. tests/llm_translation/test_gemini.py::test_gemini_image_size_limit_exceeded
The test fetches the 32MB "Blue Marble 2002" image from Wikimedia to
exercise the 50MB image-size cap. CI runners share an IP pool with
noisy traffic, so Wikimedia routinely returns HTTP 429. The size-limit
check never gets a chance to fire. Catch the 429 BadRequestError and
pytest.skip in that case.
None of these belong on this PR conceptually, but they're included per
request to unblock the workflow before morning.
The anthropic_messages route wraps client-side BadRequestError as
AnthropicError (a BaseLLMException subclass) with status_code=400, so
"except BadRequestError" missed those cells and they fell through to the
generic Exception arm, returning 500 instead of the expected 400.
Replace the isinstance-on-BadRequestError check with a tiny classifier
that prefers BadRequestError membership, then falls back to the exception's
status_code attribute (set by every BaseLLMException subclass), then 500.
Apply to both _call_chat and _call_messages for consistency.
Fixes the 13 CircleCI llm_translation_testing failures on
bedrock_invoke_messages cells where the effort was disabled / invalid /
empty / xhigh-on-unsupported / max-on-unsupported.
Two CI failures, both pre-existing in different ways:
1. reasoning_effort_grid: all 33 bedrock_invoke_messages cells failed with
AttributeError("module 'litellm' has no attribute 'messages'"). litellm
exposes the async Anthropic Messages entrypoint as litellm.anthropic_messages
(via "from .llms.anthropic.experimental_pass_through.messages.handler
import *" in litellm/__init__.py), not litellm.messages.acreate. Swap
the call.
2. tests/test_litellm/interactions/test_openapi_compliance.py::TestResponseCompliance::test_interaction_response_fields
asserts the live Google spec contains "steps". Google's spec has churned
through "outputs" -> "steps" -> neither, and presently carries neither.
The test broke on main as soon as upstream dropped "steps"; pulling the
key off the assert list realigns the test with the live schema. Re-add
the per-turn output field once upstream stabilizes on a name.
The openapi-compliance fix doesn't belong to this PR conceptually but is
included here per request to unblock CI before the morning.
The Content-Length header check in _process_image_response rejects the
image before the body is streamed, so the mock body never needs to be
materialized. Use an empty body instead of b"x" * 100MB (addresses
greptile/cursor review feedback).
Mock the image fetch instead of downloading a 50MB+ image from
upload.wikimedia.org. The runner was intermittently rate-limited
(HTTP 429), so the code raised "Unable to fetch image ... Status
code: 429" and the size-limit assertions failed even though
pytest.raises(litellm.ImageFetchError) still matched.
Mirror the established LargeImageClient pattern in
tests/test_litellm/litellm_core_utils/test_image_handling.py: stub
litellm.module_level_client with a response whose Content-Length
exceeds the 50MB limit and bypass SSRF validation, so the
size-limit rejection path is exercised deterministically with no
external network dependency.
test_completion_fireworks_ai and test_completion_cost_fireworks_ai
made real Fireworks calls and broke whenever Fireworks rotated its
serverless catalog (no externally-verifiable model list exists).
They also asserted nothing — just printed.
Mock the HTTP post and assert real behavior instead: the request is
built with the right model/messages and the OpenAI-compatible
response parses back; the cost path yields a non-zero cost against
the local cost map. No network, no model dependency, stronger than
the old smoke checks.
The live deepseek-v3p1 call kept hitting Fireworks NOT_FOUND because
Fireworks rotates its serverless catalog and no externally-verifiable
list exists. The [False] branch also never sent an image, so it only
proved the model responded.
Mock the HTTP post (mirrors test_global_disable_flag_with_transform_
messages_helper) and assert the real behavior: #transform=inline is
appended to the PDF URL unless disabled. No network, no model
dependency, and stronger coverage than the old live test.
Fireworks removed llama-v3p3-70b-instruct from serverless, so every
live test using it now fails with NotFoundError ("Model not found,
inaccessible, and/or not deployed").
Swap the 6 references (3 files) to the currently-served
accounts/fireworks/models/deepseek-v3p1 — the canonical model in
Fireworks' current docs examples and present in LiteLLM's cost map.
test_get_model_params_fireworks_ai is a pure pricing-heuristic test
(no network) asserting the >16b branch, so it uses llama-v3p1-70b-
instruct instead to keep the "fireworks-ai-above-16b" assertion and
branch coverage intact.
Google restructured the live spec at ai.google.dev/static/api/
interactions.openapi.json: the output-only fields (notably the
`steps` array, formerly `outputs`) moved off the request schema
`CreateModelInteractionParams` onto a dedicated `Interaction`
response schema. The response-side tests still read from the
request schema, so `test_interaction_response_fields` failed with
"Output field 'steps' not in spec".
Point `test_interaction_response_fields` and `test_status_enum_values`
at the `Interaction` schema (the semantic response object; all output
fields incl. `steps` present there). Request-side tests keep using
`CreateModelInteractionParams` (all request fields verified still
present). 13/13 pass against the current live spec.
- Drop the "v4" suffix throughout: it referred to the QA sweep iteration,
not this test suite. There's only one regression suite, so just call it
reasoning_effort_grid.
- Move tests/test_litellm/reasoning_effort_grid_v4/ -> tests/llm_translation/
reasoning_effort_grid/. Two reasons:
1. The parent tests/test_litellm/conftest.py installs an autouse fixture
(isolate_host_aws_config) that clears every AWS_* env var before each
test, which would silently skip every Bedrock cell.
2. tests/llm_translation/conftest.py already wires up the Redis-backed
VCR persister and auto-applies @pytest.mark.vcr to every collected
item via apply_vcr_auto_marker_to_items. Living under that conftest
means the suite gets cassette replay for free -- first CI run with
provider creds records 231 cassettes, every subsequent run replays
them with no live spend.
- Trim the suite's own conftest down to just the wire_capture fixture; the
inherited llm_translation conftest covers the VCR plumbing.
- Drop the dedicated reasoning_effort_grid_v4_e2e CircleCI job. The existing
llm_translation_testing job globs tests/llm_translation/**/test_*.py, so
the suite is gated by an existing job with no new wiring.
The runtime _validate_effort_for_model allows effort='max' for any
Claude 4.6 model (opus or sonnet), and model_prices_and_context_window
sets supports_max_reasoning_effort: true for claude-sonnet-4-6. The
grid spec previously gave sonnet-4-6 entries _CAPS_NONE, so expected()
returned status=400 for effort='max', which mismatched the runtime's
status=200 and caused 6 cells (one per route) to fail.
Rename _CAPS_OPUS_4_6 to _CAPS_4_6 (since the cap set is shared by
opus and sonnet 4.6) and assign it to all sonnet-4-6 entries.
Co-authored-by: Yassin Kortam <yassin@berri.ai>
