feat(wizard): name coordinators and recover failed setup - #1970
Merged
Conversation
Collaborator
Author
|
@/tmp/pithead-pr-1970-review.md |
…d can leave the raffle build_config now writes tari.mode EXPLICITLY on every submit. That is the one place this function departs from its own omit-and-inherit rule, and it departs on purpose: a config with no tari.mode still parses as "local", so an omitted key would quietly merge-mine on a machine whose operator answered No. The parser default is right only for configs written before the question existed. A machine that declines Tari is asked for no payout address — the key is omitted rather than sent empty, since an empty string is a value that would override the documented default. When Tari IS on, an empty address still flows through so the HOST produces the rejection, exactly as it does for Monero. #1848: xvb.enabled is true in config.reference.json, so only the OFF answer is written. Writing True would pin a default the operator never chose to pin. Two existing tests asserted the law this overturns, both now corrected in place rather than deleted, each saying what it used to claim: - test_wizard.py's "tari.mode is local|remote only — there is no Monero-only mode" (line-neutral; that file is at its 974-line ceiling) - test_wizard_form.py's "mode" not in cfg["tari"], which stays narrowly about the remote block it was written to guard Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR (cherry picked from commit e94c009)
…and whether to join the raffle The host half (#1905) taught the stack `tari.mode: "off"` and the no-JS form to write it. The page an operator actually sees could not say it: the select was `remoteTari ? "remote" : "local"`, two states for what is now three. A machine holding "off" rendered as "Run the bundled node on this machine" — the wizard misreported the machine to its own operator — and one touch of that control wrote a yes back. There was no way to answer No at all, which is the finding. The question is now "Merge-mine Tari?", No first and default, and the Tari payout address moved under it. It had to move: the field carried `required`, so left in the Payout addresses section it would have blocked submit on a form that never asks the question. It keeps `required` behind a yes — the same bar the Monero address holds. tariAnswer is the migration rule, and it reads the direction the host reads it. Only the literal "off" is a decline; a missing key is `local`, because that is what 28-parse-and-validate-config.sh does with a config written before this question existed. Reading an absent key as "off" would tell an upgraded 1.x machine it had declined merge-mining and write that decline back on submit. note lifted from the FAQ rather than invented. `xvb.enabled` is true in config.reference.json, so the default answer is the state the machine is already in. Both go in a new module: wizard.mjs was 884/889 and wizard.test.mjs 765/765, so the budget decided the design, as it did for #1850 and #1853. wizard.mjs comes out 21 lines lighter. Four existing render probes asserted the old heading and the retired Tari question. Corrected in place and line-neutral (765/765 holds). The three `doesNotMatch(/Payout addresses/)` among them mattered most: left alone they would have passed over a rendered "Payout address" forever. Prose the change falsifies, swept for rather than remembered: README's "every hash merge-mines Tari", appliance.md's "Required, like the Monero one: this stack always merge-mines both coins", and the two dev recipes whose M6 says to paste both addresses into a form that now asks for one. NOT done, and not mine: `./pithead setup` (24-config-wizard.sh) still demands a Tari address and never asks the mode, so the CLI path cannot decline. Filed separately. appliance.md says a remote Tari node saves ~200 GB while the wizard says a local one adds ~170 GB; both predate this change and I did not reconcile them by guessing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR (cherry picked from commit 0393a12)
… the decline Found by re-reading the KVM provision leg #1847 just merged, not by the tests: it posts back whatever `/api/wizard-state` served it, so I went to check what that is for a fresh box. It was `tari.mode: "local"`. The commit before this one gave the page three answers and put No first. That is not the same as No being the answer a new machine arrives holding. The state API builds the page's config as the reference merged under the last attempt, and for a machine with no attempt it fell through to a seed carrying only `local_miner.enabled`. So the reference decided, the reference says `local`, and every new operator would still have opened the wizard on "Yes — run a Tari node on this machine". The whole finding survives a correct component. The seed gains `"tari": {"mode": "off"}` and gets a name, because what it is FOR is now load-bearing: it is the page's answer for a machine that has none, and it is reached only through the `or`. The reference keeps saying `local` and must — it is the default for a config that ALREADY EXISTS, and moving it there would stop merge-mining on every upgraded install. Those two facts look contradictory and are the design; the comment says so where the constant lives. The tests go in a new sibling file for the reason its three siblings give: test_wizard.py is at its 974-line ceiling. They pin the served value rather than the component, and the pair that matters is the migration one — a last attempt with NO tari key at all is still a machine WITH answers, so it gets the reference's `local`, never the new machine's decline. The raffle needs no seed: #1848 is opt-out and the reference already says true. Asserted by moving the reference and watching the served value follow, since a value that happens to equal the reference proves nothing about its source. ⛔ THIS BRANCH MUST NOT MERGE BEFORE #1905. `28-parse-and-validate-config.sh:78-81` on `origin/develop` today is `local|remote` and errors on anything else, so a tree with this and without the host half refuses to provision ANY new machine — including #1847's browser-submit leg, which posts the served config whole. Told the appliance lane; it has recorded the order. PROVEN at this tree: 2582 passed / 0 failed, coverage 97.58% (make test-dashboard); the six wizard modules 132 passed settled from --junitxml; ruff lint+format, file-budget (wizard.py 645/674), pithead parity. A 4-mutation battery on the seed, each proven applied and restored: reverting to the old seed, answering `local`, forcing the seed over an existing attempt, and pinning the raffle each reddened the row that names them, none survived. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR (cherry picked from commit c04d122)
…tead of a view that cannot change it The second reviewer's RETURN, taken. Two sentences this branch introduced told the operator they could turn merge-mining on later from the dashboard's Configuration view. They cannot. Re-derived at source rather than taken on the reviewer's word — all three legs hold: - `control_service.py` builds the editable set from EDITABLE_ENV_KEY_PATHS, whose only tari entries are `dashboard.tari_required`, `tari.mem_limit`, `tari.data_dir` and `tari.clearnet_initial_sync`. There is no `tari.mode`, so the field renders greyed. - The host agrees independently: no TARI_MODE in CONTROL_DASHBOARD_EDITABLE_KEYS or the confirm map (`42-control-policy-and-host-checks.sh:61-79`), and the policy is default-deny. - #1905 adds TARI_MODE to `.env` (`33-render-env.sh:470`) and adds it to NEITHER list — I checked both rather than assuming the host half covered it. The capability is real, only the route named was wrong: a set-up-again boot re-runs the wizard seeded from the live config (`12a-setup-again.sh:26-28`), so the select opens on the stored value. Both sentences now say that, following the phrasing `docs/appliance.md:250-254` already uses for the other host-only switches. NOTHING GUARDED THIS — no test asserted either sentence, which is why a false claim about another subsystem's editability shipped. Added one, scoped to TariSection ON PURPOSE: the XvbField sibling says "Changeable later" and that is TRUE (XVB_ENABLED -> xvb.enabled), so a needle swept over the whole form would have pinned the honest row. The needle forbids the PROMISE, not the phrase — the correct copy names the Configuration view too, in order to say it does not carry this switch, so a bare /Configuration view/ needle reddens on the fix as readily as on the defect. Proven: mutation control — reverting the copy to the old sentence reddens exactly this guard, restored by sha256. Frontend suite 594 passed rc 0; lint-js, lint-docs-voice, lint-operator-strings all rc 0, the last run AFTER staging. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR (cherry picked from commit 30c8ddf)
…s hiding A machine booted through the boot menu's "Set up again" entry opened on two choices — Keep it and Set up again — and mentioned restoring from a backup nowhere. The operator most likely to boot that entry is the one whose machine went wrong and who is holding a backup archive, and they were shown the one screen in the product that did not offer it. To reach restore they had to press "Set up again", which reads like "start over and retype everything", and then notice a text link above the form. This is a door, not a feature. Restore-at-setup shipped under #909, and a restore submitted from a set-up-again boot lands today: 12-firstboot-wizard.sh:129 skips the wizard only when `! setup_again_mode`, and restore_apply refuses on size, magic, passphrase, integrity, unsafe paths and an unusable config — never on the machine already having a configuration. Both re-derived at source and posted on the issue. No host change. The whole change is in savedrole.mjs. renderSetup()'s own first line dispatches restoreMode to renderRestore(), so letting the saved-role screen fall through on restoreMode — the same way it already does on setUpAgain — reaches the existing branch with no new route and no line spent in wizard.mjs, which this lane does not touch. Not confirm-gated, deliberately: the restore form is itself a file, usually a passphrase and a second button, which is a more informative wall than a modal. What the operator needs is WHAT it replaces, so the note beside the choice names it — this machine's configuration and secrets, its onion address among them. The new control is a plain button rather than another link-styled one, so it does not add a second sub-24px target to a screen where the design lane has already recorded one (#1868). The over-engineering pass found one real defect in this diff and it is fixed here: restore is the first choice on this screen with a way BACK to it (the restore form's own Back clears restoreMode), so a failed Keep left in `keepError` would have been waiting on the operator's return, reporting an action they had since abandoned. Cleared on that exit, and asserted. Tier 3 (node), as rows in the existing savedrole.test.mjs. One existing row's name said the screen "offers both answers" — this change makes that false, so it is updated with the count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZuzdtxNX8ae4rnDtBqEqR (cherry picked from commit 84b9462)
Publish the install trigger only after protocol checks and candidate writes complete, bind accepted node names to the vetted numeric address, require a complete ZMTP greeting and READY exchange, and validate the monerod get_info schema. Addresses the independent security RETURN at 46cd61d without rewriting prior history.
Resolve each target once, reuse that address across its protocol checks, and require the ZMTP peer to declare PUB or XPUB. Adds adversarial controls for split DNS and a READY-speaking SUB peer after the renewed security RETURN.
VijitSingh97
changed the title
Satisfy the merged wizard-shell contract with the existing secondary button skin. The full frontend suite now covers the composed PR1927 interaction.
This reverts commit 2d5c991.
VijitSingh97
changed the title
Collaborator
Author
|
Author relay of independent hostname-extension review at full head
Author checks: Node 56/0, Python 126/0 with JUnit, hostname shell 23/0, existing identity/boot unit Held for combined integration, including the Configuration companion, host #1952/#1969, |
Collaborator
Author
|
Independent review record for additive head Author validation: 23 cases in a dashboard-only tree, a named-default removal control that fails as intended, and nine label-extractor checks. Current CI and the previously recorded config/host companion and KVM conditions remain outstanding. This comment relays the independent reviews; the author is not the merge seat. |
Collaborator
Author
|
Independent review relay for exact head
Evidence: 129 frontend wizard tests passed; the focused Python wizard suite passed; Linux hostname/spool/setup/recovery domains passed 270/0; full-history gitleaks scanned 1,736 commits clean; syntax, shfmt, targeted shellcheck, file budget, parity, and diff checks passed. This comment relays independent task reviews; it is not an author self-approval. |
This was referenced Sep 7, 2026
This was referenced Sep 7, 2026
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
1 participant
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.
Closes #1954.
Closes #1955.
Closes #1957.
Addresses #1855, #1848, #1889, #1943, #1946, #1923, and #1928.
What
The configuration companion and #1952 host protocol are now both integrated in
develop. This branch was updated by merge without rewriting its published history.Verification
e8aaa93e0971376653e68434c95b609724d01d9cNo live hostname, mDNS, served-certificate, OS-update, KVM, or public-release claim is made here; those remain appliance/manual-release evidence, not this merge gate.