feat(teams): Microsoft Teams adapter - #18
Merged
Merged
Conversation
Implements core.Adapter for the Azure Bot Framework. Inbound Activities arrive over a webhook HTTP server; replies go out through the Bot Connector REST API. Every inbound request is authenticated against the Bot Connector JWKS (RS256 signature, audience, issuer, and a serviceurl claim bound to the Activity), and outbound replies are restricted to allowlisted Bot Framework hosts to prevent SSRF. Adds golang-jwt/jwt/v5 for token verification.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR adds a Microsoft Teams adapter with internal auth, message parsing, attachment mapping, outbound send, and webhook lifecycle support, plus a public Teams wrapper, example wiring, docs, and dependency isolation updates. It also changes WhatsApp shutdown handling to use a detached dispatch context and force-cancel drains that exceed their budget. ChangesMicrosoft Teams Platform Adapter
Estimated code review effort: 4 (Complex) | ~75 minutes WhatsApp Dispatch Context and Drain Refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Teams as Microsoft Teams
participant Server as handleMessages
participant Auth as validateInbound
participant Store as recordConversation
participant Dispatch as deps.Dispatch
participant Send as adapter.Send
Teams->>Server: POST activity + Bearer token
Server->>Auth: validateInbound(activity, token)
Auth-->>Server: claims and serviceUrl accepted
Server->>Store: recordConversation(channelID, serviceURL, bot)
Server->>Dispatch: dispatch on detached context
Dispatch->>Send: Send(channelID, replyText)
Send->>Teams: POST /v3/conversations/{id}/activities
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adds first-class Microsoft Teams (Azure Bot Framework) support to botbooter via a new internal adapter plus a thin public wrapper package, maintaining the existing “public facade over internal adapter” architecture and isolation guarantees.
Changes:
- Introduces
internal/teamsadapter implementingcore.Adapter(webhook server, inbound JWT validation via Bot Connector JWKS, outbound reply send, attachments mapping). - Adds public
teams/wrapper package (New,RawMessage, and type aliases) and updatesBotTypeplumbing (TeamsBotType+String()+ re-export). - Extends isolation/import-guard tests, examples, and developer docs to include Teams and ensure
github.com/golang-jwt/jwt/v5remains Teams-only.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| teams/wrapper_test.go | Verifies the public Teams facade (New, RawMessage) behavior. |
| teams/teams.go | Public Teams wrapper: constructor, raw-message accessor, and type/error re-exports. |
| teams/imports_test.go | Enforces that the public Teams wrapper imports no platform SDKs directly. |
| isolation_deps_test.go | Extends transitive dependency isolation checks to include Teams and jwt/v5 confinement. |
| internal/teams/teams.go | New Teams adapter: webhook server, JWT/JWKS auth, outbound send, routing, attachments. |
| internal/teams/teams_test.go | Comprehensive unit tests for Teams adapter behavior and security invariants. |
| internal/core/core.go | Adds TeamsBotType and String() mapping. |
| internal/core/core_test.go | Tests TeamsBotType.String(). |
| go.mod | Adds github.com/golang-jwt/jwt/v5 dependency. |
| go.sum | Adds jwt/v5 checksums. |
| CLAUDE.md | Updates architecture/docs to include Teams adapter and isolation expectations. |
| botbooter.go | Re-exports TeamsBotType and updates package docs to mention Teams. |
| botbooter_test.go | Exercises Teams raw accessor and teams.New via the root integration tests. |
| _examples/v1/platforms.go | Adds Teams option to the example platform selector and its error message. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nostics Send now records the bot and user accounts alongside the serviceUrl and sets the reply's from (required by Bot Connector) and recipient fields. Raise the JWKS/OpenID/token read cap to 4 MiB so the real ~1 MB Bot Connector JWKS decodes instead of truncating to "unexpected EOF". Pin jwks_uri to the metadata scheme and host (not just host) to block a cleartext downgrade. Wrap 401 rejection reasons and log them, escaping attacker-controlled aud with %q. Also rename the example env var to TEAMS_APP_TENANT_ID and document the Teams adapter in the README and platforms guide.
The webhook adapters passed the run context into deps.Dispatch, but core cancels that context before Disconnect drains in-flight dispatch — so a handler replying via SendMessageContext(ctx) failed with "context canceled" mid-drain and the acked message was dropped. Dispatch on context.WithoutCancel so an acked reply can finish within the drain window; apply the same fix to Teams and WhatsApp. Teams: decode the Activity's attachments once at parse time onto Message instead of re-unmarshaling the (up to 1 MiB) Raw body on every Attachments call, matching the WhatsApp adapter's parse-once shape. CLAUDE.md: document that each adapter deliberately owns its own setup and that shared correctness fixes must be swept across the sibling adapters.
Disconnect shared one 5s context across srv.Shutdown and drainDispatch, so a slow shutdown could burn the budget and leave the drain ~0s, abandoning an already-acked in-flight dispatch mid-reply. Give each its own 5s deadline, mirroring the Teams adapter. Adds a hermetic drain-cancel test and an env-gated end-to-end slow-shutdown regression (BOTBOOTER_WHATSAPP_DRAIN_TIMING_TEST).
- Force a JWKS refresh once the cached key set ages past 24h (jwksMaxAge) so a signing key Microsoft has retired stops being trusted, rather than lingering until an unknown-kid token flushes the map. A refresh that fails falls back to the cached key so a transient JWKS outage does not reject otherwise-valid tokens. - Drop bot-authored Activities by from.role == "bot" instead of the never-firing from.id == recipient.id check, matching how the Slack, Discord and Telegram adapters ignore all bots to avoid reply loops. - Consolidate the non-2xx / JSON-decode / keep-alive-drain tail of the outbound calls into a shared decodeJSON helper (Send, accessToken, getJSON), capping decodes at maxMetaBytes.
Break internal/teams/teams.go (1094 lines) into seven same-package files by responsibility: teams.go (front door), server.go (webhook lifecycle), auth.go (JWT/JWKS + SSRF), send.go (reply + token), message.go (parsing), attachments.go (mapping) and http.go (shared plumbing). Declarations moved verbatim, then comments trimmed to the essential rationale. No behavior change.
lao
marked this pull request as ready for review
July 1, 2026 16:46
the line is missing punctuation after the bold heading, making the numbered step read awkwardly.
Using %q with gotAud (an untyped claim value) can yield fmt formatting errors like %!q(...) when aud is not a string (it can be an array per JWT spec). This makes the returned error/log output misleading and defeats the intended escaping.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
internal/teams/send.go (1)
89-134: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSerialize token refreshes after a cache miss.
The cache check releases
a.mubefore minting, so concurrentSendcalls with an empty/expiring token can all hit the client-credentials endpoint. Gate refresh with a token-specific mutex and re-check the cache after acquiring it.♻️ Suggested shape
func (a *adapter) accessToken(ctx context.Context) (string, error) { a.mu.Lock() if a.token.value != "" && time.Until(a.token.expiry) > tokenRefreshSkew { v := a.token.value a.mu.Unlock() return v, nil } a.mu.Unlock() + a.tokenMu.Lock() + defer a.tokenMu.Unlock() + + a.mu.Lock() + if a.token.value != "" && time.Until(a.token.expiry) > tokenRefreshSkew { + v := a.token.value + a.mu.Unlock() + return v, nil + } + a.mu.Unlock() + form := url.Values{Add
tokenMu sync.Mutexto the adapter struct.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/teams/send.go` around lines 89 - 134, The accessToken method in adapter currently drops a.mu before minting, so concurrent Send calls can all refresh the Teams token at once after a cache miss or near expiry. Add a dedicated tokenMu field to adapter and serialize the refresh path in accessToken by locking it before contacting the token endpoint. After acquiring tokenMu, re-check the cached token state under a.mu guard before creating the request so only one goroutine mints a new token while others reuse the refreshed value.internal/whatsapp/whatsapp_test.go (1)
519-534: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep dispatch blocked so this test exercises drain-before-cancel ordering.
The
Dispatchcallback returns immediately, soinflightmay already be zero before Line 533 callsDisconnect. This test would still pass ifDisconnectcanceled before draining. Hold the callback until the test releases it, then assert the context stays live whileDisconnectis waiting.Test-shape adjustment
gotCtx := make(chan context.Context, 1) - deps := core.AdapterDeps{Dispatch: func(c context.Context, _ *core.Message) { gotCtx <- c }} + releaseDispatch := make(chan struct{}) + deps := core.AdapterDeps{Dispatch: func(c context.Context, _ *core.Message) { + gotCtx <- c + <-releaseDispatch + }} @@ asserts.NoError(t, c.Err(), "dispatch ctx live before Disconnect") - asserts.NoError(t, a.Disconnect(), "Disconnect") + done := make(chan error, 1) + go func() { done <- a.Disconnect() }() + select { + case <-c.Done(): + t.Fatal("dispatch ctx canceled before drain completed") + case <-time.After(50 * time.Millisecond): + } + close(releaseDispatch) + asserts.NoError(t, <-done, "Disconnect") asserts.ErrorIs(t, c.Err(), context.Canceled, "dispatch ctx canceled after drain")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/whatsapp/whatsapp_test.go` around lines 519 - 534, The test around handleWebhook and Dispatch does not keep the in-flight request blocked, so it can miss the intended drain-before-cancel ordering. Update the test to have the core.AdapterDeps.Dispatch callback wait on a release signal before returning, then call a.Disconnect while Dispatch is still blocked and only release it afterward. Keep the assertions on the captured context’s liveness and cancellation to verify that c.Err() stays nil until Disconnect drains inflight, using the existing gotCtx, dispatchCtx, and a.Disconnect flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@_docs/platforms.md`:
- Around line 238-349: Normalize the Teams documentation heading hierarchy in
the markdown section by changing the new Teams subheads to the next proper level
under the existing section so the outline does not skip heading depths and
markdownlint MD001 passes; update the headings in the Teams walkthrough
consistently (including the Step 1–Step 5 titles) and keep the surrounding
content structure intact.
In `@internal/teams/message.go`:
- Around line 106-115: Only remove a leading self-mention in the message
normalization path, rather than stripping every mention entity addressed to the
bot. Update the logic in message normalization around the text/entity handling
in the message parsing flow so it checks the first mention entity at the start
of the content before calling strings.Replace, and leave any later self-mentions
intact. Keep the existing anchored-command matching behavior while preserving
user payload in messages like echo <at>Bot</at>.
In `@internal/teams/server.go`:
- Around line 164-188: Disconnect should surface a shutdown failure when the
drain times out and in-flight dispatches are force-canceled, instead of
returning nil after a successful srv.Shutdown. Update the Disconnect flow around
drainCtx, a.drainDispatch, and the in-flight check so that a drain timeout
sets/returns an error while still preserving the existing cleanup of a.srv,
a.detachedCancel, and cancelDispatch. Keep the normal nil return only when the
drain completes without dropping acked dispatches.
In `@internal/whatsapp/whatsapp.go`:
- Around line 257-283: The adapter-wide inflight tracker is being shared across
reconnects, so a new connection can interfere with the prior Disconnect drain
state. Update the connection lifecycle in adapter.Connect/Disconnect to capture
a per-connection inflight tracker alongside srv and detachedCancel, and have the
dispatch handler increment/decrement that tracker instead of a.inflight. Make
sure drainDispatch and any drain logging/reporting use the tracker tied to the
srv being shut down, not the current adapter instance state.
- Around line 285-310: Update Disconnect in whatsapp.go so a drain timeout is
surfaced to callers instead of returning nil after force-canceling in-flight
dispatches. Use the existing inflight check and cancelDispatch path to set or
wrap an error when a deadline is reached, and ensure the final return reflects
that timeout even if srv.Shutdown itself succeeded. Keep the cleanup of a.srv
and a.detachedCancel unchanged, but make the timeout state visible through
Disconnect’s returned error.
---
Nitpick comments:
In `@internal/teams/send.go`:
- Around line 89-134: The accessToken method in adapter currently drops a.mu
before minting, so concurrent Send calls can all refresh the Teams token at once
after a cache miss or near expiry. Add a dedicated tokenMu field to adapter and
serialize the refresh path in accessToken by locking it before contacting the
token endpoint. After acquiring tokenMu, re-check the cached token state under
a.mu guard before creating the request so only one goroutine mints a new token
while others reuse the refreshed value.
In `@internal/whatsapp/whatsapp_test.go`:
- Around line 519-534: The test around handleWebhook and Dispatch does not keep
the in-flight request blocked, so it can miss the intended drain-before-cancel
ordering. Update the test to have the core.AdapterDeps.Dispatch callback wait on
a release signal before returning, then call a.Disconnect while Dispatch is
still blocked and only release it afterward. Keep the assertions on the captured
context’s liveness and cancellation to verify that c.Err() stays nil until
Disconnect drains inflight, using the existing gotCtx, dispatchCtx, and
a.Disconnect flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c80cf17-876e-46b0-b420-c4d1942564d6
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
CLAUDE.mdREADME.md_docs/platforms.md_examples/v1/platforms.gobotbooter.gobotbooter_test.gogo.modinternal/core/core.gointernal/core/core_test.gointernal/teams/attachments.gointernal/teams/auth.gointernal/teams/http.gointernal/teams/message.gointernal/teams/send.gointernal/teams/server.gointernal/teams/teams.gointernal/teams/teams_test.gointernal/whatsapp/whatsapp.gointernal/whatsapp/whatsapp_test.goisolation_deps_test.goteams/imports_test.goteams/teams.goteams/wrapper_test.go
Mirror the source split: move the 72 tests from the single 1483-line teams_test.go into server_test.go, auth_test.go, send_test.go, message_test.go, attachments_test.go and http_test.go, keeping shared fixtures in teams_test.go. Identical test set, no behavior change.
lao
force-pushed
the
worktree-teams-adapter
branch
from
July 1, 2026 17:07
684014a to
528070a
Compare
Teams and WhatsApp Disconnect returned nil after a successful Shutdown even when the drain deadline expired with dispatches still in-flight, hiding force-canceled acked messages as a clean shutdown. Surface a drain-timeout error instead. Adds env-gated ~5s regression tests to both adapters.
This was referenced Jul 3, 2026
This was referenced Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Microsoft Teams (Azure Bot Framework) adapter, following the same facade-over-internal pattern as the existing platforms. Like WhatsApp, Teams is a webhook adapter: it runs its own HTTP server for inbound Activities and replies over the Bot Connector REST API.
What's included
internal/teams— thecore.Adapterimplementation: webhook server, JWT inbound auth, conversation→serviceUrl routing, Bot Connector token minting, and reply send.teams/— thin public package (teams.New,teams.RawMessage,Config/Messagetypes) importing no platform SDK.core.TeamsBotType+String(), re-exported asbotbooter.TeamsBotType.Security
serviceurlclaim bound to the Activity'sserviceUrl.*.trafficmanager.netnamespace is deliberately not allowlisted.Isolation
golang-jwt/jwt/v5is the adapter's only third-party dependency (a crypto lib, not a platform SDK). The per-packageimports_test.goguard and the module-levelisolation_deps_test.goconfirm it stays confined to the teams closure and that a Teams-only binary pulls in no other platform SDK.Testing
gofmt,go vet, andgo test ./...all pass.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests