fix(installer): harden provision location + client-detect guards (#309 bugbot)#315
Merged
Merged
Conversation
…bugbot) Address two Cursor Bugbot findings on the develop→main promotion (#309): 1. Whitespace bypasses the location requirement. In the no-timezone-detection path, `client_location` was checked with `[[ -n ]]` before trimming, so a whitespace-only answer broke the retry loop and skipped the "location is required" error, then fell through to a `client create` with no --location. Trim inside the prompt loops, before the non-empty checks (both the detected and no-detection paths). 2. mktemp failure skipped the client guard. detect_installed_client did `mktemp || return 0`, treating an environment error as "no client here" and silently skipping the Helm one-client guard and the #303 ownership check. Fall back to a path in a dir we own (same pattern as the create step) and only give up when there is genuinely nowhere to write. Regenerated scripts/manifest.sha256 for the changed scripts. NOTE: Bugbot's third finding ("Wrong CLI command in summary" — claiming `tracebloc data ingest` should be `tracebloc dataset push`) is a FALSE POSITIVE and is intentionally NOT changed. Per tracebloc/cli internal/cli/data.go, the canonical command group is `data` with subcommand `ingest`; `dataset`/`push` are deprecated aliases kept for one deprecation cycle. The summary's `tracebloc data ingest ./data` is the correct, current form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
8a9907f to
d852c2e
Compare
saadqbal
approved these changes
Jul 7, 2026
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
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.
Summary
Addresses two of the three Cursor Bugbot findings on the #309 (develop→main) promotion PR. The third is a false positive and is intentionally left unchanged (see below).
Related
Ref #309
Type of change
Findings fixed
client_locationwas checked with[[ -n ]]before trimming, so a whitespace-only answer broke the retry loop, skipped the "location is required" error, and fell through to aclient createwith no--location. Now trimmed inside both prompt loops before the non-empty checks —scripts/lib/provision.sh.detect_installed_clientdidmktemp || return 0, treating an environment error as "no client installed" and silently skipping the Helm one-client guard and the Installer mints a new client before the one-client-per-machine guard — orphans it on the backend #303 ownership check. Now falls back to a dir we own (same pattern as theclient createstep) and only gives up when there's genuinely nowhere to write —scripts/lib/install-client-helm.sh.Finding intentionally NOT changed (false positive)
Bugbot's "Wrong CLI command in summary" claimed
tracebloc data ingest ./datashould betracebloc dataset push. This is backwards. Pertracebloc/cliinternal/cli/data.go:datais canonical;datasetis a deprecated alias ("kept for one deprecation cycle")ingest <local-path>is canonical;pushis a deprecated aliasSo the installer's
tracebloc data ingest ./datais the correct, current command. (e2e-journey.shand the README happen to use thedataset pushalias — still valid during the deprecation window, but not the canonical form.) Left as-is.Test plan
bash -non changed shell scripts — clean.scripts/gen-manifest.sh --check— up to date (manifest regenerated for the two changed scripts).detect_installed_clientis stubbed inprovision.bats.Deployment notes
None.
Note
Medium Risk
Changes affect provisioning and one-client safety checks on install; logic is narrow but mistakes could allow wrong client pairing or weak location validation.
Overview
Client detection —
detect_installed_clientno longer treats a failedmktempas “no client installed” (which could skip the Helm one-client guard and the #303 ownership pre-flight). It now falls back to a temp file underHOST_DATA_DIR(same pattern asclient createoutput capture) and only returns empty when there is nowhere to write.Location prompts — During interactive provisioning,
client_locationis trimmed before[[ -n ]]checks in both the timezone-detected path and the manual retry loop, so whitespace-only input is treated as blank (Enter to accept detected zone, or keep retrying until a real zone is entered).Reviewed by Cursor Bugbot for commit d852c2e. Bugbot is set up for automated code reviews on this repo. Configure here.