fix(domains): report each added domain's outcome instead of aborting on the first conflict

[dawsontoth]

Adding a domain on the cluster Domains page reported a conflict twice to Error Tracking and threw away work it had already done. RUM for 2026-09-08: 11 of 11 POST /Domain/ 409s in 30 days landed in a single day (against 7 creations in that whole month), each session retrying 2–5× at 4–13 second intervals before anything succeeded — and every 409 produced two events, one handling:handled and one handling:unhandled.

The two defects

onSubmitClick awaited addDomain with no error handling.

The rejection escaped the submit handler. react-hook-form re-throws whatever handleSubmit's callback throws — verified in the installed dist, react-hook-form@7.86.0, dist/index.esm.mjs:3222:

if (onValidError) {
    throw onValidError;
}

so it left the DOM submit handler as an unhandled promise rejection, i.e. crash-class signal in Error Tracking for something the app had already handled with a toast one frame earlier.

Partial progress was silently discarded. The loop aborted on the first rejection, so refetch(), form.reset() and the success toast were all skipped and the table never showed the domains that had been created. This is not a hypothetical multi-domain path: the Add www as well button rewrites the field to example.com, www.example.com in one click, so a partial failure is the normal outcome for an apex domain — and resubmitting the whole string then conflicted on the entry that had just succeeded.

For the human reviewer

Five judgment calls the review surfaced. All are cheap to reverse; I'd rather you overrule one than not see it.

1. The batch stops on anything that is not a per-name refusal, and continues past a 4xx. A degraded or unreachable server answers the next 19 names the same way and each can cost the full 60s apiClient timeout — twenty minutes of a locked form to learn what the first one said, where the old fail-fast loop stopped after one. A per-name refusal — 400/403/404/409/422 — is about that name and says nothing about the next, so those continue. A 401 and a 429 do not: the 401 has already cleared auth and started a redirect (unauthorizedResponseHandler), so the rest would POST unauthenticated, and 19 immediate retries are the one thing not to send a limiter that just refused this client. Un-attempted names come back marked Not attempted. so none is silently dropped from the retry input; they are excluded from telemetry, and an attempted: false entry can never be excused by the list — this submit never sent it, so the name appearing there is somebody else's write. The abort predicate is deliberately the same isIndeterminate used for excusal, so the two cannot drift apart.

2. The reconciliation is bounded and skippable, because it can only ever excuse a failure. It is raced against a 4s window: the shared query client sets no retry, so React Query's default of 3 attempts at up to the 60s client timeout each would hold the inline message for minutes on exactly the timed-out POST that needs reconciling. A window that expires credits nothing — the same conservative direction as every other branch here. It is also skipped outright when it cannot change anything: 2b. A submit whose failures are all determinate changed nothing server-side, so waiting on the list (with its retry backoff) would only delay the inline message and keep the form locked. It runs when something was added or a failure is indeterminate.

3. Offline is handled on onlineManager, not navigator.onLine. The mutation sets networkMode: 'always', because React Query's default pauses a mutation before mutationFn runs while onlineManager reports offline — mutateAsync never settles, the sequential loop hangs on its first await with the form locked and no message, and the queued POST later fires for a submit the user abandoned. Letting it run means axios rejects ERR_NETWORK at once and the caller reports it inline. The reconcile skip gates on onlineManager.isOnline() for the same reason: it is the signal React Query pauses on, so it is exactly the condition where asking would burn the window instead of answering. A client timeout is not this case — the server was reached and may have committed — so that still reconciles. Every control the settle path rewrites — the input, Add, and "Add www as well" — shares one formLocked gate, which also covers the first load of the domain list: without a pre-submit list nothing can be credited, so a POST that commits then times out in that window would invite a retry creating the second PENDING_VALIDATION row the counts machinery exists to prevent. A failed load deliberately unlocks the form — crediting stays off, but an unreachable list must not strand the page.

4. A failure is excused only by proof the submit created the domain. The refetch alone is not enough: the org-scoped list necessarily contains a name the server rejected as a duplicate, because already owning it is why it 409'd. So the handler snapshots the list before the adds and credits only names the list holds more of than it did — counts, not presence, because central-manager accepts a second PENDING_VALIDATION row for a name it already holds — and only for an indeterminate failure — one where the write may still have landed. That is anything but a 4xx refusal: no response, a 408, or any 5xx, since a 500 can be a failure after the insert. AGENTS.md:758 states the same rule for the auth forms' non-idempotent POSTs ("only 503 promises a plain retry ... each of those means the request may already have been applied", RFC 9110 §9.2.2). An unloaded snapshot credits nothing.

This is the part I got wrong repeatedly under review, so it carries the most tests. Worth naming the last one, because it is the kind of thing that ships: I had 503 as indeterminate and 500 as determinate, which is inverted — a addDomain that commits the row and then 500s would have left the domain invisible, with every retry 409ing against an empty-looking table.

5. parseDomainList lower-cases, which changes the POST payload. Previously whatever the user typed was stored; now Example.com is submitted as example.com. DNS names are case-insensitive, and the dedup is the reason: central-manager's addDomain rejects a duplicate only when the existing row is ACTIVE, so Example.com, example.com in one submit would create two PENDING_VALIDATION rows and two DNS challenges for one identity. Keeping the fold (rather than moving it to comparison-only and reopening that window) was decided deliberately, not left open — flag it if you disagree.

6. One Error Tracking event per distinct status, and none for a pure-conflict submit. A 409 is the user naming a domain the org owns and the form now says so inline, so it is never reported — a paste of a dozen owned domains would otherwise bury real signal (#1371, #1386, #1645). I first collapsed this to one event per submit, which four review rounds pushed back on with a better argument each time: a 500 on one domain and a 403 on another is a permissions regression hiding behind a server fault, and only one reached RUM. Per-distinct-status keeps both properties — twelve 409s report nothing, twelve identical 500s report once, a 500 plus a 403 reports both.

7. Two client-side caps: 20 domains, and 20 × 254 + 64 characters of raw input. Attempting every entry removed the old loop's accidental circuit breaker, and the field splits on /[,\s]+/, so a pasted log line would have fired one sequential POST per token. The length bound is sized to the domain cap at the 253-character DNS maximum, so a legitimate maximal batch never trips it and the domain cap is what refuses one — each reports its own message. Both are UI guards only; nothing server-side enforces either.

8. Failures render via form.setError('domain', …), which replaces the persistent hint. The "Type in a domain like example.com…" guidance disappears until the next submit. I think a user who just failed wants the reason where the hint was; a dedicated error line would keep both. Same call #1612 made for sign-up.

Verification

Unit + component, 47 tests across the two new suites. The component suite mounts the real DomainsManagement, the real getOrganizationDomainsQueryOptions query and the app's own mutationErrorHandler — not a restatement of it — which is what proves the skipGlobalErrorToast opt-out and the inline path together (toast.error never fires on a 409).

Every new guard was mutation-checked: reverting it individually turns specific tests red. The ones worth naming, because each was a defect found during review rather than before it — arbitrating all failures instead of only indeterminate ones (3 red), comparing against the post-submit list instead of a snapshot (1 red), treating gateway statuses as determinate (4 red), gating Add on the mutation's isPending (1 red), dropping the cap (1 red), dropping the dedup (1 red).

Two things the tests do not prove, stated plainly:

• No end-to-end run against a real /Domain/. There is no E2E layer here to defer to.
• The 409 body shape is traced, not exercised. I read it out of the other two repos rather than capturing a live response: central-manager/src/resources/domain/addDomain.js throws new ClientError('Domain already exists', ERROR_CODE.CONFLICT) (409), and harper/server/REST.ts serializes it as RFC 9457 with code = error.code ?? error.constructor.name — so code is the class name ClientError, not the status phrase, and title carries the sentence. The fixtures use that exact body. A reviewer cannot re-run this from this repo, and it matters: if a duplicate ever answered 400 instead, the !== 409 filter would console.error every expected conflict and recreate the noise this PR reduces.

Review coverage

Ten pre-push rounds. Findings went major → minor → nit; the last full round's Harper lens traced the reconciliation through every edge (unloaded list, failed refetch, already-owned 409, concurrent creation) and found it clean.

Coverage was degraded throughout and should not be read as ten rounds of breadth:

• codex (graded) failed in 6 of 10 rounds — SIGKILL on timeout, or exit-1. Only rounds 4 and 7 have graded coverage.
• Cursor legs are capped at 2 rounds per branch, and in the rounds they did run they reviewed src/features/auth/* — files this branch does not touch — so the adjudicator dropped all of their findings in three separate rounds. Their ok status is not evidence they looked at this diff.
• Real, consistent coverage is gemini + the Harper adjudication leg.

Rejected on evidence rather than silently: gemini's describeError-may-throw (errorText guards every branch including JSON.stringify), and its major that the hook-level skipGlobalErrorToast breaks other consumers — the hook has exactly one consumer, and useSignUp, useForgotPassword and useCloudSignIn all hardcode the same meta, so it is the established shape here.

Filed separately, not fixed here

• Sign-up reports expected 409s to Error Tracking — 47 conflicts in 30 days became RUM errors for text the form already shows inline #1688 — useSignUp reports every rejection to Error Tracking; POST /User/ 409'd 47× in 30 days, all becoming RUM errors for text [RUM] Half of sign-up submissions rejected 409 with no inline feedback — 7 of 8 affected sessions never created an account #1612 made inline copy. This PR sets the opposite norm on a sibling form. Comment on it also records useForgotPassword having the inverse bug (no mutation-level onError, so a 503 during unmount reports nothing) and measures the CAPTCHA-noise concern raised alongside it at 0 events in 30 days.
• vitest exits 1 with all 2955 tests passing — an undici WebSocket dispatches a Node Event into the jsdom realm after teardown #1689 — vitest exits 1 with all tests passing, ~1 run in 3: undici's WebSocket dispatches a Node Event into the jsdom realm after teardown.
• [RUM] shouldKeepEvent's isInstanceEndpoint branches are dead: this SDK never emits source:network errors (0 in 30d vs 592 direct status-0 resources) #1690 — the RUM SDK Studio ships emits no source:network errors at all, so shouldKeepEvent's URL-based gates are dead code. Relevant here because two reviewers and an adjudicator assumed on an earlier PR that a failed instance request reaches Error Tracking as a resource error; it does not, and console.error is the only route.
• Add Domain button re-enables between sequential adds, so a double submit can report a false "already exists" #1687 — closed, not fixed. I filed it as pre-existing; a later round proved the window is new (the old code called form.reset() before the refetch, mine reordered them), so it is fixed here instead. The comment explains the misclassification.

Closes #1685

Complexity: complicated

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

_{Human-Review-Need: 3 (decisions: count-based-crediting-vs-server-idempotency, bounded-4s-reconcile-vs-awaiting-refetch, unlock-form-when-list-load-fails, networkmode-always-on-this-mutation, max-20-per-submit-cap) @ 0765c0e}

[gemini-code-assist]

Code Review

This pull request introduces sequential domain addition logic to handle multiple domains, manage failures, and reconcile results against a refreshed list, with comprehensive test coverage. It refactors the DomainsManagement component to utilize these utilities, improving error handling, form state management, and offline resilience. A review comment points out that deduplicating errors solely by HTTP status in reportUnexpectedFailures could silently swallow critical client-side errors with an undefined status, and suggests using a more specific deduplication key like the error message.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	61.16%	8390 / 13716
🔵	Statements	61.59%	8986 / 14590
🔵	Functions	53.97%	2118 / 3924
🔵	Branches	54.87%	6013 / 10958

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/features/cluster/domains/Management.tsx	67.9%	76.47%	56.25%	68.83%	86-87, 94-107, 188-214, 222, 228-229
src/features/cluster/domains/addDomainsSequentially.ts	100%	96.77%	100%	100%
src/features/organization/mutations/addDomainToOrganization.ts	100%	100%	100%	100%
src/lib/errorStatus.ts	100%	100%	100%	100%
src/react-query/pollUnlessForbidden.ts	100%	100%	100%	100%

Generated in workflow #1867 for commit 0765c0e by the Vitest Coverage Report Action