agent/tools: fix shorthand-schema bug that aborts agent turns mid-task#63
Merged
pufit merged 1 commit intoMay 13, 2026
Merged
Conversation
The Claude Agent SDK's _build_schema (claude_agent_sdk/__init__.py:393-413) converts shorthand input_schema dicts by forcing every property into required: list(properties.keys()) and silently dropping descriptions and defaults via _python_type_to_json_schema. Almost every tool in this file uses the shorthand form, so 23 tools advertised optional- with-default fields as required to the model. When the model trusted a documented default and omitted the field, the SDK threw McpToolCallError, which propagates up the stream and ends the agent's turn early; Nerve's engine then logs "Streaming completed with N stall(s)" and registers a background-task tracker, leaving users with the impression that the assistant stopped mid-task. The wrapper pre-promotes shorthand dicts to the explicit JSON Schema form the SDK passes through unchanged. The four explicit schemas already in the file (_NOTIFY_SCHEMA, _ASK_USER_SCHEMA, _REACT_SCHEMA, _SEND_STICKER_SCHEMA) are untouched. Side benefit: parameter descriptions now reach the model for every tool that previously relied on the shorthand. Adds tests/test_tools_schemas.py with 78 assertions covering wrapper behaviour and a parametrised audit of every tool in ALL_TOOLS.
Contributor
Author
|
Bumping for review. This is one of five small reliability fixes (#63, #64, #65, #66, #67) that compound: each closes one cause of chats that wedge mid-turn, lose context after restart, or get routed into the wrong session. Happy to split, rebase, or land any of them in any order; they don't depend on each other. In daily use the chat surface is the single biggest friction right now, so any of these merging would help. Let me know if you want changes. |
This was referenced May 13, 2026
pufit
pushed a commit
that referenced
this pull request
May 13, 2026
User-reported glitch: "I ask for something and there is no response, then I ask again and it answers to the previous question." Five reliability PRs (#63 shorthand-schema, #64 synthetic done, #65 stale sdk_session_id, #66 idle timeout, #67 sticky session) each close one underlying cause. Two gaps remain that none of those PRs cover. Gap 1: client-side send silently drops payloads. web/src/api/websocket.ts checked readyState === OPEN and no-op'd otherwise. The 3s reconnect window leaves a hole: send() returns to the caller and chatStore.sendMessage has already optimistically appended the user message and set isStreaming=true. The user thinks the agent is thinking but the message never reached the server, so the next reply lands against a stale prompt. Track readyState explicitly. CONNECTING or reconnect-scheduled now queues the payload (bounded to 5 entries; oldest evicted) and flushes from onopen. CLOSED-without-reconnect and CLOSING return 'dropped' so the caller can revert. chatStore.sendMessage pops the optimistic user message on 'dropped' and surfaces an inline assistant error so the user can retry. Gap 2: gateway initial-bind never replayed the broadcaster buffer. The switch_session handler already shipped session_status with buffered_events on session switch, but the initial-connect handshake at server.py:286-311 didn't. Reload mid-turn (or a transient 3s WS drop) and the in-flight stream was lost from the client's view even though the events sat in broadcaster._session_buffers waiting to be replayed. Lift the duplicated send-status construction into _send_session_status and call it from both branches. Initial-bind gates on broadcaster.is_buffering so idle sessions stay silent; switch_session calls unconditionally so the client refreshes is_running/status on every selection. The frontend handleSessionStatus already restores streamingBlocks, panels, todos, and interaction state from the buffer (handled by #69), so this is purely additive at the gateway. Tests: - 9 new asserts in tests/test_gateway_ws.py covering the helper output, the initial-bind gate, the switch_session regression path, and a load-fidelity check for buffer ordering. - Full pytest run: 444 pass, 2 skip, 2 pre-existing failures unrelated (test_bootstrap docker-env detection and test_cli_upgrade docker mode, both noted in notes/repo-conventions/nerve.md). - web/ tsc --noEmit clean, npm run build clean. Out of scope (followups, not blocking): - Stale-listener cleanup on swallowed send_json exceptions (server.py:298-301). - Application-level message_received ack from engine after sessions.add_message. - _session_locks TTL on session archive.
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
2 participants
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
The Claude Agent SDK's
_build_schema(claude_agent_sdk/__init__.py:393-413) converts shorthandinput_schemadicts by forcing every property intorequired: list(properties.keys())and silently dropping descriptions and defaults via_python_type_to_json_schema. Most ofnerve/agent/tools.pyuses the shorthand form, so 23 tools advertise optional-with-default fields as required to the model.This PR adds a thin wrapper around
claude_agent_sdk.toolthat pre-promotes shorthand dicts to the explicit JSON Schema form the SDK passes through unchanged.propertiesare kept intact (so descriptions and defaults survive) andrequiredonly lists fields that have nodefault. The four explicit schemas already in the file (_NOTIFY_SCHEMA,_ASK_USER_SCHEMA,_REACT_SCHEMA,_SEND_STICKER_SCHEMA) are untouched.The fix is one place; every existing and future tool that uses the shorthand decorator picks it up automatically.
Symptoms (when this has occurred)
When the model trusted a documented
defaultand omitted the field on a call, the bundled CLI threwMcpToolCallErrorinstead of returning a normalis_error: truetool result. The exception propagates up the streaming connection and ends the agent's turn early. From the user's POV the assistant just stopped mid-task. Recurring log signature:After the stream completes, Nerve's engine registers a background-task tracker, so the session looks "done" even though the model intended to continue. Tools that have repro'd this in the wild (recent logs) include
mcp__nerve__plan_proposeandmcp__nerve__memorize. Any tool with at least onedefault-annotated field is exposed:task_search,task_create,task_list,task_update,task_done,memory_recall,conversation_history,memory_records_by_date,memorize,memory_update,category_update,sync_status,list_sources,poll_source,poll_all_sources,read_source,plan_propose,plan_list,plan_decline,skill_create,skill_run_script,hoa_execute.Side benefit
The shorthand path also routes property values through
_python_type_to_json_schema, which has no dict branch and falls back to{"type": "string"}— so descriptions and defaults are stripped before the model ever sees them. Promoting shorthand to explicit schema fixes that as well; the model now receives the parameter descriptions Nerve has been writing for months.Test plan
pytest tests/test_tools_schemas.py -v— 78 passed (4 wrapper behaviour cases + parametrised audit of every tool inALL_TOOLSfortype/properties/requiredcorrectness and "no defaulted field is required").pytest tests/test_session_mcp.py tests/test_engine.py— 26 passed, no regressions.pytest -q(full suite, excluding network-bound CLI tests) — 509 passed; 2 pre-existing failures intest_bootstrap.pyandtest_services.pyrelated to the docker-mcp spike, unrelated to this change.Out of scope
Bug 2 from the original analysis — that
McpToolCallErroris raised at all on validation failures instead of being returned asis_error: true— lives in the bundled Claude Code CLI (/$bunfs/root/src/entrypoints/cli.js:4894), not Nerve. This PR removes the most common trigger; the upstream fix would prevent any future schema mismatch from terminating a turn.