Phase 1 is built. Phase 2 hit a blocker worth deciding rather than
silently working around.
The WebUI cannot run restic, so a password typed in the browser has to
reach the host. Both existing channels leak it. The task command string —
which is what the Backup page ALREADY uses for this exact field — lands in
a task JSON under frontend/data/tasks at 0644, world-readable, and shows
in ps while the task runs. A file in that directory does not work either:
the container writes as dockerinstall, the manager runs as libreportal,
and at 0640 the manager cannot read it (verified on the live box).
So this is an existing product-wide weakness that the restore branch
happens to surface, not one the feature would introduce — and the restore
case is its sharpest form, since that password is the key to every backup
the user has.
Recommends a one-shot secret drop: the ownership helper already solves the
mirror-image case (_webui_bind_access chowns MANAGER:cowner 0640 so the
container can read manager-owned files), so the reverse is a small,
well-scoped addition — a directory owned cowner:MANAGER 0730 that the
container drops a 0640 file into, which the manager reads once and
unlinks. Worth doing because it also fixes the Backup page.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing prompted anyone to configure backups, so the people most likely
to need a restore were the least likely to have one. The wizard now asks,
once, with the drives it already scanned as the options.
Three messages, because the honest answer differs by choice:
declined nothing is protected until you set it up
same drive still covers deletion, a bad update and ransomware — not
this disk failing, since the data and its only copy go
together
another drive the repository is encrypted; write the password down
somewhere other than this machine
That last one matters more than it reads. An encrypted repository cannot
be opened with anything stored inside itself, and the location password
lives in the system config, which is inside the backup. On a rebuilt
machine the user must supply it by hand — so the wizard says so up front
rather than letting them discover it during a restore.
The password is deliberately NOT echoed by setupApplyConfig: task output
is logged, and a secret in a log is a secret you have to treat as leaked.
It is shown on the Backup page, which is what the wizard tells the user.
locationAdd creates a location disabled, so the applier enables it and
runs engineInitLocation — an un-initialised destination silently backs up
nothing, which is the worst possible way to have "configured backups".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Proposal for making "I'm rebuilding my server" a first-class first-run
path. Recording it before building because the useful finding is how
little is actually missing.
The restore machinery is already there: locationAdd connects a repo,
restoreFirstRunDiscover reads it without touching live state,
migrateDiscoverHosts/Apps enumerate what is in it, backupRestoreSystemConfig
brings settings and credentials back, restoreFirstRunBulk restores many
apps from another host, migrateUrlRewrite fixes host-bound CFG_* fields,
and storageRestoreAppTo already handles a snapshot whose paths don't
match this machine. What is missing is the front door and one screen.
That screen is the reconciliation diff — a backup describes a machine
that no longer exists, and the failure mode worth designing against is
re-deciding those facts silently. Every difference is detectable before
anything is written: a storage location that doesn't exist here, an app
bigger than the target drive, domains pointing at the old host, ports
already taken, apps this version no longer ships.
Two things the doc is deliberately blunt about.
An encrypted repository cannot be opened with anything inside itself. The
location password lives in the system config, which is inside the backup,
so on a fresh machine the user must supply it by hand — that is what
encryption means, not a bug. It has to be asked for early, because a user
who never wrote it down does not have a backup and should learn that at
step 2 rather than step 5.
And "upload the backup file" doesn't match the engines: restic, borg and
kopia write to a repository, not a file. Local means navigating to a path;
remote means credentials. A genuine single-file import is a separate
feature (a portable per-app export built on the manifest) and shouldn't
be conflated with the backup engines.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Root-run command to move the system and/or containers root to another
disk after install, re-baking everything that carries those paths: the
nine root-owned helpers, the CLI wrapper, the systemd unit and the
WebUI's own compose bind-mounts.
Deliberately NOT in the manager's scoped sudoers, and symlinked into PATH
like the uninstaller. Moving a root re-bakes the very helpers the sudoers
allowlist trusts, so a helper that did it from a caller-supplied path
would hand the manager the entire trust boundary those helpers exist to
defend. A human with real root runs this; the WebUI can only print the
command, which is what the Storage step now does.
Copy-verify-then-leave, never move: the source tree is not removed at all
— the command tells you to delete it once you have confirmed the WebUI
works. An interrupted run therefore leaves a working install behind
rather than half of one, and the pre-relocation copies of every
root-owned file are kept under $lp_lib_dir/.relocate-<timestamp>/.
Admission mirrors libreportal-storage: absolute, no "..", not a protected
system path, not already in use, must be an empty directory, roots must
not nest, and space checked with 10% headroom.
One bug worth recording, caught on the first test run against a live
install: _validate_target was called inside $(...), and `die` runs `exit`
— which inside a command substitution kills only the subshell. Every
refusal silently became "proceed" and the relocation ran. No damage (the
copy steps were guarded on a now-empty variable, so the re-bake wrote
identical values and only the service bounced), but it is exactly the
difference between a refusal and an unintended relocation. It now sets a
global and returns, so `die` exits the script it is meant to.
footprint_version -> 7 for the new root-owned executable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reworks the Storage step: two labelled choices with tooltips at the top —
LibrePortal and New apps — and the drive list underneath as reference.
The drive cards lose their checkboxes. Choosing a drive in a dropdown IS
the request to register it, so a separate tick was a second way to say the
same thing, and the way you end up with a drive ticked that nobody
selected. Cards are now informational plus Details.
Both dropdowns only render when there is a second drive; with one disk
both answers are forced and a pair of selects showing one option each is
furniture.
Moving LibrePortal's own tree cannot be a WebUI action. It re-bakes the
six root-owned helpers, the systemd unit and the WebUI's own bind-mounts
— real root, not the scoped sudo the manager holds. A helper that re-baked
the other helpers from a manager-supplied path would hand the manager
exactly the trust boundary those helpers exist to defend. So picking a
different disk for LibrePortal surfaces the root command to run rather
than pretending the wizard can do it; the payload carries the choice so
the finish screen can repeat it. libreportal-relocate follows.
Also drops "itself" from the installer's wording.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two fixes.
The wizard's "new apps store their data on" dropdown was a bare native
<select>. The OS draws that popup and ignores our CSS, which is why it
came out as stock white chrome — the WebUI already solves this with
custom-select.js, which enhances any select.form-control into a themed
button and list. It just needed the class.
And the installer now asks for the two roots independently rather than
only app data. I had argued one question was simpler, and for a desktop
it is — the control plane is ~20 MB and moving it gains nothing. But on a
small board with an 8 GB eMMC and a USB SSD you want both moved, and
there was no way to say so without knowing the flags exist. Still one
disk list and two short questions; each is skipped if its flag was
already passed.
Fixed a bug the test caught immediately: _initAskDisk returns the chosen
path on stdout, and it was printing the prompt there too, so the question
text became part of the answer — the system root ended up named after its
own prompt. Prompts go to stderr now, stdout is the return channel.
Verified every combination under a pty: both default, apps only, both
moved, system only, and invalid-then-valid.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
--containers-dir has existed for a while and nobody running the curl|bash
installer ever learned it existed, so a box with a 4 TB disk beside a
small system SSD quietly put everything on the SSD. The installer now
asks, once, in the shape an OS installer asks it.
Deliberately narrow, because a question you answer badly is worse than no
question:
* app data only. The control plane is ~20 MB and never grows; the thing
worth placing is the data. Offering all three roots would be three
questions nobody can answer at that moment.
* a SUBDIRECTORY on the chosen disk, never its mount point — that keeps
the "root only ever takes an empty directory" rule intact and leaves
whatever is already on the disk alone.
* candidates exclude anything on the same filesystem as /, since placing
data there gains nothing.
It stays out of the way: skipped when unattended, when --containers-dir
was passed, when there is no TTY, and when the scan finds nothing else —
a prompt with one possible answer is not a question. Self-contained
(findmnt only), since scripts/ is not necessarily loadable that early.
Verified all six paths under a pty: default, valid pick, out-of-range,
non-numeric, no TTY, unattended, explicit flag, and no candidates.
Docs updated: the installer prompt is shown, and the section that said
locations are fixed after install now explains what actually is fixed
(the three roots) versus what isn't (per-app placement, storage add, and
CFG_STORAGE_DEFAULT).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Per-app placement worked but had no default: a box with a big second disk
meant setting CFG_<APP>_STORAGE on every app individually. CFG_STORAGE_DEFAULT
fixes that, and the wizard asks for it in one line.
CFG_<APP>_STORAGE now has three states rather than two, and the third is
the point:
<name> this app goes there, whatever the default says
primary this app goes on the install-time root, explicitly
default no opinion — follow CFG_STORAGE_DEFAULT
Templates ship "default", so the setting reaches every app without
touching 37 configs, while an app that was deliberately placed keeps its
placement. "primary" is new, and needed: without it there was no way to
say "keep this one on the system disk" once the global default moved.
A default naming a location that has since been removed falls back to the
primary root rather than refusing — a disk that got unregistered must not
make apps un-installable.
The wizard asks only once a second drive is ticked; with nothing ticked
there is one possible answer and a control would be furniture. It sets a
default, not a placement, and the value stored is the location NAME, so it
survives the disk being remounted elsewhere.
scripts/dev/lp-storage-default-test covers all three states plus the
removed-location fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The system disk rendered as a checked-and-disabled checkbox, which reads
as "a choice that is locked". It isn't a choice at all — apps fall back
to it whatever happens — so it now carries no control, and is a <div>
rather than a <label> since there is nothing to label. A spacer keeps its
text aligned with the real checkboxes below.
An "Additional drives" divider separates the two kinds of row: above it
is where apps go regardless, below it is what you can opt into. It only
renders when there is something to divide, so the single-disk case is
still one card and a line of text.
Verified both states.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
I had the bar filling with FREE space. It was internally consistent with
the text next to it and completely wrong in practice: every file manager
fills a capacity bar with USED space, so a nearly-empty disk rendered as
a nearly-full bar and read as "this drive is full".
Windows Explorer is the reference for exactly this pairing — a used-fill
bar beside "808 GB free of 912 GB" — so the card now matches it. Amber
above 75% used, red above 90%.
The cosmetic problem that led me to invert it (a low-usage bar looking
like a broken widget rather than an almost-empty one) was the track's
contrast, not the direction. Fixed where it belonged, in CSS.
Also drops the "Drives that can't hold app data are greyed out" line: the
cards and their badges already carry that, so it was a standing sentence
explaining something visible.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The card said "911.9G · 808.4G free · ext4". The filesystem type is a
Details row, not something you choose a drive on, so it goes.
On percentage vs size: which one matters depends on the question. This
step asks "will my data fit?", and absolute free space is what decides
that — a 4 GB disk that is 89% free is still useless for a media library.
Percentage answers "is this filling up?", a health signal rather than a
placement one. So the text carries the magnitude ("808.4G free of 911.9G")
and a thin bar carries the proportion, which is what the eye reads
fastest, with no second number competing with the first.
The bar fills with FREE space, not used. Filling by usage made a healthy
7%-full disk render as an almost-empty track that read as a broken widget
— and it pointed the opposite way to the text beside it. Filled = room to
spare, draining = filling up, matching the words. It turns amber below
25% free and red below 10%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The badge had margin-left only, so on a one-line card the pill sat flush
against the drive size — "default 911.9G". Symmetric margin now, since
text follows it on the same line as well as preceding it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The step opened with a two-line paragraph explaining what storage
locations are. Every other field in the wizard puts that kind of
background behind a "?" — same markup, same component — so this now does
too, and the step is a title, two one-line drive cards, and the buttons.
The footer line was repeating the first half of that paragraph. It now
carries only what is actionable while choosing: that a greyed drive has
its reason in Details, or that no other drives were found.
Verified the bubble renders with the full text.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three changes to the Storage step.
The system disk is now a first-class entry — pinned first, ticked, and
locked, since apps fall back to it and it therefore cannot be deselected.
Its Details work like any other drive's, which is the whole point on a
single-disk box: the step now answers "where does my data actually go?"
instead of being skipped and answering nothing. The step is consequently
unconditional; the note changes to explain that no other drives were
found rather than the step vanishing.
The system entry is excluded from the submitted payload — it is already
the primary root, and asking the helper to register it would (correctly)
be refused for nesting.
Cards are one line again. Listing every warning under each drive pushed
them to three lines and made the step tall for no gain: the badge already
carries severity and Details carries the explanation. The note now says
to open Details for the reason rather than claiming it is on the card.
Badge colours were dark-on-light, which against the wizard's mid-blue
glass read as muddy grey — the "needs care" pill in particular. Switched
to light-on-dark, legible without shouting over the drive name.
Verified with lp-shot in both states: system disk alone, and system disk
plus a second drive.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The details popup was a hand-rolled overlay instead of openEoModal, which
every other modal in the WebUI uses. Two things were wrong with that.
It was appended to the wizard container rather than document.body. The
wizard has a backdrop-filter, which makes it a containing block, so the
modal's position:fixed resolved against the wizard instead of the
viewport — the backdrop covered part of the screen and the panel sat
off-centre.
And once switched to eo-modal it still didn't appear at all: .eo-modal is
z-index 1100 while the wizard overlay is 9999, so it rendered behind the
wizard and the Details button looked inert. Raised only under
body.setup-wizard-open, so no other overlay's stacking is touched — the
wizard is the exceptional full-screen surface, so it is the wizard's
stylesheet that declares modals must sit above it.
The bespoke modal chrome is gone from setup-wizard.css; what remains is
the storage-specific content (spec table, checks list, fstab box) that
renders inside .eo-modal-body. Header, sections, backdrop, Escape,
click-outside and the footer action now all come from the shared helper.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Storage step was a technical dump: every check's full sentence
concatenated onto the card, so the fstab line the user is meant to act on
was buried in prose nobody reads.
The card now shows plain facts and at most two short flags — "Low on
space · Won't be mounted after a reboot" — with everything else behind a
Details button. The modal carries the technical spec (device, UUID, mount
options, removable), every check with its full explanation, and the
fstab offer.
That needed the shell to stop joining checks into one string: the
generator emits a record per check, plus the fstab line as its own field,
so neither the card nor the modal has to parse anything back out of the
other.
The screenshot caught a bug this restructure introduced: summaries keyed
on check id alone, so a PASSING check printed the failure wording next to
a green tick — "This drive's format can't store file ownership" above
"Filesystem: ext4". Now severity-aware.
On writing /etc/fstab — §1 ruled it out and §6.3 now records why that
reverses. The warning is useless to the audience this is for: "add this
line to fstab" assumes SSH, root, an editor, and knowing what fstab is,
and the likely outcome is a reboot where nothing starts. What makes it
defensible is nofail + x-systemd.device-timeout, which mean a missing
device can never block boot — without that pair it would stay a non-goal,
because the failure being risked (an unbootable machine) is worse than
the one being fixed.
Enforced in the root helper: UUID never /dev/sdX, append inside a marked
block, refuse a target or UUID already described, refuse the root
filesystem, require a live mount, timestamped backup, and
`findmnt --verify` before the file is installed — a file that doesn't
parse never reaches /etc. Opt-in only.
Verified against a real filesystem: entry added and verifies, the
persistence warning then disappears on the next scan, and duplicate /
root-fs / non-mountpoint / relative are each refused with the reason.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
webuiGenerateStorageCandidates now runs as part of webuiSystemUpdate, so
frontend/data/storage.json exists without anyone remembering to generate
it — the wizard reads it to decide whether its Storage step appears, and
the Disks view reads the same file, so the two can never disagree.
Warnings arrive from the shell joined with "; ". Rendering that verbatim
produced one run-on paragraph that buried the fstab line the user is
supposed to copy, so the card splits them back onto separate lines.
Verified on the live install with lp-shot: with one filesystem the wizard
shows "Step 1 of 4" and the Storage step is correctly absent; with a
second filesystem attached it becomes "Step 4 of 5" with the drive
carrying a "needs care" badge and both warnings legible. That also
exercises the visible-step mapping in both directions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 3 of docs/roadmap/storage-locations.md.
The step appears only when the candidate scan finds a filesystem
LibrePortal isn't already using, so the single-disk case — which is most
boxes — is completely unchanged. It sits before Recommended because a
location has to exist before an app can be placed on it.
Supporting two conditional steps meant the wizard could no longer treat
'position in the DOM' and 'step index' as the same number: Metrics was
advanced-only and got away with 'length minus one', but a step hidden in
the MIDDLE leaves a gap. Navigation, progress, validation and submit now
all run off _visibleSteps(), and section matching is by data-step rather
than DOM position.
Unusable candidates render greyed WITH the reason rather than being
filtered out — 'why isn't my drive listed?' is a support burden, and
'exFAT can't store file ownership' is actionable. The step is skipped
only when nothing usable was found at all.
What the wizard sends is a request, not an instruction: setupApplyConfig
feeds each path through storageAdd, so the fitness checks and the root
helper's admission rules both re-run regardless of what arrived in the
payload.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The phasing table now reflects reality rather than intent: 0, 1, 2 and 4
are done, 5 and 6 are partial (the migrate "unknown location" prompt and
the Disks WebUI page are the gaps), 3 is untouched.
§10.8 records why the app->location index detonated, because the mistake
is easy to repeat and the blast radius was the whole machine. The file's
stated requirements — manager-owned, on the system disk — are both true
of configs/, which is why it looked like the right home. The property it
violated is that configs/ is SOURCED, so a TSV of "<slug> <path>" is a
script, and the row for the app named `libreportal` made it a
self-invoking one.
The generalisable rule is in there too: ask what a directory's contract
is, not just who owns it. configs/** is executed; containers/<app>/**.config
is executed. Both are load-bearing conventions a plain data file silently
breaks.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Restore built its restic include filter from THIS host's containers root:
engineRestoreSnapshot "$idx" "$id" "/" "$containers_dir$app"
restic reproduces a snapshot's absolute paths, so that only works when
both sides agree byte-for-byte. LibrePortal has shipped configurable
roots for a while, so restoring a snapshot taken on a host installed with
--containers-dir=/mnt/ssd/apps onto a default host matched no include
path and restored NOTHING — with no error, because an include filter that
matches nothing is not a failure. Storage locations turn that from a rare
cross-host case into an ordinary one.
storageSnapshotSourcePath asks the repository where the app actually
lived. storageRestoreAppTo restores in place when that agrees with where
the app belongs here, and stages-then-moves when it does not — which is
also what makes "restore this app onto a different disk" possible at all.
Both restore_app_start.sh and resticRestoreAppLatest go through it, and
both fall back to the old behaviour when a snapshot does not report its
paths, so older snapshots restore exactly as before.
The move into place runs as root (app-adopt) for the same reason app-move
does: a restored tree carries container sub-UIDs the manager cannot
recreate. Staging is constrained to the restore/migrate area and the
destination is validated against the root-owned registry, so neither end
is taken on trust from the caller.
The manifest now records where an app lived — location name, path and fs
uuid. The name is what travels, since a path means nothing on the other
host; the rest is for diagnostics and for answering "is this the same
disk?" during a migrate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 2 and 4 of docs/roadmap/storage-locations.md. Apps can now be
placed on a location and moved between them.
CFG_<APP>_STORAGE lands in all 37 app templates, holding a location NAME
rather than a path: names survive a migrate to a host with different
disks, paths do not. The 11 infrastructure apps that other apps reach by
literal path (traefik, prometheus, grafana, adguard, gluetun, crowdsec,
headscale, dashy, pihole, unbound, wireguard) are pinned. libreportal
itself never gets the key — it is pinned structurally by webuiDir.
Pinning needed no second config key. "Pinned" is not a fact about a value,
it is a statement about whether the field may be edited, so it goes in the
comment beside **ADVANCED** and **DEV** as **READONLY**, and the field
factory renders those disabled. That marker earns its keep beyond this
feature: derived fields already warned in prose that editing them does
nothing (crowdsec.config:72) next to a perfectly editable input.
storage_app_config.sh keeps the comment honest — it carries the resolved
path for hand-recovery and regenerates the dropdown from the registry, but
only writes when something actually changed, since the app .config is
user-editable and lives in the container-owned tree.
app move stops the app (a live copy of a running Postgres is a corrupt
copy), snapshots it, copies, verifies, and only then removes the source.
The copy runs in libreportal-ownership because it must: app data holds
rootless sub-UID files the manager can neither read nor recreate.
Verified against two real ext4 filesystems that a cross-device move
preserves uid 231141 and the payload, and that the source survives every
refusal path — unregistered destination, the WebUI app, a traversal in the
app name, and an occupied destination.
Task titles registered in both tables, with a specific rule so a move
renders as "Nextcloud - Move to bigdisk" rather than the generic fallback
dropping the destination. lp-task-names could not be run to confirm — it
borrows the WebUI container's node and no containers are running.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1 of docs/roadmap/storage-locations.md — locations can now exist.
Nothing places an app on one yet; that is phase 2.
libreportal-storage is the only writer of the root-owned registry, and
its admission rules are what make adding a location safe: absolute and
canonical (a symlinked path is refused), outside the protected system
set, non-nesting with any known root in either direction, and EMPTY — or
already carrying our marker, which is the adopt case for a drive that
already holds app data. Root only ever chowns an empty directory, so
acceptance cannot hand away anything that existed. The parent must also
not be manager-writable, which is what closes the validate-then-chown
race; /mnt and /srv qualify, a path inside the manager's home does not.
The fitness checks answer a different question — "will app data actually
work here" — and grade rather than refuse. Only checks 1-5 (filesystem
type, mount options, ownership, sub-UID range, write/read-back) can block.
Reboot persistence and removability warn, because both describe supported
setups and start-up is already gated by the marker test.
The ownership probe had to move into the root helper. For a candidate the
directory is not ours yet — a fresh /mnt/disk is root-owned 0755 — so an
unprivileged probe could only ever report "cannot create a directory
here", which says nothing about the filesystem. Verified against a real
loopback ext4: it now reports ownership, sub-UID and read-back cleanly.
The Disks view unions the registry with attached hardware, registry
first. Verified on the live box that pulling a drive leaves its row in
place as not-attached, naming the app stranded on it, rather than the row
silently disappearing at exactly the moment someone needs it.
Also registers storage_scripts with both loaders, adds the CLI category
(auto-dispatched by cli_initialize.sh), and bumps footprint_version to 6
for the new root helper and the widened sudoers allowlist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to 928e244, which stopped configs/ subdirectories being sourced
without a .category marker. That closed the hole; this removes the thing
that fell into it.
storageIndexFile pointed at configs/storage/app_locations. The file's
requirements are only "manager-owned" and "not on a removable disk" —
configs/ satisfies both, which is why I put it there, and it was still
wrong: that tree carries a third property the file violates. sourceScanFiles
SOURCES what it finds under configs/, and sourcing means executing.
The index is a TSV of "<slug><TAB><root>", which bash reads as a command
and its argument. Harmless while no slug matched a real executable. The
row for the app named `libreportal` armed it, because that IS the CLI on
PATH: sourcing ran `libreportal /libreportal-containers`, which re-entered
the scan, which sourced the file again — one process pair per level until
the host OOMed and took the desktop session with it.
It now lives at $system_dir/storage/app_locations, with a one-shot
migration so an install that already has an index keeps knowing where its
apps live rather than silently forgetting. libreportal-ownership
reconciles the new directory, and scan_files.sh gained a note that
configs/storage/ carries no .category on purpose.
scripts/dev/lp-configs-guard-test covers both ends: the index never lands
in configs/, a legacy one migrates, and a file of the exact detonating
shape placed in an unmarked configs/ subdirectory is not executed while a
marked category still loads.
Also wires sourceStorageLocations into the config scan beside
sourceBackupLocations — per-location configs sit at depth 3, below the
generic scan, and need their own walker like the backup ones do.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sourceScanFiles sourced every file two levels deep under configs/, and
sourcing means executing. A directory used there as ordinary storage
therefore turned its contents into a script.
storageIndexSet caches an app -> root TSV at configs/storage/app_locations,
with no .category marker alongside it. Every line is `<slug><TAB><path>`,
which bash reads as a command and its argument. That stayed invisible while
no slug matched a real executable — and became a fork bomb the moment the
index recorded the app named `libreportal`, because that IS the CLI on PATH:
sourcing ran `libreportal /libreportal-containers`, which re-entered the same
scan, which sourced the file again, one process pair per level until the host
died of OOM. Every CLI invocation on the box detonated it, the task
processor's own poll included, so the machine black-screened out of memory
minutes after each boot.
Files in a SUBDIRECTORY are now sourced only when that directory carries
.category — the contract commandReloadConfigs already enforces in the CLI
wrapper, and one every real config category (webui, general, security,
backup, network) already satisfies. Files directly in configs/ are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The main sweep — ~260 call sites across ~100 files move from string
concatenation on a single root to appDir/storageAppDirs/storageAppConfigs.
On a single-root install the resolved paths are identical, so this is a
no-op until a location is registered.
Enumerators were the interesting half. `for d in "$containers_dir"/*/`
appears in the menus, the registry/artifact scanners and the DNS setup —
and a shell glob cannot list a rootless 751 tree at all, which is the
same bug config_find_file.sh already documents in a comment. Routing them
through storageAppDirs (which enumerates as the owning user) fixes that
alongside the multi-root work.
Three places needed judgement rather than substitution:
db_app_scan.sh deletes database rows and port allocations for apps whose
folder is missing, and reaps "empty" app dirs. With a storage location
unmounted, every app on it looks exactly like that. Each of those
branches now gates on appStorageAvailable first — an app on an unplugged
drive is skipped with a notice, never deleted.
instance_create.sh rewrites cloned hooks so an instance touches its own
directory instead of the base app's. Its sed matched ${containers_dir}<type>,
which this sweep just replaced with $(appDir <type>) — so it would have
silently stopped redirecting, and an instance would have written to the
original's files (the adguard auth adapter case its own comment warns
about). Now matches both appDir forms, verified against bare, quoted,
unrelated-app, legacy and prose cases.
peer_shell/peer_pull streamed and extracted relative to the primary root.
Both now use the app's own root, and peer_shell keeps a single-root
fallback since it runs as a restricted SSH shell with no LibrePortal env.
Also fixes a pre-existing bug found on the way: webui_app_config.sh
tested "$containers_dir/frontend/data/last_update", one level short of the
real tree under the libreportal app dir, so the WebUI refresh trigger
after a config update has never once fired.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two mechanical sweeps, no behaviour change on a single-root install.
The 14 `[[ "$p" == "$containers_dir"* ]]` prefix tests that decide
manager-vs-container-user elevation become pathIsContainerData, so a file
on a second storage root is no longer misclassified as manager-owned —
which would have written it with the wrong owner and failed later, far
from the cause. The 65 references to the WebUI's own tree become
webuiDir(), which is pinned to the primary root by design.
Two traps found while doing it:
run_privileged.sh is sourced directly by init.sh without paths.sh, so it
needs a fallback. Defining one named pathIsContainerData was wrong:
generate_function_manifest.sh indexes top-level definitions, and the
resulting autoload stub would have shadowed the real multi-root
implementation with the primary-only fallback — silently classifying
every file on a second disk as manager-owned, which is exactly the bug
this sweep exists to prevent. Renamed to _runCfgIsContainerPath, which
delegates when the real one is loaded.
setup_lock.sh built its path in a top-level assignment, so it was
evaluated at source time and needed the file flagged eager. Made it a
function instead: the path resolves on call, and the file drops off
LP_EAGER_FILES entirely.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 0 of docs/roadmap/storage-locations.md — the resolver layer. No
behaviour change yet: with no registry present, every function here
returns exactly what the old single-root code did, which is what makes
the ~200-site sweep that follows safe to land incrementally.
primaryRoot / webuiDir the install-time root, and the one tree that
never moves
storageRoots every registered root, primary first
storageRootAvailable marker present == drive mounted
pathIsContainerData replaces the `== "$containers_dir"*` idiom
that picks manager vs container-user elevation
appDir / appDirSlash THE resolver, memoised
storageLocationPath/Name name <-> path, via the root-owned registry
storageIndexGet/Set app -> location cache
appDir resolves discovery-first: whichever root actually holds
<slug>/<slug>.config wins, so a hand-move or half-finished migration
self-heals rather than corrupting.
The index exists because of a bug the unit test caught immediately.
Discovery cannot see an unmounted disk, so an installed app on an
unplugged drive looked identical to a brand-new app — and the fallback
handed back the PRIMARY root. Docker would then have created the bind
mounts there and booted the app empty on the wrong disk, which is the
precise failure the availability design exists to prevent. The index is
manager-owned (deliberately not on the removable disk: it must be
readable exactly when that disk is absent), consulted only when the scan
comes up empty, and rewritten by every successful scan so the disk stays
authoritative whenever it is actually present.
Availability is gated in appDir alone rather than at each caller: every
site reaches it by construction. It returns non-zero AND prints an
unusable sentinel path, so the many callers that will never check $?
still fail loudly on something harmless.
scripts/dev/lp-storage-test covers all of it against a throwaway tree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolves open question 1. The device becomes the organising concept and
the two registries become roles on it, so a user with one drive holding
both app data and snapshots stops seeing "bigdisk" twice meaning two
different things.
It extends a page that already exists rather than adding one:
/admin/system/storage is currently the Docker disk breakdown, and
webui_system_metrics.sh already builds a disks array from df. The view is
that array enriched with lsblk -J -e7 (model, transport, rotational,
removable — with -e7 dropping the ~15 snap loop devices a desktop box
carries) and joined against both registries.
The union is the part that matters more than the enrichment: a registered
drive that is currently unplugged does not appear in lsblk at all, and
that is exactly when someone opens this page. So rows come from the
registry first and attached hardware second — a missing device still
renders, marked not attached, naming the apps stranded on it. Rows key on
fs_uuid, since /dev/sdb1 reorders across reboots and would scramble the
table.
This also gives every standing badge from §6/§6.1/§6.2 a natural home,
because each is a property of the device rather than of either registry,
and it turns "type the same path into two different pages" into actions
on the row you are already looking at.
Two limitations written down rather than discovered later: ZFS datasets
report distinct st_dev but share a pool, so grouping by device
over-reports free space (btrfs subvolumes are fine); and the page stays
read-only about the system — it can show the fstab line to add, never
add it.
Phase 6 now builds this instead of a separate Storage page, which with
disks as the frame would have reintroduced the split this resolves.
Also folds in §10.5 — the dashboard gauge becomes a summary linking here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolves open question 2. One big disk holding both the app data and its
snapshots is what most people actually have, and two facts make it cheap:
reconcile() already chowns CONTAINERS_DIR and BACKUPS_DIR to the same
container user, so there is no permission negotiation; and sibling dirs
on one filesystem don't nest, so §3 admits them today unchanged.
The registries stay separate — that was always about trust and lifecycle,
not hardware — so §1's non-goal is reworded rather than dropped.
One hard rule survives: same drive yes, nested never. A storage location
containing a backup repo is a recursive-inclusion trap, and §3's nesting
refusal already covers both directions. What needs work is the error —
pointing storage at /mnt/bigdisk when /mnt/bigdisk/backups exists fails
the empty-dir rule, and the message must suggest a subdirectory rather
than saying "not empty", because that is the likely first attempt.
Shared fate gets the §6.1 treatment: durable badge on both locations and
a line in the backup summary, stated accurately rather than moralised —
a same-drive backup still covers accidental deletion, bad updates and
ransomware; what it doesn't survive is the disk dying. Also names the
compounding case, since nobody pictures it: a shared drive that is also
removable takes the apps and the restore path away at the same moment.
Checks: free space becomes per-device (two locations on one filesystem
draw from one pool, so a growing repo can starve the apps), plus a new
shared-device check that warns and never refuses.
Notes that this makes the naming collision worse and suggests the
resolution — a Disks view with one row per device showing which roles
LibrePortal has on it, registries separate underneath.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reboot-persistence and hot-plug become loud durable warnings instead of
refusals, and the --allow-transient flag goes away — there is nothing
left for it to unlock.
The reasoning, recorded in §6.1 so it isn't relitigated: "not in fstab"
and "removable" describe a supported setup, not a broken one — the media
library on a USB disk is a reason to want this feature at all. Refusing
there fails an honest user on their first reasonable attempt and teaches
them to hunt for an override rather than read the warning. We can afford
to allow it because the dangerous moment is start-up, not registration,
and start-up is already gated: no marker, no location, appDir fails and
dockerComposeUp refuses. Blocking at registration buys nothing the
runtime doesn't already enforce.
So the obligation moves from prevent to inform, and "loud" has to mean
durable rather than a toast: the consequence in plain words plus a
ready-to-copy fstab line at registration, a standing badge on the
location for as long as it holds, and a start-up refusal that names the
cause instead of failing generically.
Fitness checks now refuse only when a location cannot work at all
(checks 1-5), never for merely needing care. Wizard cards get three
distinct states, with warned fully selectable and explicitly not
second-class.
Marked the §10.1 gate load-bearing: it is now the only thing between an
unmounted disk and an app rebuilding itself empty, so anything that
relaxes it has to revisit §6.1 as well.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the three surfaces the plan was missing, and the check suite that
gates them.
§6 splits validation in two, which is the point: admission (§3) is a
security question answered by the root helper and it refuses; fitness is
"will app data actually work here", needs no privilege, and therefore
can be run speculatively against a disk the user hasn't chosen — which is
what lets the wizard grade candidates before anything is committed. Ten
checks, of which two are worth calling out: a real chown probe, because
NFS root_squash reports a healthy nfs4 and then can't chown; and reboot
persistence, because hand-mount → register → install Nextcloud → reboot
is §10.1's disaster on day one. Checks recur on the crontab rather than
only gating at add time — a location that passed in March can be full,
read-only or unplugged by June.
§7 adds the setup-wizard Storage step. It follows the Metrics precedent
for a conditional step but with a better trigger: only appear if the box
has somewhere else to put things, so the one-disk case is untouched.
Candidate detection has to filter hard — this dev box has ~15 snap loop
devices to drop before the real answer shows. Refused candidates render
greyed with the reason rather than vanishing.
§5.2 puts the resolved path in the field's comment for hand-recovery,
written only on change (install, move, or a regen that finds it stale) so
it isn't churn on a user-editable file in the container-owned tree. It's
a breadcrumb: nothing reads it to decide anything, and the WebUI panel
composes its description from appDir so the editor is right even when
the comment is stale.
Phasing regrouped to 7 phases; §10.4 folded into §6; open question 5
resolved (build the flag — unattended installs have no wizard).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pinning an app to the primary location isn't a fact about its storage,
it's a statement about whether the field is editable — so it belongs in
the comment, where the renderer's other display metadata already lives.
webui_generate_configs.sh already parses **ADVANCED** out of a field's
comment; **READONLY** parses the same way and needs no new key, no
engine-side list, and no special case in appDir or the migrate path. It
also has uses beyond this feature: crowdsec.config:72 warns that editing
a field does nothing, next to an editable input.
Also records why CFG_<APP>_STORAGE holds a location name rather than a
path, since it's the kind of decision that gets relitigated: mount paths
move (relabel, /media/<user>/…, a different session) and a name means
re-registering once instead of rewriting every app config; and migrate's
central question — "this host has no such location, where should it go?"
— is a registry lookup, not a string comparison. The config stores intent,
the manifest stores fact. Unlike backup locations there's no case for a
custom-path override: root only touches registered roots, so a path
outside the registry is unusable rather than merely discouraged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three refinements that make the plan smaller rather than bigger, plus
an honest accounting of what stays awkward.
The .libreportal-storage marker now does three jobs instead of one:
admission, mount detection and migrate provenance. Marker-lives-on-the-
drive replaces the findmnt/REQUIRE_MOUNT/fs_uuid guard family with a
single file read, and it covers cases those miss — a LUKS volume that
was never unlocked, a dropped network mount. It also unblocks adopting
a drive that already holds app data, which strict emptiness forbade;
that relaxation is free, because planting a marker requires write
access you'd have to already have.
appDir becomes the one place availability is checked, so an absent
drive fails centrally instead of needing a guard at 200 callers. And
the sweep verifies itself: retire containers_dir to a sentinel path so
survivors fail loudly on something that cannot exist.
Pinning infra apps becomes CFG_<APP>_STORAGE_PINNED, mirroring
MULTI_INSTANCE, rather than a hardcoded list in the engine.
New §9 records the four things that don't get a clean answer: the
TOCTOU window and the parent-writability restriction that closes it,
two registries that will look like one, a 90-file diff that only
process makes safe, and large moves simply being slow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixing _instanceRewriteTools does nothing for an instance already on disk, and
a clone from the old code is broken in ways that never announce themselves:
every Tools action answers "App '<slug>' has no tool '<id>'" because
dockerAppRunTool wants app<Ucfirst><Pascal>; `authPersistCfg <type>` writes the
instance's new admin credential into the BASE app's config; and the clone
defines the base app's adapter and tool names while its bodies exec against the
instance's container, so the loader keeps whichever it sourced last and the
base app's user tools can end up administering the instance — decided by
nothing but find(1) order. Seen on a live install: the generated manifest
resolved [appBookstackListUsers] to bookstack_test's copy.
libreportal instance repair [slug] [--dry-run]
Rewrites the template dir only — no container is touched, nothing reinstalled,
so it does not route through the task system the way create/remove do.
Idempotent by construction. Two of the three renames match their own output
(appMattermost_teest… still starts with appMattermost), and a clone from the
old code is only PARTLY wrong — its suffix hooks were always correct and end at
the slug with no trailing underscore, which the infix rule would otherwise read
as type + id + () and append the id twice
(appSetupComposeTags_nextcloud_family_family). Three sentinels park the
already-correct spellings before the rewrite and restore them after, so a
healthy instance is a no-op and an interrupted run can just be re-run.
Verified against fixtures built with the old rule set for all five
multi-instance apps that ship tools: after repair each tree is byte-identical
to a fresh clone from the fixed cloner, a second pass reports "already
correct", and --dry-run leaves checksums untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Proposal for turning the single containers root into a registry of
locations, with each app choosing one — so bulk data can live on a
second disk or an external drive while the control plane stays put.
Records what the codebase already gives us for free (relative compose
volumes, self-describing app dirs, the backup-locations pattern), the
~200 call sites that must move behind an appDir() resolver, and the
one constraint that shapes everything: the root helpers bake their
paths at install precisely so the manager cannot redirect a root
chown, so the location registry has to be root-owned with an
empty-directory admission rule.
Also notes a latent bug found while checking: restore builds its
include path from the LOCAL containers root, so migrating between
hosts with different --containers-dir values silently restores nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four defects that all reduce to "a function an instance defines is invisible
to the code that dispatches it". Reported as `mattermost_teest has no upgrade
verifier`, for an app whose verifier was on disk the whole time.
* generate_function_manifest.sh shipped 0664. lpRegenArrays invokes it as an
executable, so it died rc=126 on every call and `|| true` swallowed it — the
manifest was never rebuilt on any live system, only laid down at deploy.
Its sibling generate_arrays.sh is 0775, which is why the files_*.sh arrays
looked current while the manifest was byte-identical to the shipped copy.
* lpRegenArrays now runs both generators through bash rather than depending on
the exec bit, reports a manifest failure instead of hiding it, and treats a
new containers/<app> dir as stale — the one event on a live box that adds
functions was the one the scripts/-only mtime check could not see.
* updaterHasVerifier consults the disk before answering no. The CLI runs
LP_LAZY=1, where the container scan is skipped and every function must come
from the build-time manifest, so an app created after the build reads as
having no verifier. GATE 1 then refuses an upgrade that is fully verifiable,
and updaterUpgradeAuto's `|| continue` drops the app in silence for good.
Self-healing regardless of manifest staleness, which matters because a
self-update restores the shipped manifest and drops instance entries again.
* _instanceRewriteTools gains three renames. authAdapter_<type>_<method>() was
caught by neither the prefix rule (no word boundary before _<type>) nor the
suffix rule (needs () right after the type), so the clone defined the base
app's adapter name while pointing at its own container — every instance user
tool answered "does not implement", and which definition survived came down
to find(1) order. Bare-app arguments to authAdapterCall/authPersistCfg went
unrewritten too, so an instance's password reset wrote the credential into
the base app's config. And dockerAppRunTool wants app<Ucfirst><Pascal>, which
no rule produced, so every tool on every instance was unreachable. The infix
rename runs before the suffix rename: the reverse order appends the id half
twice (appSetupComposeTags_nextcloud_work_work).
Verified on a live install: the upgrade ladder now plans mattermost_teest
11.9 -> 11.10, and `regen arrays --force` indexes the instance hooks.
Also carries in-flight instance-removal regen work from a concurrent session
on the same worktree (_lpRegenOrphanedApp, instanceRemove's WebUI refresh).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The post-task handler only repainted the app detail page when the finished
task's app WAS the app on screen, and only ever re-rendered that same slug.
An instance removal is neither: it is dispatched for the instance, while the
list that changed — the Instances bar — is drawn on the parent and on every
sibling. Removing an instance from a family page left its pill sitting there.
Now the family (INSTANCE_OF) is read before the apps.json reload — a removed
instance is gone from it afterwards — and the page ON SCREEN is repainted
whenever it belongs to the same family as the completed task, so a pill
appears on create and disappears on remove without a manual refresh.
Also handles the page whose app no longer exists: an instance removal deletes
the app outright (a plain uninstall only flips it to not-installed), and the
removal flow parks the user on the instance's own Tasks tab, so re-rendering
printed "App not found". Land on the type it was an instance of instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two discoveries from exercising the capture path live:
- docker exec prints its 'executable file not found' OCI error to STDOUT
(a docker quirk), i.e. into the tar pipe — so the exit code (126/127)
is the only trustworthy no-tar signal, and the host-side 'not a tar
archive' noise is a symptom, not the cause. Detect on the code and say
plainly that the image has no tar.
- The two pipe halves shared one stderr file through an O_TRUNC fd and
an O_APPEND fd, racing and overwriting each other's lines — one file
per half, concatenated after.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The /tasks page's right-side tick + dynamic Select all / Clear All ⇄
Delete Selected layout now covers the other three management surfaces:
- App detail → Tasks tab: filter bar gains the Clear All button and
master tick; Clear All there scopes to that app's tasks only. The
selection set is resolved through window.tasksManager everywhere —
TasksManager is constructed in several places, and ticks previously
landed on one instance while Delete Selected read another's empty set.
- Apps overview → Updates: the header's Update all button now morphs to
Update Selected (N) + Clear in place as rows are ticked, replacing the
separate selection bar between toolbar and list.
- App detail → Backups: each snapshot row gains Delete + a right-side
tick; a toolbar atop the list morphs Delete All ⇄ Delete Selected (N).
The whole selection rides in ONE task (delete <app> 1:a,2:b,…) since
the backup surfaces hold one task per subject at a time.
- CLI: backup app delete accepts comma-separated <idx>:<snap> pairs, and
both delete and delete_all now regenerate the WebUI backup JSON so
deleted snapshots leave the screen instead of lingering until the next
backup.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Matrix was uninstalled and the Updates tab kept listing it as up to
date. Not an instance problem — updates.json and cves.json are
scan-time snapshots on a 30-minute cadence, and nothing rewrote them at
uninstall, so any removed app haunted every updater surface until the
next scan happened to run. The backend was never wrong: the DB, the
apps data and the app's own page all said uninstalled within seconds.
Fixed at both ends. Uninstall now deletes the app's rows from both
generated files, surgically — a full rescan re-runs CVE checks against
every image and has no place inside an uninstall. And the updater's
merge drops any row whose app window.apps does not list as installed,
which covers every other way the snapshot can go stale (a crashed
uninstall, a hand-edited file, the next bug). The filter only applies
when the installed list has actually loaded, preserving the page's
degrade-gracefully contract when it has not.
The stale Matrix rows on this install were purged the same surgical
way; the tab now shows 14 rows with the merge still intact.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Prometheus kept being found stopped after boots, always Exited(0),
always alone. The journal settles it: both stops sit seconds before a
host shutdown boundary — container stopped 05:45:22, boot ended
05:45:30; stopped 04:41:59, boot ended 04:42:05. This is a laptop-class
host that gets shut down, and under ROOTLESS docker the containers are
ordinary processes in the user session, torn down by systemd in
parallel with dockerd's own exit.
That parallelism is the race. An app that handles SIGTERM promptly
exits while dockerd is still alive to record "stopped" — and
unless-stopped then means what it says: not restarted at the next
boot. Apps that exit slower, or die only when dockerd does, are
recorded as running and come back. Prometheus loses reliably because it
is the best-behaved process on the box ("See you next time!"), but
which app loses is a scheduling accident — changing Prometheus's
restart policy would treat the sample, not the race.
So an @reboot crontab entry now waits for the rootless daemon (up to
five minutes, then gives up rather than hang) and `compose up -d`s
every installed app via the existing dockerComposeUpAllApps. Idempotent:
running apps see no diff, stopped ones start, ordering is compose's
problem. Registered through crontabRefresh like the other entries, and
installed on this box.
The accepted trade, stated rather than hidden: an app deliberately
stopped before a reboot comes back after it. On a self-hosting box "the
fleet is up after boot" is the promise unless-stopped was already trying
to make; a stop that must survive reboots is what uninstall is for.
Verified by direct execution: daemon answered immediately, all
installed apps reconciled, running containers untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- backup_files.sh / backup_db.sh: every docker exec/run in the capture,
sidecar-discovery, rehydrate and DB-import paths now goes through
runFileOp — bare docker can't reach the rootless daemon socket, which
made live capture fail (and silently bounce containers) on every
rootless install, and would have broken DB restores the same way.
- capture/rehydrate stderr is kept and printed on failure instead of
being discarded, with a clear message when the image has no tar.
- backup_app_start.sh: when no location produced a complete snapshot the
backup now returns 1 — the task is marked failed instead of logging a
nonexistent/incomplete backup as a success and skipping verification.
- restic engine: on restic exit 3 the orphan incomplete snapshot is
called out explicitly so nobody restores it believing it is whole.
- speedtest: capture /config through the container (root-owned TLS key
and logrotate state are unreadable from the host), which also flips
its auto strategy to live — no more container stop per backup.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two recurring console errors, each masking real ones.
"Identifier 'BackupAppCard' has already been declared": the app has two
script loaders that could not see each other's work. The boot-time
system loader injects tags with NO id and dedupes by script[src]; the
SPA's loadScript injects with an id and deduped ONLY by that id. So a
file both of them load — backup-app-card.js is on the boot list AND in
the overview Backups tab's asset chain — loaded twice whenever the SPA
loaded second: its getElementById check cannot see an id-less tag. The
class redeclared with an uncaught SyntaxError. The blindness was
one-directional, which is why it looked sporadic. spaClean.loadScript
now also matches by src, closing the asymmetry for every shared file,
not just this one.
"ConfigSidebar: config-categories-list element not found", three to
four times per navigation: renderConfig populated the admin sidebar
unconditionally, but the backup center's Configuration tab EMBEDS that
renderer inside its own pane (renderConfig('backup', <target>)), where
the sidebar element legitimately does not exist — and the refresh
coordinator re-renders that tab on every task event. Populating is now
skipped when a target is passed: an embedded render does not own the
admin shell. The error stays for target-less renders, where a missing
sidebar genuinely is a broken shell.
Verified in a live session: repeated sweeps across /tasks, the overview
Backups tab (Configuration sub-tab included) and a per-app Backups page
produce zero new occurrences of either error — confirmed against a
live console buffer via a sentinel, since the buffer retains the
pre-fix history and re-reading it can look like recurrence.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same treatment the updater buttons got. Confirming a backup produced a
toast and a blind 1.5-second refresh; the tile's "Back up" pill stayed
armed and nothing said work was happening. Restore and Delete on a
snapshot row behaved the same.
runTask now derives the SUBJECTS a command holds from the command
itself: an app backup busies that app's tile, `backup system` the
system tile, and `backup all` busies every tile at once — one task,
honest feedback everywhere it acts. Restore and delete also parse the
exact snapshot out of the command, so only the clicked row's button
spins while the app's other snapshot buttons merely disable: the
spinner marks the action running, not the ones waiting on it.
Starting a second task on a busy subject is refused with a notice — a
second restore of the same app queued behind the first is a footgun,
not a feature.
The face survives re-renders: the refresh coordinator repaints this
page on every task event, so the tile and row renderers consult the
busy map rather than relying on the instant DOM patch alone. Cleared on
the task's terminal state, before the coordinator's debounced repaint
lands, so that repaint shows the finished state. The leak backstop is
two hours — a big app's backup is legitimately slow.
Verified end to end against the live install with two real backups:
the pill flips to "Backing up…" (disabled, spinner) in the same tick as
the confirm click, holds through the run, and restores within a second
of the task completing, with the busy map empty. Subject derivation
unit-tested across all five command shapes, including `backup all`
fanning out to every tile and snapshot-level targeting for restore and
delete. Reuses the updater's .btn-spin — both stylesheets are global,
so busy looks the same everywhere in the app.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four rules rendered their bubble with `content: attr(title)` while leaving the
title attribute in place, so the browser drew its native tooltip over the styled
one — two bubbles, offset, one of them unthemed. That is the overlap.
setup-wizard.css already had it right with attr(data-tip). Everything else now
matches: the text moved to data-tip, which the browser does not render, and
aria-label carries it for assistive tech. 24 emitters migrated across 9 files,
including two that assign the text at runtime — those set both attributes so
neither goes stale.
The same sweep found the bubble was defined twice globally. core/forms/css had
the plain one; port-manager.css carried a nicer bordered, wrapping version
commented "Enhanced tooltips for main config help icons" — which never applied
to them, because forms.css loads afterwards at equal specificity and won. So
every help icon outside the port manager had been getting the plain bubble, with
`white-space: nowrap` and no max-width: a sentence-length tooltip rendered as one
unbroken line running past its own background. The wrapping version is now the
one in forms.css, the shadowed copy is gone, and the .port-manager-scoped rule
stays.
Verified on Bookstack's config page: 26 help icons, none carrying a title, all
carrying data-tip, and hovering one flips ::after content from `none` to the text
with a single themed bubble on screen.
No port-descriptor tooltip, deliberately. PORT_N fields are replaced at runtime
by the port manager, whose hidePortFieldLabels() sets the field's help icon to
display:none — anything put there is unreachable, confirmed in the DOM. The
format is already documented column by column by the port manager's own 13 help
icons, several in more detail than a single descriptor string could manage.
Three visible faults on the Tasks page, one shared root.
Upgrade tasks rendered as their raw command — "libreportal updater
upgrade rocketchat 8.7.1" beside properly named neighbours. The title
table had rows for updater check/apply/apply-all/rollback and none for
upgrade, because the upgrade command is assembled in task-actions.js
rather than task-commands.js — and lp-task-names, the guard built to
catch exactly this, only read task-commands.js. It certified 16 commands
and reported that as the whole surface; the surface was 29. The guard
now reads both dispatch sites (JS ${expr} interpolations become sample
placeholders; commented-out prose mentioning commands in backticks is
skipped, or it reports fictional commands), and all 29 pass.
A removed instance's tasks outlive it, and its slug rendered as a tech
identifier: "Bookstack_uitest - Remove Instance". getAppDisplayName
cannot help — it capitalises as its own fallback, so unknown is
indistinguishable from known-and-plain. The formatter now does the same
membership test the helper uses internally: slug absent from
window.apps, prefix before the underscore present -> render the way
live instances are shown, "Bookstack · uitest".
Same story for the icon: bookstack_uitest.svg is deleted with the
instance, and onerror="display:none" left a bare gap in the row. Now a
fallback chain — the TYPE's icon (which survives), then the LibrePortal
logo. Verified live: the dead instance's rows show bookstack.svg with
the fallback marker set, everything else keeps its own icon.
Verified in a real browser session — full render, zero console errors.
The 'add' verb also joins the app-action map so "Add Application" is
deliberate wording rather than the blind "<Verb> Application" compose.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Swept the field mappings after the Updates one. Removed where the tooltip
carried no information the label did not already give:
PORT_1..20 "Port N for this application" label: "Port N"
PORTS "Port configuration for the label: "Port Configuration"
application"
CATEGORY "The category this application label: "Category"
belongs to"
THEME "Visual theme for the application" label: "Theme"
...PRIVATE_KEY "WireGuard private key" label: "WireGuard Private Key"
Deliberately kept several the crude word-overlap check also flagged, because
they earn their place: DOMAIN says the value is a number and why, HEADSCALE and
COMPOSE_FILE carry a requirement and a warning, VPN_TYPE says it depends on the
chosen provider, and DESCRIPTION/LONG_DESCRIPTION distinguish brief from
detailed — which is the only thing separating that pair on screen.
config-form.js already guards on the field having a tooltip, so a field without
one renders no help icon rather than an empty bubble. Confirmed on Bookstack's
config page: 25 icons left, none with an empty or "undefined" title, and no
stray "undefined" in the body text. Regenerated the served JSON too — 145 fields
before and after, 135 tooltips down to 111, and no field changed in any other way.
Worth a look separately: PORT_N holds the full pipe-delimited port descriptor,
not a port number, so "Port 1 for this application" was mildly misleading as
well as redundant. A tooltip explaining that format would be an improvement
rather than a deletion.
Replaces the follow-the-task navigation from the previous commit —
carrying the user to the tasks page and back was feedback by relocation.
This is feedback where the click happened: the button becomes
"Updating…" with a spinner and stays disabled until the task reaches a
terminal state, then the data refetches and the row repaints with the
result. Update, Update all / selected, the stepped Upgrade and Roll back
all get it (verbs keep their own labels: Upgrading…, Rolling back…).
Two paths keep the face honest. A DOM patch flips the buttons the moment
the click lands — no waiting for a render pass — stashing the original
face on the element so a failed dispatch can restore it. And every
renderer now builds these buttons through one busy-aware helper, so a
repaint landing MID-task (the auto-refresh poll, a filter change)
reconstructs the spinner instead of silently re-enabling the button.
Sibling actions are held while a task runs: Roll back on an app that is
mid-update is disabled — without a spinner, which marks the action that
is running, not the ones waiting on it. Entry points also guard on the
inflight set, so a keyboard-triggered duplicate is inert.
Correlation is by task id (routeAction resolves to the created task), so
another task finishing cannot end this button's busy state early. Only
completed/failed/cancelled end it; listeners are removed on the first
terminal event, with a 30-minute timeout so a task that never reports
terminal cannot pin a spinner forever. cursor is "progress", not
"not-allowed" — the work is happening, the button is not refusing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"Install new image builds automatically, or only when you press Update" spelled
out both options, which the select's own labels already do directly below it —
"Automatic (recommended)" and "Manual — I'll press Update". The tooltip now says
only what the setting is for.
The generated apps-field-mappings.json carries this string, so the live install
was regenerated rather than left showing the old copy.
Pressing Update produced a toast and nothing else. The row did not
change, nothing on the page moved, and the work was real but invisible —
so the button read as though it had not done anything.
Update, Update all / Update selected, and the stepped Upgrade now carry
you to the task they started and return you to the Updates tab when it
reaches a terminal state.
Correlated by task ID, not by app name: routeAction resolves to the
created task, so a second update started elsewhere cannot bring this one
back early, and a completion for an unrelated app cannot either. Only
completed/failed/cancelled end the wait — taskUpdated also fires
mid-run, and treating that as done would bounce the user out of a task
still in progress.
Navigation is deliberately NOT locked, which is the one part of the
request I did not build. The task is a background job with its own
snapshot and rollback; it does not need watching, and a lock would
strand someone here if a task ever hung. Leaving is therefore treated as
a choice — the return only fires while the user is still on a tasks
page, so nobody is yanked back from somewhere they chose to be.
Both listeners are removed on the first terminal event, and a 30-minute
timeout removes them anyway, so a task that never reports a terminal
status cannot leak a pair of window listeners per click.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
