Repository navigation
feat(workflows): reserve image names; build only names a team owns - #1648
Conversation
Tenant builds push, and tenant services run, ghcr.io/mctlhq/<component name>, so the component name is the image name, and nothing tied it to the team asking. check-image-name.sh decides from a reviewed list (config/image-names.txt: reserved platform names, a mctl- prefix, and explicit grants for the services that use them today) plus the services/ tree: a name another team already has is not available. It runs in deploy-service validation, in a new image-name step that gates preview-deploy's build and deploy, and in build-image.yaml before any credential is fetched, where a direct dispatch must also push to exactly ghcr.io/mctlhq/<component_name>. Direct dispatches no longer move :latest, which nothing pulls; release builds keep it. "platform", the team the platform's own image grants use, is reserved as a tenant name. Claude-Session: https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2
|
Claude finished @mashkovd's task in 10m 6s —— View job Review complete — changes requested (0 P1, 2 P2, 4 P3)
The design is sound and the script is carefully fail-closed. What I verified by reading the tree: input grammar, unparsable-list and missing-list all exit 2; P2 — should fix before merge1.
2. The new
P3 — nits
Not verifiedI could not execute anything in this environment — |
Antigravity (agy) review — informationalModel: gemini-3.8-flash-medium · commit Finding 1: Undefined workflow inputs break image name validation on direct dispatches
Finding 2: Malformed Argo DAG dependency prevents deployment when image build is skipped
Finding 3: Unreserved
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| # Platform images built from platform-gitops/images/ and dispatched by hand | ||
| # with team "platform" (not a tenant; wft-create-tenant refuses the name). | ||
| grant platform grafana-iac |
There was a problem hiding this comment.
P2 — team_name is a caller-asserted string in the Argo path, so the platform grants are reachable without being the platform.
These five lines give team platform the right to build and push grafana-iac, mc, mctl-agent, vault-iac and zitadel-iac — images that run with Vault/Zitadel/Grafana admin credentials. The guard added for that is reserving the tenant name in wft-create-tenant.yaml:150, but that only blocks creating a tenant called platform; it does not block passing team_name: platform to deploy-service or preview-deploy.
tpl-validate-tenant's validate never checks that the team exists. Its own annotation claims it does ("checks team exists in Git tenants/ directory", line 8), but grepping the template for tenants/ finds only that annotation — steps 1→8 validate the name grammar, dockerfile_repo, and the service directory, never platform-gitops/tenants/<team>/. So action: onboard with team_name: platform passes validation, check-image-name.sh matches grant platform vault-iac, and the build is dispatched.
Before this PR a spoofed team_name bought nothing (no name was tied to a team). Now it is the key to the whole ownership model, and platform is the highest-value value it can take.
Suggested fix, and it is small: have the Argo-side callers refuse a team with no platform-gitops/tenants/<team>/ directory — in validate (after the clone, before 3b) and in the new image-name template. platform is not in tenants/ (admins erpact karabu labs mctl ovk), so that makes the platform grants unreachable from the tenant-facing workflows while leaving build-image.yaml's hand-dispatched platform builds working. Putting the check inside check-image-name.sh instead would break those platform builds, so it belongs in the callers.
There was a problem hiding this comment.
Fixed in be70817, a variation on your suggestion. The tenant check lives in the script behind a --tenant flag, so it is tested in one place, and only the two Argo callers pass that flag.
- With
--tenant, the team must haveplatform-gitops/tenants/<team>/, andplatformis refused outright. The second rule holds even if atenants/platformdirectory ever appeared; there is a fixture for that. tpl-validate-tenant'svalidate(step 3b) and the newimage-nametemplate both run with--tenant.build-image.yamldoes not, so the hand-dispatched platform builds keep working. The test asserts both callers pass the flag, and mutations dropping it are caught.wft-create-tenantnow reservesadminsas well asplatform. That workflow continues when the tenant directory already exists, soadminsmust not be creatable again.
Binding the team to the workflow namespace was considered and rejected. Admins submit tenant operations from the default namespace, so namespace == team would break them.
| run: | | ||
| set -euo pipefail | ||
| expected="${REGISTRY}/${GITHUB_REPOSITORY_OWNER}/${INPUT_COMPONENT_NAME}" | ||
| if [ "$INPUT_IMAGE_NAME" != "$expected" ]; then |
There was a problem hiding this comment.
P2 — this pin is newly mandatory for dispatchers that live outside this repo, and nothing here verifies they satisfy it.
direct == true covers every workflow_dispatch, not just the two Argo templates. Per docs/runbooks/github-app-scope-audit.md:86-87, mctl-telegram/.github/workflows/preview-deploy.yml and the release-please.yml dispatch steps in mctl-api, mctl-agent, mctl-agents, mctl-portal, mctl-docs, mctl-academy, mctl-telegram, mctl-design and pfeifenpatenschaft-backend all hold actions:write on mctl-gitops. Any of them that dispatches build-image.yaml (rather than release-deploy.yaml) now has to pass image_name == ghcr.io/mctlhq/<component_name> exactly.
That pairing is not universal in this platform today: platform-gitops/services/labs/mctl-telegram-preview/values.yaml:17 runs ghcr.io/mctlhq/mctl-telegram, i.e. the component and the image genuinely differ for the preview service that the external preview-deploy.yml feeds. tests/test_check_image_name.py asserts the pin exists (line 183) but cannot assert that real callers match it, and grant labs mctl-telegram-preview in image-names.txt:57 is there only to satisfy "every existing service passes" — it does not tell us which component_name that dispatcher sends.
So the failure mode is: merge lands, the next mctl-telegram preview build fails at this step with image_name must be ghcr.io/mctlhq/mctl-telegram-preview, and there is no warn-only period to catch it. Worth confirming each external dispatcher's (image_name, component_name) pair before merge — or landing this half as a warning first (echo "::warning::" and no exit 1) and flipping it once the logs are clean.
The Argo-side pair is fine: wft-deploy-service.yaml:144 builds {{container_registry}}/{{component_name}} and wft-preview-deploy.yaml:166 builds ghcr.io/{GITOPS_ORG}/{component_name}, both matching.
There was a problem hiding this comment.
Verified on every mctlhq repository's default branch.
Only two kinds of caller dispatch build-image.yaml directly:
| Dispatcher | team | component | image_name | Passes |
|---|---|---|---|---|
mctl-openclaw upstream-sync-release.yml |
labs | openclaw | ghcr.io/mctlhq/openclaw | yes (grant labs openclaw) |
hand dispatch, platform-gitops/images/* |
platform | grafana-iac, mc, vault-iac, zitadel-iac, mctl-agent | ghcr.io/mctlhq/ | yes (grant platform ...) |
Every other dispatcher calls release-deploy.yaml, which reaches build-image.yaml as a workflow_call. The pin does not apply there:
release-please.yml: mctl-api, mctl-agent, mctl-agents, mctl-portal, mctl-docs, mctl-academy, mctl-telegram, mctl-web, portfolio, seerrsense;deploy.yml: mctl-design;release-deploy.yml: mctl-alice, pfeifenpatenschaft-backend;preview-deploy.yml: mctl-telegram. This one also covers themctl-telegram-preview→ghcr.io/mctlhq/mctl-telegramcase you mention.
The test now pins both direct dispatchers: each of their (team, component) pairs must pass in --build mode against the real tree.
| # images are named by their release and may differ from the component | ||
| # name, and release-deploy is dispatched by the release bots and | ||
| # operators, never by a tenant workflow. GITHUB_WORKFLOW_REF names the | ||
| # caller's workflow in a called run, so it tells the two apart. |
There was a problem hiding this comment.
P3 — the comment calls this "the second layer, at the push", but it is weaker than that, and the gap is worth naming in the comment rather than leaving implied.
Two ways past it, both available to a holder of the credential that already dispatches builds (actions:write on mctl-gitops, which is what Argo's Vault PAT uses at wft-deploy-service.yaml:498):
- Dispatch
release-deploy.yamlinstead. It isworkflow_dispatchwith a free-textimage_name, it callsbuild-image.yamlviaworkflow_call, sodirect=falseand the check is skipped entirely — and it then bumps the gitopsvalues.yamltoo. - Dispatch
build-image.yamlitself at a non-mainref. Thecasepattern accepts any ref after@, and the workflow definition comes from that ref, so a chosen ref means a chosen check. (Pinning the policy tomaincloses the data half of this but not the code half.)
Neither is tenant-reachable — tenants cannot choose which mctl-gitops workflow Argo dispatches — so the Argo-side check at tpl-validate-tenant.yaml:122 remains the real boundary and this PR is still a net improvement. But if the intent is defence in depth at the push, gating release-deploy.yaml's image_name against reserved/reserved-prefix for the given team (without requiring it to equal component_name, since release names legitimately differ) would cost a few lines and close case 1.
There was a problem hiding this comment.
Both gaps are now spelled out in the step's comment (be70817):
release-deploy.yamltakes a free-textimage_nameand arrives here as an uncheckedworkflow_call.- A dispatch at a ref other than
mainruns that ref's copy of the workflow. The policy data is read frommaineither way.
The comment also says that Argo's validation is the tenant boundary, and that this step is a backstop for holders of actions:write. Gating release-deploy's image_name is tracked as a follow-up rather than done here.
| exit 1 | ||
| fi | ||
| echo "✅ image name ${PARAM_COMPONENT_NAME} is available to ${PARAM_TEAM_NAME}" | ||
| resources: |
There was a problem hiding this comment.
P3 — requests without limits, unlike the other pods in this directory.
Every other cluster template that declares resources pairs the two: cronworkflow-labs-claude-remote-restart.yaml:59-64, cwft-mctl-agents-approve.yaml:167-168, cwft-mctl-agents-implement.yaml:360-361, cwft-mctl-agents-investigate.yaml:338-339. A git clone --depth=1 of mctl-gitops is not free either — the repo is large enough that a burstable-with-no-ceiling pod is the one thing standing between a slow clone and node pressure.
resources:
requests:
memory: 32Mi
cpu: 10m
limits:
memory: 128Mi
cpu: 200mThere was a problem hiding this comment.
Done in be70817. Both the new image-name pod and the existing validate pod now have limits: {memory: 128Mi, cpu: 200m} next to their requests. validate had no resources block at all before. The test asserts limits on both, and a mutation removing them is caught.
| # A service may run an image under a name other than its own directory | ||
| # (e.g. a preview of another service): those names are taken as well. | ||
| for values in "$SERVICES"/*/*/values.yaml; do | ||
| [ -f "$values" ] || continue |
There was a problem hiding this comment.
P3 — mctlhq is hardcoded here while every caller derives the org.
build-image.yaml builds expected from ${GITHUB_REPOSITORY_OWNER}, wft-preview-deploy.yaml:166 from GITOPS_ORG, wft-deploy-service.yaml:144 from the container_registry parameter. Only this grep assumes the string. The two cannot disagree today, but if they ever do, this loop silently stops claiming any name — it would find no match and fall through to exit 0, which is the fail-open direction, unlike every other failure path in the script.
Taking the registry prefix as a fourth argument (or reading it from the list as a registry line) keeps the fail-closed property uniform. Same for the comment on line 4.
There was a problem hiding this comment.
Fixed in be70817. The registry is now a required argument: check-image-name.sh [--tenant] [--build] <checkout> <registry> <team> <name>.
- It must equal the list's single
registry ghcr.io/mctlhqline, and the values scan uses it, so nothing in the script hard-codes the organisation. A mismatch, a missing registry line or a second registry line all exit 2. - Each caller passes its own derivation:
validatepasses deploy-service'scontainer_registry, newly handed to it as a bound env value;image-namepassesghcr.io/${GITOPS_ORG};build-image.yamlpasses${REGISTRY}/${GITHUB_REPOSITORY_OWNER}.
So a caller deriving a different organisation is refused instead of failing open.
| # openclaw), never an image named after the component, and their build input is pinned | ||
| # to mctlhq/mctl-openclaw by mctl-api. The name is shared by every team that | ||
| # deploys one. | ||
| grant * openclaw |
There was a problem hiding this comment.
P3 — the wildcard grant short-circuits rules 2 and 3, which is more than the comment above it needs.
Because rule 1 wins first, grant * openclaw means any team may build and push ghcr.io/mctlhq/openclaw from any dockerfile_repo, and the "another team already has services/<other>/openclaw" check never runs. That is harmless today — the openclaw services under labs, ovk and admins all run ghcr.io/mctlhq/mctl-openclaw, covered by reserved-prefix mctl-, and nothing in the repo or the cluster references ghcr.io/mctlhq/openclaw at all (the reference scan in tests/test_check_image_name.py would have failed otherwise, since openclaw is granted rather than reserved).
So the only thing the wildcard actually needs to unlock is rule 3 — letting several teams each own a services/<team>/openclaw directory. Worth a line in the comment saying that ghcr.io/mctlhq/openclaw itself is deliberately an unused name, so the day something is deployed under it the wildcard gets revisited rather than silently being world-writable.
There was a problem hiding this comment.
Narrowed in be70817. Wildcard grants are now rejected by the parser (exit 2).
openclaw is a new shared openclaw line:
- any team may run a service of that name, which lifts the cross-team rule only;
- only a granted team may build into it. That is
grant labs openclaw, for mctl-openclaw's upstream-sync dispatch, the only builder.
Build-time callers pass --build. deploy-service also no longer builds for the openclaw template: those services run the fixed mctl-openclaw image, so the build pushed an image nothing runs. The comment in image-names.txt records all of this, including that nothing in this repository runs the openclaw image.
| for reserved in argocd argo-workflows argo-events kube-system kube-public kube-node-lease default cert-manager traefik vault external-secrets monitoring; do | ||
| # platform: the team name image-names.txt grants the platform's own | ||
| # images to; a tenant of that name would inherit those grants. | ||
| for reserved in argocd argo-workflows argo-events kube-system kube-public kube-node-lease default cert-manager traefik vault external-secrets monitoring platform; do |
There was a problem hiding this comment.
P3 — this new reservation is the only behaviour change in the PR with no test behind it.
tests/test_check_image_name.py asserts the wiring for validate, preview-deploy and build-image.yaml, and the PR body lists mutation evidence for eleven other changes, but deleting platform from this list leaves the suite green. Given the grants at image-names.txt:40-44 are the reason it is here, it is the line most worth pinning:
ct = (REPO / "platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml").read_text()
reserved_line = [ln for ln in ct.splitlines() if "for reserved in" in ln][0]
check("platform" in reserved_line.split(),
"wft-create-tenant must reserve the tenant name 'platform' (image-names.txt grants to it)")There was a problem hiding this comment.
Added in be70817. The test parses the for reserved in ... line of wft-create-tenant.yaml and requires both platform and admins. A mutation removing admins is caught.
| exit 1 | ||
| fi | ||
|
|
||
| for dir in "$SERVICES"/*/"$NAME"; do |
There was a problem hiding this comment.
P3 — rules 3 and 4 are a check-then-act race: two teams onboarding the same unclaimed name concurrently both pass.
Neither the directory scan nor the values scan sees a name that is mid-onboard. Team A and team B both submit deploy-service with action: onboard and component_name: shiny, both clone, both find no services/*/shiny, both reach rule 4, both dispatch a build of their own dockerfile_repo into ghcr.io/mctlhq/shiny, and the gitops commits do not conflict because they land in different team directories. Afterwards both services run whichever build pushed last, and the next check refuses whoever asks second — which is a confusing place to discover it.
Low likelihood and not a regression (today nothing claims names at all), so not a merge blocker. If you want it closed, the cheap version is tpl-git-commit's mutex: re-run the check inside the commit step, where the repository state being read is the state being written.
There was a problem hiding this comment.
Agreed it's a race. I've noted it in a comment at step 3b of validate rather than fixed it here.
The suggested re-check inside tpl-git-commit would come after the build has already pushed, so it would only stop the second gitops record, not the second push. The real fix is to claim the name and check it in one commit, before the build. That needs reordering deploy-service, which is out of scope for this PR.
There was a problem hiding this comment.
Has P1/P2 findings, changes requested: 0 P1, 2 P2, 4 P3.
P2-1 (image-names.txt:40) — team_name is caller-asserted in the Argo path. tpl-validate-tenant's validate never checks that the team exists (its own annotation says it does; no code does), so team_name: platform reaches the grant platform vault-iac|zitadel-iac|grafana-iac|mc|mctl-agent lines. Reserving the tenant name in wft-create-tenant blocks creating such a tenant, not passing the string. Fix: require platform-gitops/tenants/<team>/ in the two Argo callers (not in the script, or the hand-dispatched platform builds break).
P2-2 (build-image.yaml:149) — the new image_name == ghcr.io/<owner>/<component_name> pin binds every workflow_dispatch, including the out-of-repo dispatchers listed in docs/runbooks/github-app-scope-audit.md:86-87. Component≠image does occur here (services/labs/mctl-telegram-preview runs ghcr.io/mctlhq/mctl-telegram), and no test can cover external callers. Confirm each dispatcher's pair, or land this half warn-only first.
The core design is sound and the script is carefully fail-closed: input grammar, unparsable-list, and missing-list all exit 2, -v/grep interpolation is safe behind the [a-z0-9-] grammar, globs guard with -d/-f, the check precedes credential fetch and push in all three callers, and the :latest narrowing is safe (nothing pins tag: latest, and a resolved latest tag is still pushed as the primary tag). Verified every service uses the single image.repository form the script greps for, so no claim is missed today. I could not execute the test suite or linters in this environment (command approval), so the pass/fail claims in the PR body are unverified here.
P3s: comment overstates the GHA layer as a second enforcement layer (release-deploy's workflow_call and a caller-chosen ref both skip it); requests without limits on the new image-name pod, against this directory's convention; mctlhq hardcoded in the values grep while callers derive the org, and that path fails open; grant * openclaw short-circuits rules 2 and 3; the new platform tenant reservation has no test; onboard is check-then-act.
Tenant workflows now run the check with --tenant: the team must be an existing tenant (tenants/<team>/) and never "platform", so the grants for hand-dispatched platform images are unreachable from deploy-service and preview-deploy. "admins" is reserved in create-tenant as well. The registry is an argument and must equal the list's registry line, so a caller deriving another organisation is refused instead of being checked against names it does not use. The wildcard grant is gone: openclaw is a shared service name whose image only labs may build, and deploy-service no longer builds for the openclaw template, whose services run the fixed mctl-openclaw image. Build-time callers pass --build, so a shared name can be run but not pushed to by any team. Also: limits on the check pods, the push-time step names its known gaps, and the onboard race is noted where the check runs. Claude-Session: https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2
|
Replies to the agy review ( agy 1 (P1), undeclared
Real direct dispatches already send them. The latest platform image run, 37212669451 on 2026-10-04, logs agy 2 (P2), preview DAG blocked when agy 3 (P2),
agy 4 (P2), anonymous clone of a private repo: refuted. Claude review body: every P2 and P3 is answered on its own thread.
Verification on
|
Antigravity (agy) review — informationalModel: gemini-3.8-flash-medium · commit Finding 1: Undeclared
|
|
@claude review The automatic run for be70817 (run 37218071930) completed its review step in about 30 seconds (the run for 3b391a7 took 11 minutes), posted only the placeholder, and submitted no review bound to be70817. The PR therefore still carries the CHANGES_REQUESTED verdict from 3b391a7. Please review be70817, including the replies to the earlier threads. |
|
Claude finished @mashkovd's task in 8m 23s —— View job Review complete —
|
| # must not be created again by someone else (this workflow continues | ||
| # on an existing tenant directory); wft-delete-tenant-safe likewise | ||
| # refuses admins. | ||
| for reserved in argocd argo-workflows argo-events kube-system kube-public kube-node-lease default cert-manager traefik vault external-secrets monitoring platform admins; do |
There was a problem hiding this comment.
P2 — reserving admins removes the documented idempotent re-provisioning path for a tenant that already exists, and the thing it is meant to stop is already impossible.
platform is right: there is no platform-gitops/tenants/platform/, so refusing to create one is what keeps the grant platform vault-iac|zitadel-iac|grafana-iac|mc|mctl-agent lines unreachable from --tenant callers. admins is the opposite case.
-
It gains nothing.
platform-gitops/tenants/admins/already exists, socheck-image-name.sh:88acceptsteam_name: adminstoday without anyone creating anything. Reachinggrant admins mctl-webnever required creating the tenant; it requires being able to submit a deploy-service workflow as teamadmins, which is mctl-api/portal membership — the boundary you correctly identify in your reply. Andwft-delete-tenant-safe.yaml:99already refuses to deleteadmins, so the name cannot be freed and re-taken either. -
It costs the re-provision path. This workflow is deliberately re-runnable:
wft-create-tenant.yaml:461-463logsTenant directory already exists … skipping git commit, continuing provisioningand carries on to re-create the Vault policy, the ESO role, the namespace and the ArgoCD RBAC fragment. That is the recovery path after a Vault restore or a cluster rebuild.validate-tenant-nameis step 1, so the new entry fails the workflow before any of it runs — for the platform's own tenant, and only for that one. -
It is inconsistent with its own comment. The comment says these are "the teams image-names.txt grants the platform's own images to", but
labsholds ten grants (image-names.txt:64-73) andovkone, and neither is reserved. If existing-tenant-with-grants were the criterion, three names belong on the list; if it is not,adminsdoes not.
Suggest dropping admins and keeping platform, or — if you want the belt — moving the guard to "refuse when the tenant directory already exists" so it is about re-creation generally rather than one hard-coded name. Either way tests/test_check_image_name.py:225 needs the matching change.
There was a problem hiding this comment.
Fixed in 30feeab: admins is dropped from the reserved list and platform stays. The comment above the loop now gives the reason for platform alone: it is never a tenant, which is what keeps the grant platform … lines unreachable for --tenant callers. The test now requires only platform.
Separately, the general re-creation case you mention is handled in #1650. There, create-tenant fails in validate on any existing tenant unless reprovision=true is passed, so re-provisioning admins after a restore remains a deliberate re-run.
| check("image-name" in tasks["build-image"].get("dependencies", []), | ||
| "preview-deploy's build must depend on the image-name check") | ||
| check("image-name" in tasks["deploy-preview"].get("dependencies", []), | ||
| "preview-deploy's deploy must depend on the image-name check") |
There was a problem hiding this comment.
P3 — the one new wiring link with no test: preview-deploy passing git_ref to image-name.
The suite pins the templateRef and both dependencies, and line 202 pins that the image-name template reads PARAM_GIT_REF. Nothing pins that wft-preview-deploy.yaml:72-73 actually hands it over. Delete those two lines and every preview build silently drops to the run-only check — shared openclaw becomes buildable by any team from preview-deploy again, which is exactly the hole --build was added to close. That mutation is in the same class as the eleven the PR body lists, and it is the only one left green.
pparams = {p["name"]: p["value"] for p in tasks["image-name"]["arguments"]["parameters"]}
check(pparams.get("git_ref") == "{{workflow.parameters.git_ref}}",
"preview-deploy must tell image-name whether it will build")There was a problem hiding this comment.
Fixed in 30feeab. tests/test_check_image_name.py now asserts that preview-deploy's image-name task passes git_ref={{workflow.parameters.git_ref}}. Removing those two lines fails the suite (verified as mutation N2).
| - name: build-image | ||
| dependencies: [validate] | ||
| when: "\"{{workflow.parameters.action}}\" != \"update-config\" && \"{{workflow.parameters.dockerfile_repo}}\" != \"\"" | ||
| when: "\"{{workflow.parameters.action}}\" != \"update-config\" && \"{{workflow.parameters.dockerfile_repo}}\" != \"\" && \"{{workflow.parameters.service_template}}\" != \"openclaw\"" |
There was a problem hiding this comment.
P3 — the new service_template != "openclaw" clause gates on an onboard-time parameter, and a deploy that omits it is now refused where it used to pass.
service_template defaults to "default" (line 76) and describes what template to write at onboard; it says nothing about the service that already exists. tpl-git-commit.yaml:360 only consults it under ACTION = onboard. So for action: deploy on an existing openclaw service, a caller that supplies dockerfile_repo but lets service_template default gets BUILD_FLAG=--build in validate — and shared openclaw then refuses admins and ovk outright (exit 1, whole workflow fails). Under grant * openclaw on main that same call passes.
Probably unreachable: references/deploy.md:172 says dockerfile_repo is not required and no build is triggered for a non-default template, and references/k8s.md:144-148 documents the openclaw image bump as a hand-written PR on this repo, not a deploy-service call. But the comment this commit replaces said "their build input is pinned to mctlhq/mctl-openclaw by mctl-api", which reads as mctl-api does send dockerfile_repo for openclaw — worth confirming it also sends service_template on deploy, not only on onboard.
The parameter that actually describes the service is services/<team>/<name>/values.yaml's image.repository: when it is not <registry>/<component_name>, building that name is pointless regardless of which template produced it. Keying the skip off the checkout validate has already cloned would make the rule true for every action, and would also cover labs/mctl-telegram-preview (runs ghcr.io/mctlhq/mctl-telegram), which still dispatches a build of a name nothing runs.
There was a problem hiding this comment.
Fixed in 30feeab. Validate now makes the build decision once and bases it on the existing service, not on the onboard parameter:
- never for
update-config, or without adockerfile_repo; - on
onboard, by the template (openclaw runsmctl-openclaw); - on any other action, by
services/<team>/<name>/values.yaml. It builds only when arepository:line is<registry>/<component_name>(quoted or not), or when no image is named at all (the old behaviour; deploy without a values.yaml still fails at step 6).
The result is written once and used twice. It sets the --build flag for the check, and it is a new builds output. deploy-service's build-image now runs on "{{tasks.validate.outputs.parameters.builds}}" == "true" alone, so the check and the build cannot disagree.
As you noted, labs/mctl-telegram-preview (which runs ghcr.io/mctlhq/mctl-telegram) no longer dispatches a build of a name nothing runs.
The test extracts the block from the template and runs it against a fixture, with 10 cases, including deploy of an existing openclaw service with a defaulted service_template (now: no build, checked as a run). It also passes under busybox sh (python:3.12-alpine). mctl-api's copy of the decision is being updated to match.
| fi | ||
| done | ||
|
|
||
| if [ ! -r "$LIST" ] || [ ! -d "$SERVICES" ]; then |
There was a problem hiding this comment.
P3 — $TENANTS is the one input directory not in this precondition, so a missing tree reports as a refusal rather than "could not decide".
[ ! -r "$LIST" ] || [ ! -d "$SERVICES" ] exits 2, the code the script reserves for "the checkout is not what I expect". But with --tenant, a missing platform-gitops/tenants/ entirely — a sparse checkout that forgot it, a layout change — reaches line 88 and prints team 'labs' is not an existing tenant, exit 1. Both fail closed, so nothing is unsafe; it just sends whoever is debugging to look at the team name instead of the checkout.
This is live, not hypothetical: build-image.yaml's sparse-checkout (lines 136-139) fetches services, config and scripts but not tenants. Correct today because that caller never passes --tenant — but the day it does, the failure will read as "no such tenant" for every build. Adding [ "$TENANT" = true ] && [ ! -d "$TENANTS" ] to the exit-2 precondition makes it say what it is.
There was a problem hiding this comment.
Fixed in 30feeab: with --tenant, a missing platform-gitops/tenants/ now exits 2 ("cannot read …/tenants"), before any team is looked at. Without --tenant, the tree is not needed. That keeps build-image.yaml's sparse checkout working, and a test pins it. Both are tested, and removing the check fails the suite (mutation N3).
There was a problem hiding this comment.
Has P1/P2 findings, changes requested: 0 P1, 1 P2, 3 P3. One line away from approval — everything from the 3b391a7e review is resolved.
Confirmed resolved on be70817e: the --tenant flag closes P2-1 properly — check-image-name.sh:84-91 requires platform-gitops/tenants/<team>/ and refuses platform unconditionally, both Argo callers pass it, build-image.yaml does not (so hand-dispatched platform builds keep working), and the fixture covers the tenants/platform case. The registry is now a required argument compared against a single registry line, so the last fail-open path is closed and each caller's own derivation (container_registry / ghcr.io/\${GITOPS_ORG} / \${REGISTRY}/\${GITHUB_REPOSITORY_OWNER}) is checked rather than assumed. Wildcard grants now exit 2; shared openclaw + grant labs openclaw narrows run-vs-build correctly. Limits added to both pods. The build-image.yaml comment names both gaps. P2-2's dispatcher audit is pinned by the --build cases at tests/test_check_image_name.py:232-239.
agy findings, all four correctly refuted or fixed. 1 and 2 are wrong on the facts and I verified both in the tree: team_name/component_name are declared under both workflow_dispatch and workflow_call (build-image.yaml:28-35, 58-63), and wft-deploy-service.yaml:150-153 passes them to trigger-github-build, which puts them in the dispatch payload at line 538. agy 2's second claim is also wrong: legacy dependencies is .Succeeded || .Skipped || .Daemoned, and deploy-preview already depended on the conditional build-image before this PR. agy 4 is wrong about the repo being private. agy 3 was acted on — see below.
P2 (wft-create-tenant.yaml:153) — adding admins to the reserved list is the one change I would not keep. platform belongs there: no tenants/platform/ exists, so refusing to create one is what makes the grant platform … lines unreachable. admins is the inverse — tenants/admins/ already exists, so --tenant accepts team_name: admins today without anyone creating anything, and wft-delete-tenant-safe.yaml:99 already refuses to free the name. agy 3's escalation path does not exist. What the entry does cost is real: this workflow is deliberately re-runnable (wft-create-tenant.yaml:461-463, "already exists — skipping git commit, continuing provisioning") and that is the path that re-creates the Vault policy, ESO role, namespace and ArgoCD RBAC fragment after a restore. validate-tenant-name is step 1, so it now fails first, for the platform's own tenant only. The rationale is also self-inconsistent: labs holds ten grants and ovk one, and neither is reserved.
P3s: preview-deploy's git_ref → image-name parameter is the only new wiring link with no test, and dropping it silently downgrades every preview build to the run-only check; the new service_template != \"openclaw\" gate keys off an onboard-time parameter, so a deploy that supplies dockerfile_repo without it is refused for admins/ovk where it passed before (docs say unreachable, worth confirming against mctl-api); \$TENANTS is missing from the exit-2 precondition, so a missing tree reads as "no such tenant" rather than "could not decide".
Not verified here: python3 tests/test_check_image_name.py, the script itself, yamllint, actionlint and argo lint all needed command approval in this job, so the pass/fail claims in the PR body remain unverified — I reviewed by reading, and traced each fixture case by hand against the script. One cross-check worth recording: references/k8s.md:146,219 confirms ghcr.io/mctlhq/mctl-openclaw is published by mctl-openclaw's own Docker Release workflow on a v* tag, not through build-image.yaml, and that ghcr.io/mctlhq/openclaw is the legacy path — so the shared openclaw + grant labs openclaw pair is right and nothing in the openclaw image pipeline is caught by reserved-prefix mctl-.
The build-or-not decision keyed off service_template, an onboard-time parameter, so a deploy of an existing openclaw service that let it default was checked as a build and refused. validate now decides once: never for update-config or without a dockerfile_repo; on onboard by the template; otherwise by the service's own values.yaml, which builds only when it runs <registry>/<component_name> (or names no image). The result is both the --build flag and a `builds` output, and deploy-service's build step runs on that output alone, so the check and the build cannot disagree. Also: - drop `admins` from create-tenant's reserved names: tenants/admins already exists, so the entry blocked nothing but the re-run of the platform's own tenant; `platform` stays. - check-image-name.sh --tenant without platform-gitops/tenants/ is exit 2 (could not decide), not "no such tenant". - tests pin preview-deploy's git_ref handover to image-name, and team_name/component_name from both Argo callers' build tasks into the build-image dispatch payload; the decision is run from the template against fixtures. Claude-Session: https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2
|
Claude finished @mashkovd's task in 7m 15s —— View job Review complete —
|
|
Replies to the agy review of agy 1 (P1), agy 2 (P1), deploy-service does not pass
Claude review body: the P2 and the three P3s are answered on their threads. In summary:
Verification on
|
Antigravity (agy) review — informationalModel: gemini-3.8-flash-medium · commit Findings[P1] Caller classification via
|
| steps: | ||
| # --- Image name: build only names owned by the requesting team --- | ||
| # A direct dispatch is how tenant builds arrive (Argo's deploy-service | ||
| # and preview-deploy), how mctl-openclaw's upstream sync builds the |
There was a problem hiding this comment.
P3 — this comment still names a dispatcher that the same commit just made refusable.
f54c24e1 drops grant labs openclaw from image-names.txt on the grounds that mctl-openclaw is archived, and tests/test_check_image_name.py:435 now asserts the opposite of what this line describes:
check(run(REPO, "labs", "openclaw", "--build") == 1,
"no team may build openclaw now that its only builder is archived")So "how mctl-openclaw's upstream sync builds the openclaw image (team labs)" is no longer a direct dispatch this step lets through — it is one this step now refuses with component_name openclaw is not available to team labs. The image-names.txt comment was updated to say so ("that repository is archived … A successor sync that builds it again needs a reviewed grant line here"); this one was not, and it is the comment someone reads while debugging that exact failure.
Trimming it to the two dispatchers that still pass — Argo's deploy-service/preview-deploy, and the hand-dispatched platform-gitops/images/* builds as team platform — keeps the two files saying the same thing. (The image_name description at line 16 is already correct.)
There was a problem hiding this comment.
Fixed in 0c8ce30 (comment only). The dispatcher list now names Argo deploy-service and preview-deploy (tenants) and the manual platform-image builds (team platform). The openclaw sync is gone from it, matching image-names.txt and the assertion at tests/test_check_image_name.py:435.
| # repository runs the image. Shared, so the service name stays available to | ||
| # every team, and with no grant no team may build it. A successor sync that | ||
| # builds it again needs a reviewed grant line here. | ||
| shared openclaw |
There was a problem hiding this comment.
P3 — with the grant gone, shared openclaw now means different things on the two Argo paths, and preview-deploy is the one that hard-fails.
deploy-service derives --build from what the service runs (tpl-validate-tenant, BUILDS), so for all three existing openclaw services it checks in run mode and shared allows it — that is why the 30-service loop stays green. preview-deploy derives it from git_ref alone (image-name: [ -n "$PARAM_GIT_REF" ] && BUILD_FLAG="--build"), so a preview whose component_name is openclaw and which carries a git_ref reaches rule 3 in build mode and exits 1 — the whole preview fails, where before this commit grant labs openclaw let labs through.
Not reachable by anything I can see today: preview components are <service>-preview names (labs/mctl-telegram-preview), none of which is openclaw, and the archived sync was the only builder. Worth a clause in this comment all the same, since "the service name stays available to every team" is true for running and for an openclaw-template onboard, but an onboard or preview that would render ghcr.io/mctlhq/openclaw is now refused for every team rather than just the ungranted ones — which is the intent, but it is the surprise an operator hits first.
Also worth recording the premise: I could not check mctlhq/mctl-openclaw's archived flag from this job (gh api on another repo needed approval), and nothing in this tree states it — the removal is fail-closed either way, so the worst case is a restored sync failing with a message that points here.
There was a problem hiding this comment.
Recorded in 0c8ce30. The shared openclaw comment now says that an onboard or a preview with a git_ref that would push openclaw itself is refused for every team, because preview-deploy checks in build mode whenever it builds. That refusal is intended. As you note, nothing reaches it today, since preview components are <service>-preview. On the premise: gh api repos/mctlhq/mctl-openclaw returns archived: true (last push 2026-09-05), so the grant removal rests on a checked fact.
There was a problem hiding this comment.
No P1/P2 findings (2 P3). Good to merge.
Everything from the 3b391a7e, be70817e, 30feeab4, 204c0846 and 7e200fcf reviews is resolved, and this commit closes the three findings I left open on 7e200fcf.
The P2 is answered by running the thing rather than asserting it. The real-tree loop at tests/test_check_image_name.py:393-400 runs values-images.sh over all 30 service values files and all 4 values.yaml.tpl and compares with a yaml.safe_load walk. That is the assertion that keeps the indentation measures honest now that 644e29d6 traded the "a swallowed key must refuse" guard for correct YAML scoping — a mis-measured scalar that loses a claim silently is now a CI failure. And unlike my previous five reviews, this one is verified rather than read: Validate Manifests on this commit (run 37228191760, job 111512121490) logs OK: 33 fixture cases, 30 existing services, 11 referenced platform images, 34 real values files read. 34 = 30 + 4, so every file was read and agreed. That run is ubuntu-latest, i.e. mawk, which also settles the awk-portability question I could not check by reading ((^|[[:space:]]) inside a group, \047, /dev/stderr).
The dash-branch false positive is fixed at the root, not papered over. Branch 2 now requires the dash group to be the line's entire prefix (^[[:space:]]*(-[[:space:]]+)*-[[:space:]]+...), so only a genuine sequence-item scalar opens a region; a plain value merely ending in - | opens nothing, with three fixtures for it. I checked this cannot lose a legitimate opener: every YAML sequence-item block scalar has a dashes-only prefix, - !!str | / - &a >- still match through the anchor/tag group, and nested - - | measures to the inner dash's column. It also removes the one (^|...) alternation from the opener regexes, which is the portability shape I had flagged.
The two if: steps.caller.outputs.direct == 'true' guards are now pinned (:534-537), closing the asymmetry where the :latest line beside them was pinned and the steps enforcing the component-name pin were not — dropping either would have broken every release build with the suite green. The declared-inputs assertion at :540-544 reads bi.get("on", bi.get(True, {})), which is right for both PyYAML's YAML-1.1 on → True and a future 1.2 loader, and it retires agy's recurring P1 by data instead of by argument.
grant labs openclaw is removed, which is the narrowing I asked for: shared openclaw keeps the three existing services running (rule 3, run mode) and no team may push the name. Verified the existing-service loop is unaffected because deploy-service's --build comes from BUILDS, which is false for all three (they run mctl-openclaw).
P3s: the build-image.yaml comment at line 105 still lists mctl-openclaw's upstream sync as a supported direct dispatch, which this commit made refusable — the image-names.txt comment was updated, this one was not, and it is what an operator reads while debugging that refusal. And preview-deploy derives --build from git_ref rather than from what the service runs, so shared openclaw is run-only there in a way it is not on the deploy-service path; unreachable today (preview components are <service>-preview), worth a clause in the comment. I could not confirm mctlhq/mctl-openclaw's archived flag from this job (gh api on another repository needed approval) and nothing in the tree states it; the removal is fail-closed either way.
| # indicators, and an optional anchor or tag before it), after a key or a | ||
| # sequence dash. It belongs to that key, or to the last dash, and its | ||
| # text is what is indented deeper than that column. | ||
| if (line ~ /(:|(^|[[:space:]])-)[[:space:]]+([&!][^[:space:]]*[[:space:]]+)*[|>][-+0-9]*[[:space:]]*$/) { |
There was a problem hiding this comment.
P3 — the dash alternative in the opener test is unanchored, so a plain scalar whose value happens to end in - | opens a phantom block scalar.
The key alternative is self-anchoring (it needs : + space + the indicator at EOL, and only a real block scalar header looks like that). The dash alternative is (^|[[:space:]])-, which matches a lone dash anywhere on the line:
note: some text - |
image:
repository: ghcr.io/mctlhq/foonote: some text - | is a valid plain scalar in YAML, but it matches the dash branch here. match(line, /^[[:space:]]*(-[[:space:]]+)*/) then finds no leading dash, so prefix is just the indentation and scalar becomes 2 — everything indented deeper than 2 is read as text until a line at column ≤ 2. The repository: key above is at 4, so it is silently dropped: check-image-name.sh stops claiming that name for its team, and tpl-validate-tenant sees IMAGES empty and sets BUILDS=true, bumping image.tag for an image the service does not run.
Latent, and I checked rather than assumed: [^[:space:]-][[:space:]]+-[[:space:]]+[|>][-+0-9]*[[:space:]]*$ has no matches anywhere under platform-gitops/, so nothing is affected today. It is also the one remaining way the region can come out too wide, which is the case the previous "a key inside a block scalar must refuse" guard existed to catch and this commit (correctly) removed.
Anchoring the dash to the leading prefix removes the case without losing anything the tests cover — - |, - &a |, - - | and - !!str >- all still match, and - run: | is handled by the key branch:
if (line ~ /(:[[:space:]]+|^[[:space:]]*(-[[:space:]]+)+)([&!][^[:space:]]*[[:space:]]+)*[|>][-+0-9]*[[:space:]]*$/) {
| ("c: &a |\n repository: x/b\nd: !!str >-\n repository: x/c\nimage:\n repository: x/a\n", | ||
| (0, ["x/a"])), | ||
| ): | ||
| rc, out = read_images(text) |
There was a problem hiding this comment.
P3 — the structural cross-check the PR body claims for the reader is not in the committed suite, and it is the only thing that would catch a swallowed claim.
The body says "On every existing service and template, its output matches a structural YAML parse." grep -n "safe_load" on this file finds four occurrences, all on cluster templates and build-image.yaml; isinstance or a walk appear nowhere. The real-tree coverage of the reader is the loop at line 183, which asserts exit code 0 for every service — so it catches a reader that adds a spurious claim (the owning team gets refused) or that exits 2, but not one that drops a real repository: key, which is the direction this commit opened by replacing "a key inside a block scalar refuses" with "skip".
A dropped claim is not inert. For check-image-name.sh the name stops being taken; for tpl-validate-tenant, a service whose only repository: key is swallowed reads as IMAGES empty → BUILDS=true → build-image pushes <registry>/<component_name> while commit bumps image.tag on a service that runs something else. The fixture table covers the swallow cases synthetically; what it does not do is sweep the 30 real files — which is also where a future values.yaml would be caught, including one written by config_patch rather than by a PR, since CI runs this on pull_request and on push: [main].
About eight lines, next to the existing table:
def structural_images(text):
found = []
def walk(node):
if isinstance(node, dict):
for k, v in node.items():
if k == "repository" and isinstance(v, str) and v:
found.append(v)
walk(v)
elif isinstance(node, list):
for v in node:
walk(v)
walk(yaml.safe_load(text))
return sorted(found)
for p in sorted((REPO / "platform-gitops/services").glob("*/*/values.yaml")):
rc, out = read_images(p.read_text())
check(rc == 0 and sorted(out) == structural_images(p.read_text()),
f"values-images.sh disagrees with a structural parse on {p}")The four values.yaml.tpl want __SERVICE_NAME__ and __TEAM_NAME__ substituted first, the same two this step substitutes, or safe_load chokes on port: __PORT__.
| elif ! IMAGES=$(sh platform-gitops/argo-workflows/scripts/values-images.sh "$VALUES"); then | ||
| echo "❌ cannot read the images ${VALUES} runs; not deciding whether to build" | ||
| exit 1 | ||
| elif printf '%s\n' "$IMAGES" | grep -q '__[A-Z][A-Z_]*__'; then |
There was a problem hiding this comment.
P3 — the placeholder guard is the right answer to the last round, but its character class excludes digits, so it does not cover every placeholder shape tpl-git-commit could grow.
__[A-Z][A-Z_]*__ matches all eight placeholders that exist today (tpl-git-commit.yaml:370-377). It does not match one containing a digit — __PORT2__ gets as far as __PORT and then needs __, finds 2, and fails everywhere in the string. A template rendering repository: ghcr.io/mctlhq/__REGION1__-__SERVICE_NAME__ therefore slips past the guard, compares unequal to <registry>/<component_name>, and lands back in the silent BUILDS=false this guard exists to prevent: an onboard that builds nothing and then bumps image.tag to a tag nothing pushed.
__[A-Z][A-Z0-9_]*__ closes it, and the value regex in values-images.sh:79 already admits digits, so nothing upstream filters them out first.
Worth noting what I checked around this, because the pairing is what makes the guard sound: the two substitutions here (__SERVICE_NAME__, __TEAM_NAME__) are exactly the ones that appear in a repository: value in any current template, $TEAM and $SERVICE have both passed the [a-z0-9-] grammar at step 1 before reaching sed, and the other six placeholders (__IMAGE_TAG__, __PORT__, __HOST__, __DEFAULT_MODEL__, __TELEGRAM_OWNER_ID__, __TELEGRAM_OWNER_IDS_JSON__) only ever appear outside image.repository, so leaving them unsubstituted costs nothing — values-images.sh reads one key and ignores the rest of the file.
There was a problem hiding this comment.
No P1/P2 findings (3 P3). Good to merge.
The - | gap is closed, and closed at the right level: instead of patching the sequence-item form into the old refuse-on-suspicion handler, the handler now measures the scalar region the way YAML defines it and skips the text. I checked the measure in both directions rather than trusting the fixtures. Too narrow (text read as YAML) cannot happen: for a dash-opened scalar scalar is the dash column and YAML requires content deeper than the sequence indentation; for a key-opened one, including - run: |, it is the key column and content must be deeper than the mapping. Too wide (a real key swallowed) cannot happen from the prefix arithmetic either — match(line, /^[[:space:]]*(-[[:space:]]+)*/) then stripping the last - gives the dash column exactly, and nested - - | lands on the inner dash. A sibling item at the dash column closes the region, which is the case the naive fix gets wrong and the new table pins. The eight - | blocks in each openclaw values file, the four in labs/claude-remote and the eight in the openclaw template (which validate now renders on every openclaw onboard) are all handled, and claude-remote/values.yaml:774 no longer depends on which key happens to enclose it. mctl-telegram/values.yaml:593 (image: "{{ .Values.image.repository }}:...") still misses the gate correctly, since the : the gate needs is consumed by }}.
The two P3s from the last round are also addressed: 755 with a git ls-files -s assertion behind it, and the onboard render now substitutes __TEAM_NAME__ with an explicit refusal on any remaining __PLACEHOLDER__ in an image — fail-closed and loud, rather than the silent BUILDS=false I was worried about. The blast-radius trade is now written into the reader header, including that CI runs the scan on pull_request and push: [main], which is the part a future reader needs.
Everything from the 3b391a7e, be70817e, 30feeab4 and 204c0846 reviews is resolved, and the agy findings across those commits were all correctly refuted (team_name/component_name declared and passed; legacy dependencies treating Skipped as satisfied; a public repo; github.workflow_ref naming the entry-point workflow).
P3s: the opener regex's dash alternative is unanchored, so a plain scalar ending in - | would open a phantom scalar — latent, and I verified [^[:space:]-][[:space:]]+-[[:space:]]+[|>][-+0-9]*[[:space:]]*$ has no matches anywhere under platform-gitops/; the structural-parse sweep the PR body claims for the reader is not in the committed suite, and the real-tree loop catches spurious claims but not swallowed ones; the placeholder guard's class excludes digits.
Not verified here: python3 tests/test_check_image_name.py, values-images.sh, shellcheck, yamllint, actionlint and argo lint --offline all needed command approval in this job, so the fixture/mutation and busybox-awk claims remain unverified by me. I reviewed by reading, traced the new block-scalar cases by hand, and grepped every repository occurrence and every block-scalar opener under platform-gitops/services and service-templates against the handler. Note the PR head moved to f54c24e1 while this ran; this verdict is bound to 644e29d6.
The build-image comment still listed mctl-openclaw's upstream sync as a direct dispatcher after its grant was removed, and image-names.txt did not say that a preview building the openclaw image is now refused for every team. Comments only. Claude-Session: https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2
|
Re agy [P1] "Missing |
|
Claude finished @mashkovd's task in 2m 28s —— View job Review complete —
|
Antigravity (agy) review — informationalModel: gemini-3.8-flash-medium · commit No significant issues found. |
There was a problem hiding this comment.
No P1/P2 findings (1 carried-over P3). Good to merge.
0c8ce302 is comment-only, and it closes both P3s from the f54c24e1 review. build-image.yaml:104-105 no longer lists mctl-openclaw's upstream sync as a supported direct dispatch, so it now agrees with image-names.txt and with tests/test_check_image_name.py:436 (run(REPO, "labs", "openclaw", "--build") == 1) — the two dispatcher kinds it names are exactly the ones the test pins. And the shared openclaw" comment now records the asymmetry I flagged: preview-deploy derives --buildfromgit_refrather than from what the service runs, so a preview or a default-template onboard that would pushopenclawitself is refused for every team, not just the ungranted ones. Both claims are accurate against the templates: preview-deploy'simage-namesets--buildiffPARAM_GIT_REFis non-empty, which is the same condition as itsbuild-image when`.
Unlike my previous reviews, the suite is verified on this exact commit rather than read: Validate Manifests run 37229555462, job 111516148911, logs OK: 33 fixture cases, 30 existing services, 11 referenced platform images, 34 real values files read. That is ubuntu-latest, i.e. mawk, so the awk-portability question is settled here too. I also re-checked the f54c24e1 dash-anchoring fix by hand rather than by fixture: branch 2 of the opener test now requires the dash group to be the line's entire prefix, so note: some text - | opens nothing (neither branch matches — branch 1 needs the indicator immediately after : , branch 2 needs a dashes-only prefix), while - | measures to column 2, - run: | to the key column 4, and - - | to the inner dash. grant labs openclaw survives only inside the synthetic LIST_TEXT fixture, where it tests rule-1-over-rule-3 precedence; the real policy has no grant.
Carried-over P3, unaddressed and not blocking: the placeholder guard at tpl-validate-tenant.yaml:183 is __[A-Z][A-Z_]*__, which excludes digits, so a future template rendering e.g. ghcr.io/mctlhq/__REGION1__-__SERVICE_NAME__ would slip past it and land back in the silent BUILDS=false the guard exists to prevent. __[A-Z][A-Z0-9_]*__ closes it; the reader's value class at values-images.sh:79 already admits digits, so nothing upstream filters them first. Latent — all eight placeholders in tpl-git-commit.yaml:370-377 are digit-free.
The branch was 526 commits behind main. Conflicts: - build-image.yaml: kept the branch's image_name description. - wft-create-tenant.yaml: main replaced the reserved-name loop with the shared tenant-ns-guard (tenant_name_reserved), kept byte-identical in create-tenant, delete-tenant and delete-tenant-safe. The branch's reservation of "platform" moves into that guard, in all six copies. Tests: the "platform" check now runs tenant_name_reserved itself instead of parsing the old loop (mutation-checked: dropping "platform" fails it). The onboarding fixture used the openclaw service template, which main removed in c657b23; it now uses a fixture template that runs a fixed image. Claude-Session: https://claude.ai/code/session_012TfVcukEh7ry8MiLNN89N4
|
Claude finished @mashkovd's task in 6m 48s —— View job Review complete —
|
| check | evidence |
|---|---|
yamllint |
##[error]83:9 [key-duplicates] duplication of key "env" in mapping |
Validate Manifests / validate |
validate-shell-param-interpolation.py names all nine PARAM_* as "reads $PARAM_X, but this template's env does not define it" — and omits PARAM_CONTAINER_REGISTRY, which is the parser saying which block survived |
Knock-on worth fixing together: that job dies at step 21, so this PR's own Unit-test the image-name ownership check (validate-manifests.yml:437) has not run at this head, and main reshaped the tree it asserts against — service-templates/openclaw and the three openclaw services are gone, seven services are new, so the real-tree loops now cover 27 services and 30 values files rather than 30 and 34.
I checked that reshaped tree by hand rather than assuming it: every one of the 27 services passes the policy; every non-service ghcr.io/mctlhq/* reference is reserved or under reserved-prefix mctl-; and nothing in the new values files should trip values-images.sh — claude-remote:810's "repository": ["mctlhq/*"] is still inside the policy.json: | scalar opened at 794, and mctl-telegram:600's {{ .Values.image.repository }} still misses the gate because }} consumes the colon. That is reading, not the suite.
Verified intact after the merge
platformis added to all six copies ofmain's new sharedtenant_name_reservedguard (create / delete / delete-safe, two each), and the test now executes the guard out of theBEGIN/END tenant-ns-guardmarkers instead of parsing itscasepatterns — the right adaptation to that refactor.wft-deploy-service's build task still passesteam_name/component_nameand still gates on"{{tasks.validate.outputs.parameters.builds}}" == "true".wft-preview-deploy'simage-nametask, itsgit_refhandover anddependencies: [image-name, build-image]are unchanged.build-image.yaml'sClassify caller, bothif: steps.caller.outputs.direct == 'true'guards, the conditional:latestand theref: mainpolicy checkout are unchanged — andteam_name/component_nameare nowrequired: trueunder both triggers onmainitself, which retires agy's recurring P1 permanently.- Validate's onboard template selection still matches
tpl-git-commit.yaml:335-342after that file moved.
P3
- the
shared openclawrationale describes a template and three servicesmaindeleted — the line should stay (without it rule 5 lets any team buildghcr.io/mctlhq/openclaw, whichreserved-prefix mctl-does not cover), but that is now its only justification and the comment doesn't say so. Same stale example attpl-validate-tenant.yaml:171andwft-deploy-service.yaml:130. - carried over from
0c8ce302: the placeholder guard attpl-validate-tenant.yaml:202is__[A-Z][A-Z_]*__, which excludes digits, so a future template rendering e.g.__REGION1__-__SERVICE_NAME__slips into the silentBUILDS=falsethe guard exists to prevent.__[A-Z][A-Z0-9_]*__closes it.
Not verified
python3 tests/test_check_image_name.py, shellcheck, actionlint and argo lint --offline all needed command approval in this job, and CI's own run of the suite was cut short by the P1 — so there is no execution evidence for this head either way.
· branch fix/reserve-image-names
Antigravity (agy) review — informationalModel: gemini-3.8-flash-medium · commit Finding 1: Duplicate
|
The merge with main left validate with two env keys: main's PARAM_* bindings from #1812 and the branch's PARAM_CONTAINER_REGISTRY. YAML keeps only the last one, so every PARAM_* read by the script was unset. One env list now holds all ten bindings (yamllint key-duplicates and validate-shell-param-interpolation both pass). Claude-Session: https://claude.ai/code/session_012TfVcukEh7ry8MiLNN89N4
| envFrom: | ||
| - configMapRef: | ||
| name: mctl-platform-config | ||
| env: |
There was a problem hiding this comment.
P1 — duplicate env: key in the validate template: the merge resolution added a second env: list instead of extending the first, so YAML's last-key-wins drops all nine PARAM_* bindings and the step fails on every run.
script now carries two env keys:
- lines 61-79:
PARAM_TEAM_NAME,PARAM_COMPONENT_NAME,PARAM_ACTION,PARAM_COMPONENT_TYPE,PARAM_HOST,PARAM_DOCKERFILE_REPO,PARAM_IMAGE_TAG,PARAM_GIT_TAG,PARAM_SERVICE_TEMPLATE; - this line:
PARAM_CONTAINER_REGISTRYalone.
The second wins, so the container gets exactly one variable. source then does TEAM="$PARAM_TEAM_NAME" → empty, and step 1 refuses immediately:
❌ team_name must be lowercase alphanumeric with hyphens, ≤63 chars. Got:
That is every deploy-service and every preview-deploy run (validate is step 1 of both), not an edge case.
Both red checks on this commit are this one line:
yamllint→##[error]83:9 [key-duplicates] duplication of key "env" in mapping(job 114537611490);Validate Manifests / validate→scripts/validate-shell-param-interpolation.pylists all nine as "reads $PARAM_X, but this template's env does not define it" (job 114537611672).PARAM_CONTAINER_REGISTRYis absent from that list, which confirms which block the parser kept.
How it got here: main moved this template off direct {{inputs.parameters.*}} interpolation onto PARAM_* env bindings (env: before envFrom:), while this branch added envFrom: + env: PARAM_CONTAINER_REGISTRY after. Neither parent has a duplicate; the merge does. The new image-name template (line 298) is correct — one env, one envFrom.
Fix is to fold the one entry into the existing list and delete this key:
- name: PARAM_SERVICE_TEMPLATE
value: "{{inputs.parameters.service_template}}"
- name: PARAM_CONTAINER_REGISTRY
value: "{{inputs.parameters.container_registry}}"
envFrom:
- configMapRef:
name: mctl-platform-configWorth noting the knock-on: validate dies at step 21 of that job, so this PR's own Unit-test the image-name ownership check (validate-manifests.yml:437) never ran at this head. The merged main retired service-templates/openclaw and the three openclaw services and added seven others (karabu/draw-with-friends, labs/genai-leader, instruments-proxy, kuptsi-app, payment-llm-pipelines, pelican-proxy, pelican-proxy-staging), so the real-tree loops now cover 27 services and 30 values files rather than 30 and 34. I read every repository: line and every block-scalar opener in the new tree and found nothing the reader should trip on — but that is reading, not the suite, and the suite needs to go green here.
Antigravity (agy) review — informationalModel: gemini-3.8-flash-medium · commit Finding 1: P1 — Missing Workflow Dispatch Inputs in
|
|
Claude finished @mashkovd's task in 8m 21s —— View job Review complete —
|
| grant ovk mctl-academy | ||
|
|
||
| # openclaw: several teams hold a service of that name (labs, ovk, admins); | ||
| # each runs the fixed image mctl-openclaw (service-templates/openclaw), which |
There was a problem hiding this comment.
P3 — this rationale describes a template and three services that the merged main has since removed, so the only reader of the shared openclaw line can no longer check it against the tree.
As of this commit:
platform-gitops/argo-workflows/service-templates/holdsdefault,spring-worker,worker— there is noopenclawtemplate, so "each runs the fixed image mctl-openclaw (service-templates/openclaw), which the mctl- prefix reserves, and deploy-service does not build for that template" has nothing behind it;- no
services/*/openclawdirectory exists either, so "several teams hold a service of that name (labs, ovk, admins)" is no longer true.
The line itself should stay, and for a reason the comment does not currently give: with openclaw neither reserved nor shared, rule 5 would let any team build and push ghcr.io/mctlhq/openclaw — the archived sync's name, which reserved-prefix mctl- does not cover. shared is what keeps that refused (rule 3 in build mode) while leaving the service name free to re-appear. That is now the whole justification, and it is the part worth writing down.
Same stale reference at tpl-validate-tenant.yaml:171: "(e.g. the openclaw template's mctl-openclaw)" as the example of a service that runs another image. labs/mctl-telegram-preview (runs ghcr.io/mctlhq/mctl-telegram) and labs/claude-remote (runs mctl-claude-remote) are live examples in the current tree, and wft-deploy-service.yaml:130 carries the same openclaw example.
Not a behaviour issue — shared openclaw decides identically either way, and tests/test_check_image_name.py:446 still pins that no team may build the name.
There was a problem hiding this comment.
Has P1/P2 findings, changes requested: 1 P1, 0 P2, 2 P3. The P1 is a merge-resolution regression in bdc83a67, not a defect in the design — everything from the 3b391a7e → 0c8ce302 reviews stays resolved, and the 0c8ce302 approval still stands for its own tree.
P1 (tpl-validate-tenant.yaml:83) — the merge left two env: keys on the validate template's script: the nine PARAM_* bindings main introduced at lines 61-79, and this branch's PARAM_CONTAINER_REGISTRY at 83-85. YAML keeps the last, so the container gets one variable; TEAM="$PARAM_TEAM_NAME" is empty and step 1 refuses with ❌ team_name must be lowercase alphanumeric with hyphens on every deploy-service and preview-deploy run. Both red checks on this commit are that line: yamllint reports 83:9 [key-duplicates], and validate-shell-param-interpolation.py names all nine PARAM_* as unbound while omitting PARAM_CONTAINER_REGISTRY — which is the parser telling you which block survived. Neither parent has the duplicate. Fold the one entry into the first list and drop the second key; the new image-name template (one env, one envFrom) is the shape to copy.
Knock-on worth fixing together: validate dies at step 21, so this PR's own Unit-test the image-name ownership check (validate-manifests.yml:437) has not run at this head, and main reshaped the tree it asserts against — service-templates/openclaw and the three openclaw services are gone, seven services are new, so the real-tree loops now cover 27 services and 30 values files. I read every repository: line and every block-scalar opener in the new tree against values-images.sh and found nothing it should trip on (claude-remote:810's JSON is still inside the policy.json: | scalar opened at 794; mctl-telegram:600's {{ .Values.image.repository }} still misses the gate), and checked every one of the 27 services against the policy by hand plus every non-service ghcr.io/mctlhq/* reference against reserved/reserved-prefix — all pass. That is reading, not the suite.
Verified intact after the merge, rather than assumed: platform is added to all six copies of the shared tenant_name_reserved guard across create/delete/delete-safe, and the test now runs the guard itself out of the BEGIN/END tenant-ns-guard markers instead of parsing its case patterns, which is the right adaptation to main's refactor; wft-deploy-service's build task still passes team_name/component_name and still gates on "{{tasks.validate.outputs.parameters.builds}}" == "true"; wft-preview-deploy's image-name task, its git_ref handover and the dependencies: [image-name, build-image] are unchanged; build-image.yaml's Classify caller, both if: steps.caller.outputs.direct == 'true' guards, the conditional :latest and the policy checkout pinned to main are unchanged, and team_name/component_name are now required: true under both triggers on main itself; validate's onboard template selection still matches tpl-git-commit.yaml:335-342 after that file moved.
P3s: the shared openclaw rationale (and the matching examples at tpl-validate-tenant.yaml:171 and wft-deploy-service.yaml:130) describes a service template and three services main deleted — the line should stay, because without it rule 5 would let any team build ghcr.io/mctlhq/openclaw, but that is now its only justification and the comment doesn't say so. And the carried-over one from 0c8ce302: the placeholder guard at tpl-validate-tenant.yaml:202 is __[A-Z][A-Z_]*__, which excludes digits, so a future template rendering e.g. __REGION1__-__SERVICE_NAME__ slips past it into the silent BUILDS=false the guard exists to prevent; __[A-Z][A-Z0-9_]*__ closes it.
Not verified here: python3 tests/test_check_image_name.py, shellcheck, actionlint and argo lint --offline all needed command approval in this job, and CI's own run of the suite was cut short by the P1, so no execution evidence exists for this head either way.
| # preview-deploy checks in build mode whenever it builds, so such a preview | ||
| # is refused for every team. A successor sync that builds the image again | ||
| # needs a reviewed grant line here. | ||
| shared openclaw |
There was a problem hiding this comment.
P3 — the rationale above this line describes a tree the merge just deleted, and the line itself no longer behaves the way the comment says.
bdc83a67 brought in main's openclaw decommission: da3171b9..f13d2a47 removes service-templates/openclaw/values.yaml.tpl and all three of services/admins/openclaw/, services/labs/openclaw/ and services/ovk/openclaw/. So at this commit:
- "several teams hold a service of that name (labs, ovk, admins)" — none do;
ls -d platform-gitops/services/*/openclawis empty. - "each runs the fixed image mctl-openclaw (service-templates/openclaw), which the mctl- prefix reserves" — that template is gone, so
service_template: openclawnow fails the-dtest intpl-validate-tenant.yaml:179and falls through todefault, which rendersrepository: ghcr.io/mctlhq/__SERVICE_NAME__. - "and deploy-service does not build for that template" — it does now, for exactly that reason.
The net effect is that shared openclaw is a run-only reservation for a name nothing runs and nothing can build. An onboard of a service named openclaw with a dockerfile_repo gets BUILDS=true -> --build -> rule 3 -> refused for every team; without a dockerfile_repo it is allowed and creates a service pointing at an image no one may push. Both fail safe, so this is documentation drift rather than a defect — but the comment is what the next reviewer reads before deciding whether to add a grant here, and it currently argues from three services and a template that do not exist. Either trim it to "nothing in this repository runs or builds openclaw; shared so the service name stays available, no grant so no team may push it", or make the name reserved, which is what the behaviour has collapsed to.
| # substituted, the placeholders an image name can use); on any | ||
| # other action, the existing values.yaml. If they name an image and | ||
| # none of them is <registry>/<component_name>, the service runs | ||
| # another image (e.g. the openclaw template's mctl-openclaw), and |
There was a problem hiding this comment.
P3 — the only worked example in this comment block is a template the merge removed, and it is also the one place where the merge changed behaviour.
bdc83a67 brings in main's deletion of service-templates/openclaw/values.yaml.tpl, so on onboard with service_template: openclaw the -d guard at line 179 now misses and selection falls through to default/values.yaml.tpl — which renders ghcr.io/mctlhq/<component_name>, giving BUILDS=true where this comment says the openclaw case is the canonical false. The same example appears in wft-deploy-service.yaml:133.
The logic is unaffected and still correct: the decision reads the rendered template rather than its name, which is precisely why nothing broke when the template vanished. That is worth saying instead — the rule holds for any template that pins a fixed image, and the test now exercises it through the synthetic runs-fixed fixture (tests/test_check_image_name.py:263) rather than through openclaw, so the code and the fixtures already moved on while these two comments did not.
| elif ! IMAGES=$(sh platform-gitops/argo-workflows/scripts/values-images.sh "$VALUES"); then | ||
| echo "❌ cannot read the images ${VALUES} runs; not deciding whether to build" | ||
| exit 1 | ||
| elif printf '%s\n' "$IMAGES" | grep -q '__[A-Z][A-Z_]*__'; then |
There was a problem hiding this comment.
P3 — carried over from the 0c8ce302 review, still open: the placeholder guard excludes digits.
__[A-Z][A-Z_]*__ catches all five placeholders tpl-git-commit.yaml:346-350 substitutes today (__TEAM_NAME__, __SERVICE_NAME__, __IMAGE_TAG__, __PORT__, __HOST__), all digit-free. It does not catch one containing a digit: in __REGION1__ the match needs __ after __REGION, finds 1, and fails. A future template rendering repository: ghcr.io/mctlhq/__REGION1__-__SERVICE_NAME__ therefore passes the guard, compares unequal at line 203, and lands in the silent BUILDS=false this guard exists to prevent — an onboard that builds nothing and then bumps image.tag to a tag nothing pushed.
__[A-Z][A-Z0-9_]*__ closes it. Nothing upstream filters digits out first: values-images.sh:79's value class is [A-Za-z0-9._\/:@-], which admits both digits and _, so such a value reaches this line intact.
There was a problem hiding this comment.
No P1/P2 findings (3 P3). Good to merge.
The PR content is unchanged from the approved 0c8ce302 except for the merge of main (bdc83a67) and one fix (ceaf89fb). git diff 0c8ce302 ceaf89fb on check-image-name.sh, values-images.sh and image-names.txt is empty, so this review is about the merge resolution and the fix.
agy's P1 on bdc83a67 was real, and ceaf89fb fixes it correctly. Main had converted tpl-validate-tenant's validate from {{inputs.parameters.*}} interpolation to a nine-entry env: block; the merge left the PR's PARAM_CONTAINER_REGISTRY in a second env: after envFrom:, and sigs.k8s.io/yaml is last-wins, so the first nine would have been dropped and every deploy would have failed at invalid team:. At ceaf89fb the key is merged into the existing block (tpl-validate-tenant.yaml:78-79), and the file has exactly one env: and one envFrom: per script.
Merge resolution checked against each of main's refactors in these files, not just for conflict markers.
tenant_name_reserved: main replaced thefor reserved in ...loop with a# BEGIN/END tenant-ns-guardblock thattests/test_wft_create_tenant_existing.py:174asserts is byte-identical across six copies.platformis added to all six, identically, inside the guard region.platformis not a tenant directory and is not in that test'sMUST_ACCEPT, andplatform-xwas already refused by theplatform-*pattern, so nothing it pins regresses. The PR's own assertion moved with it — it now runs the extracted guard rather than parsing the retired loop line.validate's env conversion: the PR's step 3b reads$PARAM_CONTAINER_REGISTRYthe same way main's code reads its nine, and interpolates nothing into the script body (test_shell_param_injection.pypasses).tpl-git-commit's template selection: main kept it at:335-342, and the PR's onboard mirror is still line-for-line the same three branches. Main's substitution set is now__TEAM_NAME__/__SERVICE_NAME__/__IMAGE_TAG__/__PORT__/__HOST__; the PR substitutes the two that can appear in arepository:value and refuses any image still carrying a placeholder.wft-deploy-service: main addedwrite-secrets-handoffand put it incommit'sdependencies;build-imagekeeps legacydependencies, which treatsSkippedas satisfied, so the newwhenonbuildscomposes.team_name/component_nameare present on the build task and in the dispatch payload.- No new direct dispatcher of
build-image.yamlarrived with main:erpact-images.yamlis deliberately separate and does not push intoghcr.io/mctlhq, andrelease-deploy.yaml:58is still the onlyuses:.
This verdict is verified, not read. Validate Manifests on this commit (run 38162731557, job 114538850814, head ceaf89fb) logs OK: 33 fixture cases, 28 existing services, 9 referenced platform images, 31 real values files read. The real-tree sweep matters more here than in earlier rounds: main rewrote labs/claude-remote/values.yaml (216 lines), both mctl-telegram values files and the two mctl-agents-worker-* files, and the reader still agrees with yaml.safe_load on all 31. ubuntu-latest means mawk, so the awk-portability question stays settled.
P3s, all documentation drift created by the merge plus one carried-over nit. Main deleted service-templates/openclaw/ and all three services/*/openclaw/ directories, so: the shared openclaw rationale in image-names.txt:78-88 argues from three services and a template that no longer exist, and the line has collapsed to behaving like reserved; the openclaw template's mctl-openclaw example in tpl-validate-tenant.yaml:170 and wft-deploy-service.yaml:133 is now the one case where the merge changed behaviour (that template name falls through to default, so it builds), while the code is unaffected precisely because the decision reads the rendered values rather than the template name — the test already moved to a synthetic runs-fixed fixture. And the placeholder guard at :201 is still __[A-Z][A-Z_]*__, which a digit-bearing placeholder escapes into the silent BUILDS=false it exists to prevent.
|
Triage before merge (head ceaf89f):
|
What
Reserve image names, and build only names owned by the requesting team.
Tenant builds push to
<registry>/<component_name>, and tenant services run that same name, so the component name is the image name. Until now nothing tied that name to the team asking for it.The rule
platform-gitops/argo-workflows/scripts/check-image-name.sh [--tenant] [--build] <checkout> <registry> <team> <name>uses POSIX sh, grep and awk, so it runs in busybox (alpine/git) and on GitHub runners. It decides in this order:0.
--tenant(Argo callers, whereteamis requester-supplied):teammust haveplatform-gitops/tenants/<team>/and is neverplatform.grant <team> <name>line allows.reserved <name>line, or areserved-prefixmatch (mctl-), refuses.shared <name>line allows a run. It refuses a build (--build) unless step 1 matched.services/<other>/<name>, or one of itsvalues.yamlruns<registry>/<name>: refused.Exit codes: 0 allows, 1 refuses, 2 means it could not decide. Callers treat any non-zero exit as a refusal. "Could not decide" covers:
--tenant) tenants tree;registryline;The list
config/image-names.txtholds:registry ghcr.io/mctlhq;mctl-academy, and teamplatformfor the images built by hand fromplatform-gitops/images/;shared openclaw+grant labs openclaw. Several teams have anopenclawservice, which runs the fixedmctl-openclawimage. Only mctl-openclaw's upstream sync (as labs) buildsopenclaw.Where it runs
validate: runs--tenant, right after the gitops clone, for every action, with deploy-service'scontainer_registry.--buildflag and a newbuildsoutput. deploy-service'sbuild-imagestep runs on that output alone, so the check and the build cannot disagree.update-configor without adockerfile_repo.onboard, the template tpl-git-commit will render (same selection,__SERVICE_NAME__substituted), so the openclaw template does not build;values.yaml.<registry>/<component_name>, or name no image.scripts/values-images.sh, whichcheck-image-name.shalso uses. It readsrepository: <image>, quoted or not, with an optional trailing comment. Any other form of the key (flow style, a quoted key, a block-scalar value, an alias) is "could not decide": validate fails the step, and the check exits 2. Block scalars, whether a key or a sequence item (- |) opens them, are text scoped as in YAML, and are skipped. An onboard image still holding a__PLACEHOLDER__after__SERVICE_NAME__/__TEAM_NAME__substitution also fails the step.build-imagewhenreads validate's newbuildsoutput, sowft-deploy-serviceandtpl-validate-tenantmust be on the cluster together. Against an oldertpl-validate-tenant, a run errors at parameter resolution and nothing is built or committed. Both are in the same Argo CD Application, so wait for the sync before submitting.image-namestep (tpl-validate-tenant, an anonymous clone of the public repo,--tenant,--buildwhengit_refis set). Both the build and the deploy depend on it.build-image.yaml: runs--buildbefore any credential is fetched, against${REGISTRY}/${GITHUB_REPOSITORY_OWNER}. A direct dispatch must also push exactly to<registry>/<component_name>. The policy is read frommain, not from caller input.What
build-image.yamlskipsworkflow_callfromrelease-deploy.yaml) are not checked there. Every other dispatcher in the organisation goes through this path, verified on each default branch:release-please.ymlin mctl-academy, mctl-agent, mctl-agents, mctl-api, mctl-docs, mctl-portal, mctl-telegram, mctl-web, portfolio and seerrsense;release-deploy.ymlin mctl-alice;deploy.ymlin mctl-design; andpreview-deploy.ymlin mctl-telegram. A test runs the caller classification for a release-deploy ref. Their image names follow the release and can differ from the component name.github.workflow_ref, bound explicitly into the classifying step, tells the two cases apart: in a called run it names the caller's workflow.mainruns that ref's copy of the workflow; the policy data is still read frommain.actions:writeon this repository, and the workflow comment names them.:latest; release builds keep it. Nothing in this repository or in the cluster pullsghcr.io/mctlhq/*:latest.Other changes
wft-create-tenantrefuses the tenant nameplatform.Effect on today's services
services/<team>/<name>pass, and the test checks this on every run.build-image.yamldispatchers found on the organisation's default branches still pass: mctl-openclaw's upstream sync, and the hand-built platform images.grantline first.labs/mctl-telegram-previewrunsghcr.io/mctlhq/mctl-telegram, so a deploy-servicedeployof it no longer dispatches a build of a name nothing runs.Tests
tests/test_check_image_name.pyis added tovalidate-manifests.yml. It covers:--tenant,--build,shared, the registry checks, and a missing tenants tree.services/is reserved (11 today).deployof an existing openclaw service with a defaultedservice_template, a trailing comment, an unknown template and a worker-service onboard.values-images.sh: a 34-case table. Its output matches a structural YAML parse on every existing service and template, and on about 4,900 generated documents (checked locally). The suite passes under macOS awk, busybox awk and mawk.builds;git_reftoimage-name;team_nameandcomponent_namefrom the build task into thebuild-image.yamldispatch payload;:latestis conditional;platformis a reserved tenant name.Mutation evidence: each of these makes the test fail:
--tenantcheck, theplatformexception, the shared-build refusal, the registry equality or the tenants-tree precondition.--build; dropping thebuildsoutput.--tenant; restoring the oldwhen; droppingteam_namefrom deploy-service's build task; droppingcomponent_namefrom preview-deploy's dispatch payload; dropping preview-deploy'sgit_refhandover or its dependency.:latestunconditionally; dropping the image-name pin; classifying the caller from anything but the boundgithub.workflow_ref.repository; validate not failing on unreadable values, or ignoring the template on onboard, or using the wrong worker path; the ownership scan skipping an unreadable values.yaml.The test also passes under busybox sh (python:3.12-alpine). shellcheck is clean on the script. Compared with
main,argo lint --offlinereports no new findings on the changed templates. The shell-interpolation, shared-writes and yq-interpolation validators pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01D1kGUJMWeA8hyhQuxKuHd2