refactor(installer): read the training envelope from the contract, stop computing it (backend#2220)

[shujaatTracebloc]

What

Step 2 of 3 for backend#2220 / RFC-BACKEND-664 §P0. client-runtime#358 (merged) made the training-envelope arithmetic single-definition; this makes both installers readers of it instead of producers.

Deleted from install-client-helm.sh and install-k8s.ps1:

• the 1-CPU / 3-GiB overhead
• the 1-core / 2-GiB floor
• the (memory, cpu) node ordering

That last one is a real divergence, not just duplication.cli's nodeLarger ranked nodes (cpu, memory), so on a cluster of 8c/16Gi + 4c/32Gi the installer and tracebloc resources set anchored on different nodes and disagreed about the same machine. Nobody chose that — it fell out of two independent implementations. One order now, the contract's, matching cli. Installer-provisioned clusters are single-node k3d where the orders cannot differ, so the field sees no change.

Why embedded constants and not a lib file

Neither installer can call the Python. bash has no guaranteed JSON parser — jq is not a prerequisite, as the helm-namespace parser at :204 says in as many words — and this bootstrap is signed, so it must not fetch anything unsigned at install time. So the constants are embedded and the embed is drift-guarded: the same trade the GPU node-image build inputs already make (#616/#633).

Deliberately not a new scripts/lib/envelope-contract.sh: install-k8s.ps1 is a single self-contained file and cannot source a bash lib, so a lib would have meant two mechanisms, two guard shapes, and five more sync points on the signed file surface (install.sh FILES, gen-manifest.sh FILES, install.ps1 $Files, WINDOWS_FILES, plus the manifest).

Three ways this can rot, one gate each

Rot	Gate
the embed stops matching the vendored contract	scripts/gen-envelope-embed.sh --check, mirrored in both bats and Pester
the vendored contract stops matching upstream	envelope-contract-drift.yml, against scripts/.client-runtime-ref
the installers stop reproducing the contract's vectors	the golden-vector replays, in the normal test jobs

Pin, don't float — cli/.github/workflows/chart-drift.yml's stated rule: an unrelated client-runtime commit must not redden every open client PR, and the weekly run catches a pin gone stale enough to matter. Pinned to client-runtime@6293d15 (the #358 merge on develop), verified byte-identical to the vendored fixture.

The drift job mints a least-privilege App installation token (repositories: client-runtime, permission-contents: read) because client-runtime is private and GITHUB_TOKEN cannot read another repo — unlike cli's chart-drift, which reads the public tracebloc/client with no token. Scoping follows the backend#2157 sweep.

It also re-derives upstream's vectors from upstream's own code, rather than trusting the file: a vendored contract can match upstream byte-for-byte while upstream's goldens have gone stale against its own arithmetic, and a stale contract reaching us is our problem.

A live bug this turned up

[math]::Max(0, $bestMemB - $overhead) binds the Int32 overload and throws on any byte count over 2³¹ — "Value was either too large or too small for an Int32". The enclosing bare catch {} swallowed it into a silent fall-through to cpu=2,memory=8Gi. My first Pester replay run had every machine returning the literal, which is how it surfaced. Now [long]0.

The catch {} itself is a standing hazard worth its own ticket: any exception on that path silently hands the customer the unschedulable literal with no diagnostic, and PSScriptAnalyzer has been flagging it advisory-only (PSAvoidUsingEmptyCatchBlock at :4073 and :4122). Filed separately rather than widened into here.

Verification

• bats 164 pass. The 3 failures are pre-existing on clean develop — verified by stashing; they need real helm behaviour, the same gap client#751 just closed in CI
• Pester 699 pass / 0 fail, including the 6 pre-existing Get-TrainingResources tests
• make lint (required): 41 scripts parse, shellcheck severity=error clean over 47 files
• PSScriptAnalyzer 0 errors; the 3 warnings in the edited region are all pre-existing
• Guards: gen-manifest 12/12, index-invariants 17/17, check-facts 14/14, leftover-guard 32/32, bats-hygiene green
• Drift detection proven: tampered a constant, --check named the file, the value and the fix; regenerating repaired it
• manifest.sha256 regenerated — both installers' digests moved
• No client/** chart files touched, so no Chart.yaml bump

Next

• PR 3 (cli) — set.go keeps its Decision-A contract verbatim (the number the user sets IS the ceiling, written unchanged), drops the MaxRunCores/MaxRunGiB arithmetic, adds the pinned-ref drift gate, and carries the amendment comment onto cli#143.
• Migration + provenance (scope bullet 4) — there is still no provenance marker in any of the three repos, so "the user chose this" and "the installer wrote this" are indistinguishable once the value differs from cpu=2,memory=8Gi. Needs a sibling key plus an explicit helm-side unset before un-stranding can be safe.
• The sub-8GiB bug — a machine below the floor still falls through to cpu=2,memory=8Gi, which on a 4 GiB box is larger than the box. The accessor now reports viable=False for exactly this case; wiring the installers to refuse honestly is a behavior change and lands on its own so it stays revertable.

Refs: RFC-BACKEND-664 §P0, client-runtime#358, cli#143, #616/#633

🤖 Generated with Claude Code

---

Note

^{Cursor Bugbot is generating a summary for commit cd75433. Configure here.}

[cursor]

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

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

^{Reviewed by Cursor Bugbot for commit 27cad8f. Configure here.}

[saadqbal]

Reviewed this as approve-ready and it is, with one procedural blocker: the Bugbot Medium on Get-TrainingResources is stale — 9bdaaf6b ("skip unparseable nodes in the ps1 twin, don't rank them") is the head commit and fixes exactly what the thread describes. $cpuM/$memB are now $null rather than 0, and if ($null -eq $cpuM -or $null -eq $memB) { continue } skips the node, matching _machine_training_resources and the contract's skipped_nodes. Could you resolve that thread? required_conversation_resolution is on, so it blocks the merge whatever the severity, and I can't approve over an open Bugbot finding even when the code has moved past it. I'll approve on my next pass once it's closed.

On the substance, I exercised the guards rather than reading the table:

• scripts/gen-envelope-embed.sh --check is clean on the tree, and tampering the ps1 constant is caught with the file, the wrong value, the right value and the remedy all named:

  EMBED DRIFT: scripts/install-k8s.ps1 $script:TbEnvelopeOverheadCpuMilli = 999, contract says 1000
  

  That's the right thing to have proven, given the guard's own recorded failure mode was a regen that rewrote one installer while --check still passed — good that 27cad8f5 closed it and that the comment at :68 records why.
• [long]0 at :4123-4124 is the real find here. A bare 0 binding the (Int32, Int32) overload and throwing on any byte count over 2³¹, swallowed by the enclosing bare catch {} into a silent cpu=2,memory=8Gi, is exactly the class of bug that never gets reported — the customer just gets a wrong-sized envelope and no diagnostic. Finding it because every machine returned the literal in a Pester replay is a good argument for the replays existing.

Two things I agree with rather than would change:

• Embedded constants over a lib file.install-k8s.ps1 being self-contained means a shared lib buys you two mechanisms and five more sync points on a signed surface, and the bootstrap must not fetch anything unsigned. The drift gate is the right price to pay instead.
• Pin, don't float on .client-runtime-ref, plus re-deriving upstream's vectors from upstream's code rather than trusting the vendored file. A contract that matches upstream byte-for-byte while upstream's own goldens have gone stale is a genuinely nasty failure mode and it's worth the extra step.

The (memory, cpu) → (cpu, memory) reorder is the part I'd want a second pair of eyes on at some point, but the reasoning holds: it was an unchosen divergence from cli's nodeLarger, the two anchored on different nodes for the same heterogeneous cluster, and installer-provisioned clusters are single-node k3d so the field sees nothing. Fine to land as a no-op that removes a real disagreement.

Also noting the standing catch {} hazard is filed separately rather than widened into here — right call for a PR this size.