Add Microsoft To Do MCP tools - #64
Conversation
|
User does not have a PR Review subscription. Go to Team management and add this email to the PR Review subscription. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Microsoft To Do support with new list/task Graph handlers, tool registration, expanded OAuth scopes, auth-status changes, mock Graph responses, tests, and README documentation. ChangesMicrosoft To Do Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)test/todo/handlers.test.js┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.48][ERROR]: unable to find a config; path Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds Microsoft To Do integration to the MCP server, introducing tools to list task lists and tasks, along with corresponding tests and configuration updates. The review feedback focuses on improving robustness by adding optional chaining and validation guards to input arguments and API responses, as well as stripping HTML tags from task bodies to ensure clean output.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const { ensureAuthenticated } = require('../auth'); | ||
|
|
||
| async function handleListTodoLists(args = {}) { | ||
| const count = Math.min(args.count || 10, config.MAX_RESULT_COUNT); |
There was a problem hiding this comment.
To prevent potential errors from invalid or negative input values, and to safely handle cases where args might be null, use args?.count and guard the value with Math.max(1, ...).
| const count = Math.min(args.count || 10, config.MAX_RESULT_COUNT); | |
| const count = Math.max(1, Math.min(Number(args?.count) || 10, config.MAX_RESULT_COUNT)); |
| } | ||
| ); | ||
|
|
||
| if (!response.value || response.value.length === 0) { |
There was a problem hiding this comment.
Add optional chaining when checking response to prevent a TypeError if the API call returns an empty or malformed response.
| if (!response.value || response.value.length === 0) { | |
| if (!response?.value || response.value.length === 0) { |
| } | ||
|
|
||
| async function handleListTodoTasks(args = {}) { | ||
| const listId = args.listId; |
There was a problem hiding this comment.
Use optional chaining on args to prevent a TypeError if the function is called with a nullish argument.
| const listId = args.listId; | |
| const listId = args?.listId; |
| }; | ||
| } | ||
|
|
||
| const count = Math.min(args.count || 10, config.MAX_RESULT_COUNT); |
There was a problem hiding this comment.
To prevent potential errors from invalid or negative input values, and to safely handle cases where args might be null, use args?.count and guard the value with Math.max(1, ...).
| const count = Math.min(args.count || 10, config.MAX_RESULT_COUNT); | |
| const count = Math.max(1, Math.min(Number(args?.count) || 10, config.MAX_RESULT_COUNT)); |
| } | ||
| ); | ||
|
|
||
| if (!response.value || response.value.length === 0) { |
There was a problem hiding this comment.
Add optional chaining when checking response to prevent a TypeError if the API call returns an empty or malformed response.
| if (!response.value || response.value.length === 0) { | |
| if (!response?.value || response.value.length === 0) { |
| const due = formatGraphDateTime(task.dueDateTime); | ||
| const completed = formatGraphDateTime(task.completedDateTime); | ||
| const importance = task.importance || 'normal'; | ||
| const body = task.body?.content ? `\nNotes: ${task.body.content}` : ''; |
There was a problem hiding this comment.
Microsoft To Do task bodies can have a contentType of 'html'. If the content is HTML, displaying raw HTML tags to the user or LLM can be messy. Strip HTML tags using a simple regex if the content type is 'html'.
let bodyContent = task.body?.content || '';
if (bodyContent && task.body?.contentType === 'html') {
bodyContent = bodyContent.replace(/<[^>]*>/g, '');
}
const body = bodyContent ? `\nNotes: ${bodyContent}` : '';![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
auth/token-storage.js (1)
99-113: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDelete the cache file when invalidating tokens.
Both failure paths null out
this.tokensand then call_saveTokensToFile(), but_saveTokensToFile()is a no-op whenthis.tokensis falsy. The stale token file stays on disk, so the next auth-status check can reload expired credentials and repeat the same failing refresh path.Suggested fix
- this.tokens = null; // Invalidate tokens on refresh failure - await this._saveTokensToFile(); // Persist invalidation + await this.clearTokens(); // Remove the stale cache file return null; @@ - this.tokens = null; // Invalidate tokens as they are expired and cannot be refreshed - await this._saveTokensToFile(); // Persist invalidation + await this.clearTokens(); // Remove the stale cache file return null;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@auth/token-storage.js` around lines 99 - 113, The token invalidation paths in token-storage.js leave the cache file on disk because _saveTokensToFile() does nothing after this.tokens is set to null. In the refreshAccessToken failure branch and the no refresh_token branch inside the expiration check, delete the persisted token cache explicitly instead of relying on _saveTokensToFile(), using the existing token storage helpers in TokenStorage so stale credentials are not reloaded on the next auth check.
🧹 Nitpick comments (3)
README.md (1)
106-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd blank lines around the Microsoft To Do table for markdownlint compliance.
The table is missing a blank line before the
### Microsoft To Doheading (preceded immediately by the Outlook table row at line 105). This triggers MD058. Insert a blank line before line 106 and ensure line 111 remains blank after the table.| create-rule | Create inbox rule | + ### Microsoft To Do + | Tool | Description | |------|-------------| | list-todo-lists | List Microsoft To Do task lists | | list-todo-tasks | List tasks in a Microsoft To Do task list | + ### OneDrive🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 106 - 111, The Microsoft To Do section in README.md needs markdownlint-compliant spacing. Update the markdown around the Microsoft To Do table so there is a blank line before the `### Microsoft To Do` heading and a blank line after the table, matching the surrounding sections and satisfying MD058.Source: Linters/SAST tools
test/auth/tools.test.js (1)
1-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the “token loaded but unusable” path.
handleCheckAuthStatus()has a separate branch wheregetTokens()returns anaccess_tokenbutgetValidAccessToken()returnsnull, and that should still produce"Not authenticated". This suite currently skips that regression point.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/auth/tools.test.js` around lines 1 - 78, The handleCheckAuthStatus suite is missing coverage for the branch where getTokens() returns a token object but getValidAccessToken() returns null. Add a test in auth/tools.test.js that mocks token-storage so handleCheckAuthStatus() sees an access_token, then fails to obtain a usable token, and assert it still returns “Not authenticated” while only calling getValidAccessToken once.test/todo/handlers.test.js (1)
80-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the tasks
UNAUTHORIZEDbranch too.
handleListTodoTasks()returns a distinct re-auth message when Graph rejects withUNAUTHORIZED, but the tasks suite never asserts that behavior. Since this PR addsTasks.ReadWrite-gated flows, that branch is worth locking down here as well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/todo/handlers.test.js` around lines 80 - 145, The handleListTodoTasks test suite is missing coverage for the Graph UNAUTHORIZED re-auth path, so add a case that makes callGraphAPI reject with an UNAUTHORIZED error and asserts the re-authentication message returned by handleListTodoTasks. Reuse the existing handleListTodoTasks, ensureAuthenticated, and callGraphAPI setup in this describe block, and verify the branch that handles expired/invalid Graph access is locked down alongside the existing empty-state and auth-required tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@auth/oauth-server.js`:
- Line 62: The fallback scope list in oauth-server.js is missing the existing
mail-write and OneDrive permissions, so update the scopes default used by the
oauth server configuration to include Mail.ReadWrite, Files.Read, and
Files.ReadWrite alongside the current scopes. Keep the change in the scopes
initialization that reads process.env[`${envPrefix}SCOPES`] so the default
MS_SCOPES value still matches what config.js expects.
In `@auth/token-storage.js`:
- Line 20: `TokenStorage` is missing several default OAuth scopes from the rest
of the auth flow, so its fallback no longer matches `config.js` and can drop
permissions during `exchangeCodeForTokens()` and `refreshAccessToken()`. Update
the default `scopes` value in `TokenStorage` to include the same full set used
by the auth configuration, specifically restoring the mail-write and OneDrive
scopes so `this.config.scopes` stays aligned when `MS_SCOPES` is unset.
In `@auth/tools.js`:
- Around line 5-7: The test-mode token dependency was removed too early, but
handleAuthenticate() still relies on tokenManager.createTestTokens() when
USE_TEST_MODE is enabled. Restore or preserve the tokenManager dependency in
auth/tools.js until that test-mode branch is fully migrated, and ensure
handleAuthenticate() can still return the mocked auth result without throwing.
In `@todo/index.js`:
- Around line 14-17: The `count` schema in the task-list tool contract is too
permissive and should only allow positive integers. Update the relevant schema
definitions in `todo/index.js` for both occurrences of `count` (including the
other block referenced in the comment) to use `type: 'integer'` with a minimum
of 1 and the existing configured maximum, so the handlers that pass `count` into
Graph’s `$top` only receive valid values.
In `@todo/lists.js`:
- Around line 54-59: The To Do error handling only recognizes the UNAUTHORIZED
case, so Graph 403 permission failures still fall through to the generic
message. Update the response handling in lists.js and the matching path in
tasks.js to also treat 403/missing Tasks.ReadWrite as the re-auth case, or
normalize that status in graph-api.js so the existing UNAUTHORIZED branch covers
it. Use the existing error-mapping flow around the list/tasks handlers to keep
the authorization guidance consistent.
In `@todo/tasks.js`:
- Around line 13-23: The date formatting logic in the
`raw/zone/hasOffset/iso/parsed` block is incorrectly treating Graph wall times
as UTC by appending “Z” to zone-local `dateTime` values. Update the parsing in
this helper so only offset-aware strings are passed through as-is, and for
`dateTimeValue.dateTime` with `dateTimeValue.timeZone` use the supplied zone to
convert before calling `toLocaleString`, preserving the local wall time instead
of shifting it.
---
Outside diff comments:
In `@auth/token-storage.js`:
- Around line 99-113: The token invalidation paths in token-storage.js leave the
cache file on disk because _saveTokensToFile() does nothing after this.tokens is
set to null. In the refreshAccessToken failure branch and the no refresh_token
branch inside the expiration check, delete the persisted token cache explicitly
instead of relying on _saveTokensToFile(), using the existing token storage
helpers in TokenStorage so stale credentials are not reloaded on the next auth
check.
---
Nitpick comments:
In `@README.md`:
- Around line 106-111: The Microsoft To Do section in README.md needs
markdownlint-compliant spacing. Update the markdown around the Microsoft To Do
table so there is a blank line before the `### Microsoft To Do` heading and a
blank line after the table, matching the surrounding sections and satisfying
MD058.
In `@test/auth/tools.test.js`:
- Around line 1-78: The handleCheckAuthStatus suite is missing coverage for the
branch where getTokens() returns a token object but getValidAccessToken()
returns null. Add a test in auth/tools.test.js that mocks token-storage so
handleCheckAuthStatus() sees an access_token, then fails to obtain a usable
token, and assert it still returns “Not authenticated” while only calling
getValidAccessToken once.
In `@test/todo/handlers.test.js`:
- Around line 80-145: The handleListTodoTasks test suite is missing coverage for
the Graph UNAUTHORIZED re-auth path, so add a case that makes callGraphAPI
reject with an UNAUTHORIZED error and asserts the re-authentication message
returned by handleListTodoTasks. Reuse the existing handleListTodoTasks,
ensureAuthenticated, and callGraphAPI setup in this describe block, and verify
the branch that handles expired/invalid Graph access is locked down alongside
the existing empty-state and auth-required tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10558a8d-c4da-411b-8efe-a18bb9f2a544
📒 Files selected for processing (13)
README.mdauth/oauth-server.jsauth/token-storage.jsauth/tools.jsconfig.jsindex.jsoutlook-auth-server.jstest/auth/tools.test.jstest/todo/handlers.test.jstodo/index.jstodo/lists.jstodo/tasks.jsutils/mock-data.js
| clientSecret: process.env[`${envPrefix}CLIENT_SECRET`] || '', | ||
| redirectUri: process.env[`${envPrefix}REDIRECT_URI`] || 'http://localhost:3333/auth/callback', | ||
| scopes: (process.env[`${envPrefix}SCOPES`] || 'offline_access User.Read Mail.Read').split(' '), | ||
| scopes: (process.env[`${envPrefix}SCOPES`] || 'offline_access User.Read Mail.Read Mail.Send Calendars.Read Calendars.ReadWrite Contacts.Read Tasks.ReadWrite').split(' '), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore the existing mail-write and OneDrive scopes here.
This fallback now omits Mail.ReadWrite, Files.Read, and Files.ReadWrite, while config.js Line 23 still expects them. Any install that relies on the default MS_SCOPES value will authenticate successfully but then hit 403s on mail-write and OneDrive tools.
Suggested fix
- scopes: (process.env[`${envPrefix}SCOPES`] || 'offline_access User.Read Mail.Read Mail.Send Calendars.Read Calendars.ReadWrite Contacts.Read Tasks.ReadWrite').split(' '),
+ scopes: (process.env[`${envPrefix}SCOPES`] || 'offline_access User.Read Mail.Read Mail.ReadWrite Mail.Send Calendars.Read Calendars.ReadWrite Files.Read Files.ReadWrite Contacts.Read Tasks.ReadWrite').trim().split(/\s+/),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scopes: (process.env[`${envPrefix}SCOPES`] || 'offline_access User.Read Mail.Read Mail.Send Calendars.Read Calendars.ReadWrite Contacts.Read Tasks.ReadWrite').split(' '), | |
| scopes: (process.env[`${envPrefix}SCOPES`] || 'offline_access User.Read Mail.Read Mail.ReadWrite Mail.Send Calendars.Read Calendars.ReadWrite Files.Read Files.ReadWrite Contacts.Read Tasks.ReadWrite').trim().split(/\s+/), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@auth/oauth-server.js` at line 62, The fallback scope list in oauth-server.js
is missing the existing mail-write and OneDrive permissions, so update the
scopes default used by the oauth server configuration to include Mail.ReadWrite,
Files.Read, and Files.ReadWrite alongside the current scopes. Keep the change in
the scopes initialization that reads process.env[`${envPrefix}SCOPES`] so the
default MS_SCOPES value still matches what config.js expects.
| clientSecret, | ||
| redirectUri: process.env.MS_REDIRECT_URI || 'http://localhost:3333/auth/callback', | ||
| scopes: (process.env.MS_SCOPES || 'offline_access User.Read Mail.Read').split(' '), | ||
| scopes: (process.env.MS_SCOPES || 'offline_access User.Read Mail.Read Mail.Send Calendars.Read Calendars.ReadWrite Contacts.Read Tasks.ReadWrite').split(' '), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep TokenStorage's default scopes aligned with the rest of the auth flow.
This fallback dropped Mail.ReadWrite, Files.Read, and Files.ReadWrite, but both exchangeCodeForTokens() and refreshAccessToken() reuse this.config.scopes. With MS_SCOPES unset, refreshed tokens can lose existing mail-write/OneDrive permissions relative to config.js Line 23.
Suggested fix
- scopes: (process.env.MS_SCOPES || 'offline_access User.Read Mail.Read Mail.Send Calendars.Read Calendars.ReadWrite Contacts.Read Tasks.ReadWrite').split(' '),
+ scopes: (process.env.MS_SCOPES || 'offline_access User.Read Mail.Read Mail.ReadWrite Mail.Send Calendars.Read Calendars.ReadWrite Files.Read Files.ReadWrite Contacts.Read Tasks.ReadWrite').trim().split(/\s+/),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scopes: (process.env.MS_SCOPES || 'offline_access User.Read Mail.Read Mail.Send Calendars.Read Calendars.ReadWrite Contacts.Read Tasks.ReadWrite').split(' '), | |
| scopes: (process.env.MS_SCOPES || 'offline_access User.Read Mail.Read Mail.ReadWrite Mail.Send Calendars.Read Calendars.ReadWrite Files.Read Files.ReadWrite Contacts.Read Tasks.ReadWrite').trim().split(/\s+/), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@auth/token-storage.js` at line 20, `TokenStorage` is missing several default
OAuth scopes from the rest of the auth flow, so its fallback no longer matches
`config.js` and can drop permissions during `exchangeCodeForTokens()` and
`refreshAccessToken()`. Update the default `scopes` value in `TokenStorage` to
include the same full set used by the auth configuration, specifically restoring
the mail-write and OneDrive scopes so `this.config.scopes` stays aligned when
`MS_SCOPES` is unset.
| const TokenStorage = require('./token-storage'); | ||
|
|
||
| const tokenStorage = new TokenStorage(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the test-mode dependency until that branch is migrated.
Lines 5-7 remove tokenManager, but handleAuthenticate() still calls tokenManager.createTestTokens() on Line 33. With USE_TEST_MODE=true, this throws immediately instead of returning the mocked auth result.
Suggested fix
+const tokenManager = require('./token-manager');
const TokenStorage = require('./token-storage');
const tokenStorage = new TokenStorage();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const TokenStorage = require('./token-storage'); | |
| const tokenStorage = new TokenStorage(); | |
| const tokenManager = require('./token-manager'); | |
| const TokenStorage = require('./token-storage'); | |
| const tokenStorage = new TokenStorage(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@auth/tools.js` around lines 5 - 7, The test-mode token dependency was removed
too early, but handleAuthenticate() still relies on
tokenManager.createTestTokens() when USE_TEST_MODE is enabled. Restore or
preserve the tokenManager dependency in auth/tools.js until that test-mode
branch is fully migrated, and ensure handleAuthenticate() can still return the
mocked auth result without throwing.
| count: { | ||
| type: 'number', | ||
| description: 'Number of task lists to retrieve (default: 10, max: 50)' | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Constrain count to positive integers.
Both schemas currently allow values like -1 and 1.5, but the handlers pass count straight into Graph’s $top. Tighten the tool contract to type: 'integer' with a positive minimum and the configured max so clients don’t generate invalid requests.
Also applies to: 33-36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@todo/index.js` around lines 14 - 17, The `count` schema in the task-list tool
contract is too permissive and should only allow positive integers. Update the
relevant schema definitions in `todo/index.js` for both occurrences of `count`
(including the other block referenced in the comment) to use `type: 'integer'`
with a minimum of 1 and the existing configured maximum, so the handlers that
pass `count` into Graph’s `$top` only receive valid values.
| if (error.message === 'UNAUTHORIZED') { | ||
| return { | ||
| content: [{ | ||
| type: 'text', | ||
| text: 'Microsoft To Do access is not authorized for the current token. Re-authenticate after granting Tasks.ReadWrite.' | ||
| }] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For Microsoft Graph To Do endpoints, what HTTP status is returned when the token is valid but missing the required Tasks.ReadWrite permission?
💡 Result:
When a request is made to a Microsoft Graph API endpoint with a valid access token that lacks the required permissions (such as Tasks.ReadWrite), the API returns an HTTP 403 Forbidden status code [1][2][3]. In this scenario, the response body typically includes a JSON object with an error code, such as Authorization_RequestDenied, and a message stating that there are insufficient privileges to complete the operation [4]. While a 403 Forbidden status is the standard response for insufficient permissions, developers should be aware that some scenarios—such as misconfigured tokens, incorrect API endpoints, or issues with token scopes—may occasionally result in a 401 Unauthorized status [5][6][7]. However, when the token is technically valid but simply lacks the specific permission scope required for the requested action, 403 Forbidden is the expected HTTP response [1][5].
Citations:
- 1: https://learn.microsoft.com/en-us/graph/errors
- 2: https://learn.microsoft.com/en-us/powershell/microsoftgraph/troubleshooting?view=graph-powershell-1.0
- 3: https://stackoverflow.com/questions/74340075/ms-graph-api-missing-permissions
- 4: https://learn.microsoft.com/en-us/troubleshoot/entra/entra-id/app-integration/troubleshoot-authorization-requestdenied-graph-api
- 5: https://learn.microsoft.com/en-us/graph/resolve-auth-errors
- 6: https://stackoverflow.com/questions/64268117/microsoft-graph-api-beta-todo-list-api-fails-with-401
- 7: https://stackoverflow.com/questions/45354821/unauthorized-when-fetching-tasks-with-microsoft-graph
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'todo/*' 'utils/*' | sort
printf '\n== outline: todo/lists.js ==\n'
ast-grep outline todo/lists.js --view expanded || true
printf '\n== outline: todo/tasks.js ==\n'
ast-grep outline todo/tasks.js --view expanded || true
printf '\n== outline: utils/graph-api.js ==\n'
ast-grep outline utils/graph-api.js --view expanded || true
printf '\n== relevant snippets ==\n'
sed -n '1,220p' todo/lists.js | cat -n
printf '\n--- todo/tasks.js ---\n'
sed -n '1,220p' todo/tasks.js | cat -n
printf '\n--- utils/graph-api.js ---\n'
sed -n '1,240p' utils/graph-api.js | cat -nRepository: ryaker/outlook-mcp
Length of output: 17207
Handle Graph 403 permission failures here too. Graph returns 403 for missing Tasks.ReadWrite, but utils/graph-api.js only maps 401 to UNAUTHORIZED, so this falls through to the generic error message instead of the re-auth guidance. Handle 403 here (and in todo/tasks.js) or normalize it in utils/graph-api.js.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@todo/lists.js` around lines 54 - 59, The To Do error handling only recognizes
the UNAUTHORIZED case, so Graph 403 permission failures still fall through to
the generic message. Update the response handling in lists.js and the matching
path in tasks.js to also treat 403/missing Tasks.ReadWrite as the re-auth case,
or normalize that status in graph-api.js so the existing UNAUTHORIZED branch
covers it. Use the existing error-mapping flow around the list/tasks handlers to
keep the authorization guidance consistent.
| const raw = dateTimeValue.dateTime; | ||
| const zone = dateTimeValue.timeZone; | ||
| const hasOffset = /[zZ]$|[+\-]\d{2}:\d{2}$/.test(raw); | ||
| const iso = hasOffset ? raw : `${raw}Z`; | ||
| const parsed = new Date(iso); | ||
|
|
||
| if (!Number.isNaN(parsed.getTime())) { | ||
| return parsed.toLocaleString(); | ||
| } | ||
|
|
||
| return zone ? `${raw} (${zone})` : raw; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
In Microsoft Graph dateTimeTimeZoneobjects, does thedateTimefield represent local wall time in the accompanyingtimeZone when no offset is present?
💡 Result:
Yes, in Microsoft Graph dateTimeTimeZone objects, the dateTime field represents the local wall time in the specified timeZone when no offset is provided [1][2][3]. The dateTime property stores a date and time string in a combined format (e.g., YYYY-MM-DDTHH:MM:SS.sssssss) [1]. Because it does not contain offset information (such as 'Z' or a UTC offset like '+05:00'), it is considered a "floating" or "unspecified" time [3][4]. The accompanying timeZone property acts as the context, telling the application how to interpret that wall time [1][5]. When working with these objects, it is important to treat the dateTime value as a local time representation rather than a UTC timestamp [5]. If you interpret it as UTC and attempt to adjust it, you will likely introduce errors [4]. To correctly use this data, you should parse the dateTime string and apply the rules (including Daylight Saving Time adjustments) of the specified timeZone [5]. For developers using the Microsoft Graph SDK for.NET, the DateTimeTimeZone class includes methods and extensions to help handle these conversions, such as ToDateTimeOffset(), which appropriately accounts for the timeZone property to resolve the time to a specific point in time [4].
Citations:
- 1: https://learn.microsoft.com/en-us/graph/api/resources/datetimetimezone?view=graph-rest-1.0
- 2: https://practical365.com/handling-date-values-when-moving-from-ews-to-the-graph/
- 3: https://stackoverflow.com/questions/47088401/create-event-does-not-acknowledge-timezone-passed-in
- 4: DateTimeTimeZone ToDateTime returning incorrect parsed date microsoftgraph/msgraph-sdk-dotnet#2286
- 5: https://stackoverflow.com/questions/63200140/how-to-convert-microsoft-graph-datetimetimezone-to-local-datetime
Don’t treat Graph wall times as UTC. dateTime is the local time in timeZone; appending Z shifts non-UTC values before formatting, so due/completed times can render incorrectly. Parse only offset-aware values, or convert using the supplied zone.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@todo/tasks.js` around lines 13 - 23, The date formatting logic in the
`raw/zone/hasOffset/iso/parsed` block is incorrectly treating Graph wall times
as UTC by appending “Z” to zone-local `dateTime` values. Update the parsing in
this helper so only offset-aware strings are passed through as-is, and for
`dateTimeValue.dateTime` with `dateTimeValue.timeZone` use the supplied zone to
convert before calling `toLocaleString`, preserving the local wall time instead
of shifting it.
1 participant
Summary
Tasks.ReadWritein auth defaults and document the new permissionTesting
Summary by CodeRabbit