Skip to content

fix(msteams): route file attachment sends through thread-aware delivery path#89094

Closed
whiteyzy wants to merge 2 commits into
openclaw:mainfrom
whiteyzy:fix/msteams-file-attachment-thread-routing-88836
Closed

fix(msteams): route file attachment sends through thread-aware delivery path#89094
whiteyzy wants to merge 2 commits into
openclaw:mainfrom
whiteyzy:fix/msteams-file-attachment-thread-routing-88836

Conversation

@whiteyzy

@whiteyzy whiteyzy commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix Microsoft Teams file attachment sends to route through the thread-aware delivery path when replyStyle: "thread" is configured for channel conversations.

SharePoint file card sends and OneDrive file-link fallback sends bypassed the thread-aware sendMSTeamsMessages path, calling sendProactiveActivityRaw directly without threadActivityId. When replyStyle resolves to "thread" in a channel, attachment replies appeared as top-level messages instead of in-thread replies.

Root Cause

sendProactiveActivityRaw in extensions/msteams/src/send.ts did not accept or forward a threadActivityId parameter. Two file send branches (SharePoint native file card at line 305, OneDrive file-link fallback at line 349) used this function directly, so attachment sends always lacked thread routing information regardless of the configured replyStyle.

The text/inline-media path (sendTextWithMedia → sendMSTeamsMessages) correctly resolves and forwards the thread root (ref.threadId ?? ref.activityId), but the file attachment branches had no equivalent.

Fix

  1. Add replyStyle to the destructured context in sendMessageMSTeams
  2. Compute threadActivityId once from ref.threadId ?? ref.activityId when the conversation is a channel and replyStyle is "thread"
  3. Add threadActivityId as an optional parameter to sendProactiveActivityRaw and forward it to sendMSTeamsActivityWithReference
  4. Pass threadActivityId at both the SharePoint file card and OneDrive file-link call sites

Unit tests

Added 2 tests in send.test.ts:

  1. SharePoint file card sends include threadActivityId when replyStyle: "thread" in a channel
  2. OneDrive fallback sends include threadActivityId when replyStyle: "thread" in a channel

Real behavior proof

Behavior or issue addressed: Microsoft Teams channel replies with file attachments appear as top-level messages instead of in-thread when replyStyle: "thread" is configured.

Real environment tested: macOS 15.4 (darwin arm64), Node.js v22.22.3. Unit tests verify the exact code paths identified in the ClawSweeper source-level reproduction.

Exact steps or command run after this patch: Unit tests exercise both the SharePoint file card and OneDrive fallback branches with replyStyle: "thread" and conversationType: "channel", asserting that sendMSTeamsActivityWithReference receives the correct threadActivityId option.

Evidence after fix:

✓ routes SharePoint file card sends to the channel thread when replyStyle is thread
✓ routes OneDrive fallback file sends to the channel thread when replyStyle is thread

Observed result after fix: sendMSTeamsActivityWithReference is called with threadActivityId matching ref.threadId for both SharePoint and OneDrive attachment sends when replyStyle === "thread" and the conversation is a channel. The threadActivityId parameter is omitted (undefined) for non-channel conversations and when replyStyle is "top-level", preserving existing behavior.

What was not tested: Live Microsoft Teams tenant end-to-end test with actual thread routing verification. The code change follows the exact same thread-root resolution pattern already used by the text/inline-media send path.

Related

Closes #88836

🤖 Generated with Claude Code

@openclaw-barnacle openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 1, 2026
@clawsweeper

clawsweeper Bot commented Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 11, 2026, 9:20 AM ET / 13:20 UTC.

Summary
The PR forwards the resolved Microsoft Teams channel thread root into SharePoint file-card and OneDrive file-link sends and adds focused regression tests.

PR surface: Source +11, Tests +98. Total +109 across 2 files.

Reproducibility: no. live high-confidence reproduction was run in this read-only review. Source inspection shows current main sends SharePoint and OneDrive file activities through sendProactiveActivityRaw without a thread activity id while the text/media path resolves ref.threadId ?? ref.activityId for threaded channel replies.

Review metrics: 1 noteworthy metric.

  • Threaded file-send branches: 2 branches changed. Both SharePoint native file cards and OneDrive fallback file links bypassed the existing text-path threading helper, so both branches need review and proof.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
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:

  • Upload accessible redacted Teams proof showing a file attachment reply landing in the intended channel thread after the patch.
  • Confirm maintainers should land this branch rather than the duplicate Teams file-threading PR.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The updated proof is targeted unit-test output only, not accessible after-fix Teams tenant proof; add accessible screenshots, recordings, terminal/live output, or redacted logs with private IPs, API keys, phone numbers, non-public endpoints, and tenant details removed, then update the PR body to trigger re-review or ask a maintainer to comment @clawsweeper re-review.

Mantis proof suggestion
A visible Teams channel/thread transcript would materially improve review confidence for the changed message-delivery behavior. 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 a Microsoft Teams channel file attachment reply with replyStyle thread lands in the intended thread, not as a top-level message.

Risk before merge

  • [P1] Accessible live Teams proof is still missing; the updated proof is targeted unit-test output rather than an after-fix tenant transcript or recording.
  • [P1] fix(msteams): thread proactive file sends #88850 remains an open duplicate implementation for the same Teams file-threading bug, so maintainers should pick one branch and avoid landing both.

Maintainer options:

  1. Require accessible Teams proof (recommended)
    Ask for an accessible redacted Teams screenshot, recording, transcript, or logs showing an after-fix file attachment reply landing in the intended channel thread, then refresh review.
  2. Choose one duplicate branch
    Compare this PR with fix(msteams): thread proactive file sends #88850 and land only one implementation so the Teams send path and tests do not diverge.
  3. Accept only with maintainer-held proof
    A maintainer may proceed without contributor proof only if they have independent tenant proof and record that proof in the PR before merge.

Next step before merge

  • [P1] Human review remains needed for accessible real Teams proof and duplicate-branch selection; there is no narrow code repair for automation to apply.

Security
Cleared: The diff only changes Teams send routing and tests; I found no concrete security or supply-chain concern.

Review details

Best possible solution:

Land one focused Teams file-threading fix after accessible redacted Teams proof shows the file reply lands in the intended channel thread.

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

No live high-confidence reproduction was run in this read-only review. Source inspection shows current main sends SharePoint and OneDrive file activities through sendProactiveActivityRaw without a thread activity id while the text/media path resolves ref.threadId ?? ref.activityId for threaded channel replies.

Is this the best way to solve the issue?

Yes, this is the right owner boundary because it keeps the fix inside the Microsoft Teams plugin send path and reuses the existing SDK proactive thread option rather than adding a new core or config surface.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 79d7defd0ba4.

Label changes

Label justifications:

  • P2: The PR addresses a bounded Microsoft Teams channel delivery bug for file attachments.
  • merge-risk: 🚨 message-delivery: The change affects whether Teams file replies land in a channel thread or as top-level messages.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish 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 updated proof is targeted unit-test output only, not accessible after-fix Teams tenant proof; add accessible screenshots, recordings, terminal/live output, or redacted logs with private IPs, API keys, phone numbers, non-public endpoints, and tenant details removed, then update the PR body to trigger re-review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +11, Tests +98. Total +109 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 12 1 +11
Tests 1 99 1 +98
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 111 2 +109

What I checked:

  • Current main file sends bypass threaded delivery: On current main, the SharePoint native file card branch and OneDrive fallback branch call sendProactiveActivityRaw without any thread activity id, unlike the text/media helper path. (extensions/msteams/src/send.ts:306, 79d7defd0ba4)
  • PR forwards threadActivityId through the raw file send path: The PR head computes threadActivityId from ref.threadId ?? ref.activityId for channel/thread sends, passes it at both file-send call sites, and forwards it into sendMSTeamsActivityWithReference. (extensions/msteams/src/send.ts:171, 67f04bcdc8c0)
  • Sibling text path already uses the same thread-root invariant: sendMSTeamsMessages resolves the channel thread root from threadId with activityId fallback and passes it to sendMSTeamsActivityWithReference for proactive sends. (extensions/msteams/src/messenger.ts:546, 79d7defd0ba4)
  • Repository docs define replyStyle thread preservation: The Microsoft Teams docs say replyStyle: "thread" re-attaches the original thread root to the outbound conversation reference and top-level intentionally omits that suffix. Public docs: docs/channels/msteams.md. (docs/channels/msteams.md:822, 79d7defd0ba4)
  • PR tests cover both changed file branches: The PR adds focused tests asserting threadActivityId is passed for SharePoint file-card sends and OneDrive fallback file-link sends when a channel resolves to threaded replies. (extensions/msteams/src/send.test.ts:387, 67f04bcdc8c0)
  • Microsoft Teams docs support conversation-id based thread addressing: Microsoft Learn states channel conversation IDs include the top-level message ID and can be cached for future replies, and proactive sends use stored conversation references or conversation IDs. (learn.microsoft.com)

Likely related people:

  • Vincent Koc: Current shallow blame for the Teams send and messenger paths points at a recent main commit authored by Vincent Koc. (role: recent area contributor; confidence: medium; commits: 76ce9d6d228c; files: extensions/msteams/src/send.ts, extensions/msteams/src/messenger.ts)
  • charles-openclaw: A separate open PR implements the same Teams proactive file-threading behavior against the same files. (role: adjacent duplicate implementation author; confidence: medium; commits: 0c08ecb841ad; files: extensions/msteams/src/send.ts, extensions/msteams/src/send.test.ts)
  • cmc099: The linked bug report and PR comments provide the Teams user scenario and attempted real-world video proof for the attachment threading behavior. (role: issue reporter and proof provider; confidence: medium)
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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 1, 2026
@BradGroux BradGroux mentioned this pull request Jun 2, 2026
Draft
@cmc099

cmc099 commented Jun 4, 2026

Copy link
Copy Markdown

Here is a video showing proof that this fix correctly directs outbound attachments to the proper thread:

https://innercoreit-my.sharepoint.com/:v:/p/chris_mcfarling/IQATO0NnjT-FSLS4aOBdak7yAfvFJ34zG4OCNc8YMPeOUWg?nav=eyJyZWZlcnJhbEluZm8iOnsicmVmZXJyYWxBcHAiOiJPbmVEcml2ZUZvckJ1c2luZXNzIiwicmVmZXJyYWxBcHBQbGF0Zm9ybSI6IldlYiIsInJlZmVycmFsTW9kZSI6InZpZXciLCJyZWZlcnJhbFZpZXciOiJNeUZpbGVzTGlua0NvcHkifX0&e=kt0NUT

secooond added 2 commits June 11, 2026 20:42
secooond
fix(msteams): route file attachment sends through thread-aware delive…
8827fd6 
…ry path

SharePoint file card and OneDrive file-link sends bypassed the
thread-aware sendMSTeamsMessages path, calling sendProactiveActivityRaw
directly without threadActivityId. When replyStyle is "thread" in a
channel conversation, attachment replies appeared as top-level messages
instead of in-thread replies.

Add threadActivityId resolution to the file send branches and pass it
through sendProactiveActivityRaw to sendMSTeamsActivityWithReference.
This reuses the same thread-root resolution as the text/inline-media
path (ref.threadId ?? ref.activityId).

Closes openclaw#88836
secooond
fix(msteams): repair file-send test fixtures with proper app and OneD…
67f04bc 
…rive mocks

- Replace adapter:{} with app:createMockApp() in both new tests so
  sendMSTeamsActivityWithReference receives a real app parameter
- Add uploadAndShareOneDrive to mockState for proper mock control
- Mock OneDrive upload result so the production path reaches the
  threaded send call
- Add uploadAndShareOneDrive to beforeEach reset

Refs: openclaw#88836
@whiteyzy
whiteyzy force-pushed the fix/msteams-file-attachment-thread-routing-88836 branch from cabcc67 to 67f04bc Compare June 11, 2026 12:44
@whiteyzy

whiteyzy commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated (per P2 review feedback):

  • P2 fixed: Both new tests now use createMockApp() instead of bare adapter: {}, so sendMSTeamsActivityWithReference receives a real app
  • P2 fixed: OneDrive test properly mocks uploadAndShareOneDrive via mockState with a complete upload result
  • Added uploadAndShareOneDrive to mockState hoisted object and beforeEach reset
  • All 13 tests pass including the two fixed regression tests
  • Rebased on latest upstream/main

Real behavior proof: Community member @cmc099 provided video proof showing the fix correctly directs outbound attachments to the proper thread:
https://innercoreit-my.sharepoint.com/:v:/p/chris_mcfarling/IQATO0NnjT-FSLS4aOBdak7yAfvFJ34zG4OCNc8YMPeOUWg

@clawsweeper

clawsweeper Bot commented Jun 11, 2026 •
edited
Loading

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:

@whiteyzy

whiteyzy commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated proof (replaces inaccessible SharePoint video):

The community SharePoint video returned 403 FORBIDDEN and is not reviewer-accessible. Replaced with terminal test output that exercises the actual sendMessageMSTeams production path:

✓ routes SharePoint file card sends to the channel thread when replyStyle is thread
✓ routes OneDrive fallback file sends to the channel thread when replyStyle is thread

Test Files  1 passed (1)
     Tests  13 passed (13)

The fix follows the exact same threadActivityId resolution pattern as sendMSTeamsMessages (line 530 in messenger.ts), which already works correctly in production for text/inline-media sends. The file attachment branches now route through the same thread-aware path:

  • SharePoint: sendProactiveActivityRaw receives threadActivityId from ref.threadId ?? ref.activityId when conversationType === "channel" and replyStyle === "thread"
  • OneDrive: Same resolution, passed through sendProactiveActivityRaw → sendMSTeamsActivityWithReference
  • Top-level: threadActivityId is undefined (omitted), preserving existing behavior

@whiteyzy

whiteyzy commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Closing: unable to provide accessible real Microsoft Teams proof.

ClawSweeper requires redacted Teams screenshots/recording showing file attachment replies land in the intended channel thread. The community SharePoint video is inaccessible (403 Forbidden) and the contributor does not have a Teams tenant to generate new proof.

The code fix is rated 🐚 platinum hermit (patch quality) and follows the exact same threadActivityId resolution pattern used by sendMSTeamsMessages. A maintainer with Teams access can verify and land.

Closes #88836.

@whiteyzy whiteyzy closed this Jun 11, 2026
@cmc099

cmc099 commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Here's a YouTube link of the same video showing the fix working in Teams. Should resolve the 403 SharePoint issue.

https://youtu.be/k3FlxdcU9zg

@whiteyzy Could we reopen this PR for review?

@PacNxs-Admin

PacNxs-Admin commented Jun 17, 2026

Copy link
Copy Markdown

Kindly requesting to reopen this PR for review given the updated proof video.

This was referenced Jun 19, 2026
Open
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: msteams Channel integration: msteams merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: msteams messages with attachments misthreaded

3 participants

@whiteyzy @cmc099 @PacNxs-Admin