fix: roll back the user row when the signup OTP email fails to send - #208
Merged
Conversation
signup previously created the user row before confirming the OTP email actually went out. When Resend rejects the send (unverified domain, sandbox recipient restriction, bad key, etc.), that left a permanently stuck, unverifiable account occupying the email — every retry hit "email already registered" with no way to ever get a working code. Now a failed send deletes the just-created row so the email is free to retry signup cleanly. Adds EmailSender::new_failing() (test-fixtures only) to simulate the failure in a new regression test.
phalap1
added a commit
to phalap1/Octo-Protocol
that referenced
this pull request
Aug 7, 2026
Picks up the email OTP work (Octo-Protocol-org#207, Octo-Protocol-org#208, Octo-Protocol-org#209). One real conflict, plus six call sites the automatic merge left compiling against the old signatures. Conflict: - withdraw_refuses_to_sign_for_a_client_custody_wallet called auth_token(&app); main changed the helper to auth_token(&app, &state) because signup now issues its token only after the emailed code is consumed. Took main's form. Fixed while merging, all of it mechanical: - five more auth_token(&app) call sites in the withdraw idempotency tests merged without markers, since main never saw those tests. Two of them moved state into build_router, so they now clone it like every other test in the file. - withdraw_preflight_tests built AppState with five arguments; main added the EmailSender parameter. Uses EmailSender::new_captured(), same as the other test fixtures. - withdraw_preflight_tests signed up by hand and read data.token off the response. That field is gone: signup returns data.user_id and the token comes from /v1/auth/verify-email. Now goes through common::signup_and_verify_full. The withdrawal OTP main added sits on its own routes, /withdraw/request-otp and /withdraw/confirm, and gates the already-signed XDR path in submit.rs. It does not touch POST /v1/wallets/:id/withdraw, so the pre-flight balance and minimum-reserve checks are unchanged. cargo clippy --workspace --all-targets and cargo fmt --all are clean. cargo test -p octo-api runs 153 tests against Postgres with no failures and no skips, including the three pre-flight tests from Octo-Protocol-org#108. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
signupcreated theusersrow before confirming the OTP email actually sent. When Resend rejects the send (unverified domain, sandbox recipient restriction, bad key, etc.), the account was left permanently stuck and unverifiable — every retry with that email hit "email already registered" with no way to ever get a working code.EMAIL_FROM_ADDRESSis currently the Resend sandbox address (onboarding@resend.dev) whileoctohq.orgfinishes domain verification, and the sandbox only allows sending to the account owner's own address — so any other signup email failed to send and got stuck.EmailSender::new_failing()(test-fixtures only) to simulate the failure in a new regression test.Test plan
signup_rolls_back_the_user_row_when_the_otp_email_fails_to_send— confirms the row is deleted and the same email can be retried.cargo test --workspace --locked— all passingcargo clippy --workspace --all-targets --locked -- -D warnings— cleancargo fmt --all -- --check— clean