Uh oh!
There was an error while loading. Please reload this page.
release-train: develop -> staging - #876
Merged
Merged
Conversation
* ci(2651): split the chart test tier so it can be a required check The 585-test helm-unittest tier (34 suites, ~4s) lived in helm-ci.yaml and was required on no branch. All of it could be red and the PR still merged, which made every assertion in it advisory — including the ones #2606 had just repaired. backend#1729 rule 2: a guard in a non-required CI job is advice. A NEW WORKFLOW RATHER THAN A PATHS EDIT, and the reason matters because the ticket's scope assumed the edit. `helm-ci.yaml` could not simply lose its `pull_request` paths filter. It is the repo's heaviest workflow, and two jobs carry NO job-level `if:` — `upgrade-e2e` (Fleet auto-upgrade E2E, k3d, timeout 30m) and `seal-check-e2e` (Seal-check egress-enforcement, k3d, timeout 30m). They are gated ONLY by that filter, so dropping it would have started ~60 minutes of k3d E2E on every PR in the repo, docs-only ones included. Moving the cheap tier out costs nothing and leaves the expensive jobs byte-identical: helm-ci.yaml keeps its filter and now holds lint / template / ingestor-multiarch / the three e2e jobs. `helm-unit.yaml` therefore has NO `paths:` on `pull_request` — a path-filtered required check never creates its check run on a PR outside those paths, so GitHub leaves it at "Expected - waiting for status to be reported" and the PR is unmergeable forever. drift-checks.yaml's header records that blocking client#651, #657 and #660 on 2026-08-11. `push` keeps a filter: pushes are not gated by required checks, so scoping there is free. Same split drift-checks uses. scripts/tests/helm-unittest-gated.sh keeps it that way, and runs inside the REQUIRED `Source-of-truth drift` job — so the guard protecting the gate is itself gated. It refuses a `pull_request` paths filter, pins the job name (the name IS the status-check context, so a near-miss un-gates the tier exactly as silently as the paths trap), refuses a second job producing the same context, and refuses a gate that no longer runs `helm unittest ./client` — a required check that executes nothing reports success forever. WHAT THE GUARD DOES NOT DO, said in its header rather than implied: it does not read the live branch-protection list, so it cannot prove the context is currently required. That copy lives in GitHub's config, not this repo, and the drift job has no token for it. The header carries the `gh api` one-liner to check by hand. An earlier draft of helm-unit.yaml claimed the guard "asserts the name against the live protection list" — false, and precisely the rule-7 defect this ticket family is about, so it is corrected in the same change. ARMED WHILE GREEN (rule 4): `helm unittest ./client` on develop @ 76f4690 is 34 suites / 585 tests / 4.1s, all passing. Note 585, not the 581 the ticket measured — it has grown, which is the argument for gating it. Mutations on the guard, each asserting its anchor applied: G1 THE TRAP: paths filter on pull_request -> FAIL G2 rename the job to a near-miss -> FAIL G3 gate stops running the suite -> FAIL G4 a second workflow produces the context -> FAIL G5 workflow deleted -> FAIL G6 workflow unparseable -> FAIL G7 push loses its paths filter -> FAIL G8 NEGATIVE CONTROL, reword a comment -> PASS Makefile: the guard is registered in DRIFT_GUARDS, the repo's only declaration of that set. Two claims there are now false and fixed in the same change — that "`Helm unit tests` is required on neither" (it is the whole point of this PR), and that `helm-unittest` maps to "helm-ci.yaml `unittest`" (it moved). drift 24/24 green · helm-lint, helm-vocab, helm-template, check-style, shellcheck all clean. PROTECTION IS NOT CHANGED BY THIS PR. Adding the context to client/develop and client/main is an admin action on a shared repo, so it needs an explicit decision — the command and the measurements are in the PR body. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci(2651): the paths guard could not see paths-ignore (backend#2651) The guard exists to stop a required check falling into the permanent-pending trap, and it looked for `paths:` only. `paths-ignore:` is its sibling and skips the workflow the same way -- a PR outside either filter never gets a check run, so the required context sits at "Expected - waiting for status to be reported" forever. A later `paths-ignore:` would have reopened exactly the trap this guard exists to close, while the guard stayed green. Bugbot, #872. The same blind spot was in the `push` arm with the opposite sign: it warned "push lost its paths: filter" for a workflow filtered with `paths-ignore:` -- a false positive in the one place this guard is meant to be trusted. Both keys are now named once, in PATH_FILTER_KEYS, so the check and its own error message cannot disagree about which keys were examined. Two entries rather than a `paths*` prefix rule: the set is closed and short, and a prefix would also catch a future key that does something else. Mutation-proved against the real workflow, each anchor asserted before the run: pull_request + paths-ignore: -> exit 1, message names `paths-ignore:` pull_request + paths: -> exit 1, message names `paths:` (unchanged) push paths -> paths-ignore -> exit 0, no false "lost its filter" Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…nstall (#863) (#871) `Seal-check egress-enforcement (k3d)` failed intermittently BEFORE it tested anything: "resourceMonitor is enabled but the metrics.k8s.io/v1beta1 API is not registered." This is a harness race, not a chart defect. k3s registers its bundled metrics-server addon — and the v1beta1.metrics.k8s.io APIService the resource-monitor preflight looks up (client#823) — ASYNCHRONOUSLY, after nodes report Ready. The e2e harnesses gated only on `kubectl wait --for=condition=Ready nodes` and then helm-installed, so on a fast runner the install beat the addon and the preflight `fail`ed the whole release (#862 false-failed at 26s while #861 passed at 51s, neither touching the chart or these scripts). Fix: add e2e_wait_for_metrics_apiservice to scripts/tests/lib/e2e-common.sh and call it after node-Ready, before the helm install, in every harness that installs a preflight-carrying chart directly: e2e-seal-check.sh, e2e-full-seal.sh, and e2e-auto-upgrade.sh (whose first install is the last PUBLISHED chart, which carries the preflight too). The helper POLLS for the APIService to EXIST first — `kubectl wait` errors NotFound on a not-yet-created named object, so a bare `kubectl wait --for=condition=Available` would just swap one red for another in the same window — then best-effort waits for Available. It mirrors the production installer's _wait_for_metrics_apiservice (lib/install-client-helm.sh, client#553), which faces the identical race. Rejected the weaker options (resourceMonitor:false / metricsServerPreflight:false): both go green only by deleting the #823 preflight coverage this seal-check exists to exercise on a real cluster. New e2e-metrics-apiservice-wait.bats pins the invariant (all three harnesses wait before their first install; the wait polls-for-existence before the condition wait) so the guard cannot silently drift back out. Verified on real k3d (rancher/k3s:v1.36.3-k3s1): reproduced the exact resource-monitor-daemonset.yaml:69 fail when the APIService is absent; confirmed the helper blocks until registered+Available and the preflight then renders satisfied-by-apiservice. `make lint` clean; full bats suite failure set identical to develop tip (zero failures added). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…nt windows-e2e nightly (backend#2627) (#868) * fix(ci): watch scheduled workflows for silent staleness; retire dormant windows-e2e nightly (backend#2627) `Windows e2e (self-hosted)` ran nightly for 22 days and every run was `cancelled` (GitHub's 24h queue-timeout — no `self-hosted, windows, nested-virt` runner is registered). `cancelled` is not a red check and fires no alert, so a job that had NEVER once succeeded read as "the Windows path is covered nightly" for three weeks. Runner state can't be confirmed without org admin (`gh api .../actions/runners` -> 403); the 22-day queue-timeout streak is conclusive that no matching runner is online. Two parts: 1. Durable fix — a generic staleness watch (item 3, the class fix): - scripts/check-workflow-staleness.sh: read-only, test-seamed detector. For every SCHEDULED workflow in the repo it finds the most recent successful scheduled run and flags any whose newest completed scheduled run is non-success and whose last success is >= N days old (default 7). Gating on "newest completed run is non-success" makes a healthy-but-infrequent (e.g. weekly) job cadence-immune. Exits 0 on a finding on purpose — the alert is the filed issue, not a red check (the whole point of #2627 is that a red/cancelled scheduled check is the signal that gets ignored); it exits non-zero only when the watcher itself is broken, which stays loud. - scripts/alert-workflow-staleness.sh: files ONE deduplicated issue per stale workflow into the private catch-all `backend` (CI-health is internal work), labelled work-type:bug so it routes to Ready. Dedup by a hidden fingerprint marker, same shape as the e2e-agent. - .github/workflows/workflow-staleness-watch.yml: daily, GitHub-hosted (so it always runs). Mints a scoped tracebloc-release-train App token (issues:write on backend) for the cross-repo file — same App/pin as add-to-kanban.yml / envelope-contract-drift.yml. dry-run dispatch input. - bats for both scripts (offline, via the stub/clock seams). Class sweep (in the issue) confirms windows-e2e was the only self-hosted-targeting workflow in the org; the watch is generic so the next scheduled job can't rot silently. On its first real run it will file one issue for digest-drift.yml, which has failed on schedule ~13 days running unnoticed — the watch working as intended, triage separately. 2. Retire the phantom (item 2): remove the nightly `schedule:` trigger from windows-e2e.yaml (manual-dispatch only) with a DORMANT banner, and mark docs/WINDOWS-E2E.md dormant. I can't provision a self-hosted nested-virt runner (no infra/org admin), and the credentialed EC2 Windows journey (backend#2619, now on m7i.xlarge + NestedVirtualization=enabled) is the forward coverage — full deletion is the follow-up once it's green. Because it's no longer scheduled, the new watch correctly leaves it out of scope. Verified: shellcheck clean; 17 bats green; both workflows parse; the detector run live against tracebloc/client caught windows-e2e (22d) and digest-drift (13d) while healthy infrequent workflows stayed ok; end-to-end dry-run filed nothing. `make drift` fails only on a pre-existing local helm v4 kubeVersion mismatch (reproduces on clean origin/develop; CI pins v3.15.4). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): staleness detector fail-CLOSED, and bats assertions enforce (backend#2627) Two review findings on client#868: 1. Bugbot (HIGH): `die` inside `runs_for` could not stop the watcher. The function is called from `$(...)` and the script runs `set -uo pipefail` without `-e`, so `exit 2` only killed the subshell; the parent continued, printed `ok`, and exited 0 — a broken watcher (API/jq failure) looking healthy and filing nothing, the exact fail-OPEN this script claims to prevent. Fix: runs_for now `return`s non-zero on failure and the caller checks each subshell's status and `die`s in the PARENT shell (where exit works). Added a regression test: a corrupt runs payload now exits 2, not a silent `[]`/0. 2. bats-hygiene (test 77): every standalone `[ ]`/`[[ ]]` / `! cmd` assertion in both new bats files now ends in `|| return 1`, so a failing assertion fails its test rather than being advisory (scripts/tests/unenforced-assertions.awk). Verified: unenforced-assertions.awk clean over both files; bats-hygiene.bats 18/18; the two suites 18/18 (incl. the new fail-closed test); shellcheck -S warning -x clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): staleness runs via stdin + projected fields; tolerate 404 (backend#2627) Second Bugbot finding on client#868 (HIGH), plus a robustness follow-on: 1. jq --argjson hit the Linux argv limit. classify_one passed the whole workflow_runs array as a single `--argjson` argument, and runs_for fetched FULL run objects (nested repository/head_repository/actor/…). Linux caps one argv string at 128KiB (MAX_ARG_STRLEN); a daily workflow's ~100-run history exceeds that within weeks, so jq failed and — now that the watcher fails closed — it would die on ubuntu (never on macOS; the cap is Linux-only, and the 4-field stubs never reproduced it). Fixed two ways: - runs_for now PROJECTS each run to the four fields classify_one actually reads ({status, conclusion, created_at, html_url}) at the source, so the payload is tiny and the stubs are the real shape. - classify_one reads the runs array from STDIN instead of `--argjson`, which removes the argv ceiling entirely (belt-and-suspenders). New regression test feeds a >128KiB projected payload (200 padded runs) and asserts it classifies rather than dies. 2. runs_for now distinguishes a 404 (workflow file present but no registered workflow / no runs yet — a just-added scheduled workflow) from auth/other errors: 404 -> treat as an empty history and skip (the same "idle / brand-new" case classify_one already handles), everything else -> still fail closed. Without this, adding any new scheduled workflow could redden the whole watcher on its first tick and mask every other finding. Verified: shellcheck -S warning -x clean; unenforced-assertions.awk clean; both suites 19/19 (incl. the large-payload test); a live run against tracebloc/client flags digest-drift, 404-skips the not-yet-registered new workflow, and exits 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…phans (backend#2692) (#875) The perExperimentDbCreds RBAC grant gave jobs-manager `delete` on Secrets but not `list`. The orphan reaper enumerates (LISTs) cred Secrets to find any whose Job died before running cleanup, then deletes them — so it 403s on the LIST and can never reap an orphan. Orphaned credential material accumulates in etcd and the matching MySQL users are never revoked, the exact failure mode per-experiment creds exist to prevent. The reaper backs off quietly (WARNING/exponential), so it went unnoticed on dev (since 2026-08-25) and staging; prod is unaffected only because the flag is off. Add `list` to the existing flag-gated, secrets-only rule in both the ClusterRole and namespaced Role branches (the reaper runs regardless of clusterScope), giving `["list", "delete"]` — the same shape as the perDatasetPvcs GC grant directly below. The grant stays flag-gated and byte-for-byte unchanged when the flag is off. `list` on secrets does widen exposure namespace-wide; RBAC `list` cannot be resourceNames-scoped, so this is the minimal viable chart-level grant and the tradeoff is documented inline. Tests: rbac_test.yaml now asserts list+delete in both branches when on and no secrets-only rule of either shape in both branches when off; mutation-proven (reverting to delete-only reddens exactly the two on-tests). Chart.yaml 1.9.75 -> 1.9.76 for the version-bump gate. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 27, 2026
ContributorAuthor
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 897a899. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
commented
Aug 27, 2026
ContributorAuthor
Promoted with 2 open Medium/Low Bugbot finding(s), per the severity policy in release-train's README (High stops the line; Medium/Low are recorded and ship, at both hops): Required check skips PR retargets; Dedup search failures file duplicates This is a second look at once-reviewed code -- it passed per-feature review on the source branch, and has NOT had functional review yet (that happens on staging). Fix forward on What the train did with each:
|
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Flag-gated RBAC widens namespace secret list for orphan cleanup when per-experiment DB creds are enabled; remaining changes are CI and test-harness behavior with limited production blast radius.
Overview
This promotion bundles CI gating, scheduled-workflow hygiene, a chart RBAC fix, and k3d e2e stability work from develop.
Helm unit tests move out of
helm-ci.yamlinto a dedicatedhelm-unit.yamlworkflow whosepull_requesttrigger has no path filter, so theHelm unit testscontext can report on every PR and stay mergeable as a required check.helm-unittest-gated.shis added tomake driftto pin the job name, forbidpaths/paths-ignoreon PRs, and ensurehelm unittest ./clientactually runs.Windows self-hosted e2e drops its nightly
schedule:(phantomcancelledruns with no nested-virt runner); it stays manual-dispatch only, with docs noting EC2 Windows journey coverage instead.A new workflow staleness watch (daily cron +
check-workflow-staleness.sh/alert-workflow-staleness.sh) flags scheduled workflows whose latest completed run is not green and files deduplicated issues intracebloc/backend, with bats coverage.Chart 1.9.76: when
perExperimentDbCredsis on, jobs-manager RBAC gainslist+deleteon secrets (not delete-only) so the orphan cred reaper can enumerate before deleting; helm-unittest cases inrbac_test.yamlare extended accordingly.E2E harnesses call new
e2e_wait_for_metrics_apiserviceine2e-common.shbefore the firsthelm installin seal-check, full-seal, and auto-upgrade, avoiding resource-monitor preflight races whenmetrics.k8s.iois not registered yet;e2e-metrics-apiservice-wait.batslocks that contract.Reviewed by Cursor Bugbot for commit 897a899. Bugbot is set up for automated code reviews on this repo. Configure here.