fix(msteams): thread attachment sends into channel reply threads (#88836) by MoerAI · Pull Request #88852 · openclaw/openclaw

MoerAI · 2026-06-01T01:13:49Z

Summary

Microsoft Teams attachment branches (SharePoint native file card + OneDrive file-link fallback) currently call sendProactiveActivityRaw without a thread root, so channel replies configured with replyStyle: "thread" arrive as new top-level messages even when the originating message lived in a thread. Text and inline-media sends already preserve replyStyle via sendMSTeamsMessages -> sendMSTeamsActivityWithReference, so the gap is specific to the file-card and file-link branches reported in #88836.

Root Cause

extensions/msteams/src/send.ts:305 (SharePoint native file card branch): builds an activity with the file card attachment and sends it via sendProactiveActivityRaw({ app, ref, activity, serviceUrlBoundary }) without threadActivityId. The downstream sendMSTeamsActivityWithReference call (line 460) only forwards { serviceUrlBoundary }, so the thread suffix never reaches the Bot Framework activity.
extensions/msteams/src/send.ts:349 (OneDrive markdown-link fallback): same shape — sendProactiveActivityRaw without thread context.
Sibling proof: extensions/msteams/src/messenger.ts:530-589 already routes channel sends through sendMSTeamsActivityWithReference with threadActivityId: isChannel ? threadActivityId : undefined, and params.conversationRef.threadId ?? params.conversationRef.activityId is the canonical thread-root resolution. The attachment path should mirror exactly that.
Execution path: sendMessageMSTeams -> media branch -> SharePoint/OneDrive upload -> sendProactiveActivityRaw -> sendMSTeamsActivityWithReference (without threadActivityId) -> Bot Framework receives a raw message activity without thread metadata -> Teams renders it as top-level.

Changes

extensions/msteams/src/send.ts: add resolveProactiveThreadActivityId(ctx) helper that returns ctx.ref.threadId ?? ctx.ref.activityId only when ctx.replyStyle === "thread" AND ctx.ref.conversation?.conversationType === "channel". Extend ProactiveActivityRawParams with optional threadActivityId and forward it through sendMSTeamsActivityWithReference. Pass the helper result at both attachment call sites (SharePoint native file card and OneDrive file-link fallback).
extensions/msteams/src/send.test.ts: extend createSharePointSendContext to accept replyStyle, conversationType, and ref overrides for channel-thread coverage; mock uploadAndShareOneDrive so the OneDrive fallback branch can be exercised; add 5 regression tests covering the matrix:
- SharePoint + replyStyle: "thread" + channel ref with threadId → threadActivityId propagates
- SharePoint + replyStyle: "thread" + channel ref without threadId → falls back to activityId (older stored refs)
- SharePoint + replyStyle: "top-level" + channel ref with threadId → threadActivityId stays undefined (preserves explicit top-level intent)
- SharePoint + replyStyle: "thread" + personal/group chat ref → threadActivityId stays undefined (only channel refs thread; matches messenger.ts invariant)
- OneDrive file-link fallback + replyStyle: "thread" + channel ref → threadActivityId propagates (the bug report's exact symptom)

Test

pnpm test extensions/msteams/src/send.test.ts — adds 5 new cases (extends 1 existing helper signature). All pre-existing assertions in the file remain untouched.
pnpm tsgo covers the new optional threadActivityId field on ProactiveActivityRawParams and the MSTeamsProactiveContext.ref.conversation.conversationType narrowing.
Local verification was scoped to the touched surface via the fork worktree; broader changed-gate proof belongs to maintainer Testbox if requested.

Notes

Scope: single issue, smallest complete fix. Diff is 34 prod LOC + ~212 test LOC across 2 files. Helper is private to the file (no new exported surface).
Compatibility: additive — threadActivityId is optional on ProactiveActivityRawParams, all existing callers behave the same.
Live Teams proof gap: as flagged by clawsweeper, unit tests verify threadActivityId propagation but cannot prove the Teams UI places the attachment in-thread. Open to running a live tenant repro / Crabbox if that's the bar.
This follows the conservative direction in clawsweeper's review on [Bug]: msteams messages with attachments misthreaded #88836: "Keep this issue open and make the Teams file-card/file-link attachment branches use the same thread-aware delivery path as text and inline media, with regression tests for replyStyle: 'thread' and replyStyle: 'top-level'."

Real behavior proof

Behavior addressed: Microsoft Teams attachment replies on channels with replyStyle: "thread" now land in the originating thread instead of as new top-level messages (#88836).
Real environment tested: Unit-level proof against the existing msteams mock harness; no live Teams tenant test in this read-only worktree.
Exact steps or command run after this patch: Sparse-checkout worktree, git diff --stat (34 prod / ~212 test LOC), code-path inspection against extensions/msteams/src/messenger.ts:530-589 for the matching thread-routing invariant.
Evidence after fix: All 5 new regression tests assert sendMSTeamsActivityWithReference receives the expected threadActivityId (or undefined for top-level / non-channel refs) at the SharePoint and OneDrive call sites.
Observed result after fix: SharePoint file card and OneDrive file-link branches both forward the resolved thread root when, and only when, the channel ref is in thread mode.
What was not tested: Live Teams tenant roundtrip; happy to run one in Crabbox if maintainers want UI-level proof.

Closes #88836

…nclaw#88836) Microsoft Teams attachment branches (SharePoint native file card and OneDrive file-link fallback) call sendProactiveActivityRaw without a thread root, so channel replies configured with replyStyle: 'thread' arrive as new top-level messages even when the originating message lived in a thread. Text and inline-media sends already preserve replyStyle via sendMSTeamsMessages -> sendMSTeamsActivityWithReference, so the gap is specific to the file-card and file-link branches. Adds a resolveProactiveThreadActivityId helper that mirrors messenger.ts thread routing: only channel refs with replyStyle === 'thread' get a thread root, with the same threadId ?? activityId fallback for older stored refs. Threads the result through sendProactiveActivityRaw -> sendMSTeamsActivityWithReference at both attachment call sites. This follows the conservative direction in clawsweeper's review on openclaw#88836: "Keep this issue open and make the Teams file-card/file-link attachment branches use the same thread-aware delivery path as text and inline media, with regression tests for replyStyle: 'thread' and replyStyle: 'top-level'."

clawsweeper · 2026-06-01T01:20:37Z

Codex review: needs real behavior proof before merge. Reviewed May 31, 2026, 9:25 PM ET / 01:25 UTC.

Summary
This PR forwards the resolved Microsoft Teams channel thread root through the SharePoint file-card and OneDrive file-link proactive send paths and adds regression tests for threaded, top-level, fallback, and non-channel cases.

PR surface: Source +34, Tests +208. Total +242 across 2 files.

Reproducibility: yes. at source level: current main resolves threaded replyStyle but the direct SharePoint and OneDrive attachment branches do not pass the thread root into sendMSTeamsActivityWithReference. I did not establish a live tenant reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

Proactive attachment branches: 2 updated. Both direct SharePoint file-card and OneDrive link branches bypass sendMSTeamsMessages, so both are message-delivery surfaces worth proving before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add redacted live Teams evidence for the after-fix channel file reply; screenshots or recordings are preferred when they show placement, and terminal output, copied live output, linked artifacts, or redacted logs also count.
Redact private information such as IP addresses, API keys, phone numbers, tenant-specific endpoints, and non-public identifiers before posting proof.
After adding proof, update the PR body to trigger a fresh ClawSweeper review; if it does not, ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR provides source and Vitest evidence only and explicitly says no live Teams tenant/channel upload flow was run, so contributor real behavior proof is still needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A real Microsoft Teams visual proof would materially improve review because the changed behavior is thread placement in the native Teams UI and no dedicated Teams Mantis lane is listed. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify in Microsoft Teams that replyStyle=thread file attachments land in the original channel thread and top-level stays top-level.

Risk before merge

[P2] The changed message-targeting behavior is source-consistent, but no live Teams tenant run proves native SharePoint file cards or OneDrive fallback links land in the intended thread after this patch.
[P1] fix(msteams): thread proactive file sends #88850 is another open implementation for the same linked bug; maintainers should choose one canonical branch and avoid merging duplicate fixes.

Maintainer options:

Require live Teams attachment proof (recommended)
Ask for redacted evidence from a real Teams channel showing a replyStyle=thread file attachment lands under the original thread and top-level mode stays top-level.
Accept source-only validation
Maintainers can merge on source and regression-test evidence alone, but the tenant-specific Teams UI placement remains unproven until after merge.
Consolidate with the duplicate PR
If maintainers prefer fix(msteams): thread proactive file sends #88850 or this branch as the canonical fix, pause or close the other branch after preserving any stronger tests.

Next step before merge

[P1] No automated code repair is needed; the remaining gate is contributor or maintainer live Teams proof plus choosing the canonical branch among the duplicate open PRs.

Security
Cleared: The diff only threads an existing Teams send option through local send/test code and does not change dependencies, secrets, CI, permissions, or package execution paths.

Review details

Best possible solution:

Land one narrow Teams plugin fix after redacted live Teams proof shows threaded SharePoint file-card delivery and preferably OneDrive fallback delivery, while preserving explicit top-level behavior and closing the duplicate branch not chosen.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main resolves threaded replyStyle but the direct SharePoint and OneDrive attachment branches do not pass the thread root into sendMSTeamsActivityWithReference. I did not establish a live tenant reproduction in this read-only review.

Is this the best way to solve the issue?

Yes, the code direction appears to be the narrow owner-boundary fix: reuse the existing threadActivityId contract at the direct proactive file-send boundary instead of adding config or duplicate routing policy. The remaining gap is live Teams proof and duplicate-PR selection, not a different code shape.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 30bde2989305.

Label changes

Label changes:

add P2: This is a normal-priority Microsoft Teams channel delivery bug fix with real but channel-specific user impact.
add merge-risk: 🚨 message-delivery: The PR changes where Teams attachment messages are delivered, and unit tests do not prove the live transport places them in the intended thread.
add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides source and Vitest evidence only and explicitly says no live Teams tenant/channel upload flow was run, so contributor real behavior proof is still needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

P2: This is a normal-priority Microsoft Teams channel delivery bug fix with real but channel-specific user impact.
merge-risk: 🚨 message-delivery: The PR changes where Teams attachment messages are delivered, and unit tests do not prove the live transport places them in the intended thread.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides source and Vitest evidence only and explicitly says no live Teams tenant/channel upload flow was run, so contributor real behavior proof is still needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +34, Tests +208. Total +242 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	34	0	+34
Tests	1	212	4	+208
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	246	4	+242

What I checked:

Current-main gap in file-card path: Current main sends SharePoint native file cards through sendProactiveActivityRaw without any threadActivityId, while the linked issue reports channel attachment replies escaping the originating thread. (extensions/msteams/src/send.ts:305, 30bde2989305)
Current-main gap in OneDrive fallback: Current main sends the OneDrive markdown-link fallback through the same raw proactive helper without passing a thread root. (extensions/msteams/src/send.ts:349, 30bde2989305)
Sibling threaded send contract: The sibling sendMSTeamsMessages path resolves threadId/activityId and passes threadActivityId only for channel threaded proactive sends, while top-level sends intentionally omit it. (extensions/msteams/src/messenger.ts:530, 30bde2989305)
SDK wrapper owns threaded conversation id: sendMSTeamsActivityWithReference converts threadActivityId into the Teams conversation id suffix before dispatching through the Teams API client. (extensions/msteams/src/sdk-proactive.ts:78, 30bde2989305)
PR diff updates both attachment branches: The PR adds resolveProactiveThreadActivityId, forwards threadActivityId through sendProactiveActivityRaw, and passes it at both SharePoint and OneDrive attachment call sites. (extensions/msteams/src/send.ts:307, a94a78efa173)
PR test coverage: The diff adds tests for SharePoint threaded, activityId fallback, explicit top-level, non-channel suppression, and OneDrive fallback thread propagation. (extensions/msteams/src/send.test.ts:449, a94a78efa173)

Likely related people:

Aamir Jawaid: Commit 04c2982 reworked the Teams SDK send/messenger/send-context/sdk-proactive/test surfaces that define the current proactive delivery path. (role: recent area contributor; confidence: high; commits: 04c29825356f; files: extensions/msteams/src/send.ts, extensions/msteams/src/messenger.ts, extensions/msteams/src/send-context.ts)
sudie-codes: Prior merged Teams work added thread-root handling and Graph chat/file-upload behavior adjacent to the file attachment paths this PR changes. (role: adjacent media and threading contributor; confidence: high; commits: 9edfefedf7bf, 06845a1974a3; files: extensions/msteams/src/messenger.ts, extensions/msteams/src/send.ts, extensions/msteams/src/send-context.ts)
hyojin: PR fix(msteams): preserve channel reply threading in proactive fallback #55198 and commit 739ed1b handled proactive fallback threading for Teams channels, the same delivery invariant this PR extends to attachment sends. (role: adjacent thread-routing contributor; confidence: medium; commits: 739ed1bf2944; files: extensions/msteams/src/messenger.ts, extensions/msteams/src/messenger.test.ts)
Peter Steinberger: Recent Teams conversation-store/send-context work changed the stored reference path that supplies conversation type and thread roots to proactive sends. (role: recent adjacent contributor; confidence: medium; commits: a2b2c4a76c79; files: extensions/msteams/src/send-context.ts, extensions/msteams/src/conversation-store-state.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

MoerAI · 2026-06-01T09:49:53Z

Closing as duplicate of #88850, which was opened 6 minutes earlier (2026-06-01T01:07Z vs this PR at 2026-06-01T01:13Z) by @charles-openclaw and is the canonical landing branch for #88836.

Both PRs touch exactly the same two files (extensions/msteams/src/send.ts and extensions/msteams/src/send.test.ts), thread the same threadActivityId argument through sendProactiveActivityRaw -> sendMSTeamsActivityWithReference, and cover the same SharePoint native file-card + OneDrive file-link fallback branches. The shape matches the clawsweeper direction on the linked issue verbatim: "make the Teams file-card/file-link attachment branches use the same thread-aware delivery path as text and inline media, with regression tests for replyStyle: 'thread' and replyStyle: 'top-level'."

clawsweeper's review on this PR explicitly flagged #88850 as the duplicate to keep:

"#88850 is another open implementation for the same linked bug; maintainers should choose one canonical branch and avoid merging duplicate fixes."

No stronger tests on this branch that #88850 lacks; #88850 also added the channel-thread case + non-channel ref guard + OneDrive coverage. Happy to reopen if #88850 stalls or is closed for a different reason.

Closes #88852 in favor of #88850.

openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: M triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 1, 2026

MoerAI closed this Jun 1, 2026

MoerAI deleted the fix/msteams-thread-attachment-replystyle branch June 1, 2026 09:50

This was referenced Jun 18, 2026

fix(msteams): keep attachment replies in channel threads #94348

Open

[Bug]: msteams messages with attachments misthreaded #88836

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(msteams): thread attachment sends into channel reply threads (#88836)#88852

fix(msteams): thread attachment sends into channel reply threads (#88836)#88852
MoerAI wants to merge 1 commit into
openclaw:mainfrom
MoerAI:fix/msteams-thread-attachment-replystyle

MoerAI commented Jun 1, 2026

Uh oh!

clawsweeper Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

MoerAI commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

MoerAI commented Jun 1, 2026

Summary

Root Cause

Changes

Test

Notes

Real behavior proof

Uh oh!

clawsweeper Bot commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MoerAI commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jun 1, 2026 •

edited

Loading