Skip to content

feat(chat): emit goal-list card for capable clients (#301 PR β)#243

Merged
thomasluizon merged 2 commits into
mainfrom
feature/301-generative-ui-cards
Jun 24, 2026
Merged

feat(chat): emit goal-list card for capable clients (#301 PR β)#243
thomasluizon merged 2 commits into
mainfrom
feature/301-generative-ui-cards

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

What

PR β (backend) of the remaining #301 work — extends the #238 generative-UI card pattern to goals. Paired with the frontend PR in orbit-ui-mobile.

Astra emits a [[orbit:goals]] directive when the user asks to see / list / review their goals; the handler detects it, builds a GoalListCard (active goals with progress + deadline) for clients advertising SupportsGoalListCard, and returns it in ChatResponse. The model emits a one-line wrapper instead of a long prose list → fewer output tokens (the pricey, latency-linear side).

  • Backward compatible (append-only): SupportsGoalListCard is a new optional capability flag; old clients omit it → no directive instruction, prose fallback. No break for the Play-store-lagged mobile fleet.
  • Serializes automatically via ChatStreamEvent.Final(ChatResponse) — no manual mapping.

Tests

GoalListCardBuilderTests (directive parse + progress mapping + position order + null deadline). Full suite green (Domain 388 / Application 2054 / Infrastructure 1164).

Refs thomasluizon/orbit-ui-mobile#301

Mirror the #238 habit-list card for goals: Astra emits a [[orbit:goals]] directive when asked to show/list/review goals; the handler detects it, builds a GoalListCard (active goals with progress + deadline) for clients advertising SupportsGoalListCard, and returns it in ChatResponse. Old clients (no capability flag) keep the prose fallback — append-only contract, no breaking change.

Refs thomasluizon/orbit-ui-mobile#301

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thomasluizon

Copy link
Copy Markdown
Owner Author

🔗 Paired frontend PR: thomasluizon/orbit-ui-mobile#303

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean mirror of the #238 habit-list card for goals. The directive extraction, capability-flag gate, prompt injection, and ChatResponse extension all follow the established HabitListCard pattern exactly. context.ActiveGoals is already populated in ChatContext, the SupportsGoalListCard == true guard correctly handles null for old clients, and the additive GoalList field on ChatResponse is a non-breaking change. Test coverage hits the key paths: directive presence/absence, case insensitivity, position ordering, and null deadline. No correctness, security, or contract issues found.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
79.4% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@thomasluizon
thomasluizon merged commit 3581694 into main Jun 24, 2026
7 of 8 checks passed
@thomasluizon
thomasluizon deleted the feature/301-generative-ui-cards branch June 24, 2026 22:56
thomasluizon added a commit that referenced this pull request Jul 10, 2026
…activeCheckin (#243)

AgentAuditRedactor masks sensitive JSON fields (code, token, password, secret,
refresh_token, verifier, authorization, apikey) before an agent-tool argument
body is persisted to AgentAuditLogs. Wired into BOTH the legacy MCP audit path
(WebApplicationExtensions) and the structured agent path (AgentOperationExecutor),
which previously only truncated the raw body despite the "Redacted" naming.
Closes a High data-leakage finding (OTP codes / bearer tokens could land in the
audit trail).

SentProactiveCheckin gains an ON DELETE CASCADE FK to Users (migration, with a
pre-constraint orphan cleanup) and is now explicitly deleted in
AccountResetRepository.DeleteAllUserDataAsync, mirroring every sibling Sent* table.
A user's proactive-checkin history no longer survives account deletion. Closes a
High (security) / Critical (code-quality) orphaned-PII finding.

sonarcloud.yml: least-privilege top-level `permissions: contents: read`
(CodeQL actions/missing-workflow-permissions).

Part of the #243 prod-readiness campaign, security batch. Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 10, 2026
…activeCheckin (#243) (#317)

AgentAuditRedactor masks sensitive JSON fields (code, token, password, secret,
refresh_token, verifier, authorization, apikey) before an agent-tool argument
body is persisted to AgentAuditLogs. Wired into BOTH the legacy MCP audit path
(WebApplicationExtensions) and the structured agent path (AgentOperationExecutor),
which previously only truncated the raw body despite the "Redacted" naming.
Closes a High data-leakage finding (OTP codes / bearer tokens could land in the
audit trail).

SentProactiveCheckin gains an ON DELETE CASCADE FK to Users (migration, with a
pre-constraint orphan cleanup) and is now explicitly deleted in
AccountResetRepository.DeleteAllUserDataAsync, mirroring every sibling Sent* table.
A user's proactive-checkin history no longer survives account deletion. Closes a
High (security) / Critical (code-quality) orphaned-PII finding.

sonarcloud.yml: least-privilege top-level `permissions: contents: read`
(CodeQL actions/missing-workflow-permissions).

Part of the #243 prod-readiness campaign, security batch. Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 11, 2026
…deletion (#318)

* fix(api): idempotent offline-mutation replay + transactional account deletion

Background-durability/reliability batch (#243 Phase 2b iteration 2).

Idempotency (fixes offline-mutation double-apply on a lost network ACK):
- Add ProcessedRequest ledger (unique (UserId, IdempotencyKey) + cascade FK + retention index) and migration.
- IdempotencyBehavior (MediatR): requests carrying an Idempotency-Key header reserve the key and cache the response in the SAME transaction as the mutation, so a crash cannot apply a mutation without its ledger row; a replay returns the stored response; a concurrent duplicate loses the unique race and replays the winner. Registered innermost so ConcurrencyRetry re-runs it with a fresh reservation.
- HttpIdempotencyContext reads the header + user id; IdempotencyStore persists the ledger.
- Make UnitOfWork.ExecuteInTransactionAsync reentrant so a handler that self-transacts joins the ambient transaction instead of nesting (Npgsql forbids nesting).
- 30-day retention cleanup for ProcessedRequests in the daily job; delete on account reset + cascade on hard delete.

Account deletion (fixes partial-delete/orphaned PII on crash):
- Wrap the AccountDeletionService background job's delete+remove+save in ExecuteInTransactionAsync (mirrors ResetAccountCommand); the 29 ExecuteDeleteAsync calls now commit atomically. Correct the IAccountResetRepository docstring (it opens no transaction of its own).

Tests: idempotency round-trip/replay/no-key/rollback/Unit-response (SQLite) + concurrent-race replay/rethrow (mocked) + reentrancy + transactional deletion + retention.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(api): cover HttpIdempotencyContext header/claim extraction

Closes the SonarCloud new-code-coverage gate on #318 — HttpIdempotencyContext
was the only uncovered new file (0/16 lines). Adds unit tests for the header +
NameIdentifier-claim extraction: present/authenticated, trimmed, oversized,
missing header, no HttpContext, missing claim, non-GUID claim.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(api): make idempotency Result-safe, opt-in, and race-isolated (review #318)

Addresses the four defects the PR review found in the idempotency behavior — my
first cut only tested string/Unit responses and missed that Orbit commands return
Result<T>.

- Critical: Result/Result<T> now round-trip via a dedicated JSON converter that
  reads Value only on success and rebuilds through the factory methods. Before,
  serializing a Result<T> failure threw (Value getter) and deserializing the
  non-generic Result threw (protected ctor) — an unhandled 500 on the flagship
  LogHabitCommand failure paths. Covered by success + failure + non-generic tests.
- Critical: flush the reservation in its own SaveChanges before the handler runs,
  so a ledger unique-violation is isolated from the handler's own constraints and
  a concurrent duplicate loses the race before executing the handler (can no
  longer be misrouted into a handler's already-exists recovery on an aborted tx).
- High: scope the ledger by request type — key is (UserId, IdempotencyKey,
  RequestType) — so one key reused across two commands can't return the wrong
  command's cached response. Column + composite unique index (migration).
- High: idempotency is now opt-in via IIdempotentCommand, applied only to the
  non-idempotent offline commands (create habit/goal/tag, logHabit, skipHabit,
  updateGoalProgress). Secret-returning commands (e.g. CreateApiKey) are never
  marked, so a one-time secret can never land in the plaintext ledger.

Full suite green (Application 2615, Domain 503, Infrastructure 1507).

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 11, 2026
…ate, play-cleanup (#243) (#320)

Phase 2b iteration 4 (TESTS cluster) — replace rubber-stamp coverage with
behavior/edge/failure tests exercising the real SUTs.

- PushNotificationServiceTests: was a tautology suite that never instantiated
  the service (asserted inline LINQ/string/JSON re-implementations, and its
  P256dh routing was stale vs the service's Transport routing). Rewritten to
  drive SendToUserAsync against in-memory SQLite + a stub HTTP transport with
  generated VAPID/receiver EC keys: no-subs no-op, FCM-not-initialized keeps
  tokens, Web Push dead-token prune (410/404), transient 500 (kept, no throw),
  201 delivery, and mixed live+dead pruning only the dead subscription.
- ApiKeyAuthenticationHandlerTests: previously all-failure. Added the happy
  path (success ticket + identity/scope claims + MarkUsed + SaveChanges),
  revoked-key exclusion via the captured query predicate, pay-gate denial,
  expired-key rejection, and non-agent-path rejection.
- UserDateServiceTests: the tz test only asserted NotBe(default). Replaced with
  a deterministic localized-day-boundary assertion (Pacific/Kiritimati vs
  Pacific/Honolulu, 24h apart).
- PlayNotificationCleanupServiceTests (new): retention boundaries — Play >30d
  and Stripe >90d purged, records inside the window survive.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
Skip operations skipped the overdue-window bound that the complete/log
paths enforce, so a habit occurrence older than DefaultOverdueWindowDays
could still be skipped. Add the same BeyondOverdueWindow guard to
SkipHabitCommand.ValidateSkipTarget and BulkSkipHabitsCommand.ProcessSkipItem,
positioned right after the future-date check to mirror LogHabitCommand /
BulkLogHabitsCommand ordering exactly.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
The catch (DbUpdateConcurrencyException) block in RefreshSessionAsync
returned without resetting the scoped DbContext's change tracker, leaving
the session.Rotate(...)-mutated UserSession stuck as EntityState.Modified
with a stale xmin. AuthController.RefreshOperation unconditionally writes
an AgentAuditLog on the same scoped context right after, so EF re-attempted
the stale UserSession UPDATE and threw a second, uncaught concurrency
exception (HTTP 409 instead of INVALID_SESSION, and the audit row dropped).

Add unitOfWork.DiscardChanges() before returning, matching the house
convention (ConcurrencyRetry.ExecuteAsync). Add a regression test that
routes a subsequent save through the real AgentAuditService audit path on
the same context and asserts no residual Modified UserSession remains.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
Address PR review: HabitInvariants.ValidateDateOptions kept the same
`dueDate ?? DateOnly.FromDateTime(DateTime.UtcNow)` landmine one call away from
Habit.Create/Update — in Orbit.Domain, the layer the csharp-tz hook can't reach.
Both call sites already pass a non-null date, so make the parameter a required
non-null DateOnly and delete the fallback, completing the "no silent UTC-today in
the domain" guarantee.

Also delete Create_NullDueDate_DefaultsToToday: with a required DueDate the
null-default path is a compile-time impossibility, so the test only exercised the
test helper's own fallback. It is superseded by
Create_UsesProvidedDueDateVerbatim_WithoutUtcSubstitution.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
… test gaps (#243) (#329)

* fix(api): stop concurrent refresh-token double-rotation + close three test gaps (#243)

UserSession lacked the optimistic-concurrency token that User/Goal/Referral carry,
so two parallel RefreshSessionAsync calls with the same token both rotated the row
(last write wins) — a refresh-token reuse race. Map the Postgres xmin token on
UserSession and translate the losing writer's DbUpdateConcurrencyException into a
clean INVALID_SESSION failure so exactly one refresh rotates and the loser is
rejected (no retry, no double-rotation). The token is a system column, so the
migration emits no DDL; RefreshSessionCommand is not IConcurrencyRetryable, so the
conflict is resolved in-service rather than surfacing as a 409.

Intelligent unit tests for three coverage gaps:
- ValidationExceptionHandler: focused test for the 400 JSON envelope shape,
  application/json content-type, grouped-per-property errors, and request-id header.
- XpAwardLogBackfillHostedService: startup-delay deferral, idempotency skip when the
  flag is set, clean-run flag write + completion log, and failure-path logging.
- AuthSessionService concurrent refresh: exactly-one-wins with the loser getting
  INVALID_SESSION and the row rotated once (conflict injected via save interceptor,
  since the in-memory provider does not enforce xmin).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(api): reset change tracker after refresh concurrency conflict (#243)

The catch (DbUpdateConcurrencyException) block in RefreshSessionAsync
returned without resetting the scoped DbContext's change tracker, leaving
the session.Rotate(...)-mutated UserSession stuck as EntityState.Modified
with a stale xmin. AuthController.RefreshOperation unconditionally writes
an AgentAuditLog on the same scoped context right after, so EF re-attempted
the stale UserSession UPDATE and threw a second, uncaught concurrency
exception (HTTP 409 instead of INVALID_SESSION, and the audit row dropped).

Add unitOfWork.DiscardChanges() before returning, matching the house
convention (ConcurrencyRetry.ExecuteAsync). Add a regression test that
routes a subsequent save through the real AgentAuditService audit path on
the same context and asserts no residual Modified UserSession remains.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
… (#330)

* fix(api): derive habit DueDate from the user's timezone, not UTC (#243)

Habit.Create() defaulted a null DueDate to DateOnly.FromDateTime(DateTime.UtcNow),
yielding the wrong "today" for any user whose timezone differs from UTC and
violating the hard timezone rule. The domain factory can't inject
IUserDateService, so DueDate is now a required non-null input pushed in from the
Application layer and the UTC fallback is deleted; a future caller can no longer
silently reintroduce the bug (the type makes null a compile error).

Every Habit.Create caller already resolved the user's today via
IUserDateService.GetUserTodayAsync and now passes it as the required 5th
positional argument. Behavior is identical for UTC users and corrected for
everyone else.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(api): drop the sibling UTC fallback in ValidateDateOptions (#243)

Address PR review: HabitInvariants.ValidateDateOptions kept the same
`dueDate ?? DateOnly.FromDateTime(DateTime.UtcNow)` landmine one call away from
Habit.Create/Update — in Orbit.Domain, the layer the csharp-tz hook can't reach.
Both call sites already pass a non-null date, so make the parameter a required
non-null DateOnly and delete the fallback, completing the "no silent UTC-today in
the domain" guarantee.

Also delete Create_NullDueDate_DefaultsToToday: with a required DueDate the
null-default path is a compile-time impossibility, so the test only exercised the
test helper's own fallback. It is superseded by
Create_UsesProvidedDueDateVerbatim_WithoutUtcSubstitution.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
Lifts new-code coverage over the SonarCloud gate by exercising the
IsEnabled-guarded log lines that existing NullLogger tests skip. Each new
case asserts the recipient email never reaches the rendered marketing log
line (success, retry/rate-limit, send exception, and both test-account skip
paths), completing the PII contract across every ResendEmailService log site.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
…all concurrency (#243)

- Rate limit: TryResolveRefreshTokenPartitionKey now accepts a token only if it
  matches the exact server-issued shape (RefreshTokenRules.IsWellFormed, the
  single source of format truth); a malformed body (the trivial vary-per-request
  bypass) falls back to IP partitioning so the request is still throttled per IP
  and unvalidated input no longer reaches the rate-limit DB round-trip.
- RevokeAllSessionsAsync now catches DbUpdateConcurrencyException (DiscardChanges +
  INVALID_SESSION), matching RefreshSessionAsync, so a concurrent refresh during
  logout-all fails gracefully instead of 500.
- Tests: malformed/lowercase/wrong-length tokens fall back to IP; revoke-all
  concurrency conflict returns a controlled failure without throwing.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
) (#331)

* fix(api): strip email PII from remaining email/marketing log sites (#243)

Removes recipient email addresses and raw provider response bodies from the
log templates that merged PR #325 left untouched, keeping only non-PII
diagnostics (subject, HTTP status, a user/correlation id, error):

- VerifyCodeCommand: LogWelcomeEmailFailed logs {UserId} instead of {Email}.
- ResendEmailService: LogEmailSent/LogEmailFailed/LogEmailSendException/
  LogSkippingTestEmail/LogMarketingRetry drop the recipient {To}; LogEmailFailed
  also drops the raw response {Body} (which can echo submitted PII) and keeps the
  HTTP status. The two now-orphaned response-body reads are removed.
- SendMarketingBroadcastCommand: LogPreviewSent logs {Subject} instead of the
  {TestEmail}; LogBroadcastQueued drops the redundant {QueuedAtUtc} timestamp
  (the logging framework already stamps each entry, and it tripped the tz rule).
- AuthController: the Information-level LogVerificationCodeSent drops {Email}
  (RequestId already correlates) and LogUserLoggedInViaCode logs {UserId}.

Adds capturing-logger tests asserting the email address and response body never
reach the rendered log line while status/subject/user id survive.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(api): cover marketing/skip/exception log paths for PII scrub (#243)

Lifts new-code coverage over the SonarCloud gate by exercising the
IsEnabled-guarded log lines that existing NullLogger tests skip. Each new
case asserts the recipient email never reaches the rendered marketing log
line (success, retry/rate-limit, send exception, and both test-account skip
paths), completing the PII contract across every ResendEmailService log site.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
…ts (#243) (#337)

The BeforeSend scrubber cleared the request body, headers and cookies plus
top-level user PII, but never touched the response context (its body, cookies
and headers), the request query string, the user Other dictionary, or
sensitive keys in the Extra bag. A captured event could therefore still ship
a response body or PII-keyed extra to Sentry.

Extract the scrubber into SentryEventScrubber and extend it to strip the
response context, the request query string and user.Other, and to redact
Extra entries whose keys match known-PII fragments (benign debug context is
preserved). Add focused unit tests for each vector plus the no-op path.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
…very (#243) (#341)

Push notification title/body flowed into both the FCM batch/single-message
payloads and the Web Push JSON payload with no sanitization or length limit.
Strip control characters and truncate to a whole-rune UTF-8 byte budget
(title 256B, body 768B) at the SendToUserAsync boundary so neither the FCM
4KB message limit nor the Web Push 4096-octet ceiling (RFC 8291/8030) is
exceeded, even after non-ASCII JSON \uXXXX escaping. The #327 web-push retry
and FCM batch/fallback logic are untouched.

Tests: control chars stripped, overlong content truncated to the byte budget,
multi-byte runes never split at the boundary, and a 20KB hostile body still
delivers an on-wire payload under 4096 bytes.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
…ncy TOCTOU (#243) (#333)

Mirror the Play-webhook hardening from #327 onto the Stripe webhook handler:

- Mark HandleWebhookCommand as IConcurrencyRetryable so a DbUpdateConcurrencyException
  (stale xmin token when two deliveries touch the same user) re-runs the handler via
  ConcurrencyRetryBehavior instead of surfacing as an unhandled 500.
- Stop the generic catch from swallowing DbUpdateConcurrencyException as a processing
  failure; it must propagate to the retry behavior.
- Drop the check-then-insert duplicate detection (a TOCTOU race two concurrent
  deliveries both pass) and rely on the ProcessedStripeEvent.EventId unique constraint,
  caught idempotently by the existing DbUniqueViolation handler.

Tests: concurrent delivery retried-then-succeeds through the behavior, concurrency
conflict propagates instead of being swallowed, duplicate event is idempotent via the
constraint, and the pre-check is gone.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
Transactional email (ResendEmailService), OAuth token refresh
(GoogleTokenService), waitlist contact add (ResendContactsService), and
signed-URL generation (SupabaseObjectStorageService) previously issued a
single outbound HTTP request with no retry, so a transient blip (network
error, timeout, 429, or 5xx) failed the whole operation.

These paths now go through a shared HttpRetryPolicy: up to 2 retries with
~300ms exponential backoff (300ms, 600ms), retrying ONLY transient
outcomes (408/429/5xx and HttpRequestException/timeout). Permanent
responses (2xx, other 4xx such as a revoked Google refresh token or a
duplicate-contact 409) and user cancellation are never retried. Matches
the PushNotificationService pattern from #327.

Stripe SDK calls now retry transient failures via
StripeConfiguration.MaxNetworkRetries = 2 — the SDK's own network-retry,
which auto-attaches an idempotency key to retried writes, so
non-idempotent operations stay safe (a hand-rolled loop could not).

Tests: transient-then-success retries once and succeeds; a transport
exception then success retries; persistent 5xx gives up after the retry
budget; permanent 4xx / revoked-token / conflict are not retried.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
…aches (#243) (#353)

Goal mutation commands never cleared any AI cache, so goal-review (and
goal-influenced summaries) stayed stale for up to an hour after a goal edit.
Tag/ChecklistTemplate/UserFact/ApiKey reads are frequently-hit reference data
that was uncached and, once cached, had no invalidation.

- Add CacheInvalidationHelper.InvalidateGoalReviewCache and fold it into
  InvalidateUserAiCaches, so every habit AND goal mutation refreshes the
  goal-review cache (it derives from goals + linked-habit logs).
- Call InvalidateUserAiCaches from all 8 Goal mutation commands, mirroring
  the habit commands.
- Add a per-user IMemoryCache read layer to GetTags/GetChecklistTemplates/
  GetUserFacts/GetApiKeys via centralized ReferenceCacheKeys, invalidated on
  every write path: the mutation commands, AI fact extraction (batch poller),
  and API-key usage (auth handler MarkUsed).

Behavior-preserving: reads are fresh immediately after any mutation; a short
backstop TTL bounds staleness from any future unhandled write path.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
…#243)

Review caught a real regression from batching: NotifyActivatedAsync sent
pushes after the single batched SaveChanges committed all freezes, but a
throw mid-loop aborted the remaining sends. Because the freeze rows are
already persisted, StageFreeze's freeze-exists guard skips those users on
every future tick, so their "streak protected" notification was lost
permanently.

Wrap each SendToUserAsync in a shared NotifyFreezeActivatedAsync helper
(log Warning + continue), used by both the batch and per-user fallback
paths, mirroring DrainPushQueueAsync. Adds a regression test asserting a
mid-loop push throw still notifies the other staged users and persists all
freezes.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
…ing (#243) (#357)

Four independent ops/perf hardening gaps, all behavior-preserving:

- Enable Brotli/Gzip response compression for JSON responses
  (EnableForHttps for Render's TLS-terminating proxy; Fastest level).
- Add a database connectivity health check to /health so a DB outage
  surfaces as 503 instead of a superficially-live process.
- Bound UnitOfWork.ExecuteInTransactionAsync with a wall-clock timeout
  (TransactionTimeoutSeconds, default 120s) so a wedged transaction
  rolls back and releases its backend instead of leaking it.
- Log a warning for any DB command slower than SlowQueryThreshold
  (default 500ms) via an EF command interceptor, making slow queries
  observable in Render logs without EF's per-command Info logging.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 12, 2026
…ency (#355)

* perf(api): parallelize/batch background fan-out to cut sequential latency (#243)

Three background/command paths awaited slow per-item work sequentially.
Behavior is preserved; only the fan-out is now concurrent or batched, with
per-item error isolation intact.

- CreateChallengeCommand: invite pushes now fan out via a throttled
  Task.WhenAll (SemaphoreSlim cap 5), each send on its own DI child scope
  so PushNotificationService gets an isolated DbContext (EF DbContext is
  not thread-safe; matches the #301 Astra-tools isolation pattern).
- ReminderSchedulerService: record-keeping stays sequential on the single
  request-scoped DbContext (its in-memory dedup + ordering are unchanged),
  and the network sends are deferred into a bounded worker pool, each
  worker draining a shared queue on its own isolated scope. Push failures
  are now logged per recipient instead of aborting the tick.
- StreakFreezeAutoActivationService: eligible users are staged and saved
  in one batched SaveChanges; on a unique/concurrency conflict it falls
  back to the original per-user saves, preserving the failure-isolation
  contract the existing tests lock in.

Tests: multi-recipient fan-out + per-item push failure isolation for the
reminder scheduler; batched single-SaveChanges for streak-freeze; throttled
fan-out delivering each invitee exactly once for challenges.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(api): isolate per-user push failures in streak-freeze notify loop (#243)

Review caught a real regression from batching: NotifyActivatedAsync sent
pushes after the single batched SaveChanges committed all freezes, but a
throw mid-loop aborted the remaining sends. Because the freeze rows are
already persisted, StageFreeze's freeze-exists guard skips those users on
every future tick, so their "streak protected" notification was lost
permanently.

Wrap each SendToUserAsync in a shared NotifyFreezeActivatedAsync helper
(log Warning + continue), used by both the batch and per-user fallback
paths, mirroring DrainPushQueueAsync. Adds a regression test asserting a
mid-loop push throw still notifies the other staged users and persists all
freezes.

Refs thomasluizon/orbit-ui-mobile#243

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…ing AI exception detail (#243) (#362)

Reclassifies routine per-op progress and idempotency-guard log lines in the
background services from Information to Debug so production logs carry only
low-frequency business/lifecycle signal, per the CLAUDE.md logging-level policy:

- AccountDeletionService: "Processing N scheduled deletions" -> Debug
- ResendEmailService: "Skipping test email" / "Email sent" -> Debug
- StreakFreezeAutoActivationService: freeze-already-activated + concurrent-skip
  idempotency guards -> Debug (genuine "freeze activated" stays Information)
- HabitDueDateAdvancementService: "Advanced DueDate for N bad habits" -> Debug
- OAuthController: "OAuth API key created" -> Debug
- ProactiveCheckinSchedulerService: "already recorded; skipping push" -> Debug

AiRetryLoggingPolicy no longer logs the raw exception message (which can carry
implementation/network detail); it keeps the safe classification already present
(exception type name / HTTP status + retriable flag). Adds a focused test
asserting the safe format is logged without the raw exception message.

Behavior-preserving observability change. Full suite green (4972 tests).

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
Adds validators for 31 read-model queries flagged by the prod-readiness
audit as lacking a validator. Each guards UserId (non-empty Guid) and,
where present, the entity id, date range (from<=to, <=366 days), Period,
Language, and public-profile Slug — mirroring the existing query-validator
pattern (GetHabitSchedule/GetAllHabitLogs/GetDailySummary). Purely
additive and behavior-preserving: valid requests are unchanged; malformed
input is now rejected at the trust boundary via the existing
ValidationBehavior pipeline instead of failing deeper in the handler.

Representative validator tests cover every added file (UserId/entity-id/
slug guards) plus the range/period/language logic on GetRetrospective and
GetGoalReview.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…n with tests (#243) (#366)

- Collapse empty private constructors to one line in DistributedRateLimitBucket,
  PendingClarification and PendingAgentOperationState, matching Habit/User style.
- Add regression tests proving each HabitLog read excludes logs for habits the
  requesting user does not own. The streak-history, daily-summary bad-habit-slip
  and duplicate-habit completion reads already scope through an owned-habit-id
  join; GetHabitById/GetHabitFullDetail gate a foreign HabitId at the ownership
  pre-check before any log query runs. Tests capture and evaluate the actual read
  predicate so removing the ownership scope would fail them.
- Challenge progress reads are cross-participant by design (shared CoopGoal /
  StreakTogether progress spans every participant's linked habit); tests lock in
  that scope (every linked habit counts, an unlinked habit is excluded) instead
  of a per-user filter that would regress shared progress.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…243) (#370)

Close the high-severity mock-level gaps in the two AuthSessionServiceTests:

- Rotation atomicity: assert the persisted TokenHash is deterministically
  the SHA-256 of the new refresh token handed to the client (not just
  "!= original"), in both the Infrastructure and Application rotate tests.
- Concurrent refresh: model the race two ways. A DbUpdateConcurrencyException
  on save (stale #329 xmin token) is a controlled failure — INVALID_SESSION,
  DiscardChanges() runs, and no new token is issued. A loser replaying the
  already-consumed token is rejected while the session rotates exactly once
  to the winner's token (no double-rotation).

These complement the DB-integration coverage in
AuthSessionServiceConcurrentRefreshTests by asserting the service's own
contract (deterministic storage, recovery call, token non-issuance) at the
unit level.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…empotency replay (#243) (#374)

Close the tests-audit gaps for the bulk-command transaction/isolation
guarantees with behavior-first tests that fail if the guarantee breaks:

- BulkLogHabits: interleaved invalid/valid items apply per-item (valid
  logs persist, invalid reported with the right per-item error code and
  Index, batch not rolled back); a mid-batch persistence throw is isolated
  by the per-item catch so other items still succeed; a completed one-time
  task surfaces the domain failure per item and the batch continues.
- BulkDeleteHabits: replace the rubber-stamp "wrapper was called" test with
  real all-or-nothing proof — a mid-batch save failure skips streak recalc
  and cache invalidation and propagates; soft-deletes and recalc run strictly
  inside the transaction, not before it.
- IdempotencyBehaviorRace: extend the concurrent-duplicate race to the
  batch-command path — the loser replays the winner's full Result<batch>
  response through the ResultJsonConverterFactory (all items preserved),
  never re-running its own handler.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…potency (#243) (#377)

Close the remaining tests-audit gaps for four background services with
behaviour tests that exercise the tick loop and orchestration, not just the
pure helpers:

- OpenAiBatchPoller: a mixed poll finalizes a completed batch while leaving an
  in-flight one untouched; the hosted loop runs one tick and cancels cleanly.
- StreakFreezeAutoActivation: DB-backed idempotency — an existing freeze or a
  recorded guard for the missed date blocks re-activation and re-push; the
  hosted loop activates once and cancels cleanly.
- PlayNotificationCleanup: the hosted loop runs the purge on tick and respects
  the 30/90-day Play/Stripe retention windows.
- XpAwardLogBackfillHostedService: the backfill is idempotent across a restart
  and StopAsync during the startup delay unwinds without faulting.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
#243) (#379)

Extract the /health ResponseWriter into a unit-testable internal method that
now sets the HTTP status explicitly (200 Healthy / 503 Degraded or Unhealthy)
instead of relying on the default status-code map, which returned 200 for a
Degraded report and hid stale background services from load balancers and
uptime monitors. Enrich the per-check body with description and keep the raw
exception object out of the response so failure detail never leaks.

Tests build a HealthReport in-memory and invoke the writer directly: healthy
=> 200 + full body, unhealthy DB => 503 + failing check surfaced, degraded =>
503, and a check carrying an exception surfaces its description without leaking
the exception message, type, or stack trace.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…bulk-size limits (#243) (#380)

Close the remaining tests-audit gaps for the habit-schedule query and bulk
habit validators with behavior + edge + boundary assertions:

- GetHabitScheduleQueryValidator: negative Page and zero/over-max PageSize
  now assert the exact ValidationFailure (property, error code, attempted
  value); the max PageSize and minimum Page boundaries assert no error.
- GetHabitScheduleQueryHandler: totalPages arithmetic (zero, exactly
  divisible, +1 remainder), a far-beyond-last page clamping to the last
  page with its remainder item, and search+frequency filters combined with
  pagination returning the correctly filtered+paged subset with metadata.
- BulkCreate/BulkDelete/BulkLog validators: a near-limit (max) payload
  validates and an over-limit payload is rejected with the configured-max
  message, asserting the boundary at MaxBulkOperationSize.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…lationship (#243) (#381)

Assert the app-registered Kestrel MaxRequestBodySize resolves to 10 MB by
reading the built IOptions<KestrelServerOptions>, so a regression that drops
or unbounds the ceiling fails. Add a consistency guard that the transport
ceiling stays above the 8 MB upload content-type max, plus a validator test
proving a 9 MB upload is rejected by the content-type rule before the
request-body limit is reached.

Widen AddCookieAndKestrelLimits to internal (test project already has
InternalsVisibleTo) so the test exercises the exact production wiring.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
#243) (#384)

MarketingUnsubscribeTokenService and WaitlistConfirmationTokenService
shared near-identical HMAC-SHA256 signing, base64url encode/decode, token
composition, and signature-verification logic (SonarCloud flagged ~40
duplicated lines each). Extract the genuinely-shared codec into a single
internal static SignedTokenCodec; each service keeps its own payload
format, key source, and expiry semantics.

Behavior- and wire-format-preserving: the token string, signing input, and
verification are byte-identical, so tokens minted before this change still
validate. No public interface or DI registration change (static helper).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…389)

Behavior-preserving fixes for 11 SonarCloud csharpsquid rules:

- S1192 (7): extract repeated string literals to private consts
  (ResendEmailService heading/intro/footer, ProfileController
  enabled/disabled via ToggleLabel, NotificationTools update_notifications,
  OrbitDbContext '[]'::jsonb reuse the existing const).
- S3267 (10): simplify filtering loops to LINQ Where/Any/OfType/FirstOrDefault.
- S3358 (7): extract nested ternaries to statements/locals.
- S108 (3): document intentionally-empty catch blocks (URL-linked WHY).
- S1144 (3): delete unused private setters/field.
- S2589 (3): drop always-true/false conditions.
- S2325 (2): mark helper methods static.
- S1172 (1): remove unused cancellationToken param + caller.
- S1075 (1): move the Google OAuth token URL to Google:TokenUrl config.
- S3260 (1): seal the private UserDatePreferences record.
- S4581 (1): use Guid.Empty instead of default(Guid).

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…ects (#243) (#391)

Drives csharpsquid:S107 ("methods should not have too many parameters",
default max 7) from 28 open instances down to 3, behavior-preserving.

- DI-injected constructors (15): group cohesive dependencies into aggregate
  records following the existing *Repositories convention (GoalRepositories,
  BulkCreateHabitsRepositories, SkipHabitRepositories, GetCalendarEventsRepositories,
  ApplyOnboardingRepositories, CreateChallengeRepositories, GetFriendProfileRepositories,
  UserStreakRepositories, ExportUserDataRepositories) plus SocialInteractionServices,
  GamificationNotifiers, AgentPendingStores, StripeServiceClients; registered in DI.
- Methods / domain factory / interface (10): introduce cohesive parameter-object
  records (AiUsageTotals, DailySummaryContext, StreakBridgeContext, PerUserStreakLookups,
  AchievementAccumulator, HabitScheduleWindow, AgentSnapshotInputs, ClarificationToolResult,
  ResolveAuditOutcome), destructuring at method entry to keep bodies unchanged.

All call sites and unit-test constructions updated. dotnet build clean, full
dotnet test green (5284 passed). No behavior change.

Remaining (3): LogRateLimitRejected is a [LoggerMessage] source-gen method whose
parameters are required structured-log fields (verified S107 false positive — a
parameter object would fold them into one property and change log output);
AgentOperationExecutor.CreateAuditContext and IAiIntentService.SendWithToolsAsync
deferred.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…-binding, and loop smells (#243) (#392)

Behavior-preserving refactors that drive three SonarCloud rule clusters to zero:

- csharpsquid:S3776 (cognitive complexity, 13 methods): extract private
  helpers / guard clauses so each method is under the 15 threshold —
  SendCodeCommand, CreateHabitCommand, BulkSkipHabitsCommand,
  ApplyOnboardingCommand, UserStreakService, HandlePlayNotificationCommand,
  GetCalendarSyncSuggestionsQuery, SyncCleanupService, ProcessUserChatCommand,
  AiIntentService, ResendEmailService, and PushNotificationService (x2).
- csharpsquid:S6964 (14 value-type request properties): annotate with
  [property: JsonRequired] to prevent under-posting, matching the existing
  ProfileController precedent. Wire contract unchanged (append-only safe).
- csharpsquid:S1994 (14 retry loops): convert `for (var attempt = n; ; n++)`
  infinite loops to `while (true)` / counter-testing `for`, incrementing on
  every loop-back path so behavior is identical.

Full dotnet build (0 new warnings) + dotnet test (5284 tests) green.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 13, 2026
…394)

Clears every open external_roslyn / analyzer smell in the #243 SonarCloud-to-zero
campaign (the roslyn-b slice), all behavior-preserving:

- CA1068 (3): move CancellationToken to the last parameter on
  IUserStreakService.RecalculateAsync and AiCompletionClient.CompleteText/JsonAsync;
  update every call site (production + tests).
- CA1873 (4): hoist the expensive log argument into a local inside an IsEnabled guard
  (AdminController, WaitlistController, MinimumVersionMiddleware, AiUsageSummaryService).
- CA1862 (3): StringComparison.OrdinalIgnoreCase in SyncControllerMutations; the two EF
  query predicates (SetHandleCommand, FriendGraphService) keep ToLower() under a scoped,
  URL-linked suppression because EF Core cannot translate string.Equals(StringComparison).
- CA2016 (3): forward the CancellationToken in IdempotencyBehaviorDbTests handlers.
- CA2263 (3): use the generic TypeAssertions.Be<T>() overload in job-enqueue tests.
- CA1822 (1): mark ProcessUserChatCommand.BuildResponseCards static.
- CA1869 (1): cache JsonSerializerOptions in GetPublicProfileQueryHandlerTests.
- SYSLIB1045 (1): [GeneratedRegex] partial method in McpToolsDoNotDispatchCommandsTests.
- CS0618 (1): GetDeclaredQueryFilters() replaces obsolete GetQueryFilter().
- xUnit1042 (7): TheoryData<> member-data return types across OrbitDbContextTests,
  SyncChangesIndexConfigurationTests, ScheduledJobRegistryTests.
- ASP0015 (2): Headers.AcceptLanguage property in SubscriptionControllerTests.
- ASP0025 (1): AddAuthorizationBuilder in ServiceCollectionExtensions.
- RS1038 (2): split code-fix providers into a new Orbit.Analyzers.CodeFixes assembly so the
  analyzer assembly no longer references Microsoft.CodeAnalysis.Workspaces.

Verified: dotnet build 0 errors + 0 target analyzer warnings, dotnet test green (5284),
EF has-pending-model-changes = false.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 14, 2026
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 14, 2026
…ludes (#243) (#398)

Coverage burn-down toward the #243 frozen end-state. Adds intelligent
unit tests (behavior + edge + failure) for previously-uncovered logic and
coverage-excludes genuinely-untestable external-SDK/HTTP glue + CI tooling.

Tests added (~153):
- Validators: date-range query validators (streak/xp/goal-progress/completion-trends),
  CreateChallenge, JoinChallenge, AcceptFriendRequest, ApplyOnboarding (+habit/goal inputs).
- Chat tools (mediator-wrapper parse/success/failure branches): MoveHabitParent,
  LinkGoalsToHabit, ReorderHabits, UpdateChecklist, BulkDeleteHabits, BulkCreateHabits.
- AI services via the fake-ChatClient seam (success/empty/malformed/failure):
  AiGoalReview, AiSlipAlertMessage, AiProactiveCheckinMessage, AiTagSuggestion,
  AiRescheduleSuggestion; AiUsageSummary via in-memory OrbitDbContext.
- AppConfigService (in-memory DbContext + cache) and the SetSocialOptIn /
  DismissImportPrompt command handlers.

Coverage-excludes (sonar.coverage.exclusions, with rationale in the workflow):
StripeBillingService, GooglePlayBillingService, AiBatchClient, AudioTranscriptionService,
Services/Calendar/** (Google Calendar SDK glue; logic lives in the tested
GoogleCalendarEventFetcher), HangfireRecurringJobRegistrar, Orbit.Analyzers.CodeFixes/**,
and .github/** CI scripts. All are external-SDK/HTTP glue or build/CI tooling with no
mockable seam, consistent with the existing exclusion set.

Refs thomasluizon/orbit-ui-mobile#243 (coverage burn-down)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 14, 2026
…) (#399)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 14, 2026
…#402)

Clears the last CA1861 code smell (external_roslyn:CA1861, introduced by
#398's coverage tests): hoists the constant `{ "Monday", "Tuesday" }` array
literal passed as the reschedule payload's `days` argument into a
descriptively-named `private static readonly string[]` field, matching the
#387/#397 pattern. Behavior-preserving: same values, order, and element type;
full suite green.

Refs thomasluizon/orbit-ui-mobile#243

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 14, 2026
#404)

Behavior-preserving duplication burn-down for orbit-api (part of #243,
"Duplicated code → 0%"). Same outputs, error strings, and ordering
throughout; verified by the full unit suite (5454 green).

Extracted shared logic:
- Bulk validators: BulkLog/BulkSkip command validators (66.7% dup each)
  now derive from BulkHabitCommandValidatorBase<TCommand,TItem> over new
  IBulkHabitCommand/IBulkHabitItem marker interfaces.
- Auth commands: VerifyCode/GoogleAuth share AuthUserProvisioning
  .FindOrCreateUserAsync (race-safe find-or-create); GoogleAuth keeps its
  welcome-email side effect, VerifyCode its caller-side welcome.
- Bulk commands: BulkLog/BulkSkip handlers share BulkHabitLoader
  .LoadHabitsWithRecentLogsAsync (habit map + windowed logs).
- Goal tools: DeleteGoal/UpdateGoal share GoalToolHelpers (goal-id parse,
  owner-scoped lookup, not-found guard), mirroring HabitToolHelpers.
- Goal queries: GetGoalById/GetGoalDetail share GoalDetailLoader
  .BuildGoalDetailAsync (load + project GoalDetailDto).
- Chat tools: Calendar/Notification tools share ChatToolMediator.RunAsync
  (the duplicated mediator-run-to-ToolResult helper).
- AI habit tools: Bulk/Log/Skip tools share HabitToolHelpers schema
  builders; Log/Skip reuse the existing parse/not-found guards.
- ChatController: ProcessChat/ProcessChatStream share ParseChatInputsAsync.
- MCP HabitTools: BulkLog/BulkSkip share ExecuteBulkHabitOperationAsync.

Justifiably excluded (sonar.cpd.exclusions, rationale in workflow):
- CreateHabitTool/UpdateHabitTool: declarative LLM-facing JSON parameter
  schemas — overlapping fields but per-tool field sets and descriptions in
  a single anonymous object that cannot be composed without CPD-flagged
  mirror call sites or worse readability.
- AgentCatalogService.UserDataCatalog: declarative user-data catalog of
  unique rows sharing a constructor shape (parity with its existing
  S107/S1192/CA1861 suppressions).
- Slip/ProactiveCheckin schedulers: residual dup is per-class
  [LoggerMessage] source-generator partial-method boilerplate.

Refs thomasluizon/orbit-ui-mobile#243 (duplication → 0)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Jul 14, 2026
Drives orbit-api SonarCloud duplicated_lines_density 0.1% -> 0.0% (#404
deferred these files to avoid a concurrent-edit conflict).

Real extractions (genuinely-shared logic):
- HabitScheduleService: the identical active-days filter + frequency-unit
  switch tail of IsHabitDueOnDate and IsHabitHistoricallyDueOnDate is
  extracted into one private MatchesFrequency helper (self-dup ~24 lines).
- LevelDefinitions.SyncLevel(User): the GetLevelForXp + SetLevel level-sync
  idiom, previously copy-pasted in 6 places (both accept commands, SendCheer,
  ChallengeProgressService, JoinChallenge, and GamificationService.UpdateLevel),
  is unified into one canonical helper; UpdateLevel is deleted.

Justified exclusion (irreducible parallel mirror):
- AcceptAccountabilityPairCommand / AcceptFriendRequestCommand: after the
  shared level-sync is extracted, the only residual is the
  BuildRequesterNotification parallel mirror -- a null-guard + isPortuguese
  ternary skeleton emitting DISTINCT localized copy for two independent accept
  events in separate bounded contexts. CPD normalizes the differing string
  literals so the skeletons match; a shared builder would couple the two
  contexts without improving readability. Added to sonar.cpd.exclusions with
  rationale, consistent with the existing #404 exclusions.

Behavior-preserving: build 0 errors, all tests green, no EF model changes,
openapi.json unchanged.

Refs thomasluizon/orbit-ui-mobile#243 (duplication -> 0)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant