Carry main into redesign/main: the habit schedule expansion and the suppression cleanup (#746)

CLAUDE.md

@@ -19,7 +19,7 @@ These apply everywhere — they override project-local conventions if they confl
 - Write rules, docs and comments timelessly: no dates except load-bearing data, names or attributions, incident stories, or machine paths. `tools/check-timeless.mjs` enforces this when installed.
 
 - **Suppressions are a closed set.** `tools/suppression-allowlist.json` declares every `#pragma warning disable` and `[SuppressMessage]` under `src/`, per file and per rule with a site count and a reason, and `Guard Conventions` fails an undeclared one. The EF scaffolder's migration pragmas are exempted by a visible directory entry, not by a hidden regex. `node tools/check-suppression-allowlist.mjs --print` reseeds the shape; the reasons are written by hand.
-- **Timezone:** All user-facing dates MUST use `IUserDateService.GetUserTodayAsync(userId)`. `DateTime.UtcNow` is only for `*AtUtc` timestamps, cache keys, and `TimeZoneInfo` conversion into the user's timezone - Roslyn `ORBIT0004` (`src/Orbit.Analyzers`) blocks other raw `DateTime.UtcNow` reads in Api/Application/Infrastructure and blocks `DateOnly.FromDateTime(DateTime.UtcNow...)` in all four projects with no exemption.
+- **Timezone:** All user-facing dates MUST use `IUserDateService.GetUserTodayAsync(userId)`. `DateTime.UtcNow` is only for `*AtUtc` timestamp parameters, fields and properties (including `Add*` arithmetic on the instant and locals used exclusively by those sinks), cache keys, and `TimeZoneInfo` conversion into the user's timezone - Roslyn `ORBIT0004` (`src/Orbit.Analyzers`) blocks other raw `DateTime.UtcNow` reads in Api/Application/Infrastructure and blocks `DateOnly.FromDateTime(DateTime.UtcNow...)` in all four projects with no exemption.
 - **Authorization:** default `[Authorize]`; Roslyn `ORBIT0003` (`src/Orbit.Analyzers`) fails the build on a Controller with no class-level `[Authorize]`/`[AllowAnonymous]` (per-action attributes on every action also satisfy it). Exempt (mark `[AllowAnonymous]`): `/health`, `/api/auth/*`, signature-verified webhooks.
 - **Validation:** Every new feature needs validators in `Orbit.Application/<Feature>/Validators/` AND domain-entity guards in factory/update methods. The backend is the source of truth — never trust the frontend.
 - **Logging levels.** Inject `ILogger<T>`; structured properties in PascalCase, English only (`logger.LogInformation("Action {Property}", value)`). Reserve each level so prod logs carry only signal:
@@ -32,7 +32,7 @@ These apply everywhere — they override project-local conventions if they confl
 - **No dead code.** Delete unused methods, types, parameters, branches the moment they become orphaned by your change.
 - **Narration comments are `ORBIT0001` build errors** (`src/Orbit.Analyzers`; only `///`/`/** */` XML-doc or a URL-linked WHY note survive; autofix via `dotnet format`). The analyzer is silent in local builds but fails CI — grep for bare `//` before pushing.
 - **Redundant transaction rollbacks are `ORBIT0002` build errors** (`src/Orbit.Analyzers`, category Reliability): an explicit `RollbackAsync()`/`Rollback()` on a `using`/`await using`-scoped EF `IDbContextTransaction` is banned — scope disposal already rolls back an uncommitted transaction, so let the using-scope dispose it and keep the `catch` to `ChangeTracker.Clear(); throw;`. A genuinely manually-owned transaction (declared without `using`, or reached via a field/parameter) is left alone. Like ORBIT0001 it is silent in local builds but fails CI (the in-box SDK compiler is older than the analyzer's `Microsoft.CodeAnalysis` 5.6.0, so csc skips it with `CS9057` locally).
-- **Missing authorization attributes (`ORBIT0003`, Security), raw `DateTime.UtcNow` for user-facing dates (`ORBIT0004`, Reliability), and a `DbSet<T>` without a `modelBuilder.Entity<T>(...)` config (`ORBIT0005`, Design) are build errors too** (`src/Orbit.Analyzers`). They are silent in local builds but fail CI; run `dotnet test tests/Orbit.Analyzers.Tests` for local proof. Deliberate UTC-instant/UTC-window sites need targeted `#pragma warning disable ORBIT0004` blocks. **A new suppression needs an allowlist ENTRY, not a comment**: `tools/suppression-allowlist.json` declares each suppression under `src/` by file, rule, site count, and reason.
+- **Missing authorization attributes (`ORBIT0003`, Security), raw `DateTime.UtcNow` for user-facing dates (`ORBIT0004`, Reliability), and a `DbSet<T>` without a `modelBuilder.Entity<T>(...)` config (`ORBIT0005`, Design) are build errors too** (`src/Orbit.Analyzers`). They are silent in local builds but fail CI; run `dotnet test tests/Orbit.Analyzers.Tests` for local proof. The timestamp exemption trusts a callee's `*AtUtc` parameter name and cannot inspect the callee body; that name must represent an instant, never a user-facing date. Other deliberate UTC-instant/UTC-window sites need targeted `#pragma warning disable ORBIT0004` blocks. **A new suppression needs an allowlist ENTRY, not a comment**: `tools/suppression-allowlist.json` declares each suppression under `src/` by file, rule, site count, and reason.
 
 ## Cross-repo parity contract
 

src/Orbit.Analyzers/UtcNowUserFacingDateAnalyzer.cs

@@ -5,6 +5,7 @@
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
 
 namespace Orbit.Analyzers;
 
@@ -16,11 +17,11 @@ public sealed class UtcNowUserFacingDateAnalyzer : DiagnosticAnalyzer
     private static readonly DiagnosticDescriptor InstantRule = new(
         DiagnosticId,
         "Raw DateTime.UtcNow for a user-facing date",
-        "Do not use DateTime.UtcNow here. User-facing dates MUST come from IUserDateService.GetUserTodayAsync(userId); DateTime.UtcNow is only acceptable for *AtUtc timestamps, cache keys, or as input to a TimeZoneInfo conversion into the user's timezone.",
+        "Do not use DateTime.UtcNow here. User-facing dates MUST come from IUserDateService.GetUserTodayAsync(userId); DateTime.UtcNow is only acceptable for *AtUtc timestamp parameters, fields or properties (including Add* arithmetic and locals used exclusively by those sinks), cache keys, or as input to a TimeZoneInfo conversion into the user's timezone.",
         "Reliability",
         DiagnosticSeverity.Error,
         isEnabledByDefault: true,
-        description: "Raw DateTime.UtcNow reads outside timestamp, cache-key, and timezone-conversion contexts compute server-clock dates that are wrong for users in other timezones.");
+        description: "Raw DateTime.UtcNow reads outside timestamp, cache-key, and timezone-conversion contexts compute server-clock dates that are wrong for users in other timezones. Timestamp sinks include *AtUtc parameters, fields and properties, Add* arithmetic on the instant, and locals used only by those sinks. The analyzer trusts a callee's *AtUtc parameter name and cannot inspect the callee body.");
 
     private static readonly DiagnosticDescriptor DateOnlyRule = new(
         DiagnosticId,
@@ -98,10 +99,77 @@ private static void AnalyzeMemberAccess(
             return;
         if (LineNamesTimestampOrCacheKey(memberAccess, context.CancellationToken))
             return;
+        if (FlowsOnlyToUtcTimestamps(memberAccess, context.SemanticModel, context.CancellationToken))
+            return;
 
         context.ReportDiagnostic(Diagnostic.Create(InstantRule, memberAccess.GetLocation()));
     }
 
+    private static bool FlowsOnlyToUtcTimestamps(
+        ExpressionSyntax expression, SemanticModel model, CancellationToken cancellationToken)
+    {
+        expression = UnwrapInstantArithmetic(expression, model, cancellationToken);
+        if (IsUtcTimestampSink(expression, model, cancellationToken))
+            return true;
+        if (expression.Parent is not EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax variable }
+            || model.GetDeclaredSymbol(variable, cancellationToken) is not ILocalSymbol local
+            || variable.FirstAncestorOrSelf<BlockSyntax>() is not { } block)
+            return false;
+
+        var hasUse = false;
+        foreach (var node in block.DescendantNodes())
+        {
+            if (node is not IdentifierNameSyntax reference
+                || !SymbolEqualityComparer.Default.Equals(model.GetSymbolInfo(reference, cancellationToken).Symbol, local))
+                continue;
+
+            hasUse = true;
+            if (!IsUtcTimestampSink(UnwrapInstantArithmetic(reference, model, cancellationToken), model, cancellationToken))
+                return false;
+        }
+        return hasUse;
+    }
+
+    private static ExpressionSyntax UnwrapInstantArithmetic(
+        ExpressionSyntax expression, SemanticModel model, CancellationToken cancellationToken)
+    {
+        while (true)
+        {
+            if (expression.Parent is ParenthesizedExpressionSyntax parenthesized)
+            {
+                expression = parenthesized;
+                continue;
+            }
+            if (expression.Parent is MemberAccessExpressionSyntax { Parent: InvocationExpressionSyntax invocation } access
+                && access.Expression == expression
+                && model.GetSymbolInfo(invocation, cancellationToken).Symbol is IMethodSymbol method
+                && method.ContainingType.SpecialType == SpecialType.System_DateTime
+                && method.ReturnType.SpecialType == SpecialType.System_DateTime
+                && method.Name.StartsWith("Add", StringComparison.Ordinal))
+            {
+                expression = invocation;
+                continue;
+            }
+            return expression;
+        }
+    }
+
+    private static bool IsUtcTimestampSink(
+        ExpressionSyntax expression, SemanticModel model, CancellationToken cancellationToken)
+    {
+        if (expression.Parent is ArgumentSyntax argument
+            && model.GetOperation(argument, cancellationToken) is IArgumentOperation { Parameter: { } parameter })
+            return parameter.Name.EndsWith("AtUtc", StringComparison.OrdinalIgnoreCase);
+
+        if (expression.Parent is AssignmentExpressionSyntax assignment && assignment.Right == expression)
+        {
+            var symbol = model.GetSymbolInfo(assignment.Left, cancellationToken).Symbol;
+            return (symbol is IPropertySymbol || symbol is IFieldSymbol)
+                && symbol.Name.EndsWith("AtUtc", StringComparison.OrdinalIgnoreCase);
+        }
+        return false;
+    }
+
     private static bool IsDateTimeUtcNow(
         MemberAccessExpressionSyntax memberAccess, SemanticModel semanticModel, CancellationToken cancellationToken)
     {

src/Orbit.Api/Authentication/ApiKeyAuthenticationHandler.cs

@@ -39,41 +39,35 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
         var candidates = await apiKeyRepository.FindTrackedAsync(
             k => k.KeyPrefix == keyPrefix && !k.IsRevoked);
 
-#pragma warning disable S3267 // Loops should be simplified with LINQ -- BCrypt.Verify is expensive, early exit via foreach is intentional
-        foreach (var candidate in candidates)
-#pragma warning restore S3267
-        {
-            if (BCrypt.Net.BCrypt.Verify(rawKey, candidate.KeyHash))
-            {
-                if (candidate.HasExpired())
-                    return AuthenticateResult.Fail("API key expired.");
+        var candidate = candidates.FirstOrDefault(candidate => BCrypt.Net.BCrypt.Verify(rawKey, candidate.KeyHash));
+        if (candidate is null)
+            return AuthenticateResult.Fail("Invalid API key.");
 
-                var gateCheck = await payGate.CanReadApiKeys(candidate.UserId, Context.RequestAborted);
-                if (gateCheck.IsFailure)
-                    return AuthenticateResult.Fail("API keys are not available for this plan.");
+        if (candidate.HasExpired())
+            return AuthenticateResult.Fail("API key expired.");
 
-                candidate.MarkUsed();
-                await unitOfWork.SaveChangesAsync();
+        var gateCheck = await payGate.CanReadApiKeys(candidate.UserId, Context.RequestAborted);
+        if (gateCheck.IsFailure)
+            return AuthenticateResult.Fail("API keys are not available for this plan.");
 
-                cache.Remove(ReferenceCacheKeys.ApiKeys(candidate.UserId));
+        candidate.MarkUsed();
+        await unitOfWork.SaveChangesAsync();
 
-                var claims = new List<Claim>
-                {
-                    new(ClaimTypes.NameIdentifier, candidate.UserId.ToString()),
-                    new("auth_method", "api_key"),
-                    new("api_key_id", candidate.Id.ToString()),
-                    new("api_key_read_only", candidate.IsReadOnly.ToString())
-                };
+        cache.Remove(ReferenceCacheKeys.ApiKeys(candidate.UserId));
 
-                claims.AddRange(candidate.Scopes.Select(scope => new Claim("scope", scope)));
-                var identity = new ClaimsIdentity(claims, Scheme.Name);
-                var principal = new ClaimsPrincipal(identity);
-                var ticket = new AuthenticationTicket(principal, Scheme.Name);
-                return AuthenticateResult.Success(ticket);
-            }
-        }
+        var claims = new List<Claim>
+        {
+            new(ClaimTypes.NameIdentifier, candidate.UserId.ToString()),
+            new("auth_method", "api_key"),
+            new("api_key_id", candidate.Id.ToString()),
+            new("api_key_read_only", candidate.IsReadOnly.ToString())
+        };
 
-        return AuthenticateResult.Fail("Invalid API key.");
+        claims.AddRange(candidate.Scopes.Select(scope => new Claim("scope", scope)));
+        var identity = new ClaimsIdentity(claims, Scheme.Name);
+        var principal = new ClaimsPrincipal(identity);
+        var ticket = new AuthenticationTicket(principal, Scheme.Name);
+        return AuthenticateResult.Success(ticket);
     }
 
     private static bool IsAllowedApiKeyPath(PathString path)

src/Orbit.Api/Controllers/ApiKeysController.cs

@@ -7,8 +7,6 @@
 using Orbit.Application.ApiKeys.Commands;
 using Orbit.Application.ApiKeys.Queries;
 
-#pragma warning disable CA1873
-
 namespace Orbit.Api.Controllers;
 
 [Authorize]
@@ -45,7 +43,8 @@ public async Task<IActionResult> CreateApiKey(
 
         return result.ToPayGateAwareResult(value =>
         {
-            LogApiKeyCreated(logger, value.Id, HttpContext.GetUserId());
+            if (logger.IsEnabled(LogLevel.Information))
+                logger.LogApiKeyCreated(value.Id, HttpContext.GetUserId());
             return Created($"/api/api-keys/{value.Id}", value);
         });
     }
@@ -104,15 +103,19 @@ public async Task<IActionResult> RevokeApiKey(Guid id, CancellationToken cancell
 
         if (result.IsSuccess)
         {
-            LogApiKeyRevoked(logger, id, HttpContext.GetUserId());
+            if (logger.IsEnabled(LogLevel.Information))
+                logger.LogApiKeyRevoked(id, HttpContext.GetUserId());
             return NoContent();
         }
         return result.ToErrorResult();
     }
+}
 
+internal static partial class ApiKeysControllerLogging
+{
     [LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "API key created {KeyId} by user {UserId}")]
-    private static partial void LogApiKeyCreated(ILogger logger, Guid keyId, Guid userId);
+    internal static partial void LogApiKeyCreated(this ILogger logger, Guid keyId, Guid userId);
 
     [LoggerMessage(EventId = 2, Level = LogLevel.Information, Message = "API key revoked {KeyId} by user {UserId}")]
-    private static partial void LogApiKeyRevoked(ILogger logger, Guid keyId, Guid userId);
+    internal static partial void LogApiKeyRevoked(this ILogger logger, Guid keyId, Guid userId);
 }