fix(msteams): add graphMediaFallback so group-chat file attachments reach the agent#101096
fix(msteams): add graphMediaFallback so group-chat file attachments reach the agent#101096packwood wants to merge 4 commits into
Conversation
openclaw-barnacle
Bot
added
docs
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 11:29 PM ET / 03:29 UTC. Summary PR surface: Source +49, Tests +122, Docs +19, Generated 0. Total +190 across 8 files. Reproducibility: yes. Current main still skips the Graph fallback when Teams-delivered HTML has no extracted attachment IDs, and the PR plus linked issue include live tenant evidence that Graph retains the attachment reference. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land this narrow group-chat opt-in only if maintainers accept the default-off config surface; keep channel team-GUID and thread-reply URL repair with #90738 or combine the paths deliberately. Do we have a high-confidence way to reproduce the issue? Yes. Current main still skips the Graph fallback when Teams-delivered HTML has no extracted attachment IDs, and the PR plus linked issue include live tenant evidence that Graph retains the attachment reference. Is this the best way to solve the issue? Yes as an implementation shape, with a product caveat. The patch stays inside the Microsoft Teams plugin, reuses the existing Graph downloader, keeps default behavior unchanged, and now scopes the opt-in to group chats; the unresolved question is whether this config surface should exist. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d90c9a1ede8e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +49, Tests +122, Docs +19, Generated 0. Total +190 across 8 files. View PR surface stats
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 (5 earlier review cycles)
|
clawsweeper
Bot
added
proof: sufficient
Channel (team-scope) conversations keep the tag-gated fallback: the channel Graph URL builder still has known team-GUID and reply-URL defects tracked with the broader channel attachment repair, so the widened trigger must not route channel traffic into that path. Addresses ClawSweeper review P2 on openclaw#101096. Co-Authored-By: Claude Fable 5 <[email protected]>
|
Addressed the P2 in e3a28d6 (PR body updated to match):
|
…not-channel The prior scope fix excluded channel conversations with a denylist, but personal DMs also arrive as conversationType 'personal' and can carry Graph-compatible 19:[email protected] chat IDs that isBotFrameworkPersonalChatId does not catch — so a documented group-chat option silently changed DM Graph-lookup behavior. Gate on conversationType === 'groupChat' (normalized) instead. Adds a regression: personal + Graph-compatible 19: ID + flag on does not call Graph. Addresses ClawSweeper P1 on openclaw#101096. Co-Authored-By: Claude Fable 5 <[email protected]>
|
Fixed the P1 in 6174d95. The prior scope fix used a denylist (
Remaining item is the maintainer product call on the default-off config surface vs. folding into #90738 — flagged as needing maintainer direction, not an automated edit. |
|
Note on the two red checks (
Per CONTRIBUTING ("do not submit fixes for failures already red on main CI"), I've left them alone. This PR's own surfaces are green: |
…each the agent Teams strips <attachment id> tags from the message HTML it delivers to bots in group chats, so a real file attachment arrives with no reference the plugin can extract and the existing Graph fallback gate never fires (openclaw#89594). The Graph copy of the same message retains both the tag and the reference attachment, so the already-implemented Graph media path works once it is allowed to run. Add channels.msteams.graphMediaFallback (default: false) which, for non-personal conversations that carried a text/html attachment and yielded no media, attempts the Graph message fetch even when no attachment ids were extracted. Default stays off so mention-only messages do not generate spurious Graph lookups (openclaw#58617). Fixes openclaw#89594 Co-Authored-By: Claude Fable 5 <[email protected]>
…aphMediaFallback Co-Authored-By: Claude Fable 5 <[email protected]>
Channel (team-scope) conversations keep the tag-gated fallback: the channel Graph URL builder still has known team-GUID and reply-URL defects tracked with the broader channel attachment repair, so the widened trigger must not route channel traffic into that path. Addresses ClawSweeper review P2 on openclaw#101096. Co-Authored-By: Claude Fable 5 <[email protected]>
…not-channel The prior scope fix excluded channel conversations with a denylist, but personal DMs also arrive as conversationType 'personal' and can carry Graph-compatible 19:[email protected] chat IDs that isBotFrameworkPersonalChatId does not catch — so a documented group-chat option silently changed DM Graph-lookup behavior. Gate on conversationType === 'groupChat' (normalized) instead. Adds a regression: personal + Graph-compatible 19: ID + flag on does not call Graph. Addresses ClawSweeper P1 on openclaw#101096. Co-Authored-By: Claude Fable 5 <[email protected]>
clawsweeper
Bot
added
rating: 🐚 platinum hermit
6174d95 to
3ec3e9a
Compare
|
Update: rebased onto current main (
Both reproduce on sibling PRs (#101271-#101274). Per CONTRIBUTING I'm leaving main-side failures alone. This PR's own surfaces remain green (msteams tests 17/17 + 1000 lane, |
|
Incorporated and superseded by #90738, landed as The landed rewrite preserves this PR's safe default-off Thank you @packwood for the investigation and implementation; your contribution is credited in the landed commit and PR body. |
2 participants
What Problem This Solves
Files attached to Microsoft Teams group chat messages never reach the agent, even with all recommended Graph Application permissions granted (#89594). Teams strips the
<attachment id="…">tags from the message HTML it delivers to bots in group chats, soextractMSTeamsHtmlAttachmentIdsreturns an empty list and the existing Graph media fallback inresolveMSTeamsInboundMedianever fires. The Graph copy of the same message retains both the tag and the fullreferenceattachment (SharePointcontentUrl+ name), so the already-implementeddownloadMSTeamsGraphMediapath works fine once it is allowed to run — the gate is the only blocker.Verified discrepancy on a live tenant (same message,
19:…@thread.v2group chat, plugin 2026.6.11):text/htmlattachment, plugin diagnostics{'htmlAttachments': 1, 'attachmentTags': 0, 'attachmentIds': []}GET /chats/{chatId}/messages/{messageId}: body HTML contains<attachment id="…"></attachment>andattachments: [{ contentType: "reference", contentUrl: "https://…sharepoint.com/…/file.pdf", name: "file.pdf" }]Why This Change Was Made
The gate exists for a good reason — mention-only group messages also arrive as
text/htmlattachments, and keying on contentType alone caused spurious Graph lookups and 404 diagnostics (#58617). So instead of relaxing the gate unconditionally, this adds an explicit opt-in:channels.msteams.graphMediaFallback(default:false) — for group-chat conversations that carried atext/htmlattachment and yielded no media, attempt the Graph message fetch even when no<attachment id>tags were extracted. Channel (team-scope) conversations are excluded and keep the tag-gated behavior: the channel Graph URL builder has separate known team-GUID and reply-URL defects (per ClawSweeper review, tracked with the broader channel repair in #90738), so this flag must not route channel traffic into that path.false: behavior is byte-for-byte unchanged, including the [Bug]: msteams channel file attachments not downloaded — Graph fallback triggers on non-file HTML attachments #58617 protection (covered by existing tests, which still pass unmodified).docs/channels/msteams.md. RequiresChat.Read.AllandFiles.Read.AllApplication permissions with admin consent.a:/8:orgid:) chats are unaffected — they keep the Bot Framework v3 attachments path.User Impact
Operators whose agents live in Teams group chats can enable one config flag and file attachments (PDFs, docs, images uploaded as files) become readable by the agent. Channel (team-scope) attachment behavior is intentionally unchanged — that repair lands separately (#90738). Everyone else sees no change.
Evidence
downloaded attachments {'count': 1}) and the agent answered questions about its content. Before the change, the same message producedgraph fallback never attempted(extraction all-zero) and the agent saw no file.pnpm vitest run extensions/msteams/src/monitor-handler/inbound-media.test.ts— 16 passed (12 existing unmodified + 4 new: enabled-flag group fetch, default-off unchanged, personal-chat exclusion, channel-conversation exclusion)pnpm test:extension msteams— 70 files, 999 tests passedpnpm check— cleanpnpm test:contracts:channels— 34 passedRelates to #89594 — fixes the group-chat variant with live proof. Channel-specific root causes (team GUID, reply-URL shape; #90733 analysis) are intentionally left to the broader channel repair (#90738), so this PR does not close the canonical issue.
🤖 Generated with Claude Code