feat(bootstrap): prompt for Tailscale separately and harden Linux tool installs - #345
Conversation
…l installs - Decouple Linux/SteamOS Tailscale setup (`prompt_tailscale_install` / `INSTALL_TAILSCALE`) from `INSTALL_AI` so opting into Claude/Antigravity on Debian Rodete / gLinux workstations does not attempt `curl https://tailscale.com/install.sh | sh` (which fails on `rodete`). - Honour `INSTALL_TAILSCALE=true|false` from the environment and validate unrecognized values, matching `INSTALL_AI`. - Guard `piper-tts` pip install on `command -v pip3` and add non-fatal warnings for `rustup`, `uv`, and `atuin` curl installers under `set -euo pipefail`. - Add unit tests for `INSTALL_TAILSCALE` env overrides and `prompt_tailscale_install`.
| if ! command -v piper >/dev/null 2>&1; then | ||
| if ! command -v piper >/dev/null 2>&1 && command -v pip3 >/dev/null 2>&1; then | ||
| echo "Installing piper-tts..." | ||
| pip3 install --user piper-tts 2>/dev/null || pip3 install --user --break-system-packages piper-tts |
There was a problem hiding this comment.
[Important] The new command -v pip3 guard only covers pip3 being absent — it does not make a failing download non-fatal, which is what the PR body claims ("Guards piper-tts on command -v pip3 ... so blocked downloads do not abort bootstrap.sh -p"). When pip3 is present and the index is unreachable, both attempts in this || chain fail, the chain exits non-zero, and set -euo pipefail kills the run.
Fails when: on a gLinux/rodete host with pip3 installed but PyPI blocked by the corporate mirror, ./bootstrap.sh -p runs both pip3 install ... piper-tts attempts, both exit non-zero, and install_apt_packages aborts before Node.js, snaps, lazygit, Go, glow, bazelisk and the Nerd Font — and Phase 2 sync_dotfiles never runs at all.
The update path at bootstrap.sh:945 already gets this right: it ends in || true. Suggest matching it, e.g. appending a third || echo " Warning: Failed to install piper-tts (network issue?)" branch to this line.
| prompt_tailscale_install | ||
| if [[ "$INSTALL_TAILSCALE" == true ]] && ! command -v tailscale >/dev/null 2>&1; then | ||
| echo "Installing Tailscale..." | ||
| curl -fsSL https://tailscale.com/install.sh | sh |
There was a problem hiding this comment.
[Important] This is the installer the PR exists because of ("which fails on rodete"), and it is the one curl installer left unhardened — rustup, uv and atuin all gained a || echo "Warning: ..." fallback in this PR, but this one did not.
The new prompt only helps a user who actively answers 2. prompt_tailscale_install treats anything that is not 2 as install, so a bare Enter — the documented default — sets INSTALL_TAILSCALE=true and lands right back on this line.
Fails when: a user on Debian rodete runs ./bootstrap.sh -p and presses Enter at the new "Tailscale setup" prompt, tailscale.com/install.sh exits non-zero on the unsupported distro, and set -euo pipefail aborts install_apt_packages — skipping the rest of Phase 1 and all of Phase 2, which is the exact failure mode this PR set out to remove.
Suggest curl -fsSL https://tailscale.com/install.sh | sh || echo " Warning: Failed to install Tailscale (unsupported distro or network issue?)". The SteamOS path at bootstrap.sh:626-637 already degrades gracefully via its empty-ts_version guard.
| return 0 | ||
| fi | ||
|
|
||
| # Non-interactive: default to skip (install script may not support this distro) |
There was a problem hiding this comment.
[Suggestion] prompt_ai_install deliberately announces its non-TTY skip, with a comment explaining why ("a silent skip makes a partial install indistinguishable from a full one") and a dedicated regression test, test_prompt_ai_install_announces_non_tty_skip. This new prompt skips silently.
The behaviour change is real for automation: a provisioning run that previously set INSTALL_AI=true non-interactively got Tailscale, and now gets nothing, with no output saying so. Worth mirroring the existing convention — two echo ... >&2 lines here naming INSTALL_TAILSCALE=true as the opt-in.
|
|
||
| echo "" | ||
| echo "AI assistant setup (Claude, Antigravity, MCP servers, Tailscale):" | ||
| echo "AI assistant setup (Claude, Antigravity, MCP servers):" |
There was a problem hiding this comment.
[Suggestion] The user-visible string here was correctly updated to drop Tailscale, but prompt_ai_install's own docstring at bootstrap.sh:518 still reads # Ask once whether to install AI assistant packages (Claude, Tailscale). — stale now that Tailscale moved to prompt_tailscale_install.
| echo "Warning: ignoring INSTALL_AI='$INSTALL_AI' (expected 'true' or 'false')." >&2 | ||
| export INSTALL_AI="" | ||
| fi | ||
| export INSTALL_TAILSCALE="${INSTALL_TAILSCALE:-}" |
There was a problem hiding this comment.
[Suggestion] INSTALL_TAILSCALE is not discoverable. Two places document INSTALL_AI and neither mentions the new variable:
bootstrap.sh:1440— the--help"Environment:" section lists onlyINSTALL_AI=true|false.CLAUDE.md:54— Phase 2 step 3 documents theprompt_ai_installgate and theINSTALL_AI=trueescape hatch; the Phase 1 package step says nothing about the new Tailscale gate.
Since the non-TTY default is now skip, anyone automating a Linux provision needs INSTALL_TAILSCALE=true and currently has no way to learn that except from the source.
| test_install_tailscale_env_override_survives_sourcing() { | ||
| local out | ||
| out=$(INSTALL_TAILSCALE=true _run_sourced 'echo "VALUE=$INSTALL_TAILSCALE"') || return 1 | ||
| grep -q '^VALUE=true$' <<< "$out" |
There was a problem hiding this comment.
[Suggestion] The new tests mirror three of the five INSTALL_AI tests but skip two that guard real footguns:
test_install_ai_env_false_survives_sourcinghas noINSTALL_TAILSCALE=falsecounterpart. The opt-out direction is the one an automated caller reaches for on rodete, so it is worth pinning.test_install_ai_default_is_env_respectinggreps the source to stop a future edit back to a bareexport INSTALL_AI="".INSTALL_TAILSCALEhas the identical failure mode (the env value would be clobbered beforeprompt_tailscale_installreads it) and no such guard.
There was a problem hiding this comment.
Code Review — Round 1
Summary
The INSTALL_TAILSCALE decoupling is clean and mirrors the existing INSTALL_AI mechanism closely — env validation, cached prompt, already-installed short-circuit — and the new tests cover the interesting branches. The blocking findings are both about the second half of the stated goal, "so blocked downloads do not abort bootstrap.sh -p under set -euo pipefail": the two installers most likely to fail on the rodete/gLinux hosts this PR targets are still fatal.
Findings
- [Important]
bootstrap.sh:786— thecommand -v pip3guard only covers pip3 being absent, not the blocked download the PR body says it guards. Comparebootstrap.sh:945, which ends in|| true. - [Important]
bootstrap.sh:712—tailscale.com/install.shis the one curl installer that did not get a|| echo "Warning: ..."fallback in this PR, and taking the prompt default (bare Enter) onrodetewalks straight back into the abort. - [Suggestion]
bootstrap.sh:568— silent non-TTY skip, unlikeprompt_ai_install. - [Suggestion]
bootstrap.sh:542— stale docstring at line 518 still names Tailscale. - [Suggestion]
bootstrap.sh:33—INSTALL_TAILSCALEmissing from--helpand CLAUDE.md. - [Suggestion]
tests/test-bootstrap.sh:806— two of the fiveINSTALL_AItests have noINSTALL_TAILSCALEcounterpart.
Verdict
REQUEST_CHANGES - Two installers in install_apt_packages still abort Phase 1 under set -euo pipefail on exactly the blocked-download hosts this PR is meant to unblock: taking the default at the new Tailscale prompt on rodete still kills the run at bootstrap.sh:712, and bootstrap.sh:786 still aborts when pip3 exists but PyPI is unreachable — in both cases skipping the rest of Phase 1 and all of Phase 2.
Automated review by Claude Code
| # Install Tailscale via official apt repo | ||
| prompt_ai_install | ||
| if [[ "$INSTALL_AI" == true ]] && ! command -v tailscale >/dev/null 2>&1; then | ||
| prompt_tailscale_install |
There was a problem hiding this comment.
[Important] Swapping prompt_ai_install for prompt_tailscale_install here (and identically at bootstrap.sh:625 for SteamOS) removes the only calls to prompt_ai_install on the Linux path. INSTALL_AI is still consumed downstream at bootstrap.sh:944 — if [[ "$INSTALL_AI" == true ]] && command -v pip3 >/dev/null 2>&1; then, gating the piper-tts upgrade.
update_packages runs at bootstrap.sh:1466, before sync_dotfiles (bootstrap.sh:1349) — the next remaining prompt_ai_install call. macOS is unaffected because install_brew_packages still prompts at bootstrap.sh:1053.
Fails when: a Debian/gLinux host runs ./bootstrap.sh -p interactively without INSTALL_AI exported → install_apt_packages no longer prompts, so update_packages sees INSTALL_AI="", the guard is false, and the piper-tts upgrade is silently skipped — where before this PR the prompt at this very line had already set INSTALL_AI=true on the default answer.
Either add a prompt_ai_install call in update_packages just before the pip block (it is idempotent and cached, so it is a no-op on macOS), or restore one in install_linux_common_packages, which runs on every Linux distro.
| # Install Tailscale (static binary + system service) | ||
| prompt_ai_install | ||
| if [[ "$INSTALL_AI" == true ]] && ! command -v tailscale >/dev/null 2>&1; then | ||
| prompt_tailscale_install |
There was a problem hiding this comment.
[Suggestion] The SteamOS branch did not get the hardening the apt branch got. Three lines below, ts_version=$(curl -fsSL "https://pkgs.tailscale.com/stable/" | grep -oP ... | head -1) is a bare assignment under set -euo pipefail, so a blocked or unreachable pkgs.tailscale.com (or simply no grep -P match) aborts Phase 1 outright — the same class of abort this PR fixes for rustup, uv, atuin and the apt Tailscale installer. Now that the new prompt defaults to "1) Install" on a bare Enter, that path is easier to reach than it was.
Appending || echo " Warning: Failed to fetch Tailscale version, skipping" to the assignment would round this out, matching the shape the Go install already uses at bootstrap.sh:646. Pre-existing, so fair to leave out if you would rather keep the diff tight.
|
|
||
| # Install piper-tts (text-to-speech) | ||
| if ! command -v piper >/dev/null 2>&1; then | ||
| if ! command -v piper >/dev/null 2>&1 && command -v pip3 >/dev/null 2>&1; then |
There was a problem hiding this comment.
[Suggestion] The command -v pip3 guard makes a missing pip3 a fully silent no-op — the echo "Installing piper-tts..." sits inside the guard, so a host without pip3 prints nothing at all and looks like a clean full install. Elsewhere in this file a skipped optional install announces itself, e.g. install_linux_common_packages: echo "Skipping sccache (requires cargo and cc)". An else echo "Skipping piper-tts (requires pip3)" would match.
| grep -q 'export INSTALL_AI="\${INSTALL_AI:-}"' "$BOOTSTRAP" | ||
| } | ||
|
|
||
| test_install_tailscale_env_override_survives_sourcing() { |
There was a problem hiding this comment.
[Suggestion] The six new tests cover INSTALL_TAILSCALE in isolation, but none asserts the property the PR exists for: that the two gates are independent. A future refactor that re-coupled them (say, restoring INSTALL_AI in the Tailscale conditional) would still pass all six.
A one-liner pins it down:
test_install_ai_does_not_imply_tailscale() {
local out
out=$(INSTALL_AI=true _run_sourced (single-quote)PATH=; prompt_tailscale_install; echo "VALUE=$INSTALL_TAILSCALE"(single-quote)) || return 1
grep -q (single-quote)^VALUE=false$(single-quote) <<< "$out"
}
(substitute real single quotes where marked — GitHub API quoting)
There was a problem hiding this comment.
Code Review — Round 2
Summary
Both Round 1 blocking findings are resolved: the apt tailscale.com/install.sh call and the piper-tts install now carry || echo "Warning: ..." fallbacks, and all four Round 1 suggestions (non-TTY announcement, stale docstring, --help + CLAUDE.md docs, the two missing INSTALL_TAILSCALE test counterparts) were picked up too. prompt_tailscale_install mirrors prompt_ai_install faithfully.
One new finding, and it comes from the decoupling itself rather than the hardening: prompt_ai_install was removed from both Linux package paths and nothing replaced it, so INSTALL_AI is still empty when update_packages reads it on Linux.
Findings
- [Important]
bootstrap.sh:710(and:625) — removingprompt_ai_installfrom the Linux paths leavesINSTALL_AIempty atbootstrap.sh:944, silently skipping thepiper-ttsupgrade. - [Suggestion]
bootstrap.sh:625— the SteamOSts_version=$(curl ... | grep -oP ...)assignment is still a hard abort underset -euo pipefail, unlike the apt path this PR hardened. - [Suggestion]
bootstrap.sh:785— a pip3-less host now skipspiper-ttswith no output at all. - [Suggestion]
tests/test-bootstrap.sh:803— no test asserts thatINSTALL_AIandINSTALL_TAILSCALEare actually independent.
Previously Addressed (Filtered)
All six Round 1 items were implemented; none are re-raised.
Verdict
REQUEST_CHANGES - install_apt_packages and install_steamos_packages were the only Linux callers of prompt_ai_install, and both lost that call here. update_packages still gates the piper-tts upgrade on INSTALL_AI (bootstrap.sh:944) and runs before sync_dotfiles, the next remaining caller — so on Debian/SteamOS that upgrade is now silently skipped on every bootstrap.sh -p unless the user exports INSTALL_AI=true.
Automated review by Claude Code
| if ! command -v piper >/dev/null 2>&1; then | ||
| echo "Installing piper-tts..." | ||
| pip3 install --user piper-tts 2>/dev/null || pip3 install --user --break-system-packages piper-tts | ||
| prompt_ai_install |
There was a problem hiding this comment.
[Suggestion] Splitting the two gates also split the two prompts: prompt_tailscale_install fires at :712, and prompt_ai_install now fires here — after the snap installs, lazygit, Go, glow and bazelisk. An interactive ./bootstrap.sh -p that answers the Tailscale question and walks away comes back to a run parked on a second question several minutes in.
Before this PR the single prompt_ai_install sat at the old :709, so the run only ever stopped for input once, early. Hoisting both calls to the top of install_apt_packages (they are idempotent — each returns immediately once its variable is set) would restore "ask everything up front, then run unattended." The same shape applies to install_steamos_packages, which now prompts at :625 only.
| echo "Installing Tailscale..." | ||
| local ts_version | ||
| ts_version=$(curl -fsSL "https://pkgs.tailscale.com/stable/" | grep -oP 'tailscale_\K[0-9.]+(?=_amd64\.tgz)' | head -1) | ||
| ts_version=$(curl -fsSL "https://pkgs.tailscale.com/stable/" | grep -oP 'tailscale_\K[0-9.]+(?=_amd64\.tgz)' | head -1 || true) |
There was a problem hiding this comment.
[Suggestion] The || true fixes the version lookup, but the lines it guards are still fatal: curl -fsSL ... -o /tmp/tailscale.tgz (:631), tar xf (:632) and the two cps all run bare under set -euo pipefail. A SteamOS host that can reach the pkgs.tailscale.com/stable/ index but has the tarball itself blocked gets a non-empty ts_version, passes the [[ -n ]] check, and aborts Phase 1 at :631 — skipping update_packages and Phase 2 entirely, which is the failure mode the PR body set out to eliminate.
Non-blocking because those lines are pre-existing and untouched here. If you want the SteamOS path to match the hardening the apt path got, wrapping :631-:637 in an if curl ...; then ... else echo " Warning: ..."; fi covers it. The Go install at :648-:658 has the identical shape if you go that route.
| if [[ "$INSTALL_TAILSCALE" == true ]] && ! command -v tailscale >/dev/null 2>&1; then | ||
| echo "Installing Tailscale..." | ||
| curl -fsSL https://tailscale.com/install.sh | sh | ||
| curl -fsSL https://tailscale.com/install.sh | sh || echo " Warning: Failed to install Tailscale (unsupported OS or network issue?)" |
There was a problem hiding this comment.
[Suggestion] On macOS, Tailscale still arrives as cask "tailscale-app" in Brewfile.ai:5, gated on INSTALL_AI — so INSTALL_TAILSCALE is a Linux/SteamOS-only knob and setting INSTALL_TAILSCALE=false on a Mac is silently inert.
That is a defensible scope (the rodete failure this PR targets is Linux-only) and the --help text does say "on Linux/SteamOS". Worth making the asymmetry explicit in the CLAUDE.md Phase 1 note alongside the sentence you added, or moving the cask behind the same gate, so the next reader does not assume the variable is cross-platform.
| [[ $rc -eq 0 ]] && grep -q '^VALUE=true$' <<< "$out" | ||
| } | ||
|
|
||
| test_install_ai_and_tailscale_are_independent() { |
There was a problem hiding this comment.
[Suggestion] The seven new tests cover the variable plumbing thoroughly — env override in both directions, validation, non-TTY skip, already-installed detection, and now independence — but nothing covers the call sites, and that is exactly where Round 2 regression lived: prompt_ai_install was dropped from both Linux package functions and every one of these tests still passed.
A static guard in the spirit of assert_no_match_in_files would close it: assert that the install_apt_packages and install_steamos_packages bodies each contain prompt_tailscale_install, and that install_apt_packages and update_packages each contain prompt_ai_install. Cheap, and it makes the gate wiring as regression-proof as the gates themselves.
There was a problem hiding this comment.
Code Review — Round 3
Summary
The Round 2 blocker is fixed: prompt_ai_install is back on the Linux paths — once in install_apt_packages (bootstrap.sh:787, gating piper-tts) and once in update_packages (bootstrap.sh:951, gating the piper-tts upgrade) — so INSTALL_AI is no longer empty when update_packages reads it on Debian/SteamOS. All three Round 2 suggestions landed too: the SteamOS ts_version pipeline now ends in || true (:629) with an explicit warning on the empty branch (:639), a pip3-less host prints Skipping piper-tts (pip3 not installed) (:793), and test_install_ai_and_tailscale_are_independent asserts the two gates are actually separate.
The decoupling itself reads cleanly. prompt_tailscale_install mirrors prompt_ai_install branch-for-branch — cached-value short-circuit, already-installed short-circuit, announced non-TTY skip, same true|false env validation — and every path returns 0, so neither helper can trip set -euo pipefail at a call site. install_apt_packages and install_steamos_packages both switched their Tailscale guard to INSTALL_TAILSCALE, which is the whole point: taking the default on rodete no longer drags tailscale.com/install.sh in behind the Claude opt-in, and when it is requested it now degrades to a warning instead of aborting Phase 1.
No finding this round names a way the code produces a wrong result.
Findings
- [Suggestion]
bootstrap.sh:787— the AI prompt now fires ~75 lines after the Tailscale prompt, so an interactive-prun stops for input twice. - [Suggestion]
bootstrap.sh:629— the tarball download andtar xfbelow the hardened version lookup are still fatal underset -euo pipefail. - [Suggestion]
bootstrap.sh:715— macOS still getstailscale-appfromBrewfile.aiunderINSTALL_AI;INSTALL_TAILSCALEis inert there. - [Suggestion]
tests/test-bootstrap.sh:845— the new tests cover the variables but not the call sites, which is where the Round 2 regression lived.
Previously Addressed (Filtered)
All six Round 1 items and all four Round 2 items were implemented; none are re-raised. The Round 2 suggestion about the SteamOS Tailscale abort was implemented for the version lookup — the inline note at :629 concerns the three lines below it, which this PR does not touch, and is explicitly non-blocking for that reason.
Verdict
APPROVE - No blocking findings. Both Linux install paths are now hardened against the blocked-download aborts the PR targets, and the INSTALL_AI regression from Round 2 is resolved.
Automated review by Claude Code
Summary
prompt_tailscale_install/INSTALL_TAILSCALE) fromINSTALL_AIso opting into Claude/Antigravity on Debian Rodete / gLinux hosts does not triggercurl -fsSL https://tailscale.com/install.sh | sh(which fails onrodete).INSTALL_TAILSCALE=true|falsefrom the environment with validation matchingINSTALL_AI.piper-ttsoncommand -v pip3and adds non-fatal warning fallbacks forrustup,uv, andatuincurl installers so blocked downloads don't abortbootstrap.sh -punderset -euo pipefail.tests/test-bootstrap.sh.Test Plan
make check(lint, syntax, 129 hook tests, 63 bootstrap tests)