Unbinding one custom domain no longer turns off certificates for the others

[dawsontoth]

📦 This PR now carries the whole custom-domains stack

The stacked PRs below were merged into this branch, rebased into 9 linear, signed commits on current stage (no merge commits):

PR	Change	Issue
#1801	unbinding one domain no longer turns off certificates for the others	Closes #1626
#1802	certificate progress polls fast only while a certificate is generating	#1630 (closed)
#1840	Bind sends only the domains still offered for binding	#1800 (closed)
#1846	a refused domain change or cluster edit explains why on the page, and is pre-checked	Refs #1686 (root cause needs central-manager logs)
#1863	the Remove Domain tooltip opens, so a disabled Remove says why	Closes #1843

Rebase: every PR's changed lines are identical to what was reviewed in its own PR. #1846's own commit predated #1840's later fix to the same lines (invalidate via activeQueryClient); as in the earlier propagation merge, those hunks resolve to #1846's version, which already had the change. Gate on the final tree: tsc -b, oxlint, dprint check, and vitest 444/445 files. The one failure is rememberedClusterDeepLink.test.ts timing out under machine load; it passes alone and is unrelated to domains. A separate PR is fixing it and the other flaky auth-stack router test.

---

Closes #1626

⊙ Problem

Unbinding one custom domain sent generateDomainCerts: false with the remaining domain ids. The flag is cluster-wide, so unbinding one domain turned certificate generation off for every domain still bound. Since HarperFast/central-manager#669, central manager also stores that flag on the Cluster row, and later expansions read it there, so new nodes stop getting certificates for the domains that are still bound (#1626).

Every domain change also sends the whole id list, built from the cached cluster, and that list could be stale. Unbinding one domain could then silently unbind a domain bound earlier on the same page, or re-bind one just unbound.

❓ Your call: should unbinding the last domain still send false? I omit the flag on every unbind: unbinding says nothing about cert intent, with no domains bound the flag has nothing to act on, and the next bind from Studio sends true anyway. Sending false only when the list becomes empty is a one-line conditional if you'd rather clear intent there.

💡 Solution

Unbind no longer sends generateDomainCerts. Central manager reads an absent flag as unchanged (resolvedCertIntent = generateDomainCerts ?? existingCluster.generateDomainCerts in its updateCluster, merged in #669). A central manager without #669 reads an absent flag as false, exactly what Studio sent before, so the change is safe whichever one is deployed. Bind still sends true.

The id list was stale in three ways, each fixed here:

• the unbind callback's deps were [deleteDomain], so it kept the cluster from the first render (now [setDomainIds, cluster]);
• nothing refreshed the cluster query after a change: callers invalidate [organizationId], which doesn't match the cluster query's [clusterId] key, so a second change inside the 10s poll resent the old list. On success the mutation now writes the accepted list into the cluster cache (central manager stores domainIds exactly as sent), then re-reads it, so a failed re-read can't leave the old list behind;
• bind and unbind used separate mutation instances, so either could start while the other was in flight. All domain actions now share one pending state, and the mutation itself refuses to start while another is in flight, because the disabled state only lands a render after the first click.

⚖️ Alternatives

• Send the cached cluster.generateDomainCerts on unbind (the issue's first suggestion): it has the same effect against fix(deps): update tanstack-query monorepo to v5.87.4 #669, but it writes back a value read earlier, which may be stale if another tab changed it. Omitting the flag lets central manager use the value it has stored.
• Wait for the cluster re-read before settling the mutation instead of writing the accepted list into the cache: a failed GET would still settle with the old list (round 2 of the pre-push review caught this).

🔧 Changes

• setDomainIdsOnCluster.ts: generateDomainCerts is optional and documented as "omit to leave unchanged". The hook writes the accepted domainIds into the cluster cache and re-reads it, takes a shared mutation key, and refuses to start while one is in flight. useIsSettingDomainIds exposes the shared pending state.
• tableDefinition.tsx: unbind omits the flag and depends on the current cluster. The unused bindDomainToThisCluster is removed. Row actions disable on the shared pending state, and deleteDomainById's deps now include the org id it reads.
• Management.tsx: the Bind button disables on the shared pending state, not just its own mutation's.

✅ Verification

End-to-end route: not observable end-to-end here. It needs a cluster with bound custom domains and a central manager, and clicking Unbind changes real infrastructure. Verified at the request boundary instead: Management.test.tsx renders the real DomainsManagement against a mocked apiClient and asserts the serialized PUT body (JSON.stringify, as axios sends it).

• Management.test.tsx, new "binding and unbinding" block:
  • unbind omits generateDomainCerts;
  • unbind uses the current cluster, not the one from first render;
  • a second change builds on the list the first one left, both when the cluster re-read succeeds and when it fails;
  • all domain actions hold while one is in flight;
  • a click queued before the controls disable is refused;
  • bind still sends generateDomainCerts: true.
• Fails on base: the first two failed on origin/stage with the bug signatures (generateDomainCerts: false sent; stale domainIds: [] that would have unbound dom-b too). Each later guard was mutation-checked: with it reverted, its test goes red ({ domainIds: ['dom-a'] } re-bound; Bind still enabled; two PUTs sent).
• Gates on the final head: npx vitest run 395 files / 3,670 passed (exit 0), npx tsc -b exit 0, npx oxlint --format stylish . exit 0, npx dprint check exit 0. Run with Node 24.21.0, and again by the pre-commit hook.

Pre-push review: three full rounds (codex graded, gemini). The Harper-domain adjudicator failed on auth every round, and the Cursor legs failed on the 1Password SSH agent, so no round converged. Every round-1/2 finding I accepted is in this diff. From round 3 I rejected:

• "use the PUT response to update the cache": the domain branch returns { id, domains, instancesUpdated }, not a Cluster;
• "scope the pending key per cluster": the hold lasts only one in-flight request.

The queued-click guard was added after round 3, at the three-round cap, so no review leg has seen it. Out of scope and filed separately: a ticked domain that is then deleted is still sent by Bind (#1800). The server-side dangling-id theory in #1686 is the same whole-list fragility, but this PR does not touch it.

_{🤖 Generated with Claude Code (Claude Opus 5.5); posted via @dawsontoth.}

Related PRs: none found

Complexity: medium

_{Review-Coverage: authored=claude; ran=gemini,codex; blocked=cursor-composer(no-receipt),domain(auth),cursor-muse(no-receipt); declined=cursor-grok,cursor-kimi; rounds=3; full=1 @ 93a6a46}

_{Review-Attention: study ~9m (raised: degraded review, open major) @ 93a6a46}

[gemini-code-assist]

Code Review

This pull request improves the domain binding and unbinding flow on clusters by preventing race conditions, handling optimistic cache updates, and preserving certificate intent when unbinding domains. It introduces a useIsSettingDomainIds hook to disable controls during mutations and adds comprehensive tests for these scenarios. The review feedback suggests optimizing the unbindDomainById callback in tableDefinition.tsx by narrowing the query invalidation key to prevent over-fetching and destructuring the cluster dependency array to avoid unnecessary recreations.

[gemini-code-assist]

There are two opportunities for improvement in unbindDomainById:

1. Query Key Specificity: Invalidating [cluster.organizationId] is too broad and can trigger unnecessary refetches of other organization-scoped queries (e.g., organization details, members, billing). Using [cluster.organizationId, 'domains'] is more precise and matches the invalidation key used in deleteDomainById.
2. Dependency Array Optimization: Depending on the entire cluster object causes the callback to be recreated on every poll/refetch of the cluster data, even if the relevant fields (id, domainIds, organizationId) haven't changed. Depending on these specific properties directly prevents unnecessary recreations.

Suggested change

	const unbindDomainById = useCallback((domainId: string) => {
	// No `generateDomainCerts`: the flag is cluster-wide, so restating it here would decide cert
	// intent for every domain that stays bound.
	setDomainIds({
	clusterId: cluster.id,
	domainIds: (cluster.domainIds?.slice() || []).filter((id) => id !== domainId),
	generateDomainCerts: false,
	domainIds: (cluster.domainIds ?? []).filter((id) => id !== domainId),
	}, {
	onSuccess: () => {
	void queryClient.invalidateQueries({ queryKey: [cluster.organizationId], refetchType: 'active' });
	toast.success('Domain unbound!');
	},
	});
	}, [deleteDomain]);
	}, [setDomainIds, cluster]);
	const unbindDomainById = useCallback((domainId: string) => {
	// No `generateDomainCerts`: the flag is cluster-wide, so restating it here would decide cert
	// intent for every domain that stays bound.
	setDomainIds({
	clusterId: cluster.id,
	domainIds: (cluster.domainIds ?? []).filter((id) => id !== domainId),
	}, {
	onSuccess: () => {
	void queryClient.invalidateQueries({ queryKey: [cluster.organizationId, 'domains'], refetchType: 'active' });
	toast.success('Domain unbound!');
	},
	});
	}, [setDomainIds, cluster.id, cluster.domainIds, cluster.organizationId]);

[dawsontoth]

Declining both, on evidence:

1. The broad key is deliberate. Unbinding changes the cluster's domains, and the org query keyed [orgId] (getOrganizationQuery.ts:27) carries clusters[].domains (central manager src/models/schema.graphql: Organization.clusters → Cluster.domains @relationship(from: domainIds)). The clusters list renders that as each card's FQDN (ClustersList.tsx:51 → ClusterCard.tsx:197), so [orgId, 'domains'] would leave it stale. [orgId] also prefix-matches [orgId, 'domains'], so the list on this page refreshes too. Bind invalidates the same key (Management.tsx:106), and this line is unchanged from stage. Delete can use the narrow key because Remove is disabled while a domain is bound (tableDefinition.tsx:98), so no cluster changes when it runs.
2. cluster comes from useSuspenseQuery (Management.tsx:50). Its structural sharing keeps the same reference across polls that return the same data, and the columns useMemo depends on cluster itself (tableDefinition.tsx:175). Narrowing this callback's deps would not save a single recompute.

_{🤖 Generated by Claude Code (Claude Opus 5.5); posted via @dawsontoth.}

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	72.24%	11605 / 16063
🔵	Statements	72.35%	12367 / 17092
🔵	Functions	66.67%	3061 / 4591
🔵	Branches	67.72%	9066 / 13386

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/features/cluster/domains/Management.tsx	81.05%	88.13%	85%	82.41%	99, 126, 234-260, 268, 274-275
src/features/cluster/domains/canBindDomain.ts	100%	100%	100%	100%
src/features/cluster/domains/constants/tableDefinition.tsx	92.5%	87.09%	91.66%	94.73%	127, 160, 173
src/features/cluster/domains/queries/getChallengeCertificates.ts	94.44%	91.66%	100%	100%	51
src/features/clusters/hooks/useUpdateCluster.ts	83.33%	100%	100%	83.33%	18
src/features/clusters/mutations/describeDomainChangeRefusal.ts	100%	90.9%	100%	100%
src/features/clusters/mutations/setDomainIdsOnCluster.ts	100%	90.9%	100%	100%
src/features/clusters/upsert/ClusterBilling.tsx	78.57%	52%	100%	78.57%	92-106
src/features/clusters/upsert/ClusterDetails.tsx	84.84%	56.36%	100%	83.87%	74-79, 83-88, 123-124, 195-264
src/features/clusters/upsert/ClusterForm.tsx	52.53%	30.72%	60.86%	53.33%	109-118, 123-127, 134-166, 210, 213-226, 230, 233, 262-271, 280-283, 294-297, 339-372, 382-389, 393-398, 411-441, 461-462, 468, 472
src/features/organization/mutations/deleteDomain.ts	100%	100%	100%	100%
src/features/organization/queries/getOrganizationDomains.ts	100%	100%	100%	100%

Generated in workflow #2251 for commit d56bf21 by the Vitest Coverage Report Action