Uh oh!
There was an error while loading. Please reload this page.
Installer copy catalog (byte-exact golden of what a user sees) - #366
Conversation
Adds scripts/tests/copy-catalog.bats — a golden catalog mirroring the CLI's (cli repo, internal/cli/testdata/golden/), so the installer's user-facing copy can be reviewed without running an install. Two files under scripts/testdata/golden/, ordered like the run: 00-install.golden — the banner (versioned + direct-run variants), the "2. Installing" roadmap, the six running step headers (a-f; titles read live from install-k8s.sh so they can't drift), and --help. 01-outcomes.golden — print_summary's five end states (connected / starting / bad creds / image pull / crash) + the reboot footer, incl. the macOS/Windows variant. Rendered colour-off (NO_COLOR) so the text is byte-exact; the one live read (_chart_version) is stubbed for determinism, and $HOME collapses to ~, so the goldens are stable across machines/CI. The tests fail on drift; regenerate with TB_UPDATE_GOLDEN=1 bats scripts/tests/copy-catalog.bats. Not in the R8 manifest (tests/ + testdata/ aren't installer scripts); CI already globs scripts/tests/*.bats and excludes .bats from shellcheck. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 22, 2026
bugbot run |
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 f5bfebf. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
shujaatTracebloc
left a comment
There was a problem hiding this comment.
Review — installer copy catalog
Nice addition — a byte-exact copy golden is exactly the right guard against the "change copy → deploy → see it in prod → fix" loop, and the determinism work holds up: I traced it and confirmed _reboot_note keys off $OS (not uname), $HOME→~ collapse in print_summary, the _chart_version stub ordering (sourced then overridden), the 5 driven states match summary.sh's case labels, and CI genuinely globs scripts/tests/*.bats so it wires up with no changes. No CLAUDE.md or reuse issues (summary.bats covers state detection, this covers rendered copy — complementary).
Four things in the harness, ranked. Test-only, so none is a runtime risk — but #1 undercuts the catalog's own promise.
1. Step-header scrape silently drops steps + mishandles titles (blocking-ish)
sed -nE 's/.*step_header ([a-f]) "([^"]*)".*/\1 \2/p'"${SCRIPTS_DIR}/install-k8s.sh"The PR's selling point is "titles read live so they can't drift." This regex only partly delivers:
- Hard-coded
[a-f]— a futurestep_header g "Finishing up"is silently omitted. The catalog still looks complete, so a reviewer gets false "all steps covered" confidence — the exact drift this is meant to catch. "([^"]*)"truncates a title containing a", and a title passed as a variable (step_header a "$FOO") is captured as the literal$FOO, not the rendered text the user sees.
Deeper fix: instead of scraping source text, have install-k8s.sh expose its step titles (a function or array) the test can invoke — then the catalog can't diverge from the real steps regardless of count or quoting.
2. Missing golden → cryptic failure instead of the documented hint
check_golden does diff -u "$golden" "$actual" with no existence check. A new catalog file (or a deleted golden) fails with diff: …/XX.golden: No such file or directory (exit 2) rather than the run TB_UPDATE_GOLDEN=1 … guidance the header advertises — reads as a tooling error, not "generate the golden". A one-line [ -f "$golden" ] || { echo "missing golden — run TB_UPDATE_GOLDEN=1"; return 1; } fixes it.
3. print_summary 2>&1 folds stderr into the byte-exact golden
Safe today (summary is pure stdout; log() is sunk to /dev/null), but if any end-state — or a lib it sources — ever writes to stderr, that text lands in the golden, and because piped stdout is block-buffered while stderr is unbuffered, the two can interleave in a machine-dependent order → a "byte-exact" golden that flakes. Capturing only stdout (drop 2>&1) unless you specifically want to pin stderr copy.
4. Determinism leans on override ordering in setup()
Goldens are machine-independent only because setup() reassigns OS/ARCHafterload_lib sources common.sh (whose top-level OS=$(uname -s) / ARCH=$(uname -m) would otherwise capture the runner). Correct as written, but load-bearing and unremarked — a reorder, or a newly-cataloged function that reads $OS before the override, reintroduces Darwin/arm64 leakage on a Mac. Worth a one-line comment pinning why the order matters.
🤖 Reviewed with Claude Code
shujaatTracebloc
left a comment
There was a problem hiding this comment.
Approving — this is a well-built, byte-exact copy catalog and the determinism holds up (verified: _reboot_note keys off $OS, $HOME→~ collapse, _chart_version stub ordering, CI globs *.bats). Details in my earlier review comment above.
Non-blocking, in priority order:
- Worth a fast-follow: the
sed … step_header ([a-f]) "…"scrape hard-codes[a-f](silently drops a future 7th step) and mis-handles quoted/variable titles — which softens the PR's own "titles can't drift" guarantee. Ideallyinstall-k8s.shexposes its step titles rather than being scraped. Fine to land as-is and tighten later.
2–4. Optional polish: missing-golden should hintTB_UPDATE_GOLDEN=1rather than a rawdifferror; consider dropping2>&1onprint_summaryto keep the golden stdout-only; and a one-line comment on whysetup()must overrideOS/ARCHafterload_lib.
None block merge. 🚀
🤖 Reviewed with Claude Code
Bugbot: print_banner no-ops when TRACEBLOC_BANNER_SHOWN is set (the curl|bash bootstrap exports it after drawing the banner). The copy-catalog setup() never cleared it, so an inherited value — from the shell or a prior install session — would blank both banner samples in emit_install, failing the drift check or regenerating a banner-less 00-install.golden. unset it in setup(), matching the precedent in common.bats. Golden already has both banners (generated clean), so no regen needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
…den is stable (#371) Merging #371's PATH-aware connected-state CTA into a develop that has the #366 copy catalog broke copy-catalog.bats: emit_outcomes renders print_summary, and with TB_CLI_USABLE_NOW unset the new CTA branch prints 'Open a new terminal…' while 01-outcomes.golden still has the happy-path 'Run tracebloc to get started.'. Pin TB_CLI_USABLE_NOW=1 in setup() (mirrors #371's summary.bats) so the catalog deterministically renders the 'Run' line the golden captures — no golden regen needed, and it's robust to an inherited flag (same env-stability class as the TRACEBLOC_BANNER_SHOWN unset). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(install): PATH-aware final CTA — stop telling users to run a CLI this shell cannot find (B2, #1174) install-cli.sh records TB_CLI_USABLE_NOW (1 only when the CLI resolves in THIS shell, not just a fresh terminal). summary.sh reads it via _cli_runnable_now (with a live has-tracebloc fallback) so the final CTA says "Run tracebloc" only when that is true, and "Open a new terminal, then run tracebloc" otherwise — matching the honest guidance install-cli already prints for the ~/.local/bin case. 2 new summary bats; install-cli suite green; shellcheck + manifest clean. The complementary cli-repo change (installer prefers a writable dir already on PATH so the same-terminal case happens more often) is a separate PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(install): B2 CTA relies solely on the fresh-shell signal, not the mutated process PATH (Bugbot #371) _cli_runnable_now dropped its `has tracebloc` fallback: install.sh + provision.sh prepend ~/.local/bin to THIS process PATH, so `has tracebloc` was true even when the user's launching shell could not resolve it — so the CTA wrongly said "Run tracebloc" on the common curl|bash path. Now it trusts only TB_CLI_USABLE_NOW (set from install-cli.sh's FRESH-shell probe); unset/0 → the honest "open a new terminal" branch. Also fixed a pre-existing summary.bats test that assumed the unconditional CTA (pinned TB_CLI_USABLE_NOW=1) — it was RED on Linux CI (masked locally by my having the CLI installed) — and tightened the new B2 test to assert the "Run …" branch specifically. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(install): B2 usable-now gate keys on the CLI install dir, not process PATH (Bugbot #371 r2) TB_CLI_USABLE_NOW was still set from `has tracebloc` in the installer process, whose PATH install.sh prepended with ~/.local/bin — so it was true even when the users returning shell cannot resolve the CLI. New _cli_at_system_dir keys the flag on WHERE the CLI landed: a system dir (unconditionally on PATH) => usable now => "Run tracebloc"; a $HOME bin (~/.local/bin, ~/bin) or unresolved => conservative => "open a new terminal". Unit test for the gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(install): B2 step verdict must match the summary CTA (Bugbot #371) _verify_tracebloc_cli set TB_CLI_USABLE_NOW=0 for a ~/.local/bin install but then printed the usable-now verdict ("... ready — run tracebloc to use it") and returned, because `has tracebloc` is always true (install.sh prepends ~/.local/bin to the process PATH). The summary CTA, keyed on TB_CLI_USABLE_NOW=0, said "open a new terminal" — a direct contradiction on the least-privilege path, and following the step fails command-not-found. Gate the usable-now verdict on `has tracebloc && _cli_at_system_dir`; otherwise fall through to the honest "installed — open a new terminal" guidance and keep TB_CLI_USABLE_NOW=0 so the summary agrees. Add a regression test; the two existing usable-now tests now force _cli_at_system_dir (they mocked tracebloc as a function, so command -v never returned a system path). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(install): summary CTA distinguishes not-on-PATH-yet from new-terminal (Bugbot #371) TB_CLI_USABLE_NOW=0 covered two different states, and the summary always said "open a new terminal" for both: - case A: installed to ~/.local/bin, persisted — a NEW terminal resolves it (only this login shell does not). "Open a new terminal" is correct. - case B: _cli_on_fresh_path failed — a new terminal will NOT find it either; install-cli.sh prints the exact PATH fix and deliberately avoids "open a new terminal". The summary contradicted that, pointing users at a useless step. Add a second signal TB_CLI_ON_FRESH_PATH (set 1 on the fresh-path branch, 0 in the case-B fall-through) and a third summary CTA branch: case B now says "Add tracebloc to your PATH (see above)", matching install-cli.sh. Update the case-A test to set the flag; add a case-B CTA test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(install): unset fresh-path flag defaults to new-terminal, not the see-above PATH fix (Bugbot #371) The case-B CTA branch fired on `else`, catching BOTH TB_CLI_ON_FRESH_PATH=0 (install-cli.sh ran and printed the exact PATH fix) AND the flag being UNSET (CLI step skipped/failed, so nothing was printed above). In the unset case the "Add tracebloc to your PATH (see above)" message pointed at guidance that never appeared. Gate case B on the explicit "0"; fold unset + case A into the safe "open a new terminal" default (matching _cli_runnable_now's documented default). Add an unset-flag test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(install): honest CTA when a system tracebloc pre-exists but the CLI step is skipped (Bugbot #371) Bugbot (Medium): install_tracebloc_cli hard-defaulted TB_CLI_USABLE_NOW=0, so if the CLI step is skipped or fails (download/installer/temp-dir miss → early return, _verify_tracebloc_cli never runs) the connected summary said 'open a new terminal' even when tracebloc was ALREADY on a system PATH dir and works in the user's current shell. Seed the default from the PRE-install state instead: usable-now iff a tracebloc is already resolvable AND at a system dir. Gate on _cli_at_system_dir, NOT bare 'has tracebloc' — install.sh prepends ~/.local/bin to THIS process, which would false-positive a ~/.local/bin install the returning shell can't yet see (the exact trap the earlier #371 findings closed). _verify_tracebloc_cli still overrides per this run's outcome; summary.sh's _cli_runnable_now is unchanged. Tests: pre-existing system tracebloc + failed install → stays 1; ~/.local/bin + failed install → 0. install-cli.bats green; manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(installer): pin TB_CLI_USABLE_NOW in copy-catalog so the CTA golden is stable (#371) Merging #371's PATH-aware connected-state CTA into a develop that has the #366 copy catalog broke copy-catalog.bats: emit_outcomes renders print_summary, and with TB_CLI_USABLE_NOW unset the new CTA branch prints 'Open a new terminal…' while 01-outcomes.golden still has the happy-path 'Run tracebloc to get started.'. Pin TB_CLI_USABLE_NOW=1 in setup() (mirrors #371's summary.bats) so the catalog deterministically renders the 'Run' line the golden captures — no golden regen needed, and it's robust to an inherited flag (same env-stability class as the TRACEBLOC_BANNER_SHOWN unset). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: shujaat hasan <shujaathasan@shujaats-MacBook-Pro.local>

Summary
Adds a copy catalog for the installer — a byte-exact golden of every stable piece of user-facing copy, so wording + layout can be reviewed without running an install. Mirrors the CLI's golden catalog (
clirepo,internal/cli/testdata/golden/).scripts/tests/copy-catalog.batsrenders the copy colour-off (NO_COLOR) into two files underscripts/testdata/golden/, ordered like the run:00-install.golden— the banner (versioned + direct-run variants), the "2. Installing" roadmap, the six running step headers (a–f; titles read live frominstall-k8s.shso they can't drift), and--help.01-outcomes.golden—print_summary's five end states (connected / starting / bad creds / image pull / crash) + the reboot footer, including the macOS/Windows variant.Why
So we stop the "change copy → deploy → see it in prod → fix" loop: the catalog is reviewable on any PR that touches installer copy, and the test fails on drift.
Determinism
_chart_version) is stubbed;$HOMEcollapses to~in the summary, so the goldens are stable across machines/CI.install-k8s.shat test time, so they track the real installer.Test plan
TB_UPDATE_GOLDEN=1 bats scripts/tests/copy-catalog.batsregenerates; plainbats scripts/tests/copy-catalog.batsverifies (drift guard). Both green locally.tests/+testdata/aren't installer scripts). CI already globsscripts/tests/*.batsand excludes.batsfrom shellcheck, so it runs with no wiring changes.🤖 Generated with Claude Code
Note
Low Risk
Test-only additions (BATS + golden fixtures); installer runtime behavior is unchanged.
Overview
Adds an installer copy catalog: BATS tests render stable, user-facing installer text (with
NO_COLOR) into golden files underscripts/testdata/golden/, mirroring the CLI’s golden pattern.copy-catalog.batsdrivesprint_banner,print_roadmap, step headers (titles parsed live frominstall-k8s.sh),print_help, andprint_summaryfor each end state. Output is compared byte-for-byte to00-install.goldenand01-outcomes.golden; drift fails CI unless you regenerate withTB_UPDATE_GOLDEN=1.Determinism comes from stubbing
_chart_version, clearingTRACEBLOC_BANNER_SHOWN, and fixed env vars—no full install required for copy review on PRs.Reviewed by Cursor Bugbot for commit 65b9e8a. Bugbot is set up for automated code reviews on this repo. Configure here.