test(api): real critical-path coverage for push, api-key auth, user-date, play-cleanup (#243)

[thomasluizon]

What

Phase 2b iteration 4 of the #243 prod-readiness campaign — the TESTS cluster (critical-path coverage on auth / billing / notifications). Test-only; no product or contract changes.

Each finding was re-verified against the current code before writing (iterations 1–3 proved the audit severities aren't trustworthy). All four survived:

Subject	Prior state	Now
PushNotificationService	rubber-stamp — never instantiated the SUT; asserted inline LINQ/string/JSON re-implementations, and its P256dh=="fcm" routing was stale vs the service's Transport routing	drives real SendToUserAsync against in-memory SQLite + stub HTTP transport (generated VAPID + receiver EC keys so aes128gcm encryption reaches the transport): no-subs no-op · FCM-not-initialized keeps tokens · Web Push dead-token prune (410/404) · transient 500 kept, no throw · 201 delivery · mixed live+dead prunes only the dead sub
ApiKeyAuthenticationHandler	5 tests, all asserting failure	+ happy path (success ticket + identity/scope claims + MarkUsed + SaveChanges) · revoked-key exclusion via the captured query predicate · pay-gate denial · expired-key rejection · non-agent-path rejection
UserDateService (tz)	rubber-stamp — asserted only NotBe(default)	deterministic localized day-boundary: Pacific/Kiritimati (UTC+14) vs Pacific/Honolulu (UTC−10), 24h apart → east − west ≥ 1
PlayNotificationCleanupService	no test	retention boundaries — Play >30d + Stripe >90d purged, records inside the window survive

Validation

• dotnet build clean; Orbit.Infrastructure.Tests 1502 passed / 0 failed (25 in the four affected classes).

Cross-repo

Paired consumer PR (mobile login-flow orchestrator test): thomasluizon/orbit-ui-mobile#453

Refs thomasluizon/orbit-ui-mobile#243

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #320 (orbit-api)

Scope: PR #320 in thomasluizon/orbit-api — test(api): real critical-path coverage for push, api-key auth, user-date, play-cleanup (#243)
Recommendation: APPROVE

Summary

This is a test-only PR (no src/ files touched) that replaces shallow/duplicated assertions in four Infrastructure test files with tests that exercise the real production code paths: ApiKeyAuthenticationHandler (agent-path gating, expiry, pay-gate, revocation predicate), PushNotificationService (real Web Push crypto against an in-memory SQLite OrbitDbContext + stubbed HTTP transport), a new PlayNotificationCleanupServiceTests (retention-boundary purge for Play/Stripe dedup rows), and a tightened UserDateServiceTests timezone-boundary case. Every new assertion was traced against the actual entity/service source (ApiKey.cs, PushSubscription.cs, OrbitDbContext.cs, ProcessedExternalEvent.cs, PushNotificationService.cs, PlayNotificationCleanupService.cs, ApiKeyAuthenticationHandler.cs) — signatures, property names, retention math, and claim assertions all line up with the real implementation. No Critical or High findings.

Findings

Critical

None.

High

None.

Medium

None.

Low / Info

• tests/Orbit.Infrastructure.Tests/Services/UserDateServiceTests.cs:42 — (eastToday.DayNumber - westToday.DayNumber).Should().BeGreaterThanOrEqualTo(1) is looser than necessary: Pacific/Kiritimati (UTC+14) and Pacific/Honolulu (UTC-10) are exactly 24h apart and neither observes DST, so the difference is deterministically always exactly 1. Should().Be(1) would pin the exact value without weakening the test. Info only — the test already correctly exercises the timezone boundary and would still catch a real regression.
• The SqliteCompatOrbitDbContext nested class is duplicated verbatim in the two new/changed test files (PlayNotificationCleanupServiceTests.cs, PushNotificationServiceTests.cs), matching an already-established pattern duplicated across 5 pre-existing test files. Not a new problem introduced by this diff — pre-existing convention, left as-is per rubric guidance to focus on changed-code novelty.

Subagents

Agent	Verdict
security-reviewer	N/A — gate not met (no src/ files changed; diff is tests/ only)
contract-aligner	N/A — gate not met (no DTO/Controller/schema surface touched)

Validation

Check	Result
Build (dotnet)	N/A — skipped per instructions, handled by separate CI check (Build)
Tests (dotnet)	N/A — skipped per instructions, handled by separate CI check (Unit Tests)

Deferred — N/A dimensions

• Dimensions 8–12, 14 (DESIGN.md/AI-slop, parity, i18n, contract drift, security, FEATURES.md) — N/A, test-only PR with no src/, DTO/Controller, or shared-contract surface changed. Cross-repo dimensions additionally not verifiable since orbit-ui-mobile is not checked out in this job.
• Phase 6 (adversarial skeptic + cross-model second opinion) not run: no Critical/High finding survived to trigger it.

What's good

• Tests now drive real production behavior instead of re-implementing logic locally (e.g., PushNotificationServiceTests now drives SendToUserAsync end-to-end with real VAPID/EC crypto and a stub HttpMessageHandler, instead of duplicating TokenPreview/JSON-serialization snippets).
• Removed domain-level PushSubscription.Create validation tests from the service test file were confirmed still covered in Orbit.Domain.Tests/Entities/PushSubscriptionTests.cs — correct deduplication per the tests/CLAUDE.md layering convention, not a coverage loss.
• New HandlerRun record + RunHandler overload cleanly consolidates previously copy-pasted handler setup across ApiKeyAuthenticationHandlerTests cases.
• Retention-boundary test (PlayNotificationCleanupServiceTests) correctly brackets the 30-day/90-day cutoffs on both sides (29/31 days, 89/91 days) against real ExecuteDeleteAsync SQL via SQLite.
• HandleAuthenticateAsync_QueryPredicateExcludesRevokedKeys captures and compiles the actual FindTrackedAsync predicate expression to verify revoked keys are excluded at the query level, not just in application logic.

Recommendation

No changes required before merge. Approve as-is.