mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(backends/litellm): drop oversized tool names before Bedrock Converse (#2129)
## Description The Bedrock Converse API hard-rejects any request containing a tool name over 64 characters (`toolConfig.tools.N.member.toolSpec.name`). Claude Code includes every globally-added claude.ai MCP connector tool in every request it sends, even connectors the user hasn't enabled locally. One org-wide connector with a 65-char tool name is enough to fail every single request routed through this backend's Bedrock path, with no way to remove or disable the connector client-side. Direct Bedrock mode (`CLAUDE_CODE_USE_BEDROCK=1`, bypassing this proxy) is unaffected: it hits Bedrock's native Anthropic-compatible endpoint, which has no such length limit. Only the Converse API, which this LiteLLM-backed `bedrock` provider path uses, enforces it. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/backends/litellm.py`: `send_message` and `stream_message` both filter tools with names over 64 characters out of the payload before converting/forwarding, but only for `self.provider == "bedrock"`. Other providers are untouched. - `tests/test_backend_bugs.py`: new `TestBedrockOversizedToolNameFiltering` covering both `send_message` and `stream_message` — an oversized (65-char) name is dropped on `bedrock`, a name at exactly the 64-char boundary is kept, and non-`bedrock` providers forward oversized names unfiltered (the limit is a Bedrock Converse constraint, not a general one). - `CHANGELOG.md`: added a `### Fixed` entry under `Unreleased`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ uv run pytest tests/test_backend_bugs.py tests/test_backend_anyllm.py -q ============================= test session starts ============================== platform darwin -- Python 3.13.13, pytest-9.0.3, pluggy-1.6.0 collected 57 items tests/test_backend_bugs.py .......................................... [ 73%] tests/test_backend_anyllm.py ............... [100%] ============================== 57 passed in 1.42s ============================== $ uv run ruff check headroom/backends/litellm.py tests/test_backend_bugs.py All checks passed! $ uv run mypy headroom/backends/litellm.py Success: no issues found in 1 source file ``` ## Real Behavior Proof - **Environment:** personal fork deployed as a real proxy (macOS launchd service, `headroom install apply`) with `--backend bedrock --mode token --code-aware --bedrock-profile sso-bedrock`, fronting a live Claude Code session with a globally-added-but-not-locally-enabled claude.ai MCP connector (`TopCounsel`) whose tool name is 65 characters. - **Exact command / steps:** run any Claude Code request through this deployment while the org-wide `TopCounsel` connector is present (it is included in the tool list on every request regardless of local enablement). - **Observed result:** before the fix, every request failed with a LiteLLM `BedrockException`: `1 validation error detected: Value 'mcp__claude_ai_TopCounsel_by_The_L_Suite__complete_authentication' at 'toolConfig.tools.N.member.toolSpec.name' failed to satisfy constraint: Member must have length less than or equal to 64`. After applying the fix (filtering the oversized tool out before the LiteLLM call), the same session proceeds normally with no validation error, confirmed live against this deployment. - **Not tested:** truncating the name instead of dropping it was tried and discarded during investigation — the model echoes the truncated name back in `tool_use` blocks, and Claude Code matches tool calls by the original full name, so truncation breaks routing on the return path. This PR drops the tool entirely rather than truncating, which is why it is not present as an alternative in the diff. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A — backend logic change, no UI surface. ## Additional Notes - "I have made corresponding changes to the documentation" is unchecked: no file in `docs/`, `README.md`, or `CONTRIBUTING.md` documents backend-specific tool-list filtering behavior, so there is no existing section to update. - No linked issue number: this was found via independent investigation of a personal deployment (a live Bedrock validation failure), not filed as a `headroomlabs-ai/headroom` issue first. Checked `gh pr list`/`gh issue list` for existing coverage of "Bedrock Converse 64-char tool name" and found none open or merged. - A native Bedrock Anthropic-compatible endpoint backend (avoiding Converse's tool-name limit entirely) would be the more complete long-term fix, but is out of scope for this PR. --------- Co-authored-by: Ingmar Krusch <ingmar.krusch@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
This commit is contained in:
Ingmar Krusch
GitHub
parent
2976d49f18
commit
3c1a5cdb1c
3 changed files with 169 additions and 2 deletions
|
|
@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
## Unreleased
|
||||
|
||||
### Fixed
|
||||
- **backends/litellm:** drop tool names over 64 chars before calling Bedrock Converse (`send_message` and `stream_message`), instead of letting the whole request 401. The Bedrock Converse API hard-rejects any tool name past that length, and Claude Code includes every globally-added claude.ai MCP connector tool in every request, even ones the user hasn't enabled locally, so a single oversized connector name broke every call through this backend. Only the `bedrock` provider filters; other providers forward tool names unfiltered.
|
||||
- **memory:** annotate `_EMBEDDER_CACHE` as `dict[tuple[str, str, str], Embedder]` to match the 3-element key (backend, model, ollama_base_url). The stale 2-tuple annotation made `mypy headroom` fail on `main`, which broke the `lint` CI job on every open PR.
|
||||
- **install:** include `orjson` in the `[proxy]` extra so `uv tool install "headroom-ai[all]"` satisfies LiteLLM OpenRouter/provider backends that import it at runtime ([#2056](https://github.com/headroomlabs-ai/headroom/issues/2056)).
|
||||
- The dashboard's per-request metadata (the `recent_requests` / `request_logs`
|
||||
|
|
|
|||
|
|
@ -790,7 +790,14 @@ class LiteLLMBackend(Backend):
|
|||
|
||||
# Tools (convert Anthropic format to OpenAI format)
|
||||
if "tools" in body:
|
||||
kwargs["tools"] = [_convert_anthropic_tool(t) for t in body["tools"]]
|
||||
tools_in = body["tools"]
|
||||
# Bedrock Converse API hard-rejects tool names over 64 chars.
|
||||
# Claude Code injects every globally-added claude.ai MCP connector
|
||||
# tool into every request, even disabled ones; a single oversized
|
||||
# name 401s the whole call. Drop them before conversion instead.
|
||||
if self.provider == "bedrock":
|
||||
tools_in = [t for t in tools_in if len(t.get("name", "")) <= 64]
|
||||
kwargs["tools"] = [_convert_anthropic_tool(t) for t in tools_in]
|
||||
if "tool_choice" in body:
|
||||
kwargs["tool_choice"] = _convert_tool_choice(body["tool_choice"])
|
||||
|
||||
|
|
@ -900,7 +907,12 @@ class LiteLLMBackend(Backend):
|
|||
if "stop_sequences" in body:
|
||||
kwargs["stop"] = body["stop_sequences"]
|
||||
if "tools" in body:
|
||||
kwargs["tools"] = [_convert_anthropic_tool(t) for t in body["tools"]]
|
||||
tools_in = body["tools"]
|
||||
# Bedrock Converse API hard-rejects tool names over 64 chars.
|
||||
# See send_message for the full rationale; same filter here.
|
||||
if self.provider == "bedrock":
|
||||
tools_in = [t for t in tools_in if len(t.get("name", "")) <= 64]
|
||||
kwargs["tools"] = [_convert_anthropic_tool(t) for t in tools_in]
|
||||
if "tool_choice" in body:
|
||||
kwargs["tool_choice"] = _convert_tool_choice(body["tool_choice"])
|
||||
if "system" in body:
|
||||
|
|
|
|||
|
|
@ -820,3 +820,157 @@ class TestBedrockApiKeyNotForwarded:
|
|||
kwargs["api_key"] = headers["x-api-key"]
|
||||
|
||||
assert "api_key" not in kwargs
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Bedrock Converse Oversized Tool Name Filtering
|
||||
# =============================================================================
|
||||
|
||||
|
||||
class TestBedrockOversizedToolNameFiltering:
|
||||
"""Bedrock Converse hard-rejects any request containing a tool name over
|
||||
64 chars. Claude Code includes every globally-added claude.ai MCP
|
||||
connector tool in every request, even disabled ones, so a single
|
||||
oversized name would 401 the whole call. Tools over the limit must be
|
||||
dropped before the LiteLLM call, only for the ``bedrock`` provider.
|
||||
"""
|
||||
|
||||
def _make_response(self):
|
||||
mock_response = MagicMock()
|
||||
mock_response.choices = [
|
||||
MagicMock(message=MagicMock(content="ok", tool_calls=None), finish_reason="stop")
|
||||
]
|
||||
mock_response.usage = MagicMock(prompt_tokens=10, completion_tokens=5)
|
||||
return mock_response
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_message_drops_oversized_tool_name_on_bedrock(self):
|
||||
with (
|
||||
patch("headroom.backends.litellm.acompletion", new_callable=AsyncMock) as mock_acomp,
|
||||
patch("headroom.backends.litellm._fetch_bedrock_inference_profiles", return_value={}),
|
||||
):
|
||||
mock_acomp.return_value = self._make_response()
|
||||
|
||||
backend = LiteLLMBackend(provider="bedrock", region="us-west-2")
|
||||
body = {
|
||||
"model": "claude-3-5-sonnet-20241022",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"tools": [
|
||||
{"name": "short_tool", "input_schema": {"type": "object"}},
|
||||
{"name": "x" * 65, "input_schema": {"type": "object"}},
|
||||
],
|
||||
}
|
||||
|
||||
await backend.send_message(body, {})
|
||||
|
||||
call_kwargs = mock_acomp.call_args[1]
|
||||
names = [t["function"]["name"] for t in call_kwargs["tools"]]
|
||||
assert names == ["short_tool"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_message_keeps_exactly_64_chars_on_bedrock(self):
|
||||
with (
|
||||
patch("headroom.backends.litellm.acompletion", new_callable=AsyncMock) as mock_acomp,
|
||||
patch("headroom.backends.litellm._fetch_bedrock_inference_profiles", return_value={}),
|
||||
):
|
||||
mock_acomp.return_value = self._make_response()
|
||||
|
||||
backend = LiteLLMBackend(provider="bedrock", region="us-west-2")
|
||||
name_64 = "y" * 64
|
||||
body = {
|
||||
"model": "claude-3-5-sonnet-20241022",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"tools": [{"name": name_64, "input_schema": {"type": "object"}}],
|
||||
}
|
||||
|
||||
await backend.send_message(body, {})
|
||||
|
||||
call_kwargs = mock_acomp.call_args[1]
|
||||
names = [t["function"]["name"] for t in call_kwargs["tools"]]
|
||||
assert names == [name_64]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_message_does_not_filter_on_non_bedrock(self):
|
||||
"""The 64-char limit is a Bedrock Converse API constraint; other
|
||||
providers must forward oversized tool names unfiltered."""
|
||||
with (
|
||||
patch("headroom.backends.litellm.acompletion", new_callable=AsyncMock) as mock_acomp,
|
||||
patch("headroom.backends.litellm._fetch_bedrock_inference_profiles", return_value={}),
|
||||
):
|
||||
mock_acomp.return_value = self._make_response()
|
||||
|
||||
backend = LiteLLMBackend(provider="openrouter")
|
||||
oversized = "z" * 65
|
||||
body = {
|
||||
"model": "claude-3-5-sonnet-20241022",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"tools": [{"name": oversized, "input_schema": {"type": "object"}}],
|
||||
}
|
||||
|
||||
await backend.send_message(body, {})
|
||||
|
||||
call_kwargs = mock_acomp.call_args[1]
|
||||
names = [t["function"]["name"] for t in call_kwargs["tools"]]
|
||||
assert names == [oversized]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stream_message_drops_oversized_tool_name_on_bedrock(self):
|
||||
async def mock_stream():
|
||||
chunk = MagicMock()
|
||||
chunk.choices = [
|
||||
MagicMock(delta=MagicMock(content="hi", tool_calls=None), finish_reason="stop")
|
||||
]
|
||||
yield chunk
|
||||
|
||||
with (
|
||||
patch("headroom.backends.litellm.acompletion", new_callable=AsyncMock) as mock_acomp,
|
||||
patch("headroom.backends.litellm._fetch_bedrock_inference_profiles", return_value={}),
|
||||
):
|
||||
mock_acomp.return_value = mock_stream()
|
||||
|
||||
backend = LiteLLMBackend(provider="bedrock", region="us-west-2")
|
||||
body = {
|
||||
"model": "claude-3-5-sonnet-20241022",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"tools": [
|
||||
{"name": "short_tool", "input_schema": {"type": "object"}},
|
||||
{"name": "w" * 65, "input_schema": {"type": "object"}},
|
||||
],
|
||||
}
|
||||
|
||||
events = [event async for event in backend.stream_message(body, {})]
|
||||
assert events # sanity: stream produced output
|
||||
|
||||
call_kwargs = mock_acomp.call_args[1]
|
||||
names = [t["function"]["name"] for t in call_kwargs["tools"]]
|
||||
assert names == ["short_tool"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stream_message_does_not_filter_on_non_bedrock(self):
|
||||
async def mock_stream():
|
||||
chunk = MagicMock()
|
||||
chunk.choices = [
|
||||
MagicMock(delta=MagicMock(content="hi", tool_calls=None), finish_reason="stop")
|
||||
]
|
||||
yield chunk
|
||||
|
||||
with (
|
||||
patch("headroom.backends.litellm.acompletion", new_callable=AsyncMock) as mock_acomp,
|
||||
patch("headroom.backends.litellm._fetch_bedrock_inference_profiles", return_value={}),
|
||||
):
|
||||
mock_acomp.return_value = mock_stream()
|
||||
|
||||
backend = LiteLLMBackend(provider="openrouter")
|
||||
oversized = "v" * 65
|
||||
body = {
|
||||
"model": "claude-3-5-sonnet-20241022",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"tools": [{"name": oversized, "input_schema": {"type": "object"}}],
|
||||
}
|
||||
|
||||
events = [event async for event in backend.stream_message(body, {})]
|
||||
assert events
|
||||
|
||||
call_kwargs = mock_acomp.call_args[1]
|
||||
names = [t["function"]["name"] for t in call_kwargs["tools"]]
|
||||
assert names == [oversized]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue