feat(tools)!: substantial clean up of "Tool" related datatypes (#3627)

This is a sweeping change to clean up some gunk around our "Tool"
definitions.

First, we had two types `Tool` and `ToolDef`. The first of these was a
"Resource" type for the registry but we had stopped registering tools
inside the Registry long back (and only registered ToolGroups.) The
latter was for specifying tools for the Agents API. This PR removes the
former and adds an optional `toolgroup_id` field to the latter.

Secondly, as pointed out by @bbrowning in
https://github.com/llamastack/llama-stack/pull/3003#issuecomment-3245270132,
we were doing a lossy conversion from a full JSON schema from the MCP
tool specification into our ToolDefinition to send it to the model.
There is no necessity to do this -- we ourselves aren't doing any
execution at all but merely passing it to the chat completions API which
supports this. By doing this (and by doing it poorly), we encountered
limitations like not supporting array items, or not resolving $refs,
etc.

To fix this, we replaced the `parameters` field by `{ input_schema,
output_schema }` which can be full blown JSON schemas.

Finally, there were some types in our llama-related chat format
conversion which needed some cleanup. We are taking this opportunity to
clean those up.

This PR is a substantial breaking change to the API. However, given our
window for introducing breaking changes, this suits us just fine. I will
be landing a concurrent `llama-stack-client` change as well since API
shapes are changing.
This commit is contained in:
Ashwin Bharambe 2025-10-02 15:12:03 -07:00 committed by GitHub
parent 1f5003d50e
commit ef0736527d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
179 changed files with 34186 additions and 9171 deletions

View file

@ -222,16 +222,16 @@ def make_mcp_server(required_auth_token: str | None = None, tools: dict[str, Cal
def run_server():
try:
logger.info(f"Starting MCP server on port {port}")
logger.debug(f"Starting MCP server on port {port}")
server_instance.run()
logger.info(f"MCP server on port {port} has stopped")
logger.debug(f"MCP server on port {port} has stopped")
except Exception as e:
logger.error(f"MCP server failed to start on port {port}: {e}")
raise
# Start the server in a new thread
server_thread = threading.Thread(target=run_server, daemon=True)
logger.info(f"Starting MCP server thread on port {port}")
logger.debug(f"Starting MCP server thread on port {port}")
server_thread.start()
# Polling until the server is ready
@ -239,13 +239,13 @@ def make_mcp_server(required_auth_token: str | None = None, tools: dict[str, Cal
start_time = time.time()
server_url = f"http://localhost:{port}/sse"
logger.info(f"Waiting for MCP server to be ready at {server_url}")
logger.debug(f"Waiting for MCP server to be ready at {server_url}")
while time.time() - start_time < timeout:
try:
response = httpx.get(server_url)
if response.status_code in [200, 401]:
logger.info(f"MCP server is ready on port {port} (status: {response.status_code})")
logger.debug(f"MCP server is ready on port {port} (status: {response.status_code})")
break
except httpx.RequestError as e:
logger.debug(f"Server not ready yet, retrying... ({e})")
@ -261,14 +261,14 @@ def make_mcp_server(required_auth_token: str | None = None, tools: dict[str, Cal
try:
yield {"server_url": server_url}
finally:
logger.info(f"Shutting down MCP server on port {port}")
logger.debug(f"Shutting down MCP server on port {port}")
server_instance.should_exit = True
time.sleep(0.5)
# Force shutdown if still running
if server_thread.is_alive():
try:
logger.info("Force shutting down server thread")
logger.debug("Force shutting down server thread")
if hasattr(server_instance, "servers") and server_instance.servers:
for srv in server_instance.servers:
srv.close()