fix(instance): make an instance's own functions reachable

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>
This commit is contained in:
librelad 2026-08-24 02:51:18 +01:00
parent 0e6eb841bd
commit f82237da36
4 changed files with 152 additions and 10 deletions

View File

@ -89,8 +89,29 @@ updaterVerifyGeneric() {
}
# Does this app ship a real verifier? The stepped engine requires one.
#
# "Not currently defined" is NOT the same as "not shipped", and conflating the
# two is the most expensive mistake this file can make. The CLI runs lazy
# (LP_LAZY=1), where the container scan is skipped entirely and every function
# has to arrive through function_manifest.sh — a BUILD-time artifact. Any app
# that came into existence after that build has no stub, so its verifier reads
# as absent while sitting on disk two directories away. That is exactly every
# multi-instance clone, and the consequences both point the wrong way: GATE 1
# refuses an upgrade for an app that is fully verifiable, and the auto-ladder's
# `updaterHasVerifier || continue` drops it in silence, for good.
#
# So consult the disk before answering no, and source what is actually there —
# the same file the eager loader would have picked up. Cheap (one stat for the
# common case) and self-healing regardless of how stale the manifest is, which
# matters because a LibrePortal self-update overwrites the manifest with the
# shipped copy and drops every instance entry again.
updaterHasVerifier() {
local app="$1"
declare -F "${app}_upgrade_verify" >/dev/null 2>&1 && return 0
local hooks="${install_containers_dir%/}/$app/scripts/${app}_upgrade_hooks.sh"
[ -f "$hooks" ] || return 1
source "$hooks" 2>/dev/null || return 1
declare -F "${app}_upgrade_verify" >/dev/null 2>&1
}

View File

@ -363,6 +363,27 @@ _instanceRewriteTools() {
# names and prose.
sed -i -E "s/(\b(container|container_name)=\")${type}(\")/\1${slug}\3/g" "$f"
# Function names carrying the app as an INFIX. Neither the prefix rule
# (needs <type> at a word boundary — `_mattermost` has none, `_` is a
# word character) nor the suffix rule below (needs the () immediately
# after the type) can see authAdapter_<type>_<method>(), so the clone
# defined authAdapter_mattermost_listUsers while auth_adapter.sh
# dispatches authAdapter_${app}_${method} — authAdapter_mattermost_teest_…
# Every user-management action on an instance failed with "does not
# implement", and the clone's definitions (bodies already rewritten to
# exec against the INSTANCE's container) collided with the base app's
# under its own name. Which of the two survived came down to the order
# `find` happened to return the files in, so on an unlucky filesystem
# the base app's user tools would have administered the instance.
#
# MUST run before the suffix rule, not after: the suffix rule turns
# appSetupComposeTags_<type>() into appSetupComposeTags_<slug>(), which
# then reads as an infix match (_<type>_ followed by the id half of the
# slug) and gets the suffix appended a second time —
# appSetupComposeTags_nextcloud_work_work(). In this order the infix
# rule sees `_<type>(` with no trailing underscore and passes it over.
sed -i -E "s/\b([A-Za-z_][A-Za-z0-9_]*)_${type}_([A-Za-z0-9_]+)(\(\))/\1_${slug}_\2\3/g" "$f"
# Function names carrying the app as a SUFFIX. The prefix rule above
# only matches <type>_, so appSetupComposeTags_vaultwarden survived
# untouched — and docker_config_setup_data.sh dispatches that hook as
@ -374,6 +395,33 @@ _instanceRewriteTools() {
# names are touched. Affects 8 apps that ship this hook shape.
sed -i -E "s/\b([A-Za-z_][A-Za-z0-9_]*)_${type}(\(\))/\1_${slug}\2/g" "$f"
# …and the call side of the same dispatch. The app is passed as a BARE
# word (`authAdapterCall mattermost listUsers`), which no rename above
# touches: not the prefix rule (no trailing _), not the container rules
# (not a docker verb, not container="…"). The instance's own tools
# therefore asked for the BASE app's adapter by name and operated on
# the base app's container — the failure mode this whole function
# exists to prevent, reached through the one argument nobody rewrote.
#
# authPersistCfg is the same shape and worse consequence: it writes the
# admin credential the tool just set into CFG_<APP>_ADMIN_*, so an
# instance resetting its own admin password was overwriting the BASE
# app's stored credential with a password that does not open it.
#
# Anchored on the two helper names that take a bare app as $1 — a
# blanket bare-<type> rewrite is not an option, mattermost_auth.sh has
# a comment about the deprecated `mattermost` binary that must survive.
sed -i -E "s/(\b(authAdapterCall|authAdapterCanDo|authPersistCfg)[[:space:]]+)${type}\b/\1${slug}/g" "$f"
# Tool entry points. dockerAppRunTool derives the function name from the
# slug as app<Ucfirst><PascalToolId> with NO case-insensitive fallback,
# so an instance's tools must be appMattermost_teestListUsers. The clone
# kept appMattermostListUsers — same collision as the adapters, and
# every tool on every instance answered "App '<slug>' has no tool '<id>'".
# ${type^} / ${slug^} reproduce dockerAppRunTool's own ucfirst exactly;
# anything cleverer would stop matching the name it has to produce.
sed -i -E "s/\bapp${type^}([A-Za-z0-9_]*)(\(\))/app${slug^}\1\2/g" "$f"
# The uppercase tag namespace, mirroring rule 4 of the compose rewrite.
# These hooks pass tag NAMES as strings ("VAULTWARDEN_ADMIN_TOKEN_1_TAG"),
# which the lowercase renames above cannot see. The cloned compose has
@ -581,5 +629,37 @@ instanceRemove() {
dockerUninstallApp "$slug" "false" "false"
fi
rm -rf "${install_containers_dir%/}/$slug"
# Mirror of the create path: the file arrays + function manifest are keyed on
# the app dirs, and one just disappeared.
if declare -F lpRegenArrays >/dev/null 2>&1; then
lpRegenArrays force >/dev/null 2>&1 || true
fi
# Every WebUI app artifact is derived from the containers/<app> dirs, so
# dropping one has to be followed by a regen of the three that enumerate
# them. dockerUninstallApp already refreshed them — but that ran while the
# dir still existed, and its patch path can only flip an app to "not
# installed", never delete it. Without this the removed instance stayed in
# apps.json (installed false, INSTANCE_OF intact), so the app-detail
# Instances bar kept rendering a pill for it and the Apps grid kept counting
# it — across reloads, indefinitely.
#
# Called directly rather than via lpRegenWebui: that routes through the WebUI
# updater, which no-ops while another update holds the lock. A skipped run
# here is not self-correcting — the staleness check that would catch it
# compares mtimes of files that no longer exist.
isNotice "Refreshing WebUI app data after instance removal..."
local _gen _gen_rc=0
for _gen in webuiGenerateLibrePortalConfig webuiGenerateAppsServicesConfig webuiGenerateAppsToolsConfig; do
declare -F "$_gen" >/dev/null 2>&1 || continue
"$_gen" >/dev/null || _gen_rc=1
done
if [[ "$_gen_rc" -eq 0 ]]; then
isSuccessful "Refreshed WebUI app data."
else
isNotice "WebUI app data refresh reported an error — if '$slug' still shows in the Instances list, run 'libreportal regen webui --force'."
fi
isSuccessful "Removed instance '$slug'."
}

0
scripts/source/files/generate_function_manifest.sh Normal file → Executable file
View File

View File

@ -28,6 +28,25 @@ _lpRegenStale() {
find "$@" -newer "$artifact" -print -quit 2>/dev/null | grep -q .
}
# Does apps.json still list an app whose containers/<app> dir is gone?
#
# Deletions are invisible to the mtime check above: removing an app dir leaves
# no source file newer than the artifact, so a ghost entry (an uninstalled
# instance, a hand-deleted app folder) would sit in apps.json forever — still
# drawn as a pill in the app-detail Instances bar and still counted on the Apps
# grid. Compare the two sets instead of their timestamps. Returns 0 when a ghost
# is present. No jq → skip the check rather than guess.
_lpRegenOrphanedApp() {
local apps_json="$1" slug
[[ -f "$apps_json" ]] || return 1
command -v jq >/dev/null 2>&1 || return 1
while IFS= read -r slug; do
[[ -n "$slug" ]] || continue
[[ -d "${install_containers_dir%/}/$slug" ]] || return 0
done < <(jq -r '.apps[]? | (.command // "") | split(" ") | last' "$apps_json" 2>/dev/null)
return 1
}
lpRegenWebui() {
local force="$1"
local gen="${containers_dir}libreportal/frontend/data/apps/generated"
@ -36,10 +55,11 @@ lpRegenWebui() {
if [[ "$force" == "force" ]] \
|| _lpRegenStale "$apps_json" "$install_containers_dir" -maxdepth 2 -name '*.config' \
|| _lpRegenStale "$tools_json" "$install_containers_dir" -maxdepth 3 -path '*/tools/*.tools.json'; then
# Sources changed (e.g. an app folder was dropped in) — do the full,
# debounced refresh so the new app appears everywhere. Force past the
# updater's own debounce: we have already established there is real work.
|| _lpRegenStale "$tools_json" "$install_containers_dir" -maxdepth 3 -path '*/tools/*.tools.json' \
|| _lpRegenOrphanedApp "$apps_json"; then
# Sources changed (e.g. an app folder was dropped in or removed) — do the
# full, debounced refresh so the app appears/disappears everywhere. Force
# past the updater's own debounce: there is established real work.
WEBUI_UPDATER_FORCE=1 webuiLibrePortalUpdate
return $?
fi
@ -54,15 +74,36 @@ lpRegenArrays() {
local newest_array
newest_array="$(ls -t "$arrays_dir"/files_*.sh 2>/dev/null | head -1)"
# Staleness covers BOTH roots the manifest indexes. generate_function_manifest
# scans scripts/ AND containers/, so watching only scripts/ meant the one event
# that actually adds functions on a live box — a new containers/<app> dir, i.e.
# every instance clone — never registered as stale. Nothing but an explicit
# --force would rebuild, so an instance's hooks stayed invisible to lazy mode.
if [[ "$force" == "force" ]] || [[ -z "$newest_array" ]] \
|| find "$install_scripts_dir" -name '*.sh' -newer "$newest_array" -print -quit 2>/dev/null | grep -q .; then
|| find "$install_scripts_dir" -name '*.sh' -newer "$newest_array" -print -quit 2>/dev/null | grep -q . \
|| find "$install_containers_dir" -maxdepth 3 -name '*.sh' -newer "$newest_array" -print -quit 2>/dev/null | grep -q .; then
local rc=0
[[ -f "$gen_script" ]] && "$gen_script" run || rc=$?
# Both generators run THROUGH bash rather than being executed, so a missing
# exec bit cannot silently disable regeneration. Not hypothetical:
# generate_function_manifest.sh shipped 0644, so every invocation died with
# rc=126 and the `|| true` below swallowed it. The manifest was therefore
# never rebuilt on any live system — it stayed byte-identical to the copy
# laid down at deploy — and lazy mode (the CLI default) could not see a
# single function belonging to an app created after that deploy.
if [[ -f "$gen_script" ]]; then
bash "$gen_script" run || rc=$?
else
rc=1
fi
# Function manifest tracks the same source set — keep them in sync.
# Failures here don't abort: lazy-load is opt-in via LP_LAZY=1, so a
# stale manifest just means lazy mode might miss a recently-added
# function. Eager mode (the default) is unaffected.
[[ -f "$manifest_script" ]] && "$manifest_script" run >/dev/null 2>&1 || true
# A failure still doesn't abort (eager mode is unaffected by a stale
# manifest) but it is no longer silent: lazy mode is what the CLI runs, so
# "the manifest is stale" is the difference between an app's hooks existing
# and not existing, which surfaces as absurd errors elsewhere ("app has no
# upgrade verifier" for an app whose verifier is sitting on disk).
if [[ -f "$manifest_script" ]] && ! bash "$manifest_script" run >/dev/null 2>&1; then
isNotice "Function manifest regeneration failed — lazy mode may not see recently added functions. Retry with: libreportal regen arrays --force"
fi
return $rc
fi
return 0