ci(env): move the CLIENT_ENV vocabulary guard into the job that gates (backend#1729) - #715
Conversation
… (backend#1729) The guard added in #706 parses all four CLIENT_ENV declarations and compares them. It ran in helm-ci.yaml's `Helm lint` job. `Helm lint` is not a required status check on this repo -- the required contexts on develop are `Unit tests`, `Lint`, `quality/*`, `version-bump-gate/version-check` and `Source-of-truth drift` -- so a PR that made the four declarations disagree was mergeable with the guard red. It could advise; it could not block. That is the shape backend#1729 is about, in the epic's own flagship deliverable: a mechanism that appears to verify something without being connected to the outcome it claims to control. `Source-of-truth drift` is the correct home and needed no new configuration: - it is REQUIRED on develop and on main, so a disagreement blocks the merge; - it deliberately carries NO `paths:` on pull_request (its header records why -- a path-filtered required check bricks PRs outside those paths, which is what happened to #651/#657/#660 on 2026-08-11), so the guard now also runs on the installer-only PRs helm-ci's filter would have skipped; - it is already the home of the other duplicated-declaration guard (check-drift.sh), and the guard needs only bash + python3, ~2s. helm-ci loses the step and the two `paths:` entries that arrived with it in #706. `scripts/install-k8s.ps1` genuinely leaves -- helm-ci is the repo's heaviest workflow (a real k3d cluster plus two 4-platform matrices) and a PowerShell installer edit has no other reason to start it. The explicit `scripts/lib/common.sh` entry also goes, but that one was always redundant: the pre-existing `scripts/lib/**` glob still matches it, so helm-ci's triggering on a common.sh change is unchanged. Mutation-proved rather than assumed: with `staging)` renamed in scripts/lib/common.sh the guard exits 1; restored, it exits 0. The mutation anchor was asserted to have applied first -- an inert mutation and good coverage look identical in a log. Verified green on this branch before arming: all four declarations agree and all six spellings are exercised (dev 12, development 2, prod 11, production 4, staging 9, stg 6). Refs backend#1729
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bfa967d. Configure here.
saqlainsyed007
left a comment
There was a problem hiding this comment.
Reviewed for correctness (high effort). This is a clean CI-wiring change and I recommend it.
Traced the move end-to-end:
-
No coverage lost by the removed
paths:entries.scripts/lib/common.sh— still matched by the retainedscripts/lib/**glob, so helm-ci triggering on acommon.shchange is unchanged. ✓scripts/install-k8s.ps1— the PowerShell installer is exercised byinstaller-tests.yaml(scripts/**),windows-e2e.yaml, andrelease-helm-chart.yaml; helm-ci's own jobs (lint/template/unittest/ingestor-multiarch/e2e) never tested it beyond the vocabulary guard, which now runs indrift-checks. ✓scripts/tests/env-vocabulary-agreement.sh— the guard itself; now runs inSource-of-truth drift, which is unfiltered onpull_request, so an edit to it still triggers the check. ✓
-
The guard genuinely gates now.
Source-of-truth driftis a required context and its job name is unchanged by adding a step, so branch protection keeps recognizing it — no config change needed, as the PR states. -
Runtime deps satisfied in the new home.
env-vocabulary-agreement.shneeds only bash + python3 (present onubuntu-latest) and the checked-outclient/**+scripts/**sources; thedriftjob checks out the repo, so all four declarations andclient/testsare readable. The script fails closed (exit 2) if not, which is the correct posture for a required gate. -
Both changed workflow files re-parse as valid YAML; the added step is indented consistently with
check-drift.
Altitude is good: the fix lands the guard in the already-required job rather than bolting on a branch-protection special case. No correctness, cleanup, or convention issues found.
Resolves the helm-ci.yaml conflict with #715, which moved the CLIENT_ENV vocabulary-agreement guard out of `Helm lint` into drift-checks.yaml's required `Source-of-truth drift` job. Calling `make helm-vocab` from the lint job would have quietly put it back, so the guard moves to `make drift` (the target that mirrors the drift gate) and `helm-vocab` is now the chart vocabulary only. `make check` runs both, so the local tier is unchanged. Bugbot, two findings on b5c21fe: - Helm setup removed from lint job. Correct: dropping the azure/setup-helm v3.15.4 pin left both make targets running against whatever helm the runner image ships, and chart-env-vocabulary.sh branches on the helm version (its helper-backstop cases self-skip below 3.16), so the pin decides which assertions run. Installing helm is environment bootstrap, the same class as the shellcheck apt install this PR deliberately kept. Restored, with the same action + pin drift-checks.yaml uses. - Makefile missing from path filters. Correct: the lint job now executes the Makefile, so a Makefile-only edit to those targets has to be able to start this workflow. Added to both `paths:` lists. standard-checks.yml needs no equivalent -- it is deliberately not path-filtered at all, being a required check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
The vocabulary-agreement guard from #706 ran in
helm-ci.yaml→ jobHelm lint. That job is not a required status check on this repo. The required contexts ondevelopare:So a PR that made the four
CLIENT_ENVdeclarations disagree was mergeable with the guard red. It advised; it did not gate.That is backend#1729's own class, in backend#1729's own flagship deliverable — a mechanism that looks like verification but isn't connected to the outcome it claims to control.
What changed
Move the step into
drift-checks.yaml→Source-of-truth drift, which needed no new configuration to make this real:developand onmain— a disagreement now blocks the mergepaths:onpull_request, deliberately (its header records why: a path-filtered required check bricks PRs outside those paths — docs(claude): sync org-standards block (backend#1602) #651/chore(devex): install amake checkpre-push hook viamake setup(backend#1606) #657/docs(#1528): document the MySQL DB identity model + edgeuser retirement (D10) #660 on 2026-08-11). So the guard now also runs on the installer-only PRs helm-ci's filter skipped entirelycheck-drift.sh)helm-ciloses the step and the twopaths:entries that arrived with it in #706:scripts/install-k8s.ps1scripts/lib/common.shscripts/lib/**glob still matches it, so triggering is unchangedNo branch-protection change is required or included. The gate comes from landing in an already-required job.
Test plan
Mutation-proved rather than assumed:
The mutation anchor was asserted to have applied before trusting the result — an inert mutation and good coverage look identical in a log.
Armed while green. On this branch all four declarations agree and every accepted spelling is exercised:
dev12,development2,prod11,production4,staging9,stg6.Both workflow files re-parse as valid YAML;
Source-of-truth driftnow listscheck-drift+CLIENT_ENV vocabulary agreement.Checklist
developRefs backend#1729
Note
Low Risk
CI workflow wiring only; no application or chart runtime behavior changes. Effect is stronger enforcement of an existing guard via an already-required check.
Overview
Moves the CLIENT_ENV vocabulary agreement step (
env-vocabulary-agreement.sh) fromhelm-ci’sHelm lintjob intodrift-checks’sSource-of-truth driftjob so a mismatch across the four declarations (Go template, JSON Schema, bash, PowerShell) blocks merge instead of only failing a non-required check.drift-checksgains the new step and header comments that position it as the home for duplicated-declaration guards and note CLIENT_ENV alongside existing drift checks.helm-cidrops that step and removespaths:entries forscripts/tests/env-vocabulary-agreement.sh,scripts/install-k8s.ps1, and the explicitscripts/lib/common.shline (installer-only edits no longer trigger the heavy workflow;scripts/lib/**still coverscommon.sh).Reviewed by Cursor Bugbot for commit bfa967d. Bugbot is set up for automated code reviews on this repo. Configure here.