microsoft / BotBuilder-Samples

Welcome to the Bot Framework samples repository. Here you will find task-focused samples in C#, JavaScript/TypeScript, and Python to help you get started with the Bot Framework SDK!
https://github.com/Microsoft/botframework
MIT License
4.34k stars 4.86k forks source link

I can send proactive messages from a skill #2312

Open gabog opened 5 years ago

gabog commented 5 years ago

Background

Several teams have reported issues while trying to send proactive messages from a skill.

The goal of this issue is to build an experimental sample that validates a proactive skill scenario. To that purpose, we will implement a consumer, a skill and an alarm service that can do the following:

A user asks the consumer bot to setup and alarm and the consumer bot starts the Alarm skill. The skill prompts the user for information on when to set the alarm: date, time purpose. The skill sends a message to a custom alarm service to schedule the alarm and tells the consumer bot that is done for now. When the time comes, the Alarm service send a message to a custom endpoint on the skill and the skill sends a proactive message to the consumer with the alarm message.

Once this sample is written, we would fully understand any gaps in skills and create will create issues in the language repos to address.

Extra credit: see if we can determine how to handle a proactive message from a skill while the parent is in a different dialog.

The sample should involve the following artifacts:

Alarm skill

The skill will allow the user to setup an alarm, it will prompt the user for a date and time and, once it has all the information, it will send a message to the Alarm service with the information it needs to trigger the alarm and send a proactive message back to the skill when the time comes.

Alarm service

The Alarm is a standard service (not a bot) that will take the message from the skill and store it in a scheduled task that will trigger at the appropriate time. When the time is up, the service will send a message to a custom endpoint on the skill (no auth required here).

Consumer bot

The consumer bot will start the Alarm skill when the user says "set alarm" and hand off messages to the skill until the skill is done. The consumer bot will also handle messages from the skill when the alarm triggers.

Implementation notes

Tracking Status for each platform

Dotnet

Javascript

Python

guilhermecaixeta commented 4 years ago

Has some prevision about this implementation?

tracyboehrer commented 4 years ago

It would seem to me that the issue of how a bot generates an event (background service, webhook, etc...) is separate from how a skill notifies a parent. If we want to provide tools for doing this part, I would think it would apply to any bot, not just a Skill? Furthermore, can we even provide a meaningful abstraction for this given each platform will be different? Would not a sample that shows a bot with a background service be more appropriate? Our current proactive sample demonstrates a webhook style trigger (which I used in a proof of concept GitHub bot which hooked up via a webhook to GitHub).

Beyond that... What does a Skill need to communicate back to the parent? The same thing proactive needs? A ConversationReference (either from a previous conversation, or construct one from config (or wherever)). Or is there another way?

clearab commented 4 years ago

I think a good scenario would be: I'm the ToDo bot, and I am sometimes a skill, and sometimes a stand-alone bot. I need to be able to send a reminder to a user regardless of whether that user has used me through another bot as a skill, or as a stand-alone bot.

Ideally, ToDo bot wouldn't need to do anything drastically different than it would operating as a stand-alone bot in order to facilitate this. Is the ConversationReference sufficient for the consumer to fwd the message to the appropriate user?

stevengum commented 4 years ago

The ToDo bot/skill would also need the Audience. When ToDo is acting as a bot, the audience is not required, unless it is messaging both public and gov cloud channels.

When ToDo bot is acting as a skill, it needs to provide the Audience (aka the Consumer's AppId) in the token, otherwise the Consumer will likely reject the request.

In R8 we determined that the ConversationReference and the Audience should be stored for all proactive messages, not just bot-to-bot messaging.


Example, a user asks the skill to notify her as soon as a email from John Joe arrives. The skill should be able to send a proactive message to the Parent Bot when the event occurs...

This is already possible...

and (optionally) the user should be able to start a conversation with the skill (if the scenario requires it)

If I understand the above correctly, this has no wiring in BotBuilder SDKs. However, it would be interesting to support. This support would possibly hinge on things such as the parent using a SkillDialog, if the parent is using a SkillHandler, etc. Does the skill send "expectedReplies" or some new flag indicating it wants to be on the top of the stack now?

The VA supports some of this by using the IBackgroundTaskQueue, BackgroundTaskQueue and QueuedHostedService classes

To me this sounds like application-level code, as @tracyboehrer indicated. We could reach for a decent abstraction, but I'm not sure it's worth pursuing in R9. .NET already provides the interfaces required, and Microsoft.Bot.Solutions provides implementations.

stevengum commented 4 years ago

The Audience/ConversationReference work was done in R8 (initially in C#), and the struct we use to contain the necessary fields is the SkillConversationReference

tracyboehrer commented 4 years ago

Does this mean this is a samples issue, and not an SDK one?

stevengum commented 4 years ago

I believe a sample demonstrating how to have a skill send real "proactive" messages could address the issue.

@darrenj do you have any "Skills proactive" pain points that could be better addressed in the SDK and not in the Solutions packages?

tracyboehrer commented 4 years ago

So in terms of planning, it's no longer an SDK issue and can be pushed to the samples phase and repo?

danlavrushko commented 4 years ago

Example, a user asks the skill to notify her as soon as a email from John Joe arrives. The skill should be able to send a proactive message to the Parent Bot when the event occurs...

This is already possible...

@stevengum any samples of proactive messages from Skill to Parent Bot?

it looks like that any proactivity by Skill bot will receive 401 from Parent Bot because of missing claims and possibly outdated conversation reference

WizX20 commented 4 years ago

Example, a user asks the skill to notify her as soon as a email from John Joe arrives. The skill should be able to send a proactive message to the Parent Bot when the event occurs...

This is already possible...

@stevengum any samples of proactive messages from Skill to Parent Bot?

it looks like that any proactivity by Skill bot will receive 401 from Parent Bot because of missing claims and possibly outdated conversation reference

Same issue here. When I use the skill directly all works well. But when I'm calling the skill via my VA it will fail with an unauthorized exception (all authentication is off).

Also, when fetching a new token via login.microsoftonline.com and applying that as bearer auth header does not work. Example: var authToken = await this.GetAuthToken(conversation); connectorClient.HttpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", authToken);

So in my conclusion, it must be something in the VA not allowing activities.

stevengum commented 4 years ago

@danlavrushko, which version of the SDK are you using and which channels are you targetting?

@WizX20 I'm not familiar with the VA code base, but currently skills don't play well with no-auth scenarios: https://github.com/microsoft/botframework-sdk/issues/5726

gabog commented 3 years ago

FYI: reworded the original issue so we can narrow it down to the basics, a skill can proactively send a message to a user that has already engaged previously with the bot (I think we have enough challenges there). We don't have what we need to have a skill start a conversation with a user that has never talked to the skill, we will address this as part of another issue.

gabog commented 3 years ago

We've done the groundwork to validate that we can effectively send proactive messages from a skill as part of our Functional Tests effort.

The sample is in Fn tests repo. The WaterfallHost can talk to the WaterfallSkillBot and request a proactive message.

In order to support proactive messages, one of the main differences with the proactive sample is that is that the skill needs to remember the claims of the caller and the OAuthScope in order to resume the conversation (we probably don't need the entire set of claims but we can refine that later).

In the functional tests we created a ContinuationParameters type to store this data in a conversationReferences dictionary that we can use to resume the conversation using the adapters ContinueConversationAsync method (this call is in the ProactiveController class in the sample).

The test uses a dialog that can be extended to continue a conversation when the proactive message is received but doesn't account for the skill dialog stack being in a different state than the one expected. This should be doable but requires additional logic to manage the dialog stack on the skill and should handle the proactive message as an interrutpion and not as a continuation.

We didn't implement the alarm logic described in this issue but it should be possible.

I am going to move this issue to the backlog and we can address the specific alarm scenario later if this gets prioritized.

Note: we fixed a bug on the Waterfall test that will be merged as part of this PR in the next week or sooner.