chore(sonar): burn down S3776/S6964/S1994 smell clusters (#243)

[thomasluizon]

Behavior-preserving SonarCloud smell burn-down (orbit-api, key thomasluizon_orbit-api). Drives three rule clusters to ~0 at source, no suppressions.

Rules fixed

Rule	Count	Fix
csharpsquid:S3776 (cognitive complexity)	13 methods	Extract private helpers / guard clauses so each method drops under the 15 threshold
csharpsquid:S6964 (value-type under-posting)	14 properties	[property: JsonRequired] on non-nullable request-DTO properties, matching the existing ProfileController precedent
csharpsquid:S1994 (loop counter not tested)	14 retry loops	Convert for (var attempt = n; ; n++) infinite loops to while (true) / counter-testing for, incrementing on every loop-back path

S3776 methods refactored

SendCodeCommand (test-account seed helper), CreateHabitCommand (pay-gate helper), BulkSkipHabitsCommand (flexible-skip helper), ApplyOnboardingCommand (transaction body → method), UserStreakService.CalendarFallback (Math.Max + flattened if/else), HandlePlayNotificationCommand (user-resolution + subscription-apply helpers), GetCalendarSyncSuggestionsQuery (per-suggestion filter helper), SyncCleanupService (generic PurgeAsync<T>), ProcessUserChatCommand (response-card builder), AiIntentService (streaming delta/tool-call helpers), ResendEmailService (per-attempt send helper), PushNotificationService x2 (RecordBatchOutcomes, TryDeliverWebPushAsync/ClassifyTransientPushFailure).

Contract safety

S6964 uses [property: JsonRequired] (Sonar's recommended annotation) — the JSON wire shape and field types are unchanged, so no consumer/@orbit/shared change is needed. Only affects strictness for genuinely-always-sent mandatory inputs; append-only contract respected (no field retyped/renamed/removed).

Verification

• dotnet build — 0 errors, 0 new warnings for targeted rules.
• dotnet test — 5284 tests green (Domain 512, Application 2773, Infrastructure 1992, Analyzers 7).
• No ORBIT0001 narration comments; no ORBIT0002 redundant rollbacks; no #pragma/NOSONAR suppressions.

Refs thomasluizon/orbit-ui-mobile#243

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #392 — chore(sonar): burn down S3776/S6964/S1994 smell clusters

Summary: Behavior-preserving SonarCloud smell burn-down — 13 methods de-complexified via private-helper extraction (S3776), 14 non-nullable value-type DTO properties annotated [property: JsonRequired] (S6964), 14 infinite for loops converted to while(true) with explicit trailing attempt++ (S1994). Every extraction/loop-conversion was traced against its pre-refactor logic. No correctness regression found. An independent security-focused pass over the same file set also found no regression.

Findings:

• Critical / High: None.
• Info — JsonRequired strictness is a real (intentional) behavior change for malformed callers, not a pure refactor: fields like ChallengeType Type, Guid BuddyUserId, Guid RecipientId previously silently deserialized to default(T) when omitted; now they 400. Each annotated field was checked against its handler (e.g. UpdatePublicProfileCommand does a full-overwrite with no partial-update semantics, so all four booleans were already logically mandatory) — this reads as a bug fix (silent corruption → clean validation error), not a contract break. No ⚠️ breaks old mobile clients marker warranted; no field found where a shipped client could plausibly omit it.
• Info — confirmed JsonRequired violations surface as clean ASP.NET Core 400 model-binding errors, not unhandled-exception stack traces.

Cross-repo check: NOT VERIFIABLE IN CI — orbit-ui-mobile is not checked out in this job. JsonRequired additions are strictness-only (no field renamed/removed/retyped), so per the PR's own note no packages/shared change should be required, but this could not be independently confirmed against the mobile repo.

Validation: Build/Tests — skipped per CI adaptation (Build / Unit Tests / SonarCloud run as separate required checks). PR body states 5284 tests green.

What's good: every loop conversion correctly preserves its retry-bound exception-filter and increments only on the swallowed-retry path; the WebPush retry loop gained a defense-in-depth bound that's provably never the actual exit path; the streak-calendar-fallback simplification is a correct De Morgan transform; the sync-cleanup purge collapse is a clean DRY reduction of near-identical blocks; no narration comments, no suppressions, no TODOs introduced.

Recommendation: APPROVE. Non-blocking follow-up suggestion: watch 400-rate metrics on the touched endpoints for a few days post-deploy in case any client build omits a newly-required field.