release-train: staging -> main - #953
Merged
Merged
Conversation
…s (backend#2910) (#939) * fix(chart): guard every hostPath subkey read so --reuse-values renders (backend#2910) `client/templates` read `.Values.hostPath.<key>` unguarded in nine places. Under `helm upgrade --reuse-values` from a release predating the `hostPath` key, `.Values.hostPath` is nil and the first such read nil-pointers, killing the whole render (reproduced: `shared-images-pvc.yaml:3`). Route each read through the chart's own convention `(default dict .Values.hostPath).<key>` (already used at six other sites). The three ternary sites also get `| default false` so the nil subkey coerces to the bool `ternary` requires. Add scripts/tests/hostpath-reads-guarded.sh to the required `drift` gate. It DERIVES the site list from the tree — greps client/templates for the literal `.Values.hostPath.`, the one signature a guarded read `(default dict .Values.hostPath).enabled` cannot contain — so a new unguarded read is caught without restating the nine paths (CLAUDE.md rule 1), and un-guarding any site reddens it. Fails closed if the hostPath surface disappears. Bump Chart.yaml 1.9.89 -> 1.9.90 (chart-version-guard). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(chart): bump to 1.9.91 above develop after merge (backend#2910) develop advanced to 1.9.90 (#929) while this branch was open; the identical bump auto-merged, so the chart-version-guard saw no net change. Bump one patch above develop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…py it into the machine-wide tools dir (backend#2915) (#937) * fix(installer): persist the Windows CLI dir to the Machine PATH for fresh shells (backend#2904) The tracebloc CLI's own installer PATH-adds its bin dir at USER scope only. A fresh, non-interactive shell that sources no profile -- the SSM session the Windows e2e opens for its cli-windows step, which need not even run as the installing user -- never sees that entry, so `tracebloc` is not on PATH there even though every other client tool (installed to %ProgramFiles%\tracebloc\bin on the MACHINE PATH) is. After a successful CLI install, persist the CLI's bin dir onto the MACHINE PATH through one dedup-correct helper (Add-DirToMachinePath), and route Initialize-ToolDir through the same helper -- replacing its substring dedup, which both false-matched a prefix dir and mis-parsed a '[' in a path, and could append a ';;' empty (== current-directory) PATH entry. RefreshPath mirrors the value into the running process so the verify step sees it immediately. Pester covers the append/dedup/idempotency logic and the install wiring on Linux CI (the .NET Machine-scope setter is a no-op off-Windows, so the registry is simulated via mockable Get/Set-MachinePath wrappers); the self-hosted Windows e2e exercises the real path via Initialize-ToolDir. Part of tracebloc/backend#2904 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(installer): regen manifest.sha256 for the install-k8s.ps1 change (backend#2904) The R8 signed-installer manifest pins a SHA256 over each sub-script the bootstrap verifies before running privileged steps; editing install-k8s.ps1 changes its digest, so `make drift` (gen-manifest.sh --check) fails until the manifest is regenerated and committed. Part of tracebloc/backend#2904 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(installer): make the CLI-persist-failure test bite (backend#2904) The persist-failure test asserted only `Should -Not -Throw`, which the function-wide catch in Install-TraceblocCli already guarantees — so it passed even with the load-bearing inner Add-DirToMachinePath try/catch deleted (Bugbot). Assert the discriminating behavior instead: a persist throw must be CONTAINED so Test-TraceblocCli still runs and the CLI is reported installed, not bounced to the function-wide catch that misreports the successful CLI install as failed. Verified by mutation: removing the inner try/catch now fails this test. Part of tracebloc/backend#2904 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): copy the CLI into the admin-only tools dir, not LOCALAPPDATA on the Machine PATH (backend#2915) Review (saadqbal): adding %LOCALAPPDATA%\Programs\tracebloc to the Machine PATH is wrong twice over. It's a per-user, user-writable dir on the system search path (CWE-426 — a non-admin plants an exe an elevated process resolves unqualified), and in the elevate-to-a-different-admin flow it's the admin's profile, unreadable by the daily user, so the CLI is still unresolvable — the very case this was meant to fix. Instead, on a successful CLI install, COPY the exe into $TOOL_DIR (%ProgramFiles%\tracebloc\bin) — admin-only and already on the Machine PATH from Initialize-ToolDir — via Publish-TraceblocCliToToolDir. Resolvable machine-wide for any user, and no new PATH entry. Not a shim: a tb.cmd-style shim would bake the admin's LOCALAPPDATA path and reintroduce the unreadable-profile problem. Also (review): Add-DirToMachinePath's guard now rejects whitespace ([string]::IsNullOrWhiteSpace), matching Test-DirOnPath's trim — a ' ' dir no longer slips through to append a junk PATH entry. Add-DirToMachinePath stays the shared Machine-PATH helper for Initialize-ToolDir (exact per-entry dedup, ;;-collapse). Manifest regenerated for the install-k8s.ps1 change. Closes tracebloc/backend#2915 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): make a failed CLI publish loud, and gate the "ready" verdict on the machine-wide copy (backend#2915) Bugbot: Copy-Item/New-Item raise NON-terminating errors under the installer's default $ErrorActionPreference='Continue', so the try/catch around Publish-TraceblocCliToToolDir never caught a failed copy — and Test-TraceblocCli could still print "ready" off the CLI installer's User-scope LOCALAPPDATA entry even though $TOOL_DIR never received tracebloc.exe, leaving a fresh non-interactive shell unable to resolve it. * Publish-TraceblocCliToToolDir: -ErrorAction Stop on Copy-Item/New-Item + a post-copy Test-Path, so a failure THROWS and is logged by the caller. * Test-TraceblocCli: the "ready" verdict now requires the MACHINE-WIDE artifact ($TOOL_DIR\tracebloc.exe, on the Machine PATH), not just `Has tracebloc`. When the CLI resolves only via the installing user's own User PATH, it says so honestly ("installed for you, but not machine-wide") instead of a false ready. Tests: publish now propagates a copy failure and throws on a missing artifact; Test-TraceblocCli asserts the machine-wide gate and the honest user-only branch. Manifest regenerated. Part of tracebloc/backend#2915 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): gate the fast path on the machine-wide CLI copy so re-running repairs it (backend#2915) Bugbot: Test-TraceblocCli now treats the machine-wide $TOOL_DIR\tracebloc.exe as the "ready" condition, but the fast nothing-to-do path still gated only on Test-TraceblocCliCurrent — which returns true for a User-PATH-only CLI. A machine that completed a prior install with the CLI on the USER PATH only (an older installer, or a copy that failed) would shortcut past Install-TraceblocCli and never run Publish-TraceblocCliToToolDir, so re-running the installer — the documented repair — left a fresh/other-user shell unable to resolve tracebloc. Add Test-TraceblocCliMachineWide (is $TOOL_DIR\tracebloc.exe present?) and require it in the fast-path gate alongside Test-TraceblocCliCurrent, so a completed-but-User-only machine falls through and the copy is placed. Wiring test + unit tests added; manifest regenerated. Part of tracebloc/backend#2915 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): keep the machine-wide CLI copy fresh so it can't shadow an update (backend#2915) Bugbot: the $TOOL_DIR copy sits on the Machine PATH, searched BEFORE the CLI installer's updatable %LOCALAPPDATA% copy. Once the snapshot was at/above the 0.10.0 floor the fast path never republished, so a later `irm <cli>/install.ps1 | iex` or a CLI self-update refreshed only %LOCALAPPDATA% while `tracebloc` kept running the stale machine snapshot. * Publish-TraceblocCliToToolDir now SKIPS the copy when the machine copy already matches the source (SHA256), and re-copies when it differs (or the compare fails). Being a cheap no-op when in sync makes it safe to call on every run. * The fast nothing-to-do path now calls Publish-TraceblocCliToToolDir among its other idempotent "re-run is a real remedy" repairs, so an out-of-band update to %LOCALAPPDATA% is picked up on the next installer run instead of being shadowed indefinitely. Tests: publish skips-in-sync / re-copies-when-stale / re-copies-on-compare-error; a wiring assertion that the fast path calls Publish. Manifest regenerated. Part of tracebloc/backend#2915 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): make the machine-copy refresh no-downgrade, and give the -ErrorAction Stop guard a biting test (backend#2915) @LukasWodka, two non-blocking review findings: 1. The staleness refresh was direction-blind — Publish-TraceblocCliToToolDir copied on ANY hash difference, so a %LOCALAPPDATA% holding an OLDER build (a pinned CLI or a partially-failed reinstall) would let the fast path silently DOWNGRADE the machine-wide CLI for every user. Now directional: new Get-TraceblocExeVersion, and the refresh copies only when the machine copy is MISSING or the source is a strictly NEWER version (identical/older/unknown leaves it in place). Tests discriminate the downgrade case (mutation-verified: a direction-blind mutant reddens). 2. The -ErrorAction Stop guard test was inert — it mocked Copy-Item { throw }, and a thrown error is terminating regardless of -ErrorAction, so deleting the flag left the suite green. The test now emits a NON-terminating error via Write-Error, so it only throws when -ErrorAction Stop promotes it (mutation-verified: removing the flag reddens). Manifest regenerated. Part of tracebloc/backend#2915 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): repair a broken machine CLI copy instead of refusing on an unreadable version (backend#2915) Bugbot: the no-downgrade guard from the previous commit was too broad. When the machine-wide tracebloc.exe exists but won't report a version (corrupt / wrong-arch), Publish refused to replace it — and since Test-TraceblocCliMachineWide only checks presence and Test-TraceblocCliCurrent fails open on an unreadable version, the fast path printed nothing-to-do and re-running never repaired the corrupt snapshot that shadows every user's CLI. The two "unknown version" sides are NOT symmetric: * SOURCE version unreadable -> can't vouch for it -> keep the machine copy (a partially-failed reinstall must not clobber a working CLI), but * source READABLE, DEST version unreadable -> the machine copy is broken and the source is known-good -> REPAIR it (copy). Both readable still copies only toward a strictly newer version (no downgrade). Tests split into the two asymmetric cases (source-unreadable keeps; dest-unreadable repairs; both-unreadable keeps); mutation-verified the repair path bites. Manifest regenerated. Part of tracebloc/backend#2915 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): honest hint when the CLI isn't machine-wide — not "re-run as Administrator" (backend#2915) Bugbot (Low): the "installed for you, but not machine-wide" branch told the operator to re-run as Administrator. The installer has already self-elevated, so another run hits the same no-op — the machine-wide copy is absent because the copy FAILED (in the log) or a custom INSTALL_PREFIX put the CLI where Publish never sees it. Name the real causes and point at the log instead of a fix that can't work. Test asserts the hint no longer says "as Administrator" and names the real cause. Part of tracebloc/backend#2915 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): the machine-wide ready verdict names `tracebloc`, not the per-user `tb` (backend#2915) Bugbot: the machine-wide "ready" branch preferred `tb`, but `Has tb` is true only because RefreshPath pulled in the installing user's User PATH, where the CLI installer dropped its per-user `tb.cmd` shim. We copy only tracebloc.exe into $TOOL_DIR, so a fresh or other-user shell — exactly what this verdict is about — has `tracebloc` on the Machine PATH but no `tb`. Naming `tb` promises a command that won't resolve in the shell the message says works. Name the machine-wide command, `tracebloc`. Test asserts the ready verdict says run 'tracebloc' and never run 'tb'. Part of tracebloc/backend#2915 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ckend#2795) (#947) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…n, and check that it took (backend#2931) (#945) * sec(installer): set the values-file mode BEFORE the credential goes in, and check that it took (backend#2931) The generated $HOST_DATA_DIR/values.yaml holds clientId and clientPassword in cleartext on every operator and customer machine. Two defects, same line. ORDERING. The heredoc created the file at the PROCESS umask and the chmod ran after its EOF -- so under any umask but 077 the file existed world-readable for the duration of the write, with the credential already in it. A mode fixed one line too late is not a mode. It now creates the file EMPTY and 0600 first; `>` on an existing file truncates without touching its mode, so the heredoc writes into a file that is already 0600. The remaining window is on a 0-byte file. RIGHT BY ACCIDENT, AND UNTESTED. common.sh sets `umask 077`, so today the mode happens to be 0600 and the chmod is a no-op -- but the installer does not hold 077 everywhere (_install_userspace_tools sets 022, the trap telemetry.bats already documents for the spool), and nothing asserted the mode. The telemetry spool has such a test; the file holding a credential had none. FAIL-OPEN. `chmod 600 ... 2>/dev/null || true` could not fail: a chmod that did not apply left the credential readable and said nothing. It now reads the mode back -- POSIX `ls -ldn`, never GNU-only `stat -c`, which BSD rejects silently (hostpath-prep.bats:181) -- and WARNS naming the mode found. Warn, not error: chmod genuinely cannot apply on a HOST_DATA_DIR without POSIX modes (exFAT, some WSL mounts) and refusing to install there would trade a readable file for no tracebloc at all. An operator can act on "-rw-r--r--"; they cannot act on silence. Only ever the file the installer GENERATES -- this branch is the `else` of the TRACEBLOC_VALUES_FILE dev-mode override, so a caller`s own file is untouched. tenant-config.example.env told operators to take CLIENT_ID/CLIENT_PASSWORD "from the old release values" -- the one path the chart now tells them to stop using. A tenant that followed values.yaml (install once, then drop them) has nothing there, so the instruction returns empty mid-migration with nothing saying why. Secret first, values as the documented fallback. 4 tests, all mutation-proved. Two came back GREEN first and both were real: the end-state mode test CANNOT see the ordering defect (deleting the pre-heredoc chmod leaves it passing, because the trailing chmod still fixes the mode before anything reads it) -- so a separate test observes the file SIZE at the moment chmod is called and requires the first call to see 0 bytes; and the ls-ldn detector first matched its own comment saying "never stat -c", so it now strips comments and asserts the mode-read line is in the extract. bats: 244 passing vs 240 on develop = exactly the 4 new tests, 0 failures either way. shellcheck -S warning -x clean, bash -n clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(manifest): regenerate for the values-file mode change (backend#2931) install-client-helm.sh is on the bootstrap digest-pinned surface, so changing it without regenerating scripts/manifest.sha256 fails the required Source-of-truth drift check -- and takes gen-manifest.bats "a clean tree passes --check" and the Unit tests job down with it. Three red checks, one cause. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…kend#2626) (#933) * chore(make): one drift guard per line, and refuse an empty entry (backend#2626) The DRIFT_GUARDS list is one `|`-separated line, so it conflicted every time two branches each added a guard - three times on client#911 in a single day. THE OBVIOUS CLAIM IS FALSE, and I checked instead of asserting it: one-per-line does NOT make those appends auto-merge. Appending requires editing the previous last line to add its `|\` continuation, so both sides still touch the same line and git still conflicts. Measured both ways on the real 38-entry list. What changes is whether the conflict can be RESOLVED BY READING it: one line conflict hunk containing 1842-character lines. Finding which guard each side added means diffing two 1842-char strings by eye, and the resolution that looks right in a diff viewer is "take one side" - which silently DELETES the other branch`s gate. A required check stops existing and every run still reports green. one each the differing entry is a 30-character line. You can see it. So this buys legibility at the moment of highest risk, not automation. The comment in the Makefile says exactly that rather than the tidier version. AND IT CLOSES A FAIL-OPEN the new layout makes easy to type. A `||` in the middle yields an EMPTY entry; `sh -c ""` exits 0 and `ran` still increments, so the count check passes and the run reports "all N guards green" with one guard being the empty string. Measured before the fix: `DRIFT_GUARDS=true||true` printed "all 3 guards green" and exited 0. A TRAILING `|` was already caught - the `for` drops a trailing empty field so `ran` falls short - which is why only the middle case needed a new guard. Entries are trimmed, so a whitespace-only entry is refused too, and `==>` lines no longer carry the leading space Make`s continuations add. NEW: scripts/tests/drift-list-integrity.bats, 8 cases. Until now the recipe`s three fail-open guards were asserted by a COMMENT and had been found by hand twice. Every case drives `make drift` itself with a crafted list rather than re-implementing the splitting, so a copy of the rule cannot go on passing while the recipe drifts. It also fails closed if the committed list ever shrinks below 20 entries, which would leave the crafted cases green while `make drift` gated almost nothing. Mutation-proved - each of the four refusals is independently load-bearing: empty-entry guard removed -> 6 of 8 (the `||` and whitespace cases) the trim removed -> 7 of 8 the count check removed -> 7 of 8 the empty-list check removed-> 7 of 8 NOT DONE, and named in the Makefile: deriving the list from `scripts/tests/*.sh` would remove the conflict entirely and make an added guard FILE auto-merge. It is left out because it changes which scripts RUN - a new .sh in that directory never meant as a drift guard would start gating merges - so it needs its own change and its own opt-out list. VERIFIED: make drift 38/38 green; CI=true bats scripts/tests/*.bats 1603 passing, 0 failures; bats-hygiene green; shellcheck -S warning -x clean; manifest --check clean. Makefile only - no chart content, so no version bump. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(make): the list-size floor accepts both shapes Make can print (Bugbot) DEMOTED, THEN HARDENED ANYWAY. The finding said GNU Make 4.x renders an exported simply-expanded variable as `export DRIFT_GUARDS :=`, so a `^DRIFT_GUARDS :=` matcher misses on Ubuntu CI and the floor never counts. Measured: this suite PASSED on Ubuntu CI as written - `bats (bash unit, mocked) = SUCCESS` on the very platform named - because Make records the assignment and the `export` directive separately. So it does not reproduce, and a miss would have failed CLOSED (the `-n` check) rather than passing silently. But the matcher WAS depending on which of two shapes a given Make emits, and one alternation removes that dependency. Local make here is 3.81; CI`s is 4.x; the test should not care. AND THE OTHER SHAPE IS NOW TESTED DIRECTLY, which is the part that would have stayed uncovered: the committed-list assertion can only ever exercise the shape THIS Make emits, so the alternative branch would have been untested until a Make upgrade made it live - exactly the gap the finding pointed at. All four forms are driven (with and without `export`, `:=` and `=`), plus a CONTROL that a line merely MENTIONING the name is not an assignment. Mutation-proved: the optional export prefix dropped -> 8 of 9 (the reported shape) the matcher made over-broad -> 8 of 9 (the control bites) make drift 38/38; bats-hygiene green; shellcheck -S warning -x clean; manifest --check clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(make): the shape case must reach the floor matcher, not a copy of it (backend#2868) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(make): return the VALUE so the prefix strip is testable (backend#2868) Built on 5bf6002 rather than beside it - a co-driver and I fixed the same Bugbot finding on this branch at the same time, and 5bf6002 got there first. Its one-matcher-two-callers shape is kept; this is the delta. THE COUNT MADE THE STRIP UNTESTABLE. `awk -F"|" NF` returns the same number whether or not the assignment prefix was removed, because a leftover prefix just rides along inside field 1. Measured on 5bf6002 itself: narrowing the `sed` to `^DRIFT_GUARDS := ` left all NINE cases green. So the half of the matcher the finding was actually about - handling both shapes Make can print - was still only half covered. The function returns the value; callers count; the shape case asserts it BYTE-EXACT, which is the only thing a wrong strip changes. AND THE RENAME EXPOSED A VACUOUS CONTROL, which is the part worth reading. The control was `! printf ... | drift_guards_entries`. Renaming the function left that line pointing at nothing - `! missing_command` is non-zero, `!` inverts it, and the control went on passing while exercising NOTHING. Routing it through `bash -c` to capture the status made it worse: a bats-defined function is invisible there, so every run was 127. It now asserts the function EXISTS (`type -t`) and THEN that it refuses, so "it refused" can never be satisfied by "it is gone" (CLAUDE.md rule 10). Mutation-proved, all three reaching the live function: the sed narrowed -> red (the weakness this commit closes) the function renamed away -> red on TWO cases (the vacuous control, now caught) the grep narrowed -> red (the original finding shape, still covered) make drift 38/38; bats-hygiene green; shellcheck -S warning -x clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ard the class (backend#2906) (#938) * fix(installer): name the exit code when the CLI install fails (backend#2906) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(guard): derive the exit-code gate spelling, and name the code on the k3d path (backend#2906) Bugbot, Medium: the AST sweep matched only the dotted `.ExitCode -eq/-ne 0` spelling, so the house idiom after a wait -- copy the code into a local, branch on the local -- was invisible to it. `$k3dExitCode = $k3dProc.ExitCode` … `if ($k3dExitCode -ne 0) { Err … }` is exactly that, and it reported a k3d failure with no code while this guard called the class closed. Restating one spelling is what went wrong in client#913; restating two is the same mistake with a longer list. The variable names are now DERIVED from assignments whose right side reads `.ExitCode`. Compliance follows ONE HOP through the call arguments, which is what separates the two sites this newly sees. The GPU branch names no code in its Warn text, but $GPU_SKIP_REASON is assigned in that branch from Get-GpuBuildFailureReason -ExitCode $buildExit, whose fallback returns "docker build exit $ExitCode" -- the classifier is deliberately preferred over a bare number and the comment there says so. The k3d branch has no such hop: the code is read, tested and dropped. Fixed with the house Format-ExitCode helper, matching the sibling site at :4086. Mutation-proved, anchors asserted both times: revert the k3d fix -> 1 of 2 reddens -ExitCode $buildExit -> -ExitCode 0 -> 1 of 2 reddens The second mutation initially did NOT redden: the token regex matched the bare word ExitCode, which the PARAMETER NAME satisfies with a constant value, so the one-hop was a blanket pass. Tightened to the property read and the gate variable. Not widened to $LASTEXITCODE here: 28 uses, and the widening surfaces 7 further offenders. That is a red gate on arrival, so it is filed separately rather than landed (CLAUDE.md rule 4 -- arm while green). Pester: 868 total, 853 passed, 0 failed. make drift: all 38 guards green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(guard): the gate misses $LASTEXITCODE, the commonest spelling (backend#2906) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(guard): a stale exit code is worse than none, so do not demand one (backend#2906) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(guard): follow BOTH exit-code spellings, and the site that exposed (backend#2906) @saadqbal's change-request, and he was right that the floor passing at 8 is what made the hole self-concealing. THE DERIVATION FOLLOWED ONE SPELLING. `$a.Right.Extent.Text -match '\.ExitCode'` never matched `$createRc = $LASTEXITCODE`, so `$createRc` never became a gate token and `if ($createRc -ne 0)` at install-k8s.ps1:5787 was invisible to the walk. `$LASTEXITCODE` had been added as a DIRECT token, which catches `if ($LASTEXITCODE -ne 0)` but not the copy-into-a-local idiom -- the same shape round one flagged for `.ExitCode`, fixed for one spelling. Widened to `'\.ExitCode|\$LASTEXITCODE'`, floor 8 -> 9. AND WIDENING IT IMMEDIATELY NAMED A LIVE SITE, which is the point: line 5789 gated on [$createRc -ne 0]: Err "Couldn't provision the client. Re-run to retry.", but got 1. Exactly the case he described -- `Print-CreateFailure` receives `-OutFile` and `-Location`, never the code, so an operator got "The client couldn't be provisioned." followed by "Couldn't provision the client. Re-run to retry." with no code at all. Fixed with the house pattern: Err "Couldn't provision the client (tracebloc exited $(Format-ExitCode $createRc)). Re-run to retry." So the two-line change is a three-line change: the guard widening is what makes the product fix findable, and leaving the floor at 8 would have let both sit. Pester: 853 passed, 0 failed, 15 skipped. drift 38/38. Manifest regenerated (install-k8s.ps1 moved). A note on my own process, since it bit me twice here: two earlier edits in this pass were silently lost because a later anchor miss in the same script aborted before `write_text`. Both edits are now applied one at a time and each re-read from disk to confirm it landed -- the widening in particular reported "ok" once without ever being written, and the Pester run that followed was measuring the unmodified file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(guard): the THIRD spelling, and a guard that could not be satisfied (backend#2906) Bugbot Medium: the gate derived from `.ExitCode` and `$LASTEXITCODE` only, so branches on `.Code` were invisible. `.Code` is the HOUSE result shape -- Invoke-BoundedProcess and Invoke-DockerCli return @{ Code; Output }, documented at install-k8s.ps1:2307-2308 -- so this was the commonest wrapper spelling, not an edge case. DERIVED, NOT COUNTED BY HAND. Bugbot named 2 sites. Widening the gate and letting the walk report found 31 `.Code` gates in the file, 7 of them carrying a user-facing Warn/Err. The finding was 2 of 7. TWO OF THE 7 ARE DISJUNCTIONS AND ARE NOW EXCLUDED, for the same reason the ELSE and the CATCH already are: the guard cannot know WHICH disjunct fired, so demanding the code demands a possibly-false cause. `if ($res.Code -ne 0 -or $out -match "FAIL " -or $unconfirmed.Count -gt 0)` fails on a non-code disjunct with a code of 0, and "exit 0" beside a failure is the wrong-cause-reads-as-information outcome this Describe exists to refuse. @saadqbal flagged this hazard pre-emptively for the bool-collapsed sites; it arrived here first. THE FIVE REAL ONES NOW NAME THE CODE. All five are wrapper calls where 124 IS the timeout, so the code is precisely what separates the two causes the old text made the operator guess between -- ":1534 nvidia-smi failed or timed out" being the clearest case. AND THE GUARD WAS UNSATISFIABLE FOR ONE ITERATION, which is the finding worth reading. Widening the GATE alone left compliance matching `.ExitCode` only: all five sites kept failing with the code sitting in the message text. A guard that cannot be satisfied is worse than one that does not check -- it trains the reader to edit the guard instead of the code. The two sides now widen in one place, named as such. Floor 9 -> 14, MEASURED by raising it until it failed and reading the number back, not by counting additions by hand -- which is how a floor and a walk start disagreeing. Base merged (#937 had landed; conflict was scripts/manifest.sha256 alone) and the manifest REGENERATED rather than resolved by picking a side, per @saadqbal. 4 mutations, all reddening: drop the code from a fixed message; narrow the gate back; narrow compliance back (the unsatisfiable state); drop the disjunction exclusion. Full Pester suite: 1061 passed, 0 failed. gen-manifest --check clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…e too (backend#2849) (#946) * fix(installer): the dashboard link follows CLIENT_ENV on the bash side too (backend#2849) Bugbot on client#931: `Get-TraceblocDashboardUrl` fixed the Windows half, and the bash twins still hardcoded https://ai.tracebloc.io at TEN sites across install-client-helm.sh and summary.sh -- while `_backend_url` directly above was correctly env-aware. So a `CLIENT_ENV=dev` install on Linux or macOS still sent the operator to the PRODUCTION dashboard for credentials that dev-api then rejects. Same defect, other half of the contract -- and exactly the "fix landed in one twin only" rule this week added to .cursor/BUGBOT.md. `_dashboard_url` mirrors the PowerShell helper: hosts taken from the backend's OWN per-environment settings (DEVICE_VERIFICATION_URI / RESET_PASSWORD_URL in xraybackend/settings/{dev,stg,prod}.py) rather than guessed, and the SAME `tb_client_env` alias reduction plus unknown->prod fallback as `_backend_url`, so the two can never disagree about which environment an install belongs to. Verified against the REAL tb_client_env, not a stub: dev/development -> dev.tracebloc.io, staging/stg -> stg.tracebloc.io, production/unset/ unknown -> ai.tracebloc.io, plus the path and bare-host forms. My first check used a `${1,,}` stub that silently failed on macOS bash 3.2 and made every case look like prod -- the harness was wrong, not the code. The helper itself is bash-3.2 clean (no case conversion, no ${var,,}). 7 bats guards, including one pairing `_dashboard_url` with `_backend_url` per environment (the defect WAS those two disagreeing) and one asserting no live link in either file carries a hardcoded prod path. 247 bats pass; bats-hygiene clean (every assertion enforces); shellcheck --severity=error clean. Part of tracebloc/backend#2849. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(installer): _dashboard_url belongs in common.sh, not one of two siblings (client#946) @LukasWodka, and the bats failure was real. I defined the helper in install-client-helm.sh and CALLED it from summary.sh at four sites -- sibling libraries where neither sources the other -- so summary.sh silently depended on a load order nothing enforces. Loaded standalone (exactly what summary.bats does) those four rendered an EMPTY link: "See it on your dashboard:" followed by nothing, and the "Create a use case" line lost its link too. That is WORSE than the bug being fixed: a wrong URL a reader can recover from, a missing one they cannot. And it is the same defect class this PR exists to close, one layer down -- a fix that landed in one twin only, reproduced between install-client-helm.sh and summary.sh while fixing it between bash and PowerShell. common.sh is the fix Lukas named and it is right: install-k8s.sh sources it first, the bats `load_lib` sources it first, and it already holds `tb_client_env` -- which this calls. Both twins get it by construction rather than by accident. Sourcing summary.sh from the helm lib would have worked too and re-created the coupling in the other direction. Tests follow the helper into common.bats, plus two new guards for the defect itself: the helper is reachable from summary.sh loaded STANDALONE, and no lib under scripts/lib defines it except common.sh. The _backend_url pairing test stays in install-client-helm.bats -- it is the one assertion needing both helpers in scope, and the defect it guards is those two disagreeing. summary.bats and copy-catalog.bats are green again; bats-hygiene clean; shellcheck --severity=error clean on all three libs. Part of tracebloc/backend#2849. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(installer): pin CLIENT_ENV in the copy catalog, which now depends on it (Bugbot on #946) `print_summary` renders `_dashboard_url`, which maps dev|stg|prod to three different hosts -- so the 01-outcomes golden now varies with CLIENT_ENV, and `copy-catalog.bats` setup() pinned every other drift source but not that one. Reproduced before fixing rather than inferred: CLIENT_ENV=dev bats scripts/tests/copy-catalog.bats not ok 2 installer copy catalog: 01-outcomes is current Two consequences, and the second is the worse one: a developer whose shell carries CLIENT_ENV=dev fails the catalog for no reason of their own, and regenerating the golden from that shell would bake dev.tracebloc.io into the committed copy a CUSTOMER reads. Pinned to prod, which is what the golden is FOR -- the customer-facing copy -- and which `_dashboard_url` already treats unset as. The pin makes that explicit instead of dependent on the developer's environment being clean, matching the NO_COLOR and TRACEBLOC_BANNER_SHOWN pins beside it. Checked the sibling suites for the same exposure: summary.bats embeds no host literals, so it is genuinely env-agnostic. All four suites now pass under a deliberately dirty CLIENT_ENV=dev shell -- copy-catalog 2, summary 30, common 101, install-client-helm 241, zero failures. Part of tracebloc/backend#2849. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…942) * fix(chart): wait for MySQL before jobs-manager starts (backend#2913) jobs-manager exits on its first failed DB connection, so a MySQL a few seconds behind it crashloops the container. initContainers was gated on hostPath.enabled, so an edge without it had no init containers at all; the gate now covers only init-writable-data. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(chart): assert the init containers by NAME, and arm the guard that gates them (backend#2913) Two Bugbot findings, both real, and both about position rather than about the change itself. 1. HIGH -- THE REQUIRED HELM SUITE ASSERTED ORDINAL SLOTS. `wait-for-mysql` becoming `initContainers[0]` moved `init-writable-data` to `[1]`, so the #611 and #672 cases were inspecting the WRONG CONTAINER while still passing their own names, and the CSI case asserted `notExists: initContainers` -- true of the chart until this PR, false after it. Measured before fixing: 3 failed / 640 passed. NOT renumbered to `[1]`, because that re-couples the test to ordinal position and is what broke. 23 paths now select by name -- `initContainers[?(@.name=="init-writable-data")]` -- the filter form this repo already uses for `volumes` and `env`. The `[0].name == init-writable-data` equality became `exists:` on the filter, since under a name filter the old assertion was tautological. The CSI case now states the property its TITLE always claimed -- "skips the privileged init", not "has no inits": no `init-writable-data`, nothing running as `runAsUser: 0`, and -- for non-vacuity -- `wait-for-mysql` present, so both refusals are about a populated list rather than an empty one. 2. MEDIUM -- THE NEW GUARD WAS NOT ON THE LIST THE GATE RUNS. `jobs-manager-waits-for-mysql.sh` existed and passed, but `DRIFT_GUARDS` did not name it, so `make drift` and the required Source-of-truth drift job never invoked it: advice, not a gate. Now armed, 38 -> 39 guards. ARMED WHILE GREEN: the guard was run standalone first (rc 0) before being added, rather than landing a red gate. Mutation-proved with the regression the finding names -- folding `wait-for-mysql` back under `hostPath.enabled`, which leaves a VALID chart -- and `make drift` now fails with `FAIL hostPath.enabled=false: first initContainer is 'api', want 'wait-for-mysql'`. My first attempt deleted the container outright, broke the template, and produced a render error instead of the guard's refusal; that proved nothing about the guard and is not what is recorded here. helm unittest 643/643, drift 39/39, check-facts 14/14, shellcheck + bash -n clean. Chart already one patch above develop (1.9.91 vs 1.9.90); manifest unchanged, since no installer script moved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(chart): do not pin a uid on the arbitrary-UID path (backend#2913) OpenShift restricted assigns a uid from the project range; one pinned container fails SCC admission for the whole pod. runAsNonRoot is inherited from the pod, so nothing is loosened. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(chart): use the app image for the wait, so no uid needs pinning anywhere (backend#2913) Bugbot High, and it independently confirms the plain-Kubernetes regression I raised on this PR last round: dropping the pin fixed OpenShift and broke every other platform. THE MECHANISM. `runAsNonRoot: true` only FORBIDS uid 0 -- it assigns nothing -- so with no `runAsUser` anywhere (which the OpenShift arbitrary-UID path requires) the kubelet reads the effective user from the IMAGE. `library/busybox` declares `Config.User` EMPTY (measured: `[]`), i.e. root, so AKS, EKS, k3d and bare metal all fail the init with "container has runAsNonRoot and image will run as root". THE FIX IS THE IMAGE, NOT A VALUES SEAM. `Dockerfile.jobs_manager:26` declares `USER 1001` -- which is exactly why `api` and `pods-monitor-container` need no pin either. Reusing it means NO uid is pinned anywhere in this pod: OpenShift assigns from the project range, plain Kubernetes gets 1001 from the image, and the arbitrary-UID invariant this branch added holds with no operator action and no new values key. I had a `pinRunAsUser` seam built and this is strictly better -- it needs nothing from the operator and keeps the guard exactly as written. It also removes an image: busybox was a second pull on every edge for a TCP connect the app image can already do. (`library/busybox` is still used by `mysql-deployment.yaml` and by `init-writable-data`, so the values key stays live.) THE PROBE IS PYTHON NOW, because the app image carries no netcat. `socket.create_connection` is the same test with a real per-attempt timeout, which `nc -z` did not have. Every message is byte-identical -- the diagnosis was already right. Driven against a real socket, not just rendered: rc=0 with a listener, rc=1 without, and the deadline is checked AFTER an attempt so a wait starting inside the last two seconds still gets one try. Verified on the render: pod `runAsNonRoot=true`, and `runAsUser=<none>` on all three containers. Two mutations, both with the anchor asserted on disk first: * busybox restored -> the new helm-unittest case fails * a uid re-pinned -> the case fails AND the drift guard fires helm unittest 644/644, drift 39/39. Chart already one patch above develop; manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(chart): bump to 1.9.92 -- develop reached 1.9.91 (backend#2913) The version-bump gate needs one patch above develop, and develop moved to 1.9.91 while this branch sat at 1.9.91 too. Caught on the push I had just made, not by CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
release-train: develop -> staging
Contributor
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 079ab7c. Configure here.
tracebloc-release-train
Bot
added
gate-nudge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-mainbranch (a mirror ofstaging), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Changes jobs-manager pod startup order, expands flag-gated Secret RBAC, and alters Windows PATH/CLI placement—high-impact areas, but heavily guarded by new drift scripts and unit tests.
Overview
This release train bundles several edge-client fixes and CI/installer hardening, with the Helm chart bumped to 1.9.92.
jobs-manager startup: A
wait-for-mysqlinit container now runs on every install (not only hostPath), probingmysql-client:3306with a 300s bounded Python wait using the jobs-manager image (non-root, OpenShift-safe).init-writable-datastays hostPath-only; unit tests were updated to assert by init name rather than index.Chart rendering & RBAC: All
.Values.hostPathsubkey reads use(default dict .Values.hostPath)sohelm upgrade --reuse-valuesfrom older releases withouthostPathno longer nil-panics. With per-experiment DB creds enabled, jobs-manager getspatchon Secrets (in addition to list/delete) so cycle-2 inference can update cred secrets instead of 403ing silently.Installers: Bash gains
_dashboard_urlincommon.shso credential prompts and summaries point at dev/stg/prod dashboards, not always production. Generatedvalues.yamlis truncated andchmod 600’d before writingclientPassword, with a visible warning if the mode cannot be enforced. On Windows, the installer copiestracebloc.exeinto the machine-wide tools dir (directional version refresh, no downgrade), tightens Machine PATH dedup, requires a machine-wide CLI for the fast path, and surfaces exit codes in more failure messages.Drift/CI:
DRIFT_GUARDSis one guard per line; themake driftrecipe trims entries, rejects empty/||slots, and adds guards for hostPath reads and MySQL wait behavior.Reviewed by Cursor Bugbot for commit 079ab7c. Bugbot is set up for automated code reviews on this repo. Configure here.