feat(api): onboarding-before-auth apply endpoint + import-prompt flag

[thomasluizon]

No description provided.

[thomasluizon]

Paired client PR: thomasluizon/orbit-ui-mobile#400

[claude]

Automated review — PR #286 (issue-396)

Ran the /pr-review skill (rubric-driven) against the full diff: ApplyOnboardingCommand/Handler/Validator, DismissImportPromptCommand, User.HasSeenImportPrompt, the additive ProfileResponse.HasSeenImportPrompt field, the EF migration, and all touched tests. security-reviewer and a targeted correctness-verification pass also ran against this diff plus the pre-existing code it calls into (IUnitOfWork, ConcurrencyRetryBehavior, PayGateService, CacheInvalidationHelper).

The code itself has no Critical or High defect. The one High below is an administrative cross-repo tracking item, not a bug in this PR.

Severity	Count
Critical	0
High	1 (FEATURES.md cross-repo tracking — administrative)
Medium	2

High

FEATURES.md parity (cross-repo, not verifiable from this checkout)
This PR adds new user-facing feature surface (pre-auth onboarding apply flow, import-prompt dismiss). Per the cross-repo parity rubric, FEATURES.md lives in orbit-ui-mobile and must be updated (row added, Gating/Platform/Locale columns accurate) in the paired PR (thomasluizon/orbit-ui-mobile#396) before this feature is considered complete. Nothing to fix in orbit-api — flagging so it isn't dropped on the mobile side.

Medium

1. No rate limit on POST /api/profile/onboarding/apply (src/Orbit.Api/Controllers/ProfileController.cs:226). This endpoint does a bulk write (habits + optional log + optional goal) similar in shape to HabitsController.BulkCreate. Note BulkCreate itself also has no [DistributedRateLimit(...)], so this isn't a regression introduced by this PR — it's a pre-existing gap in the bulk-write surface, worth a follow-up but not blocking.

2. Latent NRE in ApplyOnboardingCommandValidator (src/Orbit.Application/Profile/Validators/ApplyOnboardingCommandValidator.cs:48-49). RuleFor(x => x.Habits).NotNull() and RuleFor(x => x.Habits.Count).LessThanOrEqualTo(...) are independent rule chains — if Habits were ever null, evaluating the second rule's expression throws before FluentValidation can report the NotNull failure. Currently unreachable (ProfileController.ApplyOnboarding does request.Habits ?? [], and there's no other callsite), but a one-line .When(x => x.Habits is not null, ...) guard on the count rule would close the trap for good.

Confirmed clean

• [Authorize] correctly present on both new actions; UserId sourced only from the JWT, never the request body.
• Advisory-lock key is a parameterized Guid in ExecuteSqlRawAsync — no injection risk.
• Pro-gate for goals re-derives entitlement server-side (PayGateService) rather than trusting a client flag; silently skips goal creation when not Pro.
• ConcurrencyRetryBehavior re-runs the entire handler on conflict (fresh tracked fetch after ChangeTracker reset) — the "exactly-once" claim holds, verified against ApplyOnboardingConcurrencyTests.
• Cache invalidation placement (post-commit, gated on Applied) matches the existing BulkCreateHabitsCommand/ResetAccountCommand convention.
• Free-plan habit cap counting (ParentHabitId == null) matches PayGateService.CanCreateHabits.
• HasSeenImportPrompt added to ProfileResponse is additive; JSON serialization is name-based so no old-client break.
• No orphaned new ProfileResponse(...) positional callsites missed.
• Comment policy, dead code, CQRS structure, type safety: clean.

Not verifiable in this job

• contract-aligner / packages/shared side of the backward-compat guard — sibling orbit-ui-mobile repo isn't checked out here.
• dotnet build / unit tests — covered by this PR's separate required CI checks (Build / Unit Tests / SonarCloud).

---

Requesting changes solely to land the FEATURES.md tracking item on the paired mobile PR (#396); no code change is required in this repo before merge.

[thomasluizon]

Addressed in 2ba9fe6: guarded the Habits.Count rule with .When(Habits is not null) (clears the SonarCloud reliability finding) and added handler tests for the goal-deadline-in-past, first-log-out-of-range, and invalid-color-scheme branches (lifts new-code coverage past the gate). The FEATURES.md row lands on the paired mobile PR #400 (b5dfebfe). The bulk-write rate-limit gap is pre-existing and left as a follow-up.

[claude]

Review Complete

Scope: PR #286 (thomasluizon/orbit-api, issue-396 → main), focused on the diff since the prior bot review (commit 40da262 → 2ba9fe6)
Recommendation: APPROVE

A prior automated review on this PR (submitted 2026-07-05T05:25:57Z, state CHANGES_REQUESTED) found 0 Critical, 1 High (an administrative FEATURES.md cross-repo tracking item, not a code defect), and 2 Medium findings:

1. Latent NRE in ApplyOnboardingCommandValidator — the Habits.Count rule could throw before the NotNull rule reported, if Habits were ever null.
2. No rate limit on the new bulk-write endpoint (flagged as pre-existing, not a regression).

The new commit (2ba9fe6, "fix(api): guard onboarding validator null-ref + cover apply branches") addresses item 1 directly and adds test coverage:

• src/Orbit.Application/Profile/Validators/ApplyOnboardingCommandValidator.cs:48-49 — added .When(x => x.Habits is not null) to the Habits.Count rule. Verified against the current file: this correctly makes the count-rule a no-op when Habits is null, so FluentValidation only ever evaluates it on a non-null collection — the NRE trap is closed and the SonarCloud reliability finding it caused is resolved.
• tests/Orbit.Application.Tests/Commands/Profile/ApplyOnboardingCommandHandlerTests.cs — three new tests (Apply_GoalDeadlineInPast_ReturnsFailureWithoutSaving, Apply_FirstLogIndexOutOfRange_AppliesWithoutLogging, Apply_InvalidColorScheme_ReturnsFailureWithoutSaving). Cross-checked each against the handler (ApplyOnboardingCommand.cs) and domain code:
  • Deadline check (goalInput.Deadline < today → ErrorMessages.DeadlineInPast, message "Deadline cannot be in the past.") — matches ErrorMessages.cs:109.
  • Out-of-range FirstLog.HabitIndex — handler guards with firstLog.HabitIndex >= 0 && < createdHabits.Count, so Applied=true/LoggedFirstHabit=false is correct for index 5 against 1 created habit.
  • Invalid color scheme — User.SetColorScheme (User.cs:201-208) validates against a fixed ["purple","blue","green","rose","orange","cyan"] list; "magenta" correctly fails.
  • All tests reuse existing fixtures (CreateProUser, SetupUser, Today, Habit(...)) already defined in the file — no new dead helpers, no narration comments, no type-safety or security surface touched.

The pre-existing rate-limit Medium was explicitly left as a deferred follow-up by the author (not a regression from this PR, consistent with the sibling HabitsController.BulkCreate also lacking it) — doesn't block. The FEATURES.md item is administrative/cross-repo and the author's PR comment states it landed in the paired mobile PR #400 — not verifiable from this checkout, nothing to fix in orbit-api.

No new Critical/High findings in this delta. dotnet build/test and SonarCloud coverage are covered by required CI checks (Build / Unit Tests / SonarCloud) run separately in this PR. contract-aligner/mobile-side verification is not possible without the sibling orbit-ui-mobile repo checked out in this job — no DTO/contract surface changed in this delta anyway, only validator + tests.

Files reviewed (delta): src/Orbit.Application/Profile/Validators/ApplyOnboardingCommandValidator.cs, tests/Orbit.Application.Tests/Commands/Profile/ApplyOnboardingCommandHandlerTests.cs.

[sonarqubecloud]

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[claude]

Review Complete

Scope: PR #286 (thomasluizon/orbit-api, issue-396 -> main)
Recommendation: APPROVE

This PR has already been through two rounds of automated review:

1. CHANGES_REQUESTED (commit 40da262): 0 Critical, 1 High (administrative FEATURES.md cross-repo tracking item, not a code defect), 2 Medium (a pre-existing rate-limit gap on bulk-write endpoints, and a latent NRE in ApplyOnboardingCommandValidator if Habits were ever null).
2. APPROVED (commit 2ba9fe6): the NRE was fixed with a .When(x => x.Habits is not null) guard, and three new handler tests were added covering the goal-deadline-in-past, first-log-out-of-range, and invalid-color-scheme branches. The rate-limit gap was left as a documented, non-blocking follow-up (consistent with the sibling HabitsController.BulkCreate also lacking it). The FEATURES.md item was confirmed by the author as landed in the paired mobile PR (thomasluizon/orbit-ui-mobile#400, commit b5dfebfe).

Since that APPROVE, the only new commit is f31eb15 ("Merge branch 'main' into issue-396") - a clean merge with no conflicts. Verified via git diff 2ba9fe6 f31eb15 --stat: the merge brings in exactly 5 files, all belonging to unrelated PR #289 (fingerprint JSON canonicalization - AgentOperationFingerprint.cs, AgentOperationExecutor.cs, WebApplicationExtensions.cs, and their tests). Zero overlap with this PR's file set (ProfileController.cs, ApplyOnboardingCommand.cs, DismissImportPromptCommand.cs, ApplyOnboardingCommandValidator.cs, User.cs, GetProfileQuery.cs, the EF migration, and the touched test files) - confirmed with a targeted git diff scoped to exactly those paths, which returned empty.

A fresh security-focused pass over the full onboarding-apply/dismiss-import-prompt surface (JWT/[Authorize] placement, IDOR via HttpContext.GetUserId() scoping, bulk-write bounds, error handling, privilege level of the import-prompt dismiss) reconfirms PASS - no Critical/High/Medium findings, consistent with the prior two reviews.

Not verifiable in this job

• contract-aligner / packages/shared side of the backward-compat guard - sibling orbit-ui-mobile repo isn't checked out here. No DTO/contract surface changed in the delta since the last APPROVE regardless.
• dotnet build / unit tests / SonarCloud - covered by this PR's separate required CI checks.

No new Critical/High findings since the last APPROVE. No code changes required before merge.