Skip to content

feat(teams): installation lifecycle and bot join events (#217) - #293

Merged
patrick-chinchill merged 7 commits into
mainfrom
sync/4.41-t2
Oct 1, 2026
Merged

patrick-chinchill merged 7 commits into
mainfrom
sync/4.41-t2

Conversation

@patrick-chinchill

@patrick-chinchill patrick-chinchill commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Ports the Teams half of upstream installation lifecycle support and bot join events.

  • installationUpdate (add, add-upgrade, remove, remove-upgrade) now dispatches to the core on_installed / on_uninstalled handlers from [4.41/C4] Core lifecycle events: message updated/deleted, installed/uninstalled, app context changed #196. The event carries a persistable channel_id built from the activity's serviceUrl, or the validated token's service_url when the activity has none. It is None when neither exists.
  • When the bot itself is added to a channel or group chat, the adapter dispatches on_member_joined_channel once, with user_id == bot_user_id and inviter_id = from.id.
  • bot_user_id decision: we adopt upstream's 28:{app_id}, which is the id Teams gives the bot in from / recipient. This is recorded in docs/UPSTREAM_SYNC.md.
  • The self check (_is_bot_account_id) is now case-insensitive. author.is_me and both new handlers use it.
  • _cache_user_context now caches Graph channel context from a team-scoped conversationUpdate that has no channelData.channel, using the base 19: conversation id.
  • New SDK-free teams/installation.py: INSTALLATION_ACTIONS, parse_installation_action, is_install_action.

Upstream commits mapped

Verify-first result

event.token.service_url exists on TokenProtocol and on JsonWebToken in microsoft-teams-api 2.0.13.4, 2.0.16 and 2.1.0. On JsonWebToken it defaults to https://smba.trafficmanager.net/teams and drops any trailing slash. In skip-auth mode the SDK's placeholder token sets it to the body's serviceUrl, or "" when the body has none.

Tests ported

All tests drive the real handle_webhook → BridgeHttpAdapter → SDK HttpServer → _dispatch_activity path. They use the #180 skip-auth fixture, with shared helpers in tests/_teams_harness.py.

  • tests/test_teams_installation.py covers all 9 tests in installation.test.ts › "Teams installation lifecycle", including the it.each cases.
    • The token-fallback cases send a real RS256 Bot Framework JWT; only the JWKS key lookup is stubbed. Skip-auth's placeholder token cannot exercise the fallback, so these cases need a real token.
    • Python additions: a: group-chat type preservation, a disallowed token URL, and action parsing.
  • tests/test_teams_joins.py covers all 14 tests in joins.test.ts › "Teams bot joins".
  • The harness app id contains hex letters. Upstream's app id is all digits, so its toUpperCase() casing tests pass whether or not the comparison ignores case.
  • Mutation check: I broke each new behavior one at a time (case-insensitive match, getattr processor probe, token fallback, URL validation, team-cache fallback, personal-chat skip, the 28: id, tenant fallback, recipient check, conversation type, self check). At least one new test failed for every break.
  • test_teams_connect.py: the bot_user_id expectations now use 28: (upstream connect.test.ts asserts 28:lazy-app etc.).
  • New tests also pass against microsoft-teams-* 2.0.16, run locally through a uv run --with overlay.

Fidelity: adapter tests are not fidelity-mapped. Delta vs committed report (HEAD): missing 104 -> 104 (+0) (after merging current main), so scripts/fidelity_target.json is unchanged. Strict at the pin: 733/733.

Teams native streaming and fixture-replay tests were run explicitly (test_teams_native_streaming.py, test_fixture_replay.py, integration/test_replay_fetch_messages_teams.py): 125 passed. A live Teams check of installs and joins is pending.

Divergences

One divergence, with a row in the non-parity table, a code breadcrumb and a regression test. When the activity has no serviceUrl, the token's service_url must pass _validate_service_url before it is encoded into the persisted channel_id. The SDK strips the token URL's trailing slash, so a root-path endpoint first gets that / back; otherwise the check would reject https://host. If it fails, the adapter logs a warning and channel_id is None. The activity's own serviceUrl is encoded without this check, exactly like message thread IDs (parity).

Consumer impact

  • New events fire on Teams:
    • on_installed / on_uninstalled for installation updates.
    • on_member_joined_channel when the bot joins a channel or group chat. A team install fires both, with the same channel_id.
  • TeamsAdapter.bot_user_id changes from {app_id} to 28:{app_id}. Plain-text @{app_id} no longer counts as a text mention in core; @28:{app_id} does. Teams mention entities are unchanged.
  • author.is_me now ignores GUID casing.
  • With no app id configured, every conversationUpdate logs a warning (upstream parity).

Merge gate

Independent review (2 reviewers, 5 findings): 4 fixed, 1 declined.

  • Fixed: an explicit null for channelData.channel, channelData.team, channelData, conversation or from crashed _cache_user_context with a 500 before the join or install was dispatched. Lookups are now null-safe, as upstream's ?. is, and the tenant comes from _tenant_id_from_activity (upstream tenantIdFromActivity). New test test_explicit_null_channel_data_fields_still_dispatch_the_join sends real JSON null (the new receive(..., keep_nulls=True)) and fails without the fix.
  • Fixed (test gap): a parametrize case where another bot is both the recipient and the added member pins the recipient-is-this-bot guard.
  • Fixed (test gap): test_prefers_the_conversation_tenant_over_channel_data pins the conversation.tenantId ?? channelData.tenant.id precedence.
  • Fixed (test gap): evil{APP_ID} and 28x{APP_ID} in the self-check negatives pin the : suffix boundary.
  • Each test-gap mutant (recipient guard removed, conversation tenant ignored, : dropped from the suffix) now fails a test.
  • Declined (pre-existing, out of [4.41/T2] Teams installation lifecycle + bot join events #217's scope): _cache_user_context is awaited, and a failing state write gives a 500 before any activity is dispatched. Upstream fire-and-forgets these writes with .catch(() => {}). This affects every activity type on main and no sibling issue under [4.41] Tracking: sync upstream chat@4.31.0 → chat@4.41.1 (0.4.41 wave) #184 owns it, so I filed Teams: make _cache_user_context state writes best-effort so a state outage does not drop dispatch #298 for a single fix covering all of them.

gpt-6-astra: 3 rounds; the final verdict on HEAD 98da0c4 is clean ("No actionable regressions found").

  • Round 1 [P2], fixed: the SDK's JsonWebToken.service_url strips the trailing slash, so a root-path token URL (https://smba.infra.gcc.teams.microsoft.com/) failed the SSRF allow-list and channel_id became None. The root / is now restored before validation. New test test_restores_the_root_slash_the_token_strips_from_a_root_path_endpoint fails without the fix. The non-parity row in docs/UPSTREAM_SYNC.md is updated.
  • Round 2: clean.
  • Round 3, after merging the new main: clean.

Bots: CodeRabbit approved 69657eb with no actionable comments. On the final merge commit it was rate-limited; that commit changed only CHANGELOG.md against main. Its walkthrough says the changelog "removes a temporary Slack divergence note"; that was a misread of a blank-line change, and the blank line is now restored, so the changelog diff only adds lines. Gemini left no comments.

CI: all green on 98da0c4 (Lint & Type Check, test 3.12/3.13, CodeQL). Local full validation is green: ruff, format, audit (0 hard failures), --check-docs, strict fidelity 733/733, pytest 7481 passed, pyrefly 0 errors.

Closes #217
Part of #184

Summary by CodeRabbit

  • New Features
    • Teams now recognizes app installation, upgrade, removal, and bot-join events, with improved bot identification and channel context.
    • Installation events can use a validated token-based service URL fallback when no URL is provided in the activity.
  • Documentation
    • Added notes on Teams event handling and known differences from upstream behavior.
    • Added a Slack changelog entry for pasted tables and alert attachments, including formatted and plain-text content, links, and routing behavior.

Port vercel/chat aaeede70 (#899, chat@4.40.0) and the Teams handler half
of 2e2426d1 (#914, chat@4.41.0):

- installationUpdate (add/add-upgrade/remove/remove-upgrade) dispatches to
  on_installed / on_uninstalled with a persistable channel_id (activity
  serviceUrl, else the validated token's service URL)
- the bot's own channel/group-chat join dispatches on_member_joined_channel
- bot_user_id is 28:{app_id}; the self check is case-insensitive
- team-scoped conversationUpdate caches Graph channel context from the
  base 19: conversation id

Closes #217
# Conflicts:
#	CHANGELOG.md
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: ab5429c7-a187-4520-b135-17d6cca0eefd

📥 Commits

Reviewing files that changed from the base of the PR and between 3782945 and 69657eb.

📒 Files selected for processing (8)
  • CHANGELOG.md
  • docs/UPSTREAM_SYNC.md
  • src/chat_sdk/adapters/teams/adapter.py
  • src/chat_sdk/adapters/teams/installation.py
  • tests/_teams_harness.py
  • tests/test_teams_connect.py
  • tests/test_teams_installation.py
  • tests/test_teams_joins.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Teams adapter now dispatches installation and bot-join events, updates bot identity and channel-context handling, and adds webhook-based tests and documentation. The changelog also removes a temporary Slack divergence note.

Changes

Teams installation and bot joins

Layer / File(s) Summary
Installation actions and bot identity
src/chat_sdk/adapters/teams/installation.py, src/chat_sdk/adapters/teams/adapter.py, tests/test_teams_connect.py
The parser accepts four installation actions. bot_user_id uses the 28: prefix, and bot-account matching ignores casing.
Installation lifecycle dispatch
src/chat_sdk/adapters/teams/adapter.py, tests/_teams_harness.py, tests/test_teams_installation.py, docs/UPSTREAM_SYNC.md, CHANGELOG.md
The webhook path dispatches install and removal events to available processors. Activity service URLs take precedence; a token-derived fallback is normalized and allow-list validated. Tests cover event routing, lifecycle processing, and service-URL handling.
Bot joins and channel context
src/chat_sdk/adapters/teams/adapter.py, tests/test_teams_joins.py, docs/UPSTREAM_SYNC.md
Qualifying conversation updates dispatch bot-join events. Context caching handles absent or malformed fields and uses a 19: conversation ID as a channel-ID fallback.

Slack changelog note removal

Layer / File(s) Summary
Remove temporary changelog note
CHANGELOG.md
The temporary Python-specific Slack divergence note is removed.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature · Severity of issue fixed: Low

Sequence Diagram(s)

sequenceDiagram
  participant WebhookRequest
  participant TeamsAdapter
  participant Chat
  WebhookRequest->>TeamsAdapter: handle_webhook(activity, options)
  TeamsAdapter->>TeamsAdapter: _dispatch_activity(installationUpdate or conversationUpdate)
  alt Installation update
    TeamsAdapter->>Chat: process_installed(event, options) or process_uninstalled(event, options)
  else Qualifying bot join
    TeamsAdapter->>Chat: process_member_joined_channel(event, options)
  end
Loading

Merge Risk: ⚪ Minimal · up to 69657

The Teams lifecycle and bot-join changes have no established merge-blocking issue. Built-in sending rejects disallowed destinations. Live Teams verification remains pending, but does not by itself prevent merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 69657

The new lifecycle callbacks preserve the existing authentication path, and built-in messaging operations validate destinations before sending. No introduced security vulnerability was established. Applications remain responsible for callback-owned state, duplicate deliveries, and installation/removal ordering; destination trust and downstream behavior were not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The new exposure is to application callbacks and their persisted conversation destinations within deployments using the Teams adapter. Actual tenant-wide effects or privilege changes depend on application handlers, which were not supplied. The included persistence example keys installations by tenant and conversation; it is test code, not a production state-ownership guarantee.

Security Findings and Attack Paths

  • inferred — An activity-supplied destination can enter a new event channel ID without validation at emission. However, the identified built-in posting, editing, deletion, and typing paths validate the decoded destination before connector use. The evidence therefore does not establish a newly introduced SSRF or credential-exfiltration path through those consumers; custom consumers remain outside the verified boundary.

Trust Boundaries and Controls

  • observed — The normal webhook path relies on SDK JWT validation before dispatch, followed by an adapter issuer check for JsonWebToken instances. The existing explicitly unauthenticated SDK mode accepts placeholder tokens. The adapter check does not compare token.service_url with the activity destination; whether the SDK independently binds them was not verified.
  • observed — Token-derived installation URLs receive validation before encoding. The existing destination policy also permits loopback HTTP for the local emulator. Native stream creation validates its activity URL, and the new lifecycle handlers do not themselves create streamers.

Resilience and Maintainability Implications

  • inferred — Applications using these callbacks for security-sensitive provisioning or removal need their own idempotency, ordering, and recovery semantics. The existing task handoff cannot make partial callback side effects atomic or undo them after interruption. No such production side effects were supplied, so a stranded insecure state is not an observed PR finding.

Hardening Proposals

  • proposed — Consider applying a shared destination-validation policy to both new event producers while retaining sink checks, so persisted channel IDs have a consistent trust contract. Document separately that once-per-activity dispatch does not guarantee exactly-once delivery or ordered installation-state transitions.
🚥 Pre-merge checks | ✅ 2 | ❌ 2 | ❓ 1

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR adds a Slack pasted-table and alert-attachment changelog entry and removes a temporary Slack divergence note. These changes do not implement or support the Teams installation and bot-join objec… Remove the unrelated Slack changelog additions and the Slack divergence-note removal, or link them to a separate active issue.
Docstring Coverage ⚠️ Warning Docstring coverage is 23.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 88 functions across 6 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive The implementation and tests cover the active coding objectives in #217: the four installation actions, validated token service-URL fallback, lifecycle dispatch, bot joins, case-insensitive identity c… Provide the result of the full validation command from CLAUDE.md so compliance with that acceptance criterion can be decided.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the two main changes: Teams installation lifecycle support and bot join events.
Full details: Linked Issues check

Explanation

The implementation and tests cover the active coding objectives in #217: the four installation actions, validated token service-URL fallback, lifecycle dispatch, bot joins, case-insensitive identity checks, 28:{app_id}, and 19: team-context fallback. docs/UPSTREAM_SYNC.md, CHANGELOG.md, and webhook-bridge tests are also reported as updated. The result of the full validation command required by #217 is not established by the available evidence. The reported fidelity and native-streaming test results do not identify that command.

Full details: Out of Scope Changes check

Explanation

The PR adds a Slack pasted-table and alert-attachment changelog entry and removes a temporary Slack divergence note. These changes do not implement or support the Teams installation and bot-join objectives in #217. The Teams source changes, tests, upstream-sync notes, and Teams changelog entry are in scope.

Full details: Docstring Coverage

Explanation

Docstring coverage is 23.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 88 functions across 6 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

A rabbit hops where Teams events flow
Four action words tell handlers where to go
A bot joins channels, joins in tune
Its 28 ID shines beneath the moon
Cached paths remember where to send
Soft paws applaud each tested end

Comment @coderabbitai help to get the list of available commands.

@patrick-chinchill

Copy link
Copy Markdown
Collaborator Author

Merge gate: CI green (Analyze (actions), Analyze (python), CodeQL, Lint & Type Check, test (3.12), test (3.13); CodeRabbit pass/approved) on 98da0c4, which already contains origin/main f381f69 (no new merge needed). Local Codex review (gpt-6-astra, xhigh, --base origin/main) on 98da0c4: "No actionable regressions found. All 792 Teams and installation-event tests passed, along with targeted lint/format checks and type checking; live Teams behavior was not verified." 3 astra rounds on this PR (round 1 P2 root-path connector URL in token fallback, fixed; rounds 2 and 3 clean). Local re-validation on the same SHA: ruff/format/audit/fidelity --check-docs/strict 733/733 green, pytest 7481 passed / 24 skipped, pyrefly 0 errors. Bots: CodeRabbit approved. Merging with --admin (Protect Main requires a code-owner approval).

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.

[4.41/T2] Teams installation lifecycle + bot join events

1 participant