fix: harden pending message leasing and module bootstrap

[PrzemyslawKlys]

Summary

• make pending message leasing atomic through the repository and surface lease contention as an explicit skip reason
• fix the root module bootstrap to default to packaged binaries, with an opt-in dev path and safer assembly loading
• add import smoke coverage in both Pester and the PowerShell test runner so bootstrap regressions fail fast

Validation

• dotnet test Sources/Mailozaurr.Tests/Mailozaurr.Tests.csproj --nologo
• Invoke-Pester -Path .\Tests\Import-Module.Tests.ps1 -PassThru
• Import-Module .\Mailozaurr.psd1 -Force -ErrorAction Stop

Notes

• IPendingMessageRepository now includes TryAcquireLeaseAsync; custom external implementations will need to add it when compiling against this version.
• Running .\Mailozaurr.Tests.ps1 still reports existing unrelated PowerShell failures in this branch (for example Clear-SmtpConnectionPool, Connect-POP3, several Send-EmailMessage* tests, and Test-SmtpConnection). The new import preflight itself passes and the dedicated import smoke test is green.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[claude]

Claude finished @PrzemyslawKlys's task —— View job

---

PR Code Review

I've completed a comprehensive review of this pull request focusing on code quality, potential bugs, performance, security, and test coverage.

Todo List:

• Read repository setup instructions
• Review core pending message leasing changes
• Analyze bootstrap module changes
• Check test coverage improvements
• Provide comprehensive feedback

---

📋 Review Summary

This PR implements important improvements to pending message processing with atomic leasing and safer module bootstrap. The changes are well-architected with good separation of concerns.

✅ Strengths

Atomic Leasing Implementation

• Thread Safety: FilePendingMessageRepository.cs:342-364 properly uses SemaphoreSlim for atomic lease acquisition
• Race Condition Prevention: The TryAcquireLeaseAsync method prevents multiple processors from handling the same message
• Clear Semantics: Interface contract is well-documented with proper return values (null when lease fails)
• Observability: New LeaseNotAcquired skip reason provides clear visibility into lease contention

Robust Bootstrap Logic

• Safe Fallback: Mailozaurr.psm1:31-34 gracefully falls back from development to packaged binaries
• Error Handling: Comprehensive exception handling during assembly loading with detailed warnings
• Flexible Framework Selection: Smart logic for choosing appropriate .NET framework assemblies

Enhanced Testing

• Import Smoke Test: New Tests/Import-Module.Tests.ps1 ensures module bootstrap regressions fail fast
• Representative Command Validation: Tests verify core functionality is available post-import
• Lease Testing: Comprehensive test coverage for the new atomic leasing functionality

⚠️ Areas of Concern

Performance Considerations

1. Index Rebuilding: FilePendingMessageRepository.cs:48-82 rebuilds entire index from disk on startup - could be slow for large files
2. File Compaction Blocking: Compaction operations may block other operations during large file processing
3. Sequential File Reading: Log parsing happens sequentially which may not scale well

Potential Issues

1. Lease Duration Logic (PendingMessageProcessor.cs:155)

var leaseDuration = processingLeaseDuration > TimeSpan.Zero ? processingLeaseDuration : MinimumLeaseDuration;

• Using MinimumLeaseDuration (1 tick) when processingLeaseDuration is zero creates extremely short leases
• Could lead to lease contention and processing inefficiencies
• Recommendation: Use a reasonable minimum (e.g., 30 seconds) instead of 1 tick

2. Assembly Loading Error Handling (Mailozaurr.psm1:140-154)

} catch {
    Write-Warning "Processing $($Import.Name) Exception: $($_.Exception.Message)"
    $LoaderExceptions = $($_.Exception.LoaderExceptions) | Sort-Object -Unique
    foreach ($E in $LoaderExceptions) {
        Write-Warning "Processing $($Import.Name) LoaderExceptions: $($E.Message)"
    }
    $true
}

• LoaderExceptions property access in generic catch block may fail if exception doesn't have this property
• Recommendation: Check exception type before accessing LoaderExceptions

🔒 Security Assessment

✅ Good Security Practices:

• No hardcoded credentials or sensitive data
• Proper file path validation and directory creation
• Safe assembly loading with appropriate error handling
• Protected password encoding maintained as Base64

📝 Minor Security Notes:

• File operations use appropriate sharing modes (FileShare.ReadWrite)
• Temporary file handling includes cleanup in finally blocks

🧪 Test Coverage Assessment

✅ Good Coverage:

• New atomic leasing functionality thoroughly tested
• Bootstrap failures now caught by import smoke tests
• Observer pattern properly tested with mock implementations

📈 Improvement Opportunities:

• Could benefit from stress testing concurrent lease acquisition
• Edge cases around file corruption during compaction
• Validation of lease duration edge cases

🏗️ Architecture & Design

✅ Excellent Design:

• Clean separation between repository concerns and processing logic
• Observer pattern provides good telemetry hooks
• Interface-based design allows for easy testing and extensibility
• Atomic operations properly isolated within repository implementation

🎯 Recommendations

High Priority:

1. Fix lease duration logic: Use reasonable minimum lease duration instead of 1 tick
2. Improve assembly loading: Add type checking before accessing LoaderExceptions

Medium Priority:

3. Performance optimization: Consider incremental index updates instead of full rebuilds
4. Monitoring: Add metrics for lease contention rates
5. Documentation: Document the breaking change impact for custom repository implementations

Low Priority:

6. Async improvements: Consider parallel processing for file parsing operations
7. Resilience: Add retry logic for transient file I/O failures

🎉 Overall Assessment

This is a well-executed PR that significantly improves the reliability of pending message processing. The atomic leasing implementation is solid, and the bootstrap hardening will prevent many deployment issues. The code quality is high with good error handling and observability.

Verdict: ✅ Approve with minor fixes recommended

The breaking change to IPendingMessageRepository is properly documented and the migration path is clear. The new functionality provides significant value for concurrent processing scenarios.

---

[codecov]

Codecov Report

❌ Patch coverage is 67.00000% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.67%. Comparing base (2384733) to head (abe396a).
⚠️ Report is 3 commits behind head on v2-speedygonzales.

Files with missing lines	Patch %	Lines
...ozaurr.PowerShell/CmdletSendEmailPendingMessage.cs	0.00%	20 Missing ⚠️
...zaurr/SentMessages/FilePendingMessageRepository.cs	68.42%	8 Missing and 4 partials ⚠️
...Mailozaurr/SentMessages/PendingMessageProcessor.cs	96.55%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                  Coverage Diff                  @@
##           v2-speedygonzales     #597      +/-   ##
=====================================================
+ Coverage              45.62%   45.67%   +0.04%     
=====================================================
  Files                    339      339              
  Lines                  22471    22530      +59     
  Branches                3981     3990       +9     
=====================================================
+ Hits                   10253    10291      +38     
- Misses                 10950    10970      +20     
- Partials                1268     1269       +1     

☔ 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.

[claude]

Claude finished @PrzemyslawKlys's task —— View job

---

PR Code Review

I've completed a comprehensive review of this pull request focusing on code quality, potential bugs, performance, security, and test coverage.

Todo List:

• Read repository setup instructions
• Review core pending message leasing changes
• Analyze bootstrap module changes
• Check test coverage improvements
• Provide comprehensive feedback

---

📋 Review Summary

This PR implements important improvements to pending message processing with atomic leasing and safer module bootstrap. The changes are well-architected with good separation of concerns.

✅ Strengths

Atomic Leasing Implementation

• Thread Safety: FilePendingMessageRepository.cs:342-364 properly uses SemaphoreSlim for atomic lease acquisition
• Race Condition Prevention: The TryAcquireLeaseAsync method prevents multiple processors from handling the same message
• Clear Semantics: Interface contract is well-documented with proper return values (null when lease fails)
• Observability: New LeaseNotAcquired skip reason provides clear visibility into lease contention

Robust Bootstrap Logic

• Safe Fallback: Mailozaurr.psm1:31-34 gracefully falls back from development to packaged binaries
• Error Handling: Comprehensive exception handling during assembly loading with detailed warnings
• Flexible Framework Selection: Smart logic for choosing appropriate .NET framework assemblies

Enhanced Testing

• Import Smoke Test: New Tests/Import-Module.Tests.ps1 ensures module bootstrap regressions fail fast
• Representative Command Validation: Tests verify core functionality is available post-import
• Lease Testing: Comprehensive test coverage for the new atomic leasing functionality

⚠️ Areas of Concern

Performance Considerations

1. Index Rebuilding: FilePendingMessageRepository.cs:48-82 rebuilds entire index from disk on startup - could be slow for large files
2. File Compaction Blocking: Compaction operations may block other operations during large file processing
3. Sequential File Reading: Log parsing happens sequentially which may not scale well

Potential Issues

1. Lease Duration Logic (PendingMessageProcessor.cs:155)

var leaseDuration = processingLeaseDuration > TimeSpan.Zero ? processingLeaseDuration : MinimumLeaseDuration;

• Using MinimumLeaseDuration (1 tick) when processingLeaseDuration is zero creates extremely short leases
• Could lead to lease contention and processing inefficiencies
• Recommendation: Use a reasonable minimum (e.g., 30 seconds) instead of 1 tick

2. Assembly Loading Error Handling (Mailozaurr.psm1:140-154)

} catch {
    Write-Warning "Processing $($Import.Name) Exception: $($_.Exception.Message)"
    $LoaderExceptions = $($_.Exception.LoaderExceptions) | Sort-Object -Unique
    foreach ($E in $LoaderExceptions) {
        Write-Warning "Processing $($Import.Name) LoaderExceptions: $($E.Message)"
    }
    $true
}

• LoaderExceptions property access in generic catch block may fail if exception doesn't have this property
• Recommendation: Check exception type before accessing LoaderExceptions

🔒 Security Assessment

✅ Good Security Practices:

• No hardcoded credentials or sensitive data
• Proper file path validation and directory creation
• Safe assembly loading with appropriate error handling
• Protected password encoding maintained as Base64

📝 Minor Security Notes:

• File operations use appropriate sharing modes (FileShare.ReadWrite)
• Temporary file handling includes cleanup in finally blocks

🧪 Test Coverage Assessment

✅ Good Coverage:

• New atomic leasing functionality thoroughly tested
• Bootstrap failures now caught by import smoke tests
• Observer pattern properly tested with mock implementations

📈 Improvement Opportunities:

• Could benefit from stress testing concurrent lease acquisition
• Edge cases around file corruption during compaction
• Validation of lease duration edge cases

🏗️ Architecture & Design

✅ Excellent Design:

• Clean separation between repository concerns and processing logic
• Observer pattern provides good telemetry hooks
• Interface-based design allows for easy testing and extensibility
• Atomic operations properly isolated within repository implementation

🎯 Recommendations

High Priority:

1. Fix lease duration logic: Use reasonable minimum lease duration instead of 1 tick
2. Improve assembly loading: Add type checking before accessing LoaderExceptions

Medium Priority:

3. Performance optimization: Consider incremental index updates instead of full rebuilds
4. Monitoring: Add metrics for lease contention rates
5. Documentation: Document the breaking change impact for custom repository implementations

Low Priority:

6. Async improvements: Consider parallel processing for file parsing operations
7. Resilience: Add retry logic for transient file I/O failures

🎉 Overall Assessment

This is a well-executed PR that significantly improves the reliability of pending message processing. The atomic leasing implementation is solid, and the bootstrap hardening will prevent many deployment issues. The code quality is high with good error handling and observability.

Verdict: ✅ Approve with minor fixes recommended

The breaking change to IPendingMessageRepository is properly documented and the migration path is clear. The new functionality provides significant value for concurrent processing scenarios.

---