Fix follow-up MCP token review feedback#1044
Merged
BenjaminMichaelis merged 3 commits intomainfrom Apr 30, 2026
Merged
Conversation
Agent-Logs-Url: https://github.com/IntelliTect/EssentialCSharp.Web/sessions/1309ce59-f673-4f1a-b124-e849af2f4737 Co-authored-by: BenjaminMichaelis <22186029+BenjaminMichaelis@users.noreply.github.com>
Agent-Logs-Url: https://github.com/IntelliTect/EssentialCSharp.Web/sessions/1309ce59-f673-4f1a-b124-e849af2f4737 Co-authored-by: BenjaminMichaelis <22186029+BenjaminMichaelis@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
BenjaminMichaelis
April 30, 2026 07:26
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Follow-up fixes to MCP token expiry validation and messaging to address prior review feedback, focusing on consistent UTC reference times, shared constants, and deterministic tests around date rollovers.
Changes:
- Capture a single
nowUtc/todayUtcwhen validating expiry bounds in the MCP token API and account-management page. - Derive the max-expiry validation message and UI help text from the shared lifetime constant.
- Make the max-expiry test deterministic across UTC date rollovers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| EssentialCSharp.Web/Services/McpApiTokenService.cs | Derives max-expiry validation message from the shared lifetime constant. |
| EssentialCSharp.Web/Controllers/McpTokenController.cs | Uses a single captured UTC reference time for ExpiresOn min/max validation. |
| EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/McpAccess.cshtml.cs | Uses a single captured UTC reference time for page-model expiry validation and bounds. |
| EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/McpAccess.cshtml | Updates help text to match end-of-day UTC expiry semantics and avoids hard-coded “6 months”. |
| EssentialCSharp.Web.Tests/McpApiTokenServiceTests.cs | Stabilizes max-expiry test across UTC date rollovers and asserts on the shared validation message. |
Comment on lines
+48
to
52
| DateTime nowUtc = DateTime.UtcNow; | ||
| DateOnly todayUtc = DateOnly.FromDateTime(nowUtc); | ||
| InitializeExpiryBounds(nowUtc); | ||
| ApplyDefaultExpiryIfMissing(); | ||
| string? userId = userManager.GetUserId(User); |
Comment on lines
+29
to
33
| DateTime nowUtc = DateTime.UtcNow; | ||
| DateOnly todayUtc = DateOnly.FromDateTime(nowUtc); | ||
| DateTime? expiresAt = null; | ||
| DateOnly maxExpiresOn = McpApiTokenService.GetDefaultExpiryDate(); | ||
| DateOnly maxExpiresOn = McpApiTokenService.GetDefaultExpiryDate(nowUtc); | ||
| if (request?.ExpiresOn is DateOnly expiresOn) |
Agent-Logs-Url: https://github.com/IntelliTect/EssentialCSharp.Web/sessions/bc3c4ad5-6f8e-47fe-a4e4-dd78459a3763 Co-authored-by: BenjaminMichaelis <22186029+BenjaminMichaelis@users.noreply.github.com>
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
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.
Uh oh!
There was an error while loading. Please reload this page.