Repository navigation
fix(auth): follow central-manager's verification outcome on the verify-email page - #2030
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the email verification flow to support a new central-manager behavior where the sign-up password is replaced and a set-password email is sent upon verification. It updates the VerifyEmail component and its hook to handle various verification outcomes (such as reset_email_sent and already_verified), implements automatic retries for 503 errors, and adds comprehensive unit and E2E tests. Feedback on the changes suggests correcting the generic type parameter on the apiClient.put call in useVerifyEmail.ts to ensure correct typing, and forwarding all arguments (error, variables, context) to the parent onError handler in the mutation hook to prevent potential runtime issues.
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
dawsontoth
marked this pull request as ready for review
October 27, 2026 20:53
kriszyp
approved these changes
Oct 28, 2026
…y-email page central-manager#785 makes a token-only `PUT /VerifyEmail/` replace the sign-up password and email a set-password link, and reports what it did in a new `outcome` field. The page ignored the response and sent everyone to sign in with a password that no longer works. The page now reads `outcome`. No outcome (today's central-manager) and `password_set` go to sign in as before. `reset_email_sent`, `password_reset_required`, `already_verified` and any outcome it doesn't know stay on the page and say how to get a working password; `already_verified` is what a reload, a second click or a retried request answers, after the first visit may already have replaced the password. A 503 is retried three times with backoff. Any verify left without an answer (503, a gateway's 502/504, no response) then gets "Try again" with the same link instead of the request-a-new-link form, in copy that claims no outcome; repeating it is safe, since one that landed answers `already_verified`. Each token is submitted once per arrival, so StrictMode's second effect can't replace the first answer, and a late answer for a token the route has dropped is ignored. The signup round-trip e2e follows either answer, so it passes before and after central-manager#785 deploys. Part of #1697. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… out A verify sent while the browser was offline fell back to the new-link form and an offline toast, so reconnecting offered no way to retry the link short of reloading. It now gets the retry panel with the offline copy, stays out of the toast and RUM like the other unconfirmed verifies, and Try again resends the same token once back online. The retry panel also gains the Back to sign in and Forgot password links the verified views already had, so a persistent 502/504 is not a dead end. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moving email verification to the page-shown offline cases dropped it from the shared test that pinned an online refusal's toast description and its single RUM report; the hook's own 401/403 test now asserts both. Also rewords the onError comment, which no longer fit the offline case, and a mail helper comment that named a central-manager function. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dawsontoth
force-pushed
the
fix/1697-verify-email-outcomes
branch
from
October 28, 2026 18:43
8b76766 to
892dc0f
Compare
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
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.
⊙ Problem
HarperFast/central-manager#785 makes
PUT /VerifyEmail/with only the token replace the sign-up password and email a link to set a new one. Studio's verify page ignores the response and sends everyone to sign in with the old password, which fails. #785 can't deploy until Studio handles this, and Studio must still work with today's central-manager, which sends nooutcome.🤔 Is this the right first step?
Yes, as specified for step C1 of #1697: handle every outcome and send no
newPassword. Asking for a password on this page is the second half. Splitting it keeps this change small and safe to ship before #785.💡 Solution
The verify page now follows the response's
outcome:password_set: toast and go to sign in, as before.reset_email_sent,password_reset_required,already_verifiedor an unknown outcome: stay on the page and say how to get a working password.Screenshots: the new states (light theme, dev server, response answered locally)
reset_email_sentalready_verifiedA 503 after three retries
Sent while offline
🤔 Should
already_verifiedstay on the page?The suggested design toasted and went to sign in. But a reload, a second click, a mail scanner or a retry gets that answer after the first visit may have replaced the password, so the page stays and points at the set-password email. Keeping the first answer instead is #2029.
⚖️ Alternatives
onSuccess: a toast is too brief to be the only notice that the password was replaced, and the new-link form can't retry the same link as a 503 asks.🔧 Changes
src/features/auth/hooks/useVerifyEmail.ts: response type with an optionaloutcome(and theuseIdtypo fixed);isVerifyEmailUnavailable(503, retried) andisVerifyEmailUnconfirmed(isUnansweredWriteor an offline send, shown as Try again with no toast).reportAuthFailurestill sends the unanswered ones to RUM and keeps an offline one out.src/features/auth/VerifyEmail.tsx: the outcome views, the retry panel's offline copy and sign-in links, the once-per-arrival guard (a late answer for a token the route dropped is ignored), and "Verifying" from the first render instead of a flash of the resend form. A token is used only when the router hands it over as a string, which a JWT always is; the router's JSON parse of other values is Auth pages read token through the router's JSON parse of search values #2026.DESIGN.md: notes why this one write is repeated, under "A write with no verdict never reads as failed". The Try again copy still claims no outcome.e2e/tests/signup-verification.anon.spec.ts: follows either answer. On the set-password view it reads the email, sets a new password on/#/reset-passwordand signs in with that. Timeout sum redone (705s, limit now 840s).e2e/tests/mail.ts:waitForSetPasswordEmailmatches the "Set your Fabric password" subject, returns only the extracted link, and names the subject when nothing arrives.src/features/auth/VerifyEmail.test.tsx,src/features/auth/hooks/useVerifyEmail.test.tsx: every outcome, 503 retry timing, Try again for each unanswered kind, offline then reconnect then Try again with the same token, a retry that finds the email verified, other errors, StrictMode and token changes, a late answer, both predicates.src/features/auth/failFastOffline.test.tsx: email verification now shows an offline failure on the page, not in a toast. The hook's own test pins its refusal toast and single RUM report instead.src/features/auth/AuthStatus.test.tsx: its hook mock now matches the hook's new shape.🤔 Should automatic retries stay 503-only?
Only a 503 retries on its own, because central-manager says to retry it; a 502, 504 or no answer goes straight to Try again. Studio can't tell central-manager's 503 from a gateway's, so both get the 14 seconds of retries. #2027 tracks narrowing that.
🤔 Should the e2e drop the legacy branch after #785 ships?
It accepts either answer, so it passes on both sides of the deploy. Once #785 is everywhere, a central-manager that regressed to the old answer would still pass. Removing the sign-in branch then is a small follow-up.
✅ Verification
vitest run: 590 files, 6,094 tests pass.tsc -b,oxlint,dprint checkand e2etsc --noEmitare clean.already_verifiedto sign in, or reporting a refusal twice turns a test red.PUT /VerifyEmail/answered locally): one PUT per token; each view renders; four 503s led to Try again with no toast; a 503 thenalready_verifiedexplained the repeat; an offline send showed the offline copy, and Try again after reconnecting resent the same token.@roundtripe2e did not run: it needs Mailosaur credentials and the dev environment, which this machine lacks. It typechecks and lists.--no-verify: at machine load 30–90 the husky hook's unboundedvitest runsegfaulted once and once failedentityAuthCheckScope.test.ts(untouched; passes alone). Its steps passed by hand. The two follow-up commits ran the full hook.🤖 Generated by Claude Code (Claude Opus 5.5); posted via @dawsontoth.
Related PRs: none found (partial scan: issue #2026 search truncated at 20 of 82)
Complexity: medium
Deferred findings
10faec7ad8362e92nit reliability —src/features/auth/hooks/useVerifyEmail.ts— every 503 on /VerifyEmail/, including a gateway's, is auto-retried and offered Try again — filed Verify page retries every 503, not only central-manager's own #20278735edf8a60533bdnit correctness —src/features/auth/VerifyEmail.tsx— router-parsed non-string token (0, numeric, over 2^53) treated as absent — filed Auth pages read token through the router's JSON parse of search values #2026ef8b2b248908c683nit correctness —src/features/auth/VerifyEmail.tsx— last-token ref: A to B to A re-PUTs A; removed-then-re-added token not resubmitted — filed Verify page re-sends a token that returns after another instead of keeping its first answer #2029Review-Coverage: authored=claude; ran=gemini,codex,cursor-composer,cursor-grok; pending=cursor-grok; late=cursor-grok:pending; adjudicated=domain; blocked=cursor-muse(failed); declined=cursor-kimi,muse; rounds=7; full=3 @ 8b76766
Review-Attention: deep ~30m (sensitive: VerifyEmail.tsx, useVerifyEmail.ts; decisions: do-less-alternative, offline-in-unconfirmed-predicate, reload-copy-vs-persisted-outcome, hook-level-503-retry, e2e-accepts-either-cm) @ 8b76766