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>
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>
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>
Task titles come from one table whose final fallback returns the raw command
string, so a dispatched command with no matching row does not error — it just
renders as "libreportal instance remove bookstack_work" beside properly named
neighbours. That silence is why this kept being fixed and kept coming back.
The guard reads BOTH files as source — the command templates from
task-commands.js and the pattern table from tasks-format.js — so it fails on a
command added without a name rather than leaving it to be noticed in the UI.
Two checks, both from source rather than guessed from rendered text:
1. Nothing falls through: a title equal to its command, or still starting with
"libreportal ", means the raw fallback was reached.
2. Every `libreportal app <verb>` verb has an actionMap entry. Without one the
generic branch composes "<Verb> Application", which is how "Up Application"
and "Down Application" shipped.
The second check reads the actionMap keys instead of pattern-matching the title,
which a first attempt did and which was wrong: "Reload Application" is both a
correct hand-written label and what the generic branch emits, so the rendered
text cannot distinguish them and the heuristic failed a title that was fine.
Verified by breaking it deliberately in both directions — adding a command with
no pattern, and deleting an actionMap verb. Each is caught, named, and pointed at
the file to edit; both files were restored byte-identical afterwards.
Lives in scripts/dev, which .gitattributes marks export-ignore, so it never ships
in a release tarball. Needs a node and borrows the running container's when the
host has none, the same constraint lp-shot works around for chromium.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The WebUI screenshot helper CLAUDE.md already tells agents to use only
ever existed on the maintainer's box. Vendoring it means it survives a
machine rebuild and the setup steps are written down.
It does NOT ship: make_release.sh builds with `git archive`, which honours
export-ignore, so scripts/dev joins scripts/release and docs on that list.
Verified — the staged tarball has 1666 files and none under scripts/dev.
Keeping it out of releases is deliberate, not incidental. lp-shot signs
itself a session from the jwtSecret in frontend/.auth.json, which is fine
on a box where you already own that file, and has no business sitting in
a user's install where it would read as a backdoor.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
