-
Notifications
You must be signed in to change notification settings - Fork 48
Cards sample using AgentApplication #167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # src/Microsoft.Agents.SDK.sln
…racyboehrer/cards-sample
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 16 out of 22 changed files in this pull request and generated 2 comments.
Files not reviewed (6)
- src/Microsoft.Agents.SDK.sln: Language not supported
- src/samples/HandlingCards/HandlingCards.csproj: Language not supported
- src/samples/HandlingCards/Resources/DynamicSearchCard.json: Language not supported
- src/samples/HandlingCards/Resources/StaticSearchCard.json: Language not supported
- src/samples/HandlingCards/appManifest/manifest.json: Language not supported
- src/samples/HandlingCards/appsettings.json: Language not supported
Comments suppressed due to low confidence (1)
src/samples/HandlingCards/Cards.cs:18
- The initialization of _cardCommands is incorrect. Replace the repeated assignment and square brackets with a proper collection initializer, e.g., 'new List { ... }'.
static IList<CardCommand> _cardCommands = _cardCommands = [
{ | ||
await adapter.ProcessAsync(request, response, agent, cancellationToken); | ||
}) | ||
.AllowAnonymous(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AllowAnonymoys should be added when IsDevelopment==true
src/samples/HandlingCards/Program.cs
Outdated
.AllowAnonymous(); | ||
|
||
// Setup port and listening address. | ||
app.Urls.Add("http://localhost:5000"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use defaults, that can be overriden with launchSettings?
why not 3978?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file produces Warnings in VS
Warning Property name is not allowed by the schema HandlingCards D:\Agents-for-net\src\samples\HandlingCards\Resources\DynamicSearchCard.json 37
Warning Value must conform to at least one of the associated schemas
| Value must be one of the following values: "compact", "expanded"
| or
| Value must match the regular expression ^([c|C][o|O][m|M][p|P][a|A][c|C][t|T])|([e|E][x|X][p|P][a|A][n|N][d|D][e|E][d|D])$ HandlingCards D:\Agents-for-net\src\samples\HandlingCards\Resources\DynamicSearchCard.json 48
When running the sample on Teams, I got two issues
Error: ReplyToActivity operation returned an invalid status code '(400) BadRequest' |
I was wondering this sample shows the value of: Agents-for-net/src/libraries/Builder/Microsoft.Agents.Builder/App/AgentApplicationBuilder.cs Line 50 in b74150f
However since it's not using the builder, I guess this has to be configured via appSettings, and seems the sample does not require it. In which cases the Application requires WithAdaptiveCardOptions ? |
I don't know. This portion of the sample came from the Teams AI SDK adaptiveCards.a.typeAheadBot. They didn't explicitly set that either. i.e., It just uses the default of "verb". |
I noticed the AdaptiveCard is always configured in the ctor Agents-for-net/src/libraries/Builder/Microsoft.Agents.Builder/App/AgentApplication.cs Line 58 in b74150f
Do we still need the "WithAdaptiveCards" method in the builder? |
Well I don't think we even need Builder. But that is another discussion. The entire thing is there for as much back-compat as we could leave. It would appear |
Also, this sample doesn't show OnExecute yet either. |
Does a ReceiptCard show in Teams using JS? Or BF SDK? I suspect not even if the Channels doc indicates it does. This is an awkward sample. Several cards don't work in Teams, and why I added more "Channels.Supports*" methods. |
Will need to ping the Teams folks about ReceiptCard. Some other cards did that same Dropped support for AnimationCard on Teams since the doc indicates not supported. |
# Conflicts: # src/Microsoft.Agents.SDK.sln
@rido-min Update from branch. ReceiptCard will still be outstanding. |
# Conflicts: # src/Microsoft.Agents.SDK.sln
This, like the one in JS, is an adaptation of the one from BF, without Dialogs, and with the addition of AgentApplication.AdaptiveCards.
This also adds some helper methods in
Channels
to determine support for cards. Teams doesn't support several.Note: This uses the new style of
Program.cs
that Matt used in the Auth samples.