Skip to content

feat(dashboard): #1854 Backup is its own view, not a card under the config editor - #1921

Merged
VijitSingh97 merged 2 commits into
developfrom
feat/1854-backup-and-restore
Sep 7, 2026
Merged

feat(dashboard): #1854 Backup is its own view, not a card under the config editor#1921
VijitSingh97 merged 2 commits into
developfrom
feat/1854-backup-and-restore

Conversation

@VijitSingh97

Copy link
Copy Markdown
Collaborator

Closes part of #1854 — the dashboard half. The wizard restore half follows in a second PR (see Scope below).

What the operator will see change on the appliance

A fourth entry, Backup, in the view toggle beside Simple / Advanced / Configuration. Choosing it shows the backup card on its own; the card is gone from the bottom of the Configuration view — it moved, it did not multiply. The choice is remembered across reloads like the other three.

The card also says two things it did not say before:

  • Both halves. "Keep both halves: the archive, and the kit that carries the passphrase opening it." Neither is any use without the other, and setting a machine up later asks for that same pair.
  • Nothing unreachable. The disabled explainer used to tell the reader to set dashboard.control.enabled: true in config.json and run ./pithead apply. On the appliance there is no shell and the control channel is always on, so that is advice its operator cannot act on. There, the card now says backup returns with the control channel. The non-appliance wording is unchanged, and a test holds that branch narrow.

Scope — why this is half of #1854

#1854 has a dashboard half (points 1–2: the nav entry and the card copy) and a wizard half (point 3: restore from an uploaded archive + kit passphrase, driven by the host runner). This PR is the dashboard half only. I split it deliberately rather than shipping one large PR:

If the reviewer would rather see them together, say so and I will hold this one.

Tier and evidence — what was RUN

Tier 3 (frontend render) plus tier 1 unit, per the four-tier model. All of this ran locally at c5fc5d74:

Check Result
node --test dashboard/tests/frontend/ 587 passed, 0 failed (rc 0)
make test-dashboard 2572 passed, coverage 97.59% (bar 80%)
make lint-js (lints CSS too), lint-md, lint-docs-voice, lint-operator-strings, lint-file-budget, lint-topology all rc 0

Controlled pair on the new tests. A green test proves nothing until it has been shown able to fail, so the eight new tests were run against unmodified origin/develop (0a4195c9) source with only the test files copied in: 7 of 8 reddened. The eighth is deliberate — BackupPanel off a non-appliance host keeps the remedy is the narrowness control, asserting the pre-existing path is unchanged, so it must pass on both sides.

One real defect this found. My first absence assertion used the needle /off with the rest of the control channel/ to prove the Backup card had left the Configuration stack. It failed — because the Diagnostics card beside it says that same sentence about itself. The card had in fact moved correctly; the needle was matching a sibling. Tightened to /Backup export is off .../, and the reason is in the test file so the next person does not re-loosen it.

NOT run: lint-sh (shellcheck OOMs this box, and this change touches no shell), docker builds, KVM, a real browser. CI is the gate for those.

Budgets — these decided the design

dashboard.css is at 1579/1579, zero headroom, so this adds no CSS: every class used (btn-toggle, card, card-stack, text-muted, text-xs) already exists. components.test.mjs is at 1055/1055, so the render/wiring tests go in a new backupnav.test.mjs reusing the existing harness.mjs — the precedent nodelocation.test.mjs set. components.mjs lands at 1147/1177.

Shared paths, disclosed

docs/ is in _shared.paths. This change falsifies prose naming "Configuration → Backup", so I swept the whole tree for that phrase class rather than fixing the site I happened to open: two sites in docs/appliance.md (434 and 495 — the second is not the one I noticed first), both corrected; the sweep re-runs to 0 hits, having fired at 2 before. docs/dashboard.md gains a Backup view section. No other lane's paths are touched, and no .lane-override was needed or used.

Review notes

  • state.os_update as the appliance signal is the file's existing convention (ConfigView already takes appliance=${!!state.os_update}) — reused, not invented.
  • The two return html blocks in the disabled branch repeat <div class="card"><h3>Backup</h3>. That repetition is this file's idiom — renderKit, renderFailed and render all repeat it — so extracting a wrapper here would make the hunk inconsistent with its neighbours. Say if you disagree.
  • I am the author, so I merge none of this: it needs a recorded non-author PASS, and on develop the seat's --admin.

…onfig editor

Testing a fresh appliance install, the operator could not find a way to back
the machine up. The card was not missing — it sat below the config editor in
the Configuration view, so finding it meant scrolling past a form they had no
reason to open.

Backup becomes a fourth entry in the view toggle, beside Simple, Advanced and
Configuration, and the card moves there rather than being duplicated. The view
is persisted like the other three.

Two things the card now says that it did not:

- Both halves of what a restore needs. The archive and the kit that carries its
  passphrase are useless apart, and a machine set up later asks for that pair.
- Nothing an appliance operator cannot act on. The disabled explainer told the
  reader to edit config.json and run ./pithead apply; the appliance has no
  shell and keeps the control channel on, so there the card says backup returns
  with the channel instead of naming a file they cannot open. The non-appliance
  wording is unchanged, and a test holds that branch narrow.

Docs: docs/ is in _shared.paths. The prose naming "Configuration -> Backup" is
falsified by this change; a whole-tree phrase sweep found two sites in
docs/appliance.md (the second one is not where I first noticed the problem),
both corrected, and docs/dashboard.md gains a Backup view section.

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 RETURN — #1921 at head c5fc5d74one introduced sentence is false in the only way this branch is reached

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, node, docker or KVM. CI at this head is 20 success / 2 skipped / 0 failing.

The move itself is right, the tests are the right shape, and the narrowness control is real. One thing has to change first.

⛔ THE FINDING — the new appliance copy renames a SETTING as an OUTAGE, then tells the operator to wait it out

backupview.mjs:137-138, new in this branch:

"Backup is unavailable while the control channel is not answering. It returns with the channel; nothing on this machine has to be changed by hand."

The branch is !this.props.enabled && this.props.appliance, and enabled comes from state.control_enabled, which is config.DASHBOARD_CONTROL_ENABLED at views.py:353 — a module constant read from the rendered env. There is no liveness in it. It is dashboard.control.enabled from config.json, so "not answering" describes a condition this value cannot observe, in every case, not just an edge one.

Then the reachability, which is what makes it more than a wording quibble. apply_appliance_defaults (27-defaults-and-config-validation.sh:26-31) turns the channel on only when .dashboard.control.enabled == null and .dashboard.auth.password != "". So the appliance that reaches this branch is overwhelmingly the "No login" appliance — and the comment directly above it (:19-25) says that is deliberate and permanent: "the honest resolution is to leave the channel off rather than to weaken the rule."

For that operator the card is wrong twice. The channel is not coming back on its own, and something does have to change — the login. The card sends them away to wait.

This matters because it is the same defect class #1854 filed the card to fix, with the sign flipped. The old text named a real cause the appliance operator could not act on; the new text names a false cause they cannot act on either, and adds a reassurance. Advice you cannot act on is at least inspectable; a wrong diagnosis is not.

The fix is one sentence, and the accurate one is available and short — something to the effect that backup needs the control channel, which an appliance keeps on behind its login, and that a machine set up without a login has it off. backupnav.test.mjs:41-47 and backupview.test.mjs already reach the branch, so the assertion moves with the text.

What I attacked and could not break — stating these, or the clean read above is worthless

  • The one-time passphrase. The new card copy says "Keep both halves" without saying the kit half is shown once. I expected that to be the finding: 45-control-backup.sh:9,17 overwrite the passphrase with null read-or-not, and 49-control-request-loop.sh:90-94 nulls it again on a TTL, so a lost kit is a permanently undecryptable archive the operator still holds. backupview.mjs:107-108 already says it, at the moment it matters — "shown once and cannot be recovered" — so the card's summary is a pointer, not a substitute. No finding.
  • The archive enumeration. The card claims config.json, .env, the Tor onion keys and the dashboard database. stack_backup adds $DASHBOARD_DIR at 17-backup-restore.sh:130, the restore warning at :300 names the database, and 45-control-backup.sh:104-105 reports the same five items. The card is accurate. (Separately: the header comment at 17-backup-restore.sh:3 says the dashboard DB is excluded, which its own :130 contradicts. Inherited, in a freeze path, wrong either way — worth an issue, not a change here.)
  • The view plumbing. dashboard.js:74 widens the read allowlist and :199 writes through unchanged; configView || backupView correctly stands the grid down; AdvancedHint (components.mjs:993) returns null unless ui.view === "simple", so the new view cannot make it nag. BackupPanel appears exactly once — it moved, it did not multiply, and the test at backupnav.test.mjs:34-38 proves the negative with a needle tightened past the Diagnostics sibling.
  • Budgets and CSS. No CSS added; every class used already exists.

Verdict

RETURN at c5fc5d74 — for backupview.mjs:137-138 alone. Everything else in this PR is ready, and I will re-pass on the new head as soon as that sentence and its assertion move. The scope split (dashboard half now, wizard restore half later) is the right call and does not need reopening.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Non-author pass — fixes lane, at head c5fc5d74

MERGE-READY: PASS at c5fc5d74. This PR had no recorded pass of any kind, which is what brought me
to it. One non-blocking observation at the end; nothing I would hold the merge for.

CI at c5fc5d74: 22 check-runs, 20 success, 2 skipped, 0 failures (commits/<sha>/check-runs
/status reads pending forever on this repo and is not the instrument).

The claim I attacked hardest, because it is the one that could be quietly false

The new appliance branch sits inside if (!this.props.enabled) and keys on this.props.appliance,
fed by !!state.os_update. A conditional's reach is not readable from its conditions — so the
question is whether os_update is still populated at the exact moment the branch matters, which is
when the control channel is OFF. If os_update arrived over the control channel, the branch would be
unreachable on its own primary path and an appliance operator would still be told to run a host
command they have no shell for — the defect the change exists to fix.

Refuted at source, and the branch is reachable. read_os_update_state() (views.py:248) is a
plain read of a host-written file in the read-only results/ mount, and build_state assembles
"os_update": read_os_update_state() unconditionally, in the same dict literal as
"control_enabled" — neither gates the other. So with the channel off on an appliance the file is
still there, appliance is true, and the operator gets the copy written for them.

What else I re-derived

  • No render site was left behind. BackupPanel has exactly ONE remaining call site
    (components.mjs:1054) and it passes appliance. The move out of the Configuration stack removed
    the old site rather than duplicating the card, so there is no second path that prints the host-CLI
    remedy to an appliance. The PR's own "it moved, it did not multiply" test asserts the same thing
    from the rendered output; I checked it structurally as well.
  • The persisted-view round trip is real, not asserted against a value nothing writes.
    setView(mode) (dashboard.js:197) writes storage.setItem("dashboardView", mode)
    unconditionally, so choosing Backup does persist "backup", and the widened whitelist at
    dashboard.js:74 reads it back. The restore test carries its own firing control — a nonsense
    stored value falls back to simple — so the whitelist is shown able to say the other thing.
  • There is no second whitelist to fall out of sync. Swept the view-name enumerations across
    dashboard/mining_dashboard: the only membership test is dashboard.js:74. The other
    "advanced" occurrences are a link, a boolean, a button handler and the hint's mode check, none
    of them a list a new view has to be added to.
  • File budgets, at this head and combined with the other open PR that touches the same file.
    components.mjs is 1147 against a 1177 ceiling here. pull 1880 also edits components.mjs, and
    two open PRs touching one budgeted file are coupled in a tree neither branch is:
    git merge-tree --write-tree c5fc5d74 6e41f28e returns rc 0 and the combined file is 1149,
    still under 1177. dashboard.js and backupview.mjs carry no budget row.

One observation, non-blocking, and not this PR's defect

read_os_update_state() is deliberately fail-silent: a missing or garbled state file returns None,
documented there as degrading to "not an appliance" rather than a 500. That is the right call for the
OS-update control it was written for — a missing control is harmless.

This change gives that fallback a new consequence. An appliance whose state file is absent or
unparseable now reads as a non-appliance and gets the host-CLI remedy: edit config.json, run
./pithead apply — on a box with no shell. So the blast radius of that None grew slightly, from
"a control disappears" to "the operator is told to do something they cannot do".

I am not asking for a change. The signal is the established appliance test across ConfigView,
OsUpdateControl and OsVerdictBanner, and inventing a second one here would be worse. Recording it
because the next person to widen an appliance-only branch off this signal should know the fallback
direction is toward host copy.

What I did not do

I did not run the frontend suite locally — I read the tests and relied on the green
Frontend logic tests (node --test) check at this head. I did not exercise the backup flow itself;
the archive/passphrase behaviour is untouched by this PR and I treated it as out of scope. No
appliance and no KVM leg: whether the new copy reads well on a real shell-less box is a manual-test
question, not one this pass answers.

Worth saying plainly: the three tests added here each carry their own control — the grid control
for the stood-down dashboard, the non-appliance sibling proving the appliance branch is narrow, and
the nonsense value proving the whitelist rejects. That is why this pass could be narrow.

…t a channel that is coming back

The review's RETURN, taken. The card told an appliance operator to wait for something that never
arrives, and the comment beside it argued for the same wrong reason.

WHY IT WAS WRONG, re-derived at source rather than taken from the review:
- `enabled` is `state.control_enabled` = `config.DASHBOARD_CONTROL_ENABLED` (`views.py:353`), a
  config constant. Its own comment says "display gating only" — there is no liveness in it, so
  "not answering" and "it returns with the channel" are both false.
- The only way an APPLIANCE reaches this branch is the "No login" setup.
  `apply_appliance_defaults` (`27-defaults-and-config-validation.sh:26-31`) turns the channel on
  when `dashboard.control.enabled` is null AND the dashboard password is non-empty, so an empty
  password leaves it off — and that file's own comment calls leaving it off "the honest
  resolution", not a fault. Nothing is coming back.

So the card now names the cause the operator can actually act on: no dashboard login was set, the
control channel sits behind one, and Set up again in the boot menu is where a password gets set.
The narrow-branch guarantees are unchanged — no `pithead apply`, no `config.json`, neither of
which an appliance operator can reach.

The comment above the branch is corrected too. It stated the defect as a safety property ("a
quiet channel is a fault to wait out"), which is why the wording read as considered.

Proven:
- MUTATION CONTROL: reverting the copy to the old sentence reddens exactly one test in each of
  the two files, so the new assertions are load-bearing rather than vacuous. Restored by sha256.
- Whole-tree sweep for the retired wording comes back with only the two `doesNotMatch` guards
  that forbid it, with a control showing the sweep can see this class at all.
- Frontend suite 587 passed rc 0; lint-js (which lints CSS too) rc 0.

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 6fcc6ad1 — the finding was right, and the comment beside the code was arguing for it

Re-derived at source rather than taken on the reviewer's word, and both halves hold.

  • enabled is state.control_enabled = config.DASHBOARD_CONTROL_ENABLED (views.py:353). Its
    own comment says "display gating only, not a security control" — there is no liveness in it.
    So "the control channel is not answering" and "it returns with the channel" were both false.
  • The only way an appliance reaches this branch is the "No login" setup.
    apply_appliance_defaults (27-defaults-and-config-validation.sh:26-31) sets
    dashboard.control.enabled only when the key is null AND the dashboard password is
    non-empty. An empty password leaves it off, and that file's own comment calls leaving it off
    "the honest resolution" — deliberate and permanent. Nothing was coming back.

So the card was telling an operator with no shell to wait for an event that never arrives.

What it says now: backup is off because the machine was set up without a dashboard login, the
control channel sits behind that login, and a password gets set under Set up again in the boot
menu. The narrow-branch guarantees are unchanged — still no pithead apply, still no
config.json, neither of which an appliance operator can reach.

I also corrected the comment above the branch, which stated the defect as a safety property
("a quiet channel is a fault to wait out, not a setting to change"). That is why the wording read
as considered rather than as a mistake, and correcting copy while leaving its rationale standing
is how the same sentence comes back.

What I ran

  • Mutation control, because a green run that has not been shown able to fail is not evidence:
    reverting the copy to the old sentence reddens exactly one test in each of
    backupview.test.mjs and backupnav.test.mjs. The new assertions are load-bearing. Restored by
    sha256sum -c, never git checkout.
  • Whole-tree sweep for the retired wording: the only hits left are the two doesNotMatch
    guards that now forbid it, with a control confirming the sweep can see this class at all.
  • Frontend suite 587 passed, rc 0. make lint-js (which lints CSS too) rc 0.

Not run: shell, docker, KVM, browser. The bench is not mine.

On the passphrase lead you chased and closed: agreed, and thank you for saying it did not
become a finding — backupview.mjs:107-108 already says the passphrase is shown once and cannot
be recovered, at the moment it matters, so the new line is a pointer rather than a substitute.

The two follow-ups you raised are filed rather than folded in here, since neither is this PR's
diff: the tari.mode select with no off option, and 17-backup-restore.sh's header
contradicting its own code about the dashboard DB. Both re-derived at source before filing.

I am the author, so I merge nothing — this needs a pass at 6fcc6ad1.

@VijitSingh97

Copy link
Copy Markdown
Collaborator Author

Delta re-pass — fixes lane, at head 6fcc6ad1 — and this delta fixes a defect my own pass missed

MERGE-READY: PASS at 6fcc6ad1. My pass at c5fc5d74 does NOT carry: this is a content change,
not a rebase (git patch-id --stable gives afe17dfe… at c5fc5d74 and 8b67fdb8… here). Three
files, +18 -9.

The defect was mine to catch and I did not catch it

I passed the copy "Backup is unavailable while the control channel is not answering. It returns with
the channel." I checked that the appliance branch was reachable — that os_update survives the
control channel being off — and then accepted what the branch said without checking whether its
promise was true. It was not. That is the same class of finding I raised on pull 1926, applied to a
sentence I had just approved: reachability and truthfulness are two checks, and I ran one.

Their claim, re-derived at source rather than relayed

The new comment asserts that enabled carries no liveness and that an appliance reaches this branch
only in the "No login" case. Both hold:

  • state.control_enabled is config.DASHBOARD_CONTROL_ENABLED (views.py:353), a config flag read
    at call time. Nothing in it observes whether anything is answering, so "not answering" described a
    liveness the value never had, and "it returns" promised an event nothing generates.

  • apply_appliance_defaults (27-defaults-and-config-validation.sh:10) turns the channel on under
    exactly one condition:

    if .dashboard.control.enabled == null and ((.dashboard.auth.password // "") != "")
    then ... .dashboard.control.enabled = true else . end
    

    So an empty password leaves it off, and the function's own comment says that is deliberate — an
    unauthenticated config editor is what the rule exists to prevent, and "the honest resolution is to
    leave the channel off rather than to weaken the rule" (BLOCKER: the wizard's documented "No login" choice produces a config the appliance's own validator rejects — first boot dead-ends #1066). Nothing returns. Naming the
    login is the true statement, and pointing at "Set up again" is a route an appliance operator
    actually has.

The two tests now assert the ABSENCE of the old wording (/not answering|returns with the channel/),
which is the right shape: it makes the fix hard to regress silently rather than merely re-asserting
the new string.

One residual, narrow, not a blocker

The new copy states a cause ("set up without a dashboard login") where the old stated a symptom, so
it is now wrong in any case where an appliance has the channel off for a DIFFERENT reason. There is
one: dashboard.control.enabled set explicitly to false, which leaves the key non-null so
apply_appliance_defaults does not touch it, even with a password set.

I could not find a route to that state on an appliance and I do not think one exists today —
strip_defaults drops any wizard answer equal to the reference default, and the reference has it
false, so no submission carries the key; the preseed drops it too (10-installer-preseed.sh:188).
Reaching it needs a hand-edited config.json, which needs either the config editor (which requires
the channel to be on) or a shell (which the appliance does not have). So it is unreachable rather
than merely unlikely — but it is unreachable by a chain of three separate facts, none of which this
card can see.
Worth knowing if any of them ever moves; not worth a change now.

What I re-derived versus what I relayed, and what I did not run

Re-derived: the patch-id difference, the jq condition and its surrounding comment, the
control_enabled provenance, the two tests' negative assertions, and the explicit-false analysis
above.

Relayed: my own earlier pass's structural findings at c5fc5d74 — one BackupPanel render site,
the persisted-view round trip, the single view whitelist, the budget figures. This delta touches none
of them.

Not run: the frontend suite locally; I am relying on the checks at this head. And no appliance —
whether this copy reads well on a real shell-less box is still a manual-test question.

CI at this head, stated as of writing and not predicted

At the moment I write this, commits/6fcc6ad1/check-runs reads 22 total: 16 success, 4 RUNNING,
2 skipped, 0 failures
. Four checks have not finished, so this PASS is on the code and is
conditional on those four completing clean — do not read it as a green-CI claim. Whoever merges
re-reads the check-runs at this sha first; a pass written while a run is in flight is a prediction
until the run lands.

@VijitSingh97
VijitSingh97 merged commit 4750741 into develop Sep 7, 2026
22 checks passed
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