feat(api): marketing email consent + code-based broadcast sender + admin concept

[thomasluizon]

Summary

Reworks marketing email from a Resend-dashboard-Broadcasts model to a code-based sender, so product-update emails go out in each user's language and your branded template like every other Orbit email. Adds a real admin concept (reusable by a future admin dashboard) and a self-hosted unsubscribe. Deploy before the client PR.

• Consent (User.MarketingEmailConsent, nullable tri-state + timestamp) is the single source of truth. PUT /api/profile/marketing-consent sets it — DB only now, no external sync.
• Admin concept: User.IsAdmin (+ migration), an admin JWT claim minted only when IsAdmin, an Admin authorization policy replacing the bare AddAuthorization(), and a config-seeded bootstrap (Admin:BootstrapEmails). The claim/policy is the gate; the seeder is only how the flag is first granted. Reusable by any future admin endpoint.
• Code sender: POST /api/admin/marketing/broadcast ([Authorize(Policy=Admin)], rate-limited) — background fan-out to consenting users, each email rendered in their language with the TL SOFTWARE ENGINEERING LTDA footer, paced with backoff (Resend sends had none). Test-send mode for a single preview.
• Own unsubscribe: Data-Protection-signed token → public GET/POST /api/marketing/unsubscribe → flips consent. List-Unsubscribe + one-click POST headers on every marketing email.
• Removed the now-dead Resend segment sync, contact.updated webhook, ResendWebhookVerifier, and the Svix dependency (they only served the abandoned dashboard-broadcast path); the consent handler is DB-only.

Ops after deploy: grant your operator account admin with a one-off DB update (UPDATE "Users" SET "IsAdmin"=true WHERE "Email"=...). The previously-planned Resend segments/webhook/env vars are no longer used.

Tests: admin policy allow/deny, IsAdmin claim minting, broadcast handler (consent filter, per-language, test-mode, pacing), unsubscribe token round-trip + tamper rejection + idempotency. Full suite green (Domain 487, Application 2592, Infrastructure 1375).

Refs thomasluizon/orbit-ui-mobile#397

🤖 Generated with Claude Code

[thomasluizon]

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

[claude]

Review — PR #287: feat(api): marketing email consent + Resend segment sync + webhook

Recommendation: Request changes (1 High finding)

High

Missing validator for UpdateMarketingConsentCommand
src/Orbit.Application/Profile/Commands/UpdateMarketingConsentCommand.cs

Every sibling command in Profile/Validators/ has a FluentValidation validator — including the structurally identical SetSocialOptInCommand (RuleFor(x => x.UserId).NotEmpty();). UpdateMarketingConsentCommand has none. This violates the repo's cross-cutting hard rule: "Every new feature needs validators in Orbit.Application/<Feature>/Validators/."

Fix: add Profile/Validators/UpdateMarketingConsentCommandValidator.cs mirroring SetSocialOptInCommandValidator:

public class UpdateMarketingConsentCommandValidator : AbstractValidator<UpdateMarketingConsentCommand>
{
    public UpdateMarketingConsentCommandValidator()
    {
        RuleFor(x => x.UserId).NotEmpty();
    }
}

What was checked and cleared

• Webhook security (Resend webhook signature verification): fails closed on empty/misconfigured secret, signature checked before payload use, [AllowAnonymous] justified and mirrors the existing Stripe webhook precedent. No issues.
• Contract / backward compatibility: ProfileResponse.MarketingEmailConsent is additive, nullable, defaults null — no old-client break. Full packages/shared comparison is not verifiable in this environment (orbit-ui-mobile not checked out); flagged for the paired mobile PR.
• Timezone hard rule: MarketingConsentUpdatedAtUtc = DateTime.UtcNow matches the existing codebase convention for XxxUpdatedAtUtc audit fields set in entity mutators.
• Tests: comprehensive coverage for the command handler, webhook handler (idempotency, unknown-email, non-matching event, signature/misconfig paths), domain entity, and verifier. No dead code, no comment-policy violations.
• FEATURES.md parity: this dimension requires the orbit-ui-mobile checkout to verify and update — not verifiable in this CI environment; needs to land in the paired frontend PR (orbit-ui-mobile#397), not a blocker on this backend PR.

Build/Unit Tests/SonarCloud run as separate required CI checks and were not re-run here.

[claude]

[High] UpdateMarketingConsentCommand has no FluentValidation validator. Every sibling command in Profile/Validators/ has one — including the structurally identical SetSocialOptInCommand. Per the repo's cross-cutting hard rule, every new feature needs a validator in Orbit.Application/<Feature>/Validators/.

Fix: add Profile/Validators/UpdateMarketingConsentCommandValidator.cs:

public class UpdateMarketingConsentCommandValidator : AbstractValidator<UpdateMarketingConsentCommand>
{
    public UpdateMarketingConsentCommandValidator()
    {
        RuleFor(x => x.UserId).NotEmpty();
    }
}

[thomasluizon]

Addressed in 38f94a7: added UpdateMarketingConsentCommandValidator mirroring SetSocialOptInCommandValidator (RuleFor(x => x.UserId).NotEmpty()) plus its unit test. FEATURES.md row lands on the paired mobile PR #401.

[claude]

Code Review: PR #287

Scope: PR #287 in thomasluizon/orbit-api — "feat(api): marketing email consent + Resend segment sync + webhook"
Recommendation: APPROVE

Summary

Adds LGPD-lawful marketing-email consent (User.MarketingEmailConsent tri-state + SetMarketingConsent), a PUT /api/profile/marketing-consent endpoint, an IMarketingContactsService extension for Resend segment sync (upsert/unsubscribe/delete), and a Svix-verified POST /api/webhooks/resend handler that flips consent to false on contact.updated+unsubscribed:true. The change is well-tested (domain, handler, validator, webhook idempotency/signature, contacts-service HTTP behavior) and follows CQRS/validator/domain-guard conventions. No Critical or High findings survive review; two Medium items are worth a quick follow-up but don't block merge.

Findings

Critical

None.

High

None.

Medium

[Medium] Webhook rejection is logged twice, at inconsistent severities
· dimension: Backend hard rules (#13) — Logging levels
· location: src/Orbit.Api/Controllers/ResendWebhookController.cs:29 and src/Orbit.Application/Resend/Commands/HandleResendWebhookCommand.cs:280-284
· issue: HandleResendWebhookCommandHandler.Handle already logs each rejection at the level matching its actual severity — LogSecretNotConfigured at Critical (ops misconfiguration) and LogInvalidSignature at Warning (expected/recoverable, since the endpoint is [AllowAnonymous] and reachable by anyone). ResendWebhookController.HandleWebhook then logs the same failure a second time via LogWebhookRejected at Level = LogLevel.Error.
· risk: Every failed signature check (which any internet client can trigger simply by POSTing garbage to the public endpoint) now produces two log lines, one of them at Error — noisier prod logs and, per this repo's own rule ("reserve each level so prod logs carry only signal"), inflated Error-level volume for what is often just bot/scanner traffic rather than an actionable failure.
· fix: Drop the controller's LogWebhookRejected call (the handler already logged the right severity) or reduce it to Debug/Trace if request-level correlation is wanted.
· reference: orbit-api/CLAUDE.md "Logging levels" hard rule

[Medium] No test asserts GetProfileQuery maps the new MarketingEmailConsent field
· dimension: Backend hard rules (#13) — Tests
· location: tests/Orbit.Application.Tests/Queries/Profile/GetProfileQueryHandlerTests.cs
· issue: ProfileResponse.MarketingEmailConsent was added and wired from user.MarketingEmailConsent in GetProfileQuery.cs, but the existing, otherwise-thorough GetProfileQueryHandlerTests suite (which has a dedicated assertion block per field) has no case covering this mapping for null/true/false.
· risk: A future refactor of GetProfileQuery's constructor call could silently drop or misplace this field (it's a positional record) with nothing failing.
· fix: Add a small test (or extend an existing one) asserting result.Value.MarketingEmailConsent reflects user.MarketingEmailConsent for the null/opted-in/opted-out cases.
· reference: orbit-api/CLAUDE.md "Tests" hard rule

Low / Info

• PII in Resend sync logs (Low): ResendContactsService.LogContactAdded/LogContactSyncFailed log the raw email address; this PR extends that pre-existing pattern (already present for waitlist adds) to the new consent-sync/segment paths, while the newer HandleResendWebhookCommandHandler and AccountDeletionService correctly log only user.Id. Consider switching ResendContactsService to log user.Id (available at both call sites) for consistency. Not gating — matches an existing pattern rather than introducing a new one.
• Catalog placement of the webhook action (Info): AgentCatalogService.Capabilities.cs adds "ResendWebhookController.HandleWebhook" into the ProfilePreferencesWrite capability's controllerActions, alongside "ProfileController.UpdateMarketingConsent". Verified this list has no runtime authorization effect — it only feeds AgentCatalogServiceTests.EveryControllerAction_IsMappedToTheCatalog (a reflection-based completeness guard requiring every public controller action to appear in some capability). Functionally inert, but semantically odd: the webhook is [AllowAnonymous] system-to-system traffic, not a user/agent-driven "write preferences" action. No functional/security consequence; cosmetic only.
• FEATURES.md (Info): This PR is explicitly backend-only plumbing ("deploy before the paired client PR"); the actual user-facing consent toggle ships in thomasluizon/orbit-ui-mobile#397, which is the PR that should update FEATURES.md. Flagging here for visibility per the cross-repo parity contract, not as a blocker on this PR.
• Redundant .ToLower() (Info): HandleResendWebhookCommand.cs:37 compares u.Email.ToLower() == normalizedEmail; User.Create already lowercases Email at construction (User.cs:109), so the .ToLower() is a no-op for all rows created through the factory. Harmless (defense-in-depth against legacy/unnormalized rows, if any exist), not worth changing.

Subagents

Agent	Verdict
security-reviewer	PASS — Svix verification fails closed on a blank/misconfigured WebhookSecret (SecretNotConfigured is checked before verification and rejected as HTTP 500); signature check delegates to the official Svix library (constant-time compare, timestamp tolerance); no auth/authz gaps; only a Low PII-in-logs note (see above).
contract-aligner	NOT VERIFIABLE — sibling orbit-ui-mobile checkout is unavailable in this sandbox. Manual inspection: ProfileResponse.MarketingEmailConsent is appended as the last positional parameter with a = null default, and record serialization is property-name-based, so old clients are unaffected — this reads as a forward-compatible additive field (Info), not a break, but should be confirmed against packages/shared/src/types/*.ts when reviewing the paired mobile PR.

Validation

Check	Result
Build (dotnet)	N/A — dotnet build/dotnet test required a permission this sandbox session couldn't grant. The PR's own description states "Full suite green"; the diff shows dedicated unit tests for every new handler, validator, domain method, and infrastructure service.
Tests (dotnet)	N/A — same restriction as above.

What's good

• Textbook persist-first/sync-second design: the local consent decision commits via SaveChangesAsync before the Resend call, and the Resend sync is wrapped in a catch-all (excluding OperationCanceledException) so a Resend outage can never roll back or block the user's decision — directly tested (Handle_ResendSyncFails_StillPersistsDecisionAndSucceeds).
• Webhook handler fails closed on both "secret not configured" and "invalid signature," is idempotent (explicitly tested with Handle_DoubleFire_IsIdempotent), and cleanly no-ops on unknown emails/non-target event types without ever touching the DB.
• IMarketingContactsService's XML doc clearly states the fire-and-forget contract, and every implementation path (create/patch/segment-add/delete) treats "already exists" / "not found" as success rather than throwing — matches the calling handlers' expectations.
• Account deletion now also removes the Resend contact (LGPD erasure), wrapped in its own try/catch so it can't block account deletion.
• Good test breadth: domain (tri-state consent + timestamp), command handler (opt-in/opt-out/user-not-found/Resend-failure), validator, webhook handler (signature states, idempotency, unknown email, wrong event type, still-subscribed no-op), real-Svix-backed verifier test (valid/tampered/no-secret), and contacts-service HTTP-level tests for every new method.

Recommendation

Safe to merge. Consider addressing the two Medium logging/test-coverage items in this PR or a fast follow-up; none of the Low/Info notes need to block anything.

[claude]

Review — PR #287 (diff since last review: 38f94a7 → HEAD)

Scope: commit 8d11291 "code-based marketing sender + admin concept + own unsubscribe" is a substantial redesign — it rips out the previously-approved Resend-webhook/segment-sync approach entirely (deletes ResendWebhookController, HandleResendWebhookCommand, IResendWebhookVerifier, ResendWebhookVerifier, Svix dependency) and replaces it with: an IsAdmin/JWT-claim/AdminPolicy admin concept, a code-based POST /api/admin/marketing/broadcast sender that queries consenting users directly and emails them via an in-process background fan-out, and a self-serve GET/POST /api/marketing/unsubscribe using Data-Protection-encrypted tokens.

Recommendation: Request changes (1 High finding)

Severity	Count
Critical	0
High	1
Medium	3

High

Unsubscribe tokens use non-persistent Data Protection keys — will break after the next auto-deploy
· dimension: Correctness / Backend hard rules (validation & no-workarounds)
· location: src/Orbit.Infrastructure/Services/MarketingUnsubscribeTokenService.cs:8-9
· issue: CreateToken/TryValidateToken use ASP.NET Core's default IDataProtectionProvider with no PersistKeysToFileSystem/PersistKeysToDbContext/SetApplicationName configured anywhere in the diff or the rest of the repo (verified via grep — no matches). The codebase already has an established, correct pattern for exactly this problem: WaitlistConfirmationTokenService signs tokens with a config-driven SigningKey (Waitlist:SigningKey) that's stable across restarts — but this new service didn't reuse it.
· risk: Render auto-deploys on every push to main (per this repo's own deployment model), and each fresh container gets a new, unpersisted Data Protection key ring. Every unsubscribe link already embedded in an already-sent marketing email becomes permanently invalid the moment the next deploy lands — recipients hit "Invalid link" trying to opt out. This directly undermines the PR's own stated purpose (LGPD-lawful marketing-email consent), since a broken unsubscribe path is the exact compliance risk the feature exists to avoid. The test suite uses EphemeralDataProtectionProvider (explicitly non-persistent, single-process) for MarketingUnsubscribeTokenServiceTests, which incidentally proves the production configuration was never exercised against a restart/redeploy scenario.
· fix: Either configure services.AddDataProtection().PersistKeysToDbContext<OrbitDbContext>() (or file system + a mounted volume) with SetApplicationName, or drop Data Protection and mirror WaitlistConfirmationTokenService's config-driven signing-key approach for consistency and guaranteed persistence.

Medium

Marketing broadcast fan-out is unrecoverable in-process work with no persisted progress
· location: src/Orbit.Application/Marketing/Commands/SendMarketingBroadcastCommand.cs:60-89 (FanOutInBackground, fire-and-forget Task.Run)
· issue: A large broadcast (thousands of recipients × SendDelayMilliseconds) can run for minutes, with no persisted record of which recipients were actually emailed.
· risk: An unrelated push-to-main mid-broadcast (which auto-deploys and restarts the container) silently truncates the send with no visibility and no way to resume without re-emailing already-sent recipients.
· fix: Track progress in a lightweight sent-log table, or move the fan-out to a durable background-job mechanism instead of Task.Run.

Unsubscribe token embeds a timestamp that's never checked (dead intent)
· location: src/Orbit.Infrastructure/Services/MarketingUnsubscribeTokenService.cs:15,36-38
· issue: CreateToken writes {userId:N}|{unixTimestamp} but TryValidateToken only ever reads the GUID segment — the token never expires.
· fix: Either wire up an actual expiry check (e.g. ToTimeLimitedDataProtector) or drop the timestamp from the payload — as written it reads like an intended check that was dropped.

No unit test coverage for the new ResendEmailService.SendMarketingEmailAsync retry/backoff path
· location: src/Orbit.Infrastructure/Services/ResendEmailService.cs:108-201; tests/Orbit.Infrastructure.Tests/Services/ResendEmailServiceTests.cs has no marketing case
· issue: The new ~95-line method (exponential-backoff retry on 429/5xx, List-Unsubscribe headers, test-account skip) isn't exercised anywhere — SendMarketingBroadcastCommandHandlerTests only fakes IEmailService, never the real HTTP implementation.
· fix: Add ResendEmailServiceTests cases mirroring the existing SendVerificationCodeAsync coverage (success, retriable-then-success, retries-exhausted, non-retriable failure, test-account skip).

Info (not gating)

• Admin JWT claim rides the existing 7-day stateless access token with no revocation path (mitigated: RefreshSessionAsync re-derives IsAdmin from the DB on refresh; blast radius is bounded to the rate-limited 5/hr broadcast endpoint) — worth a follow-up if admin surface grows.
• AgentCatalogService.Capabilities.cs's new marketing.manage capability entry is catalog-only (completeness-guard test), confirmed no runtime authorization effect — independently re-verified: ApiKeyAuthenticationHandler doesn't allow API keys on /api/admin at all, and no MCP tool wraps the broadcast/unsubscribe endpoints, so this catalog entry cannot grant real access.
• GET-based one-click unsubscribe mutates state on link click (standard industry tradeoff for marketing unsubscribe, idempotent, low stakes).

What's good

• Clean, complete removal of the prior webhook/Resend-segment design — no orphaned references to IResendWebhookVerifier, ResendWebhookController, HandleResendWebhookCommand, or the Svix package anywhere in src/tests.
• AdminController is properly gated ([Authorize(Policy = AdminPolicy.Name)]), rate-limited (5/hr), and the admin claim is minted fresh from the DB at login and on refresh.
• Good test breadth for the new domain/handler/validator code: UserAdminTests, AdminAuthorizationPolicyTests, SendMarketingBroadcastCommandHandlerTests (audience filtering, per-language rendering, test-preview path), UnsubscribeMarketingCommandHandlerTests (idempotency, invalid/unknown token), and thorough validator tests.
• Rate-limit policies for both new endpoints (admin-broadcast, marketing-unsubscribe) are registered in DistributedRateLimitService, and the anonymous unsubscribe endpoint partitions by IP correctly.

Subagent verdicts

security-reviewer PASS (surfaced the DataProtection High finding independently, plus the admin-claim-lifetime Info note); contract-aligner NOT VERIFIABLE (orbit-ui-mobile not checked out in this job — no DTO/contract surface changed in this increment regardless, so not a blocker).

Validation

Build/Tests/SonarCloud run as separate required CI checks and were not re-run here.

[claude]

High: unsubscribe tokens will break on the next deploy. dataProtectionProvider here resolves to ASP.NET Core's default IDataProtectionProvider, and nothing in this PR (or the rest of the repo — verified via grep) configures PersistKeysToFileSystem/PersistKeysToDbContext/SetApplicationName. Render auto-deploys on every push to main, so each fresh container gets a brand-new, unpersisted key ring — every unsubscribe link already embedded in a sent marketing email becomes invalid the moment the next deploy lands. WaitlistConfirmationTokenService already solves this exact problem with a config-driven SigningKey (Waitlist:SigningKey) that's stable across restarts; consider mirroring that approach here, or wire up AddDataProtection().PersistKeysToDbContext<OrbitDbContext>() with SetApplicationName.

[claude]

Review — PR #287 (diff since last review: 8d11291 → HEAD)

Scope: One commit landed since the prior review: a42fb11 "refactor(api): drop admin bootstrap seeder — grant IsAdmin directly in the DB" (28 lines deleted across WebApplicationExtensions.cs and appsettings.json).

Recommendation: Request changes — the prior review's High finding was never addressed and no subsequent commit touches it.

Carried-forward High (still unresolved, re-verified against current HEAD)

Unsubscribe tokens use non-persistent Data Protection keys — will break after the next auto-deploy
· location: src/Orbit.Infrastructure/Services/MarketingUnsubscribeTokenService.cs:8-9
· issue: CreateToken/TryValidateToken use ASP.NET Core's default IDataProtectionProvider with no PersistKeysToFileSystem/PersistKeysToDbContext/SetApplicationName configured anywhere in the repo (re-confirmed via grep -rn "DataProtection|PersistKeys|SetApplicationName" — only match is an unrelated log-filter line in Program.cs). The codebase already has an established, correct pattern for exactly this problem: WaitlistConfirmationTokenService signs tokens with a config-driven SigningKey that's stable across restarts — this new service doesn't reuse it.
· risk: Render auto-deploys on every push to main, and each fresh container gets a new, unpersisted Data Protection key ring. Every unsubscribe link already embedded in an already-sent marketing email becomes permanently invalid the moment the next deploy lands.
· fix: Configure key persistence (PersistKeysToDbContext/file system) + SetApplicationName, or switch to the existing config-driven signing-key pattern used by WaitlistConfirmationTokenService.

Other Mediums from the prior review remain open but non-blocking: unrecoverable in-process broadcast fan-out (no persisted progress), unsubscribe token embeds a timestamp TryValidateToken never checks, missing retry/backoff test coverage for ResendEmailService.SendMarketingEmailAsync.

New findings on a42fb11 itself

Medium

User.GrantAdmin() is now orphaned production code
· location: src/Orbit.Domain/Entities/User.cs:408
· issue: a42fb11 deletes SeedBootstrapAdminsAsync, the only caller of GrantAdmin(). A zero-reference grep now shows only the method definition and its own unit test — no application/controller path reaches it, matching the commit's stated intent that the first admin is now granted via a raw one-off DB update.
· fix: Delete GrantAdmin() and its test since nothing calls it, or add a one-line XML doc if it's intentionally kept for a future admin dashboard.
· reference: CLAUDE.md "No dead code"

Low

• Stale XML doc on AdminPolicy (src/Orbit.Application/Common/AdminPolicy.cs:6) still references the removed bootstrap seeder.
• PR description's "Ops after deploy: set Admin:BootstrapEmails" note is now stale — that config key no longer exists after a42fb11.

What's good in a42fb11

• Clean, complete removal — no leftover references to Admin:BootstrapEmails or SeedBootstrapAdminsAsync anywhere in src, tests, or appsettings.*.json.
• The actual authorization gate (IsAdmin → JWT admin claim → AdminPolicy) is untouched; removing the seeder only changes how the flag is first set, not how it's enforced.

Validation

Build/Tests/SonarCloud run as separate required CI checks — not re-run here.

Recommendation

Request changes. The blocker is the still-open Data Protection key persistence High finding from the prior review — it isn't touched by the latest commit and will cause already-sent unsubscribe links to break on the next deploy. The new commit (a42fb11) is otherwise clean aside from one Medium (orphaned GrantAdmin()) and two Low doc-drift nits.

[claude]

Review Complete

Scope: PR #287 in thomasluizon/orbit-api, diff since last review (a42fb11 → 41314e0, one commit: "fix(api): stable-key HMAC unsubscribe token + broadcast resilience")
Recommendation: APPROVE

Severity	Count
Critical	0
High	0
Medium	0 (new)
Low / Info	0 (new)

Summary

This commit is a direct fix-up responding to the previous review's blocking High finding. It replaces ASP.NET Data Protection (unpersisted per-deploy key ring on Render — every already-sent unsubscribe link would have broken on the next auto-deploy) with a hand-rolled HMAC-SHA256 signed token, mirroring the codebase's existing WaitlistConfirmationTokenService pattern exactly: config-driven Marketing:UnsubscribeSigningKey, constant-time comparison (CryptographicOperations.FixedTimeEquals), fail-closed on missing key/malformed token, dropped Microsoft.AspNetCore.DataProtection.Abstractions package. It also wraps each recipient send in the broadcast fan-out in its own try/catch (one bad address no longer aborts the whole run), makes the Resend retry backoff base delay configurable, and updates two stale XML docs.

Prior findings — resolution status

• [High] Data Protection key persistence (blocking) — Fixed. New HMAC scheme is deploy-stable by construction; tests rewritten for round-trip/tamper/wrong-key/empty/missing-key-throws.
• [Medium] Unsubscribe token embeds unread timestamp — Fixed. Timestamp dropped from payload entirely (links are intentionally non-expiring, which is safe here — the operation is idempotent, per-userId, and only flips a boolean consent flag).
• [Medium] Missing retry/backoff test coverage for ResendEmailService.SendMarketingEmailAsync — Fixed. New ResendEmailServiceMarketingRetryTests.cs covers 429 retry, 5xx retry, exhausted-retries give-up, and non-retriable 4xx no-retry, via a mock HttpMessageHandler.
• [Medium] Orphaned User.GrantAdmin() — Resolved via the reviewer's own suggested alternative: kept, with an updated XML doc explaining it's the seam a future admin dashboard will use. Still has no production caller, but this was an explicitly offered acceptable resolution, not a new finding.
• [Low] Stale AdminPolicy/GrantAdmin XML docs referencing the removed bootstrap seeder — Fixed, both updated to reflect the direct-DB-update bootstrap.
• [Medium] Unrecoverable in-process broadcast fan-out (no persisted progress across a mid-flight deploy) — Still open, not touched by this commit. This was already tagged "non-blocking" in the prior review; the per-recipient try/catch added here improves resilience to individual send failures but doesn't add resumability across a container restart. Worth a follow-up, does not gate this PR.

New checks on this commit

• Security (via security-reviewer subagent): PASS. HMAC construction, encoding, and fail-closed behavior verified sound; non-expiring token is acceptable given the idempotent, per-user, boolean-flip nature of the operation; per-recipient failure logging is Guid-only (no PII), and Error level matches the pre-existing convention already used for individual send failures elsewhere in ResendEmailService; DataProtection package removal confirmed to have no other callers in the repo.
• Contract drift (contract-aligner): N/A — no DTO, controller route, or shape change in this increment.
• Backend hard rules: no new endpoints (no authz gate needed); logging is structured/English/PascalCase; new/changed tests exist for the token scheme, retry backoff, and broadcast resilience.
• Dead code / comment policy: clean — no orphaned code newly introduced, all new comments are /// XML docs.

What's good

• The fix mirrors an existing, already-trusted pattern (WaitlistConfirmationTokenService) rather than inventing a new crypto scheme.
• Test rewrite is thorough and mirrors the sibling service's test shape.
• Per-recipient resilience + sent/failed count in the completion log is a clean, minimal-diff improvement.

Recommendation

Safe to merge. No Critical/High findings survive on this increment; the one carried-forward Medium (fan-out resumability) was already accepted as non-blocking follow-up work in the prior review.

Subagents: security-reviewer → PASS. contract-aligner → N/A (no contract surface touched in this increment).
Validation: Build/Tests skipped per instructions (CI handles these separately).

[claude]

Review Complete

Scope: PR #287 in thomasluizon/orbit-api — "feat(api): marketing email consent + code-based broadcast sender + admin concept"
Diff since last review: last review (bot account claude) was an APPROVE at commit 41314e0. Current HEAD is 9c88935, reached via two merge commits (deb2a01 "Merge branch 'main' into issue-397", 9c88935 the PR's synthetic merge). Diffed 41314e0..HEAD file-by-file and confirmed every changed file (ProfileController.cs, User.cs, GetProfileQuery.cs, WebApplicationExtensions.cs, AgentCatalogService.Capabilities.cs, AgentOperationExecutor.cs) belongs to the unrelated, already-merged-to-main PR #286 ("onboarding-before-auth apply endpoint + import-prompt flag") pulled in by the main merge — new ApplyOnboardingCommand, DismissImportPromptCommand, HasSeenImportPrompt field/migration, etc. None of it touches this PR's own surface (Marketing/Admin controllers, MarketingUnsubscribeTokenService, SendMarketingBroadcastCommand, AdminPolicy, ResendEmailService, User.IsAdmin/MarketingEmailConsent). ProfileController.cs's onboarding additions are purely additive and don't touch the marketing-consent endpoint code added earlier in this PR.

Recommendation: APPROVE

Findings

Critical

None.

High

None.

Medium

None (new).

Low / Info

None (new).

All prior findings (Data Protection key persistence → fixed with HMAC scheme; timestamp-in-token → fixed; missing retry/backoff tests → fixed; orphaned GrantAdmin() → resolved by keeping with updated XML doc; stale docs → fixed) were already resolved as of 41314e0 and are unaffected by the subsequent no-op merge commits. The one previously-accepted non-blocking Medium (in-process broadcast fan-out has no persisted progress across a mid-flight redeploy) remains open as a known, explicitly non-blocking follow-up — not re-flagged, since it was already surfaced and accepted.

Subagents

Agent	Verdict
security-reviewer	N/A this pass — no new src/ code in the diff since last review (only unrelated onboarding files from the main merge); prior pass's PASS stands unchanged.
contract-aligner	NOT VERIFIABLE IN CI — sibling orbit-ui-mobile repo not checked out. No DTO/contract surface changed in this increment regardless (confirmed no marketing/admin files changed since 41314e0).

Validation

Check	Result
Build (dotnet)	N/A — skipped per instructions; handled by separate required CI checks.
Tests (dotnet)	N/A — skipped per instructions; handled by separate required CI checks.

What's good

Nothing new to evaluate — this pass is a no-op confirmation that the branch is still in the previously-approved state; the two merge commits only absorbed an unrelated, already-main-merged feature (onboarding-before-auth) with zero overlap or interference with this PR's marketing/admin code.

Recommendation

Safe to merge/keep approved. No action required on this increment.

[sonarqubecloud]

Quality Gate failed

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

See analysis details on SonarQube Cloud