spikerj/spikersoft-issues
1
0
Fork 0
Code Issues 196 Pull Requests Actions Packages Projects Releases Wiki Activity

Pre-registration phone verification broken: API resolves NoOpTwilioVerifyService edge stub instead of dispatching to Notifications worker #260

New Issue
Closed
opened 2026-06-26 19:38:27 +00:00 by spikerj · 2 comments
spikerj commented 2026-06-26 19:38:27 +00:00
Owner
Copy Link
Copy Source

Symptom

Production logs after a user verifies their email during "Join SpikerSoft":

Failed to send SMS verification to +1XXXXXXXXXX after email verified for <preRegId>:
  API edge stub — Twilio Verify is owned by SpikerSoft.EventHandlers.Notifications.
TwilioVerify.Send invoked on the API edge stub (phone=..., channel=sms).
  Refactor the caller to dispatch SendVerificationCodeCommand via MediatR; the
  RemoteDispatchBehavior will route it to SpikerSoft.EventHandlers.Notifications.

(SourceContext: SpikerSoft.Business.Edge.Services.NoOpTwilioVerifyService, action VerifyPreRegistrationEmail.)

The verification SMS is never sent. The frontend reports "code sent" but nothing arrives.

Root cause

The API registers the edge stub for ITwilioVerifyService:

// SpikerSoft.Api/Extensions/ServiceCollectionExtensions.cs
services.AddScoped<ITwilioVerifyService, NoOpTwilioVerifyService>();

NoOpTwilioVerifyService just logs a warning and returns Success = false — the real Twilio implementation runs only inside SpikerSoft.Workers.Notifications (hosted by SpikerSoft.EventHandlers.Notifications). Every in-process API caller that resolves ITwilioVerifyService therefore no-ops.

This is not limited to the reported handler — it affects the entire phone-verification surface (6 callers):

Send (SMS) — SendVerificationAsync:

  • InitiatePreRegistrationCommandHandler (the initial registration SMS)
  • VerifyPreRegistrationEmailCommandHandler (post-email SMS — the reported one)
  • ResendPreRegistrationCodesCommandHandler
  • SendPhoneVerificationCommandHandler (profile phone flow)

Check (code) — CheckVerificationAsync:

  • VerifyPreRegistrationPhoneCommandHandler
  • VerifyPhoneNumberCommandHandler (profile phone flow)

The check callers are especially bad: even if an SMS did arrive, the stub's CheckVerificationAsync always returns invalid, so pre-registration phone verification can never complete. This is the SMS analogue of the email-edge-stub bug (#225).

Fix

Provide an edge ITwilioVerifyService implementation (RemoteTwilioVerifyService) in SpikerSoft.Business.Edge that dispatches SendVerificationCodeCommand / VerifyCodeCommand (both typed IRemoteCommand<T>) through MediatR. RemoteDispatchBehavior publishes them over RabbitMQ direct-reply-to and returns the worker's real response, which the adapter maps back to TwilioVerifyResult / TwilioVerifyCheckResult. Register it in place of NoOpTwilioVerifyService.

Why an adapter rather than refactoring all six callers: the ITwilioVerifyService interface is a clean 1:1 with the two remote commands and carries no per-call user id (the worker uses UserId only for audit logging, so the phone number is a safe stand-in). One registration swap repairs all callers with no change to their logic or existing tests.

Keep NoOpTwilioVerifyService for reference/tests.

Acceptance

  • Email verification → SMS verification code is actually delivered.
  • Entering the SMS code completes phone verification (pre-registration becomes fully verified).
  • Initiate / resend SMS also work.
  • Backend-only change; no SPA change.

Notes

  • Worker SMTP/Twilio worker deploy was addressed in #258; this is the API-side dispatch wiring.
  • Possible adjacent cleanup (separate): the worker's SendVerificationCodeCommandHandler / VerifyCodeCommandHandler read Twilio:VerificationServiceSid while other code was standardized to Twilio:VerifyServiceSid — worth reconciling so the configured SID is actually picked up.
## Symptom Production logs after a user verifies their email during "Join SpikerSoft": ``` Failed to send SMS verification to +1XXXXXXXXXX after email verified for <preRegId>: API edge stub — Twilio Verify is owned by SpikerSoft.EventHandlers.Notifications. TwilioVerify.Send invoked on the API edge stub (phone=..., channel=sms). Refactor the caller to dispatch SendVerificationCodeCommand via MediatR; the RemoteDispatchBehavior will route it to SpikerSoft.EventHandlers.Notifications. ``` (`SourceContext: SpikerSoft.Business.Edge.Services.NoOpTwilioVerifyService`, action `VerifyPreRegistrationEmail`.) The verification SMS is never sent. The frontend reports "code sent" but nothing arrives. ## Root cause The API registers the **edge stub** for `ITwilioVerifyService`: ```csharp // SpikerSoft.Api/Extensions/ServiceCollectionExtensions.cs services.AddScoped<ITwilioVerifyService, NoOpTwilioVerifyService>(); ``` `NoOpTwilioVerifyService` just logs a warning and returns `Success = false` — the real Twilio implementation runs only inside `SpikerSoft.Workers.Notifications` (hosted by `SpikerSoft.EventHandlers.Notifications`). Every in-process API caller that resolves `ITwilioVerifyService` therefore no-ops. This is **not** limited to the reported handler — it affects the entire phone-verification surface (6 callers): **Send (SMS) — `SendVerificationAsync`:** - `InitiatePreRegistrationCommandHandler` (the initial registration SMS) - `VerifyPreRegistrationEmailCommandHandler` (post-email SMS — the reported one) - `ResendPreRegistrationCodesCommandHandler` - `SendPhoneVerificationCommandHandler` (profile phone flow) **Check (code) — `CheckVerificationAsync`:** - `VerifyPreRegistrationPhoneCommandHandler` - `VerifyPhoneNumberCommandHandler` (profile phone flow) The check callers are especially bad: even if an SMS did arrive, the stub's `CheckVerificationAsync` always returns invalid, so **pre-registration phone verification can never complete**. This is the SMS analogue of the email-edge-stub bug (#225). ## Fix Provide an edge `ITwilioVerifyService` implementation (`RemoteTwilioVerifyService`) in `SpikerSoft.Business.Edge` that dispatches `SendVerificationCodeCommand` / `VerifyCodeCommand` (both typed `IRemoteCommand<T>`) through MediatR. `RemoteDispatchBehavior` publishes them over RabbitMQ direct-reply-to and returns the worker's real response, which the adapter maps back to `TwilioVerifyResult` / `TwilioVerifyCheckResult`. Register it in place of `NoOpTwilioVerifyService`. Why an adapter rather than refactoring all six callers: the `ITwilioVerifyService` interface is a clean 1:1 with the two remote commands and carries no per-call user id (the worker uses `UserId` only for audit logging, so the phone number is a safe stand-in). One registration swap repairs all callers with no change to their logic or existing tests. Keep `NoOpTwilioVerifyService` for reference/tests. ## Acceptance - Email verification → SMS verification code is actually delivered. - Entering the SMS code completes phone verification (pre-registration becomes fully verified). - Initiate / resend SMS also work. - Backend-only change; no SPA change. ## Notes - Worker SMTP/Twilio worker deploy was addressed in #258; this is the API-side dispatch wiring. - Possible adjacent cleanup (separate): the worker's `SendVerificationCodeCommandHandler` / `VerifyCodeCommandHandler` read `Twilio:VerificationServiceSid` while other code was standardized to `Twilio:VerifyServiceSid` — worth reconciling so the configured SID is actually picked up.
spikerj commented 2026-06-26 19:41:52 +00:00
Author
Owner
Copy Link
Copy Source

Fix in spikersoft-backend PR #20 (fix/issue-260-twilio-verify-edge-dispatch, targeting master). Adds RemoteTwilioVerifyService (edge adapter dispatching SendVerificationCodeCommand / VerifyCodeCommand via MediatR → RemoteDispatchBehavior → Notifications worker) and registers it in place of NoOpTwilioVerifyService. One swap fixes all six callers; 4 new unit tests pass. Will close once PR #20 is merged.

Fix in spikersoft-backend PR #20 (`fix/issue-260-twilio-verify-edge-dispatch`, targeting `master`). Adds `RemoteTwilioVerifyService` (edge adapter dispatching `SendVerificationCodeCommand` / `VerifyCodeCommand` via MediatR → RemoteDispatchBehavior → Notifications worker) and registers it in place of `NoOpTwilioVerifyService`. One swap fixes all six callers; 4 new unit tests pass. Will close once PR #20 is merged.
spikerj commented 2026-06-26 20:01:38 +00:00
Author
Owner
Copy Link
Copy Source

Resolved in spikersoft-backend PR #20 (merged to master). Added RemoteTwilioVerifyService (edge adapter dispatching SendVerificationCodeCommand / VerifyCodeCommand via MediatR → RemoteDispatchBehavior → Notifications worker) and registered it in place of NoOpTwilioVerifyService, repairing all six in-process Twilio Verify callers. Closing.

Note: the adjacent config-key mismatch flagged here is tracked separately in #261 (PR #21).

Resolved in spikersoft-backend PR #20 (merged to `master`). Added `RemoteTwilioVerifyService` (edge adapter dispatching `SendVerificationCodeCommand` / `VerifyCodeCommand` via MediatR → RemoteDispatchBehavior → Notifications worker) and registered it in place of `NoOpTwilioVerifyService`, repairing all six in-process Twilio Verify callers. Closing. Note: the adjacent config-key mismatch flagged here is tracked separately in #261 (PR #21).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#260