Skip to content

fix(msteams): send pairing challenge reply on unpaired DMs#62812

Closed
neeravmakwana wants to merge 1 commit into
openclaw:mainfrom
neeravmakwana:fix/msteams-pairing-dm-challenge
Closed

fix(msteams): send pairing challenge reply on unpaired DMs#62812
neeravmakwana wants to merge 1 commit into
openclaw:mainfrom
neeravmakwana:fix/msteams-pairing-dm-challenge

Conversation

@neeravmakwana
Copy link
Copy Markdown
Contributor

Summary

  • Problem: With channels.msteams.dmPolicy=pairing, unpaired users had pairing requests recorded on disk but received no Teams message with the pair code, making first-time setup depend on host access to msteams-pairing.json or CLI listing.
  • Why it matters: Operators could see HTTP 200 on the Bot Framework webhook and no agent reply, with little in-channel or structured feedback to distinguish pairing from connectivity or auth failures.
  • What changed: The MS Teams monitor now uses the shared issuePairingChallenge path (pairing.issueChallenge): on the first pairing upsert for a sender it sends the standard pairing text via context.sendActivity, and logs msteams pairing request with sender, code, and label at info; reply failures are logged at error.
  • What did NOT change: Group/channel routing, allowlists, HTTP adapter behavior, or pairing store format; repeat messages from the same unpaired sender still do not re-send the challenge (same as other channels using issuePairingChallenge).

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

  • Root cause: The MS Teams DM gate called upsertPairingRequest directly and never invoked the shared pairing challenge flow that sends the in-channel reply on newly created requests.
  • Missing detection / guardrail: N/A (behavior was intentional omission; parity with Signal/Mattermost-style pairing replies was missing).
  • Contributing context (if known): N/A

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: extensions/msteams/src/monitor-handler/message-handler.authz.test.ts
  • Scenario the test should lock in: Unpaired personal DM under dmPolicy=pairing triggers sendActivity with the mocked pair code and logs msteams pairing request with that code.
  • Why this is the smallest reliable guardrail: Asserts the wiring from access resolution through issueChallenge to the Bot Framework reply path without needing a live Teams tenant.
  • Existing test that already covers this (if any): The same file’s pairing path test was updated; it previously only asserted upsertPairingRequest.
  • If no new test is added, why not: N/A — test expectations were extended.

User-visible / Behavior Changes

  • Unpaired MS Teams users who DM the bot while dmPolicy=pairing receive the standard OpenClaw pairing message in Teams on first contact (with the pair code and approve instructions), consistent with other channels using issuePairingChallenge.

Diagram (if applicable)

N/A

Security Impact (required)

  • New permissions/capabilities? (No)
  • Secrets/tokens handling changed? (No)
  • New/changed network calls? (No — uses existing Bot Framework sendActivity for the active turn)
  • Command/tool execution surface changed? (No)
  • Data access scope changed? (No)
  • If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

  • OS: macOS (local dev)
  • Runtime/container: Node via repo pnpm
  • Model/provider: N/A
  • Integration/channel (if any): MS Teams extension unit tests
  • Relevant config (redacted): channels.msteams.dmPolicy=pairing

Steps

  1. Run pnpm test:extension msteams and scoped pnpm test extensions/msteams/src/monitor-handler/message-handler.authz.test.ts
  2. Run pnpm check

Expected

  • Tests pass; lint/typecheck pass.

Actual

  • Completed successfully in this workspace before push.

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

  • Verified scenarios: Unit tests for the pairing DM path; pnpm check after changes.
  • Edge cases checked: Mock covers first-time created: true pairing upsert; repeat created: false continues to skip duplicate pairing replies per shared issuePairingChallenge behavior.
  • What I did not verify: Live Microsoft Teams/Bot Framework tenant end-to-end message delivery.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? (Yes)
  • Config/env changes? (No)
  • Migration needed? (No)
  • If yes, exact upgrade steps: N/A

Risks and Mitigations

  • Risk: sendActivity could fail for misconfigured bots or transient Teams errors.
    • Mitigation: Errors are logged at error severity with sender context; operators still have pairing store and CLI as before.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 8, 2026

Greptile Summary

This PR fixes a missing pairing reply for MS Teams DMs under dmPolicy=pairing: the monitor handler previously called pairing.upsertPairingRequest directly and never sent the in-channel pair code to the user, leaving first-time setup entirely dependent on host-side CLI access. The fix wires through the shared pairing.issueChallenge path used by other channels, which sends context.sendActivity on the first upsert, logs the pairing code at info, and logs reply failures at error. Tests are extended to assert sendActivity receives the mock pair code and the info log is emitted correctly.

Confidence Score: 5/5

Safe to merge — a focused, well-tested bug fix with no architectural changes or security surface expansion.

All findings are P2 or lower (no blocking issues). The fix uses the established shared issuePairingChallenge contract, tests are extended to cover the new behavior, and the change is backward-compatible.

No files require special attention.

Vulnerabilities

No security concerns identified. The change uses the existing context.sendActivity Bot Framework path (no new network surface), does not alter the pairing store format, and preserves the one-reply-per-sender guarantee inherited from issuePairingChallenge.

Reviews (1): Last reviewed commit: "MS Teams: pairing DM sends challenge rep..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: msteams Channel integration: msteams size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

msteams dmPolicy=pairing silently drops unpaired senders with HTTP 200, no log line, no auto-reply

1 participant