Skip to content

Preserve inbound A2A message role instead of hardcoding assistant - #1122

Open
PratikDhanave (PratikDhanave) wants to merge 4 commits into
microsoft:mainfrom
PratikDhanaveFork:fix/a2a-inbound-role-mapping
Open

PratikDhanave (PratikDhanave) wants to merge 4 commits into
microsoft:mainfrom
PratikDhanaveFork:fix/a2a-inbound-role-mapping

Conversation

@PratikDhanave

Copy link
Copy Markdown
Contributor

The client inbound path passed a literal message.RoleAssistant for A2A messages (*a2a.Message) and task-status messages, discarding the role carried on the inbound message. So a peer message with the A2A user role was reported as assistant.

The hosting side already maps this (toAgentMessage: agent → assistant, otherwise user), and the Python client does the same (role="assistant" if msg.role == A2ARole.ROLE_AGENT else "user") — the client-path hardcode was an oversight.

Change

  • Add an agentRole helper and use it for the *a2a.Message case and the two task-status-message cases (streaming TaskStatusUpdateEvent and yieldTask input-required). Artifacts carry no role, so they stay assistant.

Test

  • TestRunPreservesInboundUserRole: a response *a2a.Message with MessageRoleUser now maps to RoleUser. Fails before (assistant), passes after. The existing MessageRoleAgent → assistant assertion still holds.

The client inbound path hardcoded message.RoleAssistant for A2A messages and
task-status messages, discarding the actual A2A role. A peer message with the
user role was reported as assistant. Map the role via a shared agentRole
helper (agent -> assistant, otherwise user), matching the hosting-side
toAgentMessage and the Python client. Artifacts carry no role and stay
assistant.
Copilot AI lite review requested due to automatic review settings September 20, 2026 04:30
@github-actions github-actions Bot added area:provider Changes files in the provider area area:provider/a2a Changes files in the provider / a2a area size:medium At most 100 changed lines across at most 5 files labels Sep 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Streaming task status messages can still report inbound user roles as assistant.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Preserves inbound A2A message roles when converting responses into framework messages.

Changes:

  • Added shared A2A-to-framework role mapping.
  • Applied role preservation to messages and task-status updates.
  • Added a user-role response test.
File Description
provider/​a2aprovider/​a2a.go Adds role mapping for inbound responses.
provider/​a2aprovider/​a2a_test.go Tests preservation of user-role messages.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

role := message.RoleAssistant
if e.Status.Message != nil {
messageID = e.Status.Message.ID
role = agentRole(e.Status.Message.Role)
Comment on lines +303 to +305
// A response message carrying the A2A user role must map to RoleUser, not be
// hardcoded to assistant, matching the Python client.
func TestRunPreservesInboundUserRole(t *testing.T) {
@github-actions github-actions Bot added kind:code Changes production behavior or code kind:tests Changes tests, fixtures, or test infrastructure labels Sep 20, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions Bot added the parity-approved Go API consistency review found no parity issues label Sep 20, 2026
Add a role assertion for the input-required task-status message path
(yieldTask) so the user-role mapping cannot regress, and restore the
TestRunWithValidUserMessage doc comment to its own test.
@github-actions github-actions Bot added size:large At most 300 changed lines across at most 10 files and removed size:medium At most 100 changed lines across at most 5 files labels Sep 21, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions Bot added the kind:dependencies Changes dependencies or manifests label Sep 21, 2026
…e-mapping

# Conflicts:
#	go.mod
#	go.sum
#	provider/a2aprovider/a2a.go
@github-actions github-actions Bot added size:medium At most 100 changed lines across at most 5 files and removed size:large At most 300 changed lines across at most 10 files labels Oct 3, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions Bot removed the kind:dependencies Changes dependencies or manifests label Oct 3, 2026
@qmuntal
Quim Muntal (qmuntal) added this pull request to the merge queue Oct 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 5, 2026
…e-mapping

# Conflicts:
#	provider/a2aprovider/a2a.go
#	provider/a2aprovider/a2a_test.go
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

API consistency review

Result Scope
aligned user-visible behavior

Changed Go contract

No exported API changes. The A2A client (provider/a2aprovider/a2a.go) now maps the inbound a2a.Message role and the role of task-status messages. It uses a new unexported agentRole helper (agent → assistant, otherwise user) instead of always returning RoleAssistant. Artifact updates stay assistant.

Upstream evidence reviewed

Implementation Source Contract checked
Python python/packages/a2a/agent_framework_a2a/_agent.py The file references A2ARole.ROLE_AGENT when converting inbound messages. This matches the mapping the PR cites.
.NET A2AAgent.cs at dc6617a ConvertToAgentResponseUpdate sets ChatRole.Assistant on streaming updates. The non-streaming path uses message.ToChatMessage() from the Extensions/ directory, which I did not inspect.

Assessment

The Go change matches the Python client and Go's own hosting-side toAgentMessage. It has no feature gate or default change. The .NET streaming path hardcodes assistant, so .NET and Python already differ from each other. The Go fix follows Python and the A2A role semantics, so I am not reporting it as a parity issue. I did not review the .NET Extensions/ message conversion because code search was rate-limited.

Generated by Go API Consistency Review Agent · copilot · auto · 37.5 AIC · ⌖ 0.621 AIC · ⊞ 5.1K · ◷

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

Labels

area:provider/a2a Changes files in the provider / a2a area area:provider Changes files in the provider area kind:code Changes production behavior or code kind:tests Changes tests, fixtures, or test infrastructure parity-approved Go API consistency review found no parity issues size:medium At most 100 changed lines across at most 5 files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants