From 83bda7ce95a8547fd472d8a414e37d056da649af Mon Sep 17 00:00:00 2001 From: aglanio Date: Mon, 10 Aug 2026 19:19:45 -0300 Subject: [PATCH] fix(sensor): attribute MCP servers and failed tool calls in Claude Code sessions The Claude Code parser hardcoded `tool_type="tool_use"` for every tool call, never populated `server_name`, and dropped the `is_error` flag from tool results. On a real corpus of 1,814 sessions / 90,626 tool calls this meant: - every call landed in a single `tool_use` bucket, so terminal commands were indistinguishable from MCP calls; - no MCP server attribution at all, even though Claude Code namespaces MCP tools as `mcp____` and the server is recoverable from the name; - zero errors reported across all 90,626 calls, because `is_error` was never read. After the fix, the same corpus yields `terminal_command` 48,008 / `tool_use` 24,617 / `mcp_tool` 18,001, 15 distinct MCP servers, and 5,327 failed calls (5.9%). Failed and blocked tool calls matter for detection, so losing them silently is a meaningful gap. This mirrors what `opencode_parser._classify_tool()` already does for opencode; the Claude Code parser had simply not received the same treatment. Also preserves `server_name` across the tool_use -> tool_result merge, which previously dropped it. Adds three tests: tool classification, server attribution surviving the result merge, and `is_error` producing `status="error"`. --- Sensor/adr_sensor/parsers/claude_parser.py | 44 +++++++++- Sensor/tests/test_parsers.py | 98 ++++++++++++++++++++++ 2 files changed, 138 insertions(+), 4 deletions(-) diff --git a/Sensor/adr_sensor/parsers/claude_parser.py b/Sensor/adr_sensor/parsers/claude_parser.py index 856de38..da22506 100644 --- a/Sensor/adr_sensor/parsers/claude_parser.py +++ b/Sensor/adr_sensor/parsers/claude_parser.py @@ -67,6 +67,24 @@ def parse_all(self) -> List[AgentEvent]: return entries + def _classify_tool(self, tool_name: str) -> tuple: + """Classify a Claude Code tool call and attribute MCP tools to their server. + + Claude Code namespaces MCP tools as ``mcp____``, so the server is + recoverable from the name alone. Without this, every call — including third-party + MCP servers — collapses into a single ``tool_use`` bucket with no server attribution. + """ + if tool_name.startswith("mcp__"): + parts = tool_name.split("__") + if len(parts) >= 3 and parts[1]: + return "mcp_tool", parts[1] + return "mcp_tool", None + + if tool_name in ("Bash", "PowerShell", "BashOutput", "KillShell"): + return "terminal_command", None + + return "tool_use", None + def _normalize_result_content(self, result_content: Any) -> str: """Normalize result content which can be a string or list of content items.""" if isinstance(result_content, str): @@ -186,7 +204,13 @@ def _extract_message_data(self, obj: Dict[str, Any]) -> Optional[Dict[str, Any]] if result_content and isinstance(result_content, str): result_content = truncate_middle(result_content, max_length=1000, edge_chars=400) - tool_results.append({"tool_use_id": tool_use_id, "result": result_content}) + tool_results.append( + { + "tool_use_id": tool_use_id, + "result": result_content, + "is_error": bool(item.get("is_error")), + } + ) if tool_results: extracted["tool_results"] = tool_results @@ -245,14 +269,23 @@ def _create_entry_from_extracted_session( for tool_result in tool_results: tool_use_id = tool_result.get("tool_use_id") result = tool_result.get("result") + is_error = tool_result.get("is_error", False) if tool_use_id in pending_tools: old_tool = pending_tools[tool_use_id] + if is_error: + status = "error" + elif result: + status = "success" + else: + status = "unknown" updated_tool = ToolUsage( tool_name=old_tool.tool_name, tool_type=old_tool.tool_type, + server_name=old_tool.server_name, arguments=old_tool.arguments, result=result, - status="success" if result else "unknown", + status=status, + error=result if is_error else None, ) for msg in entry.chat_history: if msg.role == "assistant": @@ -274,9 +307,12 @@ def _create_entry_from_extracted_session( tools = [] for tool_data in msg_data.get("tools", []): + tool_name = tool_data.get("name", "unknown") + tool_type, server_name = self._classify_tool(tool_name) tool = ToolUsage( - tool_name=tool_data.get("name", "unknown"), - tool_type="tool_use", + tool_name=tool_name, + tool_type=tool_type, + server_name=server_name, arguments=tool_data.get("input", {}), result=None, ) diff --git a/Sensor/tests/test_parsers.py b/Sensor/tests/test_parsers.py index 4112a14..a9037bb 100644 --- a/Sensor/tests/test_parsers.py +++ b/Sensor/tests/test_parsers.py @@ -76,6 +76,104 @@ def test_parse_jsonl_file(self, tmp_path): assert entry.model == "claude-sonnet-4-20250514" assert len(entry.chat_history) >= 1 + def test_classify_tool(self): + """MCP tools are attributed to their server; shell tools are terminal commands.""" + parser = ClaudeParser() + + assert parser._classify_tool("mcp__jarvis__ssh_run") == ("mcp_tool", "jarvis") + assert parser._classify_tool("mcp__chrome-devtools__navigate") == ( + "mcp_tool", + "chrome-devtools", + ) + assert parser._classify_tool("Bash") == ("terminal_command", None) + assert parser._classify_tool("PowerShell") == ("terminal_command", None) + assert parser._classify_tool("Read") == ("tool_use", None) + + def test_mcp_tool_keeps_server_after_result_merge(self, tmp_path): + """Server attribution survives the tool_use -> tool_result merge.""" + jsonl_file = tmp_path / "mcp.jsonl" + messages = [ + { + "type": "assistant", + "sessionId": "session1", + "timestamp": "2025-06-15T10:00:00Z", + "message": { + "model": "claude-sonnet-4-20250514", + "content": [ + { + "type": "tool_use", + "id": "tool1", + "name": "mcp__github__create_issue", + "input": {"title": "hi"}, + } + ], + }, + }, + { + "type": "user", + "sessionId": "session1", + "timestamp": "2025-06-15T10:00:01Z", + "message": { + "content": [ + {"type": "tool_result", "tool_use_id": "tool1", "content": "created"} + ] + }, + }, + ] + with open(jsonl_file, "w") as f: + for msg in messages: + f.write(json.dumps(msg) + "\n") + + entries = ClaudeParser().parse_jsonl_file(jsonl_file) + tools = [t for e in entries for m in e.chat_history for t in m.tools] + + assert len(tools) == 1 + assert tools[0].tool_type == "mcp_tool" + assert tools[0].server_name == "github" + assert tools[0].status == "success" + + def test_failed_tool_result_is_marked_error(self, tmp_path): + """A tool_result flagged is_error is recorded as an error, not a success.""" + jsonl_file = tmp_path / "error.jsonl" + messages = [ + { + "type": "assistant", + "sessionId": "session1", + "timestamp": "2025-06-15T10:00:00Z", + "message": { + "model": "claude-sonnet-4-20250514", + "content": [ + {"type": "tool_use", "id": "tool1", "name": "Bash", "input": {"command": "false"}} + ], + }, + }, + { + "type": "user", + "sessionId": "session1", + "timestamp": "2025-06-15T10:00:01Z", + "message": { + "content": [ + { + "type": "tool_result", + "tool_use_id": "tool1", + "content": "permission denied", + "is_error": True, + } + ] + }, + }, + ] + with open(jsonl_file, "w") as f: + for msg in messages: + f.write(json.dumps(msg) + "\n") + + entries = ClaudeParser().parse_jsonl_file(jsonl_file) + tools = [t for e in entries for m in e.chat_history for t in m.tools] + + assert len(tools) == 1 + assert tools[0].status == "error" + assert tools[0].error == "permission denied" + def test_parse_empty_file(self, tmp_path): """Test parsing an empty file.""" jsonl_file = tmp_path / "empty.jsonl"