From f7883616fd97123de6bfdb7509c3ee25112f16d1 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Sun, 12 Jul 2026 08:59:14 +0200 Subject: [PATCH 1/7] feat(cli): status-aware `tracebloc` home screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rework the bare-`tracebloc` home screen from a stateless command list into a status-aware landing screen that opens with where you actually stand — signed in? is this machine's secure environment live? — then the commands. Two separate, never-fused axes: sign-in (you) and the secure environment (the machine), because the client heartbeats with its own credential. A green "· Online" prints ONLY when the environment is live locally (chart present + jobs-manager Ready) AND positively confirmed heartbeating to tracebloc; a heartbeat we can't confirm degrades to "· running", never a false green. States: not-signed-in / online / running-not-heard-from / offline / no-environment. Detection is best-effort and bounded so bare `tracebloc` (run constantly, previously zero-I/O) stays snappy: probes run concurrently, each with its own short timeout, all capped by a ~1.5s overall budget; any error/timeout degrades to the softer state and the screen still renders. Logged out does zero cluster/backend I/O. An unreachable cluster caps at the probe timeout (~1.2s) instead of the OS default. Reuses existing seams: config sign-in + cached email, the data commands' namespace binding + release discovery, delete.go's `tb`-alias ownership check, and node allocatable for the compute parenthetical (no `resources` command on develop). `` echoes the invoked binary name (tb/tracebloc); the doctor path shown is the real `cluster doctor`. HEARTBEAT CREDENTIAL: read via the signed-in user token (ListClients), the only path that exists — there's no login-free in-cluster-client credential path, so the environment line is confirmable only while signed in (documented in realHeartbeat). Adds ui.CheckLine/CrossLine/WarnLine for the locked ✓/✗/⚠ status glyphs. Tests are table-driven and cluster-free: every state, the honesty fallback (can't-confirm → running, not Online), and the timeout/degrade path (a slow probe still renders fast with the softer state). Co-Authored-By: Claude Opus 4.8 --- internal/cli/home.go | 589 ++++++++++++++++++++++++++++++++++++++ internal/cli/home_test.go | 414 +++++++++++++++++++++++++++ internal/cli/root.go | 20 +- internal/cli/root_test.go | 15 +- internal/ui/ui.go | 24 ++ 5 files changed, 1043 insertions(+), 19 deletions(-) create mode 100644 internal/cli/home.go create mode 100644 internal/cli/home_test.go diff --git a/internal/cli/home.go b/internal/cli/home.go new file mode 100644 index 0000000..d0799d4 --- /dev/null +++ b/internal/cli/home.go @@ -0,0 +1,589 @@ +package cli + +// The status-aware `tracebloc` home screen. A bare `tracebloc` (no subcommand) +// used to print a stateless command list; now it opens with where you actually +// stand — are you signed in, and is this machine's secure environment live — +// then the commands. See root.go's RunE for the wiring. +// +// Design constraints that shape this file: +// +// - Honesty. Sign-in (you) and the secure environment (the machine) are two +// SEPARATE lines and never fused: the client heartbeats with its own +// credential, so the environment can be Online while you're logged out. A +// green "· Online" is printed ONLY when the environment is live locally AND +// positively confirmed heartbeating to tracebloc — a heartbeat we couldn't +// confirm degrades to "· running", never a false green. +// +// - Speed + robustness. Bare `tracebloc` runs constantly; the old screen did +// zero I/O. Detection here is best-effort and bounded: every probe is +// concurrent, each carries its own short timeout, and the whole thing is +// capped by homeDetectBudget so an unreachable cluster/backend still renders +// the right ⚠/✗ state fast. A probe that errors or times out degrades to the +// softer state; nothing here is ever fatal. +// +// Structure: resolveHomeModel runs the probes and returns a pure homeModel; +// renderHome turns that model into output with no I/O. The split keeps rendering +// deterministic and table-testable, and lets the detection be driven by fakes. + +import ( + "context" + "errors" + "fmt" + "math" + "os" + "path/filepath" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + + "github.com/tracebloc/cli/internal/cluster" + "github.com/tracebloc/cli/internal/config" + "github.com/tracebloc/cli/internal/ui" +) + +// doctorPath is the REAL command that diagnoses the environment on develop +// (`cluster doctor`, wired in cluster.go). Kept as a const so the home screen +// never invents a top-level `doctor` — a rename is a separate follow-up. +const doctorPath = "cluster doctor" + +// Invoked binary names we recognize. `` in the examples echoes how the user +// actually called the CLI so copy-paste matches their muscle memory. +const ( + binTracebloc = "tracebloc" + binTB = "tb" +) + +// Detection is bounded so bare `tracebloc` stays snappy even when the cluster or +// backend is unreachable. homeProbeTimeout caps each individual probe; +// homeDetectBudget caps the whole detection regardless of any single probe. +const ( + homeDetectBudget = 1500 * time.Millisecond + homeProbeTimeout = 1200 * time.Millisecond +) + +// gpuResource is the allocatable key NVIDIA's device plugin advertises; summed +// for the compute parenthetical. +const gpuResource = corev1.ResourceName("nvidia.com/gpu") + +// homeState is which of the locked screens to render. +type homeState int + +const ( + homeNotSignedIn homeState = iota // you're not signed in — minimal, sign in first + homeOnline // signed in, environment live AND heartbeating + homeRunning // signed in, environment live locally but heartbeat unconfirmed + homeOffline // signed in, environment set up here but unreachable/stopped + homeNoEnv // signed in, no environment on this machine +) + +// computeInfo is the machine's total schedulable capacity, summed across Ready +// nodes' allocatable. Shown only in the Online state; best-effort (omitted, not +// errored, when it can't be read). +type computeInfo struct { + CPU int + MemGiB int + GPU int +} + +// homeModel is the fully-resolved, I/O-free input to renderHome. Building it +// (resolveHomeModel) does the probing; rendering it is pure. +type homeModel struct { + state homeState + email string // signed-in account, "" when unknown + envName string // secure environment name, when known + compute computeInfo + hasCompute bool + inv string // invoked binary name: "tb" or "tracebloc" + tbTip bool // show the "type tb instead" tip + fullMenu bool // render the full command menu (an environment exists here) +} + +// ── Detection ── + +// heartbeatState is tracebloc's view of the client, read via the signed-in +// user's token (see realHeartbeat). "unknown" is the honest default whenever we +// can't positively confirm — logged out, backend unreachable, probe timed out — +// and it never renders as Online. +type heartbeatState int + +const ( + beatUnknown heartbeatState = iota + beatOnline + beatNotOnline +) + +// localState is what a bounded cluster probe found on THIS machine. +type localState int + +const ( + localUnreachable localState = iota // couldn't reach a cluster (no kubeconfig / connect failed / timeout) + localNoRelease // cluster reachable, but no tracebloc release installed + localDegraded // release present, but its core workload isn't Ready + localLive // release present and its core workload is Ready +) + +// envProbe is the cluster probe's result: what's here, its name, and (when live) +// the machine's compute capacity. +type envProbe struct { + local localState + name string + compute computeInfo + hasCompute bool +} + +// homeDeps are the detection seams. defaultHomeDeps wires the real +// implementations; tests inject fakes to drive every state, the honesty +// fallback, and the timeout/degrade path without touching a real cluster. +type homeDeps struct { + signIn func() (signedIn bool, email string) + probeEnv func(ctx context.Context) envProbe + probeBeat func(ctx context.Context) heartbeatState + invoked func() string + tbAvailable func() bool + budget time.Duration +} + +func defaultHomeDeps() homeDeps { + return homeDeps{ + signIn: realSignIn, + probeEnv: realProbeEnv, + probeBeat: realHeartbeat, + invoked: invokedName, + tbAvailable: tbAliasAvailable, + budget: homeDetectBudget, + } +} + +// renderHomeScreen is the bare-`tracebloc` entry point: detect (bounded, +// best-effort) then render. Called from root.RunE. +func renderHomeScreen(ctx context.Context, p *ui.Printer) { + renderHome(p, resolveHomeModel(ctx, defaultHomeDeps())) +} + +// resolveHomeModel runs the (best-effort, bounded) probes and assembles the +// homeModel. It never blocks longer than the budget and never returns an error: +// a slow or failing probe degrades to the softer state and the screen still +// renders. +func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { + inv := d.invoked() + + // Sign-in is a local config read (instant). When logged out we render the + // minimal "sign in first" screen and skip all cluster/backend I/O entirely — + // the common just-installed case returns immediately, and there's no + // login-free credential path to honestly report the environment anyway. + signedIn, email := d.signIn() + if !signedIn { + return homeModel{state: homeNotSignedIn, inv: inv} + } + + // Signed in: probe the environment and the heartbeat concurrently, both under + // one overall budget. Buffered result channels (cap 1) mean an abandoned probe + // never blocks or leaks — if it finishes after we've given up on the budget, + // its send lands in the buffer and its goroutine exits. + bctx, cancel := context.WithTimeout(ctx, d.budget) + defer cancel() + + envCh := make(chan envProbe, 1) + beatCh := make(chan heartbeatState, 1) + go func() { envCh <- d.probeEnv(bctx) }() + go func() { beatCh <- d.probeBeat(bctx) }() + + env := envProbe{local: localUnreachable} // default if the probe never reports + beat := beatUnknown // default: unconfirmed, never Online + for got := 0; got < 2; { + select { + case env = <-envCh: + envCh = nil // disable this case; a nil channel blocks forever in select + got++ + case beat = <-beatCh: + beatCh = nil + got++ + case <-bctx.Done(): + got = 2 // budget spent — render with whatever arrived, softly + } + } + + m := homeModel{ + email: email, + inv: inv, + envName: env.name, + compute: env.compute, + hasCompute: env.hasCompute, + } + + switch env.local { + case localLive: + // Online demands BOTH live-locally AND a positively-confirmed heartbeat. + // Anything short of beatOnline (not-online, or couldn't-confirm) is the + // honest "· running" state, never a green Online. + if beat == beatOnline { + m.state = homeOnline + } else { + m.state = homeRunning + } + m.fullMenu = true + case localDegraded: + // Workload present but not Ready → running, not Online. + m.state = homeRunning + m.fullMenu = true + case localNoRelease: + // Cluster reachable, nothing installed: there's genuinely no environment + // here right now, even if config remembers a past one. + m.state = homeNoEnv + m.envName = "" + case localUnreachable: + // Couldn't reach a cluster. If this machine was provisioned (the probe + // surfaced a remembered name) the environment exists but is unreachable → + // offline; otherwise there's simply nothing here. + if env.name != "" { + m.state = homeOffline + m.fullMenu = true + } else { + m.state = homeNoEnv + } + } + + // The tb tip only makes sense when we're showing a command menu and the user + // typed the long name while a real `tb` alias is installed. Probe for the + // alias lazily — only when it could actually be shown. + if m.fullMenu && inv == binTracebloc { + m.tbTip = d.tbAvailable() + } + return m +} + +// realSignIn reports the signed-in state + cached email from local config — no +// network. The cached identity is enough for the display; token validity, when +// it matters, surfaces through the heartbeat probe (which exercises the token). +func realSignIn() (bool, string) { + cfg, err := config.Load() + if err != nil || !cfg.SignedIn() { + return false, "" + } + return true, cfg.Current().Email +} + +// realProbeEnv is the bounded cluster probe. It reuses the exact namespace +// binding + discovery the data/cluster commands use, so the home screen reports +// the very environment those commands would target. Best-effort throughout: any +// failure degrades to unreachable/no-release, never an error. +func realProbeEnv(ctx context.Context) envProbe { + ctx, cancel := context.WithTimeout(ctx, homeProbeTimeout) + defer cancel() + + // The name to fall back on when the cluster can't be reached but this machine + // was provisioned — cached at `client create` time, so it needs no network. + fallbackName := "" + if cfg, err := config.Load(); err == nil { + fallbackName = cfg.Current().ActiveClientName + } + + opts := cluster.KubeconfigOptions{} + binding := bindActiveClientNamespace(&opts) + resolved, err := cluster.Load(opts) + if err != nil { + return envProbe{local: localUnreachable, name: fallbackName} + } + // Bound every API call so an unreachable API server can't hang the home + // screen (mirrors cluster.ClusterID's time-boxed best-effort read). + resolved.RestConfig.Timeout = homeProbeTimeout + cs, err := cluster.NewClientset(resolved) + if err != nil { + return envProbe{local: localUnreachable, name: fallbackName} + } + + release, nsUsed, err := discoverRelease(ctx, nil, cs, resolved.Namespace, binding.allowScan()) + if err != nil { + if errors.Is(err, cluster.ErrNoParentRelease) { + return envProbe{local: localNoRelease} + } + // A list/RBAC/connect failure: we couldn't confirm what's here. Treat it + // as unreachable (→ offline if provisioned, else no-env). + return envProbe{local: localUnreachable, name: fallbackName} + } + + name := release.ReleaseName + if name == "" { + name = fallbackName + } + ep := envProbe{name: name} + if jobsManagerReady(ctx, cs, nsUsed, release) { + ep.local = localLive + } else { + ep.local = localDegraded + } + // Compute is only surfaced on the Online line, and only worth reading when the + // environment is actually up. + if ep.local == localLive { + if c, ok := machineCapacity(ctx, cs); ok { + ep.compute, ep.hasCompute = c, true + } + } + return ep +} + +// realHeartbeat reports tracebloc's view of this machine's client — the honest +// "is it heartbeating" signal. +// +// CREDENTIAL NOTE: this reads the status via the signed-in USER token +// (lookupClientStatus → client.ListClients, keyed on the active client id). The +// CLI has no login-free path that authenticates as the in-cluster client itself +// (the chart's CLIENT_ID secret is readable, but nothing here presents it to the +// backend as a credential). So the heartbeat can only be confirmed while signed +// in; logged out we don't reach here (the not-signed-in screen omits the +// environment line). Any failure — no active client, backend unreachable, +// timeout — is beatUnknown, which never renders as Online. +func realHeartbeat(ctx context.Context) heartbeatState { + ctx, cancel := context.WithTimeout(ctx, homeProbeTimeout) + defer cancel() + + client, cfg, err := authedClient() + if err != nil { + return beatUnknown + } + active := cfg.Current().ActiveClientID + if active == "" { + return beatUnknown + } + st, found, err := lookupClientStatus(ctx, client, active) + if err != nil || !found { + return beatUnknown + } + if st == clientStatusOnline { + return beatOnline + } + return beatNotOnline +} + +// jobsManagerReady reports whether the release's core workload (jobs-manager) has +// a Ready replica — the single best local "the environment is actually up" +// signal. Best-effort: an unreadable deployment reads as not-ready. +func jobsManagerReady(ctx context.Context, cs kubernetes.Interface, ns string, release *cluster.ParentRelease) bool { + var names []string + if release != nil && release.ReleaseName != "" { + names = append(names, release.ReleaseName+"-jobs-manager") + } + names = append(names, "jobs-manager") // older unprefixed charts + for _, n := range names { + if d, err := cs.AppsV1().Deployments(ns).Get(ctx, n, metav1.GetOptions{}); err == nil { + return d.Status.ReadyReplicas >= 1 + } + } + return false +} + +// machineCapacity lists nodes and sums the Ready ones' allocatable into the +// machine's total schedulable capacity. Best-effort: a list failure or an +// all-NotReady cluster returns ok=false so the caller omits the parenthetical. +func machineCapacity(ctx context.Context, cs kubernetes.Interface) (computeInfo, bool) { + nodes, err := cs.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + if err != nil { + return computeInfo{}, false + } + return sumCapacity(nodes.Items) +} + +// sumCapacity is the pure summation behind machineCapacity — split out so the +// rounding + GPU logic is unit-testable without a clientset. +func sumCapacity(nodes []corev1.Node) (computeInfo, bool) { + var cpuMilli, memBytes, gpu int64 + ready := 0 + for i := range nodes { + n := nodes[i] + if !nodeReady(n) { + continue + } + ready++ + alloc := n.Status.Allocatable + cpuMilli += alloc.Cpu().MilliValue() + memBytes += alloc.Memory().Value() + if q, ok := alloc[gpuResource]; ok { + gpu += q.Value() + } + } + if ready == 0 { + return computeInfo{}, false + } + return computeInfo{ + CPU: int(math.Round(float64(cpuMilli) / 1000)), + MemGiB: int(math.Round(float64(memBytes) / (1 << 30))), + GPU: int(gpu), + }, true +} + +// nodeReady reports whether a node's Ready condition is True (mirrors +// doctor.nodeReady, which is unexported). +func nodeReady(n corev1.Node) bool { + for _, c := range n.Status.Conditions { + if c.Type == corev1.NodeReady { + return c.Status == corev1.ConditionTrue + } + } + return false +} + +// invokedName is the binary name the user actually typed, so examples echo it. +func invokedName() string { return sanitizeInvoked(os.Args[0]) } + +// sanitizeInvoked maps argv[0] to "tb" or "tracebloc": "tb" only when the user +// invoked the alias, otherwise "tracebloc" (covering odd argv[0]s like a `go run` +// temp name so the examples stay copy-pasteable). +func sanitizeInvoked(argv0 string) string { + base := strings.TrimSuffix(filepath.Base(argv0), ".exe") + if base == binTB { + return binTB + } + return binTracebloc +} + +// tbAliasAvailable reports whether a real tracebloc-owned `tb` alias sits next to +// this binary (the installer symlinks it there, cli#142). Reuses delete.go's +// aliasStatus so "is it ours" is judged exactly as offboarding judges it — we +// only advertise `tb` when it genuinely points at this CLI. +func tbAliasAvailable() bool { + exe, err := osExecutable() + if err != nil { + return false + } + tb := filepath.Join(filepath.Dir(exe), binTB) + if tb == exe { + return false + } + _, ours := aliasStatus(tb, exe) + return ours +} + +// ── Rendering (pure) ── + +// menuBucket is a titled group of command rows ({suffix, description}). +type menuBucket struct { + title string + rows [][2]string +} + +// renderHome writes the status-aware home screen. Pure: no I/O, no clock — it's +// a deterministic function of the model, which is what makes every state +// table-testable. +func renderHome(p *ui.Printer, m homeModel) { + p.Newline() + + // Greeting: "your secure environment" only when one actually exists here. + switch m.state { + case homeOnline, homeRunning, homeOffline: + p.Para("Welcome to your secure environment for AI 👋") + default: + p.Para("Welcome to tracebloc — your secure environment for AI 👋") + } + + // Sign-in axis (you). + if m.state == homeNotSignedIn { + p.CrossLine("Not signed in yet.") + } else if m.email != "" { + p.CheckLine("Signed in as %s", m.email) + } else { + p.CheckLine("Signed in") + } + + // Secure-environment axis (the machine) — a separate line, never fused with + // sign-in. + switch m.state { + case homeOnline: + p.CheckLine("Secure environment %q · Online%s", m.envName, computeParen(m)) + case homeRunning: + p.WarnLine("Secure environment %q · running, but tracebloc hasn't heard from it — run %s %s", + m.envName, m.inv, doctorPath) + case homeOffline: + p.CrossLine("Secure environment %q · offline — run %s %s", m.envName, m.inv, doctorPath) + case homeNoEnv: + p.WarnLine("No secure environment on this machine yet — run the installer to set one up.") + } + + renderBuckets(p, m.inv, menuFor(m.state)) + + // Closing. + p.Newline() + if m.tbTip { + p.Hintf("tip · type tb instead of tracebloc — either works") + } + if m.fullMenu { + p.Hintf("Add --help to any command for the flags.") + } + p.Newline() + p.Hintf("──────────────────────────────") + p.Para("love from tracebloc 💚") +} + +// menuFor is the command buckets to show per state. Not-signed-in and no-env +// stay deliberately lean (one actionable step); the environment states get the +// full menu. +func menuFor(state homeState) []menuBucket { + switch state { + case homeNotSignedIn: + return []menuBucket{{ + title: "Start here", + rows: [][2]string{{"login", "sign in to tracebloc"}}, + }} + case homeNoEnv: + // The actionable next step is the installer (external); still point at + // doctor to diagnose a half-installed environment. + return []menuBucket{{ + title: "Your secure environment", + rows: [][2]string{{doctorPath, "check the connection & diagnose issues"}}, + }} + default: // online / running / offline — an environment exists here + return []menuBucket{ + { + title: "Work with your data", + rows: [][2]string{ + {"data ingest", "load a dataset into your secure environment"}, + {"data list", "list your datasets"}, + {"data delete", "remove a dataset"}, + }, + }, + { + title: "Your secure environment", + rows: [][2]string{{doctorPath, "check the connection & diagnose issues"}}, + }, + { + title: "Manage", + rows: [][2]string{{"delete", "remove tracebloc from this machine"}}, + }, + } + } +} + +// renderBuckets prints each bucket as a Section + "· " +// rows, with the command column padded to a single width across ALL buckets so +// the descriptions line up in one column. +func renderBuckets(p *ui.Printer, inv string, buckets []menuBucket) { + width := 0 + for _, b := range buckets { + for _, r := range b.rows { + if l := len(inv) + 1 + len(r[0]); l > width { + width = l + } + } + } + for _, b := range buckets { + p.Section(b.title) + for _, r := range b.rows { + p.Infof("%-*s %s", width, inv+" "+r[0], r[1]) + } + } +} + +// computeParen formats the compute parenthetical, e.g. " (8 CPU · 32 GiB · 1 GPU)". +// GPU is dropped when 0; the whole thing is empty when capacity couldn't be read. +func computeParen(m homeModel) string { + if !m.hasCompute { + return "" + } + s := fmt.Sprintf(" (%d CPU · %d GiB", m.compute.CPU, m.compute.MemGiB) + if m.compute.GPU > 0 { + s += fmt.Sprintf(" · %d GPU", m.compute.GPU) + } + return s + ")" +} diff --git a/internal/cli/home_test.go b/internal/cli/home_test.go new file mode 100644 index 0000000..8ae3c43 --- /dev/null +++ b/internal/cli/home_test.go @@ -0,0 +1,414 @@ +package cli + +import ( + "bytes" + "context" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/tracebloc/cli/internal/ui" +) + +// renderToString renders a model with color off (a buffer isn't a TTY), so the +// assertions match on the literal copy. +func renderToString(m homeModel) string { + var buf bytes.Buffer + renderHome(ui.New(&buf), m) + return buf.String() +} + +// TestRenderHome_States pins the locked copy for every state: what MUST appear, +// and — just as important for honesty — what must NOT. The absent[] column is +// the mutation guard: e.g. a "running" screen that ever printed "· Online", or a +// signed-out screen that leaked the data commands, is a regression these catch. +func TestRenderHome_States(t *testing.T) { + cases := []struct { + name string + model homeModel + present []string + absent []string + }{ + { + name: "online with full compute", + model: homeModel{ + state: homeOnline, email: "alice@acme.io", envName: "acme-01", + compute: computeInfo{CPU: 8, MemGiB: 32, GPU: 1}, hasCompute: true, + inv: binTracebloc, fullMenu: true, + }, + present: []string{ + "Welcome to your secure environment for AI", + "Signed in as alice@acme.io", + `Secure environment "acme-01" · Online (8 CPU · 32 GiB · 1 GPU)`, + "tracebloc data ingest", "tracebloc data list", "tracebloc data delete", + "tracebloc cluster doctor", "tracebloc delete", + "Add --help to any command for the flags.", + "love from tracebloc", + }, + // The two axes stay separate + honest: no "running"/"offline"/signed-out + // language on a healthy online screen. + absent: []string{"running, but", "offline", "Not signed in", "No secure environment"}, + }, + { + name: "online without GPU omits the GPU dimension", + model: homeModel{ + state: homeOnline, email: "a@b.io", envName: "n", + compute: computeInfo{CPU: 4, MemGiB: 16, GPU: 0}, hasCompute: true, + inv: binTracebloc, fullMenu: true, + }, + present: []string{"· Online (4 CPU · 16 GiB)"}, + absent: []string{"GPU"}, + }, + { + name: "online without readable compute omits the parenthetical", + model: homeModel{ + state: homeOnline, email: "a@b.io", envName: "n", + hasCompute: false, inv: binTracebloc, fullMenu: true, + }, + present: []string{`Secure environment "n" · Online`}, + absent: []string{"CPU", "GiB", "("}, + }, + { + name: "running but not heartbeating never claims Online", + model: homeModel{ + state: homeRunning, email: "a@b.io", envName: "acme-01", + inv: binTracebloc, fullMenu: true, + }, + present: []string{ + `Secure environment "acme-01" · running, but tracebloc hasn't heard from it — run tracebloc cluster doctor`, + }, + absent: []string{"· Online"}, + }, + { + name: "offline points at the doctor with the invoked name", + model: homeModel{ + state: homeOffline, email: "a@b.io", envName: "acme-01", + inv: binTB, fullMenu: true, + }, + present: []string{ + `Secure environment "acme-01" · offline — run tb cluster doctor`, + "tb data ingest", // examples echo the invoked name + }, + absent: []string{"· Online", "tracebloc data ingest"}, + }, + { + name: "signed in, no environment nudges the installer", + model: homeModel{state: homeNoEnv, email: "a@b.io", inv: binTracebloc}, + present: []string{ + "Signed in as a@b.io", + "No secure environment on this machine yet — run the installer to set one up.", + "tracebloc cluster doctor", // still offer to diagnose + }, + // Lean: don't parade data commands at a machine with no environment. + absent: []string{"data ingest", "· Online", "Add --help"}, + }, + { + name: "not signed in is minimal", + model: homeModel{state: homeNotSignedIn, inv: binTracebloc}, + present: []string{ + "Welcome to tracebloc — your secure environment for AI", + "Not signed in yet.", + "tracebloc login", + "love from tracebloc", + }, + absent: []string{"data ingest", "Secure environment", "· Online", "Signed in as"}, + }, + { + name: "tb tip shown only when invoked as tracebloc with a real alias", + model: homeModel{ + state: homeOnline, email: "a@b.io", envName: "n", + inv: binTracebloc, fullMenu: true, tbTip: true, + }, + present: []string{"type tb instead of tracebloc"}, + }, + { + name: "no tb tip when it wasn't earned", + model: homeModel{ + state: homeOnline, email: "a@b.io", envName: "n", + inv: binTracebloc, fullMenu: true, tbTip: false, + }, + absent: []string{"type tb instead"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := renderToString(tc.model) + for _, want := range tc.present { + if !strings.Contains(got, want) { + t.Errorf("missing %q in:\n%s", want, got) + } + } + for _, bad := range tc.absent { + if strings.Contains(got, bad) { + t.Errorf("unexpected %q in:\n%s", bad, got) + } + } + }) + } +} + +// baseDeps is a fully-online, signed-in fake; tests override just the field +// under test. +func baseDeps() homeDeps { + return homeDeps{ + signIn: func() (bool, string) { return true, "alice@acme.io" }, + probeEnv: func(context.Context) envProbe { return envProbe{local: localLive, name: "acme-01"} }, + probeBeat: func(context.Context) heartbeatState { return beatOnline }, + invoked: func() string { return binTracebloc }, + tbAvailable: func() bool { return false }, + budget: 2 * time.Second, + } +} + +// TestResolveHomeModel_States drives the state machine through the fakes: every +// combination of sign-in / environment / heartbeat maps to the right state. +func TestResolveHomeModel_States(t *testing.T) { + cases := []struct { + name string + mutate func(*homeDeps) + want homeState + }{ + { + name: "signed out", + mutate: func(d *homeDeps) { d.signIn = func() (bool, string) { return false, "" } }, + want: homeNotSignedIn, + }, + { + name: "live + heartbeating = online", + mutate: func(d *homeDeps) {}, + want: homeOnline, + }, + { + // Honesty: a heartbeat we couldn't confirm must degrade to running, + // NEVER a green Online. Mutation guard — if the code treated unknown as + // online this flips to homeOnline and fails. + name: "live + heartbeat UNKNOWN = running (honesty fallback)", + mutate: func(d *homeDeps) { d.probeBeat = func(context.Context) heartbeatState { return beatUnknown } }, + want: homeRunning, + }, + { + name: "live + heartbeat says NOT online = running", + mutate: func(d *homeDeps) { d.probeBeat = func(context.Context) heartbeatState { return beatNotOnline } }, + want: homeRunning, + }, + { + name: "workload degraded = running", + mutate: func(d *homeDeps) { + d.probeEnv = func(context.Context) envProbe { return envProbe{local: localDegraded, name: "acme-01"} } + }, + want: homeRunning, + }, + { + name: "cluster reachable, no release = no environment", + mutate: func(d *homeDeps) { + d.probeEnv = func(context.Context) envProbe { return envProbe{local: localNoRelease} } + }, + want: homeNoEnv, + }, + { + name: "unreachable but provisioned (name known) = offline", + mutate: func(d *homeDeps) { + d.probeEnv = func(context.Context) envProbe { return envProbe{local: localUnreachable, name: "acme-01"} } + }, + want: homeOffline, + }, + { + name: "unreachable and nothing configured = no environment", + mutate: func(d *homeDeps) { + d.probeEnv = func(context.Context) envProbe { return envProbe{local: localUnreachable} } + }, + want: homeNoEnv, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + d := baseDeps() + tc.mutate(&d) + got := resolveHomeModel(context.Background(), d) + if got.state != tc.want { + t.Fatalf("state = %d, want %d", got.state, tc.want) + } + }) + } +} + +// TestResolveHomeModel_PassesThroughFields checks the model carries email, name, +// and compute from the probes into the render input. +func TestResolveHomeModel_PassesThroughFields(t *testing.T) { + d := baseDeps() + d.probeEnv = func(context.Context) envProbe { + return envProbe{local: localLive, name: "acme-01", compute: computeInfo{CPU: 8, MemGiB: 32, GPU: 2}, hasCompute: true} + } + m := resolveHomeModel(context.Background(), d) + if m.email != "alice@acme.io" || m.envName != "acme-01" { + t.Fatalf("email/name = %q/%q", m.email, m.envName) + } + if !m.hasCompute || m.compute != (computeInfo{CPU: 8, MemGiB: 32, GPU: 2}) { + t.Fatalf("compute = %+v (has=%v)", m.compute, m.hasCompute) + } + if !m.fullMenu { + t.Fatal("online should show the full menu") + } +} + +// TestResolveHomeModel_TbTip: the tip is earned only when invoked as `tracebloc` +// AND a real alias exists — never when invoked as `tb`, and never on a screen +// with no menu. +func TestResolveHomeModel_TbTip(t *testing.T) { + t.Run("earned", func(t *testing.T) { + d := baseDeps() + d.tbAvailable = func() bool { return true } + if !resolveHomeModel(context.Background(), d).tbTip { + t.Fatal("tip should show when invoked as tracebloc with a real alias") + } + }) + t.Run("invoked as tb", func(t *testing.T) { + d := baseDeps() + d.invoked = func() string { return binTB } + d.tbAvailable = func() bool { return true } + if resolveHomeModel(context.Background(), d).tbTip { + t.Fatal("no tip when the user already uses tb") + } + }) + t.Run("signed out never tips", func(t *testing.T) { + d := baseDeps() + d.signIn = func() (bool, string) { return false, "" } + d.tbAvailable = func() bool { return true } + if resolveHomeModel(context.Background(), d).tbTip { + t.Fatal("the minimal signed-out screen carries no tip") + } + }) +} + +// TestResolveHomeModel_SlowProbesDegradeFast is the timeout/degrade contract: a +// probe slower than the budget must NOT hold up the screen, and must NOT be able +// to produce a false Online. Both cases assert we return well within the budget +// with the softer state. +func TestResolveHomeModel_SlowProbesDegradeFast(t *testing.T) { + const budget = 80 * time.Millisecond + + t.Run("slow env probe → renders fast, not online", func(t *testing.T) { + d := baseDeps() + d.budget = budget + d.probeEnv = func(ctx context.Context) envProbe { + select { + case <-time.After(5 * time.Second): + return envProbe{local: localLive, name: "acme-01"} // the "would be online" answer we must NOT wait for + case <-ctx.Done(): + return envProbe{local: localUnreachable} + } + } + start := time.Now() + m := resolveHomeModel(context.Background(), d) + if elapsed := time.Since(start); elapsed > budget+400*time.Millisecond { + t.Fatalf("took %v, should degrade within ~budget (%v)", elapsed, budget) + } + if m.state == homeOnline { + t.Fatalf("a slow env probe must not yield Online; got state %d", m.state) + } + }) + + t.Run("slow heartbeat → running, never a false Online", func(t *testing.T) { + d := baseDeps() + d.budget = budget + d.probeEnv = func(context.Context) envProbe { return envProbe{local: localLive, name: "acme-01"} } + d.probeBeat = func(ctx context.Context) heartbeatState { + select { + case <-time.After(5 * time.Second): + return beatOnline // must NOT be awaited into a green Online + case <-ctx.Done(): + return beatUnknown + } + } + start := time.Now() + m := resolveHomeModel(context.Background(), d) + if elapsed := time.Since(start); elapsed > budget+400*time.Millisecond { + t.Fatalf("took %v, should degrade within ~budget (%v)", elapsed, budget) + } + if m.state != homeRunning { + t.Fatalf("live env + slow (unconfirmed) heartbeat must be running, got state %d", m.state) + } + }) +} + +// node builds a Ready (unless notReady) node with the given allocatable. +func capNode(name, cpu, mem string, notReady bool, gpu ...string) corev1.Node { + alloc := corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpu), + corev1.ResourceMemory: resource.MustParse(mem), + } + if len(gpu) == 1 { + alloc[gpuResource] = resource.MustParse(gpu[0]) + } + status := corev1.ConditionTrue + if notReady { + status = corev1.ConditionFalse + } + return corev1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: corev1.NodeStatus{ + Allocatable: alloc, + Conditions: []corev1.NodeCondition{{Type: corev1.NodeReady, Status: status}}, + }, + } +} + +// TestSumCapacity: Ready-node summation, GiB/CPU rounding, GPU aggregation, and +// the all-NotReady → omit contract. +func TestSumCapacity(t *testing.T) { + t.Run("sums ready nodes, rounds, counts GPUs", func(t *testing.T) { + got, ok := sumCapacity([]corev1.Node{ + capNode("a", "7900m", "16Gi", false, "1"), // 7.9 CPU → 8 + capNode("b", "4", "16Gi", false, "1"), // +4 CPU, +1 GPU + capNode("gone", "8", "32Gi", true), // NotReady → ignored + }) + if !ok { + t.Fatal("expected ok") + } + // 7900m + 4000m = 11900m → 11.9 → 12; 32 GiB total; 2 GPU. + if got != (computeInfo{CPU: 12, MemGiB: 32, GPU: 2}) { + t.Fatalf("got %+v, want {12 32 2}", got) + } + }) + t.Run("no GPU dimension when none present", func(t *testing.T) { + got, ok := sumCapacity([]corev1.Node{capNode("a", "2", "8Gi", false)}) + if !ok || got.GPU != 0 || got.CPU != 2 || got.MemGiB != 8 { + t.Fatalf("got %+v ok=%v", got, ok) + } + }) + t.Run("all not-ready omits compute", func(t *testing.T) { + if _, ok := sumCapacity([]corev1.Node{capNode("a", "8", "32Gi", true)}); ok { + t.Fatal("no Ready node ⇒ ok must be false so the paren is omitted") + } + }) + t.Run("empty omits compute", func(t *testing.T) { + if _, ok := sumCapacity(nil); ok { + t.Fatal("no nodes ⇒ ok=false") + } + }) +} + +// TestSanitizeInvoked: argv[0] → the name we echo in examples. +func TestSanitizeInvoked(t *testing.T) { + cases := map[string]string{ + "/usr/local/bin/tb": binTB, + "tb": binTB, + "tb.exe": binTB, + "/opt/homebrew/bin/tracebloc": binTracebloc, + "tracebloc": binTracebloc, + "tracebloc.exe": binTracebloc, + "./cli.test": binTracebloc, // go test binary → sensible default + "main": binTracebloc, // `go run` temp name + } + for argv0, want := range cases { + if got := sanitizeInvoked(argv0); got != want { + t.Errorf("sanitizeInvoked(%q) = %q, want %q", argv0, got, want) + } + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 54c7851..9a75ae5 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -99,24 +99,16 @@ Helm, no YAML, no kubectl needed.`, // so this removes tracebloc from the host and avoids colliding with `data delete`. root.AddCommand(newDeleteCmd()) - // Bare `tracebloc` (no subcommand) renders a friendly home screen - // instead of cobra's raw usage dump. Subcommands and --help are - // unaffected — cobra dispatches those before this RunE runs. + // Bare `tracebloc` (no subcommand) renders a status-aware home screen — + // where you stand (signed in? environment live?) then the commands — + // instead of cobra's raw usage dump. Subcommands and --help are unaffected: + // cobra dispatches those before this RunE runs. Detection is best-effort and + // time-bounded (see home.go); it never errors, so bare `tracebloc` exits 0. root.RunE = func(cmd *cobra.Command, args []string) error { if len(args) > 0 { return cmd.Help() // an arg that wasn't a known subcommand } - p := printerFor(cmd) - p.Banner("tracebloc", "connect this machine to tracebloc and manage its data") - p.Section("Get started") - p.Infof("tracebloc login — sign in to tracebloc (browser)") - p.Infof("tracebloc data ingest ./data — stage a dataset into your client") - p.Infof("tracebloc data list — datasets in the cluster") - p.Infof("tracebloc data delete — delete an ingested dataset") - p.Infof("tracebloc cluster doctor — diagnose connection issues") - p.Infof("tracebloc delete — remove tracebloc from this machine") - p.Newline() - p.Hintf("Add --help to any command for the full flag list.") + renderHomeScreen(cmd.Context(), printerFor(cmd)) return nil } diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 64711ba..6ae8b49 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -28,10 +28,15 @@ func TestRootCmd_HelpMentionsBinary(t *testing.T) { } } -// TestRootCmd_HomeScreen: a bare `tracebloc` (no subcommand) renders -// the branded home screen pointing at the key commands, rather than -// erroring or dumping raw usage. +// TestRootCmd_HomeScreen: a bare `tracebloc` (no subcommand) renders the +// status-aware home screen, rather than erroring or dumping raw usage. Pointed +// at an empty config dir the machine reads as "not signed in", so this exercises +// the real detection→render wiring end-to-end while staying hermetic: the +// logged-out path does no cluster I/O, so it needs no kubeconfig and returns +// instantly. (Per-state content is covered exhaustively in home_test.go.) func TestRootCmd_HomeScreen(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // no config.json ⇒ signed out + root := NewRootCmd(BuildInfo{Version: "test"}) var out bytes.Buffer root.SetOut(&out) @@ -41,9 +46,9 @@ func TestRootCmd_HomeScreen(t *testing.T) { if err := root.Execute(); err != nil { t.Fatalf("bare root failed: %v\n%s", err, out.String()) } - for _, want := range []string{"tracebloc", "login", "data ingest", "data list", "data delete", "cluster doctor"} { + for _, want := range []string{"tracebloc", "Not signed in", "login"} { if !strings.Contains(out.String(), want) { - t.Errorf("home screen missing %q:\n%s", want, out.String()) + t.Errorf("not-signed-in home screen missing %q:\n%s", want, out.String()) } } } diff --git a/internal/ui/ui.go b/internal/ui/ui.go index e55b35b..dfb4086 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -153,6 +153,30 @@ func (p *Printer) Infof(format string, a ...any) { p.out(" %s %s\n", p.paint("·", color.Faint), fmt.Sprintf(format, a...)) } +// CheckLine, CrossLine, and WarnLine render the status-aware home screen's +// two-axis state block ("Signed in as …", "Secure environment … · Online"), +// each led by a colored glyph. They exist alongside Successf/Errorf/Warnf +// (which use the heavier ✔/✖ and, for Warnf, a wider gap) because the home +// screen's locked copy pins the lighter ✓/✗ glyphs and needs all three lines +// single-spaced so they align in one column. + +// CheckLine prints an affirmative status line led by a green ✓. +func (p *Printer) CheckLine(format string, a ...any) { + p.out(" %s %s\n", p.paint("✓", color.FgGreen), fmt.Sprintf(format, a...)) +} + +// CrossLine prints a negative status line led by a red ✗ (lighter and +// non-bold, unlike Errorf's ✖) — e.g. "not signed in" or an offline environment. +func (p *Printer) CrossLine(format string, a ...any) { + p.out(" %s %s\n", p.paint("✗", color.FgRed), fmt.Sprintf(format, a...)) +} + +// WarnLine prints a caution status line led by a yellow ⚠, single-spaced to +// align with CheckLine/CrossLine (Warnf double-spaces for standalone warnings). +func (p *Printer) WarnLine(format string, a ...any) { + p.out(" %s %s\n", p.paint("⚠", color.FgYellow), fmt.Sprintf(format, a...)) +} + // Detailf prints an indented, dim step-detail line — but ONLY in verbose mode // (WithVerbose). Use for the streamed device-flow → provision → install trace // (§8.5 R-verbose) that would be noise by default; the quiet path skips it. From 0ae25a4d476c941029d11a77e0f9b6e96ae2f6a9 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Sun, 12 Jul 2026 09:26:39 +0200 Subject: [PATCH 2/7] =?UTF-8?q?fix(cli):=20address=20home-screen=20review?= =?UTF-8?q?=20=E2=80=94=20honest=20offline,=20named=20degrades,=20starting?= =?UTF-8?q?=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review follow-ups on the status-aware home screen: - A provisioned machine is never told "no environment / run the installer". A reachable cluster that doesn't host this release (wrong kube-context, or the client runs on a cluster this kubeconfig doesn't point at) now degrades to a NAMED offline when a client name is cached — the "runs elsewhere" case the sibling data commands explain via binding.explain — instead of the no-environment lie. Only a machine that was never provisioned shows no-env. - The budget-timeout degrade keeps the remembered name. resolveHomeModel now reads the cached client name up front (new rememberedName seam) and fills it whenever the probe surfaced none — including the bctx.Done() path — so a context-ignoring kubeconfig exec-credential plugin (aws eks get-token, etc.) that outlives the render degrades to a named offline, not no-env. - The offline copy is honest for BOTH causes (stopped/unreachable AND reachable-but-release-not-here): "· can't reach it from here — run cluster doctor", not the bare "offline". - Degraded workload gets its own state + line (homeStarting: "· starting up, not ready yet"), distinct from the live-but-unconfirmed-heartbeat "running, but tracebloc hasn't heard from it" (heartbeat is never consulted when the workload isn't Ready). Both still point at cluster doctor. - Header comment: the budget bounds the RENDER (wall-clock), not the probe goroutines — a context-ignoring probe can outlive it; the buffered channels just keep it from blocking. Invariants held + mutation-proven: no green Online without localLive + beatOnline (honesty), and detection never hangs (collector bails on the budget without probe cooperation). New/changed tests below; make ci green. Co-Authored-By: Claude Opus 4.8 --- internal/cli/home.go | 125 ++++++++++++++++++++++++-------------- internal/cli/home_test.go | 124 +++++++++++++++++++++++++++++-------- 2 files changed, 178 insertions(+), 71 deletions(-) diff --git a/internal/cli/home.go b/internal/cli/home.go index d0799d4..c93c12c 100644 --- a/internal/cli/home.go +++ b/internal/cli/home.go @@ -16,10 +16,13 @@ package cli // // - Speed + robustness. Bare `tracebloc` runs constantly; the old screen did // zero I/O. Detection here is best-effort and bounded: every probe is -// concurrent, each carries its own short timeout, and the whole thing is -// capped by homeDetectBudget so an unreachable cluster/backend still renders -// the right ⚠/✗ state fast. A probe that errors or times out degrades to the -// softer state; nothing here is ever fatal. +// concurrent, each carries its own short timeout, and homeDetectBudget caps +// the RENDER — the wall-clock we'll wait — not the probe goroutines. A probe +// that ignores its context (a kubeconfig exec-credential plugin like +// `aws eks get-token` is the realistic offender) can outlive the render; the +// buffered result channels just stop it from blocking or leaking, and we +// render the softer/remembered state at the budget. A probe that errors or +// times out degrades to the softer state; nothing here is ever fatal. // // Structure: resolveHomeModel runs the probes and returns a pure homeModel; // renderHome turns that model into output with no I/O. The split keeps rendering @@ -75,8 +78,9 @@ const ( homeNotSignedIn homeState = iota // you're not signed in — minimal, sign in first homeOnline // signed in, environment live AND heartbeating homeRunning // signed in, environment live locally but heartbeat unconfirmed - homeOffline // signed in, environment set up here but unreachable/stopped - homeNoEnv // signed in, no environment on this machine + homeStarting // signed in, release present but its workload isn't Ready yet + homeOffline // signed in, provisioned, but the environment isn't reachable from here + homeNoEnv // signed in, no environment provisioned on this machine ) // computeInfo is the machine's total schedulable capacity, summed across Ready @@ -138,22 +142,28 @@ type envProbe struct { // implementations; tests inject fakes to drive every state, the honesty // fallback, and the timeout/degrade path without touching a real cluster. type homeDeps struct { - signIn func() (signedIn bool, email string) - probeEnv func(ctx context.Context) envProbe - probeBeat func(ctx context.Context) heartbeatState - invoked func() string - tbAvailable func() bool - budget time.Duration + signIn func() (signedIn bool, email string) + probeEnv func(ctx context.Context) envProbe + probeBeat func(ctx context.Context) heartbeatState + // rememberedName is the active client's cached name (no network). It's the + // name we fall back on whenever the probe couldn't surface one — including + // the budget-timeout path, so a provisioned machine degrades to a *named* + // "offline", never to the "no environment / run the installer" lie. + rememberedName func() string + invoked func() string + tbAvailable func() bool + budget time.Duration } func defaultHomeDeps() homeDeps { return homeDeps{ - signIn: realSignIn, - probeEnv: realProbeEnv, - probeBeat: realHeartbeat, - invoked: invokedName, - tbAvailable: tbAliasAvailable, - budget: homeDetectBudget, + signIn: realSignIn, + probeEnv: realProbeEnv, + probeBeat: realHeartbeat, + rememberedName: realRememberedName, + invoked: invokedName, + tbAvailable: tbAliasAvailable, + budget: homeDetectBudget, } } @@ -179,6 +189,10 @@ func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { return homeModel{state: homeNotSignedIn, inv: inv} } + // The remembered client name (cached at provision time, no network). Read up + // front so it's available even on the budget-timeout path below. + remembered := d.rememberedName() + // Signed in: probe the environment and the heartbeat concurrently, both under // one overall budget. Buffered result channels (cap 1) mean an abandoned probe // never blocks or leaks — if it finishes after we've given up on the budget, @@ -206,6 +220,14 @@ func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { } } + // Whenever the probe didn't surface a name — it timed out, couldn't reach the + // cluster, or reached one that doesn't host this release — fall back to the + // remembered name. This is what keeps a provisioned machine on a *named* + // offline line instead of the "no environment" lie, on every degrade path. + if env.name == "" { + env.name = remembered + } + m := homeModel{ email: email, inv: inv, @@ -226,18 +248,18 @@ func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { } m.fullMenu = true case localDegraded: - // Workload present but not Ready → running, not Online. - m.state = homeRunning + // Release present but its workload isn't Ready — it's coming up, which is a + // different story from "live but tracebloc hasn't heard from it" (heartbeat + // was never consulted here). Its own honest line, still → cluster doctor. + m.state = homeStarting m.fullMenu = true - case localNoRelease: - // Cluster reachable, nothing installed: there's genuinely no environment - // here right now, even if config remembers a past one. - m.state = homeNoEnv - m.envName = "" - case localUnreachable: - // Couldn't reach a cluster. If this machine was provisioned (the probe - // surfaced a remembered name) the environment exists but is unreachable → - // offline; otherwise there's simply nothing here. + case localNoRelease, localUnreachable: + // Either the cluster couldn't be reached, or it was reached but doesn't + // host this client's release (wrong kube-context, or the client runs on a + // cluster this kubeconfig doesn't point at — the sibling data commands + // explain this as "runs elsewhere"). If this machine was provisioned (a + // name is remembered), the environment exists but isn't reachable from here + // → offline; with nothing remembered, there's genuinely nothing here. if env.name != "" { m.state = homeOffline m.fullMenu = true @@ -266,6 +288,16 @@ func realSignIn() (bool, string) { return true, cfg.Current().Email } +// realRememberedName is the active client's cached display name (set at +// `client create` time, RFC-0001 §7.3) — no network. Empty when this machine was +// never provisioned, which is exactly the "no environment" signal. +func realRememberedName() string { + if cfg, err := config.Load(); err == nil { + return cfg.Current().ActiveClientName + } + return "" +} + // realProbeEnv is the bounded cluster probe. It reuses the exact namespace // binding + discovery the data/cluster commands use, so the home screen reports // the very environment those commands would target. Best-effort throughout: any @@ -274,42 +306,36 @@ func realProbeEnv(ctx context.Context) envProbe { ctx, cancel := context.WithTimeout(ctx, homeProbeTimeout) defer cancel() - // The name to fall back on when the cluster can't be reached but this machine - // was provisioned — cached at `client create` time, so it needs no network. - fallbackName := "" - if cfg, err := config.Load(); err == nil { - fallbackName = cfg.Current().ActiveClientName - } - + // The name for a discovered release is set below; the unreachable / no-release + // returns leave it empty and let resolveHomeModel fill the remembered name, so + // the "provisioned ⇒ named offline" fallback lives in exactly one place. opts := cluster.KubeconfigOptions{} binding := bindActiveClientNamespace(&opts) resolved, err := cluster.Load(opts) if err != nil { - return envProbe{local: localUnreachable, name: fallbackName} + return envProbe{local: localUnreachable} } // Bound every API call so an unreachable API server can't hang the home // screen (mirrors cluster.ClusterID's time-boxed best-effort read). resolved.RestConfig.Timeout = homeProbeTimeout cs, err := cluster.NewClientset(resolved) if err != nil { - return envProbe{local: localUnreachable, name: fallbackName} + return envProbe{local: localUnreachable} } release, nsUsed, err := discoverRelease(ctx, nil, cs, resolved.Namespace, binding.allowScan()) if err != nil { if errors.Is(err, cluster.ErrNoParentRelease) { + // Cluster reachable, but this release isn't in the resolved context. + // Provisioned ⇒ resolveHomeModel turns this into a named "offline". return envProbe{local: localNoRelease} } // A list/RBAC/connect failure: we couldn't confirm what's here. Treat it // as unreachable (→ offline if provisioned, else no-env). - return envProbe{local: localUnreachable, name: fallbackName} + return envProbe{local: localUnreachable} } - name := release.ReleaseName - if name == "" { - name = fallbackName - } - ep := envProbe{name: name} + ep := envProbe{name: release.ReleaseName} if jobsManagerReady(ctx, cs, nsUsed, release) { ep.local = localLive } else { @@ -472,7 +498,7 @@ func renderHome(p *ui.Printer, m homeModel) { // Greeting: "your secure environment" only when one actually exists here. switch m.state { - case homeOnline, homeRunning, homeOffline: + case homeOnline, homeRunning, homeStarting, homeOffline: p.Para("Welcome to your secure environment for AI 👋") default: p.Para("Welcome to tracebloc — your secure environment for AI 👋") @@ -495,8 +521,15 @@ func renderHome(p *ui.Printer, m homeModel) { case homeRunning: p.WarnLine("Secure environment %q · running, but tracebloc hasn't heard from it — run %s %s", m.envName, m.inv, doctorPath) + case homeStarting: + p.WarnLine("Secure environment %q · starting up, not ready yet — run %s %s", + m.envName, m.inv, doctorPath) case homeOffline: - p.CrossLine("Secure environment %q · offline — run %s %s", m.envName, m.inv, doctorPath) + // One honest line for both causes: a stopped/unreachable cluster AND a + // reachable cluster that doesn't host this release from the current + // kube-context. "can't reach it from here" is true either way. + p.CrossLine("Secure environment %q · can't reach it from here — run %s %s", + m.envName, m.inv, doctorPath) case homeNoEnv: p.WarnLine("No secure environment on this machine yet — run the installer to set one up.") } diff --git a/internal/cli/home_test.go b/internal/cli/home_test.go index 8ae3c43..5719dd8 100644 --- a/internal/cli/home_test.go +++ b/internal/cli/home_test.go @@ -84,16 +84,31 @@ func TestRenderHome_States(t *testing.T) { absent: []string{"· Online"}, }, { - name: "offline points at the doctor with the invoked name", + // Fix 3: a not-Ready workload is "starting up", NOT "hasn't heard from + // it" (the heartbeat was never consulted). Distinct, honest line. + name: "starting up has its own line, not the heartbeat wording", + model: homeModel{ + state: homeStarting, email: "a@b.io", envName: "acme-01", + inv: binTracebloc, fullMenu: true, + }, + present: []string{ + `Secure environment "acme-01" · starting up, not ready yet — run tracebloc cluster doctor`, + }, + absent: []string{"· Online", "hasn't heard from it"}, + }, + { + name: "offline reads honestly for both causes, with the invoked name", model: homeModel{ state: homeOffline, email: "a@b.io", envName: "acme-01", inv: binTB, fullMenu: true, }, present: []string{ - `Secure environment "acme-01" · offline — run tb cluster doctor`, + `Secure environment "acme-01" · can't reach it from here — run tb cluster doctor`, "tb data ingest", // examples echo the invoked name }, - absent: []string{"· Online", "tracebloc data ingest"}, + // Not the bare "offline" (misleading for the reachable-cluster-wrong-context + // cause), and never a green Online. + absent: []string{"· offline", "· Online", "tracebloc data ingest"}, }, { name: "signed in, no environment nudges the installer", @@ -153,15 +168,18 @@ func TestRenderHome_States(t *testing.T) { } // baseDeps is a fully-online, signed-in fake; tests override just the field -// under test. +// under test. rememberedName defaults to "" (this machine was never +// provisioned), so a probe that returns no name degrades to homeNoEnv unless a +// case opts into a remembered name. func baseDeps() homeDeps { return homeDeps{ - signIn: func() (bool, string) { return true, "alice@acme.io" }, - probeEnv: func(context.Context) envProbe { return envProbe{local: localLive, name: "acme-01"} }, - probeBeat: func(context.Context) heartbeatState { return beatOnline }, - invoked: func() string { return binTracebloc }, - tbAvailable: func() bool { return false }, - budget: 2 * time.Second, + signIn: func() (bool, string) { return true, "alice@acme.io" }, + probeEnv: func(context.Context) envProbe { return envProbe{local: localLive, name: "acme-01"} }, + probeBeat: func(context.Context) heartbeatState { return beatOnline }, + rememberedName: func() string { return "" }, + invoked: func() string { return binTracebloc }, + tbAvailable: func() bool { return false }, + budget: 2 * time.Second, } } @@ -197,14 +215,37 @@ func TestResolveHomeModel_States(t *testing.T) { want: homeRunning, }, { - name: "workload degraded = running", + // Fix 3: a not-Ready workload is its own "starting" state, not running. + name: "workload degraded = starting", mutate: func(d *homeDeps) { d.probeEnv = func(context.Context) envProbe { return envProbe{local: localDegraded, name: "acme-01"} } }, - want: homeRunning, + want: homeStarting, + }, + { + // Fix 1: reachable cluster that doesn't host this release, but the + // machine WAS provisioned (probe surfaced the name) → offline, NOT the + // "no environment / run the installer" lie. + name: "no release but provisioned (probe surfaced name) = offline", + mutate: func(d *homeDeps) { + d.probeEnv = func(context.Context) envProbe { return envProbe{local: localNoRelease, name: "acme-01"} } + }, + want: homeOffline, + }, + { + // Fix 1: same, but the name arrives from the remembered-name fallback + // (the probe returned no name) — the wrong-kube-context / runs-elsewhere + // case the sibling data commands handle via binding.explain. + name: "no release but provisioned (remembered name) = offline", + mutate: func(d *homeDeps) { + d.probeEnv = func(context.Context) envProbe { return envProbe{local: localNoRelease} } + d.rememberedName = func() string { return "acme-01" } + }, + want: homeOffline, }, { - name: "cluster reachable, no release = no environment", + // Unchanged: reachable, no release, and never provisioned → no env. + name: "no release and unprovisioned = no environment", mutate: func(d *homeDeps) { d.probeEnv = func(context.Context) envProbe { return envProbe{local: localNoRelease} } }, @@ -238,6 +279,30 @@ func TestResolveHomeModel_States(t *testing.T) { } } +// TestResolveHomeModel_ProvisionedNoReleaseKeepsName is the heart of Fix 1: a +// reachable cluster that doesn't host this release, on a provisioned machine, +// must land on a NAMED offline — not the nameless "no environment" screen — +// whether the name came from the probe or the remembered-name fallback. +func TestResolveHomeModel_ProvisionedNoReleaseKeepsName(t *testing.T) { + t.Run("name from the probe", func(t *testing.T) { + d := baseDeps() + d.probeEnv = func(context.Context) envProbe { return envProbe{local: localNoRelease, name: "acme-01"} } + m := resolveHomeModel(context.Background(), d) + if m.state != homeOffline || m.envName != "acme-01" { + t.Fatalf("got state %d name %q, want offline/acme-01", m.state, m.envName) + } + }) + t.Run("name from the remembered fallback", func(t *testing.T) { + d := baseDeps() + d.probeEnv = func(context.Context) envProbe { return envProbe{local: localNoRelease} } // no name surfaced + d.rememberedName = func() string { return "acme-01" } + m := resolveHomeModel(context.Background(), d) + if m.state != homeOffline || m.envName != "acme-01" { + t.Fatalf("got state %d name %q, want offline/acme-01", m.state, m.envName) + } + }) +} + // TestResolveHomeModel_PassesThroughFields checks the model carries email, name, // and compute from the probes into the render input. func TestResolveHomeModel_PassesThroughFields(t *testing.T) { @@ -293,24 +358,33 @@ func TestResolveHomeModel_TbTip(t *testing.T) { func TestResolveHomeModel_SlowProbesDegradeFast(t *testing.T) { const budget = 80 * time.Millisecond - t.Run("slow env probe → renders fast, not online", func(t *testing.T) { + // Fix 2: a probe that IGNORES its context (a kubeconfig exec-credential plugin + // like `aws eks get-token`) must not hold up the render, and a provisioned + // machine must degrade to a NAMED offline — never the "no environment" lie, + // never Online. Proves the collector bails on the budget without the probe's + // cooperation, and that the remembered name survives that path. + t.Run("context-ignoring env probe → named offline, within budget", func(t *testing.T) { d := baseDeps() d.budget = budget - d.probeEnv = func(ctx context.Context) envProbe { - select { - case <-time.After(5 * time.Second): - return envProbe{local: localLive, name: "acme-01"} // the "would be online" answer we must NOT wait for - case <-ctx.Done(): - return envProbe{local: localUnreachable} - } + d.rememberedName = func() string { return "acme-01" } // this machine was provisioned + started := make(chan struct{}) + d.probeEnv = func(context.Context) envProbe { + close(started) + time.Sleep(800 * time.Millisecond) // deliberately ignores ctx + return envProbe{local: localLive, name: "would-be-online"} // the answer we must NOT wait for } start := time.Now() m := resolveHomeModel(context.Background(), d) - if elapsed := time.Since(start); elapsed > budget+400*time.Millisecond { - t.Fatalf("took %v, should degrade within ~budget (%v)", elapsed, budget) + elapsed := time.Since(start) + <-started // the probe really ran (its late result was abandoned) + if elapsed > budget+300*time.Millisecond { + t.Fatalf("took %v; must render at ~budget (%v) without waiting for the probe", elapsed, budget) + } + if m.state != homeOffline { + t.Fatalf("provisioned machine whose probe timed out must be offline, got state %d", m.state) } - if m.state == homeOnline { - t.Fatalf("a slow env probe must not yield Online; got state %d", m.state) + if m.envName != "acme-01" { + t.Fatalf("remembered name must survive the timeout; got %q", m.envName) } }) From b54311c785e7cda6c37fa05f28f42a5f5182e9bc Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Sun, 12 Jul 2026 11:23:40 +0200 Subject: [PATCH 3/7] fix(home): drain just-finished probes at the budget + honest running copy (Bugbot) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on the status-aware home screen: - Budget expiry could drop completed probe results: when bctx.Done() and a buffered result are ready in the same select, the pick is random — a probe that finished just as the budget fired could be discarded for the softer default (live release rendering as offline/no-env). The collector (now collectProbes, extracted for a deterministic test) drains both buffers non-blocking on Done, so only probes that truly haven't reported degrade. - The running line said "tracebloc hasn't heard from it" for BOTH heartbeat answers. That claim is only earned when the backend positively reported not-online (offline/pending); a mere couldn't-confirm (backend unreachable, timeout) now says "couldn't confirm it's connected to tracebloc" instead of asserting a backend view we never obtained. homeModel carries confirmedNotOnline so the render stays pure. Both fixes mutation-proven (drain removal and flag collapse each fail their new tests); make ci green. Co-Authored-By: Claude Fable 5 --- internal/cli/home.go | 82 ++++++++++++++++++++++++++++++--------- internal/cli/home_test.go | 79 +++++++++++++++++++++++++++++++++++-- 2 files changed, 140 insertions(+), 21 deletions(-) diff --git a/internal/cli/home.go b/internal/cli/home.go index c93c12c..5d2d53c 100644 --- a/internal/cli/home.go +++ b/internal/cli/home.go @@ -77,7 +77,7 @@ type homeState int const ( homeNotSignedIn homeState = iota // you're not signed in — minimal, sign in first homeOnline // signed in, environment live AND heartbeating - homeRunning // signed in, environment live locally but heartbeat unconfirmed + homeRunning // signed in, live locally but not Online (heartbeat unconfirmed, or confirmed not-online) homeStarting // signed in, release present but its workload isn't Ready yet homeOffline // signed in, provisioned, but the environment isn't reachable from here homeNoEnv // signed in, no environment provisioned on this machine @@ -103,6 +103,11 @@ type homeModel struct { inv string // invoked binary name: "tb" or "tracebloc" tbTip bool // show the "type tb instead" tip fullMenu bool // render the full command menu (an environment exists here) + // confirmedNotOnline distinguishes the two honest flavors of homeRunning: + // true = tracebloc's backend POSITIVELY reported this client not-online + // (offline/pending), false = we merely couldn't confirm either way. Same + // state, different knowledge — the running line words each accurately. + confirmedNotOnline bool } // ── Detection ── @@ -205,20 +210,7 @@ func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { go func() { envCh <- d.probeEnv(bctx) }() go func() { beatCh <- d.probeBeat(bctx) }() - env := envProbe{local: localUnreachable} // default if the probe never reports - beat := beatUnknown // default: unconfirmed, never Online - for got := 0; got < 2; { - select { - case env = <-envCh: - envCh = nil // disable this case; a nil channel blocks forever in select - got++ - case beat = <-beatCh: - beatCh = nil - got++ - case <-bctx.Done(): - got = 2 // budget spent — render with whatever arrived, softly - } - } + env, beat := collectProbes(bctx, envCh, beatCh) // Whenever the probe didn't surface a name — it timed out, couldn't reach the // cluster, or reached one that doesn't host this release — fall back to the @@ -240,11 +232,14 @@ func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { case localLive: // Online demands BOTH live-locally AND a positively-confirmed heartbeat. // Anything short of beatOnline (not-online, or couldn't-confirm) is the - // honest "· running" state, never a green Online. + // honest "· running" state, never a green Online — but the model records + // WHICH kind of not-Online it is, so the running line can word a + // backend-confirmed "not online" differently from a mere couldn't-confirm. if beat == beatOnline { m.state = homeOnline } else { m.state = homeRunning + m.confirmedNotOnline = beat == beatNotOnline } m.fullMenu = true case localDegraded: @@ -277,6 +272,46 @@ func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { return m } +// collectProbes receives both probe results, bounded by ctx (the detection +// budget). When the budget fires, a probe that finished JUST beforehand has its +// result sitting in the buffered channel — and select picks uniformly among +// ready cases, so without care the Done branch could win the race and DROP a +// completed answer in favor of the softer default (a live release rendering as +// offline/no-env). So on Done we drain, non-blocking, anything already +// buffered; only a probe that truly hasn't reported keeps its default +// (localUnreachable / beatUnknown — the honest "couldn't confirm" values). +func collectProbes(ctx context.Context, envCh <-chan envProbe, beatCh <-chan heartbeatState) (envProbe, heartbeatState) { + env := envProbe{local: localUnreachable} // default if the probe never reports + beat := beatUnknown // default: unconfirmed, never Online + for got := 0; got < 2; { + select { + case env = <-envCh: + envCh = nil // disable this case; a nil channel blocks forever in select + got++ + case beat = <-beatCh: + beatCh = nil + got++ + case <-ctx.Done(): + // Budget spent — render with whatever arrived, softly. Honor results + // that DID arrive (they're in the buffers), without blocking. + if envCh != nil { + select { + case env = <-envCh: + default: + } + } + if beatCh != nil { + select { + case beat = <-beatCh: + default: + } + } + got = 2 + } + } + return env, beat +} + // realSignIn reports the signed-in state + cached email from local config — no // network. The cached identity is enough for the display; token validity, when // it matters, surfaces through the heartbeat probe (which exercises the token). @@ -519,8 +554,19 @@ func renderHome(p *ui.Printer, m homeModel) { case homeOnline: p.CheckLine("Secure environment %q · Online%s", m.envName, computeParen(m)) case homeRunning: - p.WarnLine("Secure environment %q · running, but tracebloc hasn't heard from it — run %s %s", - m.envName, m.inv, doctorPath) + // Two honest flavors of running, split by what we actually KNOW. + // Backend positively reported not-online (offline/pending): "hasn't + // heard from it" is literally what that status means. Merely + // couldn't-confirm (backend unreachable, timeout, no active client): + // don't claim tracebloc's view — it may be heartbeating fine while only + // our probe failed — say we couldn't confirm. + if m.confirmedNotOnline { + p.WarnLine("Secure environment %q · running, but tracebloc hasn't heard from it — run %s %s", + m.envName, m.inv, doctorPath) + } else { + p.WarnLine("Secure environment %q · running — couldn't confirm it's connected to tracebloc — run %s %s", + m.envName, m.inv, doctorPath) + } case homeStarting: p.WarnLine("Secure environment %q · starting up, not ready yet — run %s %s", m.envName, m.inv, doctorPath) diff --git a/internal/cli/home_test.go b/internal/cli/home_test.go index 5719dd8..b44e371 100644 --- a/internal/cli/home_test.go +++ b/internal/cli/home_test.go @@ -51,7 +51,7 @@ func TestRenderHome_States(t *testing.T) { }, // The two axes stay separate + honest: no "running"/"offline"/signed-out // language on a healthy online screen. - absent: []string{"running, but", "offline", "Not signed in", "No secure environment"}, + absent: []string{"· running", "offline", "Not signed in", "No secure environment"}, }, { name: "online without GPU omits the GPU dimension", @@ -73,15 +73,32 @@ func TestRenderHome_States(t *testing.T) { absent: []string{"CPU", "GiB", "("}, }, { - name: "running but not heartbeating never claims Online", + // Heartbeat UNCONFIRMED (backend unreachable / timeout / no active + // client): we don't know tracebloc's view, so the line must not claim + // it — "couldn't confirm", not "hasn't heard from it". + name: "running with unconfirmed heartbeat says couldn't-confirm, never Online", model: homeModel{ state: homeRunning, email: "a@b.io", envName: "acme-01", inv: binTracebloc, fullMenu: true, }, + present: []string{ + `Secure environment "acme-01" · running — couldn't confirm it's connected to tracebloc — run tracebloc cluster doctor`, + }, + absent: []string{"· Online", "hasn't heard from it"}, + }, + { + // Heartbeat CONFIRMED not-online (backend says offline/pending): + // "hasn't heard from it" is literally what that status means — the + // stronger claim is earned here, and only here. + name: "running with backend-confirmed not-online says hasn't-heard, never Online", + model: homeModel{ + state: homeRunning, email: "a@b.io", envName: "acme-01", + inv: binTracebloc, fullMenu: true, confirmedNotOnline: true, + }, present: []string{ `Secure environment "acme-01" · running, but tracebloc hasn't heard from it — run tracebloc cluster doctor`, }, - absent: []string{"· Online"}, + absent: []string{"· Online", "couldn't confirm"}, }, { // Fix 3: a not-Ready workload is "starting up", NOT "hasn't heard from @@ -279,6 +296,34 @@ func TestResolveHomeModel_States(t *testing.T) { } } +// TestResolveHomeModel_RunningDistinguishesConfirmedNotOnline: both not-Online +// heartbeat answers land on homeRunning, but the model records which KIND so +// the copy can be accurate — true only for a backend-confirmed not-online, +// false for a mere couldn't-confirm, and never set on a healthy Online. +func TestResolveHomeModel_RunningDistinguishesConfirmedNotOnline(t *testing.T) { + t.Run("backend positively reports not-online", func(t *testing.T) { + d := baseDeps() + d.probeBeat = func(context.Context) heartbeatState { return beatNotOnline } + m := resolveHomeModel(context.Background(), d) + if m.state != homeRunning || !m.confirmedNotOnline { + t.Fatalf("state=%d confirmedNotOnline=%v, want running/true", m.state, m.confirmedNotOnline) + } + }) + t.Run("unknown heartbeat stays unconfirmed", func(t *testing.T) { + d := baseDeps() + d.probeBeat = func(context.Context) heartbeatState { return beatUnknown } + m := resolveHomeModel(context.Background(), d) + if m.state != homeRunning || m.confirmedNotOnline { + t.Fatalf("state=%d confirmedNotOnline=%v, want running/false", m.state, m.confirmedNotOnline) + } + }) + t.Run("online never carries the flag", func(t *testing.T) { + if m := resolveHomeModel(context.Background(), baseDeps()); m.confirmedNotOnline { + t.Fatal("a healthy Online must not set confirmedNotOnline") + } + }) +} + // TestResolveHomeModel_ProvisionedNoReleaseKeepsName is the heart of Fix 1: a // reachable cluster that doesn't host this release, on a provisioned machine, // must land on a NAMED offline — not the nameless "no environment" screen — @@ -411,6 +456,34 @@ func TestResolveHomeModel_SlowProbesDegradeFast(t *testing.T) { }) } +// TestCollectProbes_BudgetExpiryKeepsBufferedResults: when the budget fires +// while a finished probe's result is already sitting in its buffered channel, +// select picks uniformly among ready cases — the Done branch must DRAIN the +// buffers rather than let the random pick discard a completed answer (a live +// release rendering as offline/no-env). Pre-filled channels + an already- +// expired context make the ready-race deterministic; without the drain each +// round drops a result with probability ≥ 1/3, so 100 rounds catch a +// regression with near certainty. +func TestCollectProbes_BudgetExpiryKeepsBufferedResults(t *testing.T) { + for i := 0; i < 100; i++ { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // budget already spent when collection starts + + envCh := make(chan envProbe, 1) + beatCh := make(chan heartbeatState, 1) + envCh <- envProbe{local: localLive, name: "acme-01"} // both probes finished: + beatCh <- beatOnline // results sit in the buffers + + env, beat := collectProbes(ctx, envCh, beatCh) + if env.local != localLive || env.name != "acme-01" { + t.Fatalf("round %d: completed env result dropped: %+v", i, env) + } + if beat != beatOnline { + t.Fatalf("round %d: completed beat result dropped: %v", i, beat) + } + } +} + // node builds a Ready (unless notReady) node with the given allocatable. func capNode(name, cpu, mem string, notReady bool, gpu ...string) corev1.Node { alloc := corev1.ResourceList{ From df20387efe32e641ad5715a72557f089b48cc427 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Sun, 12 Jul 2026 11:37:32 +0200 Subject: [PATCH 4/7] fix(home): never adopt a foreign release as "your secure environment" (Bugbot) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With no active client cached, realProbeEnv's discovery fell back to the kubeconfig's default namespace and then the cluster-wide scan (binding.allowScan() is true when the binding isn't applied) — so on a shared cluster a colleague's release could render as YOUR live environment, full data menu included. The data commands only run that scan behind a visible retarget note and an explicit user action; the home probe passes p=nil, so even that disclosure was silently dropped, and §7.5's rule (a miss must never silently retarget to some other client) applies doubly to a status screen. Ownership gate: no active-client binding ⇒ report localNoRelease before any cluster I/O — resolveHomeModel renders the honest no-env screen (or a named offline via the remembered-name fallback). Provisioned machines are untouched (binding scopes discovery to the active client's namespace, scan already disabled). Side effect: the common unprovisioned re-entry now does zero cluster I/O. Mutation-proven both ways (gate removed → the unprovisioned probe dials out and lands unreachable; over-gated → the provisioned probe stops short); make ci green. Co-Authored-By: Claude Fable 5 --- internal/cli/home.go | 15 ++++++++++++ internal/cli/home_test.go | 51 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/internal/cli/home.go b/internal/cli/home.go index 5d2d53c..9e1f5d4 100644 --- a/internal/cli/home.go +++ b/internal/cli/home.go @@ -346,6 +346,21 @@ func realProbeEnv(ctx context.Context) envProbe { // the "provisioned ⇒ named offline" fallback lives in exactly one place. opts := cluster.KubeconfigOptions{} binding := bindActiveClientNamespace(&opts) + // OWNERSHIP GATE: no active-client binding ⇒ nothing was ever provisioned + // for this profile, so no release the kubeconfig can reach is honestly + // YOURS. Without the binding, discovery would fall back to the kubeconfig's + // default namespace and then the cluster-wide scan — either can surface an + // UNRELATED client (a shared cluster, a colleague's install), which this + // screen would then greet as "your secure environment". The data commands + // run that scan behind a visible retarget note and an explicit user action; + // a status screen has neither, and §7.5's rule (a miss must never silently + // retarget to some other client) applies doubly here. Report no-release — + // resolveHomeModel renders the honest no-env screen (or a named offline via + // the remembered-name fallback) — and skip the cluster I/O entirely, which + // also keeps the common unprovisioned re-entry instant. + if !binding.applied { + return envProbe{local: localNoRelease} + } resolved, err := cluster.Load(opts) if err != nil { return envProbe{local: localUnreachable} diff --git a/internal/cli/home_test.go b/internal/cli/home_test.go index b44e371..be80729 100644 --- a/internal/cli/home_test.go +++ b/internal/cli/home_test.go @@ -3,6 +3,8 @@ package cli import ( "bytes" "context" + "os" + "path/filepath" "strings" "testing" "time" @@ -484,6 +486,55 @@ func TestCollectProbes_BudgetExpiryKeepsBufferedResults(t *testing.T) { } } +// TestRealProbeEnv_OwnershipGate: with no active-client binding, the probe must +// never go hunting for a release — the kubeconfig-default-namespace lookup and +// the cluster-wide scan can both surface an UNRELATED client (a shared cluster, +// a colleague's install), which the home screen would then greet as "your +// secure environment". Unprovisioned ⇒ bare no-release (→ the honest no-env +// screen), without touching the cluster; provisioned ⇒ the probe proceeds. +func TestRealProbeEnv_OwnershipGate(t *testing.T) { + // Syntactically valid kubeconfig pointing at an unroutable TEST-NET address: + // loading succeeds, so only the ownership gate stops the probe before it + // dials — without the gate this run would end localUnreachable, not + // localNoRelease. + const kubeconfig = `apiVersion: v1 +kind: Config +clusters: +- name: c + cluster: {server: "https://192.0.2.1:1"} +contexts: +- name: ctx + context: {cluster: c, user: u} +current-context: ctx +users: +- name: u + user: {} +` + t.Run("no active client never adopts a foreign release", func(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // nothing provisioned here + kc := filepath.Join(t.TempDir(), "kubeconfig") + if err := os.WriteFile(kc, []byte(kubeconfig), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("KUBECONFIG", kc) + ep := realProbeEnv(context.Background()) + if ep.local != localNoRelease || ep.name != "" { + t.Fatalf("unprovisioned probe = %+v, want bare localNoRelease (the ownership gate)", ep) + } + }) + t.Run("provisioned machines still probe the cluster", func(t *testing.T) { + writeActiveClientConfig(t, "munich-radiology", "Munich Radiology") + kc := filepath.Join(t.TempDir(), "kubeconfig") + if err := os.WriteFile(kc, []byte("not a kubeconfig"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("KUBECONFIG", kc) + if ep := realProbeEnv(context.Background()); ep.local != localUnreachable { + t.Fatalf("provisioned probe must proceed to the cluster (and fail as unreachable here), got %+v", ep) + } + }) +} + // node builds a Ready (unless notReady) node with the given allocatable. func capNode(name, cpu, mem string, notReady bool, gpu ...string) corev1.Node { alloc := corev1.ResourceList{ From 94e8f98ba992bf6bc4241b8e5bec40cc89a85bc6 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Sun, 12 Jul 2026 12:35:54 +0200 Subject: [PATCH 5/7] feat(home): fold the locked home-screen design; top-level doctor; gated resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design sign-off for the status-aware `tracebloc` home screen (cli#244): fold the LOCKED reference layout into the shipping renderer — byte-for-byte for the signed-in/Online screen. - ui: add Printer.MenuRow (dim · bullet, command padded to width, 4-space gap, dimmed description) — the locked command-row style. - home: rework renderHome to the locked layout — two-blank header, greeting by first name (profile → clean email local-part → omit gracefully), a 30-col dim rule, the two honest status axes (detection + no-false-Online invariant unchanged), two command buckets ("Your data" + "Your secure environment", `delete` folded in, "Manage" dropped), rows via MenuRow, and a dim `love from tracebloc` sign-off. Not-signed-in + no-env restyled to match. - doctor: promote `doctor` to a real top-level command; `cluster doctor` stays as a hidden alias sharing one RunE (single diagnostic code path). The home screen + env-status lines now read ` doctor`. - home: gate the `resources` row on the live command tree — absent until #237 wires `resources`, appears automatically once it does (never a hardcode). - tests: byte-identical lock test against the reference render; name derivation; resources gating (render + command-tree, both ways); top-level doctor shares the cluster-doctor path; all existing state/honesty/timeout coverage kept. make ci green. Co-Authored-By: Claude Opus 4.8 --- internal/cli/cluster.go | 5 +- internal/cli/doctor.go | 27 ++-- internal/cli/group_test.go | 14 ++- internal/cli/home.go | 192 +++++++++++++++++++++-------- internal/cli/home_test.go | 244 ++++++++++++++++++++++++++++++++++--- internal/cli/root.go | 23 +++- internal/cli/root_test.go | 4 +- internal/ui/ui.go | 8 ++ 8 files changed, 430 insertions(+), 87 deletions(-) diff --git a/internal/cli/cluster.go b/internal/cli/cluster.go index a316fa0..f29d6e5 100644 --- a/internal/cli/cluster.go +++ b/internal/cli/cluster.go @@ -40,7 +40,10 @@ the wrong cluster).`, } cmd.AddCommand(newClusterInfoCmd()) - cmd.AddCommand(newClusterDoctorCmd()) + // `cluster doctor` stays as a hidden alias of the top-level `doctor` (both + // share one RunE — see newDoctorCmd) so existing docs / muscle memory keep + // working while the home screen points at the shorter top-level command. + cmd.AddCommand(newDoctorCmd(true)) return cmd } diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 422fe01..cd6dca0 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -14,16 +14,18 @@ import ( "github.com/tracebloc/cli/internal/ui" ) -// newClusterDoctorCmd implements `tracebloc cluster doctor` — the sibling of -// `cluster info` that cluster.go's doc comment anticipated. Where `info` -// answers "is the CLI pointing at the right cluster?", `doctor` answers "is -// this running cluster healthy enough to run an experiment, and if not, what -// do I fix?" — a read-only, post-install health sweep with remedies -// (epic client-runtime#116, WS3). +// newDoctorCmd builds the `doctor` command. The SAME command is registered two +// ways: as the top-level `tracebloc doctor` (visible — the home screen and its +// env-status lines point here), and, hidden, as `tracebloc cluster doctor` (its +// original path, kept working so existing docs, muscle memory, and scripts don't +// break). Pass hidden=true for the cluster-subtree alias. Both entry points +// share one RunE (runClusterDoctor), so there is a single diagnostic code path. // -// The three kubeconfig flags match `cluster info` exactly so muscle memory -// carries over; all are zero-value-safe. -func newClusterDoctorCmd() *cobra.Command { +// It answers "is this running cluster healthy enough to run an experiment, and +// if not, what do I fix?" — a read-only, post-install health sweep with remedies +// (epic client-runtime#116, WS3). The three kubeconfig flags match `cluster +// info` exactly so muscle memory carries over; all are zero-value-safe. +func newDoctorCmd(hidden bool) *cobra.Command { var ( kubeconfigPath string contextOverride string @@ -31,8 +33,9 @@ func newClusterDoctorCmd() *cobra.Command { ) cmd := &cobra.Command{ - Use: "doctor", - Short: "Diagnose a running tracebloc client cluster (✔/⚠/✖ health checks + remedies)", + Use: "doctor", + Hidden: hidden, + Short: "Diagnose a running tracebloc client cluster (✔/⚠/✖ health checks + remedies)", Long: `Runs a read-only health sweep over the tracebloc client release in the configured cluster + namespace and prints a ✔/⚠/✖ line per check with a remedy for anything that isn't green: @@ -76,7 +79,7 @@ func runClusterDoctor( p *ui.Printer, kubeconfigPath, contextOverride, nsOverride string, ) error { - p.Banner("tracebloc", "cluster doctor") + p.Banner("tracebloc", "doctor") // Auth / config checks run FIRST and don't need a cluster — so `doctor` can // diagnose a failed provision (bad/expired token, wrong env, no active diff --git a/internal/cli/group_test.go b/internal/cli/group_test.go index ddce386..ab67019 100644 --- a/internal/cli/group_test.go +++ b/internal/cli/group_test.go @@ -62,7 +62,6 @@ func TestGroup_UnknownSubcommand_Suggests(t *testing.T) { }{ {[]string{"data", "ingst"}, "ingest"}, {[]string{"cluster", "inf"}, "info"}, - {[]string{"cluster", "doctr"}, "doctor"}, } for _, c := range cases { t.Run(strings.Join(c.args, " "), func(t *testing.T) { @@ -74,14 +73,17 @@ func TestGroup_UnknownSubcommand_Suggests(t *testing.T) { } } -// The hidden `client list` (Hidden per Rev-9 §7.10) must NOT be offered as a -// suggestion even though `lst` is one edit from `list` — SuggestionsFor skips -// unavailable commands. +// Hidden subcommands must NOT be offered as suggestions even when the typo is +// one edit away — SuggestionsFor skips unavailable commands. Two are hidden: +// `client list` (Rev-9 §7.10) and `cluster doctor` (now a hidden alias of the +// top-level `doctor`, cli#244). func TestGroup_HiddenSubcommand_NotSuggested(t *testing.T) { - _, out := execGroup(t, "client", "lst") - if strings.Contains(out, "list") { + if _, out := execGroup(t, "client", "lst"); strings.Contains(out, "list") { t.Errorf("hidden `client list` must not be suggested:\n%s", out) } + if _, out := execGroup(t, "cluster", "doctr"); strings.Contains(out, "doctor") { + t.Errorf("hidden `cluster doctor` must not be suggested:\n%s", out) + } } // A bare group (no subcommand) still prints its help and exits 0 — runnable diff --git a/internal/cli/home.go b/internal/cli/home.go index 9e1f5d4..02da85d 100644 --- a/internal/cli/home.go +++ b/internal/cli/home.go @@ -37,6 +37,7 @@ import ( "path/filepath" "strings" "time" + "unicode" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,10 +48,15 @@ import ( "github.com/tracebloc/cli/internal/ui" ) -// doctorPath is the REAL command that diagnoses the environment on develop -// (`cluster doctor`, wired in cluster.go). Kept as a const so the home screen -// never invents a top-level `doctor` — a rename is a separate follow-up. -const doctorPath = "cluster doctor" +// doctorPath is the command the home screen + env-status lines tell the user to +// run to diagnose the environment. It's the top-level `doctor` (newDoctorCmd, +// registered on the root; `cluster doctor` stays working as a hidden alias), so +// the lines read "run doctor". +const doctorPath = "doctor" + +// homeRule is the 30-column dim divider drawn under the greeting and above the +// sign-off — the same string top and bottom, matching the locked design. +const homeRule = "──────────────────────────────" // Invoked binary names we recognize. `` in the examples echoes how the user // actually called the CLI so copy-paste matches their muscle memory. @@ -97,12 +103,17 @@ type computeInfo struct { type homeModel struct { state homeState email string // signed-in account, "" when unknown + name string // first name for the greeting (profile → email local-part → ""); see greetingName envName string // secure environment name, when known compute computeInfo hasCompute bool inv string // invoked binary name: "tb" or "tracebloc" tbTip bool // show the "type tb instead" tip fullMenu bool // render the full command menu (an environment exists here) + // hasResources gates the `resources` row: shown only when a `resources` + // command is actually registered on the root (root.go checks the tree), so + // the menu never advertises a command that isn't wired (#237). + hasResources bool // confirmedNotOnline distinguishes the two honest flavors of homeRunning: // true = tracebloc's backend POSITIVELY reported this client not-online // (offline/pending), false = we merely couldn't confirm either way. Same @@ -147,9 +158,14 @@ type envProbe struct { // implementations; tests inject fakes to drive every state, the honesty // fallback, and the timeout/degrade path without touching a real cluster. type homeDeps struct { - signIn func() (signedIn bool, email string) + signIn func() (signedIn bool, email, firstName string) probeEnv func(ctx context.Context) envProbe probeBeat func(ctx context.Context) heartbeatState + // hasResources reports whether a `resources` command is registered on the + // root — the gate for the home screen's `resources` row. The production + // wiring (renderHomeScreen) checks the real command tree; tests drive both + // sides. + hasResources func() bool // rememberedName is the active client's cached name (no network). It's the // name we fall back on whenever the probe couldn't surface one — including // the budget-timeout path, so a provisioned machine degrades to a *named* @@ -168,14 +184,22 @@ func defaultHomeDeps() homeDeps { rememberedName: realRememberedName, invoked: invokedName, tbAvailable: tbAliasAvailable, - budget: homeDetectBudget, + // Safe default: assume `resources` isn't wired. renderHomeScreen + // overrides this from the real command tree; a test may too. + hasResources: func() bool { return false }, + budget: homeDetectBudget, } } // renderHomeScreen is the bare-`tracebloc` entry point: detect (bounded, -// best-effort) then render. Called from root.RunE. -func renderHomeScreen(ctx context.Context, p *ui.Printer) { - renderHome(p, resolveHomeModel(ctx, defaultHomeDeps())) +// best-effort) then render. Called from root.RunE. resourcesRegistered says +// whether a `resources` command is wired on the root (root.go inspects the +// tree) — the gate for the home screen's `resources` row, so it never lists a +// command that isn't there. +func renderHomeScreen(ctx context.Context, p *ui.Printer, resourcesRegistered bool) { + d := defaultHomeDeps() + d.hasResources = func() bool { return resourcesRegistered } + renderHome(p, resolveHomeModel(ctx, d)) } // resolveHomeModel runs the (best-effort, bounded) probes and assembles the @@ -189,7 +213,7 @@ func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { // minimal "sign in first" screen and skip all cluster/backend I/O entirely — // the common just-installed case returns immediately, and there's no // login-free credential path to honestly report the environment anyway. - signedIn, email := d.signIn() + signedIn, email, firstName := d.signIn() if !signedIn { return homeModel{state: homeNotSignedIn, inv: inv} } @@ -221,11 +245,13 @@ func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { } m := homeModel{ - email: email, - inv: inv, - envName: env.name, - compute: env.compute, - hasCompute: env.hasCompute, + email: email, + name: greetingName(firstName, email), + inv: inv, + envName: env.name, + compute: env.compute, + hasCompute: env.hasCompute, + hasResources: d.hasResources(), } switch env.local { @@ -312,15 +338,49 @@ func collectProbes(ctx context.Context, envCh <-chan envProbe, beatCh <-chan hea return env, beat } -// realSignIn reports the signed-in state + cached email from local config — no -// network. The cached identity is enough for the display; token validity, when -// it matters, surfaces through the heartbeat probe (which exercises the token). -func realSignIn() (bool, string) { +// realSignIn reports the signed-in state + cached email and first name from +// local config — no network. The cached identity is enough for the display; +// token validity, when it matters, surfaces through the heartbeat probe (which +// exercises the token). +func realSignIn() (bool, string, string) { cfg, err := config.Load() if err != nil || !cfg.SignedIn() { - return false, "" + return false, "", "" } - return true, cfg.Current().Email + prof := cfg.Current() + return true, prof.Email, prof.FirstName +} + +// greetingName derives the first name shown in the signed-in greeting. It +// prefers the profile's stored first name; failing that, the email's local part +// — but ONLY when that's a single clean alphabetic token, so "lukas@…" → "Lukas" +// while "lukas.wuttke@…", "l.w+tag@…", or "user123@…" fall through; failing +// that, "" so the greeting drops the name gracefully (rendered without a comma). +func greetingName(firstName, email string) string { + if n := strings.TrimSpace(firstName); n != "" { + return n + } + local, _, found := strings.Cut(email, "@") + if !found || local == "" { + return "" + } + for _, r := range local { + if !unicode.IsLetter(r) { + return "" + } + } + return capitalizeFirst(local) +} + +// capitalizeFirst upper-cases the first rune of s, leaving the rest untouched +// ("lukas" → "Lukas"). "" stays "" (never indexes an empty rune slice). +func capitalizeFirst(s string) string { + r := []rune(s) + if len(r) == 0 { + return s + } + r[0] = unicode.ToUpper(r[0]) + return string(r) } // realRememberedName is the active client's cached display name (set at @@ -542,17 +602,20 @@ type menuBucket struct { // renderHome writes the status-aware home screen. Pure: no I/O, no clock — it's // a deterministic function of the model, which is what makes every state -// table-testable. +// table-testable. The layout is the locked design (cli#244): a two-blank-line +// header, the greeting, a dim rule, the two honest status axes, the command +// buckets, then a dim sign-off. func renderHome(p *ui.Printer, m homeModel) { + // ── header ── two blank lines above the greeting; below it a blank line, the + // rule, then a blank line before the status block. + p.Newline() + p.Newline() + p.Para(greeting(m)) + p.Newline() + p.Hintf(homeRule) p.Newline() - // Greeting: "your secure environment" only when one actually exists here. - switch m.state { - case homeOnline, homeRunning, homeStarting, homeOffline: - p.Para("Welcome to your secure environment for AI 👋") - default: - p.Para("Welcome to tracebloc — your secure environment for AI 👋") - } + // ── status: two axes (you · the machine), never fused ── // Sign-in axis (you). if m.state == homeNotSignedIn { @@ -595,9 +658,14 @@ func renderHome(p *ui.Printer, m homeModel) { p.WarnLine("No secure environment on this machine yet — run the installer to set one up.") } - renderBuckets(p, m.inv, menuFor(m.state)) + // ── command buckets ── two blank lines between the status block and the first + // section (Section leads with one blank; this Newline makes it two). + p.Newline() + renderBuckets(p, m.inv, menuFor(m)) - // Closing. + // ── footer ── two blank lines between the last row and the tip; a trailing + // blank so the sign-off stands alone above the prompt. + p.Newline() p.Newline() if m.tbTip { p.Hintf("tip · type tb instead of tracebloc — either works") @@ -606,15 +674,36 @@ func renderHome(p *ui.Printer, m homeModel) { p.Hintf("Add --help to any command for the flags.") } p.Newline() - p.Hintf("──────────────────────────────") - p.Para("love from tracebloc 💚") + p.Hintf(homeRule) + p.Newline() + p.Hintf("love from tracebloc 💚") // dim, like the supporting text + p.Newline() +} + +// greeting is the locked welcome line. States where a secure environment exists +// greet the user by name ("… for AI, Lukas 👋"), dropping the name gracefully +// when none could be derived; the not-signed-in and no-environment screens use +// the brand greeting ("Welcome to tracebloc — …") instead, since there's no +// environment here to call "yours". +func greeting(m homeModel) string { + switch m.state { + case homeOnline, homeRunning, homeStarting, homeOffline: + if m.name != "" { + return fmt.Sprintf("Welcome to your secure environment for AI, %s 👋", m.name) + } + return "Welcome to your secure environment for AI 👋" + default: + return "Welcome to tracebloc — your secure environment for AI 👋" + } } // menuFor is the command buckets to show per state. Not-signed-in and no-env // stay deliberately lean (one actionable step); the environment states get the -// full menu. -func menuFor(state homeState) []menuBucket { - switch state { +// full two-bucket menu. Offboarding (`delete`) lives under "Your secure +// environment"; the `resources` row appears only when that command is actually +// registered (m.hasResources) — see the field's note. +func menuFor(m homeModel) []menuBucket { + switch m.state { case homeNotSignedIn: return []menuBucket{{ title: "Start here", @@ -627,31 +716,33 @@ func menuFor(state homeState) []menuBucket { title: "Your secure environment", rows: [][2]string{{doctorPath, "check the connection & diagnose issues"}}, }} - default: // online / running / offline — an environment exists here + default: // online / running / starting / offline — an environment exists here + env := make([][2]string, 0, 3) + if m.hasResources { + env = append(env, [2]string{"resources", "manage compute & memory"}) + } + env = append(env, + [2]string{doctorPath, "check the connection & diagnose issues"}, + [2]string{"delete", "remove tracebloc from this machine"}, + ) return []menuBucket{ { - title: "Work with your data", + title: "Your data", rows: [][2]string{ {"data ingest", "load a dataset into your secure environment"}, {"data list", "list your datasets"}, {"data delete", "remove a dataset"}, }, }, - { - title: "Your secure environment", - rows: [][2]string{{doctorPath, "check the connection & diagnose issues"}}, - }, - { - title: "Manage", - rows: [][2]string{{"delete", "remove tracebloc from this machine"}}, - }, + {title: "Your secure environment", rows: env}, } } } -// renderBuckets prints each bucket as a Section + "· " -// rows, with the command column padded to a single width across ALL buckets so -// the descriptions line up in one column. +// renderBuckets prints each bucket as a Section title, a blank line, then one +// MenuRow per command ("· "), with the command +// column padded to a single width across ALL buckets so the descriptions line +// up in one column (commands normal, descriptions dim). func renderBuckets(p *ui.Printer, inv string, buckets []menuBucket) { width := 0 for _, b := range buckets { @@ -663,8 +754,9 @@ func renderBuckets(p *ui.Printer, inv string, buckets []menuBucket) { } for _, b := range buckets { p.Section(b.title) + p.Newline() // blank line under the title for _, r := range b.rows { - p.Infof("%-*s %s", width, inv+" "+r[0], r[1]) + p.MenuRow(width, inv+" "+r[0], r[1]) } } } diff --git a/internal/cli/home_test.go b/internal/cli/home_test.go index be80729..5cf0bc0 100644 --- a/internal/cli/home_test.go +++ b/internal/cli/home_test.go @@ -3,12 +3,15 @@ package cli import ( "bytes" "context" + "errors" + "fmt" "os" "path/filepath" "strings" "testing" "time" + "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -38,22 +41,32 @@ func TestRenderHome_States(t *testing.T) { { name: "online with full compute", model: homeModel{ - state: homeOnline, email: "alice@acme.io", envName: "acme-01", + state: homeOnline, email: "alice@acme.io", name: "Alice", envName: "acme-01", compute: computeInfo{CPU: 8, MemGiB: 32, GPU: 1}, hasCompute: true, - inv: binTracebloc, fullMenu: true, + inv: binTracebloc, fullMenu: true, tbTip: true, hasResources: true, }, present: []string{ - "Welcome to your secure environment for AI", + "Welcome to your secure environment for AI, Alice 👋", // greeting by name "Signed in as alice@acme.io", `Secure environment "acme-01" · Online (8 CPU · 32 GiB · 1 GPU)`, + "Your data", "tracebloc data ingest", "tracebloc data list", "tracebloc data delete", - "tracebloc cluster doctor", "tracebloc delete", + "Your secure environment", + "tracebloc resources", // shown because a resources command is registered + "tracebloc doctor", "tracebloc delete", + "tip · type tb instead of tracebloc — either works", "Add --help to any command for the flags.", + "──────────────────────────────", // header + footer rule "love from tracebloc", }, // The two axes stay separate + honest: no "running"/"offline"/signed-out - // language on a healthy online screen. - absent: []string{"· running", "offline", "Not signed in", "No secure environment"}, + // language on a healthy online screen. And the diagnostic is the NEW + // top-level `doctor`, never the old `cluster doctor`; the "Manage" and + // "Work with your data" buckets are gone. + absent: []string{ + "· running", "offline", "Not signed in", "No secure environment", + "cluster doctor", "Manage", "Work with your data", + }, }, { name: "online without GPU omits the GPU dimension", @@ -84,7 +97,7 @@ func TestRenderHome_States(t *testing.T) { inv: binTracebloc, fullMenu: true, }, present: []string{ - `Secure environment "acme-01" · running — couldn't confirm it's connected to tracebloc — run tracebloc cluster doctor`, + `Secure environment "acme-01" · running — couldn't confirm it's connected to tracebloc — run tracebloc doctor`, }, absent: []string{"· Online", "hasn't heard from it"}, }, @@ -98,7 +111,7 @@ func TestRenderHome_States(t *testing.T) { inv: binTracebloc, fullMenu: true, confirmedNotOnline: true, }, present: []string{ - `Secure environment "acme-01" · running, but tracebloc hasn't heard from it — run tracebloc cluster doctor`, + `Secure environment "acme-01" · running, but tracebloc hasn't heard from it — run tracebloc doctor`, }, absent: []string{"· Online", "couldn't confirm"}, }, @@ -111,7 +124,7 @@ func TestRenderHome_States(t *testing.T) { inv: binTracebloc, fullMenu: true, }, present: []string{ - `Secure environment "acme-01" · starting up, not ready yet — run tracebloc cluster doctor`, + `Secure environment "acme-01" · starting up, not ready yet — run tracebloc doctor`, }, absent: []string{"· Online", "hasn't heard from it"}, }, @@ -122,12 +135,12 @@ func TestRenderHome_States(t *testing.T) { inv: binTB, fullMenu: true, }, present: []string{ - `Secure environment "acme-01" · can't reach it from here — run tb cluster doctor`, + `Secure environment "acme-01" · can't reach it from here — run tb doctor`, "tb data ingest", // examples echo the invoked name }, // Not the bare "offline" (misleading for the reachable-cluster-wrong-context // cause), and never a green Online. - absent: []string{"· offline", "· Online", "tracebloc data ingest"}, + absent: []string{"· offline", "· Online", "tracebloc data ingest", "cluster doctor"}, }, { name: "signed in, no environment nudges the installer", @@ -135,10 +148,10 @@ func TestRenderHome_States(t *testing.T) { present: []string{ "Signed in as a@b.io", "No secure environment on this machine yet — run the installer to set one up.", - "tracebloc cluster doctor", // still offer to diagnose + "tracebloc doctor", // still offer to diagnose }, // Lean: don't parade data commands at a machine with no environment. - absent: []string{"data ingest", "· Online", "Add --help"}, + absent: []string{"data ingest", "· Online", "Add --help", "cluster doctor"}, }, { name: "not signed in is minimal", @@ -192,12 +205,13 @@ func TestRenderHome_States(t *testing.T) { // case opts into a remembered name. func baseDeps() homeDeps { return homeDeps{ - signIn: func() (bool, string) { return true, "alice@acme.io" }, + signIn: func() (bool, string, string) { return true, "alice@acme.io", "" }, probeEnv: func(context.Context) envProbe { return envProbe{local: localLive, name: "acme-01"} }, probeBeat: func(context.Context) heartbeatState { return beatOnline }, rememberedName: func() string { return "" }, invoked: func() string { return binTracebloc }, tbAvailable: func() bool { return false }, + hasResources: func() bool { return false }, budget: 2 * time.Second, } } @@ -212,7 +226,7 @@ func TestResolveHomeModel_States(t *testing.T) { }{ { name: "signed out", - mutate: func(d *homeDeps) { d.signIn = func() (bool, string) { return false, "" } }, + mutate: func(d *homeDeps) { d.signIn = func() (bool, string, string) { return false, "", "" } }, want: homeNotSignedIn, }, { @@ -390,7 +404,7 @@ func TestResolveHomeModel_TbTip(t *testing.T) { }) t.Run("signed out never tips", func(t *testing.T) { d := baseDeps() - d.signIn = func() (bool, string) { return false, "" } + d.signIn = func() (bool, string, string) { return false, "", "" } d.tbAvailable = func() bool { return true } if resolveHomeModel(context.Background(), d).tbTip { t.Fatal("the minimal signed-out screen carries no tip") @@ -610,3 +624,201 @@ func TestSanitizeInvoked(t *testing.T) { } } } + +// TestRenderHome_MatchesLockedDemo is the design sign-off: the signed-in / +// Online screen must render byte-for-byte the LOCKED reference (cmd/hsdemo). The +// expected text is reconstructed here from the reference's exact structure — +// two-blank header, greeting-by-name, dim 30-col rule, the two status axes, the +// two command buckets (command column padded to a single width, descriptions +// dim), and the dim sign-off. MenuRow spacing is computed (not hand-typed) at +// the same width the renderer uses, so the only thing under test is that the +// renderer emits this exact sequence. Any drift in spacing, copy, glyphs, or +// blank-line count fails here. +func TestRenderHome_MatchesLockedDemo(t *testing.T) { + // The locked example: signed in, Online, invoked as `tb`, a real alias so the + // tip shows, and a resources command registered — matching the demo's inputs. + m := homeModel{ + state: homeOnline, email: "lukas@tracebloc.io", name: "Lukas", envName: "lukas-01", + compute: computeInfo{CPU: 8, MemGiB: 32, GPU: 1}, hasCompute: true, + inv: binTB, tbTip: true, fullMenu: true, hasResources: true, + } + + rule := " ──────────────────────────────" // 2-space indent + 30 cols + // row mirrors ui.MenuRow's color-off output at the width the renderer uses for + // this menu (widest entry: "tb data ingest" / "tb data delete" = 14). + row := func(cmd, desc string) string { return fmt.Sprintf(" · %-14s %s", cmd, desc) } + + want := strings.Join([]string{ + "", + "", + " Welcome to your secure environment for AI, Lukas 👋", + "", + rule, + "", + " ✓ Signed in as lukas@tracebloc.io", + ` ✓ Secure environment "lukas-01" · Online (8 CPU · 32 GiB · 1 GPU)`, + "", + "", + " Your data", + "", + row("tb data ingest", "load a dataset into your secure environment"), + row("tb data list", "list your datasets"), + row("tb data delete", "remove a dataset"), + "", + " Your secure environment", + "", + row("tb resources", "manage compute & memory"), + row("tb doctor", "check the connection & diagnose issues"), + row("tb delete", "remove tracebloc from this machine"), + "", + "", + " tip · type tb instead of tracebloc — either works", + " Add --help to any command for the flags.", + "", + rule, + "", + " love from tracebloc 💚", + "", + }, "\n") + "\n" + + if got := renderToString(m); got != want { + t.Errorf("locked-demo render drifted.\n--- got (%d bytes) ---\n%q\n--- want (%d bytes) ---\n%q", + len(got), got, len(want), want) + } +} + +// TestGreetingName pins the name-derivation rule: profile first name wins; +// otherwise a clean single-alpha email local-part, capitalized; otherwise "". +func TestGreetingName(t *testing.T) { + cases := []struct{ firstName, email, want string }{ + {"Lukas", "lukas@tracebloc.io", "Lukas"}, // profile first name wins outright + {" Divya ", "someone@else.io", "Divya"}, // trimmed, and beats the email + {"", "lukas@tracebloc.io", "Lukas"}, // email local-part, capitalized + {"", "alice@acme.io", "Alice"}, // + {"", "LUKAS@x.io", "LUKAS"}, // only the first rune is touched + {"", "lukas.wuttke@tracebloc.io", ""}, // dotted → not a single clean token + {"", "l.w+tag@x.io", ""}, // punctuation → omit + {"", "user123@x.io", ""}, // digits → omit + {"", "@nolocal.io", ""}, // empty local part + {"", "noatsign", ""}, // no @ at all + {"", "", ""}, // nothing to derive from + } + for _, c := range cases { + if got := greetingName(c.firstName, c.email); got != c.want { + t.Errorf("greetingName(%q, %q) = %q, want %q", c.firstName, c.email, got, c.want) + } + } +} + +// TestResolveHomeModel_Name drives the derivation through the detection layer: +// the profile-name / email-fallback / omit branches all flow into m.name. +func TestResolveHomeModel_Name(t *testing.T) { + t.Run("profile first name is preferred over the email", func(t *testing.T) { + d := baseDeps() + d.signIn = func() (bool, string, string) { return true, "lukas@tracebloc.io", "Divya" } + if got := resolveHomeModel(context.Background(), d).name; got != "Divya" { + t.Errorf("name = %q, want Divya", got) + } + }) + t.Run("falls back to a clean email local-part, capitalized", func(t *testing.T) { + d := baseDeps() + d.signIn = func() (bool, string, string) { return true, "lukas@tracebloc.io", "" } + if got := resolveHomeModel(context.Background(), d).name; got != "Lukas" { + t.Errorf("name = %q, want Lukas", got) + } + }) + t.Run("omitted when neither yields a clean single token", func(t *testing.T) { + d := baseDeps() + d.signIn = func() (bool, string, string) { return true, "lukas.wuttke@tracebloc.io", "" } + if got := resolveHomeModel(context.Background(), d).name; got != "" { + t.Errorf("name = %q, want empty", got) + } + }) +} + +// TestRenderHome_ResourcesRowGating: the `resources` row appears iff +// m.hasResources (the render side of the command-tree gate) — driven both ways. +func TestRenderHome_ResourcesRowGating(t *testing.T) { + base := homeModel{ + state: homeOnline, email: "a@b.io", envName: "n", + inv: binTracebloc, fullMenu: true, + } + t.Run("present when a resources command is registered", func(t *testing.T) { + m := base + m.hasResources = true + if got := renderToString(m); !strings.Contains(got, "tracebloc resources") { + t.Errorf("resources row missing when registered:\n%s", got) + } + }) + t.Run("absent when no resources command is registered", func(t *testing.T) { + m := base + m.hasResources = false + if got := renderToString(m); strings.Contains(got, "resources") { + t.Errorf("resources row must not appear when the command isn't wired:\n%s", got) + } + }) +} + +// TestHasTopLevelCommand pins the command-tree gate that feeds hasResources: the +// commands wired today are detected, `resources` is NOT (so the home row stays +// absent until #237 lands), and adding one flips the gate — which is exactly how +// the row will appear automatically once the command ships. +func TestHasTopLevelCommand(t *testing.T) { + root := NewRootCmd(BuildInfo{Version: "test"}) + + for _, name := range []string{"data", "cluster", "doctor", "delete"} { + if !hasTopLevelCommand(root, name) { + t.Errorf("expected %q to be registered on the root", name) + } + } + if hasTopLevelCommand(root, "resources") { + t.Error("resources must not be wired yet — the home row would name a non-existent command") + } + root.AddCommand(&cobra.Command{Use: "resources"}) + if !hasTopLevelCommand(root, "resources") { + t.Error("resources must be detected once registered, so the home row appears automatically") + } +} + +// TestDoctor_TopLevelSharesClusterDoctor: the new top-level `doctor` runs the +// SAME code path as the (now hidden) `cluster doctor` alias. Not-signed-in + an +// unreadable kubeconfig makes the shared path deterministic — auth fails, the +// kubeconfig load fails, and the exit escalates to 2 — so both entry points must +// produce the identical exit code and the same shared diagnostic output. +func TestDoctor_TopLevelSharesClusterDoctor(t *testing.T) { + t.Setenv("TRACEBLOC_CONFIG_DIR", t.TempDir()) // signed out + badKC := filepath.Join(t.TempDir(), "kubeconfig") + if err := os.WriteFile(badKC, []byte("}{ this is not valid kubeconfig"), 0o600); err != nil { + t.Fatal(err) + } + + run := func(args ...string) (string, *exitError) { + root := NewRootCmd(BuildInfo{Version: "test"}) + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.SetArgs(append(args, "--kubeconfig", badKC)) + err := root.Execute() + var ee *exitError + if !errors.As(err, &ee) { + t.Fatalf("%v: want an *exitError, got %v", args, err) + } + return out.String(), ee + } + + topOut, topErr := run("doctor") + clOut, clErr := run("cluster", "doctor") + + if topErr.Code() != clErr.Code() { + t.Errorf("exit codes differ: `doctor`=%d, `cluster doctor`=%d — they must share one code path", + topErr.Code(), clErr.Code()) + } + if topErr.Code() != 2 { + t.Errorf("`doctor` exit = %d, want 2 (auth fail + kubeconfig fail escalates)", topErr.Code()) + } + for label, out := range map[string]string{"doctor": topOut, "cluster doctor": clOut} { + if !strings.Contains(out, "Auth & config") || !strings.Contains(out, "not signed in") { + t.Errorf("%s output missing the shared diagnostic (auth section + not-signed-in):\n%s", label, out) + } + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 9a75ae5..eb4ac08 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -88,6 +88,10 @@ Helm, no YAML, no kubectl needed.`, root.AddCommand(newVersionCmd(info)) root.AddCommand(newIngestCmd()) root.AddCommand(newClusterCmd()) + // Top-level `doctor` — the environment health sweep the home screen and the + // env-status lines point at. Shares its RunE with the hidden `cluster + // doctor` alias (see newDoctorCmd), so there's one diagnostic code path. + root.AddCommand(newDoctorCmd(false)) root.AddCommand(newDataCmd()) // RFC-0001 (backend#830): browser sign-in + client provisioning. root.AddCommand(newLoginCmd()) @@ -108,13 +112,30 @@ Helm, no YAML, no kubectl needed.`, if len(args) > 0 { return cmd.Help() // an arg that wasn't a known subcommand } - renderHomeScreen(cmd.Context(), printerFor(cmd)) + // The home screen shows a `resources` row only when that command is + // actually wired on the root (#237 unmerged today → absent; it appears + // automatically once registered). Gate it on the live tree, never a + // hardcode, so we never advertise a command that isn't there. + renderHomeScreen(cmd.Context(), printerFor(cmd), hasTopLevelCommand(cmd.Root(), "resources")) return nil } return root } +// hasTopLevelCommand reports whether a command with the given name is registered +// directly on the root. The home screen uses it to gate rows on commands that +// may not be wired yet (e.g. `resources`, #237), so a row never names a +// command that doesn't exist. +func hasTopLevelCommand(cmd *cobra.Command, name string) bool { + for _, c := range cmd.Root().Commands() { + if c.Name() == name { + return true + } + } + return false +} + // runGroup is the RunE for a parent "group" command (data, cluster, auth, // client). A bare `tracebloc ` prints its help and exits 0; a mistyped // subcommand (`tracebloc data ingst`) is a hard error (exit 1) with a diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 6ae8b49..76f870d 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -21,7 +21,9 @@ func TestRootCmd_HelpMentionsBinary(t *testing.T) { } got := out.String() - for _, want := range []string{"tracebloc", "version"} { + // `doctor` is now a top-level command (promoted from `cluster doctor`), so it + // must appear in the root help alongside the binary name + version. + for _, want := range []string{"tracebloc", "version", "doctor"} { if !strings.Contains(got, want) { t.Errorf("expected help text to mention %q, got:\n%s", want, got) } diff --git a/internal/ui/ui.go b/internal/ui/ui.go index dfb4086..22ddac6 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -153,6 +153,14 @@ func (p *Printer) Infof(format string, a ...any) { p.out(" %s %s\n", p.paint("·", color.Faint), fmt.Sprintf(format, a...)) } +// MenuRow prints a home-screen command row: a dim · bullet, the command padded to +// width (normal weight, so it's the primary element), a 4-space gap, then the +// description dimmed — so the command clearly stands out against it. Used by the +// home screen's command buckets. +func (p *Printer) MenuRow(width int, cmd, desc string) { + p.out(" %s %-*s %s\n", p.paint("·", color.Faint), width, cmd, p.paint(desc, color.Faint)) +} + // CheckLine, CrossLine, and WarnLine render the status-aware home screen's // two-axis state block ("Signed in as …", "Secure environment … · Online"), // each led by a colored glyph. They exist alongside Successf/Errorf/Warnf From 1584849fc2dd5007b759d8199e72d5de54892b8d Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Sun, 12 Jul 2026 12:58:21 +0200 Subject: [PATCH 6/7] fix(home): bound + sanitize greeting name; retarget cluster-doctor hints to `doctor` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the locked home screen (cli#244): - home: greetingName now runs BOTH the profile first name and the email local-part through one clean-token check (letters only, single word, no interior whitespace/newline/control chars) and caps the result at greetingNameMax = 14 runes — otherwise it omits the name. Stops a long local-part from stretching, or a newline-bearing FirstName from SPLITTING, the locked single-line header. The clean short demo name is unaffected, so the byte-identical golden render is unchanged. - doctor: retarget the now-hidden `cluster doctor` remediation hints to the canonical `tracebloc doctor` (the alias still runs, so non-breaking) — client.go x4 (create-fail hint, discovery-fail error, two connect-timeout errors) + cluster/discover.go's ErrNoParentRelease tail. Kept doctor.Run's suffix-strip in lockstep (it trims that exact tail so doctor never tells you to run doctor) and updated the two tests that pinned the old text. - tests: greeting-name bound/sanitize cases (over-long / interior-newline / tab / control-char / multi-word -> omit; cap boundary used); an end-to-end one-line-header guard; a direct hidden-alias assertion (`doctor` visible top-level, `cluster doctor` present but Hidden). Fixed the golden test's stale cmd/hsdemo comment. make ci green; golden render still byte-identical. Co-Authored-By: Claude Opus 4.8 --- internal/cli/client.go | 10 +-- internal/cli/home.go | 46 ++++++++-- internal/cli/home_test.go | 121 +++++++++++++++++++++------ internal/cli/verbose_install_test.go | 4 +- internal/cluster/discover.go | 2 +- internal/cluster/discover_test.go | 2 +- internal/doctor/doctor.go | 8 +- 7 files changed, 146 insertions(+), 47 deletions(-) diff --git a/internal/cli/client.go b/internal/cli/client.go index 516bb95..36a34e3 100644 --- a/internal/cli/client.go +++ b/internal/cli/client.go @@ -156,7 +156,7 @@ func authedClient() (*api.Client, *config.Config, error) { func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clientCreateOpts) (err error) { // Always leave a full provision trace on disk, even on a quiet/headless run // (RFC-0001 §8.5). On any failure, point at the (idempotent) resume command - // + `cluster doctor`, so a zero-prompt connect that breaks isn't a dead end. + // + `doctor`, so a zero-prompt connect that breaks isn't a dead end. ilog, logPath := newInstallLog() defer ilog.Close() ilog.Logf("client create: name=%q location=%q", opts.name, opts.location) @@ -166,7 +166,7 @@ func runClientCreate(ctx context.Context, p *ui.Printer, pr prompter, opts clien p.Newline() p.Hintf("Provisioning didn't complete. Re-running is safe — on the same cluster it adopts the existing client instead of minting a duplicate (idempotent):") p.Hintf(" %s", resumeCommand(opts)) - p.Hintf("Diagnose auth / cluster problems with: tracebloc cluster doctor") + p.Hintf("Diagnose auth / cluster problems with: tracebloc doctor") if logPath != "" { p.Hintf("Full log: %s", logPath) } @@ -528,7 +528,7 @@ func adoptLiveInClusterClient( "couldn't check whether a tracebloc client is already running on this cluster (%w) — "+ "provisioning now could mint a duplicate that never deploys and locks the cluster to it. "+ "Re-run (if this was transient); if it persists, ensure your kubeconfig/context can list "+ - "deployments and secrets across namespaces. Diagnose with `tracebloc cluster doctor`", err)} + "deployments and secrets across namespaces. Diagnose with `tracebloc doctor`", err)} } ilog.Logf("in-cluster client discovery skipped — cluster unreachable (non-fatal): %v", err) return nil, false, nil @@ -846,11 +846,11 @@ func runClientStatus(ctx context.Context, p *ui.Printer, wait bool, timeout time case lastState >= 0: return &exitError{code: 1, err: fmt.Errorf( "timed out after %s waiting for tracebloc to report this client online (last state: %s). "+ - "Run `tracebloc cluster doctor` to diagnose, or re-run the installer.", timeout, clientStateLabel(lastState))} + "Run `tracebloc doctor` to diagnose, or re-run the installer.", timeout, clientStateLabel(lastState))} default: return &exitError{code: 1, err: fmt.Errorf( "timed out after %s before tracebloc could confirm this client — retry, "+ - "or run `tracebloc cluster doctor`.", timeout)} + "or run `tracebloc doctor`.", timeout)} } } wait := clientStatusPollInterval diff --git a/internal/cli/home.go b/internal/cli/home.go index 02da85d..fb1f2f4 100644 --- a/internal/cli/home.go +++ b/internal/cli/home.go @@ -271,7 +271,7 @@ func resolveHomeModel(ctx context.Context, d homeDeps) homeModel { case localDegraded: // Release present but its workload isn't Ready — it's coming up, which is a // different story from "live but tracebloc hasn't heard from it" (heartbeat - // was never consulted here). Its own honest line, still → cluster doctor. + // was never consulted here). Its own honest line, still → doctor. m.state = homeStarting m.fullMenu = true case localNoRelease, localUnreachable: @@ -351,25 +351,53 @@ func realSignIn() (bool, string, string) { return true, prof.Email, prof.FirstName } +// greetingNameMax caps the greeting name, counted in runes (not bytes). A +// candidate longer than this is dropped rather than stretch the locked +// single-line header; it's generous enough for real first names. +const greetingNameMax = 14 + // greetingName derives the first name shown in the signed-in greeting. It -// prefers the profile's stored first name; failing that, the email's local part -// — but ONLY when that's a single clean alphabetic token, so "lukas@…" → "Lukas" -// while "lukas.wuttke@…", "l.w+tag@…", or "user123@…" fall through; failing -// that, "" so the greeting drops the name gracefully (rendered without a comma). +// prefers the profile's stored first name, then the email's local part, running +// BOTH through cleanGreetingToken so only a safe, single, short token is used; +// the email fallback is capitalized ("lukas@…" → "Lukas"). Anything that isn't a +// clean token — multi-word, an interior newline, control chars, digits, +// punctuation, or longer than greetingNameMax — yields "" so the greeting drops +// the name (and its comma) gracefully. func greetingName(firstName, email string) string { - if n := strings.TrimSpace(firstName); n != "" { + if n := cleanGreetingToken(firstName); n != "" { return n } local, _, found := strings.Cut(email, "@") - if !found || local == "" { + if !found { return "" } - for _, r := range local { + if n := cleanGreetingToken(local); n != "" { + return capitalizeFirst(n) + } + return "" +} + +// cleanGreetingToken trims outer whitespace and returns s only if it's a single +// clean token safe for the one-line header: a non-empty run of at most +// greetingNameMax letters and nothing else. Interior whitespace or newlines +// (which Para would split across lines), control characters, digits, +// punctuation, multi-word names, and over-long tokens all return "" so the +// caller omits the name. +func cleanGreetingToken(s string) string { + s = strings.TrimSpace(s) + if s == "" { + return "" + } + n := 0 + for _, r := range s { if !unicode.IsLetter(r) { return "" } + if n++; n > greetingNameMax { + return "" + } } - return capitalizeFirst(local) + return s } // capitalizeFirst upper-cases the first rune of s, leaving the rest untouched diff --git a/internal/cli/home_test.go b/internal/cli/home_test.go index 5cf0bc0..c17ba5c 100644 --- a/internal/cli/home_test.go +++ b/internal/cli/home_test.go @@ -626,14 +626,14 @@ func TestSanitizeInvoked(t *testing.T) { } // TestRenderHome_MatchesLockedDemo is the design sign-off: the signed-in / -// Online screen must render byte-for-byte the LOCKED reference (cmd/hsdemo). The -// expected text is reconstructed here from the reference's exact structure — -// two-blank header, greeting-by-name, dim 30-col rule, the two status axes, the -// two command buckets (command column padded to a single width, descriptions -// dim), and the dim sign-off. MenuRow spacing is computed (not hand-typed) at -// the same width the renderer uses, so the only thing under test is that the -// renderer emits this exact sequence. Any drift in spacing, copy, glyphs, or -// blank-line count fails here. +// Online screen must render byte-for-byte the LOCKED reference layout. The +// expected text is spelled out in full here (self-contained) from that locked +// structure — two-blank header, greeting-by-name, dim 30-col rule, the two +// status axes, the two command buckets (command column padded to a single +// width, descriptions dim), and the dim sign-off. MenuRow spacing is computed +// (not hand-typed) at the same width the renderer uses, so the only thing under +// test is that the renderer emits this exact sequence. Any drift in spacing, +// copy, glyphs, or blank-line count fails here. func TestRenderHome_MatchesLockedDemo(t *testing.T) { // The locked example: signed in, Online, invoked as `tb`, a real alias so the // tip shows, and a resources command registered — matching the demo's inputs. @@ -687,26 +687,42 @@ func TestRenderHome_MatchesLockedDemo(t *testing.T) { } } -// TestGreetingName pins the name-derivation rule: profile first name wins; -// otherwise a clean single-alpha email local-part, capitalized; otherwise "". +// TestGreetingName pins the name derivation + sanitize rule: profile first name +// wins, else a clean email local-part (capitalized). BOTH candidates must be a +// single letters-only token no longer than greetingNameMax — multi-word, +// control-char, interior-newline, over-long, or otherwise "dirty" candidates +// yield "", keeping the locked greeting on exactly one line. func TestGreetingName(t *testing.T) { - cases := []struct{ firstName, email, want string }{ - {"Lukas", "lukas@tracebloc.io", "Lukas"}, // profile first name wins outright - {" Divya ", "someone@else.io", "Divya"}, // trimmed, and beats the email - {"", "lukas@tracebloc.io", "Lukas"}, // email local-part, capitalized - {"", "alice@acme.io", "Alice"}, // - {"", "LUKAS@x.io", "LUKAS"}, // only the first rune is touched - {"", "lukas.wuttke@tracebloc.io", ""}, // dotted → not a single clean token - {"", "l.w+tag@x.io", ""}, // punctuation → omit - {"", "user123@x.io", ""}, // digits → omit - {"", "@nolocal.io", ""}, // empty local part - {"", "noatsign", ""}, // no @ at all - {"", "", ""}, // nothing to derive from + cases := []struct { + name, firstName, email, want string + }{ + {"profile first name wins outright", "Lukas", "lukas@tracebloc.io", "Lukas"}, + {"profile name trimmed, beats the email", " Divya ", "someone@else.io", "Divya"}, + {"clean email local-part, capitalized", "", "lukas@tracebloc.io", "Lukas"}, + {"clean email local-part again", "", "alice@acme.io", "Alice"}, + {"only the first rune is touched", "", "LUKAS@x.io", "LUKAS"}, + {"dotted local-part is not one token", "", "lukas.wuttke@tracebloc.io", ""}, + {"punctuation omits", "", "l.w+tag@x.io", ""}, + {"digits omit", "", "user123@x.io", ""}, + {"empty local-part omits", "", "@nolocal.io", ""}, + {"no @ at all omits", "", "noatsign", ""}, + {"nothing to derive from", "", "", ""}, + // Bound + sanitize (cli#244 review): a candidate that isn't a single, + // clean, <= greetingNameMax letters token is dropped, never rendered. + {"over-long email local-part omits", "", strings.Repeat("a", 64) + "@x.io", ""}, + {"over-long FirstName omits", strings.Repeat("A", greetingNameMax+1), "", ""}, + {"FirstName exactly at the cap is used", strings.Repeat("A", greetingNameMax), "", strings.Repeat("A", greetingNameMax)}, + {"interior newline in FirstName omits", "Lu\nkas", "", ""}, + {"tab in FirstName omits", "Lu\tkas", "", ""}, + {"control char in FirstName omits", "\x01Bad", "", ""}, + {"multi-word FirstName omits", "Mary Anne", "", ""}, } for _, c := range cases { - if got := greetingName(c.firstName, c.email); got != c.want { - t.Errorf("greetingName(%q, %q) = %q, want %q", c.firstName, c.email, got, c.want) - } + t.Run(c.name, func(t *testing.T) { + if got := greetingName(c.firstName, c.email); got != c.want { + t.Errorf("greetingName(%q, %q) = %q, want %q", c.firstName, c.email, got, c.want) + } + }) } } @@ -822,3 +838,58 @@ func TestDoctor_TopLevelSharesClusterDoctor(t *testing.T) { } } } + +// TestRenderHome_GreetingStaysOneLine is the end-to-end guard for the sanitize +// fix: a profile FirstName carrying an interior newline must never reach the +// header — Para splits on '\n', which would break the locked single-line +// greeting. Here neither the newline FirstName nor the punctuated email yields a +// clean token, so the greeting is the intact nameless one-liner and the unsafe +// bytes never appear. +func TestRenderHome_GreetingStaysOneLine(t *testing.T) { + d := baseDeps() + d.signIn = func() (bool, string, string) { return true, "bad.local+tag@x.io", "Bad\nName" } + got := renderToString(resolveHomeModel(context.Background(), d)) + + // The full nameless greeting on one physical line (starts after a newline, + // ends at one, no interior break) — its exact presence proves the unsafe name + // was dropped rather than split across lines. + if !strings.Contains(got, " Welcome to your secure environment for AI 👋\n") { + t.Fatalf("greeting must be the intact nameless one-liner (unsafe name dropped), got:\n%q", got) + } + if strings.Contains(got, "Bad") { + t.Fatalf("newline-bearing FirstName leaked into the render:\n%q", got) + } +} + +// TestClusterCmd_DoctorIsHiddenAlias pins the hidden-alias decision directly (so +// it's not only covered via the typo-suggestion test): `doctor` is registered +// top-level AND visible, while `cluster doctor` still exists but is HIDDEN — the +// alias keeps working without re-cluttering the `cluster` help with a duplicate. +func TestClusterCmd_DoctorIsHiddenAlias(t *testing.T) { + root := NewRootCmd(BuildInfo{Version: "test"}) + + find := func(parent *cobra.Command, name string) *cobra.Command { + for _, c := range parent.Commands() { + if c.Name() == name { + return c + } + } + return nil + } + + top := find(root, "doctor") + if top == nil || top.Hidden { + t.Fatalf("top-level `doctor` must be registered and visible (got %v)", top) + } + cluster := find(root, "cluster") + if cluster == nil { + t.Fatal("cluster command not found") + } + alias := find(cluster, "doctor") + if alias == nil { + t.Fatal("`cluster doctor` alias must still exist so existing usage keeps working") + } + if !alias.Hidden { + t.Error("`cluster doctor` must be HIDDEN — it's an alias of the top-level `doctor`") + } +} diff --git a/internal/cli/verbose_install_test.go b/internal/cli/verbose_install_test.go index 78ec84f..efd4624 100644 --- a/internal/cli/verbose_install_test.go +++ b/internal/cli/verbose_install_test.go @@ -96,8 +96,8 @@ func TestClientCreate_FailurePrintsResumeAndWritesInstallLog(t *testing.T) { if !strings.Contains(out.String(), "tracebloc client create --name 'My Client' --location DE") { t.Errorf("missing / incorrect resume command:\n%s", out.String()) } - if !strings.Contains(out.String(), "cluster doctor") { - t.Errorf("missing `cluster doctor` pointer:\n%s", out.String()) + if !strings.Contains(out.String(), "tracebloc doctor") { + t.Errorf("missing `tracebloc doctor` pointer:\n%s", out.String()) } // An install-.log is written, recording the failure. logs, _ := filepath.Glob(filepath.Join(dir, "install-*.log")) diff --git a/internal/cluster/discover.go b/internal/cluster/discover.go index f853e6d..257e6ed 100644 --- a/internal/cluster/discover.go +++ b/internal/cluster/discover.go @@ -129,7 +129,7 @@ func DiscoverParentRelease(ctx context.Context, cs kubernetes.Interface, namespa "If your client runs in another namespace, pass --namespace; "+ "if this cluster has no tracebloc client yet, run the installer: "+ "bash <(curl -fsSL https://tracebloc.io/i.sh). "+ - "Diagnose with `tracebloc cluster doctor`.", + "Diagnose with `tracebloc doctor`.", ErrNoParentRelease, namespace, ) case 1: diff --git a/internal/cluster/discover_test.go b/internal/cluster/discover_test.go index d316eb3..e7860e5 100644 --- a/internal/cluster/discover_test.go +++ b/internal/cluster/discover_test.go @@ -186,7 +186,7 @@ func TestDiscoverParentRelease_NoReleaseFound(t *testing.T) { // The error message has to be customer-actionable. Pin the // key remediation phrase so a future refactor that loses it // (or worse, replaces it with a stack trace) fails this test. - for _, want := range []string{"no tracebloc client found", "--namespace", "https://tracebloc.io/i.sh", "cluster doctor"} { + for _, want := range []string{"no tracebloc client found", "--namespace", "https://tracebloc.io/i.sh", "tracebloc doctor"} { if !strings.Contains(err.Error(), want) { t.Errorf("expected error to mention %q, got: %s", want, err) } diff --git a/internal/doctor/doctor.go b/internal/doctor/doctor.go index 43d8b0f..2dde3e6 100644 --- a/internal/doctor/doctor.go +++ b/internal/doctor/doctor.go @@ -123,10 +123,10 @@ func Run(ctx context.Context, cs kubernetes.Interface, opts Options) []Result { func checkReachable(release *cluster.ParentRelease, err error, ns string) Result { const name = "Cluster reachable" if err != nil { - // The discovery error's remediation tail points at cluster doctor — - // which is what's running. Strip it so doctor never tells the user - // to run doctor. - detail := strings.TrimSuffix(strings.TrimSpace(err.Error()), "Diagnose with `tracebloc cluster doctor`.") + // The discovery error's remediation tail points at doctor — which is + // what's running. Strip it so doctor never tells the user to run doctor. + // (Must match the exact suffix cluster.discover appends.) + detail := strings.TrimSuffix(strings.TrimSpace(err.Error()), "Diagnose with `tracebloc doctor`.") return Result{ Name: name, Status: StatusFail, From 03ad7d31f10841e7becb6cf14937c05434a4823e Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Sun, 12 Jul 2026 13:16:35 +0200 Subject: [PATCH 7/7] fix(home): two blank lines above every section title, not just the first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lukas spacing tweak (cli#244): the status->first-section gap was 2 blanks (a standalone pre-loop Newline + Section's own leading blank) while the inter- section gaps were only 1. renderBuckets now emits a Newline before each Section, and the standalone pre-loop Newline is dropped — so every section gets exactly 2 blanks above and the status->first-section gap stays 2 (not 3). Mirrors the locked demo's new Newline+Section+Newline-per-bucket render. Golden test updated (+1 blank before "Your secure environment"); make ci green, render still byte-identical to the demo (850 bytes). Co-Authored-By: Claude Opus 4.8 --- internal/cli/home.go | 17 ++++++++++------- internal/cli/home_test.go | 1 + 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/internal/cli/home.go b/internal/cli/home.go index fb1f2f4..20e3812 100644 --- a/internal/cli/home.go +++ b/internal/cli/home.go @@ -686,9 +686,9 @@ func renderHome(p *ui.Printer, m homeModel) { p.WarnLine("No secure environment on this machine yet — run the installer to set one up.") } - // ── command buckets ── two blank lines between the status block and the first - // section (Section leads with one blank; this Newline makes it two). - p.Newline() + // ── command buckets ── two blank lines above EVERY section: renderBuckets + // emits a Newline before each Section (whose own leading blank makes two), so + // the status→first-section and inter-section gaps are identical. renderBuckets(p, m.inv, menuFor(m)) // ── footer ── two blank lines between the last row and the tip; a trailing @@ -767,10 +767,12 @@ func menuFor(m homeModel) []menuBucket { } } -// renderBuckets prints each bucket as a Section title, a blank line, then one -// MenuRow per command ("· "), with the command -// column padded to a single width across ALL buckets so the descriptions line -// up in one column (commands normal, descriptions dim). +// renderBuckets prints each bucket as a leading blank line, a Section title, +// another blank line, then one MenuRow per command ("· +// ") — the leading blank plus Section's own give two blank lines +// above every section. The command column is padded to a single width across +// ALL buckets so the descriptions line up in one column (commands normal, +// descriptions dim). func renderBuckets(p *ui.Printer, inv string, buckets []menuBucket) { width := 0 for _, b := range buckets { @@ -781,6 +783,7 @@ func renderBuckets(p *ui.Printer, inv string, buckets []menuBucket) { } } for _, b := range buckets { + p.Newline() // two blank lines above each section (this + Section's own leading blank) p.Section(b.title) p.Newline() // blank line under the title for _, r := range b.rows { diff --git a/internal/cli/home_test.go b/internal/cli/home_test.go index c17ba5c..6189872 100644 --- a/internal/cli/home_test.go +++ b/internal/cli/home_test.go @@ -665,6 +665,7 @@ func TestRenderHome_MatchesLockedDemo(t *testing.T) { row("tb data list", "list your datasets"), row("tb data delete", "remove a dataset"), "", + "", // two blank lines above every section, not just the first " Your secure environment", "", row("tb resources", "manage compute & memory"),