Skip to content

fix(msteams): persist first-DM conversation reference in pairing path#43414

Open
xingsy97 wants to merge 1 commit intoopenclaw:mainfrom
xingsy97:wt/fix-msteams-pairing
Open

fix(msteams): persist first-DM conversation reference in pairing path#43414
xingsy97 wants to merge 1 commit intoopenclaw:mainfrom
xingsy97:wt/fix-msteams-pairing

Conversation

@xingsy97
Copy link
Copy Markdown
Contributor

@xingsy97 xingsy97 commented Mar 11, 2026 •
edited
Loading

Fix #43323

Problem

In MS Teams pairing mode, the first DM from a new user triggers a pairing request but returns early before saving the conversation reference. This means openclaw pairing approve msteams <code> --notify cannot deliver the approval notification — no conversation reference exists for the just-approved user.

Root Cause

The conversation reference construction and persistence happened after the pairing early-return path at line ~230. When access.decision === 'pairing', the handler creates the pairing request but immediately returns, never reaching the conversation reference save at line ~337.

Fix

Move conversation reference construction before the pairing early-return. Persist for pairing DMs with an idempotent helper so the normal (allowed) message path doesn't double-persist.

Validation

3 tests passing including a new test that verifies conversation reference IS persisted when a DM triggers a pairing request.

@xingsy97
fix(msteams): persist first-DM conversation reference in pairing path
61b67c1 
In MS Teams pairing mode, the first DM from a new user triggers a
pairing request but returns early before saving the conversation
reference. This means `openclaw pairing approve msteams <code> --notify`
cannot deliver the approval notification because no conversation
reference exists for the just-approved user.

Fix: construct the conversation reference before the pairing early-return
and persist it when the DM triggers a pairing request. Uses an idempotent
helper to avoid double-persisting on the normal (allowed) message path.

Closes openclaw#43323
@openclaw-barnacle openclaw-barnacle bot added channel: msteams Channel integration: msteams size: S labels Mar 11, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR fixes a bug in the MS Teams pairing flow where the first DM from a new user would trigger a pairing request but never persist the conversation reference needed for proactive --notify delivery. The fix moves the conversation reference construction earlier in the handler and wraps the conversationStore.upsert call in an idempotent persistConversationRef() helper that is invoked in the pairing early-return path before existing code.

Key changes:

  • message-handler.ts: StoredConversationReference construction and the persistConversationRef() helper are now placed before the access.decision === "pairing" early-return. The idempotency flag (conversationRefPersisted) ensures the normal (allowed) message path does not double-persist.
  • message-handler.authz.test.ts: A new test asserts that conversationStore.upsert is called with the correct conversation ID and user payload even when the DM is ultimately dropped due to pairing policy. The two existing tests continue to confirm that group-message and unauthorised-sender paths do not trigger persistence.

The implementation is clean and minimal. One thing worth noting: persistConversationRef() is intentionally fire-and-forget (returns void) — the --notify path is always invoked via a separate manual CLI command, so the practical race window is negligible.

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, well-tested bug fix with no behaviour changes outside the targeted pairing DM path.
  • The change is surgical: it hoists existing code upward and adds a small idempotency guard. The two existing authz tests still pass unchanged (group messages and non-allowlisted senders remain unaffected), and the new regression test directly covers the fixed scenario. No new external dependencies, no schema changes, and the fire-and-forget persistence pattern matches the pre-existing pattern already used in the normal allowed-message path.
  • No files require special attention.

Last reviewed commit: 61b67c1

@xingsy97
Copy link
Copy Markdown
Contributor Author

xingsy97 commented Mar 12, 2026 •
edited
Loading

@Takhoffman would appreciate a review. CI green, greptile 5/5.

@xingsy97
Copy link
Copy Markdown
Contributor Author

xingsy97 commented Mar 13, 2026

@frankekn could you help review? A simple fix for a known MS Teams first-DM pairing bug (plus test). CI green, greptile 5/5.

@BradGroux BradGroux mentioned this pull request Mar 17, 2026
Draft
BradGroux
BradGroux reviewed Apr 4, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@BradGroux BradGroux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick triage: this is the cleaner of the two DM pairing fixes (#43414 vs #43934). It persists conversation reference from the first incoming DM and avoids adding extra lifecycle/state complexity. I’d prioritize this one and close/supersede #43934 to avoid duplicate fixes.

@BradGroux BradGroux mentioned this pull request Apr 4, 2026
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BradGroux BradGroux BradGroux left review comments

Assignees

No one assigned

Labels

channel: msteams Channel integration: msteams size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: MS Teams pairing drops first DM before saving conversation reference, so --notify always fails

2 participants

@xingsy97 @BradGroux