fix(delete): guard before preview, block on running work, reclaim by reference

internal/api/client.go

@@ -402,6 +402,11 @@ type ProvisionedClient struct {
 	// ClusterID is the kube-system namespace UID this client is anchored to
 	// (RFC-0001 §6.3 / backend#883). Empty on legacy / not-yet-backfilled clients.
 	ClusterID string `json:"cluster_id"`
+	// NumRunningExperiments is how many training runs are active on this client
+	// right now. `tracebloc delete`'s work-guard blocks on this — not on Status
+	// (== online, which a healthy client always is) — so a live-but-idle
+	// environment can still be offboarded after the typed-name confirm.
+	NumRunningExperiments int `json:"num_running_experiments"`
 }
 
 // CreateClientRequest is the POST /edge-device/ body. The account is stamped

internal/cli/delete.go

@@ -125,49 +125,46 @@ func runDelete(ctx context.Context, p *ui.Printer, pr prompter, o deleteOpts) er
 		ns = prof.ActiveClientNamespace
 	}
 
-	// The three-way scope summary — shown BEFORE the confirm so the user sees
-	// exactly what is removed, kept, and left alone (RFC-0001 §7.10).
-	renderOffboardSummary(p, name, ns, o.keepData)
-
-	// Live-work guard (inherits §7.4): refuse if tracebloc still reports this
-	// client online / with a running job, unless --force. The heartbeat is
-	// advisory, so this is a courtesy stop, not the safety — the teardown itself
-	// is the real gate. A lookup failure is non-fatal (the client may be
-	// unreachable precisely because it's being retired); warn and continue.
+	// Work-guard: refuse to offboard while training runs are ACTIVE (offboarding
+	// would kill them), unless --force. Block on RUNNING experiments, NOT on
+	// "online": a healthy environment is always online (it heartbeats), so
+	// blocking on that would make offboard impossible without --force and mislead
+	// ("stop your training jobs" when there are none). A lookup failure is
+	// non-fatal (the environment may be unreachable precisely because it's being
+	// retired); warn and continue — the typed-name confirm and the teardown are
+	// the real gates. 426/401/403 won't recover by continuing, so fail fast.
 	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.
+		pc, lerr := client.GetClient(ctx, id)
+		switch {
+		case lerr != nil:
 			var ue *api.UpgradeRequiredError
 			if errors.As(lerr, &ue) {
 				return &exitError{code: exitFailure, err: lerr}
 			}
-			// A 401/403 means the signed-in credential is revoked/expired — it won't
-			// recover by continuing either, and every later step (the revoke included)
-			// hits the same backend. Fail fast and point at sign-in rather than march
-			// the user through the confirm only to fail at revoke. Mirrors the --wait
-			// poll loop's auth handling in client.go.
 			var ae *api.APIError
 			if errors.As(lerr, &ae) && (ae.StatusCode == http.StatusUnauthorized || ae.StatusCode == http.StatusForbidden) {
 				return &exitError{code: exitFailure, err: errors.New(
 					"tracebloc rejected your credentials — run `tracebloc login`, then retry `tracebloc delete`")}
 			}
-			// Other errors (5xx/429/network) are transient: warn and continue, since
-			// the teardown is the real gate.
-			p.Hintf("Couldn't check whether this client is still online (%v) — continuing; pass --force to skip this check.", lerr)
-		} else if !found {
+			p.Hintf("Couldn't check for active training runs (%v) — continuing; the confirmation below still guards you.", lerr)
+		case pc == nil:
 			// The stored id isn't among this account's clients — likely a stale
-			// pointer or the wrong account/env. Don't silently pass the guard: warn,
-			// then continue (the revoke below will 403/404 if it isn't really ours).
-			p.Hintf("This client isn't in the signed-in account's client list — continuing; if that's unexpected, check you're logged into the right account/env.")
-		} else if st == clientStatusOnline {
+			// pointer or the wrong account. Warn, then continue (the revoke below
+			// will 403/404 if it isn't really ours).
+			p.Hintf("This secure environment isn't in the signed-in account — continuing; if that's unexpected, check you're logged into the right account.")
+		case pc.NumRunningExperiments > 0:
 			return &exitError{code: exitFailure, err: fmt.Errorf(
-				"client %q is still online (tracebloc reports it running) — stop its training jobs first, "+
-					"or pass --force to offboard anyway", name)}
+				"your secure environment %q has %d training run(s) active — offboarding would stop them. "+
+					"Wait for them to finish or cancel them, then run `tb delete` again — or `tb delete --force` to stop them and offboard now",
+				name, pc.NumRunningExperiments)}
 		}
 	}
 
+	// The three-way scope PREVIEW — shown after the work-guard and before the
+	// typed-name confirm, so the user sees exactly what WILL be removed, kept, and
+	// left alone before committing to anything (RFC-0001 §7.10).
+	renderOffboardSummary(p, name, o.keepData)
+
 	// Typed-client-name confirmation (not [y/N]): the local data wipe is
 	// irreversible, so require the user to type the client's name. --yes skips it
 	// for automation.
@@ -233,7 +230,7 @@ func runDelete(ctx context.Context, p *ui.Printer, pr prompter, o deleteOpts) er
 		p.Hintf("Couldn't revoke the credential server-side (%v) — continuing with local teardown. "+
 			"The credential may still be live on tracebloc; revoke it from the dashboard if needed.", rerr)
 	} else {
-		p.Successf("Revoked this machine's credential (client %q kept on tracebloc as a record).", name)
+		p.Successf("Revoked this machine's credential — your secure environment %q stays on tracebloc as a record.", name)
 	}
 
 	// The teardown steps below are best-effort. (The credential is revoked when the
@@ -266,7 +263,7 @@ func runDelete(ctx context.Context, p *ui.Printer, pr prompter, o deleteOpts) er
 			p.Warnf("Chart uninstall reported: %v", uerr)
 			degraded = true
 		} else {
-			p.Successf("Uninstalled the Helm release %s.", ns)
+			p.Successf("Uninstalled tracebloc.")
 		}
 	} else {
 		// No cached namespace (a pre-cache config, or none passed) — we can't name
@@ -282,15 +279,22 @@ func runDelete(ctx context.Context, p *ui.Printer, pr prompter, o deleteOpts) er
 		p.Warnf("Cluster teardown reported: %v", terr)
 		degraded = true
 	} else {
-		p.Successf("Deleted the local cluster %q.", nodeboot.ClusterName)
+		p.Successf("Removed the local environment.")
 	}
 
 	// 4. Reclaim the tracebloc container images (SCOPED to ghcr.io/tracebloc/*,
 	//    never a blanket prune — best-effort).
 	if perr := pruneImages(ctx); perr != nil {
-		p.Warnf("Image reclaim reported: %v", perr)
+		// Pure disk cleanup — a failure here changes nothing about the offboard
+		// (it's excluded from `degraded`). Keep it a quiet, plain note; don't leak
+		// the raw docker/daemon error at the user.
+		// `docker image prune` would NOT help here: it only removes dangling
+		// (untagged) images, and these are tagged ghcr.io/tracebloc/* refs. Point
+		// at the scoped removal that actually matches the failure mode — the same
+		// reference PruneImages targets — never a blanket prune.
+		p.Infof("Some tracebloc images couldn't be reclaimed (harmless) — remove them later with `docker rmi $(docker images ghcr.io/tracebloc/* --format '{{.Repository}}:{{.Tag}}')`.")
 	} else {
-		p.Successf("Reclaimed the tracebloc container images.")
+		p.Successf("Reclaimed tracebloc's downloaded images.")
 	}
 
 	// 5. Spare or wipe ~/.tracebloc. The active-client pointer was already cleared
@@ -339,32 +343,27 @@ func runDelete(ctx context.Context, p *ui.Printer, pr prompter, o deleteOpts) er
 	return nil
 }
 
-// renderOffboardSummary prints the removed / retained / left three-way summary
-// (RFC-0001 §7.10 mock) shown before the typed confirm.
-func renderOffboardSummary(p *ui.Printer, name, ns string, keepData bool) {
-	p.Banner("tracebloc", "offboard this machine")
-
-	p.Section("Removed from this machine")
-	p.Infof("This machine's credential (revoked — tracebloc can no longer see it)")
-	if ns != "" {
-		p.Infof("The Helm release %q and the local cluster %q", ns, nodeboot.ClusterName)
-	} else {
-		p.Infof("The Helm release and the local cluster %q", nodeboot.ClusterName)
-	}
-	p.Infof("The tracebloc container images (ghcr.io/tracebloc/*)")
+// renderOffboardSummary prints the three-way PREVIEW — what will be removed,
+// kept, and left alone — shown after the work-guard and before the typed confirm
+// (RFC-0001 §7.10 mock). Plain language only; the Kubernetes/Helm/registry
+// specifics stay out of the user's way.
+func renderOffboardSummary(p *ui.Printer, name string, keepData bool) {
+	p.Section("This will remove")
+	p.Infof("This machine's credential — so tracebloc can no longer reach it")
+	p.Infof("Your secure environment %q and everything it runs on this machine", name)
+	p.Infof("tracebloc's downloaded images")
 	if keepData {
-		p.Infof("The CLI itself (local data + config kept: --keep-data)")
+		p.Infof("The tb CLI (your local data & config are kept — --keep-data)")
 	} else {
-		p.Infof("Local data + config (~/.tracebloc) and the CLI itself — irreversible")
+		p.Infof("Your local data & config (~/.tracebloc) and the tb CLI — can't be undone")
 	}
 
-	p.Section("Kept on tracebloc, as a record")
+	p.Section("Kept on tracebloc")
 	p.Infof("Your use cases and the models trained here")
-	p.Infof("The datasets' catalog entries (marked unavailable, not deleted)")
+	p.Infof("Your dataset records (marked unavailable, not deleted)")
 
-	p.Section("Left in place (system-wide)")
-	p.Infof("Docker, kubectl, k3d, helm — remove yourself if unused")
-	p.Infof("(On a GPU box: NVIDIA drivers + container toolkit)")
+	p.Section("Left alone")
+	p.Infof("Docker and related tools — remove them yourself if you no longer need them")
 }
 
 // removeHostDataDir deletes the tracebloc host data directory (~/.tracebloc, or

internal/cli/delete_test.go

@@ -554,21 +554,23 @@ func TestDelete_KubeconfigContext_ReachHelm(t *testing.T) {
 	}
 }
 
-// (d) A running/online client → refuse unless --force.
+// (d) A client with ACTIVE training runs → refuse unless --force. (Being merely
+// "online" no longer blocks — a healthy environment always heartbeats.)
 func TestDelete_RunningJob_RefusesUnlessForce(t *testing.T) {
 	newHandler := func() http.HandlerFunc {
 		return func(w http.ResponseWriter, r *http.Request) {
 			switch {
 			case r.Method == http.MethodGet && r.URL.Path == "/edge-device/5/":
-				// status 1 = online (a running client).
-				_, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":1}`))
+				// 2 active training runs — the work-guard blocks on this, NOT on
+				// status==online (which a healthy client always reports).
+				_, _ = w.Write([]byte(`{"id":5,"first_name":"gpu-box-01","namespace":"gpu-box-01","status":1,"num_running_experiments":2}`))
 			case r.Method == http.MethodPost && strings.Contains(r.URL.Path, "/revoke"):
 				w.WriteHeader(http.StatusOK)
 			}
 		}
 	}
 
-	t.Run("online without --force refuses, no revoke", func(t *testing.T) {
+	t.Run("active training without --force refuses, no revoke", func(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") {
@@ -582,18 +584,24 @@ func TestDelete_RunningJob_RefusesUnlessForce(t *testing.T) {
 
 		var out bytes.Buffer
 		err := runDelete(context.Background(), ui.New(&out), typedNamePrompter{reply: "gpu-box-01"}, deleteOpts{})
-		if err == nil || !strings.Contains(err.Error(), "still online") {
-			t.Fatalf("want online refusal, got %v", err)
+		if err == nil || !strings.Contains(err.Error(), "training run") {
+			t.Fatalf("want an active-training refusal, got %v", err)
 		}
 		if revoked {
-			t.Error("revoke must NOT run when the online guard refuses")
+			t.Error("revoke must NOT run when the work-guard refuses")
 		}
 		if len(fn.calls) != 0 {
 			t.Errorf("no teardown on refusal, got: %v", fn.calls)
 		}
+		// The removal PREVIEW must NOT have printed — the work-guard runs first, so
+		// a blocked offboard never shows a "here's what I'll remove" manifest (the
+		// bug: the manifest used to print before the guard, then error out).
+		if strings.Contains(out.String(), "This will remove") {
+			t.Errorf("preview must not print when the work-guard blocks:\n%s", out.String())
+		}
 	})
 
-	t.Run("--force offboards an online client", func(t *testing.T) {
+	t.Run("--force offboards despite active training", func(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") {
@@ -610,7 +618,7 @@ func TestDelete_RunningJob_RefusesUnlessForce(t *testing.T) {
 			t.Fatalf("offboard --force: %v", err)
 		}
 		if !revoked {
-			t.Error("--force should proceed to revoke an online client")
+			t.Error("--force should proceed to revoke despite active training")
 		}
 	})
 }
@@ -635,13 +643,15 @@ func TestDelete_ShowsRetainedAndLeftCopy(t *testing.T) {
 	}
 	s := out.String()
 	for _, want := range []string{
-		"Kept on tracebloc, as a record",
+		"This will remove",
+		"Your secure environment",
+		"Kept on tracebloc",
 		"Your use cases and the models trained here",
-		"Left in place (system-wide)",
-		"Docker, kubectl, k3d, helm — remove yourself if unused",
+		"Left alone",
+		"Docker and related tools",
 	} {
 		if !strings.Contains(s, want) {
-			t.Errorf("output missing RETAINED/LEFT copy %q:\n%s", want, s)
+			t.Errorf("output missing preview copy %q:\n%s", want, s)
 		}
 	}
 }

internal/nodeboot/nodeboot.go

@@ -102,8 +102,10 @@ func UninstallChart(ctx context.Context, namespace, kubeconfig, kubeContext stri
 	return err
 }
 
-// PruneImages reclaims the tracebloc container images pulled during install —
-// `docker images --filter=reference="ghcr.io/tracebloc/*" -q | docker rmi`. It is
+// PruneImages reclaims the tracebloc container images pulled during install by
+// reference — `docker images --filter=reference="ghcr.io/tracebloc/*" --format
+// {{.Repository}}:{{.Tag}} | docker rmi` (by repo:tag, NOT image ID: a shared ID
+// refuses `docker rmi <id>` — see the body). It is
 // SCOPED to the tracebloc image reference and best-effort by design (RFC-0001
 // §7.10): reclaiming disk is a nice-to-have on offboard, not a hard step, so a
 // docker failure or an image still in use by a container is not fatal. It is
@@ -112,23 +114,36 @@ func UninstallChart(ctx context.Context, namespace, kubeconfig, kubeContext stri
 //
 // No matching images (nothing to reclaim) is a clean no-op, not an error.
 func PruneImages(ctx context.Context) error {
-	out, err := run(ctx, "docker", "images", "--filter=reference="+imageReference, "-q")
+	// List by REFERENCE (repo:tag), not image ID (-q). An image ID shared across
+	// multiple repositories refuses `docker rmi <id>` ("must be forced — image is
+	// referenced in multiple repositories"). Removing by reference untags only OUR
+	// tracebloc references — Docker deletes the underlying image only when nothing
+	// else references it — so we never need a force that could evict an image a
+	// non-tracebloc workload shares (cli delete image-reclaim bug).
+	out, err := run(ctx, "docker", "images", "--filter=reference="+imageReference,
+		"--format", "{{.Repository}}:{{.Tag}}")
 	if err != nil {
 		return err
 	}
-	// De-duplicate the image IDs: a single image tagged multiple times
-	// (ghcr.io/tracebloc/x:1 and :latest) lists its ID once per tag, and passing
-	// the same ID twice to `docker rmi` makes the second reference error.
-	ids := dedupeLines(out)
-	if len(ids) == 0 {
+	// Dedupe and drop dangling refs: an image that lost its tag prints "<none>"
+	// for the repo or tag, which can't be removed by reference (`docker image
+	// prune` reclaims those). Best-effort, so skipping them is fine.
+	var refs []string
+	for _, ref := range dedupeLines(out) {
+		if strings.Contains(ref, "<none>") {
+			continue
+		}
+		refs = append(refs, ref)
+	}
+	if len(refs) == 0 {
 		return nil // nothing tracebloc-owned to reclaim
 	}
-	_, err = run(ctx, "docker", append([]string{"rmi"}, ids...)...)
+	_, err = run(ctx, "docker", append([]string{"rmi"}, refs...)...)
 	return err
 }
 
-// dedupeLines splits combined `docker images -q` output into unique, non-empty,
-// order-preserving lines (image IDs).
+// dedupeLines splits combined `docker images` output into unique, non-empty,
+// order-preserving lines (image IDs or repo:tag references).
 func dedupeLines(out string) []string {
 	seen := map[string]struct{}{}
 	var ids []string