Skip to content

feat(msteams): per-call topLevel override on send action for proactive new channel threads#93292

Open
leen109 wants to merge 1 commit into
openclaw:mainfrom
leen109:feat/msteams-toplevel-override
Open

feat(msteams): per-call topLevel override on send action for proactive new channel threads#93292
leen109 wants to merge 1 commit into
openclaw:mainfrom
leen109:feat/msteams-toplevel-override

Conversation

@leen109

@leen109 leen109 commented Jun 15, 2026

Copy link
Copy Markdown

Closes the gap surfaced in #93288.

Summary

Adds a topLevel: true option to the msteams message/send action so the bot can post a fresh root post / new thread in a channel without flipping the static channels.msteams.replyStyle config (which is correctly set to "thread" for the @mention reply path).

Motivation

Two distinct behaviors are needed in the same channel:

  1. Reply path (existing, works via static replyStyle: "thread"): bot is @mentioned inside a thread → reply lands in the same thread.
  2. Proactive new-thread path (missing today): bot is asked, via DM/cron/skill, to post a warehouse warning, broadcast, or cross-team announcement → must surface as a fresh root post, not as a reply under whichever thread happened to last seed the conversation reference.

Globally setting replyStyle: "top-level" is not acceptable because it breaks (1). There is currently no per-call override.

Changes

  • send-context.ts: resolveMSTeamsSendContext accepts an optional replyStyleOverride: MSTeamsReplyStyle and honors it before the config-derived resolveMSTeamsProactiveReplyStyle() lookup.
  • send.ts: SendMSTeamsMessageParams accepts replyStyleOverride and forwards it.
  • channel.ts: describeMSTeamsMessageTool exposes a topLevel boolean on the send action schema. handleAction has a new branch for send + !presentation + topLevel: true that routes through sendMessageMSTeams with replyStyleOverride: "top-level".
  • send-context.test.ts: two new tests — override honored, fallback to config when omitted.

Backwards compatibility

Omitting topLevel leaves all existing behavior untouched. topLevel: true is silently ignored for DM/group-chat conversation types (they already default to top-level regardless). Card sends (with presentation) keep their existing path; extending sendAdaptiveCardMSTeams is left as a follow-up.

Test plan

  • Unit: extensions/msteams/src/send-context.test.ts — override honored vs. config fallback.
  • Live: patched plugin running against production tenant; agent invokes the tool with topLevel: true, send goes out without the ;messageid= suffix on the conversation reference, surfaces as a fresh root post in a Posts-style channel; topLevel omitted leaves the in-thread reply behavior untouched.

Sample usage

{
  "channel": "msteams",
  "action": "send",
  "target": "conversation:19:[email protected]",
  "topLevel": true,
  "message": "Warehouse alert: incoming delivery delayed."
}

@behalto
feat(msteams): per-call topLevel override on send action
f896fec 
Adds a `topLevel: true` option to the msteams `message`/`send` action so
the bot can post a fresh root post / new thread in a channel without
having to flip the static `channels.msteams.replyStyle` config.

The static config is correct for @mention replies (which must stay inside
their thread), but proactive announcements, warehouse warnings, and
cross-team broadcasts need to surface as new root posts. Today the only
way to get a new thread is to set `replyStyle: "top-level"` for the whole
channel, which then breaks in-thread replies.

Changes:
- `send-context.ts`: `resolveMSTeamsSendContext` accepts an optional
  `replyStyleOverride` and honors it before the config-derived
  `resolveMSTeamsProactiveReplyStyle` lookup.
- `send.ts`: `SendMSTeamsMessageParams` accepts `replyStyleOverride`,
  forwards it into the send context.
- `channel.ts`: `describeMSTeamsMessageTool` exposes a `topLevel` boolean
  on the `send` action schema. `handleAction` has a new branch for
  `send + !presentation + topLevel: true` that routes through
  `sendMessageMSTeams` with `replyStyleOverride: "top-level"`.
- `send-context.test.ts`: two new tests — override honored, fallback to
  config when omitted.

Backwards compatible: omitting `topLevel` leaves the existing reply
behavior untouched. Ignored for DM/group-chat conversation types (they
already default to `top-level`).

Refs openclaw#93288
@openclaw-barnacle openclaw-barnacle Bot added channel: msteams Channel integration: msteams triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. size: S labels Jun 15, 2026
@clawsweeper

clawsweeper Bot commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 11:16 PM ET / 03:16 UTC.

Summary
The PR adds a send-scoped topLevel schema option for MS Teams, forwards a replyStyleOverride through text sends, and adds send-context tests for override and fallback behavior.

PR surface: Source +62, Tests +67. Total +129 across 4 files.

Reproducibility: yes. source-level: current main exposes no Teams send-scoped topLevel schema property and resolveMSTeamsSendContext has no per-call reply-style override. I did not run a live Teams tenant send in this read-only review.

Review metrics: 1 noteworthy metric.

  • Message Tool Schema Surface: 1 send-scoped property added. The PR adds a model-facing Teams send parameter, so schema, dispatch, and supported payload combinations need to stay aligned before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #93288
Summary: This PR is one of two open candidate implementations for the canonical MS Teams per-call top-level send request; the earlier same-topic attempt is closed unmerged.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof 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 real Teams proof showing topLevel: true creates a channel root post and omitted topLevel preserves thread replies.
  • [P1] Repair or explicitly reject topLevel media and presentation combinations with focused tests.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body claims live tenant testing but provides no inspectable redacted Teams screenshot, recording, terminal output, copied live output, logs, or linked artifact; the contributor should add redacted proof and update the PR body for a fresh review.

Proof path suggestion
Real MS Teams root-post placement is visible transport behavior and a redacted UI or diagnostic proof would materially help review. Mantis is currently scoped to Telegram, Discord, and web UI chat proof, so it is not the right proof path for this surface.
Use maintainer screenshot/manual proof, browser or Playwright proof, Crabbox where appropriate, or normal local artifact proof instead.

Risk before merge

  • [P1] topLevel with presentation can still take the card path and ignore the caller's root-post placement intent.
  • [P1] topLevel text sends with media/file parameters can report success while omitting requested media because the new branch does not forward mediaUrl or filename.
  • [P1] The PR body claims live Teams testing but includes no inspectable redacted artifact or logs, and another open PR targets the same canonical issue.

Maintainer options:

  1. Repair Payload Handling First (recommended)
    Forward top-level placement through the media and card paths or add tested, clear unsupported-combination errors before merge.
  2. Scope Text Only Deliberately
    Maintainers can choose a plain-text-only first version if non-text payloads fail clearly instead of silently changing delivery behavior.
  3. Pause For Branch Selection
    Because another open PR targets the same canonical issue, maintainers may pause this branch until one implementation is selected and proven.

Next step before merge

  • [P1] Human/contributor action is required because real Teams proof is missing and maintainers still need to select or consolidate between overlapping implementation branches.

Security
Cleared: No concrete security or supply-chain concern was found in the MS Teams runtime and test diff.

Review findings

  • [P1] Reject or handle topLevel presentation sends — extensions/msteams/src/channel.ts:739-740
  • [P1] Forward media fields into topLevel sends — extensions/msteams/src/channel.ts:747-754
Review details

Best possible solution:

Carry topLevel through the established Teams outbound/send-context path for supported payloads, or explicitly reject unsupported payload combinations with focused tests, then require redacted real Teams proof before merge.

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

Yes, source-level: current main exposes no Teams send-scoped topLevel schema property and resolveMSTeamsSendContext has no per-call reply-style override. I did not run a live Teams tenant send in this read-only review.

Is this the best way to solve the issue?

No as implemented. The per-call override seam is the right narrow direction, but the patch needs to preserve or explicitly reject supported send payload combinations and supply real Teams proof.

Full review comments:

  • [P1] Reject or handle topLevel presentation sends — extensions/msteams/src/channel.ts:739-740
    The new branch only handles topLevel when there is no presentation, so a send with both topLevel: true and a presentation still takes the existing card path without the override. That can ignore the caller's root-post intent instead of either honoring it or failing clearly.
    Confidence: 0.93
  • [P1] Forward media fields into topLevel sends — extensions/msteams/src/channel.ts:747-754
    The topLevel branch calls sendMessageMSTeams without forwarding normalized media fields such as mediaUrl or filename, even though the send action still accepts media/file path inputs. A topLevel media send can report success while sending only text.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority MS Teams channel delivery improvement with limited plugin-specific blast radius but concrete payload and placement risks.
  • merge-risk: 🚨 message-delivery: The diff can silently ignore top-level placement for cards and omit requested media on topLevel sends.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body claims live tenant testing but provides no inspectable redacted Teams screenshot, recording, terminal output, copied live output, logs, or linked artifact; the contributor should add redacted proof and update the PR body for a fresh review.
Evidence reviewed

PR surface:

Source +62, Tests +67. Total +129 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 3 72 10 +62
Tests 1 67 0 +67
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 139 10 +129

What I checked:

Likely related people:

  • vincentkoc: Local blame on the checked-out current-main snapshot places the current Teams message-tool schema, send-context signature, send helper, outbound payload path, and standard-param allowlist on recent work by Vincent Koc. (role: recent area contributor; confidence: high; commits: cb00045afca9, 03415bb69675; files: extensions/msteams/src/channel.ts, extensions/msteams/src/send.ts, extensions/msteams/src/send-context.ts)
  • steipete: GitHub path history shows repeated recent work on Teams presentations, plugin SDK boundaries, and outbound delivery paths that this PR intersects. (role: plugin/outbound architecture contributor; confidence: high; commits: f5090d262420, 1507a9701b83, 05eda57b3c72; files: extensions/msteams/src/channel.ts, extensions/msteams/src/outbound.ts, extensions/msteams/src/send.ts)
  • heyitsaamir: The Teams SDK rebase carried major proactive send, send-context, messenger, and reply-style behavior now used by this path. (role: Teams SDK migration contributor; confidence: medium; commits: 04c29825356f; files: extensions/msteams/src/send-context.ts, extensions/msteams/src/send.ts, extensions/msteams/src/messenger.ts)
  • amknight: Prior merged work preserved MS Teams proactive thread replies, directly adjacent to the replyStyle behavior this PR modifies. (role: proactive-threading contributor; confidence: medium; commits: 28e27ca5d16a; files: extensions/msteams/src/send-context.ts, extensions/msteams/src/messenger.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.
Review history (1 earlier review cycle)
  • reviewed 2026-06-22T21:30:47.376Z sha f896fec :: needs real behavior proof before merge. :: [P1] Reject or handle topLevel presentation sends | [P1] Forward media fields into topLevel sends

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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 15, 2026
@BradGroux BradGroux mentioned this pull request Jun 16, 2026
Draft
This was referenced Jun 19, 2026
Closed
Open
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 21, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added stale Marked as stale due to inactivity triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. and removed stale Marked as stale due to inactivity labels Jul 21, 2026
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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@leen109 @behalto