diff --git a/nerve/agent/tools.py b/nerve/agent/tools.py index a1ee400..5de7785 100644 --- a/nerve/agent/tools.py +++ b/nerve/agent/tools.py @@ -13,10 +13,45 @@ from typing import Any from zoneinfo import ZoneInfo -from claude_agent_sdk import SdkMcpTool, create_sdk_mcp_server, tool +from claude_agent_sdk import SdkMcpTool, create_sdk_mcp_server +from claude_agent_sdk import tool as _sdk_tool logger = logging.getLogger(__name__) + +def tool(name: str, description: str, input_schema, *args, **kwargs): + """Wrapper around ``claude_agent_sdk.tool`` that fixes shorthand-schema handling. + + The SDK's ``_build_schema`` (claude_agent_sdk/__init__.py) converts shorthand + dicts (those without a top-level ``"type"`` key) by forcing every property into + ``required: list(properties.keys())`` and silently discarding descriptions and + defaults via ``_python_type_to_json_schema``. The result is that fields with a + ``"default"`` annotation are still advertised as required to the model, and the + model never sees the parameter descriptions at all. + + Combined with the Claude Code CLI's behaviour of throwing ``McpToolCallError`` + on validation failures (which propagates up the streaming connection and ends + the agent's turn early), this caused agents to abort mid-task whenever the + model trusted a documented default and omitted the field. + + The wrapper pre-promotes shorthand dicts to the explicit JSON Schema form the + SDK passes through unchanged: properties are kept intact (so descriptions and + defaults survive) and ``required`` only lists fields that have no ``default``. + Tools that already supply an explicit ``{"type": "object", ...}`` schema are + untouched. + """ + if isinstance(input_schema, dict) and "type" not in input_schema: + input_schema = { + "type": "object", + "properties": dict(input_schema), + "required": [ + field + for field, spec in input_schema.items() + if not isinstance(spec, dict) or "default" not in spec + ], + } + return _sdk_tool(name, description, input_schema, *args, **kwargs) + # These will be set during initialization _workspace: Path | None = None _db = None # nerve.db.Database instance diff --git a/tests/test_tools_schemas.py b/tests/test_tools_schemas.py new file mode 100644 index 0000000..df97997 --- /dev/null +++ b/tests/test_tools_schemas.py @@ -0,0 +1,133 @@ +"""Tests for the tools.py schema-promotion wrapper. + +Regression test for the bug where the Claude Agent SDK's _build_schema +forces every property of a shorthand input_schema into "required", +ignoring "default" annotations and silently dropping descriptions. +The wrapper in nerve.agent.tools.tool fixes this by pre-promoting +shorthand dicts to explicit JSON Schema before handing them to the SDK. +""" + +import pytest + +from nerve.agent.tools import ALL_TOOLS, tool + + +def _properties_with_defaults(schema: dict) -> set[str]: + """Return the set of property names that declare a default.""" + props = schema.get("properties") or {} + return { + name + for name, spec in props.items() + if isinstance(spec, dict) and "default" in spec + } + + +class TestSchemaWrapper: + """The wrapper promotes shorthand dicts to explicit JSON Schema.""" + + def test_shorthand_with_defaults_is_promoted(self): + """Fields with a "default" land outside "required"; bare fields stay required.""" + + @tool( + "fixture", + "doc", + { + "required_field": {"type": "string", "description": "must be set"}, + "optional_with_default": { + "type": "string", + "description": "has a default", + "default": "x", + }, + }, + ) + async def _handler(args: dict) -> dict: # pragma: no cover - never invoked + return {"content": [{"type": "text", "text": "ok"}]} + + schema = _handler.input_schema + assert schema["type"] == "object" + assert set(schema["properties"]) == {"required_field", "optional_with_default"} + # The original property dicts (including descriptions and defaults) are preserved. + assert schema["properties"]["optional_with_default"]["description"] == "has a default" + assert schema["properties"]["optional_with_default"]["default"] == "x" + # Only fields without a default are required. + assert schema["required"] == ["required_field"] + + def test_explicit_schema_is_passed_through_unchanged(self): + """An explicit JSON Schema is preserved verbatim.""" + explicit = { + "type": "object", + "properties": { + "a": {"type": "string"}, + "b": {"type": "integer", "default": 0}, + }, + "required": ["a"], + } + + @tool("fixture-explicit", "doc", explicit) + async def _handler(args: dict) -> dict: # pragma: no cover - never invoked + return {"content": [{"type": "text", "text": "ok"}]} + + assert _handler.input_schema == explicit + + def test_no_defaults_means_everything_required(self): + """Backwards-compatible: a shorthand schema with no defaults stays fully required.""" + + @tool( + "fixture-bare", + "doc", + { + "x": {"type": "string"}, + "y": {"type": "string"}, + }, + ) + async def _handler(args: dict) -> dict: # pragma: no cover - never invoked + return {"content": [{"type": "text", "text": "ok"}]} + + assert sorted(_handler.input_schema["required"]) == ["x", "y"] + + def test_zero_field_shorthand(self): + """A tool with no parameters yields an empty required list.""" + + @tool("fixture-empty", "doc", {}) + async def _handler(args: dict) -> dict: # pragma: no cover - never invoked + return {"content": [{"type": "text", "text": "ok"}]} + + schema = _handler.input_schema + assert schema["type"] == "object" + assert schema["properties"] == {} + assert schema["required"] == [] + + +class TestAllToolsAreCorrectlyBuilt: + """Every registered tool must expose an explicit JSON Schema with a + correct "required" list (no field that declares a default appears + in required). This catches future regressions if a tool is added + using the bare claude_agent_sdk.tool decorator instead of the + Nerve wrapper. + """ + + @pytest.mark.parametrize("tool_def", ALL_TOOLS, ids=lambda t: t.name) + def test_tool_schema_is_explicit(self, tool_def): + schema = tool_def.input_schema + assert isinstance(schema, dict), ( + f"{tool_def.name} input_schema must be a dict, got {type(schema)!r}" + ) + assert schema.get("type") == "object", ( + f"{tool_def.name} input_schema must be promoted to " + f'{{"type": "object", ...}}; got {schema!r}' + ) + assert "properties" in schema, f"{tool_def.name} schema is missing properties" + assert "required" in schema, f"{tool_def.name} schema is missing required" + + @pytest.mark.parametrize("tool_def", ALL_TOOLS, ids=lambda t: t.name) + def test_no_defaulted_field_is_required(self, tool_def): + schema = tool_def.input_schema + defaulted = _properties_with_defaults(schema) + required = set(schema.get("required") or []) + leaked = defaulted & required + assert not leaked, ( + f"{tool_def.name} marks fields with a default as required: " + f"{sorted(leaked)} (this would cause the model to receive a " + f'schema where "optional with default" fields are still required, ' + f"reproducing the McpToolCallError stream-tear-down bug)" + )