feat(resources): stamp resources set as a human choice (backend#2220)

VERSION

@@ -1 +1 @@
-0.10.9
+0.10.10

internal/cli/resources.go

@@ -154,6 +154,11 @@ func renderResources(ctx context.Context, p *ui.Printer, target *clusterTarget)
  } else {
  p.Field("resource env", "(unset — using chart default "+resources.DefaultTraining+")")
  }
+ // backend#2220: who chose the ceiling above. Verbose-only — it answers a
+ // support question ("did someone set this, or did we?"), not one an
+ // operator needs on every run, and the default view should not grow a
+ // line for bookkeeping that never changes the numbers.
+ p.Field("set by", provenanceLine(train.Provenance))
  if nodeErr == nil && len(machine.GPU) == 0 {
  p.Field("gpu", "none detected")
  }
@@ -170,6 +175,24 @@ func renderResources(ctx context.Context, p *ui.Printer, target *clusterTarget)
 	return nil
 }
 
+// provenanceLine renders RESOURCE_PROVENANCE for humans (backend#2220).
+//
+// "unknown" gets an explanation rather than the bare word, because the bare word
+// invites the wrong conclusion. It does not mean something is broken: it means
+// the value predates the marker, and an installer-written size and a deliberate
+// `resources set` are indistinguishable once the value differs from the historic
+// default. That is why it is treated as a human choice and left alone.
+func provenanceLine(provenance string) string {
+	switch provenance {
+	case resources.ProvenanceUser:
+ return "explicitly set (tracebloc resources set)"
+	case resources.ProvenanceInstaller:
+ return "sized to this machine at install time"
+	default:
+ return "unknown — predates provenance tracking, treated as an explicit choice"
+	}
+}
+
 // machineLine renders the machine-capacity value: "8 CPU · 32 GiB" (+ " · 1 GPU"
 // when a device is present).
 func machineLine(m resources.Machine) string {

internal/cli/resources_set.go

@@ -238,19 +238,42 @@ func applyResourcesSet(ctx context.Context, p *ui.Printer, pr prompter, target *
 	// there is nothing for the fit-check to protect. Sizing an actual CHANGE
 	// is still validated below, before anything mutates.
 	ceilingUnchanged := sameCeiling(desired, current)
-	if ceilingUnchanged && !phantomGPU {
+	// backend#2220: an unchanged ceiling is still a HUMAN CHOICE the moment the
+	// operator runs this command, so it must not exit before BuildEnvSpec stamps
+	// RESOURCE_PROVENANCE=user. BuildEnvSpec writes the marker unconditionally
+	// within itself, but this early return could skip it entirely — so an
+	// installer-sized edge whose operator ran `resources set --max`, or passed
+	// flags restating the current ceiling, kept `installer`. That is the state
+	// BuildEnvSpec's own comment calls the most dangerous the marker can be in:
+	// a deliberate choice wearing the one label that invites a future ladder to
+	// overwrite it. Caught by Bugbot and confirmed in review on #539.
+	//
+	// `unknown` counts as stale too: a pre-marker edge whose operator restates
+	// the ceiling has now made that size explicit, and recording it as such is
+	// the honest answer. The cost is one extra apply per edge, exactly once —
+	// the second run sees `user` and is a clean no-op again.
+	staleProvenance := current.Provenance != resources.ProvenanceUser
+	if ceilingUnchanged && !phantomGPU && !staleProvenance {
  p.Newline()
  p.Successf("Each training run already uses up to %s — nothing to change.", perRunSize(desired))
  return nil
 	}
-	if ceilingUnchanged { // phantomGPU == true here
- // CPU/memory budget is unchanged, but this GPU-less machine's cluster
- // still requests a GPU (a stale chart default). Don't treat it as a
- // clean no-op — fall through to persist so BuildEnvSpec's explicit-empty
- // GPU override lands and clears it; otherwise runs stay unschedulable /
- // fall back to CPU while the heartbeat keeps advertising a GPU.
+	if ceilingUnchanged {
+ // Same shape as the phantom-GPU case, and for the same reason: the budget
+ // is unchanged but something else still needs persisting, so this is not
+ // a clean no-op. Both conditions can hold at once, so both report.
  p.Newline()
- p.Infof("Your CPU and memory budget is unchanged — but this machine has no GPU while the cluster still requests one, so I'll clear that stale GPU setting so runs can schedule.")
+ if phantomGPU {
+ // CPU/memory budget is unchanged, but this GPU-less machine's cluster
+ // still requests a GPU (a stale chart default). Don't treat it as a
+ // clean no-op — fall through to persist so BuildEnvSpec's explicit-empty
+ // GPU override lands and clears it; otherwise runs stay unschedulable /
+ // fall back to CPU while the heartbeat keeps advertising a GPU.
+ p.Infof("Your CPU and memory budget is unchanged — but this machine has no GPU while the cluster still requests one, so I'll clear that stale GPU setting so runs can schedule.")
+ }
+ if staleProvenance {
+ p.Infof("Your CPU and memory budget is unchanged — recording it as your explicit choice so it is never resized automatically.")
+ }
 	}
 
 	// (5) Validate + fit-check — ONLY when the ceiling actually CHANGES. An

internal/cli/resources_set_test.go

@@ -170,6 +170,40 @@ func TestSet_ApplyBuildsHelmArgsAndValues(t *testing.T) {
 	}
 }
 
+// TestSet_StampsUserProvenance: the apply carries RESOURCE_PROVENANCE=user all
+// the way into the values helm is given (backend#2220).
+//
+// The unit test on BuildEnvSpec proves the map is right; this proves the map
+// actually reaches helm. Worth having separately because the failure mode is
+// silent: a marker that never lands looks identical to one that landed, and the
+// consequence only shows up much later, when a ladder re-derives a size the
+// operator had chosen on purpose.
+func TestSet_StampsUserProvenance(t *testing.T) {
+	// Start from an edge the INSTALLER marked — the dangerous case. After a
+	// human `resources set`, the label must flip to user; leaving it as
+	// "installer" would advertise a deliberate choice as ours to overwrite.
+	cs := csWith("8", "32Gi", map[string]string{
+ "RESOURCE_LIMITS": "cpu=2,memory=8Gi",
+ "RESOURCE_PROVENANCE": "installer",
+	})
+	out, err := runSet(t, cs, nil, setReq{
+ cores: "4", memory: "16", coresSet: true, memSet: true, dryRun: true, yes: true,
+	})
+	if err != nil {
+ t.Fatalf("dry-run: %v\n%s", err, out)
+	}
+	if !strings.Contains(out, "RESOURCE_PROVENANCE") {
+ t.Errorf("the plan does not mention RESOURCE_PROVENANCE at all:\n%s", out)
+	}
+	if !strings.Contains(out, "user") {
+ t.Errorf("the plan does not stamp the set as a human choice:\n%s", out)
+	}
+	// And the envelope still reflects what was asked for (Decision A).
+	if !strings.Contains(out, "cpu=4,memory=16Gi") {
+ t.Errorf("plan lost the requested ceiling:\n%s", out)
+	}
+}
+
 // TestSet_KeepsUnsetDimension: `set --cores 4` changes CPU only and KEEPS the
 // current 8Gi memory (proven via the dry-run plan's resulting values).
 func TestSet_KeepsUnsetDimension(t *testing.T) {
@@ -206,7 +240,15 @@ func TestSet_MaxUsesWholeMachineMinusOverhead(t *testing.T) {
 // helm upgrade entirely.
 func TestSet_NoOpSkipsApply(t *testing.T) {
 	calls := fakeHelm(t)
-	cs := csWith("8", "32Gi", map[string]string{"RESOURCE_LIMITS": "cpu=4,memory=16Gi"})
+	// RESOURCE_PROVENANCE=user is what makes this a CLEAN no-op (backend#2220):
+	// restating the ceiling on an edge whose size is already recorded as the
+	// operator's choice leaves nothing at all to persist. Without the marker the
+	// command must fall through and stamp it — covered by
+	// TestSet_SameCeilingStampsProvenance below.
+	cs := csWith("8", "32Gi", map[string]string{
+ "RESOURCE_LIMITS": "cpu=4,memory=16Gi",
+ "RESOURCE_PROVENANCE": "user",
+	})
 	out, err := runSet(t, cs, nil, setReq{cores: "4", memory: "16", coresSet: true, memSet: true, yes: true})
 	if err != nil {
  t.Fatalf("no-op: %v", err)
@@ -221,14 +263,70 @@ func TestSet_NoOpSkipsApply(t *testing.T) {
 	}
 }
 
+// TestSet_SameCeilingStampsProvenance: the hole Bugbot found on #539 and
+// saadqbal confirmed — a same-size `resources set` used to return BEFORE
+// BuildEnvSpec, so an installer-sized edge kept RESOURCE_PROVENANCE=installer
+// even though a human had just chosen that size.
+//
+// That is the state BuildEnvSpec's own comment calls the most dangerous the
+// marker can be in: a deliberate choice wearing the one label that invites a
+// future ladder to overwrite it. `resources set` restating the current ceiling
+// IS a human choice, so it must persist.
+func TestSet_SameCeilingStampsProvenance(t *testing.T) {
+	for _, tc := range []struct {
+ name string
+ provenance string
+	}{
+ {"installer-sized edge", "installer"},
+ {"pre-marker edge", ""}, // ParseTraining normalises to unknown
+ {"junk marker", "banana"}, // ...as does anything unrecognised
+	} {
+ t.Run(tc.name, func(t *testing.T) {
+ calls := fakeHelm(t)
+ env := map[string]string{"RESOURCE_LIMITS": "cpu=4,memory=16Gi"}
+ if tc.provenance != "" {
+ env["RESOURCE_PROVENANCE"] = tc.provenance
+ }
+ cs := csWith("8", "32Gi", env)
+ out, err := runSet(t, cs, nil, setReq{
+ cores: "4", memory: "16", coresSet: true, memSet: true, yes: true,
+ })
+ if err != nil {
+ t.Fatalf("same-ceiling set: %v\n%s", err, out)
+ }
+ upgraded := false
+ for _, c := range *calls {
+ if len(c) >= 3 && c[1] == "upgrade" && c[2] != "--help" {
+ upgraded = true
+ }
+ }
+ if !upgraded {
+ t.Errorf("a stale %q marker must NOT be a clean no-op — the apply is what stamps `user`:\n%s",
+ tc.provenance, out)
+ }
+ if strings.Contains(out, "nothing to change") {
+ t.Errorf("must not claim nothing changed while the marker is being corrected:\n%s", out)
+ }
+ if !strings.Contains(out, "explicit choice") {
+ t.Errorf("the reason for the apply should be stated:\n%s", out)
+ }
+ })
+	}
+}
+
 // TestSet_NoOpEvenWhenCurrentNoLongerFits: restating the ceiling that's already
 // applied must stay a clean no-op success even when the machine has SHRUNK under
 // it (smaller Docker Desktop VM, lost node) — the no-op check runs before the
 // fit validation, because leaving things unchanged mutates nothing. An actual
 // change on the same shrunken machine is still fit-checked.
 func TestSet_NoOpEvenWhenCurrentNoLongerFits(t *testing.T) {
 	// Node 4 CPU / 8 GiB, but the cluster already runs with cpu=8,memory=16Gi.
-	cur := map[string]string{"RESOURCE_LIMITS": "cpu=8,memory=16Gi"}
+	// Marked `user` so this stays the clean-no-op case it is testing; the
+	// stale-marker fall-through has its own test (backend#2220).
+	cur := map[string]string{
+ "RESOURCE_LIMITS": "cpu=8,memory=16Gi",
+ "RESOURCE_PROVENANCE": "user",
+	}
 
 	t.Run("flags restating the current ceiling", func(t *testing.T) {
  calls := fakeHelm(t)
@@ -426,7 +524,14 @@ func TestWizard_GPURowOmittedWhenNoGPU(t *testing.T) {
 func TestWizard_LeaveAsIs(t *testing.T) {
 	calls := fakeHelm(t)
 	pr := &fakePrompter{answers: map[string]string{"How much may one training run use?": "Leave it as it is"}}
-	cs := csWith("8", "32Gi", map[string]string{"RESOURCE_LIMITS": "cpu=4,memory=16Gi"})
+	// Marked `user` so "leave it as it is" is the clean no-op this test is about.
+	// On an edge whose size is NOT yet recorded as the operator's choice, picking
+	// "leave it as it is" does still persist — the marker is being corrected, and
+	// TestSet_SameCeilingStampsProvenance covers that (backend#2220).
+	cs := csWith("8", "32Gi", map[string]string{
+ "RESOURCE_LIMITS": "cpu=4,memory=16Gi",
+ "RESOURCE_PROVENANCE": "user",
+	})
 	out, err := runSet(t, cs, pr, setReq{})
 	if err != nil {
  t.Fatalf("wizard leave: %v", err)

internal/cli/testdata/golden/zz-all-strings.golden

@@ -398,6 +398,7 @@ screen. %s/%d are runtime placeholders.
 "Wrote client id + namespace to %s (no new credential — the existing one stands)."
 "You don't have permission to %s in this account."
 "Your CPU and memory budget is unchanged — but this machine has no GPU while the cluster still requests one, so I'll clear that stale GPU setting so runs can schedule."
+"Your CPU and memory budget is unchanged — recording it as your explicit choice so it is never resized automatically."
 "Your active client is not on the cluster your kubeconfig reaches, and no client here is confirmed. Check your kubeconfig context, then run: %s doctor"
 "Your active client is not on the cluster your kubeconfig reaches. To point this machine at the client that IS there: %s client create"
 "Your data is registered as a dataset. View it at https://ai.tracebloc.io/metadata"
@@ -673,6 +674,7 @@ screen. %s/%d are runtime placeholders.
 "service %s/%s has no selector — can't resolve to a Pod for port-forwarding"
 "session: %s"
 "set -e\nrm -rf %q\nmkdir -p %q\n/bin/tar -xf - -C %q\nif [ -e %q ]; then mv %q %q; fi\nif ! mv %q %q; then\n if [ -e %q ]; then mv %q %q; fi\n exit 1\nfi\nrm -rf %q\nfind %q -maxdepth 1 \\( -name %q -o -name %q \\) -mmin +60 -exec rm -rf {} + 2>/dev/null || true"
+"set by"
 "setting up jobs-manager port-forward: %w"
 "sha256[:8]"
 "shared PVC"