Storage defaults: hang them off the mount, not the app-data path

The advanced Storage step offered /mnt/disk/apps/libreportal-system as the
default home for LibrePortal's own tree. A registered location's path is where
APP DATA goes and is usually a subdirectory of the drive, so deriving anything
else from it nests that thing inside the app data — LibrePortal's own files
buried under it, on a path that reads as a mistake because it is one.

Both defaults now come off the location's mount point, which meant adding
"mount" to each entry in the storage feed; only the system block carried one.

  LibrePortal                    /mnt/disk/apps/libreportal-system
                              -> /mnt/disk/libreportal-system
  New apps, unregistered drive   /mnt/disk
                              -> /mnt/disk/libreportal-apps
  New apps, registered location  unchanged — it exists and may hold data, and
                                 proposing a different directory on the same
                                 drive would strand it

Names follow the layout the rest of the product uses (libreportal-system,
libreportal-containers, libreportal-backups) rather than a bare "apps", so a
drive shared with anything else stays legible.

collectStorage() no longer registers the drive picked for LibrePortal. A
storage location is somewhere app data lives; the system tree is not app data
and relocate creates that directory itself as root. Picking a drive there was
producing a location nobody asked for, on a mount chosen for something else.

Also in this change, from the Backup step:

  - The backend-specific fields are boxed under their own heading with a note,
    so choosing SFTP reveals "the SFTP part" rather than three more loose rows.

  - Fields had no vertical spacing. .setup-step gives its DIRECT children a
    16px gap, which is where every other step's fields get theirs; these sit a
    level deeper inside a .setup-section and inherited none of it, so each
    input ran into the next field's label.

  - Two field icons carried U+FE0F. Those codepoints have a text form and the
    selector only requests the emoji one, so they sat on a different baseline
    to the plain emoji beside them — the box measured perfectly centred while
    the glyph did not look it.

  - ?mode=restore&type=sftp makes the restore branch reachable by URL. Getting
    there previously took a click and a change event, so every screenshot and
    test had to drive the page before it could look at it.

Two test bugs fixed while doing it: a duplicate `const visible` in one scope
(a parse error, so the whole eval silently returned nothing), and a stub that
covered the POST but not the poll, leaving a 60s loop running that kept the
page from ever going network-idle.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
librelad 2026-08-29 06:48:03 +01:00
parent 42afc20ee0
commit 9bb9ed79a9
6 changed files with 265 additions and 46 deletions

View File

@ -1519,3 +1519,49 @@ body.setup-wizard-open .custom-select-popup { z-index: 10001; }
@media (max-width: 640px) {
.lp-relocate-banner { flex-direction: column; align-items: stretch; }
}
/* The fields one backend needs, boxed under its own heading. Loose rows
appearing and disappearing beneath a type dropdown give no signal that they
belong to the choice above them. */
.setup-subgroup {
margin: 4px 0 12px;
padding: 12px 14px 14px;
border-radius: 10px;
border: 1px solid rgba(var(--text-rgb), 0.13);
background: rgba(var(--text-rgb), 0.04);
display: flex;
flex-direction: column;
gap: 12px;
}
.setup-subgroup-title {
font-size: 0.72rem;
font-weight: 600;
letter-spacing: 0.09em;
text-transform: uppercase;
color: var(--accent-color, #4fc3f7);
opacity: 0.95;
}
.setup-subgroup-note {
margin: -6px 0 0;
font-size: 0.8rem;
line-height: 1.45;
color: rgba(var(--text-rgb), 0.62);
}
.setup-subgroup-title + .setup-subgroup-note { margin-top: -6px; }
/* .setup-step gives its DIRECT children a 16px gap, which is where every other
step's fields get their spacing. The restore step's fields sit one level
deeper, inside a .setup-section, so they inherited none of it and each
input ran straight into the next field's label. */
#sw-rs-fields {
display: flex;
flex-direction: column;
gap: 16px;
}
#sw-rs-fields + .setup-field,
#sw-rs-status:not(:empty) {
margin-top: 16px;
}
.setup-step .setup-section > .setup-field + .setup-field {
margin-top: 16px;
}

View File

@ -31,7 +31,7 @@ class SetupWizard {
this.stepNames = ['Start', 'Experience', 'Identity', 'Domains', 'Storage', 'Backups', 'Import', 'Recommended', 'Metrics',
'Backup', 'Contents', 'Rebuild'];
this.stepIcons = ['\u{1F9ED}', '\u{1F331}', '\u{1FA90}', '\u{1F6F0}\u{FE0F}', '\u{1F4BE}', '\u{1F6DF}', '\u{1F4E6}', '\u{1F6E1}\u{FE0F}', '\u{1F4CA}',
'\u{1F5C4}\u{FE0F}', '\u{1F50D}', '\u{267B}\u{FE0F}'];
'\u{1F4E6}', '\u{1F50D}', '\u{267B}\u{FE0F}'];
// 'new' | 'restore'. Chosen on Start; everything downstream reads it.
this.installMode = 'new';
// What `restore read` found: host, hosts, apps, domains, location_idx.
@ -115,6 +115,25 @@ class SetupWizard {
}
}
_applyModeFromQuery() {
try {
const q = new URLSearchParams(window.location.search);
if (q.get('mode') === 'restore') {
this.installMode = 'restore';
const radio = this.container.querySelector('input[name="sw-mode"][value="restore"]');
if (radio) radio.checked = true;
this.totalSteps = this._effectiveTotalSteps();
this.renderRestoreSource();
const type = q.get('type');
const sel = this.container.querySelector('#sw-rs-type');
if (type && sel && Array.from(sel.options).some(o => o.value === type)) {
sel.value = type;
sel.dispatchEvent(new Event('change', { bubbles: true }));
}
}
} catch { /* a malformed query must never stop the wizard opening */ }
}
_visibleSteps() {
return this.stepNames.map((_, i) => i).filter((i) => this._stepVisible(i));
}
@ -140,6 +159,11 @@ class SetupWizard {
// the order, and it makes a step reachable without clicking through the
// ones before it — which is what lets a screenshot or a headless test look
// at, say, Backups directly.
// ?mode=restore selects the restore branch, and ?type=sftp the backend
// within it, so any state of this wizard is reachable by URL. Reaching the
// restore steps otherwise takes a click and a change event, which means a
// screenshot or a test has to drive the page before it can look at it.
this._applyModeFromQuery();
this.showStep(this._stepFromQuery());
}
@ -428,7 +452,7 @@ class SetupWizard {
<span class="setup-tooltip" tabindex="0" data-tip="The password this repository was encrypted with. It is handed to the host through a one-shot reference, so it never lands in a task or a log.">?</span>
</label>
<div class="setup-input-row">
<span class="setup-field-icon setup-field-icon-emoji" aria-hidden="true">\u{1F510}</span>
<span class="setup-field-icon setup-field-icon-emoji" aria-hidden="true">\u{1F512}</span>
<input type="password" id="sw-rs-pass" class="setup-input-with-icon" placeholder="Unlocks the repository" autocomplete="off">
</div>
</div>
@ -775,7 +799,16 @@ class SetupWizard {
// Not an unmounted one: choosing it would put app data on a bare
// mountpoint, which is the failure this whole feature exists to avoid.
.filter(c => c.verdict !== 'refuse' && c.state !== 'unmounted')
.forEach(c => opts.push({ value: c.path, label: c.name ? `${c.name} (${c.path})` : c.path }));
.forEach(c => opts.push({
value: c.path,
label: c.name ? `${c.name} (${c.path})` : c.path,
// The drive, as opposed to the directory app data lives in. A
// registered location's path is usually a subdirectory of its mount,
// so anything derived from the path rather than the mount ends up
// nested inside the app data.
mount: c.mount || c.path,
registered: !!c.registered
}));
// Somewhere the scan did not find: a NAS mount, an LVM volume, a directory
// on a drive already in use. The scan lists whole filesystems, so anything
// that is a path rather than a disk had no way in before this.
@ -880,12 +913,30 @@ class SetupWizard {
}
// Where a chosen drive puts things when nobody says otherwise.
//
// Both defaults hang off the drive's MOUNT POINT, never off the selected
// location's path. A location's path is where app data goes and is usually a
// subdirectory — /mnt/disk/apps — so deriving the system tree from it gave
// /mnt/disk/apps/libreportal-system: LibrePortal's own files buried inside
// the app data, on a path that reads as a mistake because it is one.
//
// The names follow the rest of the product's layout — libreportal-system,
// libreportal-containers, libreportal-backups — rather than a bare "apps",
// so a drive shared with anything else stays legible.
_defaultPathFor(which, drive) {
if (!drive || drive === 'primary') {
return which === 'system' ? '' : (this.storagePrimary || '');
}
const base = drive.replace(/\/$/, '');
return which === 'system' ? `${base}/libreportal-system` : base;
const opt = (this._storageChoices() || []).find(o => o.value === drive);
const mount = String((opt && opt.mount) || drive).replace(/\/$/, '');
if (which === 'system') return `${mount}/libreportal-system`;
// An already-registered location keeps its own path: it exists, it may
// already hold data, and quietly proposing a different directory on the
// same drive would strand it.
if (opt && opt.registered) return String(drive).replace(/\/$/, '');
return `${mount}/libreportal-apps`;
}
// Exact paths — advanced only.
@ -1018,8 +1069,8 @@ class SetupWizard {
if (!box || box.dataset.rendered === '1') return;
box.dataset.rendered = '1';
const field = (id, label, tip, icon, ph, type) => `
<div class="setup-field" data-rs-group="${type}">
const field = (id, label, tip, icon, ph) => `
<div class="setup-field">
<label for="${id}">
${label}
<span class="setup-tooltip" tabindex="0" data-tip="${this.escapeHtml(tip)}">?</span>
@ -1030,6 +1081,17 @@ class SetupWizard {
</div>
</div>`;
// The fields a backend needs, boxed together under its own heading.
// Loose rows appearing and disappearing under the type dropdown gave no
// signal that they belong to the choice above them; a titled container
// makes "these three are the SFTP part" visible at a glance.
const group = (type, title, note, fields) => `
<div class="setup-subgroup" data-rs-group="${type}">
<div class="setup-subgroup-title">${this.escapeHtml(title)}</div>
${note ? `<p class="setup-subgroup-note">${this.escapeHtml(note)}</p>` : ''}
${fields}
</div>`;
box.innerHTML = `
<div class="setup-field">
<label for="sw-rs-type">
@ -1047,33 +1109,42 @@ class SetupWizard {
</select>
</div>
</div>` +
field('sw-rs-path', 'Folder',
"The repository folder itself \u2014 the one containing config, data/ and snapshots/, not the folder above it.",
'\u{1F4C1}', '/mnt/usb/libreportal-backups', 'local') +
field('sw-rs-ssh-user', 'SSH user',
'The account used to reach the server over SSH.',
'\u{1F464}', 'backups', 'sftp') +
field('sw-rs-ssh-host', 'SSH host',
'Hostname or IP of the server holding the repository.',
'\u{1F5A5}\u{FE0F}', 'nas.example.com', 'sftp') +
field('sw-rs-ssh-path', 'Remote folder',
'Path to the repository on that server.',
'\u{1F4C1}', '/srv/libreportal-backups', 'sftp') +
field('sw-rs-uri', 'Server URL',
'The full repository URL, as restic writes it.',
'\u{1F517}', 'rest:https://backup.example.com/', 'rest') +
field('sw-rs-s3uri', 'Bucket URL',
'The bucket holding the repository, as restic writes it.',
'\u{1FAA3}', 's3:s3.amazonaws.com/my-bucket', 's3') +
field('sw-rs-s3id', 'Access key ID',
'The key ID for that bucket. Its secret goes in the password field below.',
'\u{1F511}', 'AKIA\u2026', 's3') +
field('sw-rs-b2uri', 'Bucket',
'The B2 bucket holding the repository.',
'\u{1FAA3}', 'b2:my-bucket', 'b2') +
field('sw-rs-b2id', 'Account ID',
'Your B2 account or application key ID.',
'\u{1F194}', '', 'b2');
group('local', 'On this machine',
'A folder on a disk plugged into this server, or mounted on it.',
field('sw-rs-path', 'Folder',
"The repository folder itself \u2014 the one containing config, data/ and snapshots/, not the folder above it.",
'\u{1F4C1}', '/mnt/usb/libreportal-backups')) +
group('sftp', 'SFTP server',
'Reached over SSH, with the key or password this server already uses.',
field('sw-rs-ssh-user', 'SSH user',
'The account used to reach the server over SSH.',
'\u{1F464}', 'backups') +
field('sw-rs-ssh-host', 'SSH host',
'Hostname or IP of the server holding the repository.',
'\u{1F310}', 'nas.example.com') +
field('sw-rs-ssh-path', 'Remote folder',
'Path to the repository on that server.',
'\u{1F4C1}', '/srv/libreportal-backups')) +
group('rest', 'REST server', '',
field('sw-rs-uri', 'Server URL',
'The full repository URL, as restic writes it.',
'\u{1F517}', 'rest:https://backup.example.com/')) +
group('s3', 'S3 bucket',
'The secret key goes in the password field below, not here.',
field('sw-rs-s3uri', 'Bucket URL',
'The bucket holding the repository, as restic writes it.',
'\u{1FAA3}', 's3:s3.amazonaws.com/my-bucket') +
field('sw-rs-s3id', 'Access key ID',
'The key ID for that bucket.',
'\u{1F511}', 'AKIA\u2026')) +
group('b2', 'Backblaze B2',
'The application key goes in the password field below.',
field('sw-rs-b2uri', 'Bucket',
'The B2 bucket holding the repository.',
'\u{1FAA3}', 'b2:my-bucket') +
field('sw-rs-b2id', 'Account ID',
'Your B2 account or application key ID.',
'\u{1F194}', ''));
const sync = () => {
const type = (this.container.querySelector('#sw-rs-type') || {}).value || 'local';
@ -1987,13 +2058,18 @@ class SetupWizard {
));
}
// Drives to register: whatever either dropdown points at. Selecting a drive
// IS the request to register it — there is no separate tick to forget.
// Drives to register as storage LOCATIONS. Selecting a drive for app data IS
// the request to register it — there is no separate tick to forget.
//
// The LibrePortal row is deliberately not included. A storage location is
// somewhere app data lives; LibrePortal's own tree is not app data, and
// libreportal-relocate creates that directory itself as root. Registering
// the drive because it was picked there produced a location the user never
// asked for, pointing at a mount they had chosen for something else.
collectStorage() {
const out = [];
[this.storageSystemChoice, this.storageDefault].forEach((v) => {
if (v && v !== 'primary' && !out.includes(v)) out.push(v);
});
const v = this.storageDefault;
if (v && v !== 'primary') out.push(v);
return out;
}

View File

@ -631,6 +631,39 @@ checked nothing at all. They now probe the WebUI with `lp-shot --url` and curl:
if it answers HTTP then the browser is the only thing that can have broken, and
that is a failure, not a skip.
## 12.65 — Default paths hung off the wrong thing
The advanced Storage step offered `/mnt/disk/apps/libreportal-system` as the
default place for LibrePortal's own tree, which is wrong twice over.
A registered location's **path is where app data goes**, and is usually a
subdirectory of the drive — `/mnt/disk/apps`. Deriving anything else from it
nests that thing inside the app data. LibrePortal's own files are not app data
and have no business living under it.
Both defaults now hang off the location's **mount point**, which meant adding
`mount` to each entry in the storage feed (only the `system` block carried one
before):
| | before | after |
|---|---|---|
| LibrePortal | `/mnt/disk/apps/libreportal-system` | `/mnt/disk/libreportal-system` |
| New apps, unregistered drive | `/mnt/disk` | `/mnt/disk/libreportal-apps` |
| New apps, registered location | its own path | unchanged |
The names follow the layout the rest of the product already uses —
`libreportal-system`, `libreportal-containers`, `libreportal-backups` — rather
than a bare `apps`, so a drive shared with anything else stays legible. A
location that is already registered keeps its own path: it exists, it may
already hold data, and proposing a different directory on the same drive would
strand it.
`collectStorage()` no longer registers the drive picked for LibrePortal. A
storage location is somewhere *app data* lives; the system tree is not app
data, and `libreportal-relocate` creates that directory itself as root. Picking
a drive there was producing a location the user never asked for, pointing at a
mount they had chosen for something else entirely.
## 12.7 — The relocate hand-off, and why it stays a command
Asked directly: why is moving LibrePortal itself not just a button that runs at

View File

@ -67,6 +67,35 @@ read -r -d '' DRIVE <<'JS'
out.passwordHasIcon = !!document.querySelector('#sw-rs-pass')
?.closest('.setup-input-row')?.querySelector('.setup-field-icon');
// Fields for one backend are boxed under their own heading. Loose rows
// appearing beneath the type dropdown gave no signal that they belonged to
// the choice above them.
const shownGroup = () => {
const g = Array.from(document.querySelectorAll('.setup-subgroup'))
.filter(el => el.style.display !== 'none');
return g.length === 1 ? g[0].querySelector('.setup-subgroup-title').textContent.trim() : null;
};
const selType = (v) => { const s = $('#sw-rs-type'); s.value = v; fire(s, 'change'); };
selType('sftp'); out.groupForSftp = shownGroup();
selType('b2'); out.groupForB2 = shownGroup();
selType('local'); out.groupForLocal = shownGroup();
// Every visible field must clear the one above it. The step's 16px gap only
// reaches .setup-step's DIRECT children, and these sit a level deeper inside
// a .setup-section — so each input ran straight into the next field's label.
selType('sftp');
w.showStep(1);
await new Promise(r => setTimeout(r, 200));
const onScreen = Array.from(document.querySelectorAll('.setup-step[data-step="9"] .setup-field'))
.filter(el => el.offsetParent !== null);
let minGap = Infinity;
for (let i = 1; i < onScreen.length; i++) {
const a = onScreen[i - 1].getBoundingClientRect(), b = onScreen[i].getBoundingClientRect();
minGap = Math.min(minGap, Math.round(b.top - a.bottom));
}
out.visibleFieldCount = onScreen.length;
out.smallestFieldGap = onScreen.length > 1 ? minGap : null;
out.kinds = Array.from(document.querySelectorAll('#sw-rs-type option')).map(o => o.value);
const groupsFor = (t) => {
const sel = $('#sw-rs-type'); sel.value = t; fire(sel, 'change');
@ -96,11 +125,23 @@ read -r -d '' DRIVE <<'JS'
let stashedValue = null, sentBody = null;
w.stashSecret = async (v) => { stashedValue = v; return 'secret:' + '0'.repeat(32); };
const realFetch = window.fetch;
// Both halves are stubbed, not just the POST. readBackup polls the published
// document for a full minute before giving up, and leaving that loop running
// kept the page from ever going network-idle — the whole eval then died on
// the harness's 90s cap, which reads as "the browser failed" rather than as
// a test that never finished.
window.fetch = async (url, opts) => {
if (String(url).includes('/api/setup/restore/read')) {
const u = String(url);
if (u.includes('/api/setup/restore/read')) {
sentBody = JSON.parse(opts.body);
return { ok: true, json: async () => ({ ok: true, taskId: 't', nonce: 'n' }) };
}
if (u.includes('/data/system/restore_read.json')) {
return { ok: true, json: async () => ({
nonce: 'n', host: 'oldbox', hosts: ['oldbox'],
system: { present: true, date: '2026-08-28T13:10:02+01:00', domains: [] },
apps: [] }) };
}
return realFetch(url, opts);
};
$('#sw-rs-pass').value = 'hunter2-not-a-real-password';
@ -112,12 +153,9 @@ read -r -d '' DRIVE <<'JS'
out.payloadCarriesRef = !!(sentBody && sentBody.location && sentBody.location.password_ref);
out.payloadCarriesNoPassword = !!(sentBody && sentBody.location
&& !JSON.stringify(sentBody.location).includes('hunter2'));
// Now that the poll is stubbed too, the read completes rather than hanging.
await readPromise.catch(() => {});
window.fetch = realFetch;
// Deliberately NOT awaited: the stubbed response has no matching document to
// find, so readBackup polls for a full minute before giving up. What is
// under test already happened — what left the browser — and waiting for the
// 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
@ -184,6 +222,10 @@ chk "fields carry icons" "$(g .fieldsHaveIcons)" true
chk "fields carry tooltips" "$(g .fieldsHaveTooltips)" true
chk "so does the password field" "$(g .passwordHasIcon)" true
chk "local shows only its own" "$(g .localShowsOnlyLocal)" true
chk "sftp fields are boxed together" "$(g .groupForSftp)" "SFTP server"
chk "b2 fields are boxed together" "$(g .groupForB2)" "Backblaze B2"
chk "local fields are boxed too" "$(g .groupForLocal)" "On this machine"
chk "no field overlaps the next" "$(g '.smallestFieldGap >= 8')" true
chk "sftp shows only its own" "$(g .sftpShowsOnlySftp)" true
chk "empty path refused" "$(g .emptyPathRefused)" true
chk "relative path refused" "$(g .relativePathRefused)" true

View File

@ -115,7 +115,19 @@ read -r -d '' DRIVE <<'JS'
sysSel.value = disk; fire(sysSel, 'change');
out.systemRowAppears = !!$('#sw-path-system');
out.systemPathDefault = $('#sw-path-system') ? $('#sw-path-system').value : '';
out.systemPathWant = disk + '/libreportal-system';
// Off the drive's MOUNT, never the location's path. A location's path is
// where app data goes and is usually a subdirectory, so deriving from it
// produced /mnt/disk/apps/libreportal-system — LibrePortal's own tree buried
// inside the app data.
const chosen = (w._storageChoices() || []).find(o => o.value === disk);
const mount = String((chosen && chosen.mount) || disk).replace(/\/$/, '');
out.systemPathWant = mount + '/libreportal-system';
out.systemPathIsOutsideAppData = !out.systemPathDefault.startsWith(disk.replace(/\/$/, '') + '/');
// A storage location is somewhere APP DATA lives. LibrePortal's own tree is
// not app data, and relocate creates that directory itself — registering the
// drive because it was picked here produced a location nobody asked for.
out.systemChoiceRegistersNothingExtra =
(w.collectStorage() || []).every(v => v !== mount + '/libreportal-system');
// The system directory is not an app-data location and must never be
// registered as one — collectStorage takes the drive, not the subdirectory.
out.registersDriveNotSystemDir = (w.collectStorage() || []).includes(out.systemPathDefault);
@ -227,6 +239,8 @@ else
chk "path follows the chosen drive" "$(g .pathFollowsDrive)" "$(g .pathFollowsDriveWant)"
chk "LibrePortal row appears" "$(g .systemRowAppears)" true
chk "and defaults under that drive" "$(g .systemPathDefault)" "$(g .systemPathWant)"
chk "not nested in the app data" "$(g .systemPathIsOutsideAppData)" true
chk "and registers no extra location" "$(g .systemChoiceRegistersNothingExtra)" true
chk "system dir is not registered" "$(g .registersDriveNotSystemDir)" false
echo "editing a path"

View File

@ -53,6 +53,14 @@ webuiGenerateStorageCandidates()
loc_pct=$(df -Pk "$path" 2>/dev/null | awk 'NR==2 {gsub("%","",$5); print $5}')
locations+="{\"id\":\"$(_lpJsonEsc "$id")\",\"name\":\"$(_lpJsonEsc "$name")\",\"path\":\"$(_lpJsonEsc "$path")\",\"state\":\"$(_lpJsonEsc "$state")\",\"apps\":\"$(_lpJsonEsc "$apps")\""
locations+=",\"size\":\"$(_lpJsonEsc "$loc_size")\",\"free\":\"$(_lpJsonEsc "$loc_free")\""
# The MOUNT POINT, not just the path. A location's path is where app
# data goes — often a subdirectory like /mnt/disk/apps — and deriving
# anything else from it nests that thing inside the app data. The
# wizard needs the drive itself to offer a sane default for
# LibrePortal's own tree.
local loc_mount
loc_mount=$(findmnt -no TARGET --target "$path" 2>/dev/null | tail -1)
locations+=",\"mount\":\"$(_lpJsonEsc "${loc_mount:-$path}")\""
locations+=",\"fstype\":\"$(_lpJsonEsc "$loc_fs")\",\"used_pct\":${loc_pct:-0},\"removable\":false}"
done < <(runStorage verify 2>/dev/null)
locations+="]"