From 202052f79eb4b6575e491825007f91edbe4904d7 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 25 Aug 2026 14:04:14 -0700 Subject: [PATCH 1/3] Surface apt's Failure Diagnostics in upgradable_count() A failed "apt list --upgradable" previously reported only its exit code, discarding stderr that would show why: an expired repository key, a network failure, an apt lock conflict. Captures stderr to a file under the script's existing TMP_DIR and includes a bounded (200-char) excerpt in the "unknown" status line. CodeRabbit finding from PR #1008's promotion review. --- host-setup/linux/upgrade-host.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/host-setup/linux/upgrade-host.sh b/host-setup/linux/upgrade-host.sh index 9792da8f..294415ab 100755 --- a/host-setup/linux/upgrade-host.sh +++ b/host-setup/linux/upgrade-host.sh @@ -176,10 +176,11 @@ refresh_snaps() { upgradable_count() { # A failed listing and a listing that genuinely found nothing upgradable both read as "no matches" through grep alone, so the two are told apart here rather than both printing 0. # This only ever backs a status report, so the answer here is "unknown" rather than a die: nothing downstream mutates on the strength of this count. - local out status=0 - out=$(apt list --upgradable 2>/dev/null) || status=$? + local out err_file status=0 + err_file=$(mktemp "$TMP_DIR/upgradable-count.XXXXXX") + out=$(apt list --upgradable 2>"$err_file") || status=$? if [[ $status -ne 0 ]]; then - printf 'unknown, apt list --upgradable failed (exit %s)' "$status" + printf 'unknown, apt list --upgradable failed (exit %s): %s' "$status" "$(tr '\n' ' ' <"$err_file" | cut -c1-200)" return 0 fi printf '%s package(s), against the lists as they stand' "$(grep -c '/' <<<"$out" || true)" From e0be780f1b731a4cdfb125a0053eb8c24e3a2915 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 25 Aug 2026 14:11:34 -0700 Subject: [PATCH 2/3] Check mktemp Success Before Using Its Result in upgradable_count() upgradable_count() is called as $(upgradable_count), a command substitution the script's own set -e does not reach into without shopt -s inherit_errexit, which is not set here. A failed mktemp previously left err_file empty and execution continued into an invalid redirect, misreporting a temp-storage failure as an apt failure with no diagnostic. Now checked explicitly and reported as its own unknown case. qodo finding from PR #1009's own review. --- host-setup/linux/upgrade-host.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/host-setup/linux/upgrade-host.sh b/host-setup/linux/upgrade-host.sh index 294415ab..787f1004 100755 --- a/host-setup/linux/upgrade-host.sh +++ b/host-setup/linux/upgrade-host.sh @@ -177,7 +177,11 @@ upgradable_count() { # A failed listing and a listing that genuinely found nothing upgradable both read as "no matches" through grep alone, so the two are told apart here rather than both printing 0. # This only ever backs a status report, so the answer here is "unknown" rather than a die: nothing downstream mutates on the strength of this count. local out err_file status=0 - err_file=$(mktemp "$TMP_DIR/upgradable-count.XXXXXX") + # This runs inside $(upgradable_count), where the script's own set -e does not reach without "shopt -s inherit_errexit", unset here, so a failed mktemp is checked explicitly rather than left to abort the function on its own. + err_file=$(mktemp "$TMP_DIR/upgradable-count.XXXXXX") || { + printf 'unknown, could not create a diagnostic capture file' + return 0 + } out=$(apt list --upgradable 2>"$err_file") || status=$? if [[ $status -ne 0 ]]; then printf 'unknown, apt list --upgradable failed (exit %s): %s' "$status" "$(tr '\n' ' ' <"$err_file" | cut -c1-200)" From 1d87b6eb9a78ec9da8d5bed2c750dc7f17cc8e0f Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Tue, 25 Aug 2026 14:23:55 -0700 Subject: [PATCH 3/3] Drop the Temp File From upgradable_count()'s Diagnostic Capture Bounding apt's stderr with a synchronized process substitution is real complexity for a status-line diagnostic. Simpler: only the failure path re-runs the read-only listing, with stdout discarded and stderr bounded by a pipe into head -c 200, entirely in memory. This also removes the prior mktemp-based capture and the failure case it needed to guard against, since there is no temp file left to create. Verified live against CodeRabbit's own 1MB-stderr reproduction: disk usage inside the capture directory is unchanged, output is correctly bounded and the real exit code still surfaces. CodeRabbit finding from PR #1009's own review. --- host-setup/linux/upgrade-host.sh | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/host-setup/linux/upgrade-host.sh b/host-setup/linux/upgrade-host.sh index 787f1004..c853f968 100755 --- a/host-setup/linux/upgrade-host.sh +++ b/host-setup/linux/upgrade-host.sh @@ -176,15 +176,11 @@ refresh_snaps() { upgradable_count() { # A failed listing and a listing that genuinely found nothing upgradable both read as "no matches" through grep alone, so the two are told apart here rather than both printing 0. # This only ever backs a status report, so the answer here is "unknown" rather than a die: nothing downstream mutates on the strength of this count. - local out err_file status=0 - # This runs inside $(upgradable_count), where the script's own set -e does not reach without "shopt -s inherit_errexit", unset here, so a failed mktemp is checked explicitly rather than left to abort the function on its own. - err_file=$(mktemp "$TMP_DIR/upgradable-count.XXXXXX") || { - printf 'unknown, could not create a diagnostic capture file' - return 0 - } - out=$(apt list --upgradable 2>"$err_file") || status=$? + local out status=0 + out=$(apt list --upgradable 2>/dev/null) || status=$? if [[ $status -ne 0 ]]; then - printf 'unknown, apt list --upgradable failed (exit %s): %s' "$status" "$(tr '\n' ' ' <"$err_file" | cut -c1-200)" + # Re-run once more, stdout discarded this time, so the diagnostic comes from a pipe head bounds in memory rather than a file this would otherwise have to size-cap and clean up itself. + printf 'unknown, apt list --upgradable failed (exit %s): %s' "$status" "$(apt list --upgradable 2>&1 >/dev/null | head -c 200 | tr '\n' ' ')" return 0 fi printf '%s package(s), against the lists as they stand' "$(grep -c '/' <<<"$out" || true)"