release-train: develop -> staging by LukasWodka · Pull Request #748 · tracebloc/client

LukasWodka · 2026-08-18T08:37:08Z

Automated promotion by the release train (RFC-0008 D14). Head is the train-managed release-train/to-staging branch (a mirror of develop), so it never collides with a human PR. Merged only when the fr-gate is green.

Note

Medium Risk
Touches installer exit/telemetry paths, required CI gates, and jobs-manager RBAC across namespaces; chart and docs changes are additive but egress lockdown misconfiguration could still false-pass if probe hosts are wrong.

Overview
Automated develop → staging promotion carrying several independent feature/fix trains.

Installer telemetry (backend#1907) adds scripts/lib/telemetry.sh and wires it through install-k8s.sh, common.sh (install_cleanup, step_header, error), assess skip/handoff paths, and GPU reboot exit 2 handoff. Each committed install emits one structured outcome event (phase timings, error class, opt-out via TRACEBLOC_NO_TELEMETRY / DO_NOT_TRACK); delivery is log + bounded spool until backend ingest exists. telemetry-vocabulary-agreement.sh runs in required Source-of-truth drift; .gitleaks.toml allowlists telemetry canary fixtures.

CI (#753) removes redundant shellcheck from installer-tests and standard-checks (runner image already has it); make lint derives parse + shellcheck from the tree/git ls-files. bats installs are bounded, retried, and job timeouts increased. Kanban workflow mints a release-train GitHub App org installation token instead of a PAT.

Helm chart (1.9.47) grants jobs-manager read access to DaemonSets (heartbeat version inventory), optional Role/RoleBinding in nodeAgents.namespace when split from release ns, and passes NODE_AGENTS_NAMESPACE (empty when resourceMonitor is off).

Installer behavior: shared _mysql_engine_decision so preflight and _assert_engine_runs_on_this_arch don’t refuse fresh arm64 on native 8.4; macOS Docker Desktop nudge before privileged tier; bounded _docker_answers; device sign-in retry in provision.sh; fail-closed parse in Makefile.

Docs: SECURITY.md / SEAL-CHECK.md expand §8.2 egress lockdown (CNI pre-flight, drain/flip/verify/rollback, probe-host false-pass on TCP :443).

^{Reviewed by Cursor Bugbot for commit 526fc49. Bugbot is set up for automated code reviews on this repo. Configure here.}

…e-run (#738) cli#517. `tracebloc login` failing was fatal to provision_client, so a device code that lapsed — the ordinary outcome when the human approving it is reading a phone — threw away every step the installer had already completed and asked for a full re-run. _device_sign_in wraps the call in the same shape the name prompt has used since the 2026-07-09 type-ahead report: one in-place retry, offered only when there is a live terminal to hand a fresh code to, and abandoned on a failed read (EOF / no live input) because re-prompting cannot fix that. A second failure is evidence of something other than a missed code, so it falls through to the same fatal error as before. The sign-in now runs with TRACEBLOC_INSTALLER=1 (a command prefix, not an export — no later CLI call inherits it), which makes the CLI drop its own "run `tracebloc login`" line. Under the installer that advice was actively wrong: a bare login leaves the client mint and the Helm install undone. The installer's "re-run the installer" is now the only next step printed. The /dev/tty openability probe moves into _login_tty_ok, unchanged, purely so the tests can force either branch without a controlling terminal. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…end#2047) (#739) * fix(preflight): ask the MySQL engine rule before refusing arm64 (backend#2047) A fresh install on a bare Linux arm64 host with no amd64 emulation hard-failed in preflight with "the tracebloc client images are amd64-only", telling the operator to provision an amd64 VM or install QEMU. Both were wrong: on that same host `TB_MYSQL_ENGINE=auto` selects the multi-arch MySQL 8.4 engine, so the product runs NATIVELY. The one amd64-only image left is mysql-client 5.7, and whether this install needs it is a question only the engine rule can answer — the arch gate was answering it itself, and answering it wrong. The refusal stays load-bearing for REUSED datadirs: 8.4 cannot open 5.7-format data (staged 5.7 -> 8.0 -> 8.4 only), which is why `auto` never flips an existing edge. So this is ordering, not deletion. Ordering, in two places, both keyed off ONE copy of the rule: - `_resolve_mysql_engine` is split into `_mysql_engine_decision` (pure: echoes "<engine> <reason>", no logging, no globals) plus a logging wrapper. The reason is what lets each refusal name its real cause. - `_pf_arch` consults that function instead of deciding for itself. 8.4 -> native, no refusal, no emulation advice. 5.7 -> refuse, and say why: existing datadir format, or an explicit TB_MYSQL_ENGINE=5.7. It fails CLOSED when the rule is not loaded, and treats an unusable TB_MYSQL_ENGINE as a finding rather than a pass. - `_assert_engine_runs_on_this_arch` re-asks the same question after the engine is resolved for real, because preflight cannot see an existing Helm release — that edge can read as fresh early and only pin 5.7 later. Without it, the fresh-path fix would let such a host proceed to an exec-format CrashLoop. Derive, never restate: preflight needs two inputs that are `local`s of install_client_helm, so `_client_values_file` and `_client_default_namespace` are factored out and both callers use them. A restated values.yaml path would stop seeing the sticky 8.4 opt-in and refuse an arm64 edge already running 8.4. The binfmt probe moves to common.sh so both gates read the same probe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(preflight): sanitise TB_NAMESPACE before the datadir probe (Bugbot) install_client_helm resolves the namespace through _client_default_namespace UNCONDITIONALLY, and that helper sanitises to DNS-1123. _pf_mysql_engine_decision called it only when TB_NAMESPACE was empty, so an operator-supplied TB_NAMESPACE="My Edge" stayed raw in preflight while the install path turned it into "my-edge" — the two gates then probed different per-release datadirs (".../My Edge/mysql" vs ".../my-edge/mysql"). On arm64 with existing data only under the sanitised name, preflight would have reported a native 8.4 pass and _assert_engine_runs_on_this_arch would have refused afterwards, i.e. after cluster setup and credential entry. That is the exact drift this PR removes, reintroduced by the guard itself: same rule, same inputs, or it is not the same question. The `local TB_NAMESPACE="${TB_NAMESPACE:-}"` shadow stays — it seeds the helper from the caller's value and keeps preflight from leaking a sanitised namespace into the globals later steps read. Covered by a new test that plants the datadir under the SANITISED name and asserts preflight still refuses; it first asserts the sanitiser actually changes the input, so it cannot pass vacuously. Mutation-proved by restoring the defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

LukasWodka · 2026-08-18T08:37:20Z

bugbot run

The last board writer on PROJECTS_KANBAN_TOKEN. A per-repo COPY, so one PR per repo; the content stays byte-identical across the fleet because the guard compares it that way. `owner:` makes the installation token ORG-scoped -- a repo-scoped one cannot write the org project. No fallback to the PAT: a fallback would let a broken App path keep working silently. This workflow also fires on DEPENDABOT PRs, which GitHub gates on a separate secret scope. Both app secrets are set there too; without that, Dependabot PRs would stop reaching the board with `Input required and not supplied` -- the exact failure PROJECTS_KANBAN_TOKEN already had to be dual-scoped to avoid. Refs backend#2036 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

saadqbal

Two things on the arm64 work. Also, one that isn't inline-able: preflight's datadir probe resolves TB_NAMESPACE to tracebloc because provision_client only exports the minted slug later, so on a real edge the per-release probe ($HOST_DATA_DIR/<slug>/mysql) misses and the refusal still lands after cluster setup + sign-in. The comment calls that the residual case, but with a minted namespace it's the common case, not the edge. Bugbot's two are still open. Manifest checksums check out. Fixes go on develop, not this branch.

…ch, and gate on it (#749) * docs(#248): the §8.2 egress lockdown needs an enforcing CNI — say which, and gate on it The §8.2 training-pod egress lockdown only blocks anything on a CNI that enforces EGRESS NetworkPolicy. Verified 2026-06 on tb-client-dev-templates (EKS, self-managed VPC CNI, NetworkPolicy disabled): a DNS-only egress policy did NOT block https://example.com — the probe returned 200. On such a fleet flipping allowExternalHttps=false is cosmetic: the rule renders and nothing enforces it, which reads as protection and is not. The docs did not say which CNIs actually enforce, and the rollout had no gate that would have caught it. This is the three folded #248 follow-ups (#248, #257, #258): SECURITY.md - §5.1: a concrete enforcing-CNI list, and an EKS row that distinguishes the vpc-cni MANAGED ADD-ON with enableNetworkPolicy=true (enforces, v1.14+) from a self-managed DaemonSet (does not — no control-plane PolicyEndpoint controller). Records the dev-EKS finding. - §4.2: the policy bullets are a request to the CNI, not a guarantee — link the prerequisite from where the rules are stated. - §6.2: an egress PRE-FLIGHT probe (DNS-only policy in a throwaway namespace; the connect MUST fail), the gate the old rollout lacked. - §8.2: rollout becomes Gate 0 pre-flight → drain → flip → `helm test` the egress-enforcement check → real experiment, with an explicit rollback. Drain is called out because the policy change hits RUNNING pods too. SEAL-CHECK.md - A per-fleet production flip/rollback runbook next to the local k3d one, on --reset-then-reuse-values (a plain --reuse-values re-applies the stored false and silently defeats the rollback). - The probe-host FALSE PASS: the check reads a refused connect (curl exit 7) as "blocked", and a host with nothing listening on :443 refuses identically — so a wrong enforcementProbeHost passes without testing anything. values.yaml - Same false-pass caveat where the operator actually sets the host. Docs + one values comment; no template renders differently. Chart version bumped because values.yaml is packaged chart content (chart-version-guard). Closes#248 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(#248): the drain check must select pods, not Jobs Review finding on PR #749. Both copies of the drain step used `kubectl get jobs -l tracebloc.io/workload=training`, but that label is only ever set on the POD TEMPLATE — `job.yaml` puts it under `spec.template.metadata.labels` and `jobs_manager._prepare_job_config` sets it via `pod_labels`; the Job object's own metadata carries just a name. That is all the NetworkPolicy needs, since its podSelector matches the pod. So the command returned nothing even mid-run: a false all-clear on the one step that exists to stop the operator flipping the lockdown out from under a running training pod. Selects pods in both SECURITY.md §8.2 and the SEAL-CHECK.md fleet runbook, with a note on why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

LukasWodka · 2026-08-19T06:20:12Z

bugbot run

LukasWodka · 2026-08-19T06:26:54Z

bugbot run

…n inventory (#750) * feat(rbac): let jobs-manager read daemonsets for the heartbeat version inventory client-runtime#152 adds a version inventory to the edge-device heartbeat, assembled from the configured images on the chart's own workloads. The jobs-manager Role already grants read on `deployments`, which covers jobs-manager, pods-monitor, requests-proxy, mysql-client and egress-proxy. resource-monitor is a DaemonSet — the one workload that is not a Deployment — and `daemonsets` was granted nowhere in this Role. Without it the list 403s. The runtime degrades rather than failing (it lists the two kinds separately for exactly this reason), so the symptom is not an error anyone would chase: resource-monitor is simply reported as null forever, which reads as "not deployed". Read-only, and namespace-scoped in the Role branch. Granted in BOTH branches — an edge on clusterScope: false needs it just as much — and asserted in both by the helm unit tests, since the two rule lists are maintained separately and have drifted before. Refs tracebloc/client-runtime#152 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rbac): the resource-monitor DaemonSet is not in the release namespace Review finding on PR #750. The `daemonsets` grant I added to the release-namespace Role does not reach resource-monitor: the chart puts that DaemonSet in `nodeAgents.namespace.name` (default tracebloc-node-agents), split out deliberately so it can run under PSA `privileged` without widening the release namespace. It is the ONE chart workload outside the release namespace — every other one the inventory reads is a Deployment there. So on a `clusterScope: false` edge (including OpenShift) the list would still have failed, and the heartbeat inventory would still report resource-monitor as null. The ClusterRole branch already covers it cluster-wide; this adds the second Role/RoleBinding pair in the node-agents namespace for the namespaced branch — the same split image-refresh already uses for this exact boundary. Also wires NODE_AGENTS_NAMESPACE into the jobs-manager `api` container: without it the runtime searches the wrong namespace and RBAC alone would not have helped. Only `api` gets it — pods-monitor does not read it. The pair renders only when there is something to read: resourceMonitor enabled, and the node-agents namespace actually distinct from the release namespace (where the existing Role already grants it, and where a second pair would collide on name). The namespace lookup is `dig`-guarded so a `--reuse-values` upgrade from a release predating `nodeAgents` cannot nil-pointer this always-rendered template. helm unittest: 472 passed (was 469). helm lint clean. Refs tracebloc/client-runtime#152 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rbac): empty NODE_AGENTS_NAMESPACE when resourceMonitor is off Review finding on client-runtime#348. `resourceMonitor: false` is a documented value, and it gates BOTH the node-agents namespace and the new Role/RoleBinding pair — but this env was set unconditionally. jobs-manager would then list a namespace that does not exist, with no Role authorizing it: a 403 on every 60s heartbeat, forever, about a workload the operator deliberately turned off. The empty value is load-bearing rather than a no-op. The runtime distinguishes UNSET ("chart predates this env — use the built-in default") from SET-BUT-EMPTY ("there is no node-agents namespace"), so emptying it is what actually suppresses the search; omitting the env would fall back to the default and list it anyway. Two chart tests, one per branch. The lookup keeps the `dig` guard. helm unittest: 474 passed (was 472). helm lint clean. Refs tracebloc/client-runtime#152 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(chart): bump to 1.9.47 so #750 does not collide with #749 Both PRs bumped to 1.9.46. #749 (docs) is approved and merges first, so this one takes the next version. ORDERING: this now REQUIRES #749 to merge first. If #750 lands first, #749 would carry 1.9.46 against a develop already on 1.9.47 — a version regression the chart-version-guard would not catch (it only checks that a bump happened, not that it moves forward). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(chart): tighten the comments, drop the issue references (review) 50 lines of comment removed. Kept the load-bearing "why" in each place — the DaemonSet living outside the release namespace, and the empty NODE_AGENTS_NAMESPACE being deliberate rather than a no-op — and dropped the narration around it. Issue references removed from templates and tests, including the `(client-runtime#152)` suffixes on test names. 474 helm unit tests still pass; lint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

LukasWodka · 2026-08-19T10:16:50Z

bugbot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

2 issues from previous reviews remain unresolved.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 8de5d64. Configure here.}

…rch gate (#752) * fix(installer): split existing-release from existing-datadir in the arch gate A live Helm release (existing_id, from 'helm list -A') was folded into the existing-datadir reason, so on arm64 the gate claimed "This host holds existing MySQL 5.7 data" and offered --data-dir as a fresh start. That claim can be false (on TB_STORAGE_MODE=node-local both datadir probes are empty, and the release's data format is unknown from here), and --data-dir cannot clear a release that 'helm list' reports — the next run refuses identically. Split the reason: existing_id -> 'existing-release' (5.7 as a data-safety default, no host-data claim, remedy is emulation or uninstalling the release), real datadir content -> 'existing-datadir' (unchanged, --data-dir is a valid fresh start there). Also correct the header comment that claimed assert_amd64_emulation 'owns' macOS: it refuses whenever Rosetta amd64 is off without consulting the engine rule, so a fresh arm64 Mac that 8.4 would serve natively is still turned away — named as a separate backend#2047 follow-up rather than implied covered (Asad, client#748). Tests: existing-release distinguished in the reason and purity suites; a new arch- gate test pins the corrected remedy (no false data claim, no --data-dir). Both halves mutation-proved; 157/157 bats green, shellcheck clean. Bugbot (Medium) + review (Asad), client#748 staging mirror. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(installer): datadir-first ordering + regenerate the supply-chain manifest Two Bugbot findings on the first push: - HIGH: install-client-helm.sh is a bootstrap-fetched lib verified against scripts/manifest.sha256; the edit was made without regenerating the digest, so install.sh's signed fetch (R8) would fail closed. Regenerated (gen-manifest.sh --check now clean). - Medium: with existing_id-first, a host that has BOTH a live release AND 5.7 datadir files got reason existing-release, whose 'just uninstall' remedy leaves the files to re-pin 5.7 next run. Reorder to datadir-first: files present -> existing-datadir (true data claim, remedy now also says uninstall any release); existing-release fires only with NO files, where 'uninstall the release' is a complete fresh start. New test pins the both-exist -> existing-datadir ordering. 157/157 bats, shellcheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(installer): existing-release fresh-start must remove the retained MySQL PVC Bugbot (Medium) on the prior push: the existing-release fresh-start remedy said 'uninstall the release' and treated it as complete. In the node-local case that is exactly wrong — MySQL lives on an in-cluster PVC annotated helm.sh/resource-policy: keep, so 'helm uninstall' leaves the 5.7 data behind; the next run sees no existing_id, resolves to native 8.4, and fails the format guard against the leftover volume (or loops). Make the remedy complete: to start fresh, remove BOTH the release AND the retained MySQL PVC (and any host data dir). Test asserts the PVC/keep wording. Manifest regenerated. 157/157 bats, shellcheck clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(installer): don't claim 5.7-FORMAT from a non-emptiness probe (Asad) _mysql_dir_has_content tests only that the dir is non-empty, never the on-disk format — so 'real 5.7-format datadir content' overstates exactly the way this PR removes from the existing_id path. Revert the datadir reason wording to format-neutral ('mysql datadir content, non-empty'); the engine still keeps 5.7 because the format is UNKNOWN and 8.4 cannot open a 5.7 dir if that is what it is. Manifest regenerated; 157/157 bats, shellcheck clean. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…said we were starting it (#741) * fix(installer): a stopped Docker forced the admin path, and the copy said we were starting it The assess branch for a down runtime printed Docker isn't running yet — starting it, then checking your environment. and then `return 0`. Nothing started anything. The probe found no usable runtime, macOS classified Tier 2 — the admin-password path — and the only `open -a Docker` in the tree sat inside install_docker_desktop, BEHIND preflight_sudo. So the installer could only start Docker after taking an administrator password it needed solely because Docker wasn't started. A Mac with Docker installed but stopped therefore had no path at all for a user who couldn't give that password, and had been told Docker was being started. That is the state a machine is in after every reboot. With Docker running, PROBE_RUNTIME_USABLE=1 wins first and unconditionally in _classify_from_probes → Tier 0, where the admin gate and preflight_sudo never run. Tier 0 (client#704) was unreachable for exactly the machines it was written for. _assess_handle_runtime_down now does the thing or doesn't claim it: on macOS `open -a Docker` (a GUI launch, no privileges whatsoever), wait bounded, and let the probe reclassify. If it doesn't come up, say so and name the cost — the fallback needs a password. Off macOS, starting the daemon needs root, so don't claim to be starting it. _wait_for_docker is extracted so the nudge and install_docker_desktop share one loop rather than growing a second one that drifts. ── the report named a healthy check ── Stopped at .../common.sh:527 (exit 1) — command: sudo -n true Line 527 is _real_sudo, and `sudo -n true` is the passwordless-sudo probe: it is SUPPOSED to fail on a normal Mac, and the host check prints its failure as a normal row ("sudo needs a password"). The real cause — "Could not obtain administrator privileges (sudo authentication failed)" — was never recorded, because error() calls `exit` and `exit` fires no ERR trap, so TB_ERR_* still held the last benign probe. A failure report that confidently names a healthy check is worse than one that names nothing: it sends the reader to a line that is working. error() now records its own CALLER location and message first (BASH_SOURCE[1], not [0] — [0] would name common.sh for every refusal in the tree), with an explicit exit code so it can't latch whatever $? happened to be. Verified against the failing scenario end to end: the last recorded cause goes from `cmd=sudo -n true` to `cmd=error: Could not obtain administrator privileges (sudo authentication failed)`. ── the coverage gap that let it ship ── preflight_sudo has four branches. Three were tested: root, no sudo binary, passwordless. The ordinary Mac — an admin user whose sudo wants a password — was exercised by NOTHING, because the tier-2 lifecycle tests stub preflight_sudo out via _tier0_mocks and common.bats never covered it. Both branches are now tested, each asserting its specific message rather than a bare non-zero. Mutations, each asserted to have applied: error() stops recording -> 3 tests fail error() records BASH_SOURCE[0] -> 1 fails the old "starting it" copy back -> 7 fail Three things this cost, worth recording: - One assertion passed for the wrong reason first: assess.bats wasn't sourcing setup-macos.sh, so `run _try_start_docker_desktop` exited 127 and a bare `-ne 0` accepted it. Now `run -1` asserts the one code. - _try_start_docker_desktop's "not installed" branch was unreachable on any macOS dev box, so _docker_app_installed is its own function and the test stubs it — otherwise only CI would ever run that branch. - A test assertion failed for an hour against a log that plainly contained the string: bats runs a preprocessed copy (`N-common.bats.src`) and rewrites those paths back in its OWN output, so `cat` prints something the file does not contain. The needle is now what is really there. Closes#740 * fix(installer): regenerate the manifest, harden two advisory assertions Two failures from the full suite, both introduced by the previous commit and both missed because I reported a green run that had finished BEFORE the last round of test edits. scripts/manifest.sha256 carries the checksums of the installer libs this branch edits, and it is the installer's own integrity check over the files it downloads. Merging it stale would have broken verification at install time on every platform — a worse bug than the one this branch fixes. Regenerated; gen-manifest.sh --check passes. bats-hygiene flagged two `! grep -qF "LAUNCHED" <<<"$output"` lines with no `|| return 1`. Both happened to be the last statement in their test, so they did decide the result — but the rule is unconditional for a reason, and an advisory assertion is precisely the defect class this branch's tests exist to argue against. The process error is the one worth recording: I read a tail showing the last test passing and called the suite green. The suite's exit code says whether the suite passed; the last line says what ran last. Those are not the same claim, which is why the exit code exists. * fix(test): derive the expected path instead of writing "common.bats" down The caller-location test passed on macOS bats 1.13 and failed on CI. The needle was the literal "common.bats", but bats executes a PREPROCESSED copy of the file whose name varies by version — and rewrites those paths back in its OWN output, so the failure message shows a path the log does not contain. That display cost an hour locally before CI cost another round. The expectation now comes from the same mechanism under test: whatever bash calls this file (BASH_SOURCE[0] at test scope) is exactly what error() must record as BASH_SOURCE[1] when called from a function defined here. True on every platform and every bats version, because both sides read the same thing rather than agreeing with a string I typed. Still mutation-proven: pointing error() back at BASH_SOURCE[0] reddens it. Manifest regenerated for the test-only change — gen-manifest --check passes. * fix(installer): the Docker nudge could hang forever on the daemon it exists for Bugbot, High, #741. _wait_for_docker and _try_start_docker_desktop called a bare `docker info`, which does not return against a WEDGED daemon — only a stopped one. Wedged is precisely the state that reaches them. _assess_runtime_down classifies runtime-down from _bounded's 124, i.e. from the bounded probe having ALREADY timed out; the new assess-time path then re-entered an unbounded probe. So it hung on the one input that routes to it, and the operator saw "starting it" followed by nothing, forever. The shape of that is worth naming: this branch exists because a message promised an action the code never took. Unbounded, it promised the action and then froze — a worse version of the same defect. _docker_answers (common.sh, beside _bounded) is now the single probe every "is the runtime up?" check routes through, defaulting to the same 10s as TB_ASSESS_DOCKER_TIMEOUT: both answer the same question about the same daemon and must not disagree. Wired into all four probes on this path, including install_docker_desktop's two, which bracket the loop extracted here and hang identically. _wait_for_docker also moves from a poll COUNT to a wall-clock deadline. Now that each probe is bounded, counting iterations would make "20 polls" mean 60s against a live daemon and ~260s against the wedged one this exists to survive. $SECONDS keeps the caller's polls*3s budget true either way. Three tests, four mutations, each anchor confirmed to apply (7 passing -> 6, 6, 4, 6). The fourth sets the bound to 0: still routed through _bounded, bound disabled — covered-looking and not covered. Two things the fix surfaced, both mine: - The first test asserted on a stub's stdout, but _docker_answers redirects its whole call to /dev/null, so the assertion was invisible either way. It passed. Now a marker file. - Bounding the probe runs it through timeout(1) as an EXTERNAL process, so a `docker() { … }` shell-function stub no longer intercepts — that broke a passing test. Worse, _bounded runs bare when neither timeout nor gtimeout exists, so such a stub works on some hosts and not others and the test's meaning depends on the machine. Stubs now sit at _docker_answers, the seam present everywhere. 16 more unbounded probes remain across preflight.sh, setup-linux.sh, diagnose.sh and setup-macos.sh's other branches, plus probe.sh hand-rolling its own copy of _bounded. Filed as #744 with a proposed check-style guard, rather than widened into this PR. Full suite green; manifest regenerated. * chore: re-trigger CI — the push event for 616e967 was dropped No code change. GitHub's 2026-08-17 incident (Actions in major outage from 13:40 UTC) dropped the push event for 616e967: the remote is at that sha and `actions/runs?head_sha=616e967…` reports total_count 0, while unrelated workflows on other branches ran normally half an hour ago. So it is that one event, not the platform. An empty commit rather than the alternatives, deliberately: * `--amend` + force-push would work but force-pushing needs an explicit human instruction, so it is not mine to do. * closing and reopening the PR would fire `pull_request`, but closing a PR is also not mine to do on a nudge. * `workflow_dispatch` exists on standard-checks and installer-tests but NOT on version-bump-gate-caller — and `version-bump-gate / version-check` is one of develop's eight required contexts, so dispatching would leave the PR permanently short of a required check while looking busy. A commit is the only nudge that reaches all eight. * fix(installer): four more from Bugbot, three of them the same lie one level down ── the regression this branch introduced ── _wait_for_docker returns non-zero on timeout, and install_docker_desktop called it as a BARE statement under `set -e`. So the script exited there — before the whale-icon guidance and the deliberate error() that exist precisely for "Docker didn't come up". A silent death replacing a helpful message, in a branch about messages that lie. The old inline loop ended on printf/tput and so always fell through; extracting it moved the timeout's status somewhere errexit could see. `|| true` now guards it, and the `if ! _docker_answers` below remains the verdict. Guarded by a source-level assertion: driving install_docker_desktop end to end would need brew, hdiutil and a DMG. ── the same defect, one layer down ── On a Colima-only or headless Mac, _assess_handle_runtime_down announced "starting it" and only then found there was no Docker Desktop to start — and the failure copy went on to tell someone WITHOUT Docker Desktop to open Docker Desktop. _docker_app_installed is now part of the condition, not just of the nudge. The off-macOS branch said "start it, then re-run this installer" and then returned 0 and carried on into the privileged flow: guidance contradicting the next thing on screen. It now names its continue, as the macOS failure branch already did. Both are this branch's own bug reproduced inside the fix for it. That is worth stating plainly rather than filing under "review feedback": extracting and re-routing a path is exactly where message-and-behaviour drift gets reintroduced. ── the host-dependent test ── CI failed on "won't launch an app that isn't installed": it stubbed `docker()`, but a bounded probe runs through timeout(1) as an external process, so on the ubuntu runner — which has a live daemon — the real probe answered and the function returned 0 at "already up" without reaching the branch under test. It passed here only because macOS ships no timeout(1), so _bounded fell through to the bare call and the stub worked. The fix is this file's existing convention, `_bounded() { shift; "$@"; }`, which the _assess_runtime_down tests have used all along. I documented this exact trap in #744 and in the previous commit message, then shipped it. Verified the other way round too: local Docker is running here, so the test now passes under the same condition that failed on CI. Mutations, each anchor confirmed to apply (13 passing -> 12 each): bare _wait_for_docker under set -e -> 1 fail announce without checking the app exists -> 1 fail drop the off-macOS "Continuing" line -> 1 fail Full suite 1141 passed, 0 failed. Manifest regenerated, style guard clean. * docs(installer): two comments this branch made false (Asad review, #741) Both are the defect this branch exists to remove — a statement that no longer describes the behaviour — so they belong in the same PR rather than a follow-up. ── assess.sh says STRICTLY NON-MUTATING, and this branch mutates ── `open -a Docker` plus a 60s wait is a mutation by the header's own definition, and the header still forbade it. The risk is not the stale sentence, it is which way the next reader resolves the contradiction: revert the nudge as an obvious breach, or read the ban as lifted and start the k3d cluster from `cluster-stopped` — the one mutation the header exists to forbid, and the one _assess_cluster_servers_running explains is off-limits. Restated as "non-mutating, with exactly ONE narrow exception", with the four properties that make this one admissible written down so the boundary is checkable rather than a matter of taste: unprivileged (a GUI launch, no sudo), idempotent (launching a running Docker is a no-op), bounded (a wall-clock deadline, every probe through _docker_answers), and load-bearing (without it a stopped-but-installed Docker takes Tier 2 and demands a password to start a runtime that is already installed). Plus the explicit non-consequence: this does not lift the ban, cluster-stopped still only prints. ── TB_ERR_CMD promises UNEXPANDED text, and error() stores interpolated ── The declaration said BASH_COMMAND yields `cmd "$VAR"` and never the value, "so this cannot leak a credential into the log". error() now stores "error: $*" — interpolated — and it reaches LOG_FILE twice, via _record_err and via install_cleanup's `FAILED at … command:` line. Nothing leaks today; every interpolating error() call carries paths, sizes, versions and arch names. But that comment is exactly what a future author would rely on when writing an error message, and it was false for the error() path. It now documents both producers and their different guarantees, and carries the rule forward: never interpolate a credential into an error() message — name the path, not the value. Considered making that rule a machine check, since a load-bearing claim should be one. Rejected, deliberately: the only name-shaped hit today is `error "…($cred_file)"`, a PATH — the safe pattern the comment recommends. A name-based guard would fire solely on that false positive, and silencing it needs a hand-maintained allowlist, which is the restate-the-rule defect. All noise, no signal. Comments only; no behaviour change. Full suite 1141 passed, 0 failed; manifest regenerated, style guard clean.

LukasWodka · 2026-08-19T10:31:08Z

bugbot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 598ccb6. Configure here.}

… no test (#743) #736 extended ConvertTo-SanitizedInput with the SS3 family and the unknown-family floor, and shipped both with zero PowerShell coverage. The four cases in that Describe block are all CSI, so on Windows the entire SS3 half and the whole floor were unverified while bash and Go both had cases. That asymmetry is the mechanism behind the bug it was fixing. The rule is hand-copied into three languages, only one shape was ever tested in one of them, and SS3 went missing from all three at once (tracebloc/cli#516) — exactly as the CSI gap had before it (cli#364 / client#362). A rule with coverage in two of three implementations is a rule that drifts in the third. Eight It blocks, mirroring the bats corpus in install-client-helm.bats case-for-case: SS3 around content, SS3-only (arrows, Home/End, F1/F2) to empty, SS3+CSI mixed, a bare O is not an escape, truncated ESC O, the unknown family alone and beside content, and the floor counting non-Latin letters as content. The non-Latin case is asserted as a PAIR with the ASCII one. Either alone proves nothing: the point is that keep-vs-reject does not depend on the script a name is written in (Bugbot, #736), and only the two together say that. Mutation-proven, each anchor confirmed to apply, 12 cases in the block: drop SS3 from the strip (the cli#516 bug) -> 2 fail remove the floor entirely -> 2 fail floor uses [A-Za-z] instead of \p{L} -> 1 fail match O<final> without requiring ESC -> 4 fail Full Pester suite: 671 passed, 0 failed, 13 skipped. This is backend#2084's carved-out sub-task, not the ticket. The structural half — one corpus file all three suites derive from, with a cross-repo drift check — is untouched here, and deliberately: a fixture wired into two of the three implementations is the same defect wearing a different hat. Refs backend#2084

@saadqbal

… path (backend#1907) (#747) * feat(telemetry): one outcome event per install, with nowhere to put a path (backend#1907) The installer is the highest-variance, least-observed step in the product: it runs on machines we have never seen, under package managers, proxies and shells we do not control, and it reports to nobody. Each of the backend#736 failures — the CLI landing in ~/.local/bin with PATH advice only printed, apt-get appearing hung because unattended-upgrades held the dpkg lock — was invisible until a customer happened to mention it. scripts/lib/telemetry.sh emits one contract-shaped event per run from install_cleanup, the EXIT trap, so it fires on every path including the interrupted and the fatal one. It carries the phase reached, per-phase durations, the exit code, the client state, OS/arch, the version, an error class, and — for the #736 PATH case specifically — TB_CLI_ON_FRESH_PATH, which install-cli.sh has always computed and only ever printed advice about. "NO ARGUMENTS, NO PATHS, NO DATA" IS A SHAPE, NOT A RULE. Every value goes through _telemetry_attr, which admits a string only if it matches ^[A-Za-z0-9._-]{1,64}$ and an integer only if it is one. A path contains '/', a proxy credential contains ':' and '@', a token is longer than 64 characters, a name contains a space. Values that fail are dropped, never trimmed: a redactor has to imagine what it is stripping, and a shape only admits what it was told to. The phase and the client state are additionally checked against their closed sets at the render boundary — a canary assigned straight to TB_TELEMETRY_PHASE reached the record before that line existed, and it was shaped exactly like a legal value, so the token regex waved it through. The vocabularies are DERIVED, and a new guard proves it. scripts/tests/telemetry-vocabulary-agreement.sh parses install-k8s.sh's step_header calls, summary.sh's CLIENT_STATE writers, gen-manifest.sh's FILES array and install.sh's release-tag regex, and compares each to telemetry.sh's declaration; the error classes have no second declaration, so it exercises the classifier over the full cross-product and checks both that every answer is registered and that every registered class is reachable. It runs in drift-checks' `Source-of-truth drift` job, which is required — a guard in a job nobody must wait for is advice. It found one thing on its first run: summary.sh's own CLIENT_STATE docstring had been missing image_pull_ca since #424. TWO REAL BUGS THE TESTS CAUGHT, both of the same shape and both fatal: `printf | grep -q` returns 141 on a match under `set -o pipefail`, and so does `tr -dc < /dev/urandom | head -c 16` — the latter at SOURCE time, which killed the whole installer before it printed a line. Every unit-level test passed throughout; only the test that runs install_cleanup for real under the installer's own shell options went red. That test stays. install-bootstrap.bats held a hand-written second copy of install.sh's FILES array, in two places, so adding a lib turned ten unrelated supply-chain tests red. It now derives the list, and fails closed on an inert parse. WHAT IS NOT CONNECTED: the transport. The 17 Aug decision (rfcs#28) replaced the Collector gateway with an ingest endpoint on the backend — backend#1905, which does not exist yet — so _telemetry_deliver writes the install log and a bounded 0600 local spool that #1906's forwarder can drain, and posts nothing. Opt-out (default on) via TRACEBLOC_NO_TELEMETRY or DO_NOT_TRACK, documented in --help — and that promise is itself checked, because a user who exports a stale name believes they have opted out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(telemetry): drop the literal `curl -u user:pass` from a canary fixture (backend#1907) gitleaks' curl-auth-user rule fired on the TB_ERR_CMD fixture, and it was right to: a source file containing that spelling is a finding whatever the surrounding test claims, and a reviewer scanning the diff has no way to tell a canary from the real thing at a glance. The fixture's purpose is unchanged — TB_ERR_CMD holds the failing command UNEXPANDED, which is free text carrying a path, and must not be emitted. It now carries a path instead of a credential. The credential half of the same test is already covered by HTTPS_PROXY, which encodes user:pass in a proxy URL and is what a hospital network actually configures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(telemetry): a --help run is not a successful install (backend#1907) install_cleanup is the EXIT trap, so it fires for every exit of install-k8s.sh — including the terminal commands that touch no machine. `--help` exits 0 and was emitting a full install.run.succeeded with phase `bootstrap`. Reproduced: $ HOST_DATA_DIR=$T bash scripts/install-k8s.sh --help {"attributes":{"event.name":"install.run.succeeded", "tracebloc.install.phase":"bootstrap","tracebloc.install.exit_code":0,…}} That is the worst bug this feature could have. `--help` is the command people run MOST while a real install is broken, so a free success lands in the denominator of the failure RATE — the one number the ticket exists to produce — and moves it in the direction that hides the problem. Found by Bugbot on client#747. The fix is a latch, not a phase test: main() calls telemetry_run_started once --help / --diagnose / prepare-host have had their chance to dispatch, and telemetry_emit_outcome returns early without it. A phase test would have been wrong — a genuine failure IN the bootstrap phase (the leftover-data guard, validate_config) is an install attempt and must still be reported, which is now pinned by a test that drives the real entrypoint into a validate_config rejection. Found while fixing it: the assess handoff exits 0 having run no step, so counting it as succeeded would grow the success count with re-runs on machines nothing happened to. It now emits install.run.skipped — a registered outcome verb (contract §6.4), so no new vocabulary — which also makes "how often is the installer re-run on a machine that was already done" answerable. prepare-host is deliberately still not reported: it is a different command with its own registry component (§10.1), and filing it under tracebloc.component=install would be mislabelling it rather than measuring it. TWO OF THE SIX NEW MUTATIONS CAME BACK INERT on the first pass — "main stops setting the latch" and "assess.sh stops marking the handoff" — because the tests set those flags themselves and so could not see the WIRING disappear. That is the same class as the bug Bugbot found: a unit test of the emitter cannot observe which exits reach the trap. Both are now driven end to end, through the real install-k8s.sh and the real _assess_handoff, and both mutations redden. Bugbot's second finding (the source lookup aborting the whole event under set -e) is reported as unreachable with evidence rather than fixed: a command substitution in an ARGUMENT position does not propagate its status to the enclosing command, measured on bash 3.2.57 and 5.x. The invariant is pinned anyway — an unrecognised source location drops the field, never the event. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(telemetry): the phase that was still running had no duration (backend#1907) telemetry_phase_begin only closes a phase when the NEXT one starts, and nothing closed the active phase before the event rendered. That lost the most important number in the file, and it lost it in exactly the case the ticket names: * on every SUCCESSFUL install, phase_connect_ms was absent — the readiness wait, up to READY_TIMEOUT (600s), the single longest phase; * on every failure and every cancel, the phase named by tracebloc.install.phase had no duration. The dpkg-lock case in its likeliest real form is stuck twenty minutes in `prerequisites` and then killed or given up on, never reaching step c. Reproduced before fixing: "tracebloc.install.phase":"prerequisites" "tracebloc.install.phase_preflight_ms":0 <no prerequisites duration at all> * `bootstrap` had no key at all, because the loop iterated the letter map and bootstrap has no step letter. So the download + verify + leftover-guard + assess time was an unnamed remainder — which is also why subtracting the other keys from duration_ms could not recover the missing active phase. Found by Bugbot on client#747. My own "a slow phase is visible" test passed throughout, because it only ever measured a phase a later step_header had closed — the exact shape of a test that proves the easy half. The live delta is added at READ time rather than by a "close the phase" call in the emit path, so render stays idempotent: the tests call it repeatedly, and a render that mutated the accumulators would report different numbers each time. The clock is now read once per event, so the per-phase numbers and the total are exactly consistent — which is an invariant a test asserts. That test needed a FAKE CLOCK. The obvious fixture is wrong: winding _TB_TELEMETRY_PHASE_STARTED_MS backwards after the step_headers have already attributed that time invents milliseconds that never elapsed, and the first version failed for precisely that reason (sum 1200000 vs total 900000). Four mutations run. Three redden — dropping the live delta, going back to the letter map, and counting the delta for every phase. The fourth (reading the clock per attribute instead of once) is INERT and is reported as inert rather than counted: _telemetry_now_ms has second resolution, so two reads inside the same second are identical, and the guard only matters across a second boundary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(telemetry): the spool must not create the data dir the installer refused (backend#1907) _telemetry_deliver ran from the EXIT trap and did `mkdir -p "$HOST_DATA_DIR/telemetry"` unconditionally — including on the path where early_data_dir_guard had just REFUSED that directory for being on a network filesystem and called `error`. That guard deliberately skips an existing directory ("an EXISTING data dir has no at-risk mkdir here", client#441), so anything creating the directory behind its back disarms it for every later run: run 1: guard refuses (dir absent) -> error -> EXIT trap -> telemetry creates it run 2: guard sees the dir, returns 0 -> MySQL installs onto NFS which is exactly the InnoDB corruption client#432 exists to prevent, reintroduced by the telemetry that was only supposed to watch. Reproduced before fixing: guard exit=1 HOST_DATA_DIR created on the REJECTED volume? YES ./nfs-volume/.tracebloc/telemetry An observer that changes the install's own preconditions is not an observer. The spool now only writes INTO a data dir that already exists; a run that dies before that still reports through the install log, which is what a support bundle collects and which _choose_log_file has already placed somewhere safe (falling back to $TMPDIR). Found by Bugbot on client#747 — the third real finding of three rounds, and the most serious: the other two corrupted the metric, this one corrupted a customer's database. Both directions mutation-proven: removing the existence check reddens the new test, and disabling delivery outright reddens four others, so the fix cannot pass by simply turning the feature off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(telemetry): a pre-log failure had nowhere to go, so it went nowhere (backend#1907) Fallout from the previous commit, and the worst kind: the fix that stopped telemetry disarming the NFS guard also made the NFS refusal itself invisible. _telemetry_deliver's comment claimed "the install log always gets it". It does not. `log` is a no-op until setup_log_file sets LOG_FILE, and setup_log_file runs AFTER validate_config and early_data_dir_guard — deliberately, because #432 refuses a network data dir BEFORE logging starts. So on exactly those paths there was no log AND (correctly, since the previous commit) no data dir, and the rendered event was discarded: $ early_data_dir_guard # target reads as nfs guard exit=1 any telemetry written anywhere? 0 A run refused for being on NFS is a real, actionable field failure, and it was the single case producing no record at all — invisible to the very failure rate this feature exists to produce. Those pre-log failures are also precisely the class the run-started latch was built to preserve, so losing them undid that too. Found by Bugbot on client#747, which also spotted that this file's own NFS test masked the bug by setting LOG_FILE=/dev/null. That test now leaves LOG_FILE unset, as the real path does, and asserts the refusal IS reported. Fix: when there is no data dir, spool to a mktemp'd file in $TMPDIR. mktemp and not a fixed name — /tmp is world-writable on Linux and the installer runs privileged steps, so a predictable path is a symlink target for an append that may be running under sudo; mktemp creates with O_EXCL. This mirrors _choose_log_file's own fallback, so an early-failure run leaves one small file beside the install log it already leaves there, rather than a new class of litter. #1906's forwarder reads both locations. A comment that claims something untrue is itself the defect (workspace CLAUDE.md rule 7), so the false claim is replaced with what actually holds and why. Four mutations, all reddening: removing the fallback, giving it a predictable shared path, dropping its 0600 mode, and dropping the data-dir existence test (which would disarm the NFS guard again). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(telemetry): the record went into the directory the bootstrap deletes (backend#1907) Four findings from @saadqbal's review, each reproduced before it was fixed, plus the nit. The first one is the significant one: it silently undid round 4 on the primary macOS path. 1. The fallback spool landed inside the bootstrap's own scratch dir. install.sh:238 does `TMPDIR="$(mktemp -d)"` and :239 traps `rm -rf "$TMPDIR"`. A plain assignment to a name that is ALREADY EXPORTED keeps the export attribute — and TMPDIR is always exported on macOS — so install-k8s.sh inherited the doomed directory and `mktemp "${TMPDIR:-/tmp}/…"` wrote the record into it. Reproduced end-to-end: the spooled file was gone the moment the bootstrap returned. So the NFS refusal, and every other pre-setup_log_file failure, still produced no record anywhere on macOS — the exact hole the fallback closed. _telemetry_fallback_dir now disqualifies TMPDIR when the running installer is inside it, which is true precisely when TMPDIR is the bootstrap's scratch dir, and falls back to $HOME (never $HOME/.tracebloc — telemetry must not create HOST_DATA_DIR) and then /tmp. DERIVED rather than agreed: asking install.sh to export its original TMPDIR under another name would work only when the bootstrap is new, and install.sh is served from a URL a user may have curl'd months ago. Both sides of that comparison are resolved with `pwd -P`. The first cut compared them as written and missed every Mac, because /var is a symlink to /private/var — caught by re-running the reproduction against the fix, not by reading it. 2. grep is line-based, so a value with a newline passed the shape check. The one input shape "nowhere for a path to go" does not cover, because what lands is not a path — it is a second line: TB_VERSION=$'v1.9.3\n","tracebloc.install.injected":"yes' → "service.version":"v1.9.3 ","tracebloc.install.injected":"yes",… A forged attribute AND one record split across two lines of a .jsonl spool, so #1906's forwarder reads two malformed events. Reproduced on all four checks — the key, str and int shapes in _telemetry_attr, _telemetry_version, and _telemetry_source_line's inline regex. All now `[[ =~ ]]`, which anchors at end of string. As a bonus it removes the external process, so the backend#1778 SIGPIPE class the here-strings were working around cannot recur here at all. The agreement check proved the two version regexes were byte-identical while they behaved differently, and reported that as "the service.version shape is install.sh's own release-tag gate" — a claim about behaviour that byte-identity does not support, because each side was matched with a different operator. It now checks both: byte-identity, then verdict agreement over a corpus, each side evaluated the way the file that owns it evaluates it. The corpus is written down independently of either matcher and the check fails closed if it contains no embedded-newline input, since without one it degenerates into the byte check. 3. exit 2 is the "complete this step and re-run" handoff, not a failure. gpu-nvidia.sh:55 exits 2 after install_nvidia_drivers SUCCEEDED, to ask for a reboot. That call sits under step_header b, so every unattended GPU host's first install booked an `install.run.failed` with error.type=prerequisites_failed — a fabricated prerequisite failure in the rate this ticket exists to produce. Same shape as the --help bug, opposite direction; install_cleanup has treated 2 as its own outcome ("Re-run required") since client#681. It now renders install.run.cancelled and carries no error.type. It rides an existing verb rather than a new one because §6.4's outcome list is closed and adding to it is a PR against the contract, not an emitter's unilateral call; of the registered verbs, `cancelled` is the only terminal one that is true here. exit_code stays on the record, so 2 (handoff) and 130/143 (Ctrl-C) remain separable — which is why the exit code is an attribute in the first place. Event names are now a declared closed set with a guard. The guard derives the emitted names two ways — the literals in the case statement, and what the function actually renders over the installer's exit codes — rather than reading the declaration twice, and checks §6.1's grammar. It cannot check the §6.4 half from this repo: the verb registry is in rfcs, and a hand-copied second list of verbs would be the defect rather than the fix. 4. The chmod ran before the trim replaced the file. `tail > "${spool}.tmp"` creates under the process umask and `mv` keeps the tmp file's mode, so 0600 did not survive. common.sh's `umask 077` normally covers it, but _install_userspace_tools (setup-linux.sh:893) and its macOS twin set `umask 022` and restore it only afterwards. Reproduced: spool 644. The chmod now runs on the inode that survives, before the mv — one chmod, not two, because a second one on the spool afterwards is unreachable belt and braces that no test can redden. The test pinning 600 could not see any of this, because load_lib sources common.sh first and every test therefore ran under 077. There is now one that sets umask 022 and asserts the umask actually took. 5. nit: the comments claimed coverage the file does not have. `bootstrap` means "install-k8s.sh before step a", not "everything before step a" — download and verify happen in install.sh, which never sources this file and whose EXIT trap is `rm -rf "$TMPDIR"`. install.sh in TB_TELEMETRY_SOURCES is unreachable for the same reason: TB_ERR_LOC has exactly one writer, common.sh's _record_err. Both comments now say so. No bootstrap telemetry added. Also noted on install.run.skipped, which reads wider than it is: install.sh:132 reaches a healthy machine and `exec tracebloc`s at :144 before install-k8s.sh is fetched, so on curl|bash the assess gate is never reached at all. Tests: 4 new bats tests (36 in telemetry.bats, 1161 across the suite, all green), 3 new checks in the agreement guard. Every fix mutation-proved: 11 mutations, 11 reddened, each with its anchor asserted. Two first-pass mutations came back inert and are fixed rather than counted — one removed a redundant chmod nothing could observe (the redundancy is now gone), the other rewrote the guard's own detector alongside its corpus so the detector matched its mutated needle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(installer): regenerate the manifest after the handoff-marker change scripts/gen-manifest.sh output, required by the Static analysis R8 gate after any installer script changes (backend#1907). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(telemetry): the fallback was unreachable in the case it was built for (backend#1907) Two Bugbot findings, both reproduced before either was touched. 1. HIGH — a failed data-dir spool write dropped the outcome event. `_telemetry_deliver` had `|| return 0` on both the spool's `mkdir -p` and its append, so a data dir that EXISTS but refuses the write ended the function with the record nowhere — never reaching the $TMPDIR fallback added in 46a33de. That is not an exotic path: HOST_DATA_DIR present but unwritable is precisely when `_choose_log_file` (common.sh:769-775) has already fallen back to a mktemp log, and on curl|bash that log sits in the bootstrap's own doomed TMPDIR, so the `log` line kept nothing either. The fallback existed and was unreachable in exactly the case it was written for. Reproduced both halves, each with an anchor proving the fixture was not inert: * a 0500 data dir with no telemetry/ yet. Anchor A: `_choose_log_file` really does fall back out of that dir (called, not asserted). Anchor B: telemetry/ was not created afterwards, so the mkdir really failed. Result: no data-dir spool, no fallback file. The event was gone. * pending.jsonl replaced by a directory, so the append cannot open it. Same result, and it holds for root too. Both now fall through. The single-write guarantee comes from control flow rather than a flag: the `return 0` sits after a SUCCESSFUL append and nowhere else, so the fallback is reachable only on a path that wrote nothing. Everything after the append — chmod, trim, mv — may fail freely, because the line is already on disk and re-filing it would turn one install into two rows. The control case still writes exactly one spool line and zero fallback files. The trim moved into `_telemetry_trim_spool` so the append's success is the last thing in that branch. Inline, the trim sat between the append and the return, which is what made it easy to write the `|| return` that skipped the fallback decision in the first place. Also fixed on the same line: `2>/dev/null` now precedes the `>>`. Redirections apply left to right and a failing `>>` is reported by the SHELL, not by printf, so the old order printed `…/pending.jsonl: Is a directory` — with the customer's path — out of an EXIT trap. 2. LOW — a line number was emitted with no file to attach it to. `tracebloc.install.source` and `tracebloc.install.source_line` were two independent gates over one fact. A location whose basename is outside TB_TELEMETRY_SOURCES dropped the file and kept the number. Reproduced: `/home/someone/evil.sh:9` rendered `"tracebloc.install.source_line":9` with no source key, and so did `?:118` — the shape the ERR trap produces whenever BASH_SOURCE is empty, which is a real installer path, not a synthetic one. A line with no file is not a partial answer; it is a confident wrong one. The gate is now `_telemetry_source_basename`'s own exit status, so TB_TELEMETRY_SOURCES stays the single declaration of what counts as one of our files. Deliberately not gated the other way: TB_ERR_LOC has exactly one writer in the tree (install-k8s.sh:118) and it always appends `:${LINENO}`, so source-without-line is unreachable — a branch for it would be belt and braces no test could redden. A file with no line is honest information anyway. Mutation results — 10 mutations, each asserting its own anchor applied first: M1 both halves return instead of falling through RED M2 only the mkdir half returns RED M3 only the append half returns RED M4 a successful append no longer returns (double file) RED M5 source and line gated independently again RED M5b the same mutation vs the PRE-EXISTING source test RED (it now covers the line) M6 common.sh dropped from TB_TELEMETRY_SOURCES GREEN — survivor, by design M6b cluster.sh dropped (the anchor names it) RED M7a the trim's else-branch cleanup removed RED M7b a failed trim treated as success RED M7c the trim is never called RED M7d a failed mv leaves the trimmed copy behind RED M6 is a deliberate survivor and is reported rather than hidden: the pairing test derives its expectation FROM the vocabulary, so a consistent removal moves both sides. Verified that the gate which owns the vocabulary catches it instead — telemetry-vocabulary-agreement.sh goes red with "source basenames disagrees with gen-manifest.sh's FILES array". The independent anchor at the end of the pairing test also names cluster.sh in its own right, which is why M6b reddens. M7 found a defect in this commit's own first draft. The trim-failure fixture broke `tail` with a PATH shim, and common.sh:8 PREPENDS the system directories to PATH, so the real `tail` ran and both assertions passed against a trim that had worked perfectly. Rewritten to use a shell function — which beats PATH lookup outright — plus a behavioural anchor: SPOOL_MAX=3 against 7 spooled lines must leave 7, or the trim ran after all. scripts/manifest.sha256 regenerated (Static analysis R8). Full suite: 1168 bats tests green. `make lint` clean; telemetry.sh clean under `shellcheck -S warning -x` and `bash -n` on bash 3.2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(telemetry): a Ctrl-C on the already-set-up screen cancelled nothing (backend#1907) Third Bugbot finding on this branch, reproduced before it was touched. `_assess_handoff` marks the run skipped and then hands the user the interactive `tracebloc` home screen before `exit 0`, with install-k8s.sh:122's `trap 'exit 130' INT` live. The 130/143 branch of telemetry_render_event booked `cancelled` without consulting `_TB_TELEMETRY_SKIPPED`, so Ctrl-C on that screen — the most ordinary thing a user does there — filed a cancelled install for a run that installed nothing. Reproduced through the real INT and EXIT traps, not just the render function, with the skipped latch set identically in both arms: skipped run, ordinary exit 0 -> install.run.skipped skipped run, then Ctrl-C (130) -> install.run.cancelled <- the defect `cancelled` asserts that an install was cancelled. On this path there was no install to cancel, and the row would land in the denominator of "how often do installs not complete" — inflating it with runs that never attempted anything. Same class and same direction as the `--help` bug fixed earlier on this branch: it makes the product look worse while telling nobody anything actionable. No contract change. §6.4's verb list is untouched — no verb added, removed or redefined. Both `skipped` and `cancelled` were already registered and already emitted by this file; only which of the two a given run books changes, and `skipped` is the true one here. A wrong verb from a closed list is an ordinary defect. SKIPPED DOES NOT WIN OVER A FAILURE, and the asymmetry is deliberate. The flag is consulted only on the exits that mean nothing was installed — 0 and the two signals. A skipped run that then dies with a real non-zero stays `failed` and keeps its error.type, so the `*)` branch does not look at the flag. The shorter spelling — hoisting a blanket "skipped wins" ahead of the case — would have hidden a genuine failure, which is why it is not used and why there is a test for it. The comment claiming 130/143 were "unconditional" is now false and was rewritten rather than left to mislead the next reader. Mutation results, each asserting its anchor applied first: N1 130/143 unconditional again (the reported defect) RED N2 130/143 ALWAYS skipped (deletes the cancelled signal) RED <- positive control N3 the flag test inverted RED N4 only 130 consults the flag, 143 forgotten RED N5 blanket skipped-wins hoisted ahead of the case RED N5b only the failure branch consults the flag RED N2 is the control that matters: without it, "renders skipped" would be satisfied by a change that never renders cancelled at all, which would silently delete the interrupted-install signal. N5b exists because N5 tripped an earlier assertion (exit_code) before reaching the failure-swallowing one — it mutates only the `*)` branch, leaving exit_code untouched, and proves that assertion is live rather than decorative. telemetry-vocabulary-agreement.sh still passes unchanged: the case statement gains no new event-name literal, and its (b) sweep already exercised 130 x skipped=1, so the input domain was already derived from the producer. scripts/manifest.sha256 regenerated (Static analysis R8). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(telemetry): allowlist the canary curl -u fixture in .gitleaks.toml (backend#1907) The telemetry canary tests feed a fabricated `curl -u $CANARY:hunter2` through the redaction guards to prove they strip credentials. The value is never a real secret, but a git-mode range scan keeps re-finding it in earlier commits of this branch (commit 72af29e) even after the fixture was refactored, so a code change cannot clear it. Per code-quality.yml, a deliberate false positive belongs in .gitleaks.toml (commit-independent), not the baseline. Scoped to the exact canary match; default rules extended. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

@aptracebloc

…753) (#754) shellcheck ran in three places. Two apt-installed it and hung 35+ minutes on 2026-08-19; the third, the org reusable `quality / shellcheck`, passed in ~5 s because shellcheck is preinstalled on ubuntu-latest (0.9.0). The comment justifying the install -- "shellcheck itself is not on the runner image" -- was false. Measured before deleting: the two enumerated lists were byte-identical to each other, expanded to 34 files, and were a strict subset of the derived set. The enumeration had drifted past eight real scripts, and a parse error planted in index-invariants.sh was invisible to it. Both local copies are gone; the Makefile now derives the set instead of listing it. Scope is stated honestly rather than overclaimed: `quality / shellcheck` reads only the PR diff, so `Standard checks / Lint` runs `make lint` over the whole tree. Two complementary required gates, neither installing anything, neither holding a file list. Verified green under the runner's 0.9.0, not just a local 0.11.0. Also: `make parse` now fails closed (it exited 0 on a missing scripts/ dir while being the entire content of a required check), both bats installs are bounded and retried after one stalled on this very PR, every job in standard-checks.yml has a timeout, and four comments that had gone false were fixed -- file counts removed from prose entirely, since they rotted twice in one day. Review by @aptracebloc and @saadqbal found three real defects in this PR, all fixed here. Follow-ups deliberately left open: backend#2182 / #2183 (make the R8 manifest gate required) and #755's rebase, whose `lint-warnings` will silently no-op against the deleted SHELLCHECK_FILES if it is merged without one. Closes#753

…ent#748) (#756) * fix(installer): make the macOS amd64-emulation gate engine-aware (client#748) assert_amd64_emulation refused every Apple Silicon Mac whenever Rosetta amd64 was off, hard-erroring that 'the client images are amd64-only' — but only the MySQL 5.7 image is; the multi-arch 8.4 engine runs natively. A FRESH Mac that _mysql_engine_decision serves on 8.4 was turned away for emulation it does not need (Bugbot High). Ask the SAME rule the Linux gate uses: skip the smoke test when the engine resolves to 8.4. Safe before the cluster exists — the sticky check reads values_file and the datadir check reads HOST_DATA_DIR (default ~/.tracebloc), both known here, and on macOS the data lives on that host path, so an existing 5.7 install is still seen (helm-list existing_id, the one early-unavailable signal, is not how macOS surfaces prior data). Tighten the 5.7-path messages to name the 5.7 engine rather than 'all client images'. Tests: 8.4 -> skips the smoke / no docker run; 5.7 + broken emulation -> fails naming the 5.7 engine. Mutation-proved. The smoke-path tests now mock the engine to 5.7 explicitly rather than passing because the function is unsourced. 13/13 setup-macos-arch; manifest regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(installer): use the TB_NAMESPACE-sanitising engine wrapper on the macOS gate Bugbot (High) on the first push: assert_amd64_emulation called _mysql_engine_decision directly, which reads TB_NAMESPACE via dynamic scope — and it is usually UNSET this early, so only HOST_DATA_DIR/mysql was probed, not the per-release HOST_DATA_DIR/<ns>/mysql. A re-run with existing 5.7 data in the per-release dir then resolved 8.4, skipped the smoke, and crash-looped later (no Linux gate on macOS). Call _pf_mysql_engine_decision instead — the same wrapper the preflight gate uses: it sets values_file and the SANITISED TB_NAMESPACE, and FAILS CLOSED to 5.7 if the engine lib is missing. 7/7 assert tests; manifest regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(installer): fail-closed macOS arch gate — late backstop, shared emulation check (Arturo, client#756) Arturo's review: the early assert_amd64_emulation skips on an 8.4 GUESS, but existing_id needs a live release (unknowable before helm), so on TB_STORAGE_MODE= node-local or a --data-dir re-run an existing 5.7 release read as 8.4, skipped the smoke, and crash-looped — the Linux backstop _assert_engine_runs_on_this_arch returned early on macOS, so macOS had NO late gate. - Extract _macos_amd64_emulation_ok (the Rosetta/Docker smoke, as a boolean) and _macos_amd64_refusal (the Rosetta remedy). One emulation answer in one place. - _assert_engine_runs_on_this_arch now runs on macOS too, with a PER-OS emulation check (binfmt on Linux, the smoke on macOS — binfmt does not exist there) and fails closed. The early gate stays optimistic; this is the backstop it relies on. - assert_amd64_emulation uses the shared helpers + ${_decision%% *} (no awk, no 2>/dev/null), per _pf_arch. Manifest regenerated. Tests follow in the next commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(installer): pin the macOS backstop with the real engine rule (Arturo, client#756) Point 2 of the review: the macOS assert tests mocked _mysql_engine_decision (the raw rule), so the wrapper and the raw rule were indistinguishable — reverting the fix stayed green. Source the REAL rule (as preflight.bats does) and drive verdicts from HOST_DATA_DIR fixtures. New PER-RELEASE test builds HOST_DATA_DIR/<ns>/mysql 5.7 data with TB_NAMESPACE unset: the wrapper's sanitised namespace finds it and refuses; the raw rule misses it and skips — so reverting to the raw rule reddens this test (mutation-proved). Late gate gains macOS coverage: emulation-ok proceeds, emulation-missing and helper-absent both refuse (fail closed). 13/13 setup-macos-arch, 7/7 late gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

LukasWodka · 2026-08-19T14:02:22Z

bugbot run

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 526fc49. Configure here.}

LukasWodka · 2026-08-19T14:16:24Z

Resolving threads to unblock the staging promotion:

Makefile macOS xargs (Medium, Bugbot) → tracebloc/backend#2200 (shipped-with-ticket, non-High policy)
scripts/lib/install-client-helm.sh (@saadqbal) → addressed by client#756 (reviewed by @aptracebloc); thread is outdated.

due to constraints on release train

LukasWodka · 2026-08-19T15:49:30Z

/fr-pass

Best-effort functional review passed (triage: merged clean, non-interactive; behavioral evidence limited while e2e journey is red — backend#2206). Advancing to Ready for prod.

LukasWodkaand others added 2 commits August 18, 2026 08:31

LukasWodka requested a review from saadqbal as a code owner August 18, 2026 08:37

cursorBot reviewed Aug 18, 2026
View reviewed changes

Comment threadscripts/lib/install-client-helm.sh
Comment threadscripts/lib/provision.sh

tracebloc-release-trainBot added gate-nudge Toggled by the release train to (re-)fire the fr-gate and removed gate-nudge Toggled by the release train to (re-)fire the fr-gate labels Aug 18, 2026

saadqbal previously requested changes Aug 18, 2026
View reviewed changes

Comment threadscripts/lib/install-client-helm.sh
Comment threadscripts/lib/install-client-helm.sh Outdated