Uh oh!
There was an error while loading. Please reload this page.
fix(installer): honor TRACEBLOC_CLIENT_ADOPTED — reconcile a connected client in place (RFC-0001 §7.2 / #838) - #298
Conversation
410c894 to
a5a5b7dComparesaadqbal
commented
Jul 2, 2026
Restacked: rebased onto #295's branch and retargeted the base there, so this PR now shows only the adopt-reconcile diff (the #295 canary-restore conflict — bats + manifest — is already resolved in the stack). Regenerated Release path: merge #295 → develop first; GitHub auto-retargets this to develop; then merge this → cut v1.8.4 with both. |
LukasWodka
commented
Jul 2, 2026
👋 Heads-up — Code review queue is at 35 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
Uh oh!
There was an error while loading. Please reload this page.
saadqbal
commented
Jul 2, 2026
Fixed in e6be65d — good catch. |
Uh oh!
There was an error while loading. Please reload this page.
…d client in place (RFC-0001 §7.2 / #838) Re-running the installer on an already-connected client did not behave per RFC-0001 §7.2. provision_client (Step 3) correctly adopts the existing client (get-or-create keyed on the cluster) and writes TRACEBLOC_CLIENT_ADOPTED=1 + the UUID + namespace to the credential file — but install-client-helm.sh had NO adopt handling. Its non-interactive path is gated on ID *and* PASSWORD both being set; adopt issues no password (the existing one stands, write-only on the backend), so Step 5 fell through to the manual "Client ID:" prompt, defaulting to the stale clientId in the previous values.yaml — a numeric dashboard id on any cluster first installed during the cli#125 window — which verify_credentials then rejected. Teach Step 5 an adopt branch: on TRACEBLOC_CLIENT_ADOPTED=1, find the live client release and `helm upgrade` it in place, reusing its stored credential (no prompt, no verify, no duplicate) and healing the stored clientId to the adopted UUID so a cli#125-era install self-corrects. Prefer --reset-then-reuse-values (Helm >= 3.14), fall back to --reuse-values. Falls through to the normal connect flow only when no live release is found. The "previous settings?" prompt is skipped on adopt. Extracted _resolve_chart_ref (shared by the fresh-install and adopt paths). bats: 3 new adopt tests (reconcile-in-place + heal, older-Helm fallback, no-release fallback); full suite green (47). No Chart.yaml bump here — ships with #295 as v1.8.4 (avoids a version conflict). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…D (Bugbot) Bugbot (High): _reconcile_adopted_client read the adopted UUID from TRACEBLOC_CLIENT_ID for --set clientId, but provision_client CLEARS TRACEBLOC_CLIENT_ID on a real adopt (keeping only TB_NAMESPACE + TRACEBLOC_CLIENT_ADOPTED=1 — "let install_client_helm reconcile the existing release from the local values.yaml"). So reconcile always bailed on the empty id and fell through to the interactive prompt — exactly what this PR set out to fix. The bats tests masked it by exporting TRACEBLOC_CLIENT_ID directly. Reconcile the discovered live release in place with --reset-then-reuse-values (fallback --reuse-values), reusing its stored credential — no dependency on TRACEBLOC_CLIENT_ID, no --set clientId heal (which fought provision's design and couldn't work anyway). Tests now export ONLY TRACEBLOC_CLIENT_ADOPTED=1, matching what provision_client actually leaves in the env. Manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolves both Bugbot findings on the adopt path, which were in tension: • High — reconcile read the UUID from TRACEBLOC_CLIENT_ID for --set clientId, but provision_client cleared it on adopt → reconcile bailed on the empty id and fell through to the interactive prompt. • Medium — the interim fix reconciled with --reuse-values only, so a cli#125-era numeric clientId was preserved and the workload still couldn't auth. provision_client now KEEPS + exports the adopted client id (its UUID username) on the adopt path (dropping only the absent password), so install_client_helm can: • heal the stored clientId to the UUID via `--set clientId=<uuid>` when present (reused values keep the correct stored password) — fixes cli#125-era installs; • reconcile WITHOUT a heal (no bail to a prompt) when no id was handed over (rebuilt host / R7 orphan) — the existing credential stands. Optional --set built as an args array (bash-3.2 safe). Tests: heal-with-UUID + no-id-reconcile-fallback for Step 5; provision adopt test asserts the id is kept. provision.bats 12/12, install-client-helm.bats green; manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e6be65d to
dcbc23fComparesaadqbal
commented
Jul 2, 2026
Both findings fixed in dcbc23f — they were in tension, so resolved together:
Tests: heal-with-UUID + no-id-reconcile-fallback (Step 5); provision adopt test asserts the id is kept. Retargeted to develop now that #295 is merged, so the full installer CI runs. |
saadqbal
commented
Jul 2, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit dcbc23f. Configure here.
Closes#297. Draft — pending the same dev e2e as #295 (the adopt-path run), and a small merge coordination with #295 (below).
Problem
Re-running the installer on an already-connected client doesn't behave per RFC-0001 §7.2.
provision_client(Step 3) correctly adopts the existing client and writesTRACEBLOC_CLIENT_ADOPTED=1+ the UUID + namespace — butinstall-client-helm.shhas no adopt handling. Its non-interactive path needs ID and password both set; adopt issues no password, so Step 5 falls through to the manualClient ID:prompt, defaulting to the staleclientIdin the oldvalues.yaml(a numeric dashboard id on any cluster first installed during the cli#125 window) →verify_credentialsrejects it → the loop seen on dev.RFC-0001 §7.2
A re-run on a connected client must be a silent, idempotent reconcile — no new client, no new credential, no prompt.
Fix
Step 5 adopt branch: on
TRACEBLOC_CLIENT_ADOPTED=1, discover the live client release andhelm upgradeit in place, reusing its stored credential (no prompt, no verify, no duplicate) and healing the storedclientIdto the adopted UUID so a cli#125-era install self-corrects. Prefers--reset-then-reuse-values(Helm ≥ 3.14), falls back to--reuse-values. Falls through to the normal connect flow only when no live release exists. The "previous settings?" prompt is skipped on adopt.Also extracted
_resolve_chart_ref(shared by the fresh-install and adopt paths).Tests
3 new bats (reconcile-in-place + clientId heal; older-Helm
--reuse-valuesfallback; no-live-release fallback). Fullinstall-client-helm.batsgreen (47).Merge coordination with #295
Both PRs touch
install-client-helm.sh(different hunks — auto-merge) and regeneratescripts/manifest.sha256(same line — conflicts). Merge #295 first, then rebase this +bash scripts/gen-manifest.sh. NoChart.yamlbump here — ships with #295 as v1.8.4.Refs: RFC-0001 §7.2, backend#830, #838. Fossil source: tracebloc/cli#125.
Note
Medium Risk
Changes installer credential and Helm upgrade behavior on re-runs; mistakes could break reconnects or leave wrong clientId, but scope is bounded to the adopt path with tests and fallback to manual connect.
Overview
Re-running the installer on an already-registered cluster now follows RFC-0001 §7.2: silent in-place reconcile instead of credential prompts and failed verification on stale
values.yaml.When
TRACEBLOC_CLIENT_ADOPTED=1, Step 5 discovers the live client Helm release and runshelm upgradewith--reset-then-reuse-values(or--reuse-valueson older Helm), reusing the release’s stored password. Optionally--set clientId=<adopted UUID>heals cli#125-era installs that stored a numeric dashboard id. No verify, no duplicate--install. If no release exists, it warns and falls through to the normal connect flow.provision.shnow exportsTRACEBLOC_CLIENT_ID,TB_NAMESPACE, and the adopt marker (only clears password) so Helm can healclientId. The “use previous settings?” prompt is skipped on adopt._resolve_chart_refis shared between fresh install and adopt paths.manifest.sha256and bats cover reconcile, UUID heal, Helm fallback, and no-release fallback.Reviewed by Cursor Bugbot for commit dcbc23f. Bugbot is set up for automated code reviews on this repo. Configure here.