fix(backend/chat): Address remaining PR review comments

- Fix tool_call_id always being "sdk-call" by generating unique IDs per invocation
- Fix validation using original tool_name instead of clean_name in security hooks
- Fix duplicate StreamFinish in Anthropic fallback path
- Fix ImportError fallback returning plain dict instead of re-raising
- Extract _build_input_schema helper to deduplicate schema construction
- Add else branch for unhandled SDK message types for observability
- Truncate large tool results in conversation history to prevent context overflow
This commit is contained in:
Zamil Majdy
2026-02-08 19:39:10 +04:00
parent b9c759ce4f
commit 7592deed63
6 changed files with 3879 additions and 3053 deletions

View File

@@ -241,6 +241,9 @@ class SDKResponseAdapter:
)
responses.append(StreamFinish())
else:
logger.debug(f"Unhandled SDK message type: {class_name}")
return responses
def create_heartbeat(self, tool_call_id: str | None = None) -> StreamHeartbeat:

View File

@@ -254,12 +254,12 @@ def create_strict_security_hooks(
},
)
# Run standard validations
result = _validate_tool_access(tool_name, tool_input)
# Run standard validations using clean_name for consistent checks
result = _validate_tool_access(clean_name, tool_input)
if result:
return cast(SyncHookJSONOutput, result)
result = _validate_user_isolation(tool_name, tool_input, user_id)
result = _validate_user_isolation(clean_name, tool_input, user_id)
if result:
return cast(SyncHookJSONOutput, result)

View File

@@ -174,8 +174,11 @@ def _format_conversation_history(session: ChatSession) -> str:
f" [Called tool: {func.get('name', 'unknown')}]"
)
elif msg.role == "tool":
# Pass full tool results - SDK handles compaction
history_parts.append(f" [Tool result: {msg.content or ''}]")
# Truncate large tool results to avoid blowing context window
tool_content = msg.content or ""
if len(tool_content) > 500:
tool_content = tool_content[:500] + "... (truncated)"
history_parts.append(f" [Tool result: {tool_content}]")
history_parts.append("</conversation_history>")
history_parts.append("")
@@ -428,6 +431,8 @@ async def stream_chat_completion_sdk(
async for response in stream_with_anthropic(
session, system_prompt, text_block_id
):
if isinstance(response, StreamFinish):
stream_completed = True
yield response
# Save the session with accumulated messages
@@ -435,10 +440,10 @@ async def stream_chat_completion_sdk(
logger.debug(
f"[SDK] Session {session_id} saved with {len(session.messages)} messages"
)
# Always yield StreamFinish to signal completion to the caller
# The adapter yields StreamFinish for the SSE stream, but we need to
# yield it here so the background task in routes.py knows to call mark_task_completed
yield StreamFinish()
# Yield StreamFinish to signal completion to the caller (routes.py)
# Only if one hasn't already been yielded by the stream
if not stream_completed:
yield StreamFinish()
except Exception as e:
logger.error(f"[SDK] Error: {e}", exc_info=True)

View File

@@ -6,6 +6,7 @@ into in-process MCP tools that can be used with the Claude Agent SDK.
import json
import logging
import uuid
from contextvars import ContextVar
from typing import Any
@@ -78,10 +79,12 @@ def create_tool_handler(base_tool: BaseTool):
try:
# Call the existing tool's execute method
# Generate unique tool_call_id per invocation for proper correlation
effective_id = tool_call_id or f"sdk-{uuid.uuid4().hex[:12]}"
result = await base_tool.execute(
user_id=user_id,
session=session,
tool_call_id=tool_call_id or "sdk-call",
tool_call_id=effective_id,
**args,
)
@@ -121,6 +124,15 @@ def create_tool_handler(base_tool: BaseTool):
return tool_handler
def _build_input_schema(base_tool: BaseTool) -> dict[str, Any]:
"""Build a JSON Schema input schema for a tool."""
return {
"type": "object",
"properties": base_tool.parameters.get("properties", {}),
"required": base_tool.parameters.get("required", []),
}
def get_tool_definitions() -> list[dict[str, Any]]:
"""Get all tool definitions in MCP format.
@@ -133,11 +145,7 @@ def get_tool_definitions() -> list[dict[str, Any]]:
tool_def = {
"name": tool_name,
"description": base_tool.description,
"inputSchema": {
"type": "object",
"properties": base_tool.parameters.get("properties", {}),
"required": base_tool.parameters.get("required", []),
},
"inputSchema": _build_input_schema(base_tool),
}
tool_definitions.append(tool_def)
@@ -183,11 +191,7 @@ def create_copilot_mcp_server():
decorated = tool(
tool_name,
base_tool.description,
{
"type": "object",
"properties": base_tool.parameters.get("properties", {}),
"required": base_tool.parameters.get("required", []),
},
_build_input_schema(base_tool),
)(handler)
sdk_tools.append(decorated)
@@ -202,13 +206,8 @@ def create_copilot_mcp_server():
return server
except ImportError:
logger.warning(
"claude-agent-sdk not available, returning tool definitions only"
)
return {
"tools": get_tool_definitions(),
"handlers": get_tool_handlers(),
}
# Let ImportError propagate so service.py handles the fallback
raise
# List of tool names for allowed_tools configuration

File diff suppressed because it is too large Load Diff

View File

@@ -22,7 +22,7 @@ cryptography = "^45.0"
discord-py = "^2.5.2"
e2b-code-interpreter = "^1.5.2"
elevenlabs = "^1.50.0"
fastapi = "^0.116.1"
fastapi = "^0.128.0"
feedparser = "^6.0.11"
flake8 = "^7.3.0"
google-api-python-client = "^2.177.0"
@@ -36,7 +36,7 @@ jinja2 = "^3.1.6"
jsonref = "^1.1.0"
jsonschema = "^4.25.0"
langfuse = "^3.11.0"
launchdarkly-server-sdk = "^9.12.0"
launchdarkly-server-sdk = "^9.14.1"
mem0ai = "^0.1.115"
moviepy = "^2.1.2"
ollama = "^0.5.1"
@@ -53,8 +53,8 @@ prometheus-client = "^0.22.1"
prometheus-fastapi-instrumentator = "^7.0.0"
psutil = "^7.0.0"
psycopg2-binary = "^2.9.10"
pydantic = { extras = ["email"], version = "^2.11.7" }
pydantic-settings = "^2.10.1"
pydantic = { extras = ["email"], version = "^2.12.5" }
pydantic-settings = "^2.12.0"
pytest = "^8.4.1"
pytest-asyncio = "^1.1.0"
python-dotenv = "^1.1.1"
@@ -66,11 +66,11 @@ sentry-sdk = {extras = ["anthropic", "fastapi", "launchdarkly", "openai", "sqlal
sqlalchemy = "^2.0.40"
strenum = "^0.4.9"
stripe = "^11.5.0"
supabase = "2.17.0"
supabase = "2.27.2"
tenacity = "^9.1.2"
todoist-api-python = "^2.1.7"
tweepy = "^4.16.0"
uvicorn = { extras = ["standard"], version = "^0.35.0" }
uvicorn = { extras = ["standard"], version = "^0.40.0" }
websockets = "^15.0"
youtube-transcript-api = "^1.2.1"
yt-dlp = "2025.12.08"