fix(installer): say where a step died, and stop calling a stopped runtime a fresh machine (client#681, client#682) - #683
Merged
Conversation
…time a fresh machine Two failures that were reported together, both bash-only gaps the PowerShell installer had already closed. 1. A step could fail with zero diagnostics (client#681). Under `set -euo pipefail` a command failing outside an if/&&/|| context killed the run with no output at all — and the install log, which is the whole session tee'd, recorded nothing either, so "check the install log" led to a log that said nothing. There was no ERR trap anywhere in scripts/. Adds an ERR trap (armed with `set -E`, without which it would only fire at top level and miss every failure inside install_macos/install_linux) that records file:line, the unexpanded command, and the exit status. The closer names the site on screen and logs the command; step b logs a breadcrumb per stage so the log narrows the failure even if the trap is bypassed. Ctrl-C and SIGTERM now read as "interrupted", not as an installer failure — they were indistinguishable from a real one, on screen and in the log. Counterparts: Show-FatalError / Show-Interrupted (#577), Err's detail lines (#423). 2. A stopped container runtime was classified as a fresh machine (client#682). `_cluster_exists` is a boolean whose three probes all swallow stderr and return 1, so a down daemon looked exactly like an empty machine: a laptop that only needed Docker started was told "setting up for the first time". Classifies an installed-but-unreachable runtime as degraded/runtime-down before the cluster probe, and says so. Deliberately narrow: no docker binary is still fresh, and "permission denied" is a different remedy that keeps its own path. The run CONTINUES — install_docker_desktop already starts Docker Desktop and create_cluster reconciles the existing cluster, and taking that away would trade one bad outcome for another. The bug was the claim, not the flow. Counterpart: the tri-state Get-ClusterRunStateFromList (#557). Covered by 15 new bats tests (940 total, green), including the pipefail/SIGPIPE death class that previously produced no output whatsoever. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Conflict was scripts/manifest.sha256 only (a generated file); resolved by re-running scripts/gen-manifest.sh over the merged tree.
…b runs first #680 swept this hazard across the fleet but did not reach setup-macos.sh, whose _macos_user_is_admin is the FIRST command step b executes. printf '%s\n' $groups | grep -qx admin `grep -q` stops at its first match and `admin` sits near the FRONT of a macOS group list, so printf is often still writing when the pipe closes: SIGPIPE, pipefail, 141. The caller reads that as "not an administrator" and hard-fails a perfectly fine machine with the managed-Mac remedy. Reproducible, not theoretical — with a long group list the old form returns 141 on every run and the new one returns 0: old=141 new=0 (x5) Match POSITION is the trigger, not producer size, so a directory-bound or MDM-managed Mac with a long group list hits it and a short one does not. Same transform as #680 (capture, then match with a here-string), plus two more sites in the same file and one in common.sh where a SIGPIPE'd producer inside an `if` would MISBRANCH rather than abort: - setup-macos.sh: hw.optional.arm64 -> would call an Apple Silicon Mac amd64 and fetch the Intel Docker Desktop DMG - setup-macos.sh: the Docker.app arch probes -> `case`, which also drops the `A && B` set -e subtlety - common.sh: the load-time ARCH override -> would pick the wrong download for every pinned tool on Apple Silicon Mutation-real regression test in setup-macos-lifecycle.bats, driven through a real script so pipefail is genuinely in force (943 bats, green). Refs tracebloc/backend#1778 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
Update: a third, reproducible bug — and #680's sweep missed this fileWhile rebasing onto #680 (which swept the # _macos_user_is_admin, setup-macos.sh
printf '%s\n' $groups | grep -qx admin
This is reproducible, not theoretical. Old form vs new, long group list, five consecutive runs: Exactly as #680 documented: match position is the trigger, not producer size — which is why it bites a directory-bound / MDM-managed Mac with a long group list and not a short one. Also converted three more sites where a SIGPIPE'd producer inside an
Mutation-real regression test added, driven through a real script so On the originating reportI still cannot claim this was the cause there — that run emitted zero bytes, and this path prints its remedy before exiting. So it is a genuine bug found on the way, not a confirmed root cause. The diagnostics in the first commit are what will settle the next occurrence. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1111d4b. Configure here.
…se to it Bugbot, correctly. The real Linux docker-group error contains BOTH the permission wording and a `dial unix …` clause: permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Get "http://…/info": dial unix /var/run/docker.sock: connect: permission denied so matching the connection phrases first classified a docker-group problem as a down daemon and answered it with "start Docker" — the exact confusion _assess_runtime_down exists to prevent. Checks permission-denied FIRST and returns not-down. A negative match before the positive one is the only ordering that survives an error string containing both. The test was vacuous for the same reason: its fixture was a shortened message with no `dial unix`, so it passed against the broken code. Both fixtures are now the real full messages (the `permission denied` and `Got permission denied` variants), and are mutation-real — dropping the guard fails them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_cluster_exists is the function client#682 names, and guarding the daemon-down case did not finish the job: all three of its probes piped k3d into a consumer that stops at the FIRST matching line — and our own cluster is usually that line. k3d takes SIGPIPE, pipefail makes the pipeline 141, and inside these `if`s that reads as "no such cluster". The gate then calls a machine with a live, running cluster FRESH and offers a first-time install: the same user-visible bug as a down daemon, reached a completely different way. Capture-then-match (#680's transform) on all three probes, which also spares two extra k3d invocations. Same fix in two more spots in this file: - _handle_existing_cluster's non-jq server count — awk `exit` closes the pipe on our row, so this could abort the installer mid-reconcile with no message - the proxy-env check — grep -Eq stops at the first match, so a present variable could be reported MISSING and produce a spurious warning Two tests in cluster.bats. Note the "found" one is mutation-real only against the WHOLE pre-fix function: reverting probe 2 alone still passes, because probe 3's grep fallback finds the cluster anyway — the vacuity trap #680 called out. The test comment says so. 946 bats, green. Refs tracebloc/backend#1778 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
Related tickets — full set for reviewEverything connected to this work, in one place. Fixed by this PR
Merged just before this, same hazard class
Follow-up this PR deliberately does NOT do
Adjacent, open, same user-visible failure
The Windows counterparts this PR ports to bash (closed, for context)
The through-line: Windows was already correct on all four of these; bash was not. This PR closes the gap rather than inventing new behaviour, which is why the fixes look the way they do. Not a ticket, but part of the same storyThe field report that started this was not an installer failure at all — that machine had a working environment from weeks earlier, Docker Desktop simply was not running, and its disk was 98% full. #682 is precisely why that read as "first time on this machine". |
This was referenced Aug 12, 2026
saadqbal
approved these changes
Aug 12, 2026
saadqbal
previously approved these changes
Aug 12, 2026
saadqbal
left a comment
saadqbal
left a comment
Contributor
There was a problem hiding this comment.
Really careful PR — both routes to the #682 misclassification are genuinely closed (runtime-down probe ahead of the cluster check, and the capture-then-match SIGPIPE fix in _cluster_exists), the #681 breadcrumbs + ERR trap attribute the right site, and the new bats are mutation-real. Ran assess/cluster/common/setup-macos-lifecycle locally: 226/226 green, manifest hashes match. One tiny non-blocking nit inline.
…that reads it Asad on #683: `_list` was captured at the top of _cluster_exists but is only read by probe 2. On the common re-run — jq present, our cluster found by the JSON probe — that shell-out ran and was thrown away, so the comment claiming the capture "spares two extra k3d calls" was backwards for exactly the path that matters: it ADDED one. Each capture now sits inside the probe that reads it, so a probe that never runs never shells out. The k3d call count is identical to the pre-fix code, and the common path is back to one call. Comment corrected to say that rather than the opposite. No behaviour change; 946 bats green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
saadqbal
approved these changes
Aug 12, 2026
LukasWodka
added a commit
that referenced
this pull request
Aug 12, 2026
…686) (#688) * fix(scripts): retire the remaining early-exit pipe consumers (client#686) The sites #680 and #683 did not reach. Same transform: capture the producer, match the captured value, so the producer always runs to completion. `case` where the needle is a fixed substring — it also drops the `A && B` set -e subtlety two of these carried. Fixed — misbranch (inside `if`/`&&`, so pipefail's 141 reads as "no match"): - detect-gpu.sh:28,34 -- `lspci | grep -qi` -> GPU_VENDOR left "none" on a GPU host, i.e. a CPU-mode cluster. lspci is the one producer here that is routinely large enough to lose the race on its own (a dense server enumerates well past a stdio buffer), and one capture now serves both probes plus the AMD label. - install-client-helm.sh:449 -- repo believed absent -> re-runs `helm repo add`, which is unguarded on the next line and fails when the name exists with a different URL, escalating the misbranch into an aborted install. - install-client-helm.sh:594 -- loses --reset-then-reuse-values, so a reconcile silently stops picking up new chart defaults. `helm upgrade --help` is several KB in chunks and the flag sorts early. - install-client-helm.sh:835 -- sticky 8.4 lost -> resolves 5.7 against an 8.4 datadir, which MySQL 5.7 will not open. - setup-linux.sh:281,348 -- docker-group membership misread; 348 is nested now so the two mode guards still short-circuit ahead of `id`, which the old `&&` also did. - setup-linux.sh:898 -- nvidia runtime not detected -> CPU-only cluster on a Tier-0 GPU host that already has the toolkit. - setup-linux.sh:1151 -- the capture was already there (Asad #458); this drops the leftover `printf | grep -q` re-pipe of it. Fixed — abort: - preflight.sh:100 -- `findmnt | head -1` in an ASSIGNMENT, so 141 aborts the installer inside preflight with no message. The sibling mount pipeline two lines down was fixed in #680; this one was missed. Note errexit only propagates out of a command substitution on bash >= 4.4, so this bites on Linux (where findmnt exists at all) and not on the macOS system bash. Hardening, not live bugs — the shape is retired but the abort cannot happen today, and the commit says so rather than implying a field fix: - install.sh:538 -- the cosign checksum slice. Its only caller is `if ! ensure_cosign`, and a condition context suppresses errexit for the whole function, so the 141 is swallowed and `want` is already correct. Retired anyway: a function in the signature-verification path should not depend on how its caller happens to be written. - common.sh:262 -- argument position, where a 141 never trips errexit. Deliberately NOT changed, with the reason, so the next sweep does not re-open them: - diagnose.sh:61,96 -- `run_diagnose` runs `set +e` as its first statement, so no site in that function can abort. The support bundle was never at risk. - gpu-plugins.sh:112 -- the `|| echo ""` already guards it, and `head -5` has emitted its lines before the SIGPIPE propagates, so RAW keeps the correct value (verified: the pre-fix pipeline returns 141 but RAW is intact). - detect-gpu.sh:22,23,36 -- argument position inside `success`/`log`. - preflight.sh:727, common.sh:393, and the `awk`-without-`exit` sites -- a builtin printf under the buffer, an existing `|| true`, or a consumer that reads to EOF. 14 tests across 4 files, every one checked against the pre-fix code. Two things make them non-vacuous and both were got wrong first: the match must LEAD (a trailing match makes grep read the whole stream), and the filler must come from an EXTERNAL command — a producer built from bash builtins, or a mock ending in `return 0`, masks the SIGPIPE and the test passes unfixed. The preflight test additionally calls the function BARE, because the production command- substitution shape cannot abort on the bash 3.2 the suite runs on locally. setup-linux.bats' `id -nG` shape assertion is updated: it pinned the old `| grep -qw docker` text. It still pins what it was written to pin — that both probes key off $_grant_user and never bare $USER. Refs tracebloc/backend#1778 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(supply-chain): regenerate manifest.sha256 for the five changed libs The bootstrap verifies every sub-script it fetches against this manifest before running the privileged steps, so editing common/detect-gpu/install-client-helm/ preflight/setup-linux without regenerating it makes the installer refuse its own scripts. Produced by scripts/gen-manifest.sh. install.sh itself is the bootstrap and is not listed in its own manifest, so the cosign change there needs no hash. Refs tracebloc/backend#1778 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
added a commit
that referenced
this pull request
Aug 13, 2026
…al one (#702) * fix(installer): last failing command wins, so the report names the real one The ERR recorder shipped in #683 kept the FIRST failure. That is wrong, and a field report showed why: the run reported Stopped at .../lib/common.sh:527 (exit 1). command: sudo -n true for a failure two steps later. common.sh:527 is _real_sudo, reached from step a's _probe_privilege, whose `sudo -n true` returns non-zero to mean "a password is needed" — the installer then PRINTS that as a normal row in the host check. The trap fires for every failing command, benign ones included, so first-wins latched onto a routine probe inside a step that SUCCEEDED and refused every later record. The fatal command was never captured. A confidently wrong location is worse than the blank screen #683 replaced: it sends the reader to a line that is working as designed. Last-wins is precise. errexit stops the script AT the fatal command, and the trap fires once per failing command with no per-frame re-firing as the error unwinds — verified on bash 3.2 (macOS) and 5.x. Also: - Re-entrancy guard. `set -E` makes the recorder inherit its own trap, and the new `log` call is exactly the kind of command that fails inside it (its `[[ -n "${LOG_FILE:-}" ]] && …` form returns non-zero with no log open). Without the guard that recurses forever. - install_cleanup disarms the ERR trap before reading the record. Its own lines fail routinely — a `kill` on a dead pid, a false `[[ … ]]` — and under last-wins each would overwrite the fatal command with a cleanup detail. - The full ERR trail now goes to the log. The benign entries are not noise: reading them in order is what identified this bug. Five bats tests, mutation-real against the first-wins guard, including the field shape end to end — a probe that fails inside an `if`, a step that then succeeds, a fatal command afterwards. 982 bats green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(common): make the _record_err re-entrancy test actually exercise the guard (client#702) The recursion test drove the ERR trap with `command false || true`, which does not fire it — a command in a || list is excluded from ERR (bash manual). On bash 5.3 that form fires the trap zero times, so _record_err never ran and SURVIVED printed with or without the guard. bash 3.2 does fire it, which is why it looked green on macOS while being vacuous on Linux CI. `unset LOG_FILE` was the other half: log() is `[[ -n $LOG_FILE ]] && echo …`, so with no log open the write never attempts and nothing inside the recorder fails. The failure has to come from the redirection, so point LOG_FILE at a path whose parent does not exist (fails for root too, unlike chmod 000). Fixing only that is not enough. bash re-enters an ERR trap at most once, so deleting _TB_IN_RECORD_ERR does not hang anything and the survival test passes either way. Add a test for what the guard actually protects: a re-entrant call must not overwrite TB_ERR_* with the recorder's own log failure, which would turn 'died at helm upgrade' into 'died writing its log'. Verified by mutation — with the guard removed, the new test goes red and the survival test stays green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Shujaat Hasan <shujaat@tracebloc.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #681. Fixes #682.
Both came out of one field report: a macOS install that stopped at b) Installing what tracebloc needs and printed nothing — no reason on screen, and nothing in the install log either, even though that log is the entire session
tee'd. The machine turned out to have a healthy environment from weeks earlier; Docker Desktop simply wasn't running.Auditing outward from that, the bash installer was missing three diagnostic affordances the PowerShell installer already had. This PR closes them.
1 — A step could fail with zero diagnostics (#681)
grep -rn 'trap.*ERR\|BASH_LINENO\|BASH_COMMAND' scripts/returned nothing. Underset -euo pipefail, any command failing outside anif/&&/||context killed the installer silently, andinstall_cleanupprinted a fixed string with no file, line, command, or exit code.file:line, the command, and the status.set -Eis the load-bearing part — without errtrace an ERR trap fires only at top level, so every failure insideinstall_macos/install_linux(nearly all of them) would still be invisible.BASH_COMMANDis unexpanded (cmd "$VAR", never the value), so it cannot leak a credential.install-k8s.shroutes SIGINT/SIGTERM toexit 130/143, and those fell into the same "Installation did not complete" branch as a real error — which is a large part of why the original report was unresolvable.log()breadcrumbs in step b on both platforms.Counterparts already shipped on Windows:
Show-FatalError/Show-Interrupted(#577),Err's detail lines (#423).2 — A stopped runtime was classified as a fresh machine (#682)
_cluster_existstries three probes and all three send stderr to/dev/nulland return 1, so a down daemon returns exactly what an empty machine returns._assess_classifythen setfresh/no-clusterand announced a first-time setup over a working environment.degraded/runtime-down, before the cluster probe, and says so honestly.fresh(correct — that machine really is new), andpermission deniedis not "down" (a Linux user outside the docker group has a different fix). A wedged daemon that won't answer inside the bound counts as down.Counterpart already shipped on Windows: the tri-state
Get-ClusterRunStateFromList(#557).One design note worth flagging
My first cut blocked on a down runtime and told the user to start Docker. That was wrong and I backed it out:
install_docker_desktopalready launches Docker Desktop and waits, andcreate_clusterreconciles an existing cluster — so blocking would have removed a step that works today, trading one bad outcome for another. The bug was the claim, not the flow, so the run now continues and only the misleading "first time on this machine" line is gone.Test plan
make check— green (lint, shellcheck, drift, helm lint).make bats— 940 tests, 0 failures (15 new)._assess_runtime_downcases, and two mutation-real guards that a down runtime is never reported as fresh.scripts/manifest.sha256regenerated (scripts/gen-manifest.sh), as the changed files are in the signed-installer trust root.What this does not do
It does not identify the root cause of the original report — that machine's step b emitted zero bytes and I could not reproduce the silent exit on comparable hardware. This makes the next occurrence self-reporting: the log will name the file, line, command, and status.
🤖 Generated with Claude Code
Note
Medium Risk
Touches installer-wide ERR/EXIT handling and the stop-and-check gate that decides fresh vs reconcile, so a regression could misroute re-runs or obscure real failures. Changes are narrowly scoped and covered by new bats tests.
Overview
Makes silent installer deaths self-reporting, and stops telling machines with a stopped Docker daemon that they are a first-time install.
Failure diagnostics (#681). Arms an ERR trap with
set -Eso failures insideinstall_macos/install_linuxrecord file, line, command, and exit status.install_cleanupnames the site on screen and logs the command; Ctrl-C / SIGTERM (130/143) now report as interrupted instead of "did not complete". Adds per-stagelog()breadcrumbs in step b on both platforms.Runtime classification (#682). New
_assess_runtime_downprobe runs before the cluster check and marks an installed-but-unreachable Docker asdegraded/runtime-down. The run continues (Docker start + reconcile still work); only the misleading "first time" claim is removed. Missing Docker staysfresh;permission deniedis not treated as down.Pipefail / SIGPIPE (#680). Switches early-exit pipes to capture-then-match in
_cluster_exists, macOS admin/arch detection, and related probes so SIGPIPE underpipefailno longer reads as "no cluster" / "not admin" / wrong arch — a second route into the #682 misclassification.Reviewed by Cursor Bugbot for commit 8b5b7f5. Bugbot is set up for automated code reviews on this repo. Configure here.