From 3708e15b700a0da02622f6e1d1f545cef00c5e4e Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Tue, 7 Jul 2026 18:56:23 +0500 Subject: [PATCH] fix(cli): delete guard fails fast on 426; keep-data save failure marks offboard degraded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more Cursor Bugbot findings on the develop->main PR (#164), same "never-lie / fail-fast" class as the prior rounds: - High: under --keep-data, if cfg.Save() fails after clearing the pointer in memory, the on-disk config still names the revoked client AND the closing line could still read as a clean offboard. Mark the offboard degraded on that Save failure (honest closing) and point the user at `tracebloc logout` / manual removal. - Medium: the pre-offboard online guard treated ANY lookupClientStatus error — including *api.UpgradeRequiredError (426, CLI too old) — as a warn-and-continue. A 426 won't recover by proceeding (the whole offboard hits the same backend), so fail fast with the upgrade message, matching runClientStatus. Other errors (5xx/429/network) stay transient. Test added: a 426 during the guard fails fast with the upgrade message and runs neither revoke nor any teardown step. go build/vet/test green (11 pkgs). Rolls up under the §7.10 offboarding work (Bugbot follow-up). Co-Authored-By: Claude Opus 4.8 --- internal/cli/delete.go | 15 ++++++++++++++- internal/cli/delete_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/internal/cli/delete.go b/internal/cli/delete.go index d84b901b..c1c45159 100644 --- a/internal/cli/delete.go +++ b/internal/cli/delete.go @@ -137,6 +137,14 @@ func runDelete(ctx context.Context, p *ui.Printer, pr prompter, o deleteOpts) er // unreachable precisely because it's being retired); warn and continue. if !o.force { if st, found, lerr := lookupClientStatus(ctx, client, prof.ActiveClientID); lerr != nil { + // A 426 (CLI too old) won't recover by continuing — the whole offboard + // talks to the same backend, so fail fast with the upgrade message rather + // than imply the guard was merely skipped. Other errors (5xx/429/network) + // are transient: warn and continue, since the teardown is the real gate. + var ue *api.UpgradeRequiredError + if errors.As(lerr, &ue) { + return &exitError{code: 1, err: lerr} + } p.Hintf("Couldn't check whether this client is still online (%v) — continuing; pass --force to skip this check.", lerr) } else if !found { // The stored id isn't among this account's clients — likely a stale @@ -233,7 +241,12 @@ func runDelete(ctx context.Context, p *ui.Printer, pr prompter, o deleteOpts) er prof.ActiveClientID, prof.ActiveClientName, prof.ActiveClientNamespace = "", "", "" if o.keepData { if serr := cfg.Save(); serr != nil { - p.Warnf("Kept local data, but couldn't clear the active-client pointer (%v).", serr) + // The in-memory pointer was cleared but not persisted — the on-disk config + // still names the revoked client. Mark degraded so the closing doesn't read + // as a clean offboard, and tell the user it needs a hand. + degraded = true + p.Warnf("Kept local data, but couldn't clear the stored active-client pointer (%v) — "+ + "the on-disk config still names the revoked client; run `tracebloc logout` or remove it by hand.", serr) } else { p.Infof("Kept local data and config (~/.tracebloc); cleared the active-client pointer — --keep-data.") } diff --git a/internal/cli/delete_test.go b/internal/cli/delete_test.go index 2593ef0b..a535fc70 100644 --- a/internal/cli/delete_test.go +++ b/internal/cli/delete_test.go @@ -299,6 +299,36 @@ func TestDelete_TeardownFailure_HonestClosing(t *testing.T) { } } +// A 426 (CLI too old) during the pre-offboard online check must fail fast with the +// upgrade message — not warn-and-continue into the revoke/teardown. +func TestDelete_Guard426_FailsFast(t *testing.T) { + revoked := false + withClientBackend(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke") { + revoked = true + } + if r.Method == http.MethodGet && r.URL.Path == "/edge-device/" { + w.WriteHeader(http.StatusUpgradeRequired) // 426 + _, _ = w.Write([]byte(`{"error":"upgrade_required","min_version":"1.2.3"}`)) + } + }) + setActiveForDelete(t, "5", "gpu-box-01", "gpu-box-01") + fn := &fakeNodeboot{executable: filepath.Join(t.TempDir(), "tracebloc")} + fn.install(t) + + var out bytes.Buffer + err := runDelete(context.Background(), ui.New(&out), nil, deleteOpts{yes: true}) + if err == nil || !strings.Contains(err.Error(), "too old") { + t.Fatalf("want a fail-fast upgrade error, got: %v", err) + } + if revoked { + t.Error("revoke must NOT run after a 426 guard failure") + } + if len(fn.calls) != 0 { + t.Errorf("no teardown after a 426 guard failure, got: %v", fn.calls) + } +} + // --kubeconfig/--context must reach the helm uninstall — otherwise the release is // uninstalled against the ambient current-context, which may be the wrong cluster. func TestDelete_KubeconfigContext_ReachHelm(t *testing.T) {