Skip to content

Conversation

@yashwantbezawada
Copy link

Summary

Fixes #1063 - MCP Tools receive empty arguments when using permission approval flow

Problem

When a permission callback returns PermissionResultAllow() without the updated_input parameter, the SDK incorrectly includes "updatedInput": {} in the control_response. This causes Claude CLI to discard the original tool arguments and use an empty object instead, breaking all multi-parameter MCP tools.

Root Cause

In query.py:238-242, the SDK always included updatedInput in the response:

"updatedInput": (
    response.updated_input
    if response.updated_input is not None
    else original_input  # Bug: Falls back to original_input
),

This meant updatedInput was always present, even when the user didn't set it.

Solution

Only include updatedInput in the control_response when explicitly provided:

response_data = {"behavior": "allow"}
# Only include updatedInput if explicitly provided by the user
if response.updated_input is not None:
    response_data["updatedInput"] = response.updated_input

Changes

  1. query.py: Modified permission response handling to conditionally include updatedInput
  2. test_tool_callbacks.py: Added regression test to verify updatedInput is omitted when not provided

Testing

  • Added explicit regression test for issue #1063
  • Existing test for input modification still passes
  • CI will verify all tests pass

Before & After

Before (buggy):

{
  "type": "control_response",
  "response": {
    "behavior": "allow",
    "updatedInput": {}  // Always included, overwrites arguments
  }
}

After (fixed):

{
  "type": "control_response",
  "response": {
    "behavior": "allow"  // updatedInput only included when explicitly set
  }
}

Impact

  • Fixes all MCP tools that broke with permission approval
  • Maintains backward compatibility (updatedInput still included when set)
  • No breaking changes to the API

… callbacks

Fixes #1063

When a permission callback returns `PermissionResultAllow()` without
the `updated_input` parameter, the SDK should not include `updatedInput`
in the control_response. Previously, the SDK always included this field,
either with the modified input or falling back to the original input,
which caused MCP tools to receive empty {} arguments instead of Claude's
original arguments.

Changes:
- Modified query.py to only include updatedInput when explicitly set
- Added regression test to verify updatedInput is omitted when not provided
- Existing test for input modification still passes

Before this fix:
```json
{
  "behavior": "allow",
  "updatedInput": {}  // Bug: Always included even when not set
}
```

After this fix:
```json
{
  "behavior": "allow"  // updatedInput only included when explicitly provided
}
```
@ashwin-ant
Copy link
Collaborator

Thanks for the PR, but I don't think this change is correct.

The SDK protocol requires updatedInput to always be present in allow responses. With this PR, when PermissionResultAllow() is returned without updated_input, we'd end up sending something that fails validation and gets treated as a denied permission.

The current code is actually correct. It always sends updatedInput, falling back to original_input when the user doesn't explicitly provide one.

I tested this locally with an MCP server that has a multi-parameter tool and a can_use_tool callback returning PermissionResultAllow(), and the arguments flow through correctly. The tool receives the expected parameters.

If there's a bug where MCP tools receive empty arguments, the issue is likely elsewhere, perhaps in how original_input is being captured or passed through. Could you share a repro case showing where {} comes from?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants