protocol JSON schema: required fields of ConversationAccount don't match written specification

[drmason13]

Github issues should be used for bugs and feature requests. Use Stack Overflow for general "how-to" questions.

Version

commit 0fa7c51 of the JSON schema (itself unversioned as far as I'm aware)

Describe the bug

The JSON schema doesn't match the written specification at https://github.com/microsoft/botframework-sdk/blob/main/specs/botframework-activity/botframework-activity.md#conversation.

ConversationAccount requries all of id, name, isGroup and conversationType in the JSON schema.

Whereas the written specification explicitly states:

A2080: Channels, bots, and clients MUST include the conversation and conversation.id fields when generating an activity.

The conversation.name field is optional and represents the display name for the conversation if it exists and is available.

A2081: Channels SHOULD include the conversation.name and conversation.isGroup fields if they are available.

A2082: Bots and clients SHOULD NOT include the conversation.name field unless it is semantically valuable within the channel.

A2083: Bots and clients SHOULD NOT include the conversation.isGroup and conversation.converationType fields in activities they generate.

A2084: Channels SHOULD include the conversation.conversationType field if more than one value is defined for the channel. Channels SHOULD NOT include the field if there is only one possible value.

name being a required field doesn't match "The conversation.name field is optional", for a start.

To Reproduce

Steps to reproduce the behavior:
This can be reproduced using the emulator, see microsoft/BotFramework-Emulator#2397

Expected behavior

Only id field in ConversationAccount should be required in the JSON Schema definition.
The following JSON should be a valid ConversationAccount:

"conversation": {
    "id": "40f7b490-2609-11ed-a12d-37cd6a1cd89f|livechat"
}

The JSON schema should be able to validate activities that meet the specification generated by trusted/well behaved clients such as the emulator.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

Add any other context about the problem here.

Tracking Status

Dotnet SDK TODO

• PR
• Merged

Javascript SDK TODO

• PR
• Merged

Python SDK TODO

• PR
• Merged

Java SDK TODO

• PR
• Merged

Samples TODO

• PR
• Merged

Docs TODO

• PR
• Merged

Tools TODO

• PR
• Merged

[LeeParrishMSFT]

@johnataylor this is related to the issue you are assigned to here: microsoft/BotFramework-Emulator#2397

[johnataylor]

@drmason13 thanks for highlighting the inconsistencies. I'm trying to determine what would be the most effective approach to resolving this, perhaps you could provide some input to the process. I'm wondering whether it could be resolved in the implementation or the spec. I don't think there are other implementations outside of the Azure Bot Service and the Bot Framework SDK. But we have to avoid shipping any breaking changes to implementation on a minor release. At the same time, we would want to be mindful of the original intentions captured in the spec. For example, how is this impacting your particular scenario?

[drmason13]

Thanks, the impact on me personally is minimal. I have edited a local copy of the JSON schema and used that to generate client API bindings, and I have been editing those by hand since.

I think the implementations are actually correct, according to the spec and it would be unwise to change them now.

I think the JSON schema (as opposed to the spec) is simply incorrect, it seems safe to change from my perspective.

Is it a breaking change to change the schema in such a way that increases the set of valid activities? It's definitely the friendlier kind of breaking change. Nothing should suddenly start erroring on previously valid inputs as a result of the change I proposed.

[drmason13]

Oh, the breaking part would be that clients that previously assumed those required fields were always there would break if/when they stopped being there... even if that isn't true in practice now (for the emulator at least), it could become true.

Safest to consider it for the next major release I expect.

[johnataylor]

Thanks, we'll consider this in the next major release.