Skip to content

main - > enhance CLI features and tests - #266

Merged
saadqbal merged 47 commits into
mainfrom
develop
Jul 14, 2026
Merged

main - > enhance CLI features and tests#266
saadqbal merged 47 commits into
mainfrom
develop

Conversation

@saadqbal

@saadqbalsaadqbal commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Note

Medium Risk
Touches destructive delete/offboard flows, cluster discovery messaging, and a large new home-screen detection path that could misreport environment state; mitigated by extensive tests and fail-closed delete behavior.

Overview
Replaces the stateless bare tracebloc invocation with a status-aware home screen that probes sign-in, local cluster health, and backend heartbeat (bounded timeouts), then renders context-specific menus and honest Online vs running vs offline states. doctor is promoted to a top-level command (with hidden cluster doctor alias); hints and copy shift from cluster doctor to doctor, and doctor now binds the active client namespace like other cluster commands.

Data path fixes and UX:data delete resolves dataset names case-insensitively and fails closed when the list cannot be read (new exit codes 4/5); deprecation warnings for dataset/push/rm aliases; ingest help derives task count from the registry; friendlier errors for missing labels and keypoint flags; semantic segmentation discovery wired in ingest. Cluster discovery improves empty-scan messaging (“run the installer”) vs RBAC scan failures. Shared addKubeconfigFlags / loadClusterFn test seams enable heavy unit coverage for cluster info, doctor, and delete teardown.

CI/Makefile: Lint tools pinned (lockstep with Makefile); advisory deadcode scan; e2e runs merged unit+integration coverage (make cover*); new k3d offboard teardown job for delete e2e. Broad API client and installer client list --plain contract tests added. Docs: CLI navigation map (Mermaid), RFC updates on dataset immutability / rejected --append.

Reviewed by Cursor Bugbot for commit a9435f6. Bugbot is set up for automated code reviews on this repo. Configure here.

saadqbaland others added 30 commits July 10, 2026 21:49
…op-2026-07-10
chore: back-merge main → develop after v0.8.0 promote
Adds real e2e coverage of the top-level `tracebloc delete` offboard
(RFC-0001 §7.10), which CI's e2e.yml previously left untested (it only
exercised `data ingest`). New test/integration/delete_e2e_test.go:
- TestE2E_RevokeUsesPostNotDelete: a real HTTP round-trip against a
recording stub asserts acceptance (a) — the credential is REVOKED via
POST /edge-device/<id>/revoke/, never a hard DELETE of the row.
- TestE2E_DeleteTeardown: builds the real binary and runs
`tracebloc delete --yes --force` black-box against a throwaway k3d
cluster with a real Helm release, asserting (b) the release is
uninstalled, (c) the k3d cluster is deleted, (d) ~/.tracebloc is wiped,
and (e) the foreign-`tb` guard (#171) leaves a `tb` it didn't create in
place. Opt-in via TB_E2E_K3D=1 and refuses a pre-existing `tracebloc`
cluster, so it never clobbers a dev machine.
The black-box run is kept fully offline (egress through a dead proxy) so
the revoke takes its documented best-effort transport-failure path while
the local teardown runs for real; the POST-not-DELETE contract a live
backend enforces is covered by the stub test. Revoke against a live
backend is left to the pre-prod FR (CI can't provide one — no base-URL
override on the CLI).
Wires a new `delete-teardown` job (k3d + helm) into e2e.yml, mirroring
the ingest e2e job's nightly/dispatch/`e2e`-label gating.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… (#242)
`client list` is Hidden (RFC-0001 §7.10), but the client installer's #303
one-client-per-machine pre-flight still shells out to it:
`scripts/lib/provision.sh` `_account_owns_namespace` runs
`tracebloc client list --plain` and greps the output for
`namespace=<ns>([[:space:]]|$)` to refuse a cross-account re-provision.
Hidden ≠ disabled, so the command still runs — but nothing pinned that
`--plain` stays a valid flag or that the output keeps emitting the
`namespace=<ns>` field in a form the installer's grep matches. If either
drifts, the grep silently fails and #303 stops firing in the field with
no error.
Add a cli-side guard that drives the REAL root (`NewRootCmd` → Execute,
as the installer invokes it, not `runClientList` directly) and asserts:
- `client list --plain` still dispatches + exits 0 while Hidden;
- the output contains the literal `namespace=<ns> location=<loc>`
field (canary for any format drift);
- the installer's exact regex matches the live output (the real break
condition), and is prefix-anchored (no false-accept on a strict
prefix or an absent namespace).
Mutation-verified: renaming the `namespace=`/`location=` fields fails
this test. A format change now breaks CI here instead of silently
disabling the installer's #303 refusal.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…tracebloc may use (P1) (#237)
Adds the top-level `tracebloc resources` command (cli#143): the one-knob,
read-only view of how much of this machine tracebloc may use. No Kubernetes
vocabulary in the output — one number for the machine, one for tracebloc's
per-training-run share.
P1 (SHOW) — built:
- New pure `internal/resources` package: machine capacity from Ready-node
allocatable (summed; single-node installer path is normally one node),
per-run ceiling parsed from the jobs-manager RESOURCE_LIMITS env (the same
source `cluster doctor`'s checkNodeFit reads, so the two never disagree),
chart-default fallback, GPU surfacing, and user-language CPU/GiB formatting.
- Release-scoped jobs-manager env reader that mirrors doctor.findDeployment's
attribution rule on BOTH branches (release-known: prefixed name or
instance-label-matched bare; release-unknown: unique suffix match, else nil)
— never reads another release's component.
- `internal/cli/resources.go`: bare `tracebloc resources` shows; resolves the
cluster via the shared resolveClusterTarget seam (exit 3 kubeconfig / 4
no-release), --verbose adds the raw env + node/GPU breakdown. Wired into
root.go's command tree and home screen.
- Tests: resources pkg ~97%, internal/cli stays above its coverage floor.
P2 (set --cpu/--memory, set max) and P3 (macOS Docker VM raise) — deferred:
the shipped groundwork doesn't yet re-expose a safe persistence path to the
CLI. `set` must write Helm values (a `kubectl set env` is reverted by the
hourly auto-upgrade CronJob), and `helm upgrade` needs the chart reference the
installer resolves via TRACEBLOC_HELM_REPO_NAME / a dev path — not recoverable
from `helm list`. Rather than shell Helm blindly at a live training cluster,
`set` is wired as a proper subcommand — `--cpu`/`--memory` flags and an
optional `max` positional in the approved shape — whose RunE returns an honest
exit-1 "not supported in this build yet" message that points back at
`tracebloc resources`. Wiring the flags now means the designed invocation
parses cleanly (no cobra "unknown flag") and P2 slots in behind it.
Refs #143
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…essage (#879) (#229)
Two remaining acceptance items for the RFC-0001 data-verb rename (backend#879);
the rename + active-client binding already shipped via cli#128.
- Deprecation notices: data's PersistentPreRunE warns once on stderr when a
command is invoked via a deprecated alias — push→"data ingest", rm→"data
delete", bare dataset→"data". Detection uses cobra's reliable
Command.CalledAs() on the executed command (no reflection); the verb notices
point at the full canonical form so they nudge the group rename too. The
already-migrated `dataset <canonical-verb>` case is intentionally unwarned
(documented + pinned in the table test).
- Clear no-client message: when the cluster-wide scan finds NO tracebloc client
(one-machine-one-client, §7.10 — machine not provisioned), discoverRelease now
returns a "run the installer / point at the right cluster with --context" hint
instead of the bare namespace error. Still wraps ErrNoParentRelease (exit 4);
the scanErr, >1 "pick one", and active-client-elsewhere paths are untouched.
Tests: alias-path table test (every alias + canonical + the accepted gap);
no-client exit-4 + scan-unavailable + multi-client regression. build/vet/gofmt
clean; full suite green.
Refs tracebloc/backend#879.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…+ unexport dead symbols (#235)
Refs #127 (three tech-debt quick-wins; the optional data-delete CLI-layer
tests remain out of scope).
ITEM 1 — addKubeconfigFlags helper (internal/cli/flags.go):
Replace the inline --kubeconfig/--context registrations duplicated across
client.go, cluster.go, data.go, data_delete.go, data_list.go, delete.go and
doctor.go with a single shared helper, plus a separate addNamespaceFlag so
commands without a namespace (e.g. `client create`) don't grow a spurious -n.
Pure refactor: flag names/shorthands/defaults/usage strings are byte-identical
(shared constants where the phrasing already matched; per-command literals
kept where it differed). Diff limited to the registration lines.
ITEM 2 — pinned lint tools + deadcode gate (Makefile + build.yml):
Pin errcheck@v1.20.0, ineffassign@v0.2.0, misspell@v0.3.4 (was @latest) in
both the Makefile and the CI workflow, and refresh the now-stale "unpinned"
comment. Add a deadcode (golang.org/x/tools/cmd/deadcode@v0.48.0) gate over
./cmd/tracebloc. The gate is ADVISORY (continue-on-error / `|| true`) for now:
of the 7 funcs it reports, 4 are unsafe or out-of-scope to remove — two
Stringer methods reached only via fmt reflection (Status.String,
JobOutcome.String) and two di#349 test-only parity harnesses (ReadLabelValues,
inferColumnType) — and the 3 ITEM-3 symbols stay test-reachable-only, so a
blocking gate would red-fail CI. Flip to blocking once that backlog clears
(tracked in #6 / #127).
ITEM 3 — unexport test-only exports:
config.Clear -> clearAll (clear is a Go builtin), push.AllCategoryIDs ->
allCategoryIDs, submit.IsSubmitError -> isSubmitError. Each had zero
production callers and is referenced only by same-package tests, so unexport
(not delete) per the ticket; tests updated to match.
Gates from a clean worktree: go build, go vet, gofmt -l (empty), go test ./...
(all green), scripts/coverage-floor.sh (cli 73.9% >= 68%, submit 76.1% >= 72%),
pinned errcheck/ineffassign/misspell (clean), deadcode (advisory).
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* docs: add the CLI navigation map (single source of truth)
Four Mermaid flowcharts (top-level, the two gate chains, data ingest, resources)
+ exit-code legend + cross-links + known gaps. Diffable, renders on GitHub, kept
current via PRs. Flags: two independent gate chains; stateless home today (status
redesign proposed); delete exits 0 on partial offboard; resources unshipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: nav-map — resources show shipped (#237); rebase on develop
#237 put `resources show` on develop, so un-dash SHOW (keep `set`/#241
dashed), bump the basis commit to develop @27c5392, and reconcile the §4
heading + known-gaps note. The exit-6/--overwrite flow already matches
current develop (data.go:236), so it's left as-is.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Arturo Peroni <arturo@tracebloc.io>
…sed (#1027) (#230)
A mis-cased name was a silent no-op. Resolve via listDatasetsFn + EqualFold to the stored spelling (fed into teardown), exit 5 not-found, exit 4 on listing failure. Refs #1008.
feat(ingest): guide users from dataset to creating a use case
…chema types (#233)
Four small ingest/preflight polish tickets, one coherent change. The data
INGESTOR owns validation; the CLI only PREVIEWS it (Principle 6), so every
new rule here mirrors an authoritative ingestor grammar rather than inventing
a CLI-only one.
- #215: derive the "Supports N tasks" count from the task registry
(push.SupportedCategoryIDs) instead of a stale hardcoded "9", and fix the
text example to show sequences/ for masked_language_modeling (its
primary_subdir per layout.v1.json), both read from the vendored contract so
they can't drift again.
- #214: a tabular task with no --label-column now gets the friendly
flag-naming message (listing the CSV's columns) via a targeted pre-check,
instead of the opaque label-oneOf schema dump. Only the missing case is
intercepted; other schema errors still surface.
- #213: validate each --schema TYPE token locally against the ingestor's REAL
accepted set (mirrors database.py::_get_sqlalchemy_type, di#349) so a bogus
type (e.g. age:BANANA) is caught before the upload — on both the --schema
flag path (ParseSchema) and `data validate`. Also fixes ParseSchema's
comma-split so DECIMAL(p,s)/NUMERIC(p,s) — types the ingestor accepts —
parse as one entry instead of being torn apart.
- #76: (a) `data delete ""` gives a delete-appropriate positional-arg message,
not the ingest-path "set --name"; (b) --number-of-keypoints distinguishes
unset ("requires") from an explicit non-positive value ("must be a positive
integer (got N)") via cmd.Flags().Changed; (c) suppress the redundant
parent-level "label: got object, want string" oneOf type-noise when a
specific label.policy error is present.
Tests added/updated for each fix across internal/cli, internal/push, and
internal/schema. go build/vet/test/gofmt all clean; coverage floors hold
(internal/cli 74.4%).
Closes#213#214#215#76
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…#156) (#238)
Adds Principle 6 (datasets are immutable snapshots) and rewrites the §13 --append non-goal into an explicit rejection now that cli#156 is closed obsolete. In-place append breaks dataset integrity/reproducibility; growth = dataset versioning, not mutation. Rationale of record: backend#1073.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…em 3) (#245)
The idempotency key the CLI already sends is becoming the end-to-end
ingest correlation id: jobs-manager derives the Job name from it, labels
every spawned resource with it, and (client-runtime) stamps it into the
ingestor container as TRACEBLOC_INGEST_CORRELATION_ID, where the
ingestor (data-ingestors) logs it and carries it into the backend
registration payload.
The CLI was the only layer that never showed the key, so the customer
had no copy of the one string that threads all layers together. Print
it as a hint line on every submit path — fresh and replay (a replayed
run is exactly when you reach for the id to find the already-running
Job).
No wire change: the key was already in the POST body.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…cy + per-group time order) (#234)
The time_series_classification local preflight only checked column presence
(CheckSequenceSchemaColumns) and null/empty sequence_id rows (CheckSequenceRows).
It did NOT mirror the two grouped validators di#359 (WS1) added, so a
mid-sequence label flip or per-group unsorted timestamps passed the local
dry-run and were rejected only in-cluster after the upload.
Add CheckTSCGroupIntegrity — one pass over the CSV grouped by sequence_id,
previewing both in the ingestor's factory order:
- labelConstantViolation ≙ LabelConstantWithinGroupValidator: one label per
sequence_id. Groups drop null ids (dropna=True), a null/empty label counts
as its own value (nunique(dropna=False)), and numeric labels collapse only
when the whole column is numeric ("1"/"1.0" are not a false flip).
- perGroupTimeViolation ≙ PerGroupTimeOrderedValidator: monotonic non-
decreasing per sequence (ties allowed; interleaving fine). Faithfully mirrors
the NUMERIC branch (pd.to_numeric coerce; invalid/NA → reject). Deliberately
does NOT preview the TIMESTAMP/date branch — reproducing pandas' mixed-format
parse + locale-ambiguity guard in Go would risk over-rejecting a date the
cluster accepts (the dangerous direction). That date-typed ordering stays a
documented under-preview.
Fix an NA-sentinel parity over-reject (review): the grouped validators plain-
read_csv (keep_default_na=True), so their label-numeric-collapse and
sequence-id dropna see pandas' GLOBAL default NA set (STR_NA_VALUES), NOT the
curated coercion.NA_SENTINELS that naSentinels mirrors (used only by the
label-diversity preview, which pins na_values to that set). Mirroring the
curated set here OVER-rejected: a label column pandas reads as all-numeric
(a default sentinel like "#NA" collapsing to NaN, so "1"/"1.0" don't flip) was
seen by the CLI as a mixed object column and flagged as a false mid-sequence
flip. Add pandasDefaultNA (verbatim STR_NA_VALUES at the pinned pandas 3.0.3)
and use it for the grouped checks. Also stop trimming cells in scanTSCRows —
pandas keeps whitespace on object columns / groupby keys and matches NA on the
raw cell; the numeric-collapse and numeric-time paths trim only inside the
parse attempt, mirroring pandas' numeric coercion (which does tolerate " 1").
The float64 collapse still under-approximates pure-int64 columns past 2^53
(safe under-reject; documented).
Strictly grouped-scoped: the sole caller gates on GroupingFor's grouping trait,
so no non-grouped category runs this — important because TSC is develop-ahead
(di#359 is NOT in the deployed v0.7.0 ingestor).
Flip cases.json tsc-label-flip / tsc-unsorted-timestamp from the documented
accept/reject gap to reject/reject (Go preview now mirrors); add
tsc-numeric-label-na-sentinel (accept/accept) pinning the over-reject fix
against the REAL validator; goldens regenerated from the pinned #359 ref. Add
TestCheckTSCGroupIntegrity (14 boundary cases, incl. the pandas-default-NA
sentinel accept and a padded-object-label flip) and verify
sync-validator-goldens.sh --check passes.
Closes#218
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… ids (#239) (#240)
CheckSequenceRows / sequenceScanFrom decided whether a sequence_id cell is
null by trimming the cell and testing the curated coercion.NA_SENTINELS map
(naSentinels). But it previews SequenceGroupValidator, which plain-reads the
CSV (pd.read_csv, keep_default_na=True) and computes null as
`ids.isna() | (ids.astype(str).str.strip() == "")` — i.e. pandas' DEFAULT
STR_NA_VALUES (matched on the raw cell) UNION whitespace-only cells, NOT the
coercion set. Two divergences resulted:
- "none" (lowercase): pandas keeps it → the ingestor ACCEPTS, but the CLI
treated it as null → false REJECT (the dangerous over-reject direction —
blocks an ingest the cluster accepts).
- "#NA": pandas drops it to NaN → the ingestor rejects, but the CLI's
coercion set lacks it → under-reject.
Reuse the pandasDefaultNA map cli#218 added for the grouped label/time checks:
null iff `raw ∈ pandasDefaultNA` OR `strings.TrimSpace(raw) == ""`, matched on
the raw cell (dropping the pre-strip), and count distinct ids on the raw value
(pandas groups the object key verbatim, consistent with the grouped previews).
Parity: new tsc-none-sequence-id case (accept), goldens regenerated against the
pinned data-ingestors ref; tsc-null-sequence-id (empty + NA) still rejects.
TestCheckSequenceRows gains none/#NA/whitespace/padded-NA boundaries — each
ground-truthed against the real validator's null_mask; reverting to naSentinels
fails the pins (mutation-proved).
Closes#239
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ow (#1028) (#231)
checkRestartHistory warns when any init/regular container RestartCount >= 3 — the current-state check missed pods that already crashed. Additive. Refs #1008.
* feat(cli): status-aware `tracebloc` home screen
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). `<inv>` 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 <noreply@anthropic.com>
* fix(cli): address home-screen review — honest offline, named degrades, starting state
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
<inv> 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 <noreply@anthropic.com>
* fix(home): drain just-finished probes at the budget + honest running copy (Bugbot)
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 <noreply@anthropic.com>
* fix(home): never adopt a foreign release as "your secure environment" (Bugbot)
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 <noreply@anthropic.com>
* feat(home): fold the locked home-screen design; top-level doctor; gated resources
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 `<inv> 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 <noreply@anthropic.com>
* fix(home): bound + sanitize greeting name; retarget cluster-doctor hints to `doctor`
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 <noreply@anthropic.com>
* fix(home): two blank lines above every section title, not just the first
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 <noreply@anthropic.com>
* fix(home,doctor): source the provisioned/namespace signal uniformly (review #2/#3)
Addresses @aptracebloc's review — both confirmed Medium, one shared root:
the provisioned/namespace signal wasn't sourced uniformly across the home
screen and doctor.
- doctor ignored the active-client binding: runClusterDoctor now applies
bindActiveClientNamespace before cluster.Load — the same seam `cluster
info` / data / the home screen use — so with no --namespace/--context it
targets the active client's cached namespace, not the kubeconfig default.
Removes the home-screen <-> doctor contradiction. Binding-only: a local
config read, no extra cluster dial, matching what the home screen does
when provisioned (check the bound namespace, no cluster-wide scan).
- provisioned-vs-no-environment keyed off env.name (ActiveClientName) while
the probe's ownership gate keyed off the cached namespace, so a
provisioned-but-unnamed profile misread as "no environment / run the
installer". resolveHomeModel now takes a `provisioned` signal from the
SAME field the gate uses (ActiveClientNamespace) and renders offline on
`provisioned || env.name != ""`; the display name falls back to the
client ID so it stays a *named* offline.
Tests (mutation-proven): new TestResolveHomeModel_States case (provisioned,
namespace only, no name -> offline) + TestClusterDoctor_BindsActiveClientNamespace.
Updated TestHasTopLevelCommand (resources is wired now that #237 merged).
make ci green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…2, wizard + GPU) (#143) (#241)
* feat(resources): tracebloc resources — show how much of this machine tracebloc may use (P1)
Adds the top-level `tracebloc resources` command (cli#143): the one-knob,
read-only view of how much of this machine tracebloc may use. No Kubernetes
vocabulary in the output — one number for the machine, one for tracebloc's
per-training-run share.
P1 (SHOW) — built:
- New pure `internal/resources` package: machine capacity from Ready-node
allocatable (summed; single-node installer path is normally one node),
per-run ceiling parsed from the jobs-manager RESOURCE_LIMITS env (the same
source `cluster doctor`'s checkNodeFit reads, so the two never disagree),
chart-default fallback, GPU surfacing, and user-language CPU/GiB formatting.
- Release-scoped jobs-manager env reader that mirrors doctor.findDeployment's
attribution rule on BOTH branches (release-known: prefixed name or
instance-label-matched bare; release-unknown: unique suffix match, else nil)
— never reads another release's component.
- `internal/cli/resources.go`: bare `tracebloc resources` shows; resolves the
cluster via the shared resolveClusterTarget seam (exit 3 kubeconfig / 4
no-release), --verbose adds the raw env + node/GPU breakdown. Wired into
root.go's command tree and home screen.
- Tests: resources pkg ~97%, internal/cli stays above its coverage floor.
P2 (set --cpu/--memory, set max) and P3 (macOS Docker VM raise) — deferred:
the shipped groundwork doesn't yet re-expose a safe persistence path to the
CLI. `set` must write Helm values (a `kubectl set env` is reverted by the
hourly auto-upgrade CronJob), and `helm upgrade` needs the chart reference the
installer resolves via TRACEBLOC_HELM_REPO_NAME / a dev path — not recoverable
from `helm list`. Rather than shell Helm blindly at a live training cluster,
`set` is wired as a proper subcommand — `--cpu`/`--memory` flags and an
optional `max` positional in the approved shape — whose RunE returns an honest
exit-1 "not supported in this build yet" message that points back at
`tracebloc resources`. Wiring the flags now means the designed invocation
parses cleanly (no cobra "unknown flag") and P2 slots in behind it.
Refs #143
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(resources): tracebloc resources set — raise a run's share of this machine (P2)
Builds the mutating half of `tracebloc resources` (cli#143 P2). The number the
user sets is the per-run ceiling written verbatim to the chart's RESOURCE_*
env (Decision A); tracebloc's ~1 core / 3 GiB overhead is a fit-check safety
margin only, never subtracted.
- Interactive wizard by default on a TTY: shows the current per-run budget vs
the machine, offers "use as much as possible" (pre-selected), "choose an
amount" (bounded per-dimension prompts so over-asking is impossible), or
"leave it".
- Flags for scripting / non-TTY: --cores (hidden --cpu alias), --memory
(GB/G/Gi/GiB/bare → GiB), --gpus, and the `max` positional. Single-dimension
sets keep the others. Human validation messages on every bad path; exit 2.
- GPU is first-class + whole-unit; omitted entirely on a GPU-less machine (the
chart's default GPU env is normalized away so a plain --cores change doesn't
fail the GPU fit-check on a CPU-only host).
- Fit-check against the largest single Ready node (mirrors doctor.checkNodeFit),
overhead included; on macOS the honest ceiling message points at Docker
Desktop → Resources (VM auto-raise stays deferred to P3).
- Apply via new internal/helm seam mirroring the installer/auto-upgrade idiom:
resolve chart ref (or TRACEBLOC_CHART_PATH), version-gate
--reset-then-reuse-values, pin --version, temp -f values file (dodges the
--set comma footgun), resolved --namespace/--kube-context/--kubeconfig,
--wait. --dry-run prints the command + values and runs nothing.
Cluster-free tests via the fake clientset, prompter seam, and a fake helm
Runner. make ci green.
Closes#143
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(resources set): Ctrl-C in the wizard is a clean cancel, not an error (Bugbot)
Interrupting a wizard prompt surfaced survey's errInteractiveCancelled raw
from decideDesired, so bare `tracebloc resources set` + Ctrl-C exited 1 with
"Error: cancelled by user" — unlike every other prompting command. Map it to
the same clean exit 0 + "Cancelled — nothing was changed." note the confirm
decline already prints; validation errors (exit 2) and real terminal failures
pass through unchanged. Mutation-proven test: cancelled wizard → nil error,
the note, and no helm mutation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(resources set): unchanged ceiling is a no-op even when it no longer fits (Bugbot)
validateDesired ran before the sameCeiling no-op check, so on a machine that
shrank under an already-applied ceiling (smaller Docker Desktop VM, lost
node), "Leave it as it is" — and flags merely restating the current values —
failed with exit 2 instead of the promised no-op ("→ no-op skip downstream").
Check sameCeiling first: leaving things unchanged mutates nothing, so there is
nothing for the fit-check to protect; an actual change is still validated
before anything mutates. sameCeiling is a pure desired-vs-current comparison
and needs nothing validateDesired computes, so the reorder is safe.
Mutation-proven test (flags-restate + wizard leave-as-is succeed with no helm
call on a shrunken machine; a real change still exits 2); make ci green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(resources set): confirm-prompt Ctrl-C prints the cancel note too (Bugbot)
Ctrl-C at the final "Let each training run use up to …?" confirm was routed
through mapClientErr — exit 0, but SILENT, so a user could finish the wizard,
interrupt the confirm, and walk away believing the change went through. The
wizard interrupt, the confirm decline, and data delete all print "Cancelled —
nothing was changed."; the confirm interrupt now does the same (exit 0, note,
no helm mutation). Real terminal failures still go through mapClientErr.
Mutation-proven: reverting to the bare mapClientErr path keeps exit 0 but
drops the note, failing TestSet_ConfirmCtrlCIsACleanCancel. make ci green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(resources set): clear a phantom GPU on a GPU-less host even on a no-op ceiling (Bugbot #241)
Bugbot MED (Arturo flagged for a decision): on a GPU-less machine whose cluster
still carries the chart-default GPU_REQUESTS=nvidia.com/gpu=1, restating the
current CPU/mem ceiling (or "leave it as it is") made sameCeiling() true → skipped
persistCeiling → the phantom GPU stayed. It is NOT inert: client-runtime
_gpu_available_from_env treats any non-empty value as a GPU cluster (only an
explicit-empty value means no GPU), so jobs-manager requests a nonexistent
nvidia.com/gpu — unschedulable, forcing a GPU→CPU fallback with a false GPU
heartbeat.
Fix: capture phantomGPU (env has GPU but machine doesn't) before normalizing
HasGPU away, and force the persist on an otherwise-no-op so BuildEnvSpec's
explicit-empty override lands and clears it. New regression test
TestSet_PhantomGPUForcesPersistOnNoOpCeiling (mutation-proven: fails if the gate
is removed).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(resources set): don't fit-check an unchanged ceiling on the phantom-GPU cleanup (Bugbot #241)
Follow-up to 4aa7934: the phantom-GPU cleanup fell through to validateDesired on
an UNCHANGED ceiling, so a machine that shrank under its current ceiling blocked
the cleanup with exit 2 ("phantom GPU cleanup blocked by fit-check"). Gate
validateDesired on the ceiling actually CHANGING — an unchanged ceiling mutates
nothing the fit-check protects (the established no-op-when-shrunk rule), and the
phantom cleanup only REMOVES a GPU request. New regression test
TestSet_PhantomGPUCleanupNotBlockedByFitCheck (mutation-proven).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(resources set): fail honestly when the machine is too small to choose an amount (Bugbot #241)
Bugbot MED: on a node too small to give a run even the 1-core / 2-GiB minimum
after tracebloc's ~1-core, ~3-GiB overhead, the wizard's "Choose an amount" path
prompted an impossible bounded range (e.g. "1–0"), so every answer was rejected
and the guided flow could only be escaped by interrupting. Guard the branch:
fail with an honest exit-2 "machine too small" message instead of trapping the
user. New test TestWizard_ChooseAnAmountTooSmallMachine (mutation-proven).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…lpers (#246)
Pure-additive tests from the CLI test-coverage audit (no production
changes), using harnesses that already exist. Closes two P0s + several
P1/P2 gaps:
- labelSchemaType case/whitespace fallback (P0, label-diversity parity):
a regression silently mis-collapses a case-mismatched VARCHAR label and
false-rejects a diverse dataset the cluster accepts.
- discoverSidecarFiles symlink-dir / symlink-entry / not-a-dir rejects
(P0, security): the shared annotations//masks/ walker had 0% guard
coverage — arbitrary-local-file-disclosure class.
- CheckSchemaColumns missing-column rejection (P1).
- TruncateList "… and N more" truncation (P1).
- ParseMinSize grammar + surfaces the "min size" flag name (was 0%).
- stderrSuffix remote-stderr parenthetical (was 0%).
- FinalDestPrefix path-traversal panic guard (security).
make ci green. First of a short series from the coverage backlog.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
#247)
* feat(data ingest): wire semantic_segmentation (RFC-0002 phase 4, #182)
Closes the last CLI-pending task now that its blockers landed: di#358
shipped the ingestor's mask_id require-and-enforce (v0.7.0) and backend#816
closed. semantic_segmentation now ingests like the other image tasks, with a
masks/ sidecar and the mask_id link-column contract.
- category.go: flip semantic_segmentation CLISupported -> true.
- DiscoverSemanticSegmentation: labels.csv + images/ + masks/*.png (required),
mirroring DiscoverObjectDetection's sidecar path.
- spec: emit spec["masks"] + declare the mask_id column
(schema:{mask_id: VARCHAR(255)}, matching the canonical example) so the
ingestor STORES it -- an undeclared mask_id is dropped and the training
client then can't locate masks (backend#816).
- preflight mirrors the ingestor's semseg validators (modalities/validators.py):
CheckMaskPairing (images<->masks by the "_mask" suffix, FilePairingValidator)
+ CheckMaskIdColumn (mask_id declared exact-lowercase + populated on every
row, NA-sentinel-aware -- MaskIdColumnValidator, backend#816).
- Vendored layout.v1.json already declared semseg's masks sidecar, so no
schema change (drift check green).
Tests: discovery (valid/missing masks), pairing (4 cases), mask_id (5 cases).
Updated the registry/picker/gate tests that pinned "semseg pending" (all 16
categories are CLI-supported now); retired the known-but-unsupported
pending-note test (no such category remains; the defensive branch stays).
make ci green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(data ingest): semseg self-review — NA parity, dotfile skip, rows↔images, friendly label
Adversarial self-review (3-angle /code-review) + a walk of the backend#1074
category blueprint (§3.2 CLI touchpoints, §7 traps) before human review. Fixes:
- CheckMaskIdColumn tests the RAW cell against the NA set (+ a whitespace-only
clause) instead of trimming first, so a padded token like " NULL " isn't
false-flagged empty -- pandas keeps the spaces, so it's a real value
in-cluster (the cli#218/#239 over-rejection parity trap; flagged by all 3
review finders).
- CheckMaskPairing skips hidden files (macOS AppleDouble ._x), mirroring
FilePairingValidator._stems, so a stray ._x.png can't fake a mismatch.
- semseg preflight adds CrossCheckLabels (labels.csv rows -> images/), the same
fail-fast image_classification runs.
- the #214 friendly "needs --label-column" guard now covers semseg (the ingest
schema's allOf requires label for it; the vestigial-looking per-image label
makes the flag easy to forget -> was an opaque schema dump).
- empty mask_id rows reported as "data row N"; graceful empty UnsupportedNote.
Tests (mutation-proven): padded-NA + hidden-file cases, TestBuild_SemanticSegmentation
(masks + schema{mask_id} emission + schema validation), TestSemsegSidecarMirrorsContract
(pins hardcoded masks/*.png/mask_id to the vendored contract). make ci green.
Deferred (noted in PR): mask-resolution/PNG preview (under-rejection); OD's
same pre-existing dotfile gap.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ry path (#248)
- runDataList called push.ListDatasets directly, bypassing the
listDatasetsFn seam its sibling dataset-query callers (destTableExists,
resolveDeleteTarget) already use (data.go:1462). A real consistency bug
that also left the path untestable. Now routes through the seam.
- Split listDatasetsWith(ctx, exec Executor, …) out of ListDatasets so the
exec + error-wrap + parse path is fakeable — it was 0% because
ListDatasets built a real SPDYExecutor internally. Public signature and
the listDatasetsFn seam are unchanged.
- TestListDatasetsWith: happy parse, empty database, and a failing exec
surfacing "querying datasets" + the remote stderr. fakeExecutor gains a
stdoutToReturn field (no impact on the tar-stream callers).
make ci green. Follow-up: the loadClusterFn routing + teardown/stage seams
+ their command-path tests (interlock with the resolveClusterTarget fixture).
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…tial-fail) (#249)
runDataDelete was 22% — every test stopped at cluster discovery, so the
actual DROP TABLE + file removal (unrecoverable) had no coverage. Add two
fn-var test seams (default to the real funcs, zero behavior change),
mirroring loadClusterFn/listDatasetsFn:
- resolveClusterTargetFn: inject a canned target (fake clientset + release
+ PVC) without seeding the k8s objects discoverRelease/DiscoverSharedPVC
look for.
- teardownFn: over push.Teardown.
TestRunDataDelete_Execute drives past discovery and exercises the three
teardown outcomes: clean -> exit 0; table dropped but file removal fails
-> exit 7 + recovery hint; fails before the drop -> exit 7 "teardown
failed". The mixed-case "Churn" input also pins the case-insensitive
resolveDeleteTarget match (backend#1027).
make ci green. Follow-ups: loadClusterFn routing for cluster-info/doctor/
home + stageFn for ingest --overwrite; resolveClusterTarget's own success
tail (needs a k8s-object fixture).
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
)
realProbeEnv was 35% — it called cluster.Load / cluster.NewClientset
directly, so tests could only reach the ownership-gate and load-failure
returns; the release discovery (the core of the bare-`tracebloc` screen)
was dark. Route it through the existing loadClusterFn / newClientsetFn
seam (both default to the real helpers → zero behavior change) so a fake
clientset can drive discovery.
- TestRealProbeEnv_Discovery: live (ready jobs-manager + Ready node ->
localLive + compute), degraded (jobs-manager ReadyReplicas 0 ->
localDegraded), no-release (empty cluster -> localNoRelease). Ownership
gate + load failure stay covered by TestRealProbeEnv_OwnershipGate.
- withClusterSeams now sets RestConfig (realProbeEnv sets
RestConfig.Timeout); additive, its resolveClusterTarget users don't read
it (all pass).
make ci green. Follow-up: runClusterInfo/runClusterDoctor still call
cluster.Load directly — their routing + post-discovery tests next.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…U helpers (#251)
The no-infrastructure "free batch" from the coverage audit (pure logic, no
production changes):
- mapErr (was 0%): Ctrl-C (terminal.InterruptErr) -> errInteractiveCancelled
(the clean-exit-0 seam contract); other errors pass through.
- mapClientErr (was 0%): cancel -> nil; else exit-1 *exitError.
- worseStatus (was 40%): the Fail>Warn>OK verdict truth-table, both orders.
- machineGPUShort / currentGPUCount / defaultGPUChoice (all 0%): the GPU
wizard's pure helpers.
make ci green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…apacity, realSignIn, realRememberedClient (#252)
More of the coverage-audit "free batch" (existing fakes/config fixtures, no
production changes):
- jobsManagerReady (0%): prefixed + ready, zero-replicas, unprefixed legacy
fallback, absent, nil-release.
- machineCapacity (0%): Ready node -> compute; node-list error -> ok=false.
- realSignIn (60%): the signed-in success return (email + first name).
- realRememberedClient (0%): provisioned keys on the cached namespace; the
display name falls back to the client ID.
make ci green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…reak (#253)
Coverage-audit "free batch", no production changes:
- doctor.httpProbe (P0, was 0%): the real proxy-aware connectivity prober
the CLI ships — the checks inject Options.HTTPProbe in tests, so it was
never exercised. Reachable (any status) -> nil; closed host -> error;
unbuildable URL -> error.
- resources.nodeLarger (P1, was 67%): the equal-CPU memory tie-break no
test exercised, so LargestReadyNode determinism on equal-CPU nodes was
unverified.
make ci green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…tegrity) (#254)
The config-fault harness from the coverage audit — no production changes,
root-proof fault injection (parent-is-a-file, path-is-a-directory,
non-empty-dir) plus one chmod case guarded by a non-root euid:
- Load (74%): corrupt JSON -> parse error; an unreadable path (a directory)
-> read error. A garbled/unreadable config must error, never silently
read as "not signed in".
- Save (~65%): un-creatable dir (parent is a file) -> error; read-only dir
-> temp-file creation error (the atomic-write fault, the most
safety-relevant path); fully-empty profiles are pruned (and stay off disk
on reload).
- clearAll (67%): removes an existing config; missing file is a no-op; a
non-empty dir at the path surfaces a remove error.
make ci green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The api transport-error harness from the coverage audit — an errRoundTripper
that fails every request, simulating the most common real-world API failure
the httptest-server tests can't reach (connection refused / DNS / reset, i.e.
HTTP.Do itself erroring). Previously untested in get / bodyRequest.
- get wraps the transport error (errors.Is).
- bodyRequest surfaces it.
- an exported call (WhoAmI) fails on a dead transport rather than reading a
zero-value identity as success.
make ci green. No production changes.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The ui color-matrix harness from the coverage audit. ui was the lowest
own-coverage package (62%) — the merged profile hid it because internal/cli
tests hit the render helpers transitively while asserting only plain
substrings, never the glyph or the color. Now pinned in BOTH color modes:
- CheckLine / CrossLine / WarnLine / Errorf / MenuRow / Infof / PromptHeader
/ PromptHint (all 0% own): glyph + text render, and ANSI escapes appear
iff color is on.
- Para multi-line indent branch (0% own).
ui own-coverage 62% -> 74.7%. No production changes.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The CLI had zero fuzz targets (coverage audit). Fuzz the user-facing
--target-size / --min-size parsers with two invariants that must hold for
ANY input (and the fuzzer proves neither panics):
- success => both dimensions strictly positive;
- error => both dimensions zero (no partial/garbage pair leaks out).
Seed corpus runs under plain `go test`; a 6s local `-fuzz` burst explored
25k+ execs with no failures. No production changes.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* test(cli): cover runClusterInfo/runClusterDoctor post-discovery paths
Route the last two commands that reached a cluster directly — runClusterInfo
(`cluster info`) and runClusterDoctor (`doctor`) — through the existing
loadClusterFn/newClientsetFn seams, and add a doctorRunFn seam over doctor.Run,
so their post-discovery logic is testable without a real kubeconfig or
apiserver. Pure seam routing: the package vars default to the real functions,
so there is no production behavior change.
New coverage (cli package own-coverage 77% -> 79.3%):
- runClusterInfo 95.6%: exit-4 no-client, exit-5 release-found-but-no-token
(proving the install-print block renders before token minting), and all three
arms of the token-expiry switch (server ExpiresAt / requested-only / static-
secret "never"), plus the image-digest configured/not-configured branches.
Pins the security contract: the raw token is never printed, only sha256[:8].
- runClusterDoctor 100%: the clientset-build error arm, the check render loop
(all three status arms + remedy hints), and the overall-verdict switch
(Fail->exit 2 silent / Warn->exit 0 / OK->exit 0), with the auth half stubbed
healthy so the verdict reflects the cluster checks.
make ci green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(cli): pin the server-ExpiresAt arm distinctly (self-review)
Both arms of the token-expiry switch print the "expires in" label, so the
happy-path assertion didn't prove the server-timestamp arm was taken vs the
requested-lifetime fallback. Add a negative assertion that the arm-2 message is
absent when ExpiresAt is set — makes the test resistant to an arm-1/arm-2 swap.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(cli): assert the arm-2 requested-lifetime value (mutation gap)
Gremlins found the arm-2 test asserted only the 'requested; server may cap
shorter' note, not the rendered duration — so a `*`->`/` mutant on
`time.Duration(ExpirationSeconds) * time.Second` (which would display ~0s)
survived. Assert 600s renders as 10m0s. Mutation-proven: the `/` mutant now
fails the test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…red label (#274)
#267 made resolveHomeModel overwrite env.name with the remembered client name
for display. But the offline-vs-no-env classifier reads `env.name != ""` to mean
"the probe surfaced an environment name" — so a leftover ActiveClientName/ID with
no cached namespace (provisioned=false) now masquerades as a reachable
environment and renders Offline + full menu instead of the no-environment
installer path.
Capture the probe's own surfaced name (probeNamedEnv) BEFORE the display-name
override and classify on that: `provisioned || probeNamedEnv`. A remembered
display label with no namespace is no longer evidence of an environment. Adds a
regression test for the leftover-name-without-namespace case.
Fixes the Cursor Bugbot "Remembered name skews offline detection" finding on #266
(learned rule: commands targeting the active client must use bindActiveClientNamespace).
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ae11322. Configure here.

Comment threadinternal/cli/home.go Outdated
LukasWodkaand others added 6 commits July 14, 2026 15:42
First wave-2 batch (post-merge of the coverage sprint), driving the small
self-contained packages up with real, measured tests:
- pathutil: 91.7% -> 100% (ExpandHome's no-home + unknown-~user fallbacks)
- slug: 94.3% -> 97.1% (Derive fallback + collision-truncation branches)
- schema: 84.5% -> 94.4% (unwrap/errors_as helpers; FormatErrors same-Path
tiebreak; flatten nil-guard)
- config: 75.8% -> 93.4% (Current; Profile nil-map; Dir/Path/Load/Save/clearAll
error branches via cleared HOME; Load 2nd-unmarshal; migrateV1 error;
Save rename-onto-dir)
pathutil + slug clear 95%. config + schema plateau just under, blocked ONLY by
by-construction-uncoverable branches: config's atomic-write defensive I/O
(Chmod/Write/Close on a fresh temp file) + the unreachable MarshalIndent check;
schema's NewV1Validator "embedded schema malformed" defense-in-depth (can't
happen — CI drift check + go:embed guarantee it) + ValidateYAML's "jsonschema/v6
always returns a *ValidationError" defensive else. Reaching those means faking
the filesystem / removing idiomatic error handling — deliberately not done.
make ci green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…2) (#270)
- doctor -> 95.6% (over 95): Status.String (was 0%); nodeReady no-condition arm;
getDeployment empty-candidate/missing; jobsManagerEnv valueFrom-skip;
findDeployment ambiguous-unknown-release nil.
- helm -> 91.2% (honest ceiling): supportsResetThenReuse probe (flag-present +
probe-error), repoPresent match/absent, ensureRepo add+update fatal errors,
and Upgrade's ensureRepo-failure abort. Remaining uncovered = Upgrade's
temp-file defensive I/O (CreateTemp/WriteString/Close errors) + the production
exec Runner var — by-construction-uncoverable, left per the ceiling policy.
(nodeboot was already 97.8% — untouched.)
make ci green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…271)
- Newline: the blank-line emitter (was 0%).
- autoColor: NO_COLOR-set, non-*os.File writer, and non-terminal *os.File — all
three "plain output" arms.
- Spinner/run/Stop: both the static (color-off) one-liner and the animated
(color-on) goroutine. The animated test sleeps past one 120ms tick to cover
run's tick arm (frame advance + redraw), then Stop drains the goroutine before
the buffer is read — race-clean under -race.
Package hits 100%.
make ci green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…tor errors) (#272)
* test(push): cover the pure category/family helpers
FamilyNoun/FamilyFromNoun/FamilyNouns/SupportedCategoriesList (were 0%) +
TextSidecarDir happy path. First of several push batches toward 95%
(87.7% -> 88.5%); the package's remaining gap is spread across ~55 functions
(preflight validators, pod/stage lifecycle, orphan/list), a multi-PR grind.
make ci green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(push): cover the CSV-validator file-error paths
HasBOM/ReadCSVHeader/CheckTabularBOM/CheckHasDataRows/CheckCSVEncoding open-error
arms (missing file), ReadCSVHeader empty-file EOF, HasBOM short-file. push
88.5% -> 89.0%.
make ci green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ount (#273)
The integration suite already exercises the real-cluster seams that are 0% in the
unit suite by design — cluster.NewClientset, push.SPDYExecutor.Exec,
submit.PortForwardJobsManager, cluster.DiscoverInClusterClient — but `go test
-cover` (unit only) never credits them, so submit/cluster read artificially low.
Add three targets using the built-in `go tool covdata` (no external merger):
- `cover` — honest own-coverage per package (NO -coverpkg, so no
transitive cross-package inflation).
- `cover-integration`— the integration suite under -coverpkg, so it credits the
internal packages its tests exercise; also the test gate.
- `cover-merge` — unions both data dirs, prints per-package + overall.
Wire all three into e2e.yml's kind job. Verified locally for the unit half
(own-coverage matches baseline: cluster 78.5, submit 80.4, ui 74.7); the
integration half runs on kind in CI — this PR carries the `e2e` label to
exercise it.
make ci green.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* test(api): cover the client endpoints + error paths (77.9% -> 96.3%)
Wave-2 coverage for internal/api, all via httptest stubs + a dead transport:
- APIError.Error(), RevokeClient, ListClientAdmins — were 0%, now covered
(2xx + non-2xx + bad-JSON arms).
- Every endpoint's transport-error arm (post/get returned an error) via an
errRoundTripper client.
- Non-2xx APIError arms + 2xx-undecodable decode arms across RequestDeviceCode,
PollToken, RevokeToken, CreateClient, PatchClientClusterID, ListClients, WhoAmI.
- PollToken edge cases (2xx-missing-token, expired_token -> ErrExpiredToken);
RequestDeviceCode empty-fields; ListClients DRF `next` pagination + nextPath.
Package clears 95% (96.3%). The few remaining branches are the 426-upgrade
detection + nextPath's bad-URL guard + the maxListPages backstop.
make ci green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(api): check the fmt.Fprintf error in the pagination stub (errcheck)
The CI Lint job (errcheck) flagged the unchecked fmt.Fprintf in
TestListClients_Pagination's httptest handler. Assign to _,_ like the sibling
w.Write calls. No behavior change; unblocks #269's Lint check.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…nes (#275)
The chart stamps GPU_REQUESTS/GPU_LIMITS=nvidia.com/gpu=1 as literal container
env on every install, even CPU-only hosts, so ParseTraining reports a phantom
HasGPU=true. `resources show` then printed "per training run: up to N CPU · M
GiB · 1 GPU" on a machine with no GPU — and under --verbose ALSO printed
"gpu: none detected", contradicting itself.
Normalize train.HasGPU to false when the node read succeeded and exposes no GPU
(the same guard already used for the "none detected" detail), mirroring the set
path's phantom-GPU handling (Bugbot #241). A node-read failure leaves it as-is,
since we can't confirm absence. Adds a regression test.
Pre-empts the Cursor Bugbot "phantom GPU" class of finding on #266.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…hed (#277)
The offline-vs-no-env classifier intentionally keeps a provisioned machine with
no cached display name on a NAMED offline line (namespace known, name not). But
renderHome formatted every env line with %q, so that case printed
`Secure environment "" · can't reach it from here` — an empty-quoted name.
Build the env label once: `Secure environment %q` when a name exists, else a
bare `Secure environment`. Every state (Online/Running/Starting/Offline) now
degrades to a clean nameless line. Output is byte-identical when a name is
present (the locked-demo test still passes). Adds a regression test across all
named states.
Fixes the Cursor Bugbot "Empty env name renders badly" finding on #266.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@saadqbal@LukasWodka