Repository navigation
bug(infrastructure): 56 shell scripts source lib/ssot-config.sh — the file does not exist, so every one silently runs on hardcoded fallbacks #14041
Description
Found while reviewing PR #14037 (#13076). Verified directly.
The finding
56 shell scripts under autobot-infrastructure/ contain a line of this shape:
source "${SCRIPT_DIR}/lib/ssot-config.sh" 2>/dev/null || truelib/ssot-config.sh does not exist anywhere in the repository:
$ find . -name "ssot-config.sh" -not -path "./.worktrees/*"
(no results)
$ grep -rln "ssot-config.sh" --include=*.sh autobot-infrastructure/ | wc -l
56
Because of 2>/dev/null || true, the failure is silent. Every one of those scripts continues, and every configurable value in them resolves to its literal fallback — ${AUTOBOT_SOMETHING:-/some/hard/path} always takes the right-hand side.
Why this matters more than the individual hardcodes
This is the mechanism the hardcode cleanup keeps assuming exists. A script that reads:
source "${SCRIPT_DIR}/lib/ssot-config.sh" 2>/dev/null || true
websockify --web "${AUTOBOT_NOVNC_PATH:-/opt/novnc}" ...looks config-driven in review and in a diff. It is not. The env var is only ever set if the caller happened to export it; the SSOT helper that was supposed to populate it was never shipped.
So:
- work that "converts a hardcode into a config lookup" in these scripts produces no behavioural change at all
- reviewers (including me, on chore(vnc): 4 infrastructure scripts still hardcode /usr/share/novnc after the #13069 pin — serve nothing or a stale 1.0.0 #13076) read the
${VAR:-default}form as evidence the value is configurable - the fallback literal is the only value that ever applies, which is indistinguishable from a plain hardcode at runtime
The || true is what makes it invisible: a missing SSOT source is treated as a normal, expected condition.
Suggested approach
Two parts, in order:
- Decide whether the helper should exist. Either write
autobot-infrastructure/shared/scripts/lib/ssot-config.shso it actually exports the values these scripts reference, or accept that shell scripts read config from the environment only and delete the misleadingsourcelines. Either is defensible; the current state is the one that is not. - Stop it being silent. If the helper is meant to exist, sourcing it should fail loudly when missing —
|| trueon a config bootstrap turns a deployment error into a wrong-value-at-runtime. If it is not meant to exist, remove the lines rather than leaving 56 dead references.
A guard is worth adding either way: a check that every source .../lib/*.sh in the tree resolves to a real file.
Acceptance criteria
-
lib/ssot-config.sheither exists and exports the values the 56 scripts reference, or the deadsourcelines are removed - A missing config bootstrap fails loudly rather than continuing on fallbacks
- A repo check flags any
sourceof a non-existent file underautobot-infrastructure/ - The scripts whose
${VAR:-literal}forms were previously inert are listed, so their values can be re-reviewed as genuine hardcodes if the helper is not built
Related
- chore(vnc): 4 infrastructure scripts still hardcode /usr/share/novnc after the #13069 pin — serve nothing or a stale 1.0.0 #13076 / PR chore(vnc): converge 12 hardcoded /usr/share/novnc references on canonical /opt/novnc (#13076) #14037 — where this surfaced; that PR's
${AUTOBOT_NOVNC_PATH:-/opt/novnc}is inert for this reason - tech-debt(infrastructure): no canonical project-root resolution — 60 unexpanded Python literals + 34 shell scripts defaulting to the live install #13149 — project-root resolution; its shell half was completed via
scripts/lib/project_root.sh, which does exist — that is the working precedent for what this should look like - bug(packaging): the autobot_shared editable install registers 16 submodules as TOP-LEVEL names and never registers autobot_shared itself #14035 — the Python-side equivalent: an install that reports success while not registering the package
Activity
mrveiss commentedon Aug 12, 2026
Decision — build it, not delete it
This was parked as an owner decision: build
lib/ssot-config.sh, or remove the 56 deadsourcelines.Taking the first, because the project's standing rule already answers it: anything that looks like debris is unfinished work — complete and wire it in, never delete. Deletion is correct only where the work has demonstrably landed elsewhere, and here nothing has: 56 scripts source a file that does not exist, so every
${VAR:-literal}fallback in them is inert and each script is silently running on its hardcoded default.Removing the
sourcelines would make that permanent and invisible. Building the library makes 56 scripts start honouring the SSOT they already believe they are reading.That is also the risk to handle carefully. Today every one of those scripts uses its literal default. The moment the library exists, they start taking real values — so this is not a no-op refactor, it is 56 scripts changing behaviour at once. The implementation needs to enumerate what each
${VAR:-literal}would resolve to and flag every case where the SSOT value differs from the literal, before it lands.Unlabelling
needs-decision. Queued behind #14151 and #14124.fix(infrastructure): extend the infrastructure/ path fix to all 23 af…
fix(infrastructure): preserve the #2224 fail-loud Redis guard, guard …
fix(infrastructure): build the shell SSOT library 56 scripts have alw…
mrveiss commentedon Aug 12, 2026
The library landed in
526586c68onDev_new_guivia PR #14174. Leaving this open — two of the original acceptance criteria are deferred to #14172 and #14173, and partial delivery does not close an issue.Verified in base
The finding that nearly shipped
This PR does not add a code path — it activates several dozen dormant ones at once, and one of them was a deliberate fail-loud guard.
bootstrap-slm.sh:406used${AUTOBOT_REDIS_HOST:?...}, added by #2224 because this script bakes the value into a remote node's generated.envover SSH and a loopback default there silently points a distributed Redis at the wrong host. Line 19 of that same file was one of the 23 source paths this PR corrects, and the library assignsAUTOBOT_REDIS_HOSTat source time, ~390 lines before the guard. The guard would have become permanently unreachable.The enumeration missed it because it searched for
${VAR:-literal}and this is a different operator. The re-sweep across all 70 sourcing scripts confirmed it was the only:?in the set and there are zero:=. Fixed by capturing the operator's value before the source rather than by weakening the guard.Also found and fixed in the same file:
INFRA_ROOT="${PROJECT_ROOT}/infrastructure"one line below the corrected source path, feeding eightcatcalls at lines 453-518 that read service templates and unit files — all of which resolved to a directory that does not exist.The path fix itself went from 9 files to 23 after a first pass scoped its search to one directory. A
git ls-files-anchored regression guard now asserts no tracked*.shsources the non-autobot-path; it was verified failing against base with 23 offenders before being relied on.Correction on the guard's reach
The guard is registered in
.pre-commit-config.yaml, so it enforces in the local pre-commit stage. It does not enforce in CI, contrary to what the PR body says..github/workflows/enforce-precommit.ymlrunspre-commit run --all-files || { echo "::warning::..."; exit 0; }— the step cannot fail. That is filed as #14181, where the same swallowing is hiding sixteen hooks that fail on every run.Remaining for this issue
sourceguardAUTOBOT_VNC_HOSTname-match atssot_config.py:1853that the first audit pass missedmrveiss commentedon Aug 15, 2026
Measurement against current
Dev_new_gui, in case it is useful for #14172/#14173 — the librarylanded, but almost nothing can reach it yet.
"Resolve correctly" means
<script_dir>/<relative path in its own source line>is a tracked file.Four distinct shapes are in use, and they cannot all be right from one library location:
The 21
$_PROJECT_ROOTones are the shape that works. The 40 relative ones point at alib/sibling most of those directories do not have.
Every shape still ends
2>/dev/null || true, so this is the same silence the issue describes, onelayer further along: the library now exists, the scripts still take their literal fallbacks, and
nothing reports it. A reviewer reading
${VAR:-default}in one of those 65 still has no way to seethat the SSOT was never loaded.
Two things that would make the remaining work self-verifying, if they are not already in
#14172/#14173:
sourcepath in a tracked.shresolves to a tracked file. That convertsthis class from silent to a CI failure, and would have caught the original 56 the day they were
written.
2>/dev/null || trueonce the path is correct. While the redirect is there, afuture move of the library re-creates the whole defect silently — the failure mode is the
suppression, not the path.
Not touching this myself — #14174 is yours and the remaining criteria are already tracked. Recording
the numbers so "the library landed" is not mistaken for "the scripts read it".
mrveiss commentedon Aug 23, 2026
Addressed by PR #14877, which delivers the two acceptance criteria that were still open after the library itself was built:
2>/dev/null || trueto an explicit failure block that writes to stderr and exits non-zerorepo_tests/shell_lib_sources_resolve_test.pyMeasured against the pristine base, that guard flags 57 of 57 sites across 56 of 56 scripts — the exact population this issue enumerated.
The build-vs-delete decision this issue raised was settled when the library was built; the follow-on scope question was #14172, decided and recorded there.
fix(infrastructure): a missing shell lib must abort, not fall back si…
test(infrastructure): guard the three ways a missing dependency stays…
fix(infrastructure): make a missing script dependency fail loudly, an…
mrveiss commentedon Aug 23, 2026
Delivered and merged.
35f48ba92817d7d5c1. The 3cancelledentries were superseded by later green runs of the same checks.code-reviewerpass, no material findings. The reviewer re-derived the coverage denominator from a pristinegit archiveof the base rather than trusting the PR body.Verified in base, not just in the PR:
Sample of the landed call-site shape (
vm-management/status-all-vms.sh:22):Coverage, counted against the pristine base and independently re-derived in review: 57 of 57 decisive source sites, across 56 of 56 scripts. 51 single-path sites, 5 two-path sites, 1 already-bare.
Caveat recorded rather than glossed: the new guards live in
repo_tests/, and the Python suite is not a required check onDev_new_gui— so they run and report but gate no merge today. Tracked as #13162 / #14353, with the specific gap filed as #14878.