Skip to content

Conversation

@ShanmathiMayuramKrithivasan
Copy link
Collaborator

@ShanmathiMayuramKrithivasan ShanmathiMayuramKrithivasan commented Dec 4, 2025

Problem

App.Send() cannot send targeted messages because it builds ConversationReference, dropping the required User field that specifies the recipient id for targeted messages.

Fix

  • Set ConversationReference.User = activity.Recipient when isTargeted=true in App.Send()
  • This ensures the sender plugin can properly set the outgoing activity's recipient
  • Maintains backward compatibility for non-targeted messages (User remains null)

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the App's Send method to properly support targeted messages by ensuring the ConversationReference User field is set when sending targeted activities. The fix addresses the issue where targeted messages weren't being delivered to the correct recipient because the User field in the ConversationReference was not being populated.

Key changes:

  • Added validation to ensure activity.Recipient is provided when sending targeted messages
  • Set ConversationReference.User to activity.Recipient for targeted messages, which enables proper message routing through the sender plugin

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +196 to +199
if (isTargeted && activity.Recipient is null)
{
throw new ArgumentException("activity.Recipient is required for targeted messages", nameof(activity));
}
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The new validation logic for targeted messages lacks test coverage. Consider adding tests to verify:

  1. An ArgumentException is thrown when isTargeted is true but activity.Recipient is null
  2. The validation passes when both isTargeted is true and activity.Recipient is set
  3. No exception is thrown when isTargeted is false regardless of activity.Recipient value

These tests would ensure the validation behaves correctly under all scenarios.

Copilot uses AI. Check for mistakes.
Name = Name,
Role = Role.Bot
},
User = isTargeted ? activity.Recipient : null,
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

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

The new User field assignment logic lacks test coverage. Consider adding tests to verify:

  1. When isTargeted is true, reference.User is set to activity.Recipient
  2. When isTargeted is false, reference.User is set to null
  3. The ConversationReference is passed correctly to the sender plugin with the User field properly set

This would ensure that targeted messages are routed to the correct recipient.

Copilot uses AI. Check for mistakes.
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.

1 participant