mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-04 02:31:27 +00:00
fix(skills): execute DB skills by matching the litellm_skill_ tool name prefix (#30116)
Skill IDs are generated as litellm_skill_<uuid> and the model-facing tool name is the sanitized skill ID, but the post-call execution gates in SkillsInjectionHook only ran tools whose name starts with "skill_", so DB skills were silently returned to the client as raw tool calls. Fixes #28122. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
217cedc988
commit
168d809bfa
5 changed files with 90 additions and 17 deletions
|
|
@ -18,7 +18,7 @@ flowchart TB
|
|||
F[Request with container.skills] --> G[SkillsInjectionHook]
|
||||
G --> H{skill_id prefix?}
|
||||
|
||||
H -->|"litellm:skill_abc"| I[Fetch from LiteLLM DB]
|
||||
H -->|"litellm_skill_abc"| I[Fetch from LiteLLM DB]
|
||||
H -->|"skill_xyz" no prefix| J[Pass to Anthropic as native skill]
|
||||
|
||||
I --> K{Model provider?}
|
||||
|
|
@ -57,7 +57,7 @@ sequenceDiagram
|
|||
|
||||
Note over LiteLLM,PreHook: PRE-CALL HOOK
|
||||
LiteLLM->>PreHook: Intercept request
|
||||
PreHook->>PreHook: Fetch skill from DB (litellm:skill_id)
|
||||
PreHook->>PreHook: Fetch skill from DB (litellm_skill_id)
|
||||
PreHook->>PreHook: Extract SKILL.md from ZIP
|
||||
PreHook->>PreHook: Inject SKILL.md into system prompt
|
||||
PreHook->>PreHook: Add litellm_code_execution tool
|
||||
|
|
@ -105,7 +105,7 @@ response = await litellm.acompletion(
|
|||
model="gpt-4o-mini",
|
||||
messages=[{"role": "user", "content": "Create a bouncing ball GIF"}],
|
||||
container={
|
||||
"skills": [{"type": "custom", "skill_id": "litellm:skill_abc123"}]
|
||||
"skills": [{"type": "custom", "skill_id": "litellm_skill_abc123"}]
|
||||
},
|
||||
)
|
||||
|
||||
|
|
@ -261,7 +261,7 @@ response = litellm.completion(
|
|||
messages=[{"role": "user", "content": "Analyze this data..."}],
|
||||
container={
|
||||
"skills": [
|
||||
{"type": "custom", "skill_id": "litellm:skill_abc123"} # litellm: prefix
|
||||
{"type": "custom", "skill_id": "litellm_skill_abc123"} # litellm_skill_ prefix
|
||||
]
|
||||
}
|
||||
)
|
||||
|
|
@ -277,7 +277,7 @@ response = litellm.completion(
|
|||
"messages": [{"role": "user", "content": "Help me analyze data"}],
|
||||
"container": {
|
||||
"skills": [
|
||||
{"type": "custom", "skill_id": "litellm:skill_abc123"}
|
||||
{"type": "custom", "skill_id": "litellm_skill_abc123"}
|
||||
]
|
||||
}
|
||||
}
|
||||
|
|
@ -287,7 +287,7 @@ response = litellm.completion(
|
|||
|
||||
The hook (`litellm/proxy/hooks/litellm_skills/main.py`) intercepts the request:
|
||||
|
||||
1. **Detects `litellm:` prefix** → Fetches skill from database
|
||||
1. **Detects `litellm_skill_` prefix** → Fetches skill from database
|
||||
2. **Checks model provider** → Bedrock is not Anthropic
|
||||
3. **Extracts SKILL.md** from stored ZIP file
|
||||
4. **Converts skill to tool** + **Injects content into system prompt**
|
||||
|
|
@ -361,8 +361,8 @@ model LiteLLM_SkillsTable {
|
|||
| Create skill on Anthropic | `anthropic` | N/A | Forward to Anthropic API |
|
||||
| Create skill in LiteLLM DB | `litellm_proxy` | N/A | Store in database |
|
||||
| Use Anthropic native skill | N/A | `skill_xyz` | Pass to Anthropic container.skills |
|
||||
| Use LiteLLM skill on Anthropic | N/A | `litellm:skill_abc` | Convert to tools |
|
||||
| Use LiteLLM skill on Bedrock/OpenAI | N/A | `litellm:skill_abc` | Convert to tools + inject SKILL.md |
|
||||
| Use LiteLLM skill on Anthropic | N/A | `litellm_skill_abc` | Convert to tools |
|
||||
| Use LiteLLM skill on Bedrock/OpenAI | N/A | `litellm_skill_abc` | Convert to tools + inject SKILL.md |
|
||||
|
||||
## Testing
|
||||
|
||||
|
|
|
|||
|
|
@ -4,6 +4,10 @@ Constants for LiteLLM Skills
|
|||
Centralized constants for skills processing, code execution, and sandbox configuration.
|
||||
"""
|
||||
|
||||
LITELLM_SKILL_ID_PREFIX: str = "litellm_skill_"
|
||||
"""Prefix for DB-backed skill IDs. The model-facing tool name is the skill ID
|
||||
with hyphens/spaces replaced by underscores, which leaves this prefix intact."""
|
||||
|
||||
# Code execution loop settings
|
||||
DEFAULT_MAX_ITERATIONS: int = 10
|
||||
"""Maximum number of iterations for the automatic code execution loop."""
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ from typing import Any, Dict, List, Optional
|
|||
|
||||
from litellm._logging import verbose_logger
|
||||
from litellm.caching.in_memory_cache import InMemoryCache
|
||||
from litellm.llms.litellm_proxy.skills.constants import LITELLM_SKILL_ID_PREFIX
|
||||
from litellm.proxy._types import LiteLLM_SkillsTable, NewSkillRequest, UserAPIKeyAuth
|
||||
from litellm.proxy.common_utils.resource_ownership import (
|
||||
get_primary_resource_owner_scope,
|
||||
|
|
@ -68,7 +69,7 @@ class LiteLLMSkillsHandler:
|
|||
) -> LiteLLM_SkillsTable:
|
||||
prisma_client = await LiteLLMSkillsHandler._get_prisma_client()
|
||||
|
||||
skill_id = f"litellm_skill_{uuid.uuid4()}"
|
||||
skill_id = f"{LITELLM_SKILL_ID_PREFIX}{uuid.uuid4()}"
|
||||
owner = get_primary_resource_owner_scope(user_api_key_dict) or user_id
|
||||
if owner is None:
|
||||
# Identity-less callers (no user_id / team_id / org_id /
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ Usage:
|
|||
response = await litellm.acompletion(
|
||||
model="gpt-4o-mini",
|
||||
messages=[{"role": "user", "content": "Create a bouncing ball GIF"}],
|
||||
container={"skills": [{"skill_id": "litellm:skill_abc123"}]},
|
||||
container={"skills": [{"skill_id": "litellm_skill_abc123"}]},
|
||||
)
|
||||
# Response includes file_ids for generated files
|
||||
"""
|
||||
|
|
@ -31,6 +31,7 @@ from typing import Any, Dict, List, Optional, Union
|
|||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.caching.caching import DualCache
|
||||
from litellm.integrations.custom_logger import CustomLogger
|
||||
from litellm.llms.litellm_proxy.skills.constants import LITELLM_SKILL_ID_PREFIX
|
||||
from litellm.llms.litellm_proxy.skills.prompt_injection import (
|
||||
SkillPromptInjectionHandler,
|
||||
)
|
||||
|
|
@ -43,7 +44,7 @@ class SkillsInjectionHook(CustomLogger):
|
|||
Pre/Post-call hook that processes skills from container.skills parameter.
|
||||
|
||||
Pre-call (async_pre_call_hook):
|
||||
- Skills with 'litellm:' prefix are fetched from LiteLLM DB
|
||||
- Skills with 'litellm_skill_' prefix are fetched from LiteLLM DB
|
||||
- For Anthropic models: native skills pass through, LiteLLM skills converted to tools
|
||||
- For non-Anthropic models: LiteLLM skills are converted to tools + execute_code tool
|
||||
|
||||
|
|
@ -78,7 +79,7 @@ class SkillsInjectionHook(CustomLogger):
|
|||
Process skills from container.skills before the LLM call.
|
||||
|
||||
1. Check if container.skills exists in request
|
||||
2. Separate skills by prefix (litellm: vs native)
|
||||
2. Separate skills by prefix (litellm_skill_ vs native)
|
||||
3. Fetch LiteLLM skills from database
|
||||
4. For Anthropic: keep native skills in container
|
||||
5. For non-Anthropic: convert LiteLLM skills to tools, inject content, add execute_code
|
||||
|
|
@ -108,7 +109,7 @@ class SkillsInjectionHook(CustomLogger):
|
|||
continue
|
||||
|
||||
skill_id = skill.get("skill_id", "")
|
||||
if skill_id.startswith("litellm_"):
|
||||
if skill_id.startswith(LITELLM_SKILL_ID_PREFIX):
|
||||
# Fetch from LiteLLM DB
|
||||
db_skill = await self._fetch_skill_from_db(
|
||||
skill_id,
|
||||
|
|
@ -287,7 +288,7 @@ class SkillsInjectionHook(CustomLogger):
|
|||
Fetch a skill from the LiteLLM database.
|
||||
|
||||
Args:
|
||||
skill_id: The skill ID (without 'litellm:' prefix)
|
||||
skill_id: The skill ID (including the 'litellm_skill_' prefix)
|
||||
|
||||
Returns:
|
||||
LiteLLM_SkillsTable or None if not found
|
||||
|
|
@ -382,10 +383,10 @@ class SkillsInjectionHook(CustomLogger):
|
|||
has_executable_tool = False
|
||||
for tc in tool_calls:
|
||||
tool_name = tc.get("name", "")
|
||||
# Execute if it's litellm_code_execution OR a skill tool (skill_xxx)
|
||||
# Execute if it's litellm_code_execution OR a skill tool (litellm_skill_xxx)
|
||||
if (
|
||||
tool_name == LiteLLMInternalTools.CODE_EXECUTION.value
|
||||
or tool_name.startswith("skill_")
|
||||
or tool_name.startswith(LITELLM_SKILL_ID_PREFIX)
|
||||
):
|
||||
has_executable_tool = True
|
||||
break
|
||||
|
|
@ -543,7 +544,7 @@ class SkillsInjectionHook(CustomLogger):
|
|||
result = await self._execute_code(
|
||||
code, skill_files, executor, generated_files
|
||||
)
|
||||
elif tool_name.startswith("skill_"):
|
||||
elif tool_name.startswith(LITELLM_SKILL_ID_PREFIX):
|
||||
# Skill tool - execute the skill's code
|
||||
result = await self._execute_skill_tool(
|
||||
tool_name, tool_input, skill_files, executor, generated_files
|
||||
|
|
|
|||
67
tests/test_litellm/proxy/hooks/litellm_skills/test_main.py
Normal file
67
tests/test_litellm/proxy/hooks/litellm_skills/test_main.py
Normal file
|
|
@ -0,0 +1,67 @@
|
|||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from litellm.proxy.hooks.litellm_skills.main import SkillsInjectionHook
|
||||
|
||||
SKILL_TOOL_NAME = "litellm_skill_e2b8dca8_031a_4481_b034_b9ec7d4eb7bf"
|
||||
|
||||
|
||||
def _request_data():
|
||||
return {
|
||||
"model": "claude-sonnet-4-5",
|
||||
"messages": [{"role": "user", "content": "run the skill"}],
|
||||
"litellm_metadata": {
|
||||
"_litellm_code_execution_enabled": True,
|
||||
"_skill_files": {SKILL_TOOL_NAME: {"main.py": b"print('hi')"}},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def _tool_use_response(tool_name):
|
||||
return {
|
||||
"stop_reason": "tool_use",
|
||||
"content": [
|
||||
{"type": "tool_use", "id": "toolu_1", "name": tool_name, "input": {}}
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_post_call_success_hook_executes_litellm_skill_tool():
|
||||
"""DB skill tool names carry the litellm_skill_ prefix and must trigger the execution loop."""
|
||||
hook = SkillsInjectionHook()
|
||||
response = _tool_use_response(SKILL_TOOL_NAME)
|
||||
|
||||
with patch.object(
|
||||
hook, "_execute_code_loop_messages_api", new=AsyncMock(return_value=response)
|
||||
) as mock_loop:
|
||||
result = await hook.async_post_call_success_deployment_hook(
|
||||
request_data=_request_data(), response=response, call_type=None
|
||||
)
|
||||
|
||||
mock_loop.assert_awaited_once()
|
||||
assert result is response
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_execute_code_loop_dispatches_litellm_skill_tool():
|
||||
"""The agentic loop must route litellm_skill_ tool calls to _execute_skill_tool."""
|
||||
hook = SkillsInjectionHook()
|
||||
final_response = {"stop_reason": "end_turn", "content": []}
|
||||
|
||||
with (
|
||||
patch.object(
|
||||
hook, "_execute_skill_tool", new=AsyncMock(return_value="skill ran")
|
||||
) as mock_exec,
|
||||
patch("litellm.anthropic.acreate", new=AsyncMock(return_value=final_response)),
|
||||
):
|
||||
result = await hook._execute_code_loop_messages_api(
|
||||
data=_request_data(),
|
||||
response=_tool_use_response(SKILL_TOOL_NAME),
|
||||
skill_files={"main.py": b"print('hi')"},
|
||||
)
|
||||
|
||||
mock_exec.assert_awaited_once()
|
||||
assert mock_exec.await_args.args[0] == SKILL_TOOL_NAME
|
||||
assert result is final_response
|
||||
Loading…
Add table
Reference in a new issue