Skip to content

fix(msteams): route attachment proactive sends through channel threads (fixes #88836)#94522

Closed
zenglingbiao wants to merge 1 commit into
openclaw:mainfrom
zenglingbiao:fix/issue-88836-msteams-attachment-threading
Closed

fix(msteams): route attachment proactive sends through channel threads (fixes #88836)#94522
zenglingbiao wants to merge 1 commit into
openclaw:mainfrom
zenglingbiao:fix/issue-88836-msteams-attachment-threading

Conversation

@zenglingbiao

Copy link
Copy Markdown
Contributor

Summary

  • Problem: In Microsoft Teams channels with replyStyle: thread, messages containing file attachments arrive as new top-level messages instead of inside the channel thread they belong to.
  • Root Cause: sendProactiveActivityRaw (used for all three attachment send paths — FileConsentCard, SharePoint native file card, OneDrive fallback link) calls sendMSTeamsActivityWithReference without passing threadActivityId. The text-only path (sendTextWithMediasendMSTeamsMessages) correctly passes it, but the attachment paths bypass that function and lose the thread routing.
  • Fix: Resolve the thread root ID (ref.threadId ?? ref.activityId) in sendMessageMSTeams for channel conversations and pass it through sendProactiveActivityRawsendMSTeamsActivityWithReference, following the same pattern already used in sendMSTeamsMessages.
  • What changed:
    • extensions/msteams/src/send.ts — add threadActivityId to ProactiveActivityParams/ProactiveActivityRawParams types; wire it through sendProactiveActivityRaw and sendProactiveActivity; resolve and pass resolvedThreadId from sendMessageMSTeams for the three attachment paths
  • What did NOT change (scope boundary):
    • Poll and Adaptive Card proactive sends — these are standalone features that follow the same pattern but are not attachment sends; keeping scope focused on the reported bug

Reproduction

  1. Configure a Teams channel with channels.msteams.replyStyle: thread
  2. Ask the agent to create a file and send it to the channel
  3. Before this PR: The file card / link message posts at the top level of the channel, losing the thread relationship
  4. After this PR: The file card / link message lands inside the same channel thread as the text reply

Real behavior proof

Behavior or issue addressed (#88836): In sendMessageMSTeams, attachment proactive sends (FileConsentCard, SharePoint file card, OneDrive fallback link) now carry threadActivityId so they route into the correct channel thread. Before this fix, sendProactiveActivityRaw always omitted threadActivityId — the attachment message would post at the top level even when replyStyle is thread.

Real environment tested: Linux, Node 22 — msteams extension test runner via test/vitest/vitest.extension-msteams.config.ts

Exact steps or command run after this patch: node scripts/run-vitest.mjs extensions/msteams/src/send.test.ts

Evidence before fix:

[test] starting test/vitest/vitest.extension-msteams.config.ts
 RUN  v4.1.8
 Test Files  1 passed (1)
      Tests  11 passed (11)

The existing 11 tests all pass because none of them verify thread routing for proactive attachment sends. The missing coverage is the bug — sendProactiveActivityRaw was never tested for threadActivityId passthrough.

Evidence after fix:

[test] starting test/vitest/vitest.extension-msteams.config.ts
 RUN  v4.1.8
 Test Files  1 passed (1)
      Tests  12 passed (12)

The new test ("passes threadActivityId to proactive attachment sends for channel thread routing") sets up a channel conversation with threadId: "thread-root-msg-1", sends a message with a SharePoint file attachment, and asserts that sendMSTeamsActivityWithReference receives options.threadActivityId === "thread-root-msg-1".

Observed result after fix: sendMSTeamsActivityWithReference is called with threadActivityId set to the resolved thread root ID when the conversation is a channel. The new test assertion passes, confirming the fix routes attachment messages into the correct channel thread.

What was not tested: Live Microsoft Teams round-trip with an actual Bot Framework conversation reference — the test uses mocked SDK calls. The sendProactiveActivityRaw logic mirroring sendMSTeamsMessages's existing channel guard (isChannel ? threadActivityId : undefined) was verified through code-path analysis rather than a separate test case.

Repro confirmation: The new test case verifies the fix by asserting options.threadActivityId equals the expected thread root on the sendMSTeamsActivityWithReference mock. Before the fix, this assertion would fail because threadActivityId was never passed through sendProactiveActivityRaw.

Risk / Mitigation

  • Risk: The resolved threadActivityId could be stale or incorrect for some edge-case conversations.
    Mitigation: Uses the same resolution pattern (ref.threadId ?? ref.activityId) already battle-tested in sendMSTeamsMessages lines 534-536. The isChannel guard mirrors the same condition used in the messenger.
  • Risk: Poll and Adaptive Card sends still lack thread routing — they use the same sendProactiveActivity path.
    Mitigation: Those are separate features not covered by this bug report. The fix pattern is the same (add threadActivityId to the call), so they can be addressed in a follow-up if bugs are reported.

Change Type (select all)

  • Bug fix

Scope (select all touched areas)

  • msteams extension

Regression Test Plan

  • node scripts/run-vitest.mjs extensions/msteams/src/send.test.ts — 12 tests including the new thread routing assertion

Review Findings Addressed

N/A (initial submission)

Linked Issue/PR

Fixes #88836

@zenglingbiao

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@zenglingbiao

Copy link
Copy Markdown
Contributor Author

关闭原因:CI 多项检查失败 — Auto response FAILURE, Dependency Guard FAILURE, Labeler FAILURE, Security Sensitive Guard FAILURE, CodeQL NEUTRAL。4个竞争者抢同一个issue,不再维护此PR。

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.

[Bug]: msteams messages with attachments misthreaded

1 participant