fix(auth): answer a retryable sign-in failure with copy, not the server's words

[dawsontoth]

Why

Since 2026-09-01 20:27 UTC the central-manager API has answered 503 on its unauthenticated auth resources, and nothing else. In one 24h window /Login returned 503 seventeen times against nine successes — 17 of the 26 responses that were not a credential rejection — and three of the four affected sessions never signed in at all. Zero 503s on those paths in the preceding 30 days, and four more in the two hours before this PR was opened.

With no body, describeError falls back to the Axios message, so the user was shown the bare Request failed with status code 503. That says nothing about whether to retry, or whether the request got far enough to check their credentials. Sign-in also had no inline failure line, so it arrived as a toast that fades, away from the inputs. RUM caught one session resubmitting /Login nine times over seventeen minutes, another four times in twelve seconds.

The 503 itself is server-side and stays open on #1676; this is the client half. Sign-up got exactly this treatment in #1613 (for #1612's 409 dead end) — sign-in never did, and sign-in is the endpoint now failing.

What changed

A 5xx is answered with our own copy, and the server's words are never rendered. This is the design decision the review moved me off, and it is worth stating plainly: these pages are anonymous and the inline alert this PR adds persists, where the old toast faded. A 5xx body is our own infrastructure talking — Harper's exceeded request queue limit for resolving cache record, or an upstream connect ECONNREFUSED 10.0.3.x:9925 — none of it actionable by a signed-out visitor, and some of it our topology. 4xx still defers, because that is where an authored, actionable reason lives (Invalid email or password, User has not verified email address), and describeError already knows how to extract it.

Gating on status rather than on body content is also what makes the rule hold. My first two passes gated on whether the body contained a usable sentence — first !response.data, then a length-and-leading-character heuristic — and both leaked (proxy HTML, {"title":" "} rendering a blank alert) while duplicating describeError's extraction somewhere it could drift. Status cannot drift.

Which 5xx copy appears turns on whether the request could already have been processed — not on curryRetryGatewayErrors's retry list, which is installed on instance clients only and never on apiClient, so nothing auto-retries an auth call and the retry this copy invites is the user's own. Only 503 promises a plain retry, because a declining server very likely never processed the request — and a 503 is exactly the production failure this PR exists for. 502, 504, any timeout, and ERR_NETWORK each mean the request may already have been applied: all three of these submits are non-idempotent POSTs, so the write's outcome is unknown and "try again" would turn a completed sign-up into a Conflict: user already exists with the verification email already sent — #1668's dead end, manufactured by our own copy. Every remaining 5xx gets copy that does not promise waiting helps and offers a way to escalate.

For that unknown-outcome case the shared copy states the uncertainty and stops; the recovery comes from the caller. Reloading an anonymous form performs no status check, so a generic "reload and try again" just repeats the side effect (RFC 9110 §9.2.2: a retry is safe only once you know the request was not applied). Only the form knows what recovery means for its endpoint — sign-up says to check for the verification link, forgot-password says to check the inbox, and sign-in says to simply try again, being the one of the three that is genuinely safe to repeat. Each form's test asserts its own clause reaches the alert and rejects the other's, so swapping the two constants fails instead of silently shipping the wrong advice.

The escalation is decided by the message, not by each caller. My first pass wired it through sign-in's state only, and left sign-up and forgot-password telling users "trying again may not help" with no way out — the fix-one-of-N trap. SubmitErrorMessage now derives it from the message it was handed, so a fourth form cannot ship without it, and all three forms have a test asserting the link reaches the DOM.

429 gets its own copy. Folding it in with the 5xx text tells a throttled user that it "isn't a problem with the details you entered" and invites the immediate resubmit that rolls the throttle window forward — the one place that sentence is both untrue and counterproductive. It says "Too many attempts", not "from this device": a per-IP throttle behind corporate NAT would otherwise blame the visitor for a colleague's attempts.

Nothing that ends without an answer invites a bare retry, and that includes ERR_NETWORK: axios reports any request that got no response that way, including a CORS rejection and a connection dropped after the POST applied, so it cannot claim the server was never reached. There is no "we couldn't reach the server" message any more. Transport codes are enumerated, not defaulted. A client-side configuration fault (ERR_INVALID_URL, ERR_BAD_OPTION) never touched the network and a cancelled request reached nothing, so neither claims anything — previously a bad API base URL in a deploy would have told every user to check their internet. The sets are built from AxiosError's own constants and a test pins them, because an earlier pass of mine invented two codes (ERR_BAD_RESPONSE_TIMEOUT, ECONNRESET) that axios's browser adapter never emits: a branch keyed off a code that never arrives is dead, and the failure it was meant to describe falls through silently.

Sign-in renders its failure beside the inputs, and clears it on an invalid resubmit too. handleSubmit skips the submit handler entirely when the resolver rejects, so clearing only inside it would leave Invalid email or password standing next to a fresh Please enter your password. — two contradictory alerts, one describing an attempt that never left the browser. The clear hangs off handleSubmit's invalid handler as well.

Forgot-password (503 on three of its four responses in the same window) routes retryable failures inline alongside the CAPTCHA rejections it already handled that way; everything else keeps its toast.

The RUM report moved to the mutation, not the caller — minus control flow. The RUM report for sign-in and sign-up now lives in the mutation's own onError rather than the caller's mutate callback, because React Query skips the latter when the component unmounted mid-flight — exactly when someone gave up on a slow sign-in and navigated away. A test pins the unmount case. The unverified-email 403 is excluded there: submitForm redirects into the verification flow on it, so reporting it would file every unverified sign-in as an error — a regression a review leg caught in the move itself, now pinned by its own test.

Three copies of the same role="alert" markup became one SubmitErrorMessage. Sign-up and forgot-password carried byte-identical blocks; sign-in would have been a third. Nothing else in src still spells that markup out. It also bounds what a 4xx body can render: those still defer to the server verbatim, the alert persists where the old toast faded, and an edge/WAF block page in front of central-manager arrives as one long string — break-words bounds the width, so the component bounds the length too.

For the human reviewer

Seven open judgment calls, from the final review's decision ledger. Each is rulable from the entry alone.

1. blanket-5xx-substitution — the one to push back on if you disagree with anything. Every 5xx now gets our copy and the server's words are never rendered, because these pages are anonymous and the alert persists where the old toast faded: a 5xx body is our infrastructure talking (Harper's exceeded request queue limit for resolving cache record, an upstream connect ECONNREFUSED 10.0.3.x:9925). The cost is real — a deliberately client-facing 5xx like {code:'InternalError', title:'Signup is unavailable'}, which fix(auth): report a failed sign-up in the form, not a fading toast #1613's test pinned, now shows generic copy, and a genuine CM reason is visible only in RUM. I narrowed fix(auth): report a failed sign-up in the form, not a fading toast #1613's deliberately status-agnostic design to do this. The AGENTS.md note tells the next author not to revisit it, so say now if that is too strong.

2. render-4xx-bodies-verbatim. 4xx bodies still render verbatim (bounded to 240 units) on that same anonymous persistent alert — the PR's own test renders a 4,000-char WAF block page. I kept it because 4xx is where the authored, actionable reason lives (Invalid email or password, User has not verified email address, which isEmailNotVerifiedError keys off), but an edge appliance's text is not authored by us either. Treating unauthored 4xx like 5xx is the alternative.

3. caller-supplied-recovery — resolved in review, worth confirming. The classifier used to say "reload before trying again"; kriszyp pointed out a reload performs no status check, so following it repeats the side effect. It now states only the uncertainty and each form appends its own recovery. The residual judgment call is whether three per-form strings are better than one classifier-owned map — I chose the former because the classifier has no business knowing what a verification email is.

4. signup-remains-root-user. Sign-in and forgot-password now keep their failure in component state, cleared from handleSubmit's invalid handler; sign-up is the last root user. That split is deliberate rather than finished: forgot-password moved because this PR had widened its stale-root window from CAPTCHA rejections to every retryable failure, so the regression was mine to fix here. Sign-up's is pre-existing and belongs in its own reviewable change — Sign-up and forgot-password keep a stale server error on screen after a resubmit that fails client-side validation #1677, which now records that handleSubmit(fn, () => clearErrors('root')) does not fix it; the state has to move.

5. support-link-by-message-identity. SubmitErrorMessage derives the escalation link from message === SERVER_ERROR_MESSAGE rather than a flag from the classifier. That is what stops a fourth form shipping without it (my first pass wired it into sign-in only and left the other two dead-ended), but it degrades silently to "no link" if a caller ever composes that copy itself, and the unknown-outcome message gets no escalation at all — it carries the caller's recovery instead.

6. forgot-password-stays-hybrid. Forgot-password renders CAPTCHA and retryable failures inline and still toasts the rest; sign-in and sign-up went fully inline. Two contracts on one surface, which e2e/README.md now has to carry. I kept the split because widening it changes behavior the RUM data does not implicate.

7. e2e-left-doc-only. This diff corrects e2e/tests/sign-in.anon.spec.ts's now-false "errors are toasts, NOT inline" note but adds no routed Playwright case forcing a 503. See Verification for what that leaves unproven, and why it could not be added here.

One thing the review caught that is worth calling out on its own: my first forgot-password test fixture used a 404 No such account, and I wrote that into e2e/README.md as the expected toast path — which would have codified an account-enumeration oracle on a page whose own copy promises not to reveal whether an address exists. Fixture and README both changed.

Findings I rejected on evidence, so you can check the evidence rather than the claim. A leg reported four times, escalating to blocker, that e2e/tests/sign-in.anon.spec.ts asserts on [data-sonner-toast] and will time out in CI; the file contains no toast assertion and no failed-submit test at all — only the doc comment mentioned toasts. A leg reported that Object.values(AxiosError) returns nothing (non-enumerable constants), making the dead-code guard vacuous and red; node -e returns 14 enumerable string values including ERR_NETWORK, and the test would fail if the set were empty. A leg filed the same phantom major twice — that SignUp's onError "lacks" console.error — which is the first statement of that block, unchanged by this diff. Two gemini findings the domain leg then ruled unreachable (a bodyless 429, the ERR_CANCELED exclusion) I kept anyway and flag here so you can strike them: both are inert on today's paths and neither fixes a live bug. A late round reported that moving sign-up's report to mutation level newly sends CAPTCHA and 409 rejections to RUM; git show stage:src/features/auth/SignUp.tsx has console.error(error) as the first statement of that onError, so the move changed when it fires, not what it reports — and 409 visibility is what made #1612/#1668 measurable in the first place.

Three claims of mine the review falsified, corrected here, in AGENTS.md, and in #1676:

• A bodyless 503 does not show "We had some trouble!" — that needs message absent too, which real AxiosErrors never are; it appeared only because the repo's test fixtures omit it. Production showed the raw status line.
• AGENTS.md said every handled rejection "reaches Error Tracking"; shouldKeepEvent drops timeouts, 401s and third-party stacks in beforeSend. A durable invariant doc that overstates coverage sends the next author hunting events that were never kept.
• I twice reported the wrong thing about sign-up's stale root error, and the third answer is the one in the diff. A leg read react-hook-form's source and said root is cleared on a resolver-rejected resubmit; a minimal useForm probe agreed, so I closed Sign-up and forgot-password keep a stale server error on screen after a resubmit that fails client-side validation #1677. Re-run against the real SignUp with expect(post).toHaveBeenCalledTimes(1) — proving the second submit was rejected client-side rather than re-failing — the stale alert is still there, and handleSubmit(fn, () => clearErrors('root')) does not change it. Sign-up and forgot-password keep a stale server error on screen after a resubmit that fails client-side validation #1677 is reopened, this PR carries the regression test that documents it, and AGENTS.md records that a minimal probe does not reproduce these forms.

Cursor legs are structurally unavailable on this PR. Both refuse any diff that edits agent instructions, and this one records notes in AGENTS.md; the CLI pruned them every round. That is a policy refusal, not failed reviews — coverage is codex + gemini + the Harper domain leg across six completed rounds (codex was down on an upstream 404 for two of them and ran in the other four).

Verification

Full repo gate on Node 24.19.0: vitest run 338 files / 2914 passing, tsc -b, oxlint and dprint check all clean — exit codes captured directly rather than through a pipe, since cmd | tail; echo $? reports the wrong process.

End-to-end route: component-level tests, not a browser run. SignIn.test.tsx renders the real SignIn with the real useCloudSignIn and the app's own mutationErrorHandler — only the HTTP client is mocked — and asserts on the rendered role="alert" text, so it exercises the path a user takes rather than a mocked-away hook. A dev-server preview was not available: this ran in an unattended scheduled session, where starting one is refused. What that leaves unproven (ledger entry 6): no test sees a real axios response off the wire, and none proves Datadog keeps the event — the spies prove the call. e2e/tests/sign-in.anon.spec.ts is the file to extend for the browser half.

Every guard was mutation-checked — reverted locally, confirming a test goes red rather than watching one go green:

Guard removed	Result
5xx defers to its body again (the disclosure)	7 failed
describeRetryableAuthFailure always returns undefined	13 failed across 4 suites
429 folded back into the 5xx copy	2 failed
ERR_NETWORK back to its own claim rather than unknown-outcome	1 failed
the unknown-outcome copy loses the caller's recovery clause	6 failed
forgot-password reverted to the react-hook-form root model	1 failed
sign-up's recovery swapped for forgot-password's	1 failed
skipGlobalErrorToast deleted from the login mutation	3 failed
<SubmitErrorMessage> dropped from SignIn	3 failed
setSubmitError(undefined) on resubmit	2 failed
invalid-submit clear dropped from SignIn	1 failed
console.error dropped from useCloudSignIn / SignUp / forgot-password	1 failed each
the escalation decision removed from SubmitErrorMessage	3 failed (one per form)
502 or 504 moved back into the transient set	1 failed
the render-length cap removed	1 failed
a code axios never emits added back to the pinned set	1 failed

One change is deliberately not in that table: break-words on the alert is a style jsdom cannot measure, so no test bites when it is removed. Asserting the class name would be a tautology, so it is verified by inspection and named here rather than covered by a decorative test.

That table includes one test that was previously vacuous: the hook suite's QueryClient had no MutationCache onError, so expect(toast.error).not.toHaveBeenCalled() passed however the hook behaved. It now routes through the app's own handler and bites when skipGlobalErrorToast is deleted.

New coverage: 500/501/502/503/504/505 → our copy, including when the body carries problem details, a plain sentence, a legacy error field with an internal address, proxy HTML, {statusCode:503}, or a whitespace-only title — with an explicit assertion that 10.0.3.14 does not reach the DOM; 429 with and without a body → throttle copy; 400/401/403/404/409 → the server's own text; ERR_NETWORK/ECONNABORTED/ETIMEDOUT → unknown outcome, carrying the caller's recovery; ERR_CANCELED/ERR_INVALID_URL/ERR_BAD_OPTION and a plain Error → no claim; a response with no status and a non-error input → no throw; stale failure cleared on both a valid and a resolver-rejected resubmit; no alert before a submission fails.

Not in scope

ResetPassword, VerifyEmail, Verifying and ClusterInstanceSignIn have no submit-error handling of their own and were not in the RUM data; they still fall through to the global toast, unchanged. CheckOAuth's fixed-string toast is #1674. Forgot-password's CAPTCHA branch renders its failure without a console.error, so those rejections reach neither the toast nor RUM — pre-existing, unchanged here, and deliberately left: an ad-blocker-driven CAPTCHA 403 is the noise class #1658 says not to report. The 503 root cause needs a server-side look and stays on #1676: central-manager's Login.post throws only 400/401/403/409, so it originates below or in front of the resource.

Complexity: medium

_{Review-Coverage: authored=claude; ran=codex,gemini; adjudicated=domain; declined=cursor-grok,cursor-composer; rounds=13 @ b324dd0}

_{Human-Review-Need: 4 (decisions: err-network-as-unknown-outcome, caller-supplied-recovery, anonymous-4xx-verbatim, status-gate-not-body, signup-remains-root-user, telemetry-placement-asymmetry, console-error-as-rum-channel, agents-md-volume) @ b324dd0}

[gemini-code-assist]

Code Review

This pull request refactors authentication error handling across the sign-in, sign-up, and forgot-password forms to render submission failures inline using a new SubmitErrorMessage component instead of relying on global toasts. It introduces a centralized utility (describeAuthFailure) to classify errors (such as 5xx, 429, timeouts, and network issues) and provide user-friendly, actionable copy while keeping raw infrastructure details out of anonymous forms. Additionally, it ensures that telemetry/RUM reporting is preserved by moving console.error logging to the mutation level, preventing lost reports when components unmount mid-flight. Comprehensive unit and integration tests have been added to verify these behaviors. I have no feedback to provide as there are no review comments.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	60.22%	8217 / 13644
🔵	Statements	60.67%	8806 / 14514
🔵	Functions	52.92%	2066 / 3904
🔵	Branches	54.13%	5903 / 10904

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/features/auth/ForgotPassword.tsx	96.96%	100%	87.5%	96.87%	61
src/features/auth/SignIn.tsx	83.33%	66.66%	60%	83.33%	114-119
src/features/auth/SignUp.tsx	85.36%	69.23%	86.66%	87.5%	129-133
src/features/auth/describeAuthFailure.ts	100%	100%	100%	100%
src/features/auth/components/SubmitErrorMessage.tsx	100%	100%	100%	100%
src/features/auth/hooks/useCloudSignIn.ts	97.22%	70%	100%	97.14%	112
src/features/auth/hooks/useForgotPassword.ts	83.33%	75%	100%	83.33%	24
src/features/auth/hooks/useSignUp.ts	85.71%	75%	100%	85.71%	22

Generated in workflow #1851 for commit b324dd0 by the Vitest Coverage Report Action

[kriszyp]

Lots of helpful improvements, nice!
_{🤖 Reviewed with Codex}

[dawsontoth]

Review response — a0b0af40

@kriszyp all three of your comments are addressed in one follow-up commit rather than a force-push, so the diff you approved stays reviewable and the delta is visible on its own. Threads answered and resolved individually; the short version:

Your point	What changed
Reload does not make the outcome known	The shared copy states the uncertainty and stops. Each form supplies its own recovery: check the inbox / check for the verification link / just sign in again. A test asserts the shared half contains no "again".
ERR_NETWORK does not establish unreachability	Reclassified as outcome-unknown; SERVER_UNREACHABLE_MESSAGE is deleted.
This PR widened the stale-root window on forgot-password	Forgot-password moved to component state cleared from the invalid callback, like sign-in, with an invalid-resubmit regression test that also asserts the second submit never reached the network.

The PR description above still describes the pre-review behaviour for the first two rows — I am refreshing it once the review receipt catches up to this head, and did not want to leave you waiting on that to see the response.

Two things worth flagging rather than burying:

• I did not consolidate sign-up onto the same state model here. It is the identical fix and belongs somewhere it can be reviewed on its own; #1677 tracks it. Note there that handleSubmit(fn, () => clearErrors("root")) does not fix the root variant — verified against the real component — so it needs the state move, not a callback.
• The type gate caught a vacuous test while I was doing this: removing an export left the expected value undefined, and waitFor matched that against the initial state, so it passed for the wrong reason. Repaired and mutation-checked.

Full gate green on Node 24.19.0: 338 files / 2920 tests, tsc -b, oxlint, dprint. Every guard in this commit was mutation-checked.

[DavidCockerill]

Approving — no findings.

🧊 In plain terms: when a sign-in fails for a reason worth retrying, the form now shows its own wording instead of relaying whatever the server said. That keeps server-side detail off a page anyone can reach, and it is the same instinct as not revealing whether an email is registered.

What held up on a read: the failure classification is conservative about uncertain POST outcomes rather than assuming failure, server-side details stay out of anonymous forms, and telemetry survives unmount.

Same family as central-manager#657 on not revealing whether an email is registered — opposite end of the stack, same principle about what an unauthenticated surface is allowed to tell you.

One caveat on the verification rather than the change: the focused Vitest suite was not executed, because the review checkout is read-only and has no node_modules. That is a property of how I review rather than a gap in this PR, but it means the assertion coverage here is read, not run.

_{— DAIvid (Claude Opus 5) · cross-model: Codex (graded) + Gemini · no domain leg this pass}