fix(api): keep schedule Instances/ScheduledDates consistent + Stripe session idempotency (#403 follow-up)

[thomasluizon]

Why

These are the two review findings from PR #403's post-merge re-review. #403 was squash-merged at an earlier APPROVED state; the review bot then re-reviewed and returned CHANGES_REQUESTED with the two issues below. The fix commit lived on an orphaned branch and was never merged, so the findings are currently live on main. This PR lands them fixed-forward.

[HIGH] Schedule field consistency

GetHabitScheduleQuery returned:

• ScheduledDates computed via the uncapped HabitScheduleService.GetScheduledDates (validator GetHabitScheduleQueryValidator allows a range up to MaxRangeDays = 366), and
• Instances computed via GetInstances, which internally caps the forward horizon to MaxInstanceHorizonDays = 90.

So a validator-legal 91–366-day request returned a full-range ScheduledDates next to a 90-day-truncated Instances on the same DTO — silently inconsistent.

Fix (query-handler level, server-authoritative): the handler now clamps the effective dateTo to dateFrom + MaxInstanceHorizonDays before it flows into both FilterScheduledHabits (which produces parent ScheduledDates) and the ScheduleMapContext (which drives Instances and child ScheduledDates/Instances). Both fields now derive from the same 90-day window, so they cannot diverge. No reliance on frontend clamping. HabitScheduleService.cs is not touched. The validator is left at 366 so 91–366-day requests still succeed (backward-compatible) — they just return consistent, both-capped data.

Regression test added: Handle_RangeExceedingHorizon_BoundsScheduledDatesAndInstancesConsistently proves a 200-day request returns ScheduledDates.Max() == Instances.Max() == Today + MaxInstanceHorizonDays.

[MEDIUM] Stripe session idempotency

CreateCheckoutSessionAsync and CreatePortalSessionAsync lacked a RequestOptions.IdempotencyKey. Now that both are wrapped in the new transient-retry policy (#403), a fail-then-retry could create duplicate sessions. CreateCustomerAsync already sets a deterministic key — mirrored here:

• Checkout: orbit-checkout-{userId}-{priceId}-{referralCouponId ?? "std"} — deterministic per the reviewer's ask; the coupon marker is included so a retry with a different discount body (referral vs promo branch) cannot trigger a Stripe 409 Idempotency conflict.
• Portal: orbit-portal-{customerId} — returnUrl is static config, so no body-variance risk.

Verification

• dotnet build Orbit.slnx — 0 errors
• dotnet test Orbit.slnx — green (Analyzers 7, Domain 516, Application 2895, Infrastructure 2055)
• dotnet ef migrations has-pending-model-changes — false (no model changes)
• src/Orbit.Api/openapi.json — unchanged (validator max untouched; only computed values and internal Stripe options changed — no contract shape change)

Refs thomasluizon/orbit-ui-mobile#243, #403

🤖 Generated with Claude Code

[claude]

Code Review: PR #405

Recommendation: APPROVE

Summary

Small, well-scoped fixed-forward PR addressing two findings from a post-merge re-review of #403: (1) a clamp in GetHabitScheduleQuery.HandleScheduledHabits forcing ScheduledDates and Instances to derive from the same 90-day-capped window, and (2) Stripe RequestOptions.IdempotencyKey on CreateCheckoutSessionAsync/CreatePortalSessionAsync to prevent duplicate-session creation on the transient-retry path added in #403. Both fixes verified against the actual call graph — no prior review threads exist on this PR, nothing to re-flag.

Findings

Critical / High

None.

Medium

Deterministic Stripe idempotency key isn't scoped to a single retry cycle

• src/Orbit.Infrastructure/Services/StripeBillingService.cs:78,95
• orbit-checkout-{userId}-{priceId}-{referralCouponId ?? "std"} and orbit-portal-{customerId} are deterministic across every call, not just across the retries of one logical attempt (StripeRetryPolicy.ExecuteWithRetryAsync, ~200-800ms backoff). Stripe caches an idempotency key's response for its documented TTL (~24h).
• Risk: a user who abandons checkout/portal and repeats the same action (same price + referral state) same-day gets back the identical cached Stripe response instead of a fresh session — if the first session has since expired/been used, the user is silently handed a dead URL. Not exploitable cross-user (userId is always server-derived and part of the key, confirmed via CreateCheckoutCommand.cs:44,61-67 — no raw client-supplied strings at this call site), just a UX-correctness edge case.
• Suggested fix: scope the key to the individual user-initiated attempt (e.g. a short-lived correlation id minted per command invocation) rather than the (userId, priceId, coupon) tuple indefinitely.
• Not blocking — doesn't regress anything #403 was trying to fix, and is a reasonable fast-follow rather than part of this PR's scope.

Low / Info

• Schedule-handler fix verified correct end-to-end: dateTo clamp at GetHabitScheduleQuery.cs:217-218 flows through to both ScheduledDates (HabitScheduleFilters.FilterScheduledHabits) and Instances (ScheduleMapContext.DateTo → GetScheduledDates/GetInstances, parent and children), matching HabitScheduleService.GetInstances's own internal 90-day clamp. Regression test Handle_RangeExceedingHorizon_BoundsScheduledDatesAndInstancesConsistently exercises exactly the prior failure mode. Validator untouched at 366 days (backward compatible).
• No unit test added for the Stripe idempotency-key change, but StripeBillingService has zero pre-existing unit tests (concrete Stripe SDK types, not abstractions) — a pre-existing gap this diff doesn't worsen.
• Security pass: no auth/authorization changes, no secret/credential exposure, no new injection surface (priceId/referralCouponId/customerId are server-resolved, not raw client input). Idempotency-key composition is not a cross-tenant collision risk.

What's good

• Both fixes are root-caused at the source rather than patched at each symptom.
• Regression test asserts the actual prior failure mode (ScheduledDates.Max() == Instances.Max()), not just "doesn't crash."
• Idempotency key on checkout correctly includes the referral-coupon marker to avoid a retry-with-differing-discount 409 conflict.
• Small, single-purpose diff (7 lines of production code across 2 files).

Deferred / not applicable

• Cross-repo dimensions (contract-aligner, packages/shared backward-compat) — not verifiable in this job (orbit-ui-mobile not checked out), but also not triggered: no DTO/record field added, removed, or renamed; openapi.json unchanged per PR body and confirmed by direct diff read.
• Frontend-only dimensions (DESIGN.md/AI-slop, parity, i18n, FEATURES.md) — N/A, no apps/* files touched.

[claude]

Review Complete

Scope: PR #405 in thomasluizon/orbit-api (fix/api-schedule-consistency-stripe-idempotency → main)
Recommendation: APPROVE

Summary

Checked prior review state first. The PR's head commit is aec6e94f2c75d90e92ad0c868f3c7bbbd7a66614 (a main-merge commit; the substantive fix commit is d7ffab6). A prior claude bot review already APPROVED this exact commit (submitted 2026-07-14T17:39:46Z). gh pr view confirms the PR has not moved since — no new commits, no diff to review. This run reaffirms the standing approval.

Recap (unchanged, still valid)

• Fix 1 (schedule consistency): GetHabitScheduleQuery.cs:217-218 clamps dateTo to MaxInstanceHorizonDays before it feeds both FilterScheduledHabits (→ ScheduledDates) and ScheduleMapContext (→ Instances), so both fields now derive from the same capped window. Backed by a regression test (Handle_RangeExceedingHorizon_BoundsScheduledDatesAndInstancesConsistently) asserting ScheduledDates.Max() == Instances.Max() == Today + MaxInstanceHorizonDays.
• Fix 2 (Stripe idempotency): StripeBillingService.cs:78,95 adds deterministic RequestOptions.IdempotencyKey to CreateCheckoutSessionAsync/CreatePortalSessionAsync, closing a duplicate-session gap on the transient-retry path from #403.
• One Medium, non-blocking finding carried over from the prior review (not new, doesn't force NEEDS WORK): the idempotency key is deterministic across any call with the same (userId, priceId, coupon) / customerId, not scoped to a single retry cycle — Stripe's ~24h key-response cache means a same-day repeat checkout/portal attempt could get a cached (possibly stale/expired) session URL instead of a fresh one. Suggested as a fast-follow, not this PR's scope.
• No Critical/High findings. Validator (MaxRangeDays = 366) untouched, openapi.json unchanged, no contract-shape change — contract drift / backward-compat guard not triggered.

Severity	Count
Critical (incl. ⚠️ old-client breaks)	0
High	0
Medium	1 (carried over, non-blocking)
Low / Info	3 (carried over)

Subagents

Agent	Verdict
security-reviewer	No auth/authz change, no new injection surface, server-derived userId/priceId/customerId
contract-aligner	Not triggered — no DTO/route/shared-type change in the diff

Validation

Check	Result
Build / Unit Tests / SonarCloud	Covered by separate required CI checks (not re-run here per instructions)

Deferred — N/A dimensions

• Parity / i18n — frontend-only, N/A (no apps/* changed, orbit-ui-mobile not checked out here — not verifiable in CI)
• DESIGN.md/AI-slop — N/A (no UI files)
• FEATURES.md parity — N/A (pure bugfix, no user-facing feature surface changed)
• Contract drift — checked, none; openapi.json confirmed unchanged

Recommendation

APPROVE. Clean, targeted bugfix PR — no Critical/High findings, diff unchanged since the last approved review at this commit.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud