mirror of
https://github.com/hanzoai/python-sdk.git
synced 2026-10-23 20:23:00 +00:00
fix(hanzo-mcp): 'tool list' crash on @property descriptions (v0.15.11)
DynamicToolRegistry stored getattr(tool_class, 'description'), which for a class-level @property returns the property DESCRIPTOR (not a str). 'tool list' then did len(config.description) → 'object of type property has no len()', breaking the entire tool-catalog command. Add _extract_tool_description (mirrors _extract_tool_name): resolve a @property via a cheap instance, else coerce to str, always returns str. Plus a defensive isinstance guard in the list display path. Regression test covers property/unevaluable-property/plain/ absent + asserts every registered entry's description is len()-safe. 2 passed.
This commit is contained in:
@@ -122,7 +122,7 @@ class DynamicToolRegistry:
|
||||
continue
|
||||
for tool_class in tools_list:
|
||||
name = _extract_tool_name(tool_class)
|
||||
desc = getattr(tool_class, "description", "") or ""
|
||||
desc = _extract_tool_description(tool_class)
|
||||
cls._entries[name] = ToolConfigEntry(
|
||||
name=name,
|
||||
description=desc,
|
||||
@@ -174,6 +174,32 @@ def _extract_tool_name(tool_class: type) -> str:
|
||||
return tool_class.__name__.lower().replace("tool", "")
|
||||
|
||||
|
||||
def _extract_tool_description(tool_class: type) -> str:
|
||||
"""Get a tool class's description as a STRING, handling @property.
|
||||
|
||||
When a tool defines ``description`` as a ``@property`` (evaluated per
|
||||
instance), ``getattr(tool_class, "description")`` returns the property
|
||||
DESCRIPTOR object, not a string — storing that made ``list_all`` crash later
|
||||
with ``object of type 'property' has no len()``. Mirror ``_extract_tool_name``:
|
||||
if it's a property, instantiate to read the real value; otherwise coerce to
|
||||
a plain string. Always returns a ``str`` (empty when unavailable).
|
||||
"""
|
||||
for klass in tool_class.__mro__:
|
||||
if "description" in getattr(klass, "__dict__", {}):
|
||||
attr = klass.__dict__["description"]
|
||||
if isinstance(attr, property):
|
||||
try:
|
||||
val = getattr(tool_class(), "description", None)
|
||||
if isinstance(val, str):
|
||||
return val
|
||||
except Exception:
|
||||
pass
|
||||
return ""
|
||||
break
|
||||
desc = getattr(tool_class, "description", "")
|
||||
return desc if isinstance(desc, str) else ""
|
||||
|
||||
|
||||
# Module-level convenience -- DynamicToolRegistry doubles as the dict-like TOOL_REGISTRY.
|
||||
# Code that does `TOOL_REGISTRY.items()` or `TOOL_REGISTRY.keys()` works via __getattr__.
|
||||
class _RegistryProxy:
|
||||
|
||||
@@ -214,11 +214,11 @@ Examples:
|
||||
lines.append(f"[{cat}]")
|
||||
for tool_name, config, is_enabled in sorted(tools, key=lambda x: x[0]):
|
||||
status = "✓" if is_enabled else "○"
|
||||
desc = (
|
||||
config.description[:50] + "..."
|
||||
if len(config.description) > 50
|
||||
else config.description
|
||||
)
|
||||
# Coerce defensively: a tool whose `description` is a @property
|
||||
# can leave a non-str here — never let a display path crash the
|
||||
# whole `tool list`.
|
||||
desc_str = config.description if isinstance(config.description, str) else ""
|
||||
desc = desc_str[:50] + "..." if len(desc_str) > 50 else desc_str
|
||||
lines.append(f" {status} {tool_name}: {desc}")
|
||||
lines.append("")
|
||||
|
||||
|
||||
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
|
||||
|
||||
[project]
|
||||
name = "hanzo-mcp"
|
||||
version = "0.15.10"
|
||||
version = "0.15.11"
|
||||
description = "The Zen of Hanzo MCP: One server to rule them all. The ultimate MCP that orchestrates all others."
|
||||
readme = "README.md"
|
||||
requires-python = ">=3.12"
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
"""Regression: a tool whose `description` is a @property must not crash `tool list`.
|
||||
|
||||
Bug: DynamicToolRegistry stored `getattr(tool_class, "description")`, which for a
|
||||
class-level @property returns the property DESCRIPTOR (not a str). Later
|
||||
`len(config.description)` in `tool list` raised:
|
||||
object of type 'property' has no len()
|
||||
This made the whole `tool list` action unusable.
|
||||
"""
|
||||
|
||||
from hanzo_mcp.config.tool_config import (
|
||||
DynamicToolRegistry,
|
||||
_extract_tool_description,
|
||||
)
|
||||
|
||||
|
||||
class _PropDescTool:
|
||||
"""A tool that exposes `description` only via an instance @property."""
|
||||
|
||||
name = "propdesc"
|
||||
|
||||
@property
|
||||
def description(self) -> str: # noqa: D401
|
||||
return "described via property"
|
||||
|
||||
|
||||
class _PropDescNoInstanceTool:
|
||||
"""A @property that blows up without proper init — must degrade to ''."""
|
||||
|
||||
name = "propdesc_bad"
|
||||
|
||||
def __init__(self):
|
||||
raise RuntimeError("cannot instantiate cheaply")
|
||||
|
||||
@property
|
||||
def description(self) -> str:
|
||||
return "unreachable"
|
||||
|
||||
|
||||
class _PlainDescTool:
|
||||
name = "plaindesc"
|
||||
description = "a plain string description"
|
||||
|
||||
|
||||
class _NoDescTool:
|
||||
name = "nodesc"
|
||||
|
||||
|
||||
def test_extract_property_description_is_a_string():
|
||||
# The @property is resolved to its real string value (via a cheap instance).
|
||||
assert _extract_tool_description(_PropDescTool) == "described via property"
|
||||
# Every branch returns a real str — never a `property` object.
|
||||
for cls in (_PropDescTool, _PropDescNoInstanceTool, _PlainDescTool, _NoDescTool):
|
||||
assert isinstance(_extract_tool_description(cls), str)
|
||||
# A property that can't be evaluated degrades to empty, not a crash.
|
||||
assert _extract_tool_description(_PropDescNoInstanceTool) == ""
|
||||
assert _extract_tool_description(_PlainDescTool) == "a plain string description"
|
||||
assert _extract_tool_description(_NoDescTool) == ""
|
||||
|
||||
|
||||
def test_registered_entry_descriptions_are_never_property_objects():
|
||||
# The bug's fingerprint: len() on a stored description must always work,
|
||||
# i.e. every registered entry's description is a real str.
|
||||
DynamicToolRegistry.reset()
|
||||
DynamicToolRegistry.initialize()
|
||||
for name, entry in DynamicToolRegistry.list_all().items():
|
||||
assert isinstance(entry.description, str), (
|
||||
f"{name} description is {type(entry.description).__name__}, not str"
|
||||
)
|
||||
# This is the exact op that used to crash.
|
||||
_ = len(entry.description)
|
||||
Reference in New Issue
Block a user