Skip to content

fix(msteams): preserve proactive thread replies#78387

Merged
amknight merged 1 commit intomainfrom
codex/msteams-proactive-threading
May 6, 2026
Merged

fix(msteams): preserve proactive thread replies#78387
amknight merged 1 commit intomainfrom
codex/msteams-proactive-threading

Conversation

@amknight
Copy link
Copy Markdown
Member

@amknight amknight commented May 6, 2026

Summary

  • Resolve the Microsoft Teams proactive send replyStyle from stored conversation context and the existing Teams channel/team/global policy instead of hardcoding top-level.
  • Allow no-context threaded proactive sends to use the existing proactive thread-targeting path, so channel sends with threadId or legacy activityId build the ;messageid=<threadRoot> conversation id.
  • Add focused regression coverage for proactive reply-style resolution, send-path forwarding, no-context thread fallback, and configured top-level channel overrides.
  • Add an unreleased changelog entry.

Closes #78298.

Root Cause

The proactive text/media send path always passed replyStyle: "top-level" into sendMSTeamsMessages(), so CLI/message-tool sends ignored stored Teams channel thread roots. Switching that path to thread was previously blocked because sendMSTeamsMessages() threw when no live turn context was present.

Manual CLI Repro Proof

I exercised the actual CLI command path with a temp OpenClaw config/state directory and a network shim that only replaced AAD token acquisition and the Bot Framework service URL. The command under test was:

openclaw message send --channel msteams --target conversation:19:abc@thread.tacv2 --message "CLI proactive reply" --json
  • Local before/after CLI proof: origin/main sent to https://service.example.com/v3/conversations/19:abc@thread.tacv2/activities; this branch sent to https://service.example.com/v3/conversations/19:abc@thread.tacv2;messageid=thread-root-msg-id/activities.
  • Hetzner Crabbox cbx_2a5cfab75860 (coral-crab): same CLI command passed before/after proof with HETZNER_CLI_SUMMARY beforeStatus=0 beforeId=19:abc@thread.tacv2 afterStatus=0 afterId=19:abc@thread.tacv2;messageid=thread-root-msg-id afterRef=93b8a7c9848e3eadaf2f3e03ddcc22f72e5446d2.
  • Lease was released after the Hetzner run.

No live Teams tenant send was run; no Teams/Microsoft/Azure credentials were present in env or ~/.profile.

Additional Manual Adapter Proof

A throwaway harness also exercised the Bot Framework proactive adapter boundary directly with a stored channel reference containing threadId: "thread-root-msg-id" and no live turn context.

  • Hetzner Crabbox cbx_2c7ac22b6281 (crimson-krill): before patch on origin/main failed with {"ok":false,"name":"Error","message":"Missing context for replyStyle=thread"}.
  • Same Hetzner Crabbox run after patch on 93b8a7c9848e3eadaf2f3e03ddcc22f72e5446d2 succeeded with {"ok":true,"ids":["id:manual proactive reply"],"sent":["manual proactive reply"],"proactiveConversationId":"19:abc@thread.tacv2;messageid=thread-root-msg-id","proactiveActivityId":null}.
  • Lease was released after the run.

Verification

  • Manual local CLI before/after proof: passed as described above.
  • Manual Hetzner Crabbox CLI before/after proof on cbx_2a5cfab75860: passed as described above.
  • Manual local adapter before/after harness: before origin/main failed with Missing context for replyStyle=thread; after branch succeeded with threaded proactive conversation id.
  • Manual Hetzner Crabbox adapter before/after harness on cbx_2c7ac22b6281: passed as described above.
  • pnpm test extensions/msteams/src/send-context.test.ts extensions/msteams/src/send.test.ts extensions/msteams/src/messenger.test.ts locally: 3 files, 47 tests passed.
  • pnpm exec oxfmt --check --threads=1 extensions/msteams/src/send-context.ts extensions/msteams/src/send.ts extensions/msteams/src/messenger.ts extensions/msteams/src/send-context.test.ts extensions/msteams/src/send.test.ts extensions/msteams/src/messenger.test.ts CHANGELOG.md
  • git diff --check
  • Crabbox/Testbox focused test on tbx_01kqy8brzxk1e58fhcdggmkdfs: 3 files, 47 tests passed; lease stopped.
  • Crabbox/Testbox pnpm check:changed on tbx_01kqy8k9cb8300643vamz6d3ss: passed; lease stopped.

@openclaw-barnacle openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: M maintainer Maintainer-authored PR labels May 6, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 6, 2026

Codex review: needs maintainer review before merge.

Summary
The branch updates MS Teams proactive sends to resolve replyStyle from stored conversation context and policy, routes no-context threaded sends through proactive thread targeting, adds regression tests, and adds a changelog entry.

Reproducibility: yes. Current main source inspection shows proactive sends force replyStyle: "top-level" and the existing threaded path throws without live context; the PR body also reports a before/after adapter harness.

Real behavior proof
Not applicable: The external-contributor proof gate does not apply because this is a MEMBER/maintainer-labeled PR, though the body includes before/after adapter-harness and Testbox proof without a live tenant send.

Next step before merge
Remaining work is maintainer review and merge coordination; no narrow ClawSweeper repair defect was found.

Security
Cleared: The diff only changes MS Teams routing logic, tests, and changelog text; it does not add dependencies, workflows, permissions, secrets handling, or new execution paths.

Review details

Best possible solution:

Merge one canonical fix for the MS Teams proactive threading path, then resolve #78298 and supersede #78299 if this branch is chosen.

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

Yes. Current main source inspection shows proactive sends force replyStyle: "top-level" and the existing threaded path throws without live context; the PR body also reports a before/after adapter harness.

Is this the best way to solve the issue?

Yes. Reusing the existing Teams route policy and proactive ;messageid=<threadRoot> construction is the narrow maintainable fix, and the PR preserves configured top-level overrides.

What I checked:

  • Current main forces top-level proactive sends: sendTextWithMedia() still calls sendMSTeamsMessages() with replyStyle: "top-level", matching the linked bug report's root cause for CLI/message-tool channel sends. (extensions/msteams/src/send.ts:399, e437763246de)
  • Current main blocks no-context threaded sends: sendMSTeamsMessages() resolves threadId ?? activityId, but the replyStyle === "thread" path throws when no live turn context exists, which is the proactive send shape. (extensions/msteams/src/messenger.ts:570, e437763246de)
  • Stored references carry thread routing data: Inbound MS Teams channel handling stores a normalized conversation id plus threadId from the ;messageid= suffix or replyToId, and also stores teamId for policy lookup. (extensions/msteams/src/monitor-handler/message-handler.ts:246, e437763246de)
  • Reply style is documented behavior: The MS Teams docs define thread vs top-level and warn that using top-level in Posts-style channels creates separate top-level posts instead of in-thread replies. Public docs: docs/channels/msteams.md. (docs/channels/msteams.md:724, e437763246de)
  • PR uses existing policy and thread helper: The diff adds resolveMSTeamsProactiveReplyStyle(), passes its result into sendMSTeamsMessages(), and changes no-context threaded sends to call sendProactively(messages, 0, resolvedThreadId). (extensions/msteams/src/send-context.ts:47, 93b8a7c9848e)
  • Regression and real-behavior proof reported: The PR body reports before/after adapter-harness output, local targeted tests, focused Testbox tests, and pnpm check:changed; no live Microsoft Teams tenant send was run. (93b8a7c9848e)

Likely related people:

  • steipete: Blame on the current affected lines and recent commit metadata show broad maintenance across the MS Teams send, messenger, policy, and inbound routing files. (role: recent maintainer; confidence: high; commits: 05eda57b3c72, 3f002b10d281, ffe67e9cdc9e; files: extensions/msteams/src/send.ts, extensions/msteams/src/messenger.ts, extensions/msteams/src/policy.ts)
  • sudie-codes: Prior merged MS Teams work added channel thread routing and adjacent outbound/message lifecycle behavior that this PR builds on. (role: threading and proactive-send maintainer; confidence: high; commits: 9edfefedf7bf, 784318799bf0, ba1b8424f48e; files: extensions/msteams/src/messenger.ts, extensions/msteams/src/messenger.test.ts, extensions/msteams/src/send.ts)
  • amknight: Beyond authoring this PR, prior merged MS Teams work touched adjacent message tracking and monitor-handler behavior in current main. (role: recent adjacent maintainer; confidence: medium; commits: 6ae09d029c38; files: extensions/msteams/src/monitor-handler/message-handler.ts, extensions/msteams/src/sent-message-cache.ts)

Remaining risk / open question:

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

@amknight amknight marked this pull request as ready for review May 6, 2026 09:18
@amknight amknight merged commit 28e27ca into main May 6, 2026
163 of 168 checks passed
@amknight amknight deleted the codex/msteams-proactive-threading branch May 6, 2026 10:01
@harrisali0101
Copy link
Copy Markdown
Contributor

Just to add a real-world data point: this fix resolved a production case for us on @openclaw/msteams 2026.5.5 where a long-running agent's proactive replies (sent via mcp__openclaw__message after the inbound Bot Framework turn context had expired) kept landing as new top-level posts in #Approvals instead of inside the user's thread, even with channels.msteams.replyStyle: "thread" configured globally.

Holding pattern until this lands in npm: patch-package against 5.5 with the equivalent line-575 change applied locally — replies now thread correctly. Confirmed on a Posts-style channel with replyStyle: "thread" + requireMention: true, and verified the legacy activityId fallback on a freshly-seeded conversation reference that didn't yet have threadId set.

Looking forward to the next release. Thanks @amknight for the fix and the test coverage in messenger.test.ts / send-context.test.ts — both of those scenarios match exactly what we hit.

Filed a small docs follow-up at #78835 to document the replyStyle resolution precedence (channel/team/global/implicit-from-requireMention) and the thread-root preservation behavior, since we burned a few hours debugging this without a docs anchor.

amknight pushed a commit that referenced this pull request May 7, 2026
…ontext preservation (#78835)

The existing replyStyle section explains the Posts-vs-Threads tradeoff but
doesn't document how the value is actually resolved at send-time, nor what
happens to thread-root context across configurations. Operators who hit
unexpected top-level posts (e.g., requireMention=false setups, long-running
agents whose proactive sends fall outside the live Bot Framework turn) have
no docs-side anchor for understanding which knob to flip.

Add two subsections under Reply Style:

1. Resolution precedence — channel > team > global > implicit default,
   plus the requireMention-derived implicit default.
2. Thread context preservation — describes that replyStyle=thread re-attaches
   the original thread root on outbound (live and proactive paths after
   #78387), with the threadId/activityId fallback for legacy stored refs.
   Calls out the deliberate "no thread suffix" behavior for replyStyle=top-level.

Pure documentation change, no behavior or surface impact.
rogerdigital pushed a commit to rogerdigital/openclaw that referenced this pull request May 7, 2026
Co-authored-by: Alex Knight <15041791+amknight@users.noreply.github.com>
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
Co-authored-by: Alex Knight <15041791+amknight@users.noreply.github.com>
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
…ontext preservation (openclaw#78835)

The existing replyStyle section explains the Posts-vs-Threads tradeoff but
doesn't document how the value is actually resolved at send-time, nor what
happens to thread-root context across configurations. Operators who hit
unexpected top-level posts (e.g., requireMention=false setups, long-running
agents whose proactive sends fall outside the live Bot Framework turn) have
no docs-side anchor for understanding which knob to flip.

Add two subsections under Reply Style:

1. Resolution precedence — channel > team > global > implicit default,
   plus the requireMention-derived implicit default.
2. Thread context preservation — describes that replyStyle=thread re-attaches
   the original thread root on outbound (live and proactive paths after
   openclaw#78387), with the threadId/activityId fallback for legacy stored refs.
   Calls out the deliberate "no thread suffix" behavior for replyStyle=top-level.

Pure documentation change, no behavior or surface impact.
rogerdigital pushed a commit to rogerdigital/openclaw that referenced this pull request May 9, 2026
Co-authored-by: Alex Knight <15041791+amknight@users.noreply.github.com>
rogerdigital pushed a commit to rogerdigital/openclaw that referenced this pull request May 9, 2026
…ontext preservation (openclaw#78835)

The existing replyStyle section explains the Posts-vs-Threads tradeoff but
doesn't document how the value is actually resolved at send-time, nor what
happens to thread-root context across configurations. Operators who hit
unexpected top-level posts (e.g., requireMention=false setups, long-running
agents whose proactive sends fall outside the live Bot Framework turn) have
no docs-side anchor for understanding which knob to flip.

Add two subsections under Reply Style:

1. Resolution precedence — channel > team > global > implicit default,
   plus the requireMention-derived implicit default.
2. Thread context preservation — describes that replyStyle=thread re-attaches
   the original thread root on outbound (live and proactive paths after
   openclaw#78387), with the threadId/activityId fallback for legacy stored refs.
   Calls out the deliberate "no thread suffix" behavior for replyStyle=top-level.

Pure documentation change, no behavior or surface impact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: msteams Channel integration: msteams maintainer Maintainer-authored PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: msteams proactive send path hardcodes replyStyle top-level, ignoring thread config

2 participants