Uh oh!
There was an error while loading. Please reload this page.
fix: Bugbot findings from promotion PR #383 (helm probe timeout + node-local reuse) - #405
Merged
Merged
Conversation
…e-local reuse) A) Ungated Helm in client probe: Get-InstalledClientInfo ran helm list/get with no bounded kubectl probe, so a wedged API server hung Step 4/5 (helm has no request timeout). Add Test-ApiReachable (5s-bounded kubectl probe, mirrors Get-TrainingResources) and gate the enumeration behind it — an unreachable API now degrades to ListUnknown instead of hanging. B) Reuse ignores node-local data: under TB_STORAGE_MODE=node-local there is no /tracebloc host bind-mount, so "reuse = keep and adopt" is false. Make the leftover-guard prompt option and the reuse branch honest under node-local (data left on disk, NOT adopted; cluster starts empty in-node). Hostpath behavior unchanged. Regenerate scripts/manifest.sha256 for the cluster.sh + install-k8s.ps1 changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…obe-nodelocal # Conflicts: # scripts/manifest.sha256
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ce98e6c. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…obe-nodelocal # Conflicts: # scripts/manifest.sha256
A) Keep reply rejected under node-local: the node-local prompt shows "[r] keep", but the parser only accepted r/reuse, so typing "keep" aborted the install. Lowercase the reply (tr, bash 3.2-safe) and accept r/reuse/k/keep -> reuse. B) Non-interactive reuse still claimed adopt: the no-TTY recovery guidance advertised --reuse-data as "adopt the existing data" for every mode. Make it storage-mode-aware — under node-local it keeps data on disk, NOT adopted (cluster starts empty in-node), matching the interactive reuse branch. Regenerate scripts/manifest.sha256 for the cluster.sh change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…obe-nodelocal # Conflicts: # scripts/manifest.sha256
saadqbal
approved these changes
Jul 27, 2026
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.

Resolves two Cursor Bugbot findings on the client promotion PR (#383, develop → main).
Fixes:
Get-InstalledClientInfo(scripts/install-k8s.ps1) ranhelm list -A/helm get valueswith no bounded probe. Helm has no request timeout, so a wedged API server hung Step 4 (after browser sign-in) and Step 5's one-client guard indefinitely. Added a small sharedTest-ApiReachablehelper (a 5s-boundedkubectlprobe, mirroring the gateGet-TrainingResourcesalready uses) and gated the enumeration behind it — an unreachable API now degrades to the existingListUnknownshape instead of hanging. (Release develop → main #383 (comment))scripts/lib/cluster.sh) offered "[r] reuse — keep and adopt the existing data" and logged adoption, but underTB_STORAGE_MODE=node-localthere is no/traceblochost bind-mount, so the data is stranded and the cluster starts empty. Made the prompt option label and the reuse branch honest under node-local (data left on disk, not adopted; guidance to re-ingest or switch to hostpath). Hostpath behavior is unchanged. (Release develop → main #383 (comment))Coverage: Pester tests for
Test-ApiReachableandGet-InstalledClientInfodegrading on an unreachable API without calling helm; bats tests for node-local reuse (non-interactive + interactive) staying honest while hostpath reuse is unchanged.scripts/manifest.sha256refreshed for thecluster.sh+install-k8s.ps1changes. No golden impact (the interactive prompt/reuse copy isn't captured in the copy-catalog goldens).Lands on develop; the promotion PR head picks these up on the next develop sync and Bugbot re-reviews.
🤖 Generated with Claude Code
Note
Low Risk
Installer UX and fail-closed behavior only; no auth or runtime changes. Hostpath paths unchanged; Windows change degrades to existing ListUnknown instead of hanging.
Overview
Addresses two Bugbot findings from the promotion PR: Windows installer hangs and misleading node-local “reuse”.
On Windows (
install-k8s.ps1), addsTest-ApiReachable(5skubectl get --raw=/readyz) and runs it at the start ofGet-InstalledClientInfobefore anyhelm list/helm get values. If the API is down, the function returnsListUnknownand skips helm so Steps 4–5 fail closed instead of freezing indefinitely.On bash (
cluster.shguard_leftover_data), whenTB_STORAGE_MODE=node-local, the leftover prompt no longer claims “reuse = adopt”: it offers [r] keep (data left on disk, cluster starts empty), accepts keep/k as well as r/reuse (any case), warns on the reuse branch, and updates non-interactive--reuse-datahelp to say data is NOT adopted. Hostpath reuse behavior is unchanged.Pester and bats cover the probe, helm gating, and node-local vs hostpath paths;
scripts/manifest.sha256is refreshed for the touched scripts.Reviewed by Cursor Bugbot for commit a22c784. Bugbot is set up for automated code reviews on this repo. Configure here.