fix(scripts): abort deploy when compose_file_args reports a missing compose file - #7862
Conversation
compose_file_args() exits 1 on a missing compose file, but all 4 callers consumed it via `mapfile -t compose_args < <(compose_file_args)`. The process substitution runs the function in a subshell, so its exit 1 only kills that subshell; mapfile itself returns 0, so set -e never fires and the caller kept going -- invoking `docker compose` with an empty or truncated -f set instead of aborting on a stale/mistyped compose path. Consume it via a checked command-substitution assignment (`if ! compose_args_raw="$(compose_file_args)"; then exit 1; fi`) at all 4 call sites, then split into the array with a here-string. This propagates the real exit code (including the truncated-partial-output case) regardless of set -e. compose_file_args's own logic is unchanged. Adds compose_file_args exit-propagation tests to selfhost-deploy-common.test.ts: happy path continues, a missing sole file aborts before the consumer, and a later missing file aborts instead of continuing with a truncated arg list. Closes JSONbored#7765
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7862 +/- ##
=======================================
Coverage 91.37% 91.37%
=======================================
Files 729 729
Lines 74694 74694
Branches 22795 22792 -3
=======================================
Hits 68252 68252
Misses 5396 5396
Partials 1046 1046
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-21 15:34:40 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Closes #7765
compose_file_args()(scripts/lib/selfhost-deploy-common.sh)exit 1s on a missing compose file, but all 4 callers consumed it through a process substitution:The process substitution runs
compose_file_argsin a subshell, so itsexit 1only kills that subshell —mapfileitself returns 0,set -enever fires, and the caller keeps going. Verified:So a stale/mistyped
docker-compose.override.ymlor a badSELFHOST_COMPOSE_FILESentry printed the intendederror: compose file not found: X, but the deploy script continued and randocker compose … pull/up/pswith an empty or truncated-fset instead of aborting.Fix: consume it via a checked command-substitution assignment at all 4 call sites (
deploy-selfhost-image.sh,deploy-selfhost-prebuilt.sh,selfhost-post-update-check.sh,selfhost-post-update-regression-gate.sh):A command-substitution assignment does propagate the inner exit code (including the truncated-partial-output case), regardless of
set -e.compose_file_args's own logic is unchanged. Since it always emits at least one-f <file>pair on success, the here-string split can't produce a spurious empty element.Tests: adds a
compose_file_args exit propagation (#7765)block toselfhost-deploy-common.test.ts(which didn't cover it) — happy path continues with the right-fargs, a missing sole file aborts before the consumer runs, and a later missing file aborts instead of continuing with a truncated arg list (the case a naive non-empty check would miss). Verified locally:bash -nclean on all 4 scripts, 11/11 tests pass,tsc --noEmitclean for the changed files. (scripts/**andtest/**are outside thesrc/**99% patch gate.)