msteams: fix DM image delivery + user target routing#18716
msteams: fix DM image delivery + user target routing#18716ktsushilofficial wants to merge 5 commits intoopenclaw:mainfrom
Conversation
Issue 1: target=user: returns 403 - When looking up conversation by user ID, now prefers 'personal' conversation type over stale group/channel references that cause BotNotInConversationRoster Issue 2: Images silently dropped in DM - Added buffer parameter support to outbound adapter and send function - Base64 buffer data now properly converted to inline media URL - Fixes openclaw#18693
Additional Comments (2)
When However, The buffer-to-data-URL conversion should instead parse the base64 data directly into a Prompt To Fix With AIThis is a comment left during a code review.
Path: extensions/msteams/src/send.ts
Line: 116:142
Comment:
**Data URL passed to `loadWebMedia` will throw at runtime**
When `buffer` is provided without `mediaUrl`, this code converts it to a data URL (e.g., `data:image/png;base64,...`) and assigns it to `resolvedMediaUrl`. On line 142, `resolvedMediaUrl` is then passed to `loadWebMedia()`.
However, `loadWebMedia()` (in `src/web/media.ts`) does not handle data URLs — it only supports HTTP/HTTPS URLs and local file paths. A data URL will fall through to the local file path branch and fail on `fs.readFile()` or `assertLocalMediaAllowed()`.
The buffer-to-data-URL conversion should instead parse the base64 data directly into a `Buffer` and skip the `loadWebMedia` call entirely (similar to how the existing personal-chat image path works downstream at lines 202-208, where it converts media back to base64 after loading). Consider decoding the buffer inline and constructing the media object directly rather than round-tripping through `loadWebMedia`.
How can I resolve this? If you propose a fix, please make it concise.
The old code used While the store is capped at 1000 entries and this won't cause performance issues in practice, the Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: extensions/msteams/src/send-context.ts
Line: 91:95
Comment:
**Replaces targeted `findByUserId` with full store scan**
The old code used `store.findByUserId(recipient.id)` which iterated conversations until it found a match. The new code calls `store.list()` to load *all* conversations (up to 1000) and then filters them in-memory.
While the store is capped at 1000 entries and this won't cause performance issues in practice, the `findByUserId` method already performs the same user-matching logic (`aadObjectId` / `id` comparison) in `conversation-store-fs.ts:116-131`. Consider extending the store interface with a method like `findAllByUserId` that returns all matches for a user, rather than bypassing `findByUserId` and re-implementing its filtering logic at the call site. This keeps the search logic encapsulated in the store.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise. |
|
Hi @ktsushilofficial — thanks for the submission. I’m the new Microsoft Teams maintainer for OpenClaw. Please give me a day or two to work through the open Teams backlog. Also, join the Twitter community for daily MS Teams feedback + updates: https://x.com/i/communities/2031170403607974228 |
1 similar comment
|
Hi @ktsushilofficial — thanks for the submission. I’m the new Microsoft Teams maintainer for OpenClaw. Please give me a day or two to work through the open Teams backlog. Also, join the Twitter community for daily MS Teams feedback + updates: https://x.com/i/communities/2031170403607974228 |
Summary
Change Type
Scope
Linked Issue/PR
User-visible / Behavior Changes
Security Impact
Repro + Verification
Environment
Steps
Compatibility / Migration
Failure Recovery
Risks and Mitigations
Greptile Summary
This PR fixes two MSTeams issues: (1)
target=user:routing returning 403 BotNotInConversationRoster errors by preferring "personal" conversation references over stale group/channel ones, and (2) addingbufferparameter support for inline base64 media delivery in DMs.bufferstring parameter through the outbound adapter pipeline, converting base64 data to a data URL format for media delivery.send.tspasses the resultingdata:image/...;base64,...string toloadWebMedia(), which does not handle data URLs (only HTTP URLs and local file paths). This will cause a runtime error whenbufferis provided withoutmediaUrl, meaning the DM image delivery fix will not work as intended.bufferfield is added toChannelOutboundContextbutcreateChannelOutboundContextBase()indeliver.tsnever populates it, so buffer data cannot flow through the standard delivery pipeline.Confidence Score: 2/5
extensions/msteams/src/send.tsneeds the most attention — the buffer-to-loadWebMedia path will throw.src/infra/outbound/deliver.tsalso needs attention to wire buffer throughcreateChannelOutboundContextBase.Last reviewed commit: 0acbbc6
(1/5) You can manually trigger the agent by mentioning @greptileai in a comment!