Skip to content

ci: add advance-deploy-env caller workflow - #64

Merged
saadqbal merged 1 commit into
developfrom
chore/add-advance-deploy-env-caller
Apr 27, 2026
Merged

ci: add advance-deploy-env caller workflow#64
saadqbal merged 1 commit into
developfrom
chore/add-advance-deploy-env-caller

Conversation

@LukasWodka

Copy link
Copy Markdown
Contributor

Summary

Adds a 9-line caller workflow that triggers the org-wide reusable workflow tracebloc/.github/.github/workflows/advance-deploy-env.yml on push to develop/staging/master/main.

What changes after merge

Every PR merged into one of those branches will auto-update its card on the engineer kanban:

  • merge to developDeploy environment = dev
  • merge to stagingDeploy environment = staging
  • merge to master/mainDeploy environment = prod AND Status = Done

No effect on PRs not on the kanban (those are auto-skipped).

Why a caller pattern

Same as the kanban auto-add workflow we shipped earlier — keep the logic in one place (tracebloc/.github), each repo just calls it. If we change the rules later, one file edit propagates everywhere.

Test plan

  • After merge: open a small test PR, merge it to develop, verify the kanban card flips Deploy environment = dev
  • On next develop → main release: verify the same PR's card flips to Status = Done, Deploy environment = prod

🤖 Generated with Claude Code

Calls the reusable workflow in tracebloc/.github to advance this PR's
Deploy environment field on the engineer kanban when develop/staging/main
receives a merge.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@saadqbal
saadqbal merged commit 3b737cc into developApr 27, 2026
1 check passed
@saadqbal
saadqbal deleted the chore/add-advance-deploy-env-caller branch July 9, 2026 11:41
shujaatTracebloc added a commit that referenced this pull request Aug 20, 2026
…#766)
The generator grepped in one place and rewrote in another, so a single string
had to serve as both an ERE pattern and a Python literal. The PowerShell prefix
was passed with a leading backslash so grep would read the dollar as literal;
that backslash then reached re.escape(), which searched install-k8s.ps1 for a
backslash-dollar assignment that does not exist there.
--check was unaffected, because it never gets past the grep. So the gate looked
healthy while the documented adopt path was broken: a real regen rewrote the
bash installer, died on the first PowerShell constant, and left the two embeds
disagreeing about the same contract -- the exact drift this whole mechanism
exists to prevent. Reproduced before fixing: bash moved to 4294967296, ps1
stayed at 3221225472, exit 1.
Now python3 does the whole job -- find, compare, and rewrite -- against a
LITERAL key, so there is one escaping domain instead of two. Exit codes carry
the outcome back to bash (0 correct/rewritten, 2 no such assignment, 3 present
but wrong under --check), which also drops the leaked backslash from the drift
message.
Why the existing tests missed it, which is the part worth keeping: they only
ever ran --check (a read) or a regen against an already-correct contract, where
every constant short-circuits before the rewrite. The mutating path had no
coverage at all. Two tests added that do have it, both in a scratch copy so they
cannot mutate the repo:
* adopt a genuinely changed contract, assert BOTH installers moved and the
result is self-consistent
* delete a PowerShell constant, assert the generator fails LOUDLY rather than
reporting a healthy embed -- the fail-open shape gen-manifest.sh warns about
in its own empty-surface guard
Verified: 6/6 envelope tests, bats-hygiene 18/18, install-client-helm 162 ok
with the same 3 pre-existing failures clean develop has (#64-66, they need real
helm), make lint clean over 47 files, manifest unchanged (this touches only dev
tooling and tests, neither of which is on the signed file surface).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
shujaatTracebloc added a commit that referenced this pull request Aug 20, 2026
…op computing it (backend#2220) (#766)
* refactor(installer): read the training envelope from the contract, stop computing it (backend#2220)
Both installers computed "how much of this machine may one training run have"
from scratch, with a third copy in cli's set.go and a fourth, disagreeing
policy in client-runtime's node_sizing.py. client-runtime#358 made the
arithmetic single-definition; this makes the installers readers of it.
Deleted from install-client-helm.sh and install-k8s.ps1: the 1-CPU/3-GiB
overhead, the 1-core/2-GiB floor, and — the part nobody had chosen — the
(memory, cpu) node ordering. That last one is a real divergence, not just
duplication: cli's nodeLarger ranked (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. 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.
Neither installer can call the Python: bash has no guaranteed JSON parser (jq
is not a prerequisite — the helm-namespace parser says so 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/*.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.
Three ways that can rot, one gate each:
1. the embed stops matching the vendored contract
-> scripts/gen-envelope-embed.sh --check, mirrored in bats AND Pester
2. the vendored contract stops matching upstream
-> envelope-contract-drift.yml, against scripts/.client-runtime-ref
3. 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 rule): an unrelated
client-runtime commit must not redden every open client PR; the weekly run
catches a pin gone stale. The drift job mints a least-privilege App token
(named repositories, contents:read) because client-runtime is private and
GITHUB_TOKEN cannot read another repo.
Fixes a live bug found while writing the Pester replay: [math]::Max(0, ...)
binds the (Int32, Int32) overload and throws on any byte count over 2^31
("Value was either too large or too small for an Int32"). The enclosing bare
`catch {}` swallowed that into a silent fall-through to cpu=2,memory=8Gi — the
first replay run had EVERY machine returning the literal. Now [long]0. Note the
`catch {}` itself is a standing hazard: 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 widening this PR.
manifest.sha256 regenerated — both installers' digests moved.
Verified: bats 164 pass (the 3 failures are pre-existing on clean develop —
they need real helm, the gap client#751 just closed in CI); Pester 699/0;
`make lint` clean at severity=error over 47 files; PSScriptAnalyzer 0 errors;
gen-manifest/index-invariants/check-facts/leftover-guard/bats-hygiene all green.
Drift detection proven by tampering a constant and repairing it.
No client/** chart files touched, so no Chart.yaml bump.
Refs: RFC-BACKEND-664 P0, client-runtime#358, cli#143
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(envelope-embed): a real regen must rewrite BOTH installers (Bugbot #766)
The generator grepped in one place and rewrote in another, so a single string
had to serve as both an ERE pattern and a Python literal. The PowerShell prefix
was passed with a leading backslash so grep would read the dollar as literal;
that backslash then reached re.escape(), which searched install-k8s.ps1 for a
backslash-dollar assignment that does not exist there.
--check was unaffected, because it never gets past the grep. So the gate looked
healthy while the documented adopt path was broken: a real regen rewrote the
bash installer, died on the first PowerShell constant, and left the two embeds
disagreeing about the same contract -- the exact drift this whole mechanism
exists to prevent. Reproduced before fixing: bash moved to 4294967296, ps1
stayed at 3221225472, exit 1.
Now python3 does the whole job -- find, compare, and rewrite -- against a
LITERAL key, so there is one escaping domain instead of two. Exit codes carry
the outcome back to bash (0 correct/rewritten, 2 no such assignment, 3 present
but wrong under --check), which also drops the leaked backslash from the drift
message.
Why the existing tests missed it, which is the part worth keeping: they only
ever ran --check (a read) or a regen against an already-correct contract, where
every constant short-circuits before the rewrite. The mutating path had no
coverage at all. Two tests added that do have it, both in a scratch copy so they
cannot mutate the repo:
* adopt a genuinely changed contract, assert BOTH installers moved and the
result is self-consistent
* delete a PowerShell constant, assert the generator fails LOUDLY rather than
reporting a healthy embed -- the fail-open shape gen-manifest.sh warns about
in its own empty-surface guard
Verified: 6/6 envelope tests, bats-hygiene 18/18, install-client-helm 162 ok
with the same 3 pre-existing failures clean develop has (#64-66, they need real
helm), make lint clean over 47 files, manifest unchanged (this touches only dev
tooling and tests, neither of which is on the signed file surface).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(installer): skip unparseable nodes in the ps1 twin, don't rank them (Bugbot #766)
Get-TrainingResources coerced an unparseable CPU or memory quantity to 0 and
then ranked the node anyway. The contract's skipped_nodes says such a node is
SKIPPED, and the bash twin has always done that with an explicit `|| continue` —
the ps1 was the odd one out.
The old memory-first order hid it: a memB of 0 could never win the anchor.
Ranking cpu-first, which this PR introduced for parity with cli, exposes it. A
node with a good core count and a memory unit we do not speak takes the anchor,
fails the memory floor, and drops the WHOLE machine to cpu=2,memory=8Gi while a
sibling node was perfectly sizeable. So the reorder was safe on k3d but not on a
heterogeneous BYO cluster.
Reproduced before fixing, as a test: nodes "16 64GB" + "8 32Gi" returned
cpu=2,memory=8Gi where cpu=7,memory=29Gi was correct. $null now marks
unparseable and the node is skipped, matching bash and the contract.
Coverage added on both sides, because the single-node golden vectors could not
have caught this — they only ever feed ONE node, and the bug needs an
unparseable node COMPETING with a valid one:
Pester: unparseable memory / unparseable cpu / all-unparseable
bats: the same two cases against _machine_training_resources, pinning that
the twins agree rather than assuming it
bash needed no change — its test passed on the first run, which is the point of
adding it.
manifest.sha256 regenerated: install-k8s.ps1's digest moved.
Verified: Pester 743 passed / 0 failed / 13 skipped, envelope bats 8/8,
bats-hygiene 18/18, PSScriptAnalyzer 0 errors, make lint clean over 47 files,
gen-envelope-embed --check clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
shujaatTracebloc added a commit that referenced this pull request Aug 20, 2026
Scope bullet 4 -- "migration for edges already pinned by resources set: they
must not be stranded". The blocker was never the migration mechanics, it was
that provenance is currently unknowable, which Lukas called the actual hard part
of the ticket. This adds the marker that makes it knowable from now on, and is
honest about the edges where it never will be.
Why it cannot be inferred: RESOURCE_* has no unset state once helm's
--reset-then-reuse-values has seen it (fleet auto-upgrade CronJob, installer
reconcile, and the CLI's own apply all use it). _existing_training_resources
carries any value forward unless it equals the historic cpu=2,memory=8Gi, and
that is the ONLY discriminator available -- so an installer-written envelope and
a deliberate `tracebloc resources set` are indistinguishable. Without a marker,
future automatic sizing must either strand every pinned edge or silently
overrule operators who chose a size on purpose.
env.RESOURCE_PROVENANCE, three states:
installer sized to this machine at install time
user explicit `resources set`, or a TRACEBLOC_TRAINING_RESOURCES override
unknown carried forward from before this key existed
`unknown` is deliberate, not a placeholder. Guessing `installer` there would
risk overruling a human, so consumers MUST treat unknown as user. Existing edges
report unknown and keep their size until someone opts in explicitly -- offering
that opt-in is a product decision, not something P0 should make silently.
An existing marker is authoritative and is preserved across re-install, in both
installers. Without that, a re-install would quietly downgrade a `user` choice
to `unknown`, and the marker would be worse than useless -- it would decay
toward the ambiguity it exists to remove. Pinned in bats and Pester.
_training_resources is now a wrapper over _resolve_training_size, which returns
the size AND the verdict from one pass. Deriving them separately would mean
either running the bounded cluster probes twice or re-implementing the branch
logic beside itself -- the exact duplication this ticket deletes. All 15
pre-existing size tests pass unmodified.
The marker rides inside the same template gate as the pair it describes: a
provenance marker with no envelope to describe is noise, and gating them
together means one explicit unset clears all three. Also excluded from both
generic env passthrough loops, or it would render twice per container.
MIGRATION.md carries the un-pin recipe, VERIFIED rather than assumed: helm
deletes null-valued keys during value coalescing, so `--set env.RESOURCE_*=null`
drops all three vars and makes the derive path reachable -- confirmed by
rendering the chart both ways (6 occurrences set, 0 with null). That is the
explicit helm-side unset the ticket's trap 2 asks for, and it needs no chart
change. The recipe carries a warning: derivation is gated OFF (backend#2167), so
clearing the keys today returns the edge to the fixed literal, which on a
sub-8GiB machine cannot schedule at all.
Chart 1.9.48 -> 1.9.49, version + appVersion in lockstep. manifest.sha256
regenerated: both installers moved.
Verified: bats 172 ok (same 3 pre-existing failures clean develop has, #64-66 --
they need real helm), 8 new provenance tests; Pester 750 passed / 0 failed, 7
new; helm unittest 480 passed vs 477 on develop, so 3 added and no new failures
among the 5 pre-existing; helm lint clean; PSScriptAnalyzer 0 errors;
gen-envelope-embed --check clean.
Refs: RFC-BACKEND-664 P0, client-runtime#358, client#766, cli#538, backend#2167
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
shujaatTracebloc added a commit that referenced this pull request Aug 20, 2026
Scope bullet 4 -- "migration for edges already pinned by resources set: they
must not be stranded". The blocker was never the migration mechanics, it was
that provenance is currently unknowable, which Lukas called the actual hard part
of the ticket. This adds the marker that makes it knowable from now on, and is
honest about the edges where it never will be.
Why it cannot be inferred: RESOURCE_* has no unset state once helm's
--reset-then-reuse-values has seen it (fleet auto-upgrade CronJob, installer
reconcile, and the CLI's own apply all use it). _existing_training_resources
carries any value forward unless it equals the historic cpu=2,memory=8Gi, and
that is the ONLY discriminator available -- so an installer-written envelope and
a deliberate `tracebloc resources set` are indistinguishable. Without a marker,
future automatic sizing must either strand every pinned edge or silently
overrule operators who chose a size on purpose.
env.RESOURCE_PROVENANCE, three states:
installer sized to this machine at install time
user explicit `resources set`, or a TRACEBLOC_TRAINING_RESOURCES override
unknown carried forward from before this key existed
`unknown` is deliberate, not a placeholder. Guessing `installer` there would
risk overruling a human, so consumers MUST treat unknown as user. Existing edges
report unknown and keep their size until someone opts in explicitly -- offering
that opt-in is a product decision, not something P0 should make silently.
An existing marker is authoritative and is preserved across re-install, in both
installers. Without that, a re-install would quietly downgrade a `user` choice
to `unknown`, and the marker would be worse than useless -- it would decay
toward the ambiguity it exists to remove. Pinned in bats and Pester.
_training_resources is now a wrapper over _resolve_training_size, which returns
the size AND the verdict from one pass. Deriving them separately would mean
either running the bounded cluster probes twice or re-implementing the branch
logic beside itself -- the exact duplication this ticket deletes. All 15
pre-existing size tests pass unmodified.
The marker rides inside the same template gate as the pair it describes: a
provenance marker with no envelope to describe is noise, and gating them
together means one explicit unset clears all three. Also excluded from both
generic env passthrough loops, or it would render twice per container.
MIGRATION.md carries the un-pin recipe, VERIFIED rather than assumed: helm
deletes null-valued keys during value coalescing, so `--set env.RESOURCE_*=null`
drops all three vars and makes the derive path reachable -- confirmed by
rendering the chart both ways (6 occurrences set, 0 with null). That is the
explicit helm-side unset the ticket's trap 2 asks for, and it needs no chart
change. The recipe carries a warning: derivation is gated OFF (backend#2167), so
clearing the keys today returns the edge to the fixed literal, which on a
sub-8GiB machine cannot schedule at all.
Chart 1.9.48 -> 1.9.49, version + appVersion in lockstep. manifest.sha256
regenerated: both installers moved.
Verified: bats 172 ok (same 3 pre-existing failures clean develop has, #64-66 --
they need real helm), 8 new provenance tests; Pester 750 passed / 0 failed, 7
new; helm unittest 480 passed vs 477 on develop, so 3 added and no new failures
among the 5 pre-existing; helm lint clean; PSScriptAnalyzer 0 errors;
gen-envelope-embed --check clean.
Refs: RFC-BACKEND-664 P0, client-runtime#358, client#766, cli#538, backend#2167
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
shujaatTracebloc added a commit that referenced this pull request Aug 20, 2026
…end#2220)
A machine below the training floor was handed cpu=2,memory=8Gi. On a box with
~4 GiB allocatable that envelope is LARGER THAN THE MACHINE, so every training
pod stays Pending forever -- precisely the failure install-k8s.ps1's own header
says the machine sizing exists to remove ("dead on arrival on nodes under 8 GiB,
the WSL2 field case"), recreated by its fallback. And
--reset-then-reuse-values then re-applies that value on every upgrade, forever.
This is not a hypothetical band. preflight.sh hard-fails below 5 GB on Linux and
only WARNS on macOS/Windows (PF_MIN_MEM_GB=5, PF_WARN_MEM_GB=8), while its own
comment notes "16 GiB is needed to train locally (a job's limit is ~8 GiB+)". So
the installer permits exactly the machines whose envelope cannot schedule.
The root cause was that two different situations produced the same answer:
"I cannot read the machine" and "I read it and it is too small" both returned
empty and both fell through to the literal. Now they are separated:
unreadable -> the literal, unchanged. We cannot do better than history,
and warning about machine size would be a fabrication.
below the floor -> the honest remainder (>= 1 core and >= 1 GiB), which FITS.
A run can be scheduled and fail for a REASON instead of
hanging, and the operator is warned.
below even that -> the literal plus a hard warning. cpu=0 is not a training
request, so there is no honest number to write; refusing
the install would break the run-but-do-not-train-locally
case preflight deliberately allows.
The warning lives in the CALLER, never in the resolver: _training_resources and
_training_provenance are captured with $(...) and their tests compare the whole
output, so a warn emitted from the resolver would land inside RESOURCE_LIMITS.
Pinned by a test that captures the value and asserts it is clean.
_machine_training_resources keeps its exact old semantics (empty when non-viable)
so its callers and its golden replay are untouched; the new viability-aware
_machine_training_ceiling sits beside it.
Two expectations CHANGED rather than added, and both encoded the bug:
bats "below-floor machine falls back to the static default"
Pester "below-floor machine falls back to the static default"
Both asserted the unschedulable literal as if it were the contract. Neither loses
coverage -- the unreadable-cluster tests still pin the case where the literal is
genuinely right, which is now a distinct branch rather than the same one.
The Pester golden replay is strengthened, not just repaired. It used to map EVERY
non-viable vector onto the literal, which is what let this bug live inside a
passing replay; it now checks them against the contract's own render_gi. Those
numbers already matched: wsl2-too-small-4gi renders 3c/1Gi, which is exactly what
the installer now writes. The accessor and the installer agree on small machines
too. (The bats replay still exercises the viable-only wrapper, by design -- its
flattened vector table carries no remainder for non-viable rows; the targeted
bats tests cover those instead.)
Verified: bats 176 ok (+4) with the same 3 pre-existing failures clean develop
has (#64-66, they need real helm); Pester 754 passed / 0 failed (+4); PSSA 0
errors; make lint clean over 47 files; gen-envelope-embed --check clean;
manifest regenerated. No client/** chart files touched, so no Chart bump beyond
the 1.9.49 this branch already carries.
Refs: RFC-BACKEND-664 P0, client-runtime#358, client#766, client#768, backend#2167
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
shujaatTracebloc added a commit that referenced this pull request Aug 20, 2026
Scope bullet 4 -- "migration for edges already pinned by resources set: they
must not be stranded". The blocker was never the migration mechanics, it was
that provenance is currently unknowable, which Lukas called the actual hard part
of the ticket. This adds the marker that makes it knowable from now on, and is
honest about the edges where it never will be.
Why it cannot be inferred: RESOURCE_* has no unset state once helm's
--reset-then-reuse-values has seen it (fleet auto-upgrade CronJob, installer
reconcile, and the CLI's own apply all use it). _existing_training_resources
carries any value forward unless it equals the historic cpu=2,memory=8Gi, and
that is the ONLY discriminator available -- so an installer-written envelope and
a deliberate `tracebloc resources set` are indistinguishable. Without a marker,
future automatic sizing must either strand every pinned edge or silently
overrule operators who chose a size on purpose.
env.RESOURCE_PROVENANCE, three states:
installer sized to this machine at install time
user explicit `resources set`, or a TRACEBLOC_TRAINING_RESOURCES override
unknown carried forward from before this key existed
`unknown` is deliberate, not a placeholder. Guessing `installer` there would
risk overruling a human, so consumers MUST treat unknown as user. Existing edges
report unknown and keep their size until someone opts in explicitly -- offering
that opt-in is a product decision, not something P0 should make silently.
An existing marker is authoritative and is preserved across re-install, in both
installers. Without that, a re-install would quietly downgrade a `user` choice
to `unknown`, and the marker would be worse than useless -- it would decay
toward the ambiguity it exists to remove. Pinned in bats and Pester.
_training_resources is now a wrapper over _resolve_training_size, which returns
the size AND the verdict from one pass. Deriving them separately would mean
either running the bounded cluster probes twice or re-implementing the branch
logic beside itself -- the exact duplication this ticket deletes. All 15
pre-existing size tests pass unmodified.
The marker rides inside the same template gate as the pair it describes: a
provenance marker with no envelope to describe is noise, and gating them
together means one explicit unset clears all three. Also excluded from both
generic env passthrough loops, or it would render twice per container.
MIGRATION.md carries the un-pin recipe, VERIFIED rather than assumed: helm
deletes null-valued keys during value coalescing, so `--set env.RESOURCE_*=null`
drops all three vars and makes the derive path reachable -- confirmed by
rendering the chart both ways (6 occurrences set, 0 with null). That is the
explicit helm-side unset the ticket's trap 2 asks for, and it needs no chart
change. The recipe carries a warning: derivation is gated OFF (backend#2167), so
clearing the keys today returns the edge to the fixed literal, which on a
sub-8GiB machine cannot schedule at all.
Chart 1.9.48 -> 1.9.49, version + appVersion in lockstep. manifest.sha256
regenerated: both installers moved.
Verified: bats 172 ok (same 3 pre-existing failures clean develop has, #64-66 --
they need real helm), 8 new provenance tests; Pester 750 passed / 0 failed, 7
new; helm unittest 480 passed vs 477 on develop, so 3 added and no new failures
among the 5 pre-existing; helm lint clean; PSScriptAnalyzer 0 errors;
gen-envelope-embed --check clean.
Refs: RFC-BACKEND-664 P0, client-runtime#358, client#766, cli#538, backend#2167
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
shujaatTracebloc added a commit that referenced this pull request Aug 20, 2026
…#768)
* feat(envelope): record WHO chose the training envelope (backend#2220)
Scope bullet 4 -- "migration for edges already pinned by resources set: they
must not be stranded". The blocker was never the migration mechanics, it was
that provenance is currently unknowable, which Lukas called the actual hard part
of the ticket. This adds the marker that makes it knowable from now on, and is
honest about the edges where it never will be.
Why it cannot be inferred: RESOURCE_* has no unset state once helm's
--reset-then-reuse-values has seen it (fleet auto-upgrade CronJob, installer
reconcile, and the CLI's own apply all use it). _existing_training_resources
carries any value forward unless it equals the historic cpu=2,memory=8Gi, and
that is the ONLY discriminator available -- so an installer-written envelope and
a deliberate `tracebloc resources set` are indistinguishable. Without a marker,
future automatic sizing must either strand every pinned edge or silently
overrule operators who chose a size on purpose.
env.RESOURCE_PROVENANCE, three states:
installer sized to this machine at install time
user explicit `resources set`, or a TRACEBLOC_TRAINING_RESOURCES override
unknown carried forward from before this key existed
`unknown` is deliberate, not a placeholder. Guessing `installer` there would
risk overruling a human, so consumers MUST treat unknown as user. Existing edges
report unknown and keep their size until someone opts in explicitly -- offering
that opt-in is a product decision, not something P0 should make silently.
An existing marker is authoritative and is preserved across re-install, in both
installers. Without that, a re-install would quietly downgrade a `user` choice
to `unknown`, and the marker would be worse than useless -- it would decay
toward the ambiguity it exists to remove. Pinned in bats and Pester.
_training_resources is now a wrapper over _resolve_training_size, which returns
the size AND the verdict from one pass. Deriving them separately would mean
either running the bounded cluster probes twice or re-implementing the branch
logic beside itself -- the exact duplication this ticket deletes. All 15
pre-existing size tests pass unmodified.
The marker rides inside the same template gate as the pair it describes: a
provenance marker with no envelope to describe is noise, and gating them
together means one explicit unset clears all three. Also excluded from both
generic env passthrough loops, or it would render twice per container.
MIGRATION.md carries the un-pin recipe, VERIFIED rather than assumed: helm
deletes null-valued keys during value coalescing, so `--set env.RESOURCE_*=null`
drops all three vars and makes the derive path reachable -- confirmed by
rendering the chart both ways (6 occurrences set, 0 with null). That is the
explicit helm-side unset the ticket's trap 2 asks for, and it needs no chart
change. The recipe carries a warning: derivation is gated OFF (backend#2167), so
clearing the keys today returns the edge to the fixed literal, which on a
sub-8GiB machine cannot schedule at all.
Chart 1.9.48 -> 1.9.49, version + appVersion in lockstep. manifest.sha256
regenerated: both installers moved.
Verified: bats 172 ok (same 3 pre-existing failures clean develop has, #64-66 --
they need real helm), 8 new provenance tests; Pester 750 passed / 0 failed, 7
new; helm unittest 480 passed vs 477 on develop, so 3 added and no new failures
among the 5 pre-existing; helm lint clean; PSScriptAnalyzer 0 errors;
gen-envelope-embed --check clean.
Refs: RFC-BACKEND-664 P0, client-runtime#358, client#766, cli#538, backend#2167
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(chart): bump to 1.9.50 — 1.9.49 was taken by #767
The chart-version guard failed against the new base: client#767 landed on
develop and claimed 1.9.49 while this branch was open, so relative to that base
this PR bumped nothing.
Same collision shape as two migrations picking the same number off one parent —
the version has to be re-checked after develop advances, not chosen once when
the branch is cut. Rebased onto develop (3c40ef9 -> 16adfc7) and moved to
1.9.50, version + appVersion in lockstep.
Verified after the rebase: my provenance env block and #767's GPU_LIMITS gate
coexist cleanly in jobs-manager-deployment.yaml; helm lint clean; helm unittest
486 passed vs 477 on develop with the SAME 5 pre-existing failures and 5 errors,
so 9 added and none broken; embed check, manifest check, chart-version-guard
23/23 and the provenance/envelope/training-size bats all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(installer): one shared lookup for size + provenance (Bugbot + review, #768)
Bugbot's Medium, confirmed by @saadqbal, and it was a fresh instance of the
exact class client#766 exists to remove: the two installers diverging by
construction, same file pair, days later, in the fail-unsafe direction.
Get-TrainingProvenance performed its OWN `helm get values`, separate from the one
Get-TrainingResources did immediately below, each wrapped in its own bare
`catch {}`. So on Windows, if the size read succeeded and carried a live
RESOURCE_LIMITS but the provenance read then threw -- a wedged API, a
ConvertFrom-Json hiccup, anything the catch eats -- the generated values pinned
that carried envelope as `installer`. A future ladder trusting that label would
re-derive and overrule what may well have been a deliberate human choice.
`unknown` is the safe verdict there, and the bash twin already gave it.
Fixed by adopting the bash shape rather than patching the symptom:
Get-CarriedTrainingValues does the lookup ONCE and returns @{Size; Provenance}
or $null. That $null is the point -- either the read succeeds and both resolvers
see the same carried pair, or it fails and NEITHER takes the carry path, so the
size is machine-derived and `installer` is then correct. Size and provenance can
no longer disagree, structurally, rather than by two functions happening to
agree.
The values generation now does that one lookup and hands it to both via
-Carried/-CarriedResolved, which also drops the redundant `helm get values` per
install that @saadqbal noted. Both params are optional, so every existing caller
and test that invokes the resolvers bare is unaffected.
Four Pester tests added for the fail-unsafe path specifically: a values read that
returns unparseable JSON must carry nothing and answer `installer` (not pin a
carried size as ours); one lookup handed to both resolvers must agree; the
historic literal is still treated as the absence of a choice even with a marker
beside it; an unreadable namespace carries nothing without throwing.
Also rebased onto develop to clear the DIRTY merge state. The only conflict was
scripts/manifest.sha256 -- a GENERATED file, so it was regenerated rather than
hand-merged. Chart stays at 1.9.50, still ahead of develop's 1.9.49.
Verified: Pester 754 passed / 0 failed (+4); bats 175 ok / 0 failed -- develop's
#759 fixed the three helm-dependent tests that were failing before, so this
branch is now clean rather than clean-except-three; helm lint clean; helm
unittest 486 passed with the same 5 pre-existing failures develop has;
PSScriptAnalyzer 0 errors; make lint clean over 47 files; embed + manifest checks
clean.
Refs: backend#2220, client#766, cli#539
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(installer): never write an envelope larger than the machine (backend#2220) (#769)
A machine below the training floor was handed cpu=2,memory=8Gi. On a box with
~4 GiB allocatable that envelope is LARGER THAN THE MACHINE, so every training
pod stays Pending forever -- precisely the failure install-k8s.ps1's own header
says the machine sizing exists to remove ("dead on arrival on nodes under 8 GiB,
the WSL2 field case"), recreated by its own fallback. And
--reset-then-reuse-values then re-applies that value on every upgrade, forever.
Not a hypothetical band: preflight hard-fails below 5 GB on Linux and only WARNS
on macOS/Windows (PF_MIN_MEM_GB=5, PF_WARN_MEM_GB=8), while its own comment notes
"16 GiB is needed to train locally (a job's limit is ~8 GiB+)". The permitted
band and the unschedulable band overlap.
Root cause was that two situations produced one answer: "I cannot read the
machine" and "I read it and it is too small" both returned empty and both fell
through to the literal. Now separated:
unreadable -> the literal, unchanged. We cannot do better, and warning
about machine size would be a fabrication.
below the floor -> the honest remainder (>= 1 core and >= 1 GiB), which FITS.
A run can be scheduled and fail for a REASON instead of
hanging, and the operator is warned.
below even that -> the literal plus a hard warning. cpu=0 is not a training
request, and refusing the install would break the
run-but-do-not-train-locally case preflight allows.
The warning lives in the CALLER, never in the resolver: _training_resources and
_training_provenance are captured with $(...) and their tests compare the whole
output, so a warn emitted from the resolver would land inside RESOURCE_LIMITS.
Pinned by a test that captures the value and asserts it is clean.
_machine_training_resources keeps its exact old semantics (empty when
non-viable), so its callers and its golden replay are untouched; the new
viability-aware _machine_training_ceiling sits beside it.
Two expectations CHANGED rather than added, and both encoded the bug -- the bats
and Pester "below-floor machine falls back to the static default" tests each
asserted the unschedulable literal as if it were the contract. Neither loses
coverage: the unreadable-cluster tests still pin the case where the literal is
genuinely right, now a distinct branch rather than the same one.
The Pester golden replay is strengthened, not just repaired. It mapped EVERY
non-viable vector onto the literal, which is what let this bug live inside a
passing replay; it now checks them against the contract's own render_gi. Those
numbers already matched -- wsl2-too-small-4gi renders 3c/1Gi, exactly what the
installer now writes -- so the accessor and the installer agree on small machines
too, and that agreement is asserted.
Rebuilt on top of the current #768 rather than merged into it. #768 has since
taken review fixes and a develop merge that restructured the same two sizing
functions, so a replay produced seven conflict hunks across both installers --
the serialize-don't-parallelize hazard the ticket itself warns about. Re-applying
this self-contained delta was safer than hand-resolving them; the test files
applied from the original patch unchanged.
Verified: bats 179 ok / 0 failed (develop's #759 fixed the three helm-dependent
tests that used to fail); Pester 758 passed / 0 failed; manifest + envelope-embed
checks clean.
Refs: RFC-BACKEND-664 P0, client-runtime#358, client#766, client#768, backend#2167
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(installer): one shared lookup in the bash twin too (Bugbot #768)
I fixed this on the PowerShell side last push and left the bash side split --
which is precisely the two-installers-diverge class client#766 exists to remove.
Bugbot caught the half I missed, and its failure mode is the MIRROR of the ps1
one:
ps1 a failed provenance read reported `installer` for a carried size,
inviting a future ladder to overrule a human choice
bash a failed provenance read reported `unknown`, which consumers treat as a
human pin -- so an installer-sized edge was PERMANENTLY STRANDED as a
deliberate choice
The second is worse, and it is the exact outcome scope bullet 4 exists to
prevent: stranding is the defect, and a fail-unsafe read was manufacturing it.
_existing_training_values now does ONE `helm get values` and echoes
"<size>|<provenance>", or nothing. Nothing is the important case: a failed or
empty read carries NOTHING, so the caller machine-sizes and `installer` is then
the correct verdict, keeping the edge eligible for a future ladder. Provenance
is normalised in that one place -- anything unrecognised, including absent, is
`unknown`, never a guess.
_existing_training_resources and _existing_training_provenance are now thin
readers over it, so they cannot disagree, and _resolve_training_size takes both
fields from a single call. All 15 pre-existing size/provenance tests pass
UNMODIFIED, and the sizing path is down to exactly one `helm get values` (the
other in the file is the unrelated reconcile path).
Five tests added on the fail-unsafe path specifically: a failed read and an empty
read must each carry nothing; a failed read must leave the verdict at `installer`
rather than a stranding `unknown`; one lookup returns both fields together; and a
carried size with genuinely no marker still pairs with `unknown` -- because
`unknown` is correct when the read SUCCEEDS and there is no marker. The bug was
reporting it when the read failed.
Verified: bats 180 ok / 0 failed (+5), bats-hygiene 18/18, shellcheck -S warning
-x clean on the edited script, manifest + envelope-embed checks clean.
Refs: backend#2220, client#766, client#769
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(manifest): regenerate for the shared-lookup change
The digest for scripts/lib/install-client-helm.sh was stale in 1d0b72d: I
regenerated the manifest in the working tree AFTER committing, then reverted
that working-tree change instead of committing it. The rebase had resolved the
manifest conflict with --ours, which is never right for a generated file.
Caught by running gen-manifest.sh --check against the pushed tree rather than
trusting the earlier "up to date" line, which had been printed after a
regeneration that never made it into a commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(installer): only call a machine too small once we've measured it (Bugbot #768)
Third instance of the same divergence class in this PR, and this one I wrote into
my own commit message and then contradicted in the code: "warning about machine
size would be a fabrication" -- while the code fabricated exactly that.
_TB_TRAINING_UNSCHEDULABLE was INFERRED by re-probing `kubectl get nodes -o name`
whenever the ceiling helper returned nothing. But the helper returns nothing for
three different reasons, and only one of them is "too small": no kubectl, an
unreadable/failed node read, nothing parseable, OR a parsed node whose remainder
is not a requestable shape. So any listable node -- one whose allocatable would
not parse, or one not Ready yet -- tripped a hard "this machine is too small"
warning plus the cpu=2,memory=8Gi literal, about a machine we had never
successfully measured. The PowerShell twin only flagged it after a PARSED node
($seen), so the two installers disagreed on the same cluster.
Fixed at the source rather than at the caller: _machine_training_ceiling now
returns "|unschedulable" as its own verdict when it HAS parsed a node and the
remainder is not requestable, and stays silent only when it genuinely could not
measure. Silence now means one thing -- "I could not read the cluster" -- and the
resolver keys off the verdict instead of guessing from a second probe. That also
drops the redundant `kubectl get nodes` call.
Three tests, the first of which is the bug: readable-but-unparseable nodes must
NOT be called too small; the helper reports |unschedulable only after measuring;
the helper stays silent when nothing parses. The existing "too small for even a
1c/1Gi run" test still passes -- that node parses, so it is a measurement.
Verified: bats 187 ok / 0 failed (+3), bats-hygiene 18/18, shellcheck -S warning
-x clean, manifest + envelope-embed checks clean.
Refs: backend#2220, client#766
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <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

@LukasWodka@saadqbal