From 429ec3fa2df515c7290026e167806024d68af7b1 Mon Sep 17 00:00:00 2001 From: librelad Date: Sat, 29 Aug 2026 05:51:24 +0100 Subject: [PATCH] Contents step: show the two snapshot kinds as two things MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repository holds one system=config snapshot and one snapshot per app. They are separate because they are used separately: the settings tree is small, changes rarely and is meaningless per-app, while app data is large, changes at its own rate, and has to be restorable, movable and ageable on its own — which is what the per-app tag buys. The step listed "Apps" and "Domains" as peer sections, which hid that entirely. It read as though a backup held three kinds of thing, and gave no clue that the domains come OUT of the system snapshot rather than being a third kind. Now: a Settings section (one snapshot, dated, saying plainly that it carries every backup repository with its credentials and is restored first because it is what makes the others reachable) with the domains nested under it and each one's DNS verdict; then an App data section, one snapshot each, dated and sized. A repository with app data and no settings snapshot says so — the consequence, that repositories and logins do not come back, is not something to find out afterwards. restoreInspect emits that shape now, assembled with jq against the discover JSON rather than by hand-rolled string concatenation, and carries each snapshot's date: a backup's age is what people actually judge it by. Co-Authored-By: Claude Opus 5 --- .../frontend/core/setup/js/setup-wizard.js | 108 +++++++++++--- docs/roadmap/first-run-restore.md | 33 ++++- scripts/dev/lp-restore-wizard-test | 37 +++++ scripts/restore/restore_inspect.sh | 133 +++++++++++------- 4 files changed, 231 insertions(+), 80 deletions(-) diff --git a/containers/libreportal/frontend/core/setup/js/setup-wizard.js b/containers/libreportal/frontend/core/setup/js/setup-wizard.js index 709b4a8..3d77b01 100755 --- a/containers/libreportal/frontend/core/setup/js/setup-wizard.js +++ b/containers/libreportal/frontend/core/setup/js/setup-wizard.js @@ -1141,9 +1141,12 @@ class SetupWizard { this.restoreInfo = data; if (status) { + const nApps = (data.apps || []).length; + const hasSys = !!(data.system && data.system.present); status.innerHTML = `

Found backups from ${this.escapeHtml(data.host || '')} \u2014 - ${(data.apps || []).length} app(s). Continue to see what will happen.

`; + ${hasSys ? 'settings and ' : ''}${nApps} app${nApps === 1 ? '' : 's'}. + Continue to see what will happen.

`; } await this.renderRestoreContents(); } catch (e) { @@ -1156,9 +1159,21 @@ class SetupWizard { } } - // What is in there, and what it will mean on this machine. The domain checks - // are the part that has no equivalent anywhere else: a restored domain still - // points wherever DNS says, which after a rebuild is usually the old server. + // What is in there, and what it will mean on this machine. + // + // A repository holds two DIFFERENT kinds of snapshot, and this step shows + // them as two things because they ARE two things: + // + // the system snapshot one snapshot of the settings tree — logins, the + // domains, and every backup repository with its + // credentials. Restored first, because it is what + // makes the others reachable. + // the app snapshots one per app, each with its own size and date, so + // an app can be restored or aged out on its own. + // + // The first version listed "Apps" and "Domains" as peers, which hid all of + // that: the domains are not a third thing in the backup, they are part of + // the system snapshot. async renderRestoreContents() { const box = this.container.querySelector('#sw-rs-contents'); if (!box) return; @@ -1168,34 +1183,74 @@ class SetupWizard { return; } + const system = d.system || {}; const apps = d.apps || []; - const domains = d.domains || []; + const domains = system.domains || []; + const others = (d.hosts || []).filter(h => h !== d.host); box.innerHTML = `

From ${this.escapeHtml(d.host || '')}${ - (d.hosts || []).length > 1 - ? ` \u2014 this repository also holds backups from ${this.escapeHtml((d.hosts || []).filter(h => h !== d.host).join(', '))}` + others.length + ? ` \u2014 this repository also holds backups from ${this.escapeHtml(others.join(', '))}` : ''}

-
Apps
+ +
Settings
+ ${system.present + ? `
+ System settings + 1 snapshot + ${this._restoreWhen(system.date)} +
+

+ Your logins, your domains, and every backup repository you had \u2014 + with its credentials, so this one password brings the rest back. + Restored first, because it is what makes the others reachable. +

+
${ + domains.length + ? '

Checking where the domains point\u2026

' + : '

No domains were configured on that machine.

'}
` + : `

+ This repository has no settings snapshot \u2014 only app data. Your + backup repositories and logins will not come back with it. +

`} + +
App data
${apps.length - ? apps.map(a => ` + ? `

+ One snapshot each, restored after the settings. +

` + + apps.map(a => `
${this.escapeHtml(a.name)} - ${this.escapeHtml(a.size || '')} + ${this.escapeHtml(a.size || '')}${ + a.size && a.date ? ' \u00b7 ' : ''}${this._restoreWhen(a.date)}
`).join('') - : '

No app backups in this repository.

'} -
Domains
-
${ - domains.length - ? '

Checking where these point\u2026

' - : '

This backup carries no domains.

'}
`; + : '

No app backups in this repository.

'}`; if (!domains.length) return; + await this._checkRestoreDomains(domains); + } - // Checked one at a time through the endpoint the Domains step already - // uses, rather than adding a second way to ask the same question. + // "28 Aug 2026, 13:10" — a snapshot's age is the thing people actually judge + // a backup by, and an ISO timestamp is not something anyone reads at a + // glance. + _restoreWhen(iso) { + if (!iso) return ''; + const d = new Date(iso); + if (isNaN(d.getTime())) return ''; + return this.escapeHtml(d.toLocaleString(undefined, { + day: 'numeric', month: 'short', year: 'numeric', + hour: '2-digit', minute: '2-digit' + })); + } + + // Where each restored domain actually points. Checked one at a time through + // the endpoint the Domains step already uses, rather than adding a second + // way to ask the same question. + async _checkRestoreDomains(domains) { const rows = []; for (const domain of domains) { let verdict = 'unknown', detail = 'could not check'; @@ -1207,15 +1262,18 @@ class SetupWizard { if (j && j.matches) { verdict = 'ok'; detail = 'points at this server'; } else if (j && j.domain_ip) { verdict = 'elsewhere'; detail = `points at ${j.domain_ip}, not this server`; } else { verdict = 'unresolved'; detail = 'no DNS record found'; } - } catch { /* left as unknown — see below */ } + } catch { /* left as unknown — never offered for deletion */ } rows.push({ domain, verdict, detail }); } this.restoreDomains = rows; - const strays = rows.filter(r => r.verdict === 'elsewhere' || r.verdict === 'unresolved'); const el = this.container.querySelector('#sw-rs-domains'); if (!el) return; + // Unverifiable is not the same as wrong, so only a domain that + // demonstrably resolves elsewhere is offered for removal. + const strays = rows.filter(r => r.verdict === 'elsewhere' || r.verdict === 'unresolved'); el.innerHTML = + `

Domains it will bring across:

` + rows.map(r => `
${this.escapeHtml(r.domain)} @@ -1245,11 +1303,15 @@ class SetupWizard { return; } const apps = d.apps || []; + const hasSys = !!(d.system && d.system.present); box.innerHTML = `

This will, in order:

-
1. Settings and credentials - Including every backup repository you had, so one password brings the rest back.
-
2. ${apps.length} app(s) + ${hasSys + ? `
1. The settings snapshot + Logins, domains, and every backup repository with its credentials \u2014 first, because it is what makes the rest reachable.
` + : `
1. No settings snapshot + This repository holds app data only.
`} +
2. ${apps.length} app snapshot${apps.length === 1 ? '' : 's'} ${this.escapeHtml(apps.map(a => a.name).join(', '))}

Apps this version no longer ships, or that will not fit, are skipped diff --git a/docs/roadmap/first-run-restore.md b/docs/roadmap/first-run-restore.md index fd7465b..5e1ba8c 100644 --- a/docs/roadmap/first-run-restore.md +++ b/docs/roadmap/first-run-restore.md @@ -503,10 +503,35 @@ schedule, no enable toggle. The password leaves through the one-shot never appears in the payload, since that payload reaches a task command line and tasks are recorded world-readable. -**Contents** is §3's reconciliation, rendered. Apps with sizes, and the domains -with a verdict each — checked through the same `/api/setup/dns-check` the -Domains step uses rather than adding a second way to ask the question — plus -the offer to leave the strays out until DNS is repointed. +**Contents** is §3's reconciliation, rendered — and it has to make the +repository's *shape* visible, which the first version did not. + +A repository holds two different kinds of snapshot: + +| | How many | Holds | Restored | +|---|---|---|---| +| `system=config` | **one** | the whole configs tree: logins, domains, and every backup repository with its credentials | first — it is what makes the others reachable | +| `app=` | **one per app** | that app's data directory, with its own manifest | after, each independently | + +They are separate because they are *used* separately. The settings tree is +small, changes rarely, and is meaningless per-app. App data is large, changes at +its own rate, and has to be restorable, movable and ageable **on its own** — +which is what the per-app tag buys: `restore app ` works, retention +applies per app, and an app can be placed on a different drive than it came +from. + +The step listed "Apps" and "Domains" as peer sections, which hid all of that. +It read as though a backup contained three kinds of thing, and gave no clue +that the domains come *out of* the system snapshot. Now it shows **Settings** +(one snapshot, dated, with the domains nested under it and each domain's DNS +verdict) and **App data** (one snapshot each, dated and sized). A repository +with app data and no settings snapshot says so explicitly, because the +consequence — your repositories and logins do not come back — is not something +to discover afterwards. + +Domain verdicts come from the same `/api/setup/dns-check` the Domains step uses +rather than adding a second way to ask the question, and the offer to leave the +strays out only ever covers domains that demonstrably resolve elsewhere. **Rebuild** hands over to `restore rebuild`, which is the installer's order with the same reasoning: settings first (they carry every other repository's diff --git a/scripts/dev/lp-restore-wizard-test b/scripts/dev/lp-restore-wizard-test index 6833452..79acf93 100755 --- a/scripts/dev/lp-restore-wizard-test +++ b/scripts/dev/lp-restore-wizard-test @@ -108,6 +108,35 @@ read -r -d '' DRIVE <<'JS' // timeout only makes the test take a minute longer than it needs to. readPromise.catch(() => {}); + // The Contents step must present the two snapshot KINDS as two things. A + // repository holds one system=config snapshot and one per app, restored by + // different machinery; listing "Apps" and "Domains" as peers hid that, and + // hid that the domains come out of the system snapshot rather than being a + // third kind of thing in the backup. + w.restoreInfo = { + host: 'oldbox', hosts: ['oldbox'], + system: { present: true, date: '2026-08-28T13:10:02+01:00', domains: [] }, + apps: [{ name: 'linkding', size: '1M', date: '2026-08-28T13:10:02+01:00' }] + }; + await w.renderRestoreContents(); + const contents = $('#sw-rs-contents').textContent.replace(/\s+/g, ' '); + out.showsSettingsSection = /Settings/.test(contents); + out.showsAppSection = /App data/.test(contents); + out.explainsSettingsFirst = /makes the others reachable/i.test(contents); + out.showsSnapshotDate = /28 Aug 2026/.test(contents); + // Domains belong under Settings, so with none there is no stray heading. + out.noDomainsHeadingWhenEmpty = !/Domains it will bring across/.test(contents); + + // A repository with app data but no settings snapshot must say so: the + // user's repositories and logins will NOT come back, and finding that out + // afterwards is the worst possible time. + w.restoreInfo = { host: 'oldbox', hosts: ['oldbox'], + system: { present: false, date: '', domains: [] }, + apps: [{ name: 'linkding', size: '1M', date: '' }] }; + await w.renderRestoreContents(); + const noSys = $('#sw-rs-contents').textContent.replace(/\s+/g, ' '); + out.warnsWhenNoSystemSnapshot = /no settings snapshot/i.test(noSys); + // submit() must route to the restore path, not the install payload. let routedTo = null; w.submitRestore = async () => { routedTo = 'restore'; }; @@ -151,6 +180,14 @@ chk "leaves the payload as a ref" "$(g .payloadCarriesRef)" true chk "and never as a value" "$(g .payloadCarriesNoPassword)" true chk "and is cleared from the DOM" "$(g .passwordClearedFromDom)" true +echo "the contents step separates the two snapshot kinds" +chk "a Settings section" "$(g .showsSettingsSection)" true +chk "an App data section" "$(g .showsAppSection)" true +chk "says why settings come first" "$(g .explainsSettingsFirst)" true +chk "shows when each was taken" "$(g .showsSnapshotDate)" true +chk "no domain heading when there are none" "$(g .noDomainsHeadingWhenEmpty)" true +chk "warns when there is no settings snapshot" "$(g .warnsWhenNoSystemSnapshot)" true + echo "submit" chk "routes to the restore path" "$(g .submitRoutedToRestore)" true diff --git a/scripts/restore/restore_inspect.sh b/scripts/restore/restore_inspect.sh index 16a1e9a..2d4f266 100644 --- a/scripts/restore/restore_inspect.sh +++ b/scripts/restore/restore_inspect.sh @@ -58,9 +58,24 @@ restoreInspectDomains() # # restore inspect [host] # +# A repository holds two DIFFERENT kinds of snapshot and the report keeps them +# apart, because they are restored by different machinery and mean different +# things to the person reading: +# +# system=config ONE snapshot of the whole configs tree — settings, logins, +# the domains, and every backup repository with its +# credentials. Restored first, because it is what makes the +# others reachable. +# app= ONE SNAPSHOT PER APP of that app's data directory, each +# with its own manifest, its own size and its own schedule. +# Separate so an app can be restored, moved or aged out on +# its own without touching the rest. +# +# Showing them as one flat list was confusing precisely because it hid that. +# # With no host it reports every host it found and picks the one with the most -# apps as the suggestion — a repository that has been pointed at two machines -# is a normal thing to end up with, and guessing silently is not. +# apps as the suggestion — a repository pointed at two machines is a normal +# thing to end up with, and guessing silently is not. restoreInspect() { local idx="${1:-}" want_host="${2:-}" @@ -69,82 +84,94 @@ restoreInspect() return 1 fi - local snaps - snaps=$(restoreFirstRunDiscover "$idx" 2>/dev/null) - if [[ -z "$snaps" || "$snaps" == "null" ]]; then + local raw snaps + raw=$(restoreFirstRunDiscover "$idx" 2>/dev/null) + # The function can print notices before the JSON, so take the array itself + # rather than assuming the whole of stdout is the document. + snaps=$(printf '%s' "$raw" | sed -n '/^\[/,$p') + if [[ -z "$snaps" ]] || ! jq -e 'type == "array"' >/dev/null 2>&1 <<< "$snaps"; then # The single most common cause by a distance, and worth saying plainly # rather than as "discovery failed". echo '{"error":"Could not read that repository — wrong password, or not a LibrePortal backup."}' return 1 fi - local -a hosts=() - local h - while IFS= read -r h; do [[ -n "$h" ]] && hosts+=("$h"); done \ - < <(printf '%s' "$snaps" | grep -o '"hostname":"[^"]*"' | cut -d'"' -f4 | sort -u) - - if (( ${#hosts[@]} == 0 )); then + local hosts + hosts=$(jq -r '[.[].hostname] | unique | .[]' <<< "$snaps" 2>/dev/null) + if [[ -z "$hosts" ]]; then echo '{"error":"No LibrePortal backups found in that repository."}' return 1 fi - local host="$want_host" + local host="$want_host" h if [[ -z "$host" ]]; then # The one with the most apps, not simply the first: a repository often # carries a stray snapshot from a machine that was only ever tested. local best="" best_n=-1 n - for h in "${hosts[@]}"; do - n=$(migrateDiscoverApps "$h" "$idx" 2>/dev/null | grep -c .) + while IFS= read -r h; do + n=$(jq -r --arg h "$h" \ + '[.[] | select(.hostname == $h) | .tags[] | select(startswith("app="))] | unique | length' \ + <<< "$snaps" 2>/dev/null) + [[ "$n" =~ ^[0-9]+$ ]] || n=0 if (( n > best_n )); then best_n=$n; best="$h"; fi - done + done <<< "$hosts" host="$best" fi - local -a apps=() - local a - while IFS= read -r a; do [[ -n "$a" ]] && apps+=("$a"); done \ - < <(migrateDiscoverApps "$host" "$idx" 2>/dev/null) + # The system snapshot: newest one tagged system=config for this host. + local system_json + system_json=$(jq -c --arg h "$host" ' + [ .[] | select(.hostname == $h) + | select(any(.tags[]?; . == "system=config")) ] + | sort_by(.time) + | if length > 0 then {present: true, date: (.[-1].time)} else {present: false, date: ""} end + ' <<< "$snaps" 2>/dev/null) + [[ -n "$system_json" ]] || system_json='{"present":false,"date":""}' - # --- assemble --- - local out='{' - out+='"host":"'$(_lpJsonStr "$host")'",' + # One entry per app, newest snapshot of each. + local apps_json + apps_json=$(jq -c --arg h "$host" ' + [ .[] | select(.hostname == $h) + | . as $s + | (.tags[]? | select(startswith("app=")) | ltrimstr("app=")) as $name + | {name: $name, time: $s.time} ] + | group_by(.name) + | map({name: .[0].name, date: (sort_by(.time) | .[-1].time)}) + | sort_by(.name) + ' <<< "$snaps" 2>/dev/null) + [[ -n "$apps_json" ]] || apps_json='[]' - out+='"hosts":[' - local first=1 - for h in "${hosts[@]}"; do - [[ $first -eq 0 ]] && out+=',' - out+='"'$(_lpJsonStr "$h")'"'; first=0 - done - out+='],' - - out+='"apps":[' - first=1 - local size_b size_h - for a in "${apps[@]}"; do - [[ $first -eq 0 ]] && out+=',' - # Size and date come from the app's own manifest where there is one. - # An older backup without a manifest still lists — it just has less to - # say about itself, which is better than being left out of the list. + # Sizes come from each app's own manifest. An older backup without one + # still lists — it just has less to say about itself, which is better than + # being left out of the list. + local sized='[]' a size_b size_h + while IFS= read -r a; do + [[ -z "$a" ]] && continue size_b=$(restorePreflightManifest "$idx" "$a" "$host" 2>/dev/null \ | tr -d ' \n\t' | grep -o '"size_bytes":[0-9]*' | cut -d: -f2) size_h="" [[ -n "$size_b" ]] && size_h=$(_restorePfSize "$size_b") - out+='{"name":"'$(_lpJsonStr "$a")'","size":"'$(_lpJsonStr "$size_h")'"}' - first=0 - done - out+='],' + sized=$(jq -c --arg n "$a" --arg s "$size_h" \ + '. + [{name: $n, size: $s}]' <<< "$sized") + done < <(jq -r '.[].name' <<< "$apps_json" 2>/dev/null) - out+='"domains":[' - first=1 - local d - while IFS= read -r d; do - [[ -z "$d" ]] && continue - [[ $first -eq 0 ]] && out+=',' - out+='"'$(_lpJsonStr "$d")'"'; first=0 - done < <(restoreInspectDomains "$idx" "$host") - out+=']}' - - printf '%s\n' "$out" + jq -nc \ + --arg host "$host" \ + --argjson hosts "$(jq -c '[.[].hostname] | unique' <<< "$snaps")" \ + --argjson system "$system_json" \ + --argjson apps "$apps_json" \ + --argjson sizes "$sized" \ + --argjson domains "$(restoreInspectDomains "$idx" "$host" | jq -Rsc 'split("\n") | map(select(length > 0))')" \ + '{ + host: $host, + hosts: $hosts, + # Domains live under system because that is where they come from — + # the configs tree, in the one system=config snapshot. Presenting + # them as a peer of the app list is what made the step confusing. + system: ($system + {domains: $domains}), + apps: [ $apps[] as $a + | $a + {size: (([$sizes[] | select(.name == $a.name) | .size] | first) // "")} ] + }' return 0 }