Preserve message id casing in confirmation tokens

[PrzemyslawKlys]

Summary

• preserve exact message-id casing when generating confirmation tokens
• keep message ordering normalized without collapsing case-distinct ids
• add regression coverage for casing-sensitive token generation

Verification

• dotnet build Sources/Mailozaurr.sln -c Debug
• dotnet test Sources/Mailozaurr.Tests/Mailozaurr.Tests.csproj -c Debug --filter "ApplicationMessageActionConfirmationTokensTests|ApplicationRoutingServicesTests|ApplicationMessageActionPreviewServiceTests"

[claude]

Claude finished @PrzemyslawKlys's task —— View job

---

Code Review Complete ✅

I've reviewed this PR that preserves message ID casing in confirmation tokens. Here's my comprehensive feedback:

Todo List:

• Read repository guidelines (CLAUDE.md) - No repo-specific guidelines found
• Review modified MessageActionConfirmationTokens.cs
• Review new test file ApplicationMessageActionConfirmationTokensTests.cs
• Analyze code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage adequacy
• Provide comprehensive feedback

📋 Review Summary

Overall Assessment: This is a high-quality, focused change that addresses a specific casing sensitivity issue with excellent test coverage.

✅ Strengths

Code Quality & Best Practices

• Minimal, surgical change: Only 2 lines modified in MessageActionConfirmationTokens.cs:65-66
• Consistent approach: Both Distinct() and OrderBy() operations now use StringComparer.Ordinal
• Clear intent: The change from OrdinalIgnoreCase to Ordinal explicitly preserves message ID casing
• Good separation of concerns: Logic remains in the same place, no architectural changes needed

Security

• No security concerns: The change actually improves token uniqueness by preserving casing distinctions
• Cryptographic integrity maintained: SHA256 hashing and Base64 encoding remain unchanged
• Input validation preserved: All existing null/whitespace checks remain in place

Performance

• Performance neutral/positive: StringComparer.Ordinal is typically faster than StringComparer.OrdinalIgnoreCase
• No additional allocations: Same memory usage pattern as before

🧪 Test Coverage - Excellent

Comprehensive test scenarios in ApplicationMessageActionConfirmationTokensTests.cs:

1. Casing preservation test (CreateDeleteTokenPreservesMessageIdCasing): Verifies "abc123" and "ABC123" generate different tokens
2. Ordering normalization test (CreateDeleteTokenIgnoresMessageIdOrdering): Confirms ["msg-b", "msg-a"] and ["msg-a", "msg-b"] generate identical tokens

Test quality highlights:

• Uses realistic test data (work-imap, shared@example.com, Inbox)
• Tests both the fix and existing behavior
• Clear, descriptive test names
• Appropriate assertions (NotEqual for casing, Equal for ordering)

⚠️ Minor Considerations

Potential Edge Cases (already well-handled):

• Unicode message IDs: StringComparer.Ordinal handles these correctly with byte-level comparison
• Whitespace handling: Trim() is applied before comparison, maintaining consistency
• Empty/null filtering: Where() clause removes problematic entries before processing

Backward Compatibility:

• This is a breaking change for existing tokens with case-distinct message IDs
• However, this appears to be intentional based on the PR description
• Consider documenting this behavior change if not already covered

🚀 Recommendation

Approve with confidence. This PR demonstrates:

• Clear problem understanding
• Minimal, targeted solution
• Excellent test coverage
• No introduction of bugs or security issues
• Performance-neutral implementation

The change successfully preserves message ID casing while maintaining normalized ordering, exactly as described in the PR summary.

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90c07818cb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Keep token dedupe aligned with preview normalization

Switching token deduplication to StringComparer.Ordinal makes case-variant IDs (for example "msg-1" and "MSG-1") hash as separate entries, but preview generation still normalizes IDs case-insensitively in MailMessageActionPreviewService.NormalizeMessageIds before issuing the token. That means a client can request a preview, then execute the same action payload with the returned token and hit confirmation_token_mismatch because validation now recomputes a different token from the raw request list. This is a regression introduced by the comparer change and breaks preview→execute confirmation flows for inputs that differ only by casing.

Useful? React with 👍 / 👎.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.11%. Comparing base (9dbb44d) to head (90c0781).
⚠️ Report is 1 commits behind head on v2-speedygonzales.

Additional details and impacted files

@@                  Coverage Diff                  @@
##           v2-speedygonzales     #601      +/-   ##
=====================================================
+ Coverage              50.79%   51.11%   +0.32%     
=====================================================
  Files                    473      473              
  Lines                  31170    31170              
  Branches                5359     5359              
=====================================================
+ Hits                   15832    15933     +101     
+ Misses                 13380    13272     -108     
- Partials                1958     1965       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[PrzemyslawKlys]

Addressed the review in acb1e0a by aligning preview-side message-id normalization with the confirmation token comparer. Preview generation now preserves case-distinct ids the same way token validation does, and I added a preview-to-execute regression covering case-variant message ids.

Verification:

• dotnet build Sources/Mailozaurr.sln -c Debug
• dotnet test Sources/Mailozaurr.Tests/Mailozaurr.Tests.csproj -c Debug --no-build --filter "ApplicationMessageActionConfirmationTokensTests|ApplicationMessageActionPreviewServiceTests|ApplicationRoutingServicesTests"

[claude]

Claude finished @PrzemyslawKlys's task —— View job

---

📋 Code Review Complete ✅

I've reviewed this PR that preserves message ID casing in confirmation tokens. Here's my comprehensive feedback:

Todo List:

• Read repository guidelines (CLAUDE.md) - No repo-specific guidelines found
• Review modified MessageActionConfirmationTokens.cs
• Review new test file ApplicationMessageActionConfirmationTokensTests.cs
• Review preview service changes in MailMessageActionPreviewService.cs
• Review updated test files in detail
• Analyze code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage adequacy
• Provide comprehensive feedback

📋 Review Summary

Overall Assessment: This is a high-quality, focused fix that successfully addresses the preview→execute regression while preserving message ID casing. The solution demonstrates excellent understanding of the problem and provides comprehensive test coverage.

✅ Strengths

Code Quality & Best Practices

• Minimal, surgical changes: Only 4 lines modified across core files
• Consistent approach: Both confirmation token generation and preview normalization now use StringComparer.Ordinal
• Clear intent: The change from OrdinalIgnoreCase to Ordinal explicitly preserves message ID casing
• Problem resolution: Successfully addresses the automated review feedback about preview→execute token mismatch

Security

• No security concerns: The change actually improves token uniqueness by preserving casing distinctions
• Cryptographic integrity maintained: SHA256 hashing and Base64 encoding remain unchanged
• Input validation preserved: All existing null/whitespace checks remain in place

Performance

• Performance neutral/positive: StringComparer.Ordinal is typically faster than StringComparer.OrdinalIgnoreCase
• No additional allocations: Same memory usage pattern as before

🧪 Test Coverage - Excellent

Core confirmation token tests in ApplicationMessageActionConfirmationTokensTests.cs:

1. Casing preservation test: Verifies "abc123" and "ABC123" generate different tokens
2. Ordering normalization test: Confirms different orderings generate identical tokens

Integration regression test in ApplicationRoutingServicesTests.cs:

• Preview→execute flow test: RoutedMessageActionServiceAcceptsPreviewGeneratedDeleteConfirmationTokenWithCaseDistinctIds
• Tests the complete flow: preview generation → token validation → action execution
• Uses realistic case-variant IDs: "msg-1", "MSG-1", "msg-2"
• Verifies the token from preview works for actual delete operation

Updated preview tests in ApplicationMessageActionPreviewServiceTests.cs:

• Tests now correctly verify case-distinct IDs are preserved in preview generation
• Test data updated to match new casing behavior

✅ Regression Fix Verification

The latest commit (acb1e0a) successfully addresses the automated review concern:

Problem identified: Preview service was using OrdinalIgnoreCase while tokens used Ordinal, causing mismatches.

Solution implemented:

• MailMessageActionPreviewService.NormalizeMessageIds now uses StringComparer.Ordinal (line 357)
• Both preview generation and token validation now handle casing consistently
• Added regression test proving the fix works end-to-end

⚠️ Minor Considerations

Backward Compatibility:

• This is a breaking change for existing tokens with case-distinct message IDs
• However, this appears intentional and is well-documented in the PR description
• The change improves correctness of the system

Edge Cases (already well-handled):

• Unicode message IDs: StringComparer.Ordinal handles these correctly
• Whitespace handling: Trim() is applied consistently
• Empty/null filtering: Proper validation prevents issues

🚀 Recommendation

Approve with confidence. This PR demonstrates:

• Clear problem understanding and systematic solution
• Excellent test coverage including regression prevention
• No introduction of bugs or security issues
• Performance-neutral implementation with improved correctness
• Proper addressing of automated review feedback

The change successfully preserves message ID casing while maintaining normalized ordering and fixes the preview→execute confirmation flow regression.

---