[codex] Sanitize empty optional MCP arguments

[Tobi1chi]

Summary

• sanitize MCP tool arguments before dispatch so empty optional payload values are omitted
• preserve the reconnect-and-retry path while sending cleaned arguments to the MCP session
• add debug logging so sanitized payload changes are visible when tracing tool calls

Why

Some MCP servers reject requests when optional arguments are present but empty. This change strips empty placeholder values before the tool call so those optional fields are omitted instead of being sent as invalid payload.

Validation

• ./.venv-precommit/bin/pre-commit run --from-ref upstream/master --to-ref HEAD

Summary by Sourcery

Sanitize MCP tool call arguments to avoid sending empty optional values that some servers reject.

Bug Fixes:

• Strip empty and null optional MCP tool arguments before dispatch so servers do not reject requests with invalid payloads.

Enhancements:

• Log sanitized MCP tool arguments at debug level to make payload transformations visible when tracing tool calls.

[gemini-code-assist]

Code Review

This pull request introduces a recursive sanitization function for MCP tool arguments to remove null values, empty strings, and empty collections. A review comment identifies an inconsistency where empty dictionaries are not handled the same as empty lists and suggests a more idiomatic implementation using comprehensions.

[gemini-code-assist]

There's an inconsistency in how empty collections are handled: empty lists are converted to _EMPTY_MCP_ARGUMENT, but empty dictionaries are returned as is. This could lead to unintentionally sending empty dictionary objects as arguments, which this PR aims to prevent. The behavior should be consistent for both types of collections.

Additionally, the implementation for both lists and dictionaries can be made more concise by using comprehensions with assignment expressions (the := operator), assuming your project uses Python 3.8+.

The suggested change below addresses both the inconsistency and the opportunity for more idiomatic code.

    if isinstance(value, list):
        cleaned_items = [
            cleaned_item
            for item in value
            if (cleaned_item := _sanitize_mcp_arguments(item)) is not _EMPTY_MCP_ARGUMENT
        ]
        return cleaned_items if cleaned_items else _EMPTY_MCP_ARGUMENT

    if isinstance(value, dict):
        cleaned_dict = {
            key: cleaned_item
            for key, item in value.items()
            if (cleaned_item := _sanitize_mcp_arguments(item)) is not _EMPTY_MCP_ARGUMENT
        }
        return cleaned_dict if cleaned_dict else _EMPTY_MCP_ARGUMENT

[Tobi1chi]

Updated in 4200317e.

I kept the top-level fallback to {} because MCP tool calls still need an object payload when every argument is omitted, but nested empty collections are now handled consistently: empty dicts and empty lists both collapse to the sentinel and get removed from the parent payload.

I also added unit coverage for both cases:

• nested empty collections are removed from the payload
• a fully-sanitized top-level argument object still falls back to {} before dispatch

Validation:

• uv run pytest tests/unit/test_mcp_client.py
• ./.venv-precommit/bin/pre-commit run --files astrbot/core/agent/mcp_client.py tests/unit/test_mcp_client.py