fix(provider-microsoft): populate attachments on getMessage (fixes #55)#56
Merged
stevenobiajulu merged 1 commit intomainfrom Apr 28, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
stevenobiajulu
added a commit
that referenced
this pull request
Apr 28, 2026
…lect (fixes #59) (#60) * fix(provider-microsoft): cast contentId on polymorphic attachment $select (#59) Microsoft Graph rejects `contentId` in `$select` against the polymorphic `microsoft.graph.attachment` base type — the property is declared on `microsoft.graph.fileAttachment`. PR #56 shipped the bare field, so every Outlook `getMessage()` call (read_email, list_attachments, get_thread, reply flows) throws HTTP 400. Source: cast the field — `microsoft.graph.fileAttachment/contentId`. Verified live against real Graph (HTTP 200, contentId populated). Tests: add a tiny inlined schema-validating mock with explicit allow-lists for base attachment props and approved type casts. Convert the three `getMessage`/`getThread` tests that hard-coded the broken URL to use it. PR #56's broken string would now fail unit tests instead of passing. Doc-link comments added to the source constant and the test allow-lists so future readers can verify the schema against the canonical Graph docs. Fixes #59. * chore(provider-microsoft): tighten allow-list and pin Graph doc URLs Codex peer review on #60 surfaced two optional improvements; both applied: - Drop `microsoft.graph.fileAttachment/contentLocation` from ALLOWED_ATTACHMENT_CASTS. The provider never selects it and Graph documents it as unsupported on fileAttachment, so pre-whitelisting weakened the "future casts must be deliberate" guard. - Pin Graph Learn URLs with ?view=graph-rest-1.0 so the doc-link comments stop redirecting (and so the version lined up against the schema is explicit).
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GraphEmailProvider.getMessage()to request Microsoft Graph attachment metadata with$expand=attachments($select=id,name,contentType,size,isInline,contentId)GET /messages/{id}/attachments?$select=...when Graph rejects the nested$selectEmailMessage.attachmentssolist_attachmentsreturns both file and inline attachments instead of[]Root Cause
list_attachmentsreads fromctx.provider.getMessage(message_id), but the Microsoft provider previously fetched only the message record and never populatedEmailMessage.attachments. The core action then defensively coerced that missing field to[].Tests
Scenario: getMessage maps Graph attachment metadata and inline content idsScenario: getMessage falls back to /attachments when expanded query is rejectednpm run test:run -w @usejunior/provider-microsoft -- src/email-graph-provider.test.tsnpm run test:runnpm run buildFixes #55.