fix: empty a2a message

[Lancetnik]

Related issue number

Bassils' report about A2A empty message

raise A2AClientJSONRPCError(response.root)
a2a.client.errors.A2AClientJSONRPCError: JSON-RPC Error code=-32603 data=None message='TextPart content cannot be empty'

We should fix it. Empty-content messages are fine for our framework. We are using message metadata to pass info about context and tools using, so the body can be empty.

Checks

• I've included any doc changes needed for https://docs.ag2.ai/. See https://docs.ag2.ai/latest/docs/contributor-guide/documentation/ to build and test documentation locally.
• I've added tests (if relevant) corresponding to the changes introduced in this PR.
• I've made sure all auto checks have passed.

[joggrbot]

📝 Documentation Analysis

All docs are up to date! 🎉

---

✅ Latest commit analyzed: f485964 | Powered by Joggr

[claude]

Code Review - PR #2163: Fix empty A2A message

Thank you for addressing the empty message issue in A2A! I've reviewed the changes and have the following feedback:

✅ Strengths

1. Problem correctly identified and fixed: The manual construction of the Task object in autogen/a2a/agent_executor.py properly addresses the issue where empty-content messages were causing JSON-RPC errors. This is a valid fix for supporting metadata-only messages.

2. Good test coverage: The addition of test_empty_message_send provides clear verification that empty messages are now handled correctly. The test follows the existing pattern and integrates well with the test suite.

3. Code simplification in client_utils.py: The refactoring of merge_config_with_tools is cleaner and more concise. Reducing intermediate variables improves readability.

🔍 Observations & Suggestions

1. agent_executor.py - Task construction (Lines 38-46)

The manual Task construction duplicates logic that likely exists in the new_task utility. Consider these points:

• Rationale documentation: The comment "build task object manually to allow empty messages" is helpful, but it would be valuable to add a more detailed explanation. Why does new_task fail with empty messages? Is this a validation in the a2a library itself?

• Potential for upstream fix: If new_task from the a2a library is the one enforcing the non-empty constraint, consider whether this could/should be fixed upstream in the a2a library rather than working around it here.

• UUID generation consistency: The fallback UUID generation (request.task_id or str(uuid4())) appears reasonable, but ensure this matches the behavior of the original new_task function.

2. client_utils.py - Logic change (Lines 132-135)

While the refactoring looks cleaner, there's a subtle potential behavioral change:

Before:

config_tools = config.get("tools", [])
client_tools = client_config.get("tools", [])

if config_tools or client_tools:
    if "functions" not in full_config:
        full_config["tools"] = config_tools + client_tools

After:

tools = config.get("tools", []) + client_config.get("tools", [])
if tools and "functions" not in full_config:
    full_config["tools"] = tools

The logic appears equivalent, but the key difference is the condition evaluation:

• Old: Checked if config_tools OR client_tools is truthy separately
• New: Checks if the merged tools list is truthy

This should be functionally identical for lists, but wanted to call this out for verification. The new version is more Pythonic.

3. Test coverage for client_utils.py

The merge_config_with_tools function in client_utils.py is modified but there don't appear to be dedicated tests for this function in test/oai/test_client_utils.py. Consider adding unit tests that verify:

• Merging tools from both configs
• Not adding tools when functions parameter is present
• Handling empty tools arrays
• Various edge cases

4. Missing documentation checkbox

The PR description has the documentation checkbox unchecked. While this change might not require user-facing documentation, consider whether:

• Internal documentation about A2A message handling should be updated
• Any developer guides about A2A integration should mention that empty messages are supported

🔒 Security & Performance

• ✅ No security concerns identified
• ✅ No performance implications expected
• ✅ UUID generation is using proper uuid4() for uniqueness

📋 Minor Nits

1. Import order: The new import of uuid4 is correctly placed, maintaining alphabetical ordering within the standard library imports.

2. Type consistency: The code maintains consistency with the existing codebase's type handling.

Summary

The core fix is solid and addresses the reported issue effectively. The main recommendation is to add tests for merge_config_with_tools to ensure the refactoring doesn't introduce subtle bugs, and to consider whether the empty message validation could be addressed upstream in the a2a library itself.

Recommendation: ✅ Approve with minor suggestions

[claude]

Pull Request Review: Fix Empty A2A Message

Summary

This PR addresses an important bug where A2A (Agent-to-Agent) messages with empty content were causing A2AClientJSONRPCError exceptions. The fix allows empty message bodies, which is valid in AG2 since metadata can carry context and tool information independently of the message content.

Code Quality & Best Practices

✅ Strengths

1. Well-documented change: The PR description clearly explains the issue and rationale
2. Appropriate test coverage: Added test_empty_message_send to verify the fix works
3. Minimal, focused changes: The PR addresses the specific issue without unnecessary refactoring
4. Good commit messages: Clear, conventional commit format ("fix:", "lint:", "refactor:")

🔍 Observations & Suggestions

1. Manual Task Construction (autogen/a2a/agent_executor.py:38-46)

The replacement of new_task(context.message) with manual Task object construction is reasonable given the constraint, but consider:

• Documentation: Add a comment explaining why new_task() cannot be used (i.e., it validates non-empty content)
• Upstream fix: Consider filing an issue with the a2a library to support empty messages in new_task(), as this is valid for metadata-carrying messages

Suggested improvement:

# Build task object manually to allow empty messages.
# new_task() validates TextPart content as non-empty, but AG2 allows
# empty content when metadata carries context/tool information.
task = Task(
    # ... rest of the code
)

2. Code Simplification (autogen/oai/client_utils.py:132-135)

The refactoring is cleaner and more Pythonic. However, there's a subtle behavioral change:

Before:

if config_tools or client_tools:  # Checks if either list is truthy
    if "functions" not in full_config:
        full_config["tools"] = config_tools + client_tools

After:

tools = config.get("tools", []) + client_config.get("tools", [])
if tools and "functions" not in full_config:  # Only checks combined list
    full_config["tools"] = tools

Potential edge case: If config_tools = None and client_tools = [], the old code would skip the block, while the new code would attempt None + [] earlier. However, since you're using .get("tools", []), this is handled correctly. ✅

Minor suggestion: The logic is sound, but if you wanted to be extra explicit about the intent:

# Merge tools from both configs if present and not using deprecated functions API
tools = config.get("tools", []) + client_config.get("tools", [])
if tools and "functions" not in full_config:
    full_config["tools"] = tools

3. Metadata Key Prefix Refactoring (autogen/a2a/utils.py:13-15)

Good refactoring for DRY (Don't Repeat Yourself) principles. The prefix constant makes it easier to identify AG2-specific metadata keys.

Suggestion: Consider adding a brief docstring or comment explaining the purpose:

# AG2-specific metadata keys are prefixed to avoid conflicts with other A2A implementations
AG2_METADATA_KEY_PREFIX = "ag2_"

Potential Issues

⚠️ Minor Concerns

1. Import changes: The PR removes from a2a.utils import new_task and adds from uuid import uuid4 and from a2a.types import Task. These are necessary for the fix, but ensure new_task isn't used elsewhere in the codebase without proper handling.

2. Test coverage completeness: The new test test_empty_message_send verifies that empty messages work end-to-end, which is excellent. Consider adding:

   • A test for messages with only metadata (no parts at all)
   • Edge case: empty string vs None vs missing content field

🔒 Security Concerns

No security issues identified. The changes:

• Don't introduce any input validation bypasses
• Don't expose sensitive data
• Properly handle the empty message case without introducing vulnerabilities

Performance Considerations

✅ No performance concerns. The changes are:

• Constructing objects directly instead of through a utility function (minimal difference)
• Simplifying boolean logic (micro-optimization, if anything)
• No additional loops, allocations, or blocking operations

Test Coverage

✅ Test Quality

The new test test_empty_message_send is well-structured:

• Uses existing test infrastructure (TestAgent, fixtures)
• Clear arrange-act-assert pattern
• Tests the actual bug scenario (empty content string)
• Verifies the response is still received correctly

📋 Suggestions for Additional Tests

Consider testing these edge cases:

@pytest.mark.asyncio()
async def test_message_with_only_metadata(remote_agent, a2a_client):
    """Test that messages can carry information via metadata alone."""
    # Test with metadata but no content parts
    
@pytest.mark.asyncio()
async def test_none_vs_empty_content(remote_agent, a2a_client):
    """Verify None and '' are handled consistently."""

Summary & Recommendation

✅ Approve with minor suggestions

This is a solid fix for a real bug. The code changes are minimal, well-tested, and address the root cause. The refactoring in client_utils.py and utils.py improves code quality without introducing risks.

Action Items (Optional Improvements)

1. Add explanatory comments for why manual Task construction is necessary
2. Consider filing an upstream issue with a2a library about new_task() validation
3. Add edge case tests for metadata-only messages
4. Verify new_task() removal doesn't affect other parts of the codebase

Final Notes

Great job on:

• Clear problem statement in the PR description
• Minimal, surgical fix
• Including appropriate test coverage
• Following conventional commit format

The codebase will be more robust with this fix! 🎉

---

Review generated with Claude Code

[Lancetnik]

@marklysze can we merge this one? Bassil told, that everything fine now

[qingyun-wu]

LGTM! Thanks @Lancetnik

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
autogen/oai/client_utils.py	33.33%	1 Missing and 1 partial ⚠️

Files with missing lines	Coverage Δ
autogen/a2a/agent_executor.py	84.84% <100.00%> (ø)
autogen/a2a/utils.py	94.20% <100.00%> (+0.08%)	⬆️
autogen/oai/client_utils.py	92.18% <33.33%> (+1.27%)	⬆️

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.