Repository navigation
feat(config): fan cluster config saves out with replicated set_configuration on Harper 5.2+ - #1861
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request optimizes the rolling configuration update process for Harper clusters (version 5.2.0+). Instead of writing the configuration to each instance individually, it writes and restarts a canary instance, then fans out the configuration write to the rest of the cluster via replication. Subsequent instances skip their local configuration write if the replication fan-out successfully acknowledged them. The PR also adds comprehensive unit tests and design documentation. Feedback on the changes suggests a minor optimization in the update loop to break early when the operation is canceled.
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This was referenced Oct 11, 2026
Merged
Merged
dawsontoth
added this pull request to stack #1908
October 11, 2026 21:27
…uration on Harper 5.2+ Saving the config editor on a cluster wrote set_configuration to every RUNNING instance from the browser, one at a time, before restarting it, and never wrote to members it did not restart. Harper 5.2.0 added `replicated: true` to set_configuration (harper#1556), which applies the write on every replication peer in one call. It does not apply the config: each node still needs its restart, so the one-at-a-time roll stays. On a cluster with more than one live member, all reporting 5.2.0 or later, the first instance is still written and restarted on its own. Once it is back up on the new config, the write fans out from it with `replicated: true` -- also when it was the last instance to restart, so STOPPED or failed members still get it. A later instance skips its own write only when the response acknowledged that exact node: a success entry carrying the peer's `message` string and a `node` equal to the instance's replicationFqdn (the name Fabric gives each Harper node). Pre-5.2, mixed-version and unknown-version clusters, per-instance views, a rejected fan-out, and any unacknowledged target keep the per-instance write. After a fan-out, the Cancel and partial-failure toasts say instances that were not restarted may already have the new config saved. Also: a failure loading the cluster's instances left Save disabled and the loading toast spinning, with an unhandled rejection; it now reports that nothing changed and clears the pending state. A cluster with no instances reported Success; it now reports that none were found. Closes #1339 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kipping each target Cancellation never unsets, so every remaining iteration only re-checked the flag. Refs #1339 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…of staying disabled useRestartClusterClick awaited getClusterInfo, and built the per-instance clients, outside any try. When either failed, the click handler rejected unhandled: the Restart button stayed disabled until remount, the "Restarting" toast spun until its 60s duration ran out, and the rejection reached the console as an unhandled one. Mirror useRollingConfigUpdate's handling from #1861: load the restart targets inside a try, end the restart toast with "Could not load the cluster's instances, so nothing was restarted.", and clear the pending state in a finally. Both hooks built their targets the same way (fetch the cluster, check Fabric Connect, keep RUNNING instances, build a client each, reverse), so that now lives in one getClusterRestartTargets helper. Also stop reporting an empty cluster as "Cluster fully restarted!", the same zero-instance fix #1861 made to the config update. Closes #1853 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The restart, config-update and apply-licenses loops only looked at their Cancel flag at the top of each iteration. A Cancel clicked while an instance's availability check was in flight was therefore ignored for that instance: the restart, the config write, or the license install went out anyway. Recheck the flag once the check returns, before the first request that changes the instance. Also stop telling the user no instance was RUNNING when every RUNNING instance failed to restart. Both cluster hooks keyed that message on nothing having restarted; key it on there being no RUNNING target, so a cluster whose restarts all failed reads "Only 0 RUNNING instances were restarted of N". Found by the pre-push cross-model review of the previous commit; both predate it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s during the check Cancellation never unsets, so continuing only re-checked the flag on every remaining target. Refs #1853 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dawsontoth
force-pushed
the
claude/1339-replicated-set-configuration
branch
from
October 12, 2026 21:19
3581be6 to
411e292
Compare
dawsontoth
marked this pull request as ready for review
October 12, 2026 21:23
dawsontoth
added a commit
that referenced
this pull request
Oct 12, 2026
…of staying disabled useRestartClusterClick awaited getClusterInfo, and built the per-instance clients, outside any try. When either failed, the click handler rejected unhandled: the Restart button stayed disabled until remount, the "Restarting" toast spun until its 60s duration ran out, and the rejection reached the console as an unhandled one. Mirror useRollingConfigUpdate's handling from #1861: load the restart targets inside a try, end the restart toast with "Could not load the cluster's instances, so nothing was restarted.", and clear the pending state in a finally. Both hooks built their targets the same way (fetch the cluster, check Fabric Connect, keep RUNNING instances, build a client each, reverse), so that now lives in one getClusterRestartTargets helper. Also stop reporting an empty cluster as "Cluster fully restarted!", the same zero-instance fix #1861 made to the config update. Closes #1853 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dawsontoth
added a commit
that referenced
this pull request
Oct 13, 2026
…fig fan-out After the rebase onto #1861's rolling config update, the stop rule also decides the fan-out: a canary that is not back never proved it can run the new configuration, so it is never fanned out from, and a stop after the fan-out keeps the note that the members left alone may already have the configuration saved. Both are pinned in useRollingConfigUpdate.test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dawsontoth
added a commit
that referenced
this pull request
Oct 13, 2026
…fig fan-out After the rebase onto #1861's rolling config update, the stop rule also decides the fan-out: a canary that is not back never proved it can run the new configuration, so it is never fanned out from, and a stop after the fan-out keeps the note that the members left alone may already have the configuration saved. Both are pinned in useRollingConfigUpdate.test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dawsontoth
added a commit
that referenced
this pull request
Oct 13, 2026
…fig fan-out After the rebase onto #1861's rolling config update, the stop rule also decides the fan-out: a canary that is not back never proved it can run the new configuration, so it is never fanned out from, and a stop after the fan-out keeps the note that the members left alone may already have the configuration saved. Both are pinned in useRollingConfigUpdate.test. Co-Authored-By: Claude Opus 5.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
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.
📦 This PR now carries #1867 too
Rebased into 5 linear, signed commits on current
stage, with no merge commits and no conflicts: #1861 (fan cluster config saves out with replicatedset_configurationon Harper 5.2+, Closes #1339) and #1867 (Restart Cluster reports a failed cluster load instead of staying disabled, and Cancel during an instance check is honored, #1853). Both PRs' changes are identical to what was reviewed. Gate on the final tree:tsc -b, oxlint,dprint check, vitest 481/481 files. Overlaps #1899, which reworks the same restart loops; whichever lands second rebases.⊙ Problem
Saving the config editor on a cluster writes
set_configurationto every RUNNING instance from the browser, one at a time, right before restarting it. Instances Studio does not restart (STOPPED, or failing their health check) never get the write and silently keep the old config. Harper can now fan the write out itself withreplicated: true(HarperFast/harper#1556). That landed in 5.2.0, not "v5+" as the issue says: it is in no 5.0.x or 5.1.x tag. The write still applies nothing until each node restarts, so the one-at-a-time restart roll has to stay.💡 Solution
On a cluster where every live instance reports Harper ≥ 5.2.0 and at least one other live member exists, the roll changes in one place:
set_configurationwithreplicated: truethrough that instance's client. This happens even when it was the last instance to restart, so live members the roll skips still get the write.replicated[]acknowledged that exact node. Fabric names each Harper node after its CMreplicationFqdn(host-manager setsNODE_HOSTNAMEandREPLICATION_HOSTNAMEto it). Each successful entry carries the peer's ownmessageplus thenodeharper-pro'sreplicateOperationattaches.restart_servicecall and recovery wait as before.Every other case keeps the per-instance write: pre-5.2, mixed-version, or unknown-version clusters; per-instance views; local Studio; a rejected fan-out (the instance client already turns any failed peer into a rejection); a response with no usable
replicated[]; and a target with noreplicationFqdn.Also fixed (same hook):
useRestartClusterClickis filed as Restart Cluster stays disabled and its toast spins when the cluster fetch fails #1853.⚖️ Alternatives
config/configUtils.ts:1320-1324).restart_servicewithreplicated: true). Rejected: it also writes everywhere before any restart. It runs as a job Studio does not poll, which would drop per-instance progress and Cancel. And it is no stronger at reporting a peer it could not reach (bin/restart.ts:360-364).🔧 Changes
src/hooks/useRollingConfigUpdate.tsx(best read with whitespace ignored, since the body moved into atry):planRollingConfigUpdatebuilds the restart targets and theirreplicationFqdns, as the inline code did before. Also: the fan-out with its rejection mapped to "acknowledged nobody", the per-target write skip, and the shared "may already be saved" note used in the Cancel copy and the partial-failure copy once a fan-out was attempted.src/integrations/api/instance/status/setConfiguration.ts:shouldFanOutSetConfigurationrequires more than one live member, with every live member on ≥ 5.2.0. Terminating, terminated, and removed instances are ignored. A missing version, or a 5.2.0 prerelease, counts as unsupported.src/integrations/api/replication.ts: addsacknowledgedReplicationNodes, and widens a success entry'smessagetype to the string thatset_configurationpeers actually send.DESIGN.md: records the invariant: when an instance may skip its own write, and why a count is not proof.✅ Verification
End-to-end route: not verified against a live Harper 5.2+ cluster. No cluster harness is available here, and port 5173 belongs to another session. The Harper side is traced from source, cited above. Mechanism-level evidence:
src/hooks/useRollingConfigUpdate.test.tsx(new, jsdom) drives the real hook with mocked transport. It covers: the canary → fan-out → acknowledged-skip order; strictly one restart in flight, held open with deferred promises; a stale peer standing in for a target, which still gets written; a missingreplicationFqdn; a rejected fan-out, where the canary is still counted and no "Failed" toast appears; fan-out deferred past a failed first instance; fan-out after the last restart; both Cancel copies; mixed, missing, prerelease, and pre-5.2 versions never sendingreplicated; the per-instance view; an empty cluster; and a failed cluster load.src/integrations/api/instance/status/setConfiguration.test.ts(new) pins the gate, including exactly 5.2.0 and a single live member.src/integrations/api/replication.test.tscovers acknowledgements: lower-casing, failed peers, and malformed shapes (no array, node-only entries, and non-string, empty, or boolean messages).origin/stage's hook swapped in, 8 of the 16 hook tests failed at the time, including the failed-cluster-load test, and the legacy-path tests passed.vitestsuite (397 files, 3699 passed, 11 skipped), plusoxlintanddprint check --staged. Separately,npx tsc -bexited 0.Cross-model review: three rounds (codex graded + gemini each round), after a planning round. Cursor legs could not run, because the 1Password SSH agent refused to sign their
git fetch. The Harper-domain adjudicator could not run either: its Claude OAuth session had expired. So outside findings were triaged by hand. I rejected gemini's claim thatwasAReleasedBeforeBis a strict comparison: it returnsb >= a, and a test pins exactly 5.2.0. Its comment nits pointed at pre-existing comments that only moved under the newtry.gemini-code-assist feedback: took its one comment in 3581be6, so a cancel now
breaks out of the update loop rather thancontinue-ing through the remaining targets. The flag never resets, so behavior is unchanged and the cancellation tests still pass. No further review round ran.Closes #1339
🤖 Generated with Claude Code
🤖 Generated by Claude Code (Anthropic Claude); posted via @dawsontoth.
Related PRs: #1867 overlaps (stacked on this branch; replaces its target planning with a shared helper), #1899 overlaps (reworks the same config-update loop in
useRollingConfigUpdate.tsx)Complexity: complicated
Review-Coverage: authored=claude; ran=gemini,codex; blocked=cursor-grok(no-receipt),cursor-composer(no-receipt),domain(auth); declined=cursor-kimi,cursor-muse; rounds=3; full=1 @ 3581be6
Review-Attention: study ~20m (critical: replication.ts; raised: degraded review) @ 3581be6