Skip to content

fix(sensor): attribute MCP servers and failed tool calls in Claude Code sessions #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aglanio wants to merge 1 commit into
base: main
Choose a base branch
from
+138 −4
Open

fix(sensor): attribute MCP servers and failed tool calls in Claude Code sessions #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions Sensor/adr_sensor/parsers/claude_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__<server>__<tool>``, 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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Comment on lines +275 to +280

@pengyuzhang pengyuzhang Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

status="unknown" is provably wrong for empty non-error results.

A tool_result block is the completion signal, and is_error is authoritative — anything with falsy is_error succeeded, whether or not its content normalizes to a non-empty string.

Measured against ~1,300 real tool calls in ~/.claude/projects: 12 come out as unknown, all of them successful ToolSearch calls whose result content normalizes to "", each carrying an explicit is_error: false in the log. Collapsing the branch also matches the stated point of the PR:

Suggested change
if is_error:
status = "error"
elif result:
status = "success"
else:
status = "unknown"
status = "error" if is_error else "success"

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":
Comment on lines 290 to 291

@pengyuzhang pengyuzhang Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result merge two lines below matches by value, not identity — and this PR makes the misattribution worse.

ToolUsage is @dataclass(frozen=True), so two distinct pending calls with the same name/type/arguments compare equal while both still have result=None. if t == old_tool then hits whichever comes first, not the one this result belongs to.

Concrete failure: an assistant message issues two parallel Bash calls with the same command, and their tool_results complete out of order (the erroring one first). The new status="error" and error=… land on the call that actually succeeded, and the success lands on the one that failed. Before this PR that only swapped result strings; now it mislabels which call failed. Value-identical repeated calls do occur in practice — 4 identical ToolSearch invocations in a ~1,300-call sample.

The sibling parser already fixes exactly this, at claude_desktop_parser.py:428:

# Match on identity: two identical calls in different messages compare
# equal, and only the one this result belongs to should change.
if tool is not old_tool:
    continue

Separately, the inner break exits only the for idx, t in enumerate(msg.tools) loop, not the enclosing for msg in entry.chat_history, so a single result can be applied in more than one message.

Expand All @@ -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")

@pengyuzhang pengyuzhang Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A tool_use block with "name": null now drops the entire session.

.get("name", "unknown") only defaults when the key is absent — a present-but-null name yields None, and _classify_tool(None) raises AttributeError on None.startswith("mcp__"). That escapes to the broad except Exception in _create_entry_from_extracted_session, which prints and returns None, so the whole session — every user prompt and every other tool call in it — is silently discarded from telemetry. On main the same input parsed fine, since tool_name=None was simply stored.

Reproduced locally: the parser prints [CLAUDE] Error creating entry for session s1 and returns [].

Suggested change
tool_name = tool_data.get("name", "unknown")
tool_name = tool_data.get("name") or "unknown"

An isinstance(tool_name, str) guard at the top of _classify_tool would also cover the other call sites.

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,
)
Expand Down
98 changes: 98 additions & 0 deletions Sensor/tests/test_parsers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down