Repository navigation
Add web fetch and gateway-executed server web tools - #95
Merged
Merged
Conversation
- New upstream /agenttool/v1/webfetch client (app/web_fetch.py) with REST POST /v1/fetch and an MCP web_fetch tool alongside web_search. - app/server_tools.py adapts Anthropic web_search_*/web_fetch_* and OpenAI web_search / web_search_options declarations into WorkBuddy-CLI-shaped injected function tools, executes them on the agenttool endpoints and drives the extra upstream rounds (max_uses, domain filters, search_context_size honored; user_location ignored). - Protocol converters emit native blocks: server_tool_use + web_search_tool_result / web_fetch_tool_result (Messages) and web_search_call items (Responses); chat replay stays unchanged. - Simulate the webfetch endpoint in preview; document /v1/fetch and the server-tool behavior in the paired docs.
- Surface client tool calls instead of fabricating results: the loop stops on client calls and returns them (with gateway calls already executed). - Enforce declared domain filters on every search: model-given domains may only narrow, never widen; declared allow/block lists always post-filter. - Relax tool_choice to auto for continuation rounds and restore it (plus the tool list) in a finally block. - Map hosted tool_choice names (web_search/web_fetch, OpenAI hosted types) onto the injected gateway functions across all three protocols. - Emit protocol-native error shapes: *_tool_result content carries the official *_tool_result_error object with canonical error codes. - Emit the official web_fetch_result document structure with retrieved_at. - Split public server_tool_use IDs (srvtoolu_*) from internal call IDs. - Add status (in_progress/completed/failed) to Responses web_search_call items in both stream events and the final response. - Re-apply the request-size guard to tool continuation rounds (413). - Filter client-owned tool names out of the server-tool spec up front, so injection and execution agree; web_search_options shares the filter.
Reviewer's GuideThis PR adds end-to-end web fetching and gateway-executed server web tools, including upstream clients, REST/MCP surfaces, protocol-specific tool loops and output rendering, domain and usage policies, continuation handling, preview support, and comprehensive routing/endpoint tests. Sequence diagram for gateway-executed server web toolssequenceDiagram
participant Client
participant Gateway
participant Model as UpstreamModel
participant WebTools as server_tools
participant AgentTool as AgentToolAPI
Client->>Gateway: Request with hosted web tool declaration
Gateway->>WebTools: split_server_tools and merge_injected
Gateway->>Model: Chat request with WebSearch or WebFetch
Model-->>Gateway: Gateway tool_call
Gateway->>WebTools: execute_call
WebTools->>AgentTool: web_search or web_fetch request
AgentTool-->>WebTools: Results or fetched content
WebTools-->>Gateway: Tool record
Gateway->>Model: Continuation with tool result
Model-->>Gateway: Final response or client tool_call
Gateway-->>Client: Protocol-native result
Sequence diagram for REST and MCP web fetchsequenceDiagram
participant Client
participant Gateway
participant Fetch as web_fetch
participant AgentTool as AgentToolAPI
alt REST request
Client->>Gateway: POST /v1/fetch
Gateway->>Fetch: request_body
Gateway->>AgentTool: POST /agenttool/v1/webfetch
AgentTool-->>Gateway: JSON fetch envelope
Gateway->>Fetch: parse_content
Gateway-->>Client: fetch.result
else MCP request
Client->>Gateway: tools/call web_fetch
Gateway->>Fetch: request_body
Gateway->>AgentTool: POST /agenttool/v1/webfetch
AgentTool-->>Gateway: Fetch content
Gateway->>Fetch: result_text
Gateway-->>Client: MCP text result
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 18 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="converter.py" line_range="5315" />
<code_context>
+ if not isinstance(arguments, dict):
+ return "", "", "invalid arguments: expected an object"
+ url = arguments.get("url")
+ if not isinstance(url, str) or not url.strip().lower().startswith(("http://", "https://")):
+ return "", "", "invalid arguments: url must be an http(s) URL"
+ prompt = arguments.get("prompt")
</code_context>
<issue_to_address>
**Fetch can target private services**
When an API caller submits a private-network URL and the upstream fetch service can reach that address, the REST, MCP, and server-tool paths accept any string beginning with `http://` or `https://` and pass it to `_web_fetch`, which sends it to the upstream fetch service. An API caller can therefore use the fetch feature to request loopback, link-local, or private-network URLs and retrieve their content if that service can reach those addresses.
Reject loopback, link-local, private, and otherwise disallowed destinations after DNS resolution, and enforce the same restrictions on every redirect at the component that performs the fetch.
Also at `app/mcp_server.py:258`, `app/server_tools.py:256`.
</issue_to_address>
### Comment 2
<location path="app/server_tools.py" line_range="192-193" />
<code_context>
+
+
+def _hostname(url: str) -> str:
+ match = re.match(r"^[a-z][a-z0-9+.-]*://([^/:?#]+)", url, re.IGNORECASE)
+ return (match.group(1).lower() if match else url.lower()).lstrip(".")
+
+
</code_context>
<issue_to_address>
**Domain policies miss the actual host**
When a fetch URL or search result uses userinfo or a trailing-dot hostname, `_hostname` treats URL userinfo as part of the hostname and retains trailing dots, so `_domain_allowed` misses the actual host and permits a fetch or search result that violates the declared domain policy.
Parse the URL authority to obtain its hostname and normalize away a trailing DNS root dot before applying domain policies.
Also at `app/server_tools.py:194`, `app/server_tools.py:196-198`.
</issue_to_address>
### Comment 3
<location path="app/server_tools.py" line_range="243" />
<code_context>
+ declared_allowed = config["allowed_domains"]
+ declared_blocked = config["blocked_domains"]
+ per_call_allowed = _domain_list(arguments.get("allowed_domains"))
+ upstream_allowed = declared_allowed or per_call_allowed
+ results = await search_fn(query, config["max_results"],
+ upstream_allowed, str(arguments.get("freshness") or ""))
</code_context>
<issue_to_address>
**Per-call search restrictions are ignored**
When a declared allow-list and a narrower model-supplied `allowed_domains` list are both present, `execute_call` uses the declared allow-list for the upstream search and post-filter instead of applying the per-call restriction, so results outside the requested subset reach the caller.
Intersect the declared and per-call allow-lists, and use the resulting restriction for both the upstream search and post-filter.
</issue_to_address>
### Comment 4
<location path="app/server_tools.py" line_range="317" />
<code_context>
+ gateway_calls = [call for call in calls
+ if (call.get("function") or {}).get("name") in spec]
+ client_calls = [call for call in calls if call not in gateway_calls]
+ body["messages"].append(_assistant_message(message))
+ for call in gateway_calls:
+ function = call.get("function") or {}
</code_context>
<issue_to_address>
**Failover replays mutated tool history**
When a gateway tool executes, a continuation fails with a failover-eligible error, and routing reuses the canonical body, `run_chat_tool_loop` appends assistant and tool turns to the shared `body["messages"]` before the continuation; failover then retries with that mutated history, causing duplicate tool execution or a response that omits the executed tool blocks.
Run continuation attempts against a copy of the message history or restore the original messages when the loop exits.
Also at `converter.py:3913-3914`, `converter.py:4975-5097`, `app/server_tools.py:318-326`.
</issue_to_address>
### Comment 5
<location path="converter.py" line_range="3881-3882" />
<code_context>
+ hosted_choice = payload.get("tool_choice")
+ if isinstance(hosted_choice, dict) and hosted_choice.get("type") in server_tools.OPENAI_SEARCH_TYPES:
+ if server_tools.TOOL_SEARCH in server_spec:
+ payload = {**payload, "tool_choice": {
+ "type": "function", "function": {"name": server_tools.TOOL_SEARCH}}}
+ else:
+ payload = {**payload, "tool_choice": "auto"}
</code_context>
<issue_to_address>
**Hosted search choice is rejected**
When a Chat Completions request selects the hosted `web_search` tool, the hosted choice is updated only in `payload`, while `_prepare_chat_body` receives the unchanged `body["tool_choice"]`; `_normalize_tool_choice` rejects it with HTTP 400 instead of executing the selected search tool.
Pass the normalized function choice to `_prepare_chat_body` in the body it actually processes.
</issue_to_address>
### Comment 6
<location path="app/mcp_server.py" line_range="198" />
<code_context>
params = message.get("params")
name = params.get("name") if isinstance(params, dict) else None
- if name != TOOL["name"]:
+ if name not in tools:
return 200, error_body(
identifier, JSONRPC_METHOD_NOT_FOUND, f"Unknown tool: {name}"
</code_context>
<issue_to_address>
**Malformed tool names cause server errors**
When an MCP `tools/call` request supplies an array or object as `params.name`, `name not in tools` hashes the supplied value and raises `TypeError`, so dispatch escapes without returning a JSON-RPC error response.
Validate that `name` is a string before checking whether it is in `tools`.
</issue_to_address>
### Comment 7
<location path="app/server_tools.py" line_range="330" />
<code_context>
+ if client_calls:
+ # Client tools belong to the caller: surface their calls and stop;
+ # executed gateway calls stay recorded as protocol blocks.
+ return _strip_gateway_calls(result, client_calls), records
+ # Continuations answer with text, so a required/named choice cannot
+ # hold for them; relax it for every follow-up round.
</code_context>
<issue_to_address>
**Gateway results disappear in Chat**
When an upstream round requests both a gateway web tool and a client-owned tool through Chat Completions, `run_chat_tool_loop` executes gateway calls and returns only the client calls, while the Chat endpoint discards the returned records. The client sees its own call but no gateway result, and the model never gets a continuation with that result, so the search or fetch cannot inform the answer.
Preserve gateway results in a Chat-compatible response and continuation flow when surfacing the client tool call, rather than discarding the records.
Also at `converter.py:3913`, `converter.py:4535`.
</issue_to_address>
### Comment 8
<location path="converter.py" line_range="4901" />
<code_context>
+ if response.status_code != 200:
+ _check_upstream_status(response.status_code, await read_bounded_error(response),
+ cred, None, headers=response.headers)
+ raw = await response.aread()
+ try:
+ payload = json.loads(raw.decode("utf-8", "replace"))
</code_context>
<issue_to_address>
**Slow fetches never time out**
When the upstream webfetch service sends response data continuously but too slowly to finish, `response.aread()` waits for the entire response, while `_backend_stream` configures only an HTTP read timeout; a fetch service that keeps sending data before each idle timeout holds the request and its credential lease indefinitely, tying up gateway capacity.
Apply an overall deadline to the complete fetch operation, not only an idle read timeout.
</issue_to_address>
### Comment 9
<location path="converter.py" line_range="4901" />
<code_context>
+ if response.status_code != 200:
+ _check_upstream_status(response.status_code, await read_bounded_error(response),
+ cred, None, headers=response.headers)
+ raw = await response.aread()
+ try:
+ payload = json.loads(raw.decode("utf-8", "replace"))
</code_context>
<issue_to_address>
**Oversized fetches exhaust memory**
When the upstream webfetch service ignores `max_length` and returns an unusually large response, `response.aread()` buffers the entire upstream body without a response-size limit; if the fetch service returns a body much larger than the requested `max_length`, the gateway allocates it all before parsing, consuming memory and potentially destabilizing concurrent requests.
Read the response with a strict byte limit and fail cleanly when the limit is exceeded.
</issue_to_address>
### Comment 10
<location path="app/server_tools.py" line_range="230" />
<code_context>
+ """Execute one injected tool call; never raises, failures become error records."""
+ record = {"kind": "fetch" if name == TOOL_FETCH else "search",
+ "name": "web_fetch" if name == TOOL_FETCH else "web_search",
+ "id": f"srv_{name.lower()}_{id(arguments) & 0xffffff:x}",
+ "input": {}, "error": None, "error_code": None}
+ try:
</code_context>
<issue_to_address>
**Search call IDs can collide**
When a request executes multiple searches whose short-lived argument dictionaries reuse an address or have colliding masked addresses, `execute_call` derives the record ID from `id(arguments) & 0xffffff`, but the argument dictionary is not retained after execution. Sequential calls can reuse the same address (and the mask also discards most address bits), so Responses can emit distinct `web_search_call` items with duplicate IDs.
Generate these record IDs with a collision-resistant unique ID, as the code already does for public server-tool IDs.
</issue_to_address>
### Comment 11
<location path="app/server_tools.py" line_range="230" />
<code_context>
+ """Execute one injected tool call; never raises, failures become error records."""
+ record = {"kind": "fetch" if name == TOOL_FETCH else "search",
+ "name": "web_fetch" if name == TOOL_FETCH else "web_search",
+ "id": f"srv_{name.lower()}_{id(arguments) & 0xffffff:x}",
+ "input": {}, "error": None, "error_code": None}
+ try:
</code_context>
<issue_to_address>
**Tool-use blocks show incomplete inputs**
When a server-tool call has optional arguments or execution fails, `execute_call` records input only on success and stores just the query or URL, so the Messages adapter emits `server_tool_use.input` with missing arguments or `{}` instead of the call that was executed.
Record the original tool arguments before execution and use them in the emitted tool-use block.
Also at `app/server_tools.py:251`, `app/server_tools.py:262`.
</issue_to_address>
### Comment 12
<location path="app/adapters/anthropic_adapter.py" line_range="451-460" />
<code_context>
+ entry["page_age"] = item["page_age"]
+ results.append(entry)
+ return {"type": "web_search_tool_result", "tool_use_id": use_id,
+ "content": results}
+ fetched = record.get("content") or {}
+ page = {"type": "web_fetch_result",
+ "url": fetched.get("url") or (record.get("input") or {}).get("url"),
+ "retrieved_at": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()),
+ "content": {"type": "document",
+ "title": fetched.get("title"),
+ "source": {"type": "text", "media_type": "text/plain",
+ "data": fetched.get("content") or ""}}}
+ return {"type": "web_fetch_tool_result", "tool_use_id": use_id, "content": page}
+
def get_nonstream_response(self) -> dict:
</code_context>
<issue_to_address>
**Follow-ups lose fetched context**
When a client resubmits the returned assistant blocks as history in a later Messages request, `_convert_anthropic_message` drops the emitted `server_tool_use` and web-tool result blocks from assistant history; `_convert_content_blocks` preserves only text and images. The next Messages request therefore omits the search or fetched content from the model’s context, so follow-up questions cannot reliably build on those results.
Preserve server-tool uses and results in a model-visible form when converting echoed history, without re-executing completed tools.
</issue_to_address>
### Comment 13
<location path="app/server_tools.py" line_range="334" />
<code_context>
+ saved_choice = body.get("tool_choice")
+ saved_tools = body.get("tools")
+ try:
+ result = await fetch_chat(body)
+ for _ in range(max_rounds):
+ choice = (result.get("choices") or [{}])[0]
</code_context>
<issue_to_address>
**Tool rounds underreport token usage**
When a request executes a gateway web tool and makes a continuation model call, `run_chat_tool_loop` replaces `result` with each continuation and returns only the last completion's `usage`; `_completion_to_merged` and the Chat response pass that value through, so callers see only final-round tokens even though every model round was billed.
Accumulate usage across all model rounds and return the aggregate with the final completion.
</issue_to_address>
### Comment 14
<location path="app/server_tools.py" line_range="214-216" />
<code_context>
+def _search_tool_text(record: dict) -> str:
+ if record.get("error"):
+ return f"web search failed: {record['error']}"
+ lines = [f"- [{item.get('title') or item.get('url')}]({item.get('url')})"
+ for item in record.get("results") or []]
+ return "\n".join(lines) or "No search results found."
+
+
</code_context>
<issue_to_address>
**Search snippets withheld from model**
When the search endpoint returns results with non-empty snippets, `_search_tool_text` sends only titles and URLs to the upstream model, omitting available snippets; the model therefore receives less search evidence and can produce an answer that ignores useful result summaries.
Include result snippets in the text returned to the upstream model, as the existing search renderer does.
</issue_to_address>
### Comment 15
<location path="tests/test_server_tools.py" line_range="558" />
<code_context>
+ payload["tools"] = [{"type": "web_search_20250305", "name": "web_search",
+ "max_uses": 2, "allowed_domains": ["example.com"]}]
+ response = self.client.post("/v1/messages", json=payload)
+ self.assertEqual(response.status_code, 200, response.text)
+ data = response.json()
+ types = [block["type"] for block in data["content"]]
</code_context>
<issue_to_address>
**History blocks can be forwarded unnoticed**
When the adapter forwards echoed server-tool blocks but the mocked upstream still accepts the request, `test_history_echo_blocks_are_accepted` checks only for HTTP 200, and its mocked upstream returns scripted success without checking the converted messages. The test passes even if the echoed server-tool blocks are forwarded upstream instead of skipped.
Assert that the captured upstream messages omit the echoed server-tool and result blocks.
</issue_to_address>
### Comment 16
<location path="tests/test_server_tools.py" line_range="347-348" />
<code_context>
+ self.assertEqual(output[0]["action"]["queries"], [QUERY])
+ self.assertEqual(output[1]["type"], "message")
+ self.assertEqual(output[0]["status"], "completed")
+ self.assertIn('"status": "completed"', events)
+ self.assertIn('"status": "in_progress"', events)
+
+ def test_responses_search_call_failed_status(self):
</code_context>
<issue_to_address>
**Lifecycle statuses mask missing tool statuses**
When the stream contains lifecycle statuses but omits statuses from its `web_search_call` items, `test_responses_search_call_items_lead` searches the whole stream for status strings, so lifecycle events satisfy these assertions even when the `web_search_call` items have no status transitions.
Parse the stream events and assert `in_progress` and `completed` on the corresponding web-search output item.
</issue_to_address>
### Comment 17
<location path="tests/test_server_tools.py" line_range="539" />
<code_context>
+ sent = json.loads(self.chat_requests[0].content)
+ self.assertEqual([t["function"]["name"] for t in sent["tools"]], ["WebSearch"])
+ self.assertEqual(self.tool_requests, [])
+ self.assertEqual(response.json()["stop_reason"], "tool_use")
+
+ def test_history_echo_blocks_are_accepted(self):
</code_context>
<issue_to_address>
**Collision test misses renamed tool calls**
When the collision path returns a tool call with a name other than the client-declared `WebSearch`, `test_client_owned_websearch_name_stays_client` checks only the stop reason for the returned call. A response that exposes the colliding client call under the wrong tool name still has `stop_reason == "tool_use"`, so the test passes while the client cannot dispatch it.
Assert that the returned `tool_use` block retains the declared name and input.
</issue_to_address>
### Comment 18
<location path="tests/test_web_fetch.py" line_range="134-135" />
<code_context>
+ result = response.json()["result"]
+ self.assertFalse(result["isError"])
+ text = result["content"][0]["text"]
+ self.assertIn("# Example Guide", text)
+ self.assertIn(URL, text)
+ sent = json.loads(self.fetch_requests[-1].content)
+ self.assertEqual(sent["prompt"], "summarize")
</code_context>
<issue_to_address>
**MCP test misses fetched page content**
When the MCP result includes the title and URL but drops the fetched body, `test_mcp_fetch_call_returns_markdown` checks only the generated heading and source URL. A result containing those metadata lines but omitting the fetched page body still passes, leaving the caller without the fetched content.
Assert that the fetched body appears in the returned text.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 18 findings to address first, and the new fetch endpoint and server tools send caller-supplied URLs to the upstream fetch service and return the fetched content; if URL handling lets a request reach a private resource, that data can be exposed to the caller or model. Reverting removes the capability but cannot undo content already disclosed.
Blocking findings: converter.py:5315, app/server_tools.py:193, app/server_tools.py:243, app/server_tools.py:317, converter.py:3882, and 13 more
| raise HTTPException(status_code=400, detail={"error": {"message": f"bad json: {e}", | ||
| "type": "invalid_request_error"}}) | ||
| url = payload.get("url") if isinstance(payload, dict) else None | ||
| if not isinstance(url, str) or not url.strip().lower().startswith(("http://", "https://")): |
There was a problem hiding this comment.
🟡 Medium · Fetch can target private services
When an API caller submits a private-network URL and the upstream fetch service can reach that address, the REST, MCP, and server-tool paths accept any string beginning with http:// or https:// and pass it to _web_fetch, which sends it to the upstream fetch service. An API caller can therefore use the fetch feature to request loopback, link-local, or private-network URLs and retrieve their content if that service can reach those addresses.
Reject loopback, link-local, private, and otherwise disallowed destinations after DNS resolution, and enforce the same restrictions on every redirect at the component that performs the fetch.
Also at app/mcp_server.py:258, app/server_tools.py:256.
Prompt for AI agents
In `converter.py` at line 5315:
**Fetch can target private services**
When an API caller submits a private-network URL and the upstream fetch service can reach that address, the REST, MCP, and server-tool paths accept any string beginning with `http://` or `https://` and pass it to `_web_fetch`, which sends it to the upstream fetch service. An API caller can therefore use the fetch feature to request loopback, link-local, or private-network URLs and retrieve their content if that service can reach those addresses.
Reject loopback, link-local, private, and otherwise disallowed destinations after DNS resolution, and enforce the same restrictions on every redirect at the component that performs the fetch.
Also at `app/mcp_server.py:258`, `app/server_tools.py:256`.
Comment on lines
+192
to
+193
| match = re.match(r"^[a-z][a-z0-9+.-]*://([^/:?#]+)", url, re.IGNORECASE) | ||
| return (match.group(1).lower() if match else url.lower()).lstrip(".") |
There was a problem hiding this comment.
🟡 Medium · Domain policies miss the actual host
When a fetch URL or search result uses userinfo or a trailing-dot hostname, _hostname treats URL userinfo as part of the hostname and retains trailing dots, so _domain_allowed misses the actual host and permits a fetch or search result that violates the declared domain policy.
Parse the URL authority to obtain its hostname and normalize away a trailing DNS root dot before applying domain policies.
Also at app/server_tools.py:194, app/server_tools.py:196-198.
Prompt for AI agents
In `app/server_tools.py` at lines 192-193:
**Domain policies miss the actual host**
When a fetch URL or search result uses userinfo or a trailing-dot hostname, `_hostname` treats URL userinfo as part of the hostname and retains trailing dots, so `_domain_allowed` misses the actual host and permits a fetch or search result that violates the declared domain policy.
Parse the URL authority to obtain its hostname and normalize away a trailing DNS root dot before applying domain policies.
Also at `app/server_tools.py:194`, `app/server_tools.py:196-198`.| declared_allowed = config["allowed_domains"] | ||
| declared_blocked = config["blocked_domains"] | ||
| per_call_allowed = _domain_list(arguments.get("allowed_domains")) | ||
| upstream_allowed = declared_allowed or per_call_allowed |
There was a problem hiding this comment.
🟡 Medium · Per-call search restrictions are ignored
When a declared allow-list and a narrower model-supplied allowed_domains list are both present, execute_call uses the declared allow-list for the upstream search and post-filter instead of applying the per-call restriction, so results outside the requested subset reach the caller.
Intersect the declared and per-call allow-lists, and use the resulting restriction for both the upstream search and post-filter.
Prompt for AI agents
In `app/server_tools.py` at line 243:
**Per-call search restrictions are ignored**
When a declared allow-list and a narrower model-supplied `allowed_domains` list are both present, `execute_call` uses the declared allow-list for the upstream search and post-filter instead of applying the per-call restriction, so results outside the requested subset reach the caller.
Intersect the declared and per-call allow-lists, and use the resulting restriction for both the upstream search and post-filter.| gateway_calls = [call for call in calls | ||
| if (call.get("function") or {}).get("name") in spec] | ||
| client_calls = [call for call in calls if call not in gateway_calls] | ||
| body["messages"].append(_assistant_message(message)) |
There was a problem hiding this comment.
🟡 Medium · Failover replays mutated tool history
When a gateway tool executes, a continuation fails with a failover-eligible error, and routing reuses the canonical body, run_chat_tool_loop appends assistant and tool turns to the shared body["messages"] before the continuation; failover then retries with that mutated history, causing duplicate tool execution or a response that omits the executed tool blocks.
Run continuation attempts against a copy of the message history or restore the original messages when the loop exits.
Also at converter.py:3913-3914, converter.py:4975-5097, app/server_tools.py:318-326.
Prompt for AI agents
In `app/server_tools.py` at line 317:
**Failover replays mutated tool history**
When a gateway tool executes, a continuation fails with a failover-eligible error, and routing reuses the canonical body, `run_chat_tool_loop` appends assistant and tool turns to the shared `body["messages"]` before the continuation; failover then retries with that mutated history, causing duplicate tool execution or a response that omits the executed tool blocks.
Run continuation attempts against a copy of the message history or restore the original messages when the loop exits.
Also at `converter.py:3913-3914`, `converter.py:4975-5097`, `app/server_tools.py:318-326`.
Comment on lines
+3881
to
+3882
| payload = {**payload, "tool_choice": { | ||
| "type": "function", "function": {"name": server_tools.TOOL_SEARCH}}} |
There was a problem hiding this comment.
🟡 Medium · Hosted search choice is rejected
When a Chat Completions request selects the hosted web_search tool, the hosted choice is updated only in payload, while _prepare_chat_body receives the unchanged body["tool_choice"]; _normalize_tool_choice rejects it with HTTP 400 instead of executing the selected search tool.
Pass the normalized function choice to _prepare_chat_body in the body it actually processes.
Prompt for AI agents
In `converter.py` at lines 3881-3882:
**Hosted search choice is rejected**
When a Chat Completions request selects the hosted `web_search` tool, the hosted choice is updated only in `payload`, while `_prepare_chat_body` receives the unchanged `body["tool_choice"]`; `_normalize_tool_choice` rejects it with HTTP 400 instead of executing the selected search tool.
Pass the normalized function choice to `_prepare_chat_body` in the body it actually processes.
Comment on lines
+214
to
+216
| lines = [f"- [{item.get('title') or item.get('url')}]({item.get('url')})" | ||
| for item in record.get("results") or []] | ||
| return "\n".join(lines) or "No search results found." |
There was a problem hiding this comment.
🟡 Medium · Search snippets withheld from model
When the search endpoint returns results with non-empty snippets, _search_tool_text sends only titles and URLs to the upstream model, omitting available snippets; the model therefore receives less search evidence and can produce an answer that ignores useful result summaries.
Include result snippets in the text returned to the upstream model, as the existing search renderer does.
Prompt for AI agents
In `app/server_tools.py` at lines 214-216:
**Search snippets withheld from model**
When the search endpoint returns results with non-empty snippets, `_search_tool_text` sends only titles and URLs to the upstream model, omitting available snippets; the model therefore receives less search evidence and can produce an answer that ignores useful result summaries.
Include result snippets in the text returned to the upstream model, as the existing search renderer does.| {"type": "text", "text": "and now?"}]}, | ||
| ] | ||
| response = self.client.post("/v1/messages", json=payload) | ||
| self.assertEqual(response.status_code, 200, response.text) |
There was a problem hiding this comment.
🟡 Medium · History blocks can be forwarded unnoticed
When the adapter forwards echoed server-tool blocks but the mocked upstream still accepts the request, test_history_echo_blocks_are_accepted checks only for HTTP 200, and its mocked upstream returns scripted success without checking the converted messages. The test passes even if the echoed server-tool blocks are forwarded upstream instead of skipped.
Assert that the captured upstream messages omit the echoed server-tool and result blocks.
Prompt for AI agents
In `tests/test_server_tools.py` at line 558:
**History blocks can be forwarded unnoticed**
When the adapter forwards echoed server-tool blocks but the mocked upstream still accepts the request, `test_history_echo_blocks_are_accepted` checks only for HTTP 200, and its mocked upstream returns scripted success without checking the converted messages. The test passes even if the echoed server-tool blocks are forwarded upstream instead of skipped.
Assert that the captured upstream messages omit the echoed server-tool and result blocks.
Comment on lines
+347
to
+348
| self.assertIn('"status": "completed"', events) | ||
| self.assertIn('"status": "in_progress"', events) |
There was a problem hiding this comment.
🟡 Medium · Lifecycle statuses mask missing tool statuses
When the stream contains lifecycle statuses but omits statuses from its web_search_call items, test_responses_search_call_items_lead searches the whole stream for status strings, so lifecycle events satisfy these assertions even when the web_search_call items have no status transitions.
Parse the stream events and assert in_progress and completed on the corresponding web-search output item.
Prompt for AI agents
In `tests/test_server_tools.py` at lines 347-348:
**Lifecycle statuses mask missing tool statuses**
When the stream contains lifecycle statuses but omits statuses from its `web_search_call` items, `test_responses_search_call_items_lead` searches the whole stream for status strings, so lifecycle events satisfy these assertions even when the `web_search_call` items have no status transitions.
Parse the stream events and assert `in_progress` and `completed` on the corresponding web-search output item.| sent = json.loads(self.chat_requests[0].content) | ||
| self.assertEqual([t["function"]["name"] for t in sent["tools"]], ["WebSearch"]) | ||
| self.assertEqual(self.tool_requests, []) | ||
| self.assertEqual(response.json()["stop_reason"], "tool_use") |
There was a problem hiding this comment.
🟡 Medium · Collision test misses renamed tool calls
When the collision path returns a tool call with a name other than the client-declared WebSearch, test_client_owned_websearch_name_stays_client checks only the stop reason for the returned call. A response that exposes the colliding client call under the wrong tool name still has stop_reason == "tool_use", so the test passes while the client cannot dispatch it.
Assert that the returned tool_use block retains the declared name and input.
Prompt for AI agents
In `tests/test_server_tools.py` at line 539:
**Collision test misses renamed tool calls**
When the collision path returns a tool call with a name other than the client-declared `WebSearch`, `test_client_owned_websearch_name_stays_client` checks only the stop reason for the returned call. A response that exposes the colliding client call under the wrong tool name still has `stop_reason == "tool_use"`, so the test passes while the client cannot dispatch it.
Assert that the returned `tool_use` block retains the declared name and input.
Comment on lines
+134
to
+135
| self.assertIn("# Example Guide", text) | ||
| self.assertIn(URL, text) |
There was a problem hiding this comment.
🟡 Medium · MCP test misses fetched page content
When the MCP result includes the title and URL but drops the fetched body, test_mcp_fetch_call_returns_markdown checks only the generated heading and source URL. A result containing those metadata lines but omitting the fetched page body still passes, leaving the caller without the fetched content.
Assert that the fetched body appears in the returned text.
Prompt for AI agents
In `tests/test_web_fetch.py` at lines 134-135:
**MCP test misses fetched page content**
When the MCP result includes the title and URL but drops the fetched body, `test_mcp_fetch_call_returns_markdown` checks only the generated heading and source URL. A result containing those metadata lines but omitting the fetched page body still passes, leaving the caller without the fetched content.
Assert that the fetched body appears in the returned text.- Reject internal-network fetch targets (localhost, private/link-local and other non-public IPs, legacy IPv4 spellings, .local/.internal) before any upstream request, on the REST endpoint, the MCP tool and gateway-executed web_fetch alike; normalize hosts through WHATWG-style parsing and IDNA so policy checks match the host the fetcher connects to. - Run gateway web tools under observe_nested: each surfaces as one server_tool attempt inside the triggering request's audit record, with summed usage, instead of creating audit records of their own. - Extract the aggregated-loop replay (_replay_merged_sse) shared by the chat and adapted streaming paths, and keep the realtime accumulator off the server-tool replay path. - Replay echoed protocol blocks (server_tool_use, web_search_tool_result, web_fetch_tool_result, web_search_call) back to the model as text context; emit gateway-owned cb2api: encrypted_content snapshots so round-trips keep result context. Accept dated OpenAI hosted search types. - Simplify the tool-choice restore with an absent-marker map, spend the last continuation round without injected tools, and unify the hosted tool-choice rewrite through is_openai_search_type.
- Aggregate server_tool attempts per request (searches, fetches, failures, total duration, truncation marker) and render them as KPI cards on the request detail, with failed counts highlighted. - Label web-search / web-fetch model names and the dropped-attempt count.
- _assistant_message now carries reasoning_content into tool-call continuation rounds, matching the rest of the reasoning pipeline. - Merge the two _validated_max_* validators into one _validated_count and extract _bad_json and _domain_policy helpers replacing six duplicated 400 blocks and the duplicated domain-policy normalization. - Drop the unused log parameter of run_chat_tool_loop and the dead seeded branch of the test feed helper. - Chinese user-facing errors for the new /v1/search and /v1/fetch field validation messages (bad json keeps the existing gateway-wide wording).
- Merge the web-search and web-fetch failover loops into one _AgentTool descriptor with a single _agenttool_call: shared routing, failover, observability and logging; only URL, timeout, parser and summaries differ. - Gateway-executed web tools run on the triggering attempt's credential lease (borrowed): the loop no longer leases capacity of its own and never touches the session's sticky binding; failover after a borrowed attempt leases non-sticky. - The tool loop appends assistant/tool turns to its own copy of the message list and restores the original on exit, so a failover retry re-routed from the same body never carries this attempt's tool turns. - Measure only continuation rounds against the request-size guard (the first round is measured during routing), in a worker thread. - One _mcp_tool factory for the MCP web tool adapters; Chinese texts for agenttool parse failures; drop duplicated comments/docstrings; guard the MCP tool name type.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/agenttool/v1/webfetch(app/web_fetch.py): request shaping (url / format / prompt / timeout / max_length) and envelope validation, exposed three ways — RESTPOST /v1/fetch, an MCPweb_fetchtool next toweb_search, and as the executor for protocol server tools.app/server_tools.pyadapts server-side web tool declarations — Anthropicweb_search_*/web_fetch_*types and OpenAIweb_searchtools /web_search_options— into WorkBuddy-CLI-shapedWebSearch/WebFetchfunction tools the upstream models already know, executes them on the agenttool endpoints and drives the extra upstream rounds.max_uses, domain filters andsearch_context_size(low/medium/high → 3/5/8 results) are honored;user_locationis accepted and ignored;blocked_domainsfolds into-site:query suffixes.allowed_domainsmay only narrow a declared allow-list, never widen it, and declared allow/block lists always post-filter results and fetch URLs.tool_choicetoauto(restored in afinallyblock) soany/required/ named choices no longer end inInvalid upstream tool_calls, hostedtool_choicenames (web_search/web_fetch) map onto the injected gateway functions across all three protocols, and the request-size guard re-applies to every continuation round.server_tool_useplusweb_search_tool_result/web_fetch_tool_resultblocks (officialweb_fetch_resultdocument structure withretrieved_at, official*_tool_result_errorobjects with canonical error codes, publicsrvtoolu_*IDs separate from internal call IDs); Responses emitsweb_search_callitems withstatus(in_progress→completed/failed) in both the stream and the final object.WebSearch/WebFetchnames win: the server-tool spec is collision-filtered up front, so injection and execution always agree on which names the gateway owns.server_tool_use/*_tool_resultblocks is accepted on follow-up turns (skipped, not rejected)./v1/searchnow forwardsallowed_domainsandfreshness.Summary by Sourcery
Add gateway-managed web search and fetch capabilities across REST, MCP, and supported inference protocols.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: