Skip to content

docs: add Socket Mode design doc - #689

Open
Mehak Bindra (MehakBindra) wants to merge 10 commits into
mainfrom
mehakbindra-socket-mode-design-doc
Open

Mehak Bindra (MehakBindra) wants to merge 10 commits into
mainfrom
mehakbindra-socket-mode-design-doc

Conversation

@MehakBindra

Copy link
Copy Markdown
Member

Adds docs/SocketMode-Design.md describing the intended Socket Mode feature for TeamsBotApplication, following the style of docs/Architecture.md and docs/Observability-Design.md.

This branch only has the low-level Socket Mode transport primitives (SignalRClientConnection, SignalRSocketConnection, SocketModeConnection, SocketModeEnvelope, SocketModeJson, SocketModeNegotiator, SocketModeProtocol, SocketModeProtocolModels). The doc is forward-looking design documentation for the full feature (GeoSocket, SocketModeTransport, SocketModeOptions, SocketModeHostedService, and the UseSocketMode/host.UseTeamsBotApplication() integration), which is landing via a follow-up PR stack based on #686.

No other files were touched.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@MehakBindra
Mehak Bindra (MehakBindra) marked this pull request as ready for review September 30, 2026 22:36
Copilot AI balanced review requested due to automatic review settings September 30, 2026 22:36

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

Several statements conflict with the planned implementation, and the primary usage example targets an endpoint where Socket Mode is unavailable.

Review effort: Balanced
Findings: 6 Low severity

Open (6)
What changed in this PR

Adds forward-looking Socket Mode design documentation covering architecture, transport flows, and hosting integration.

Changes:

  • Documents Socket Mode configuration and usage.
  • Describes connection lifecycle, rotation, and reconnect flows.
  • Explains DI, hosting, dispatch, and invoke-response handling.
File Description
docs/​SocketMode/​SocketMode-Design.md User-facing architecture and configuration design.
docs/​SocketMode/​SocketMode-Flow-Walkthrough.md End-to-end transport lifecycle walkthrough.
docs/​SocketMode/​SocketMode-Hosting-Integration.md Hosting, DI, and activity dispatch integration.

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

Comment thread docs/SocketMode/SocketMode-Design.md
Comment thread docs/SocketMode/SocketMode-Design.md
Comment thread docs/SocketMode/SocketMode-Design.md
Comment thread docs/SocketMode/SocketMode-Flow-Walkthrough.md
Comment thread docs/SocketMode/SocketMode-Hosting-Integration.md
Comment thread docs/SocketMode/SocketMode-Hosting-Integration.md
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Each geo is managed independently by a `GeoSocket` supervisor, which:

- Establishes an initial ready connection within a startup budget

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.

i would mention here that any geo failing during startup fails startup for ALL geos (and retries w/ backoff). the third bullet about not affecting other geos is only true after startup


`SocketModeHostedService` is an `IHostedService` that starts the transport.
Host startup does not complete until every configured geo has an initial ready
connection; if a geo fails to become ready within its startup budget, the

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.

i see its mentioned here so maybe u could add the part about it failing the startup for all geos and keep it here

(`SocketModeNegotiator`) to obtain SignalR connection details:

1. The negotiate URL is `NegotiateBaseUrl` (default
`https://botapi.skype.com`) plus a geo-specific path segment.

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.

not sure if it would go here but we do override this URL depending on the testing ring (we currently override for Canary, and next we'll need a different URL for R0)

| Class | Responsibility | Status |
|---|---|---|
| `SocketModeHostedService` | `IHostedService`; blocks host startup until every geo is ready | PR #686 |
| `SocketModeTransport` | Implements `IGeoSocketOwner`; owns all `GeoSocket`s; dispatches to `TeamsBotApplication` | PR #686 |

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.

can update all of these to main

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.

3 participants