google / google/adk-python

BigQueryAgentAnalyticsPlugin records error-bearing tool results as TOOL_COMPLETED with status OK

Đang mở
#7,112 3 bình luận 0 reaction 2 người được giao Được @haiyuan-eng-google nhận Xem trên GitHub
bq needs review
Ngôn ngữ chính
Python
Star
21.5k
Fork
4k
Merge trung bình
1 ngày 22 giờ
Pull request đã merge (30 ngày)
31

Mô tả

**Describe the bug**

`BigQueryAgentAnalyticsPlugin` writes `TOOL_ERROR` (with `status = 'ERROR'` and `error_message`) only from `on_tool_error_callback`, that is, only when a tool raises and no plugin registered ahead of it answers the error. `after_tool_callback` writes `TOOL_COMPLETED` with `status = 'OK'` for whatever the tool returned, without looking at it. Two common failures therefore land as successes:

1. **An MCP tool whose server answers `isError: true`.** `McpTool` returns the `CallToolResult` as a result dict (`_dump_mcp_model`, which restores `isError`) instead of raising, which is the MCP protocol's shape for a tool-level failure. The row is `TOOL_COMPLETED`, `status = 'OK'`, `error_message = NULL`; the failure only shows in `content.result.isError`.
2. **A tool that raises while `ReflectAndRetryToolPlugin` is registered ahead of the analytics plugin.** `PluginManager._run_callbacks` stops at the first plugin that returns a value, so the reflect-and-retry response becomes the tool's result and reaches `after_tool_callback`: `TOOL_COMPLETED`, `status = 'OK'`, with the error in `content.result.error_details`. `ToolboxToolset` tools raise on failure, so a Toolbox error lands here.

An audit, report or alert built on `status`, `error_message` or `TOOL_ERROR` reads both failures as successes.

**To Reproduce**

google-adk 2.9.0; no BigQuery is needed, because the rows are captured by replacing `_log_event`. The MCP case uses a `FunctionTool` that returns the dict `McpTool` returns for `isError: true` — the plugin never inspects the result, so the recorded row is the same.

```python
import asyncio
from unittest.mock import AsyncMock

from google.adk.agents.llm_agent import LlmAgent
from google.adk.apps import App
from google.adk.models.base_llm import BaseLlm
from google.adk.models.llm_response import LlmResponse
from google.adk.plugins import ReflectAndRetryToolPlugin
from google.adk.plugins.bigquery_agent_analytics_plugin import BigQueryAgentAnalyticsPlugin
from google.adk.runners import InMemoryRunner
from google.adk.tools import FunctionTool
from google.genai import types

def mcp_style_failure() -> dict:
return {"content": [{"type": "text", "text": "Git reset to remote failed."}], "isError": True}

def raising_tool() -> str:
raise RuntimeError("MCP request failed with code 403: Access Denied")

def build(with_reflect: bool):
rows = []
analytics = BigQueryAgentAnalyticsPlugin(project_id="p", dataset_id="d", table_id="events")

async def record(event_type, callback_context, raw_content=None, is_truncated=False, event_data=None):
if event_type.startswith("TOOL_"):
rows.append((event_type, event_data.status if event_data else "OK", (raw_content or {}).get("tool")))

analytics._log_event = record
analytics._ensure_started = AsyncMock(return_value="ok")
analytics.flush = AsyncMock()
turns = [[types.Part(function_call=types.FunctionCall(name="mcp_style_failure", args={}, id="c0"))]]
if with_reflect: # without a plugin that answers the error, the raise ends the run
turns.append([types.Part(function_call=types.FunctionCall(name="raising_tool", args={}, id="c1"))])
turns.append([types.Part(text="done")])

class Scripted(BaseLlm):
async def generate_content_async(self, llm_request, stream=False):
yield LlmResponse(content=types.Content(role="model", parts=turns.pop(0)))

agent = LlmAgent(name="repro", model=Scripted(model="scripted"), instruction="x",
tools=[FunctionTool(mcp_style_failure), FunctionTool(raising_tool)])
plugins = ([ReflectAndRetryToolPlugin(max_retries=3)] if with_reflect else []) + [analytics]
return InMemoryRunner(app=App(name="repro", root_agent=agent, plugins=plugins)), rows

async def main():
for with_reflect in (False, True):
runner, rows = build(with_reflect)
await runner.session_service.create_session(app_name="repro", user_id="u", session_id="s")
async for _ in runner.run_async(user_id="u", session_id="s",
new_message=types.Content(role="user", parts=[types.Part(text="go")])):
pass
print(f"ReflectAndRetryToolPlugin ahead={with_reflect}:", rows)

asyncio.run(main())
```

Output:

```
ReflectAndRetryToolPlugin ahead=False: [('TOOL_STARTING', 'OK', 'mcp_style_failure'), ('TOOL_COMPLETED', 'OK', 'mcp_style_failure')]
ReflectAndRetryToolPlugin ahead=True: [('TOOL_STARTING', 'OK', 'mcp_style_failure'), ('TOOL_COMPLETED', 'OK', 'mcp_style_failure'), ('TOOL_STARTING', 'OK', 'raising_tool'), ('TOOL_COMPLETED', 'OK', 'raising_tool')]
```

**Expected behavior**

An error-bearing result is recorded as a failure — `TOOL_ERROR`, or `status = 'ERROR'` with a non-null `error_message` — while the tool's result still reaches the model unchanged. Two shapes ADK itself produces would cover both cases above: an MCP result with `isError: true`, and a `ReflectAndRetryToolPlugin` response (`response_type == REFLECT_AND_RETRY_RESPONSE_TYPE`). A way for an application to classify a result (for example a callable in `BigQueryLoggerConfig` returning a status and a message) would also cover application-specific shapes.

**Related, minor**

`_get_tool_origin` classifies `McpTool`, `TransferToAgentTool`, `AgentTool` and `FunctionTool`, and returns `UNKNOWN` for everything else. The tools of ADK's own `ToolboxToolset` (`toolbox_adk.ToolboxTool`, a `BaseTool`) and the `SkillToolset` tools therefore land as `UNKNOWN`, so the table cannot separate BigQuery calls from skill loads by origin.

**Desktop**

- OS: macOS
- Python version: 3.12
- ADK version: 2.9.0. On `main` (checked 2026-09-13) the plugin still has no reference to `isError` or to the Toolbox.

**Model Information:** not applicable; the reproduction uses a scripted `BaseLlm`.

**Additional context**

Searched existing issues for `BigQueryAgentAnalyticsPlugin isError`, `BigQueryAgentAnalyticsPlugin TOOL_ERROR`, `tool_origin UNKNOWN` and `ToolboxToolset analytics` and found none. The nearest precedent is #5073 (closed), a `tool_origin` classification gap for `RemoteA2aAgent`. We currently work around it with a subclass that classifies the result in `after_tool_callback` and writes the row through the plugin's `_log_event`, which depends on private helpers.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.