feat(msteams): add native plugin interactivity parity#55828
Conversation
openclaw-barnacle
Bot
added
docs
Greptile SummaryThis PR adds native Microsoft Teams plugin interactivity to OpenClaw, bringing Teams to parity with Discord, Telegram, and Slack for the core plugin interactive dispatch path. The changes wire Key points:
Confidence Score: 4/5Safe to merge for the common case; one P1 pagination bug in the team-id fallback path affects large tenants (more than 999 teams) where the Bot Framework team key is not directly usable as a Graph GUID. The bulk of the PR (interactive dispatch wiring, runtime surface, allowlist hardening, Graph message operations, send/edit/typing primitives) is well-structured and backed by targeted tests. The single P1 finding — graphPathFromNextLink doubling the /v1.0 version prefix on paginated @odata.nextLink URLs — only affects the deep fallback path in resolveTeamGroupId, which is only reached when the direct /teams/{id} call fails and the raw ID does not look like a GUID. Most tenants will never reach this code; large enterprises could. The fix is a one-line regex in graphPathFromNextLink. All other findings are P2. extensions/msteams/src/graph-thread.ts — graphPathFromNextLink pagination URL construction.
|
| Filename | Overview |
|---|---|
| extensions/msteams/src/graph-thread.ts | Adds paged group-scan fallback for Bot Framework team-key → Graph group-id resolution; contains a URL-construction bug that doubles the API version prefix on pagination, and a per-team primaryChannel fan-out that can be expensive in large tenants. |
| extensions/msteams/src/plugin-interactions.ts | New file wiring Teams invoke payloads through native OpenClaw plugin interactive dispatch; authorization, conversation-ID extraction, and respond-callback wiring look correct. |
| extensions/msteams/src/resolve-allowlist.ts | Switches from find-General-channel to primaryChannel API for team-key resolution and adds fail-closed behavior when the lookup fails; behavioral change is intentional and covered by tests. |
| extensions/msteams/src/graph-messages.ts | Rewrites conversation-target resolution to a typed discriminated union, adding proper team-group-id resolution for channel paths; logic is sound and test coverage updated. |
| extensions/msteams/src/graph.ts | Adds PATCH support, getPrimaryChannelForTeam, and editChannelMSTeams; channel rename correctly resolves the Graph team id before issuing the PATCH. |
| extensions/msteams/src/send.ts | Makes edit-message text optional and adds card support; adds sendTypingMSTeams with correct channel-type guard. |
| src/plugins/interactive.ts | Adds msteams as a first-class interactive channel alongside telegram/discord/slack; dispatch routing and overload signatures look correct. |
| src/plugins/runtime/runtime-msteams.ts | New file exposing the Teams runtime surface to plugins via lazy-loaded operations; typing.start correctly validates cfg presence before creating a lease. |
| extensions/msteams/src/sdk.ts | Uses string-concatenated module IDs to prevent bundler static analysis from eagerly resolving optional Teams SDK deps; intentional lazy-load pattern. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/msteams/src/graph-thread.ts
Line: 27-33
Comment:
**Pagination URL doubles the API version prefix**
`graphPathFromNextLink` returns `url.pathname + url.search`. Microsoft Graph `@odata.nextLink` values embed the API version in the path (the pathname is `/v1.0/groups`, not `/groups`). When `fetchGraphJson` prepends `GRAPH_ROOT` (`https://graph.microsoft.com/v1.0`), the resulting URL becomes `https://graph.microsoft.com/v1.0/v1.0/groups?...` — a 404. Team-ID resolution silently fails to paginate in any tenant with more than 999 teams.
Strip the API-version segment from the pathname before returning it:
```typescript
function graphPathFromNextLink(nextLink: string): string | null {
try {
const url = new URL(nextLink);
// nextLink includes the API version (e.g. /v1.0/groups).
// Strip it so the caller can prepend GRAPH_ROOT without doubling the prefix.
const pathWithoutVersion = url.pathname.replace(/^\/(?:v\d+(?:\.\d+)*|beta)\b/, "");
return pathWithoutVersion + url.search;
} catch {
return null;
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/msteams/src/graph-thread.ts
Line: 86-120
Comment:
**O(N) per-team primary-channel fan-out in the fallback path**
The new fallback enumerates all provisioned teams with `$top=999`, then calls `/teams/{id}/primaryChannel` individually for each candidate until one matches. In a large tenant this can be hundreds of sequential Graph calls and will hit rate limits before succeeding. Consider batching with `$expand=primaryChannel` on the groups query so the primary channel id is returned alongside the group id in a single response per page, reducing the fan-out to one call per page instead of one call per team.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/msteams/src/graph-thread.ts
Line: 19-21
Comment:
**`looksLikeGraphTeamId` is duplicated across three files**
The same helper (`/^[0-9a-fA-F-]{16,}$/.test(value.trim())`) is now defined identically in `graph-thread.ts` (line 19), `graph.ts`, and `graph-messages.ts`. Extracting it to a shared internal utility (e.g., `graph-id-utils.ts`) would remove the drift risk and make the regex easier to update or tighten in one place.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Teams: add native plugin interactivity p..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df84e99e95
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08c6cb5eb6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
08c6cb5 to
25463ab
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25463ab775
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
25463ab to
e0ca60c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0ca60cfae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
e0ca60c to
d94e1ee
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e0d02cc10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!teamId || !channelId) { | ||
| throw new Error( | ||
| `Conversation ${cleaned} is a Teams channel but missing teamId or graphChannelId in the conversation store.`, | ||
| ); |
There was a problem hiding this comment.
This branch makes channel Graph operations hard-fail when a stored channel conversation lacks graphChannelId, but that field is newly introduced here and older msteams-conversations.json entries will not have it. After upgrade, existing bound channel conversations can throw missing teamId or graphChannelId for getMessage/pin/react/search until a new inbound message refreshes the store, which is a regression for already-persisted state. Please add a compatibility fallback (or migration/backfill) instead of immediately throwing.
Useful? React with 👍 / 👎.
| teamId, | ||
| graphChannelId: activity.channelData?.channel?.id, | ||
| channelId: activity.channelId, |
There was a problem hiding this comment.
The upsert payload always includes graphChannelId: activity.channelData?.channel?.id, so activities without channelData.channel write undefined for this field. Because the store merge path spreads incoming fields and does not preserve graphChannelId, this can erase previously known channel metadata for that conversation, and later channel Graph calls will fail on the new required-metadata check. Only write graphChannelId when it is present.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
the
rating: 🌊 off-meta tidepool
|
Codex review: found issues before merge. Reviewed July 5, 2026, 12:25 PM ET / 16:25 UTC. Summary PR surface: Source +1031, Tests +390, Docs +1200, Generated 0. Total +2621 across 39 files. Reproducibility: no. high-confidence live Teams reproduction/proof is present; the PR body explicitly says live tenant verification is not done. The target-parsing and graphChannelId upgrade blockers are source-reproducible from the PR head. Review metrics: 3 noteworthy metrics.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Proof path suggestion Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Rebase onto current main, route Teams target parsing through current channel route contracts, make graphChannelId upgrade-safe, and require maintainer API/security signoff plus redacted live Teams proof before merge. Do we have a high-confidence way to reproduce the issue? No high-confidence live Teams reproduction/proof is present; the PR body explicitly says live tenant verification is not done. The target-parsing and graphChannelId upgrade blockers are source-reproducible from the PR head. Is this the best way to solve the issue? No, not as submitted. The direction is plausible, but the best fix needs current-main route-contract alignment, upgrade-safe Teams metadata handling, maintainer API/security approval, and live tenant proof before merge. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 70f2bef6b4cf. Label changesLabel justifications:
Evidence reviewedPR surface: Source +1031, Tests +390, Docs +1200, Generated 0. Total +2621 across 39 files. View PR surface stats
Security concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (1 earlier review cycle)
|
|
ClawSweeper PR egg 🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress. Hatch commandComment Hatchability rules:
What is this egg doing here?
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
status: ⏳ waiting on author
1 participant
Summary
DO NOT MERGE YET -- for review
src/plugins/interactive.tsandsrc/plugins/interactive-dispatch-adapters.ts, adds the Teams runtime surface insrc/plugins/runtime/runtime-msteams.ts,src/plugins/runtime/runtime-msteams-typing.ts, andsrc/plugins/runtime/runtime-msteams-ops.runtime.ts, wires Teams invoke handling throughextensions/msteams/src/plugin-interactions.tsandextensions/msteams/src/monitor-handler.ts, extends command/target normalization insrc/plugins/commands.tsandsrc/channels/plugins/target-parsing.ts, and fixes Teams Graph/edit/allowlist correctness inextensions/msteams/src/send.ts,extensions/msteams/src/graph.ts,extensions/msteams/src/graph-messages.ts,extensions/msteams/src/graph-thread.ts,extensions/msteams/src/resolve-allowlist.ts, andextensions/msteams/src/policy.ts.mainagent/skills TypeScript failures, and it does not claim live-tenant validation for every Teams topology (for example shared-channel behavior still needs real-environment confirmation outside this PR).Change Type
Scope
Linked Issue/PR
openclaw-codex-app-serverRoot Cause / Regression History
git blame, prior PR, issue, or refactor if known): the current code shape reflected the earlier minimal Teams MVP, wheresrc/plugins/interactive.tsand runtime channel exposure were intentionally limited and Teams only had the smaller send-oriented surface.Regression Test Plan
extensions/msteams/src/monitor-handler.plugin-interactions.test.tsextensions/msteams/src/graph-messages.test.tsextensions/msteams/src/send.test.tsextensions/msteams/src/resolve-allowlist.test.tsextensions/msteams/src/policy.test.tssrc/plugins/interactive.test.tssrc/plugins/runtime/index.test.tssrc/plugins/commands.test.tsUser-visible / Behavior Changes
teams:/msteams:conversation targets consistently.Diagram
Security Impact
Yes/No) YesYes/No) NoYes/No) YesYes/No) YesYes/No) YesYes, explain risk + mitigation:runtime.channel.msteamssurface for Teams-native operations and adds Teams-native interactive dispatch. The risk is widening what a plugin can do in Teams or routing to the wrong target. Mitigations in this PR are explicit runtime typing insrc/plugins/runtime/types-channel.ts, targeted runtime wiring instead of broad generic exposure, conversation-scoped interactive handling inextensions/msteams/src/plugin-interactions.ts, Graph team-id resolution fixes inextensions/msteams/src/graph-thread.ts/extensions/msteams/src/graph.ts, and fail-closed allowlist behavior inextensions/msteams/src/resolve-allowlist.ts/extensions/msteams/src/policy.ts.Repro + Verification
Environment
pnpmworkspace checkoutextensions/msteams)Steps
Expected
runtime.channel.msteams.Actual
main, repo-widepnpm checkis blocked by unrelatedsrc/agents/**TypeScript failures already present on top ofmain; those are out of scope for this PR.Evidence
Concrete issues fixed in this branch and covered by passing tests/review loops:
Evidence run locally during this work:
pnpm test -- extensions/msteams/src/graph-messages.test.ts extensions/msteams/src/monitor-handler.plugin-interactions.test.ts extensions/msteams/src/resolve-allowlist.test.ts extensions/msteams/src/policy.test.tsextensions/msteams/src/send.test.tssrc/plugins/interactive.test.tssrc/plugins/runtime/index.test.tssrc/plugins/commands.test.tsmain, this branch also passed:pnpm checkpnpm buildpnpm plugin-sdk:api:checkHuman Verification
NOT YET!
extensions/msteams/src/plugin-interactions.tsintosrc/plugins/interactive.tsorigin/mainand resolved the resulting conflictsmainsrc/agents/**failuresReview Conversations
Compatibility / Migration
Yes/No) YesYes/No) NoYes/No) NoRisks and Mitigations
extensions/msteams/**, cover the adapter seams with tests, and preserve explanatory comments in the non-obvious parsing/Graph-resolution paths.