diff --git a/scripts/cli/commands/updater/cli_updater_verify.sh b/scripts/cli/commands/updater/cli_updater_verify.sh index 4c71dce..dd7f6de 100644 --- a/scripts/cli/commands/updater/cli_updater_verify.sh +++ b/scripts/cli/commands/updater/cli_updater_verify.sh @@ -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 } diff --git a/scripts/instance/instance_create.sh b/scripts/instance/instance_create.sh index f9a860c..5ec10b4 100644 --- a/scripts/instance/instance_create.sh +++ b/scripts/instance/instance_create.sh @@ -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 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__(), 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_() into appSetupComposeTags_(), which + # then reads as an infix match (__ 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 `_(` 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 _, 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__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- 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 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 '' has no tool ''". + # ${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/ 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'." } diff --git a/scripts/source/files/generate_function_manifest.sh b/scripts/source/files/generate_function_manifest.sh old mode 100644 new mode 100755 diff --git a/scripts/webui/webui_regen.sh b/scripts/webui/webui_regen.sh index f507171..4ba3851 100644 --- a/scripts/webui/webui_regen.sh +++ b/scripts/webui/webui_regen.sh @@ -28,6 +28,25 @@ _lpRegenStale() { find "$@" -newer "$artifact" -print -quit 2>/dev/null | grep -q . } +# Does apps.json still list an app whose containers/ 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/ 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