fix(wizard): #1923 the "Set up again" screen offers the restore it was hiding - #1927
fix(wizard): #1923 the "Set up again" screen offers the restore it was hiding#1927VijitSingh97 wants to merge 1 commit into
Conversation
…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
Non-author pass —
|
Finding accepted, filed rather than amended — and the reason is the pass itself
Not amended here, deliberately. This PR carries a recorded non-author PASS at Filed as #1928 so it cannot merge silently, which was the reviewer's stated condition. Natural home is the #1868 heading-retag PR, which already opens One thing I got wrong, stated because it is mine: I noticed this label while building the PR and decided then to record it in this body — and did not. It reached the record because the reviewer found it independently, not because I carried my own note across. The defect is small; the dropped note is the part worth naming. Their two kills also stand and I am not re-walking them: |
Sequencing note — this branch and pull 1893 conflict, and neither branch can see itNot a new verdict: my non-author pass at Both branches edit
Whichever of pull 1927 and pull 1893 merges second must rebase before it is merged. Order is otherwise free. Flagging it here as well as there so the rebase does not land on whichever author reads their PR second. A budget check on the same combined tree, for completeness: no file-budget row is breached by the combined set. |
1 participant
Closes #1923
What the operator sees change on the appliance
A machine booted through the boot menu's Set up again entry opens on a screen offering Keep it and Set up again. It gains a third: Restore from a backup.
The operator most likely to boot that entry is the one whose machine went wrong and who is holding a backup archive. They were being shown the one screen in the product that did not mention restoring from one — to reach it 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 — and that claim is the one to check
Restore-at-setup shipped under #909. A restore submitted from a set-up-again boot lands today; the machine simply never offers it. Both halves re-derived at source, and posted in full on the issue rather than only here:
lib/pithead/12-firstboot-wizard.sh:129isif [ -f "$PWD/config.json" ] && ! setup_again_mode; then, andsetup_again_mode()is[ -n "${PITHEAD_SETUP_AGAIN:-}" ](12a-setup-again.sh:8). A set-up-again boot does not take the skip, so the wizard loop runs with a config already on disk.restore_applyrefuses on oversize, bad magic, missing/wrong passphrase, failedtarintegrity, unsafe paths or links, and a staged config that is missing or fails validation. That is the complete list, and none of them consults the machine's current state. Its call site adds nothing —firstboot_consume_restoreguards only on[ -f "$archive" ] || return 2.No host change here.
#1854's scope note says restore "is refused on a machine that already has a configuration"; that note does not survive either site, and the re-derivation is on #1923.Why the whole diff is one file
renderSetup()'s own first line isif (this.state.restoreMode) return this.renderRestore();. So letting the saved-role screen fall through onrestoreMode— exactly as it already does onsetUpAgain— reaches the branch #909 built with no new route and no line spent inwizard.mjs, which this lane does not touch (wizard.mjsalso has 5 lines of headroom against its 889 ceiling; this spends none of them).savedrole.mjsgoes 73 → 101 lines, no budget row (the threshold is 400).Both sides of that seam are covered by different files: this PR proves the screen lets go, and
wizard.test.mjsalready exercisesrestoreMode → renderRestoreat seven sites.Two decisions taken rather than assumed
Not confirm-gated. The issue asks whether overwriting a machine's identity warrants a confirmation step when the fresh-boot path has none. It does not: the restore form is itself a file, usually a passphrase, and a second button — a more informative wall than a modal, and a modal answers "are you sure" without saying what of. 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.
A plain button, not another link-styled one. The design lane recorded (#1868) that
<button class="wizard-link">Set up again</button>already fails axetarget-sizeon this very screen at under 24 px. A third link-styled control would have added a second instance of a known defect. That existing failure is not fixed here — it belongs to the #1868 retag follow-up, which is sequenced below.The over-engineering pass found one real defect, 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 button clears
restoreMode, which lands on this screen again. So a failed Keep sitting inkeepErrorwould have been waiting for the operator on their return, reporting an action they had since abandoned. Neither existing exit could produce that, which is why it did not exist before. Cleared on that exit, and asserted with its own control.Considered and kept, with reasons: the 8-line header comment (it records two host facts that cost real re-derivation and that the next reader would otherwise redo), and the named
leavingconst over an inline condition.What was RUN, at the pushed head
84b9462anode --test dashboard/tests/frontend/— 582 passed, 0 failed.savedrole.mjsatorigin/develop, the new tests kept — 4 failed, 578 passed, and the four are exactly the four rows that assert the new behaviour. The fifth new assertion (restore absent from the post-Keep branch) passes on both sides by design: it is a placement control, and the string is genuinely on the page in the other branch, so it is not vacuous.keepErrorfix: removing only that one clause reddens exactly one row and nothing else. Restored byte-identical bysha256sum -cafter every control.make lint-js(which also lints CSS),lint-md,lint-docs-voice,lint-operator-strings,lint-file-budget— all rc 0, run aftergit add.syncviewrows + 3 new rows here = 582. I branched this off the wrong base first and caught it that way; the figures above are all re-measured at the develop base, not carried over.What was NOT run
make lint-sh,tests/stack/run.sh, pytest, docker, KVM, any browser. Zero shell and zero Python in this diff;lint-shalso dies at the shellcheck memory cap on this box and takes a fleet-wide lock. CI is the gate for those. The axe/target-sizeclaim above is the design lane's measurement, relayed, not re-derived by me — I ran no browser.Sequencing — please do not merge this in parallel with the #1868 retag
The design ruling on #1868 retags 15
<h3>→<h2>, two of them in this same file (savedrole.mjs:41,43). That work is also this lane's, and it is not buildable yet: it edits.wizard-shell h3rules that exist only on #1893's headc480e766and are not ondevelop. So the order is forced rather than chosen — this PR first, the retag rebased over it once #1893 merges. They were not folded because folding would drag this unblocked change behind a blocked one.Shared-file disclosure
None. Two files, both
dashboard/, both in this lane's paths.Scope
RC2, not RC1.
dashboard/is image bytes, and RC1's gate is the #1318 battery leg green on the tip — a merge here moves the tip out from under a verdict that names aBUILD_COMMIT. Note that the leg would not otherwise notice this change:tests/os/setup-again-leg.shnever renders the screen, it drives the host by writing spool files over SSH, so a third button has no tier-4 cover and the node rows above are the coverage.