Skip to content

feat(wizard): #1855 #1848 the setup page can decline merge-mining, and a new machine arrives declining it - #1919

Open
VijitSingh97 wants to merge 4 commits into
developfrom
feat/1848-wizard-tari-and-xvb
Open

feat(wizard): #1855 #1848 the setup page can decline merge-mining, and a new machine arrives declining it#1919
VijitSingh97 wants to merge 4 commits into
developfrom
feat/1848-wizard-tari-and-xvb

Conversation

@VijitSingh97

Copy link
Copy Markdown
Collaborator

The setup page can finally answer No to merge-mining, and a new machine arrives holding that answer. This is #1855's wizard half plus #1848, and it completes the operator's finding that a fresh box showed Tari coming up for someone who never asked for it.

This must not merge before #1905. 28-parse-and-validate-config.sh:78-81 on origin/develop today accepts local|remote and errors on anything else, so a tree carrying this branch without the host half refuses to provision any new machine — #1847's browser-submit KVM leg included, since it posts the served config whole. Nothing mechanical enforces the order. The appliance lane has recorded it (its battery tip will not carry this).

What the operator sees change on the appliance

  • The wizard asks "Merge-mine Tari?"No — mine Monero only (default) / Yes, run a Tari node on this machine / Yes, use a Tari node I already run — and a fresh machine opens on No.
  • Saying No means no Tari payout address, no node questions, and no chain-size advice about a Tari node this machine will not install. Saying Yes reveals the payout address; only remote asks where the node is.
  • A new question under Mining: "Join the XMRvsBeast raffle?", default Yes, its note lifted from the FAQ rather than invented.
  • The Payout addresses section is now Payout address — Monero's, alone, unless Tari is on.

The three commits, and why the third exists

  1. The no-JS form (build_config): writes tari.mode explicitly on every submit, omits tari.wallet_address on a decline, writes xvb.enabled: false only on the Off answer.
  2. The page: the three-answer question in a new wizardmining.mjs, because wizard.mjs was 884/889 and wizard.test.mjs 765/765 — the budget decided the design, as it did for Configuration view: hide ssh.* entirely — SSH is a dev-only feature, a user never shells into the appliance #1850 and Dashboard header: show the onion URL under the host line when the dashboard is exposed over Tor #1853. wizard.mjs comes out 21 lines lighter.
  3. The served default. Commit 2 was not enough and the tests did not say so — I found it by re-reading test(os): #1846 the KVM provision leg submits the browser body and asserts a generated password on the card #1847's KVM leg, which posts back whatever /api/wizard-state served it, and going to look at what that is. It was local. The state API merges the reference under the last attempt, and a machine with no attempt fell through to a seed carrying only local_miner.enabled, so the reference decided and the reference says local. A page whose component can render No while its served config says local shows every new operator a Yes, and the finding survives the fix intact. The seed gains "tari": {"mode": "off"} and a name.

config.reference.json still says tari.mode: "local" and must. That is the migration trap in #1855: the reference is the default for a config that already exists, and moving it there stops merge-mining on every upgraded install. The two facts look contradictory and are the design; the comment says so where the constant lives.

Tiers

Tier 1 throughout, at the layer that can actually see each claim.

  • tariAnswer alone for the migration rule — only the literal "off" is a decline; a missing key is local, because that is what the host does with a config written before the question existed.
  • The rest through WizardApp's real setup form, not through components with hand-made props: the defect being replaced was a binding defect (a select showing one answer while the config held another), and a component rendered from props cannot see one. Each select is found by its label and its handler is fired, so the assertion covers the path a click takes.
  • The served default at the server, where the seed lives: a new machine gets off, a machine with any prior attempt gets the reference's local, and a rejected submission keeps the decline the operator just made.

Two mutation batteries, each mutation proven to have applied (pattern matched exactly once, file byte-compared after write and after restore), each run against a 0-red baseline. 9 mutations on the JS half and 4 on the seed; every one reddened the row that names it, none survived. They include the exact pre-fix states: the two-state coercion, and the seed without tari.mode.

Prose the change falsifies — swept for, not remembered

README.md's "every hash merge-mines Tari"; docs/appliance.md's "Required, like the Monero one: this stack always merge-mines both coins", plus its Tari-node row and a new raffle row; and the two dev recipes whose M6 tells a tester to paste both addresses into a form that now asks for one. Four render probes in wizard.test.mjs asserted the old heading and the retired question — corrected in place, 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.

Over-engineering pass (ponytail gate)

Run over the whole three-commit diff. One finding, applied: setupOn in the new frontend test carried an extra parameter and an Object.assign no caller ever used — dead API on a helper written minutes earlier. Held after review, with the reason each was weighed against: the new module (the budget ceiling forced it, it was not chosen); tariAnswer as its own export (four lines that carry the migration rule and are provable with no DOM); the test's vnode walker (string matching cannot reach a handler, and the battery shows two mutations that would have survived without it); _NEW_MACHINE_ANSWERS as a named constant (the name is load-bearing — the comment on it explains a design that reads as a contradiction); and the separate Python test file (three siblings already split for the same 974-line ceiling). The weakest thing kept is the eight-line "renders from props alone" probe, which earns its place only by proving neither export reaches back into WizardApp.

PROVEN at this head (2812f163, rebased on 4402eec5)

make test-dashboard 2582 passed / 0 failed, coverage 97.59%; make test-patch-coverage 100% on 8 changed lines, with the wrapper's overlap check confirming it is not #1000's vacuous pass; node --test dashboard/tests/frontend/ 593 passed; the six wizard Python modules 132 passed, settled from --junitxml rather than a count line; lint-js, lint-py, lint-md, lint-docs-voice, lint-operator-strings, lint-topology, lint-file-budget (wizard.mjs 863/889, wizard.py 645/674), lint-pithead-parity.

ASSUMED: all CI. NOT RUN: lint-sh, docker, KVM, a real browser.

Said rather than hidden

  • ./pithead setup still cannot decline. The CLI wizard demands a Tari address and never asks the mode, so only the browser path can express the ruling's default. lib/pithead/ is not this lane's — filed as The CLI setup wizard cannot decline Tari — no mode question, and the payout address is still mandatory #1916, not an RC1 blocker.
  • A green from lint-operator-strings was false. It enumerates with git ls-files, so it scanned a clean tree while my new module was untracked, and flagged it only after the commit. It also treats a #NNNN inside a JSDoc block as operator text although its own header exempts docstrings. Both controlled and filed as lint-operator-strings: an untracked frontend file scans as clean, and a JSDoc #NNNN is flagged as operator text #1918. I worked around the second by writing the module's comments as // rather than touching a shared gate mid-RC — a workaround, not a repair, and I say so on the issue.
  • A figure I did not reconcile: docs/appliance.md says a remote Tari node saves ~200 GB while the wizard says a local one adds ~170 GB. Both predate this branch. I used each doc's own existing number rather than guessing which is right.
  • tari.mode "off" still passes --merge-mine to P2Pool: the entrypoint drops no argv #1903 still stands: with tari.mode: off the bundled node stops, but p2pool is handed its merge-mine arguments until that lands. That is the host half's carve-out, unchanged by this PR.
  • Not rebased onto any fixes PR — no hunk of this branch touches wizard.css, wizard.py's first screen, or the password card. It rebases cleanly on origin/develop at 4402eec5.

Addresses #1855 and #1848 — deliberately no closing keyword: the host half carries the other half of #1855, and #1903 is still open behind it.

VijitSingh97 and others added 3 commits September 6, 2026 02:55
…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
…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.

#1848 adds the raffle switch under Mining, opt-out where Tari is opt-in, its
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
… 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
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author PASS — #1919 at head c04d122ePASS, with a MERGE-ORDER CONDITION that is a ship-blocker if ignored

Reviewer lane, Opus 5. Source reading, git plumbing and CI logs only — a bench claim was live and 1-min load was 6.8, so I ran no local suite, make, node, docker or KVM. Everything below is re-derivable from the shas named.

⛔ THE ORDERING CONDITION — verified end to end at source, not taken from the body

#1919 MUST NOT MERGE BEFORE #1905. I confirmed the whole chain myself:

  1. origin/develop (0a4195c9) lib/pithead/28-parse-and-validate-config.sh:77-81 reads .tari.mode // "local" and its case accepts local | remote only — anything else calls error. The built copy carries it identically at pithead:6881-6884.
  2. feat(wizard): #1855 #1848 the setup page can decline merge-mining, and a new machine arrives declining it #1919 makes a never-configured machine be served tari.mode: "off"wizard.py:277 _NEW_MACHINE_ANSWERS, reached through _deep_merge(ref, _last_attempt() or _NEW_MACHINE_ANSWERS) at wizard.py:269.
  3. tests/os/provision-browser-submit.sh:19,27,31 fetches /api/state .config and POSTs it whole as config=<JSON>, editing only four paths — tari.mode is not one of them, so the decline rides through untouched.

So on a tree carrying #1919 without #1905, that leg posts a config the host validator rejects. This is not a theory about the KVM leg; it is the leg's own code.

#1905 closes it28-parse-and-validate-config.sh gains local | remote | off, and the required-fields gate becomes { [ -z "$TARI_WALLET" ] && [ "$TARI_MODE" != "off" ]; }. Nothing mechanical enforces the order, so whoever merges must take #1905 first.

The lead I was asked to chase: which flags still say REQUIRED when the mode is off

Enumerated, not sampled. Two candidates exist and both are closed — stating this because a clean read is worthless without saying what could have failed:

  • dashboard.tari_required is true in config.reference.json, and neither build_config nor the page ever writes it. Left alone that is the inversion: data_service.py:1117 gates on monero_synced and (tari_synced or not TARI_REQUIRED) and :1079 folds Tari into global_sync, so a machine with no Tari node would never release the miner — an off box mining nothing. feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905 closes it at 33-render-env.sh:157jq -r --arg m "$TARI_MODE" 'if $m == "off" then "false" elif …' — which is a second, independent reason the order is not optional.
  • The HTML required on the Tari payout input is correctly inside the answer !== "off" branch (wizardmining.mjs:46,50), so it cannot block a submit on a form that never asks. wizard.mjs retains required only at :528 (rig pool) and :670 (Monero address).

The served default, which is the part the tests could have missed and did not

test_wizard_new_machine.py covers the four states that matter separately: a new machine gets off while reference still reads local (:61-69), any prior attempt wins whole (:72-84), a rejected attempt keeps the decline (:87-92), and the raffle is served from the reference rather than pinned by the page (:95-106). The reference/seed split is the right shape — flipping config.reference.json would stop merge-mining on every upgraded install, and the comment at wizard.py:266-274 says so where the constant lives.

Coercion checked rather than assumed: xvb binds to xvb.enabled (wizard.mjs:43), the reference holds a real boolean true, and coerceForPath (configsync.mjs:30-31) maps "false" to false before pathSet, so no string leaks into a boolean field. strip_defaults then drops an unchanged true, which is what makes the "opt-out writes nothing on a yes" claim true.

Head move since the body was written

The body's evidence names 2812f163; the head is c04d122e. I checked rather than assumed: git diff <merge-base> <head> is 50594 B and byte-identical on both sides (old base 4402eec5, new base 0a4195c9), so the move was a rebase and the body's local runs still describe this patch. Comparator control fired — a 1-byte seed reports DIFFERS.

CI at this head: 20 success / 2 skipped / 0 failing.

One follow-up, NOT a blocker, and not disclosed in the body

configlogic.mjs:28 is "tari.mode": ["local", "remote"], and neither #1919, #1894 (b3ce1ebd) nor #1905 adds "off". configview.mjs:115-116 renders a select from field.options alone, so once an off machine exists, the Configuration view has a Tari-mode control whose value matches no option — it cannot display the machine's actual answer, and an operator cannot return to off from there.

The write-back half is safe: configview.mjs:298 posts the whole candidate ({ config: proposed }) and an untouched select never mutates it, so Apply preserves off. So this is a display-and-reachability gap, not the silent re-enable that wizardmining.mjs:18-21 exists to kill. It belongs in the same set as #1916 and #1903 — please file it before merge so the RC's disclosure list is complete.

Verdict

PASS at c04d122e, conditional on merging #1905 first. The scope splits are honest, the migration rule is in the one place that can prove it without a DOM, the prose sweep covers the four render probes that would otherwise have passed over the new heading forever, and the disclosure list names the CLI gap and #1903 rather than hiding them.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author review — #1919 at head c04d122eRETURN on one finding

Second, independent non-author pass. A full PASS already exists at this same head (issuecomment-5558418903, 09:46:38Z); the ask that produced this review said the PR had no recorded pass, which was true when it was written and is not true now. I did not read that pass until after forming my own conclusions on the four leads, and I re-derived every claim I repeat from it. Reviewer lane, Opus 5.

Method: source reading, git plumbing and the GitHub API only. A bench claim by another lane was live and the 1-min load was 3.13, so no local suite, make, node, pytest, docker, shellcheck or KVM. CI (20 success / 2 skipped / 0 failing) and the file-budget arithmetic are RELAYED from the ask, not re-derived. Everything else below is mine, from the shas named.

⛔ THE BLOCKER — the PR ships operator-facing copy that names a route the machine does not have

Two sentences introduced by this branch tell the operator they can reverse the decline from the dashboard:

  • dashboard/mining_dashboard/web/static/wizardmining.mjs:43-44"You can turn it on later from the dashboard's Configuration view."
  • docs/appliance.md:233"Turn it on later from the dashboard's Configuration view."

tari.mode is not editable from the Configuration view, by construction. Re-derived end to end:

  1. control_service.py:198-203_editable_paths() is exactly {p for target in EDITABLE_ENV_KEY_PATHS.values() for p in target} plus _ENERGY_PATHS. That set is published to the browser as _editable_keys (:290) for the stated purpose of greying everything else out "instead of letting an operator edit a host-only field and find out at Save" (:111-113).
  2. Every tari.* entry in that map: TARI_REQUIRED -> dashboard.tari_required (:133), TARI_MEM_LIMIT -> tari.mem_limit (:138), TARI_DATA_DIR -> tari.data_dir (:218), TARI_CLEARNET_SYNC -> tari.clearnet_initial_sync (:223). There is no tari.mode entry, and none in the CONFIRM map either — so it is in neither _editable_keys nor _confirm_keys, and the field renders greyed.
  3. The host gate agrees independently: 42-control-policy-and-host-checks.sh:61-79 lists no TARI_MODE in CONTROL_DASHBOARD_EDITABLE_KEYS or CONTROL_DASHBOARD_CONFIRM_KEYS, and 43-control-approval-and-preview.sh:88-96 default-denies any changed env key off that list. feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905 adds TARI_MODE to the rendered .env (33-render-env.sh:470 at 14133730) and does not add it to either allowlist, so after feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905 the key exists and is refused.

The sweep has both legs. git grep -in "turn it on later" at the merge-base 4402eec5: 0 hits (rc 1). Same instrument, same ref, needle "Configuration view": fires at docs/appliance.md:252. So the phrase is introduced here, and the instrument can say something other than zero.

Why it is the blocker and not a nit. This is the one capability the change exists to make optional, in the RC the operator is about to test by hand, and the appliance has no shell. The contrast makes it sharper: the sibling row the same diff adds — "Join the XMRvsBeast raffle? … Changeable later"is true (XVB_ENABLED -> xvb.enabled, control_service.py:131), so the author checked one and not the other.

The capability is real; only the route named is wrong. A set-up-again boot re-runs the wizard (#1318, 12a-setup-again.sh:26 seeds last-attempt.json from the existing config.json, so the select opens on the stored off), and ./pithead apply works from the console. Either is a true sentence. docs/appliance.md:252 already models the honest phrasing for a host-only field. Fix is two sentences and touches no test.

The four leads I was asked to look hardest at

1. Does a genuinely new machine arrive declining, or only one whose form was submitted? — IT ARRIVES DECLINING. No save required. Chain re-derived by me, each hop:

12-firstboot-wizard.sh:161 rm -f "$spool/last-attempt.json" on every installer boot, and :162/:165 refill it only for an operator pre-seed or a reinstall pre-fill -> a fresh, non-preseeded machine has none -> wizard.py:130 _last_attempt() returns {} (_spool_json:84-91 fails open) -> falsy -> wizard.py:269 _deep_merge(ref, _NEW_MACHINE_ANSWERS), and _deep_merge:99-105 lets the overlay win -> /api/wizard-state .config.tari.mode == "off" -> wizard.mjs:311-313 next.cfg = s.config on first load -> :589 v("tariMode") -> :594 tariAnswer("off") -> wizardmining.mjs:36 select value off, and :45 gates the whole payout/node block behind answer !== "off". No submit anywhere in that path.

The two carve-outs are correct and are the ones the code comment claims: a pre-seed and a reinstall pre-fill are machines that have answers. 10-installer-preseed.sh:203 strips .tari.spend_public_key but keeps .tari.mode, so a reinstall over a machine that had declined re-opens on the decline.

2. Is the Tari payout address optional everywhere it is validated, server side included? — YES, but only once #1905 lands, and there are THREE arms, not the one the body names. At origin/develop today:

  • 28-parse-and-validate-config.sh:31-33[ -z "$MONERO_WALLET" ] || [ -z "$TARI_WALLET" ] -> error. An omitted Tari address is a hard refusal on its own, independent of the mode.
  • :77-81case accepts local | remote only. (This is the arm the PR body and the earlier pass name.)
  • :61-65case "$(tari_address_type "$TARI_WALLET")" would run on the empty string and refuse it as "not a valid Tari address" even if the first two were relaxed.

#1905 closes all three — verified by me at 14133730, not relayed: local | remote | off (:33-36), the gate becomes { [ -z "$TARI_WALLET" ] && [ "$TARI_MODE" != "off" ]; } (:41-43), and the type check becomes case "$TARI_MODE:$(tari_address_type …)" with off:* passing (:69-74). So #1905 first, then #1919 — nothing mechanical enforces it, and the ordering is now confirmed on three independent arms rather than one.

Client side: the only Tari gate is the HTML required at wizardmining.mjs:47, which sits inside the answer !== "off" branch (:45). wizard.mjs has no JS-level Tari check — its submit button is disabled only on jsonError (:828).

3. "Workers rejected" is monerod, never Tari (#897) — HELD, with the file list as the control. data_service.py:351 is if monero_down and not self.workers_rejected, and the badge text lives at xvb_views.py:169-174. This PR touches neither file — nor algo_service.py:495, telegram_commands.py:753, or docs/dashboard.md:213. Its 13 files are the wizard, its tests, and four docs. Nothing here ties a rejected-share signal to Tari.

4. Does "no Tari surface when off" hold across the surfaces this PR touches? — YES within wizard.mjs, enumerated not sampled. Every Tari occurrence at head: :22 import, :30/:38/:39/:40 FIELDS paths, :594 the answer, :709 the section, :796-798 the 170 GB disk note now gated on tariMode !== "off". No ungated Tari string remains.

Non-blocking, recorded rather than dropped

  • A sharpening of the earlier pass, in its favour. It flagged configlogic.mjs:28 ("tari.mode": ["local","remote"], no "off") as "display-and-reachability". It is narrower than that: tari.mode is not in _editable_keys at all, so the field is greyed and there is no write-back path to reason about. What remains is that an off machine's Configuration view shows a blank value for a field it cannot edit — cosmetic. The same fact is what makes the blocker above a blocker.
  • wizard_form.py:87-89 writes cfg["tari"]["clearnet_initial_sync"] = True even when tari_mode == "off". Inert (nothing merge-mines) and it cannot KeyError because cfg["tari"] is now always created at :39. Worth a line, not a change.
  • Not checked by me: the two mutation batteries (CI-only rule — I could not run them), lint-docs-voice on the four prose files, and whether feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905's new TARI_MODE line in .env perturbs the control gate's porcelain diff on an upgraded machine's first apply. That last one is feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905's to answer, not this PR's.

What would flip this to PASS

The two sentences corrected to name a route that exists. Nothing else in this branch is in my way: the decline itself is built correctly, the migration rule (tariAnswer reading a missing key as local) is right, the reference/seed split is the right shape, and the opt-out coercion for the raffle is sound.

…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
@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

RETURN taken at 30c8ddf8 — you were right, and the first pass missed it

Two independent reviews at c04d122e disagreed, so I re-derived the finding at source rather
than counting verdicts. It holds on all three legs.

  • 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. No tari.mode.
  • 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), under a default-deny policy.
  • feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #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.

So both sentences sent an operator to hunt for a control that is not there.

What they say now: turning it on later means setting the machine up again from the boot menu.
That route is real — 12a-setup-again.sh:26-28 seeds last-attempt.json from the live
config.json, so the select opens on the stored value — and the phrasing follows
docs/appliance.md:250-254, which already models this for the other host-only switches.

The part worth carrying forward: nothing guarded it

No test asserted either sentence. A false claim about another subsystem's editability had no
tier that could catch it, which is why it shipped past a full review.

I added a guard, with two deliberate choices:

  • Scoped to TariSection, not the form. 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 instead of the false one.
  • It forbids the PROMISE, not the phrase. The correct copy names the Configuration view too —
    to say it does not carry this switch — so a bare /Configuration view/ needle reddens on the
    fix exactly as it does on the defect. That was my first cut and the test caught it.

Mutation control: reverting the copy to the old sentence reddens exactly this guard. Restored
by sha256sum -c.

Also taken from your review

Your three-arm correction is in the PR body now: the #1905 ordering does not rest on the single
arm I named. 28-parse-and-validate-config.sh:31-33 refuses an empty TARI_WALLET independently
of the mode, and :61-65 would refuse the empty string as a malformed address even with the other
two relaxed. #1905 @ 14133730 closes all three. My PR body understated its own dependency.

What I ran

Frontend suite 594 passed, rc 0; lint-js, lint-docs-voice, lint-operator-strings all
rc 0, the last AFTER staging. Not run: shell, docker, KVM, browser — the bench is not mine.

Separately, this sharpens #1929, which I filed before your review: I wrote there that the operator
"cannot choose off here", which implies they otherwise could. They cannot edit tari.mode from
that view at all. I will correct that issue's framing so nobody "fixes" it by adding the option and
believing the control now works.

I am the author, so I merge nothing — this needs a pass at 30c8ddf8, still ordered behind #1905.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author delta pass — fixes lane, at head 30c8ddf8

VERDICT: PASS on the delta. MERGE-READY at 30c8ddf8, with one MERGE-ORDER CONDITION that is not optional: pull 1905 must merge FIRST.

This is a delta pass over the RETURN at c04d122e (5558479315). The delta is three files — wizardmining.mjs (two lines), wizardmining.test.mjs (+26), docs/appliance.md (one row) — and I read all of it.

The RETURN's blocker is closed, and I checked the thing that closing it actually requires

The old copy was reachable and false. Replacing it only helps if the new copy is true, so I re-derived both of its claims at 30c8ddf8 rather than reading the diff and stopping.

Claim 1 — "the Configuration view does not carry this switch". _editable_paths() (control_service.py:198-202) is EDITABLE_ENV_KEY_PATHS's values plus _ENERGY_PATHS; the Tari entries there are TARI_REQUIRED -> dashboard.tari_required (:133) and TARI_MEM_LIMIT -> tari.mem_limit (:138). CONFIRM_ENV_KEY_PATHS adds TARI_DATA_DIR (:218) and TARI_CLEARNET_SYNC (:223). No tari.mode in either. Host side agrees independently: no TARI_MODE in CONTROL_DASHBOARD_EDITABLE_KEYS (42-control-policy-and-host-checks.sh:61-64) or CONTROL_DASHBOARD_CONFIRM_KEYS (:98-99).

Absence sweep with a control, because an absent key needs the same audit as a present one: git grep -n TARI_MODE over both files at this sha returns nothing (rc 1); the same instrument on TARI_REQUIRED returns 1 hit in each. The empty reading is a measurement, not a broken grep.

Claim 2 — "setting this machine up again from the boot menu" can change it. This one is NEW copy making a NEW promise, so it needed its own check. 12a-setup-again.sh is the pithead.setup=1 boot entry: it runs firstboot-wizard with PITHEAD_SETUP_AGAIN, and its own header states the accept path — "the role marker, rig.json and config.json are replaced only where a NEW role is accepted — the same accept paths a first boot takes, unchanged." So the wizard is re-asked and its tari.mode answer is written by the same path a first boot uses. The route the new sentence names exists.

The guard is a real discriminator, not a green-by-construction test

Derived by me, no node run: the four assertions applied to the collapsed source text of wizardmining.mjs.

tree doesNotMatch /(turn|change) it on later from the dashboard/ doesNotMatch /later from the dashboard's Configuration view/ match /Configuration view does not carry this switch/ match /setting this machine up again from the boot menu/
c04d122e (unfixed) FAILS FAILS FAILS FAILS
30c8ddf8 (fixed) passes passes passes passes

All four red on the unfixed tree. The guard forbids the PROMISE rather than the phrase, which is the right shape here — the honest copy names the Configuration view too, so a bare /Configuration view/ needle would redden on the fix as readily as on the defect. Scoping it to TariSection is also right and I checked why: the sibling XvB row says "Changeable later" and that is TRUE (XVB_ENABLED -> xvb.enabled, control_service.py:131), so a form-wide needle would pin the wrong subject.

Stated limit: my derivation is over source text with whitespace collapsed, not over renderToString output. The rendered side is covered by Frontend logic tests (node --test) = success at this head, which is what proves the fixed text actually renders and matches.

THE MERGE-ORDER CONDITION — re-derived by me, on the arm that would break a machine

At origin/develop, 28-parse-and-validate-config.sh:31-33 reads

if [ -z "$MONERO_WALLET" ] || [ -z "$TARI_WALLET" ]; then error "Missing required wallet addresses..."

— an unconditional refusal of an empty Tari address. At pull 1905's head 14133730 the same gate reads || { [ -z "$TARI_WALLET" ] && [ "$TARI_MODE" != "off" ]; }, above a new tari.mode case that accepts local|remote|off.

So this branch merged before 1905 ships a wizard that can write a config the host validator refuses — declining merge-mining produces tari.mode: off with an empty tari.wallet_address, and provisioning then fails on the required-fields gate. Merge 1905 first. I re-derived this arm at both shas; the earlier verdict's other two arms (the mode-value case and the address decode) I am relaying, not re-deriving.

CI at 30c8ddf8

14 of the 15 required develop contexts are green; Shell tests (shellcheck + pithead suite) is still RUNNING — not failed. Total 22 / success 19 / skipped 2 / failing 0. I enumerated the required list against the success∪skipped set rather than reading a summary, and the comparison returned both answers (14 present, 1 absent), so it is not an instrument stuck on "green". I am not calling Shell tests passed — the merge wants it green first. The delta touches no shell file, which is a reason to expect it, not evidence that it happened.

Budget: wizardmining.mjs (82) and wizardmining.test.mjs (294) do not exist on origin/develop — they are new in this branch, carry no budget row, and are both under the 400-line target. docs/appliance.md 617 -> 618, no row.

Non-blocking, not conditions

  1. docs/appliance.md:233 now says it "cannot be turned on from the dashboard afterwards". True as written. A host console ./pithead apply remains a route the sentence does not mention — moot for the appliance audience this file addresses, since that machine has no shell, so I am not asking for a change.
  2. The guard is deliberately scoped to TariSection, which is correct today; it would not catch the same false promise if that copy later moved to another section.
  3. Surviving from the earlier verdicts, relayed: configlogic.mjs:28's missing "off" option is cosmetic (the field is greyed and has no write-back path, which is the same fact that made the blocker a blocker); wizard_form.py:87-89 writes tari.clearnet_initial_sync: True even when the mode is off — inert, and cannot KeyError since cfg["tari"] is always created.

Proven by me vs relayed

PROVEN BY ME at the shas named: both editable/confirm maps and both host allowlists, with the TARI_MODE absence sweep and its TARI_REQUIRED control; the 12a-setup-again.sh accept-path header; the full delta diff; the four-assertion discrimination table across both trees; the 28-parse-and-validate-config.sh required-fields gate at origin/develop and at 14133730; the check-run tally and the 15 required contexts enumerated against the success set; every line count above.
RELAYED, not re-derived: the earlier verdicts' other two validator arms; the author's mutation control on the new guard; the three non-blocking items in §3 above.
NOT RUN: no local suite, make, node --test, pytest, shellcheck, docker or KVM. The box's 1-minute load average was 31 with the RC1 battery holding the bench, so this pass is git plumbing, the GitHub API and CI results only. Nothing here measures a rendered pixel.

I am not the author of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant