Repository navigation
test(e2e): add a codex row that calls an mcp tool whose name is over 64 characters - #49826
Draft
ryan-crabbe-berri wants to merge 9 commits into
Draft
ryan-crabbe-berri wants to merge 9 commits into
ryan-crabbe-berri wants to merge 9 commits into
Conversation
…64 characters Claude on Bedrock is a strict xfail because LiteLLM passes the namespaced tool name to Bedrock uncapped and Bedrock rejects names over 64 characters (LIT-10289)
Contributor
Author
|
@greptileai re review |
Contributor
Author
|
bugbot run |
Contributor
[Low impact] The PR appears safe to merge SummaryAdds a Codex test for an MCP tool whose name exceeds 64 characters, using the existing five-provider setup
No actionable new issues found. No live tests were run during this review Reviews (2) · Last reviewed commit: "test(e2e): check the long mcp tool name ..." · Reviewed by Greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…o litellm_e2e_codex_long_mcp_tool_name_review
Contributor
Author
|
@greptileai re review |
Contributor
Author
|
bugbot run |
Contributor
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3a1fab8. Configure here.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Handwritten 1-3 sentence description
<make-me-human-readable>This adds a Codex test where Codex calls a tool with a very long name on a small local MCP server through the gateway. Bedrock refuses tool names over 64 characters and the gateway passes them through as is, so every Codex turn on Claude on Bedrock fails once such a tool is set up. The test marks that one setup as an expected failure and goes red once the fix lands
</make-me-human-readable>Details
TLDR
User Flow
Before: a Codex user on Claude on Bedrock adds an MCP server with a long tool name and every turn fails
[mcp_servers.vault]in Codex'sconfig.tomlwhose tool name is 78 characters, and point Codex at the gatewaycodexon a Claude on Bedrock model and ask it to call theread_the_current_secret_value_from_the_vault_that_this_codex_session_was_giventoolmcp__vault__read_the_current_secret_value_...must have length less than or equal to 64After: nothing changes for that user yet, but the suite now runs this flow on every setup and will flip to a pass once the gateway shortens long names for Bedrock
Relevant issues
Guards LIT-10289, also reported by a customer in Pylon #11252. The open fixes are #42963 and #46243
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or moreDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
This PR only adds a test, so the proof is the new row running live plus the same flow driven by hand in the Codex TUI. The gateway runs main's code on port 47330 with a Postgres DB and real OpenAI, Azure (
gpt-6.1-soldeployment) and Bedrock (us-east-1) credentials. Each test registers its own deployment through/model/new, drives the real Codex CLI 0.165.0 and deletes the deployment afterBefore (9ea4531)
Codex TUI
The row does not exist on the base branch, so nothing checks a long MCP tool name through the gateway
With a key scoped to a fresh Claude on Bedrock deployment,
config.tomlpointing aresponsesprovider at the gateway and[mcp_servers.vault]runningtests/e2e/codex/mcp_server.pywithTOOL_NAMEset to the 78 character name,codex --model <claude deployment> "<prompt>"in a tmux pane shows (captured withtmux capture-pane -p)After (3a1fab8)
Codex TUI
1ede38e08676f47cNew row
bash run_e2e.sh <worktree> tests/e2e/codex/test_long_mcp_tool_name_e2e.py -n 5 -rA -q --reruns 0, which runsuv run --no-sync pytestfrom the worktree withLITELLM_PROXY_URL=http://127.0.0.1:47330and the gateway's master key, three full runs in a rowAll three runs had the same per-setup result. The passing setups only pass when Codex's
mcp_tool_callitem names the 78 character tool, its result is exactly the random value, the value is in the final answer and Codex exits cleanly--runxfail -vvfails on the Bedrock 400 this PR guardsTOOL_NAMEType
✅ Test
Caveats (if any)
Low
QA runbook
bedrock/us.anthropic.claude-sonnet-5-5withaws_region_name: us-east-1) and make a key scoped to it with POST /key/generateconfig.tomlat the gateway as aresponsesprovider and add[mcp_servers.vault]runningpython tests/e2e/codex/mcp_server.pywithenv = { SECRET_FILE = "<path>", TOOL_NAME = "read_the_current_secret_value_from_the_vault_that_this_codex_session_was_given" }anddefault_tools_approval_mode = "approve"toolSpec.namebeing over 64 characters, and on the GPT setups expect the tool call to return the value and the same value in Codex's replyFinal Attestation
Note
Low Risk
Test and documentation only; no production gateway or routing code changes.
Overview
Adds a Codex compatibility row that drives the real
codex execCLI through the gateway to call a local stdio MCP tool whose name exceeds Bedrock Converse’s 64-character limit (78 characters), asserting the tool output appears inMcpCallrecords and the final answer.mcp_server.pynow registers the tool with@server.tool(name=os.environ.get("TOOL_NAME"))so the long-name case can set the name via env; existing MCP rows keep using the short name whenTOOL_NAMEis omitted.claude_bedrockis marked strict xfail (LIT-10289): the gateway forwards the full prefixed name and Bedrock returns 400. GPT setups are expected to pass; fixing name truncation should flip that xfail to a hard fail.tests/e2e/AGENTS.mddocuments the new row and fixes thecodex/suite bullet (it was incorrectly nested undercli_harness/).Reviewed by Cursor Bugbot for commit 3a1fab8. Bugbot is set up for automated code reviews on this repo. Configure here.