Repository navigation
fix(cd): a bake-only reconcile no longer blames promote for its own broken query - #2642
Merged
Merged
Conversation
…roken query The read a bake-only reconcile uses to recover the release version off the promoted image has never once succeeded. It crashed, the crash was swallowed, and the resulting error accused promote — which was working fine. `az acr manifest list-metadata --query "[?contains(tags, '<sha>')]"` throws `In function contains(), invalid type for value: None` on any manifest whose `tags` is null — the orphaned per-platform children an index push leaves behind (16 of memex-portal-ai's 2101 manifests). jmespath aborts the WHOLE query on that throw, so one orphan makes the read unable to find any tag. `2>/dev/null || true` then turned that crash into an empty tag list, and the step reported "memex-portal-ai:<sha> carries no version tag ... promote's Phase C never armed a release for this sha. Fix promote." Promote had armed it: memex-portal-ai:aaf95af, :main and :3.0.0-rc8.ci.6360 all sit on digest sha256:6c3abd508033db59865e8bedf68076bdb75b4c044e3fa1159c01eff98b2a1089. Nine scheduled reconciles (2026-08-28 22:12Z through 08-29 05:26Z) died pointing at the healthy half of CD. The swallow is the root cause; the guard only stops the crash: - `tags && contains(tags, ...)` short-circuits the untagged rows out. - the stderr swallow is gone, so an az failure now fails the step with az's own message instead of masquerading as a missing tag. Verified against live ACR: aaf95af resolves 3.0.0-rc8.ci.6360; a sha with no version tag still exits 1 with the (now accurate) message; a failing az exits 1 carrying az's error text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ing --detail) `az acr repository show-tags` returns an array of plain STRINGS unless `--detail` is passed, so `--query "[].name"` matched nothing and newest_short was always empty. Every gate log has read `Newest published set: <none>` for as long as this step has existed. Latent, not the outage: the probe fails open (relevant stays true), so it never turned CD red. It only meant the image-irrelevant skip below could never fire, and CD rebuilt the full image set for clients-only changes it was written to ignore. The batching probe 60 lines up already passes --detail; this one did not. Note the behaviour this restores: with the probe working, a push whose whole diff since the newest published set is under clients/(react-native|voice-gateway)/ will now correctly skip the image build, which it never did before. With --detail the probe returns aaf95af, as intended. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the main-cd.yml reconcile path so bake-only runs correctly recover the promoted release version from ACR tags (and fail with actionable az errors instead of silently misdiagnosing missing tags), and restores the “image-irrelevant changes” skip logic by making the show-tags query actually return tag objects.
Changes:
- Add
--detailtoaz acr repository show-tagsso--query "[].name"works andnewest_shortis no longer always empty. - Harden the bake-only “resolve release version” ACR query against
nulltag sets (tags && contains(...)) and stop swallowingazstderr so real query failures fail loudly.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Test Results (shard 3)490 tests 486 ✅ 4m 20s ⏱️ Results for commit 9cae035. |
Contributor
Test Results (shard 0)898 tests 898 ✅ 5m 36s ⏱️ Results for commit 9cae035. |
Contributor
Test Results (shard 2)957 tests 957 ✅ 5m 28s ⏱️ Results for commit 9cae035. |
Contributor
Test Results (shard 1)1 020 tests 1 020 ✅ 6m 25s ⏱️ Results for commit 9cae035. |
Contributor
Test Results (shard 5)1 585 tests 1 584 ✅ 7m 14s ⏱️ Results for commit 9cae035. |
Contributor
Test Results (shard 4) 10 files 10 suites 6m 0s ⏱️ Results for commit 9cae035. |
Contributor
Test Results 42 files 42 suites 35m 5s ⏱️ Results for commit 9cae035. |
rbuergi
added a commit
that referenced
this pull request
Aug 30, 2026
…ness writing to GitHub ## The gap main-cd's `decide` step chooses between building an image set and re-asserting the content bake (`bake_only`). The bake-only branch is reached only on a reconcile that finds a COMPLETE image set — i.e. only when nobody is pushing. On a busy trunk it never runs: its first execution was in production at 22:12 on a Friday, and it failed nine times out of nine over 33 h, because every fix-verifying push took the other branch and repaired the APPEARANCE of the problem. #2642 fixed that specific defect and test-cd-steps.py executes THAT step. What neither did is remove the shape, which is what this issue is about. `decide` is a pure-shell step with `id:`, no `${{ }}`, and every input arriving via `env:` — exactly the shape the harness already requires — so it is extracted, not copied, and driven with the inputs the real step reads. Four cases: the bake-only branch is taken on a complete set; it does not also publish; it says WHY in the decision log; and — the one that keeps the other three honest — an INCOMPLETE set on the same event still builds. If those two ever collapse into one, delivery either stops or doubles. Vacuity guard in the harness's own idiom: if the step stops writing `bake_only=true` it dies rather than passing, the same way the release cases die if the step stops calling `az acr manifest list-metadata`. Mutation-checked: making the branch unreachable (`COMPLETE = "MUTATED"`) fails three of the four; restored, all pass. ## 🚨 A safety device the harness was missing Writing these surfaced something worse than the gap. `decide` posts a heal COMMENT to the CD-failure issue on a reconcile attempt, and `run_step` inherits the caller's environment — so on a machine with a live `gh` login, RUNNING THE TESTS POSTS REAL COMMENTS TO REAL ISSUES. It did, three times, to #2810; I deleted them by hand. `gh` is now stubbed on PATH beside `az` (records the call, answers nothing, so the step's control flow is unchanged and its side effect is not), and GH_TOKEN / GITHUB_TOKEN / GH_ENTERPRISE_TOKEN are blanked with GH_CONFIG_DIR pointed into the temp dir. Either alone would do; both are there because one will still be present after someone edits the other. Re-running the mutation created ZERO comments. The harness also now supplies GITHUB_STEP_SUMMARY and GITHUB_REPOSITORY, which the runner provides to every step and without which a `set -u` step dies for a reason unrelated to the logic under test — which is how a harness gets disabled. python3 .github/scripts/test-cd-steps.py all cases passed, 2 steps Fixes #2643 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FhP7Jg65q8cAATnFfeaTK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Core
mainCD was red on nine consecutive scheduled reconciles (2026-08-28 22:12Z → 08-29 05:26Z). The delivery itself was fine the whole time — what was broken is the reconcile's release-version read, and its error message sent readers at the wrong component.What was actually failing
Every red run failed on one step,
Resolve the release version this bake belongs to:Two things are wrong with that sentence.
1. The error text accuses a healthy component
Promote had armed the release. Run 33210930983 job
98987531545pushed all three tags onto one digest:and the bake in that same run published, sealed and wrote
_releases/3.0.0-rc8.ci.6360 → identity s35f21e69e34d028fa49c70d79455a93d.aaf95afwas fully delivered at 21:23Z. Every later reconcile then told the reader that the one component doing its job was the one to fix.An error that names a healthy component as the culprit is worse than a silent failure — it is a silent failure with directions attached. It cost nine hours in the wrong direction.
2. The swallow is the root cause; the query crash is only the trigger
tagsis null on an untagged manifest — the orphaned per-platform children an index push leaves behind.memex-portal-aiholds 2101 manifests, 16 of them untagged (dated 08-16 → 08-22). jmespath'scontains()throws on null rather than returning false, and the throw aborts the whole query:So one orphan makes the read find nothing. Then
2>/dev/null || trueconverted "the query crashed with an invalid-jmespath error" into "there is no version tag" — and only because of that could the step reach the message blaming promote. Removing the swallow is the more important half of this fix; thetags &&guard just stops the crash.The fix (commit 62f4734)
[?tags && contains(tags, '${SHORT_SHA}')]—&&short-circuits, so untagged rows never reachcontains().2>/dev/null || trueremoved — an az failure now fails the step with az's own message, so the next reader debugs the query rather than promote.Verified against live ACR, all three paths:
aaf95af, bake-onlyrelease version 3.0.0-rc8.ci.6360— what the nine red runs should have printedSecond, separate defect (commit 9cae035)
az acr repository show-tagsreturns an array of plain strings unless--detailis passed, so the gate's--query "[].name"matched nothing andnewest_shortwas always empty. Every gate log has readNewest published set: <none>for as long as that step has existed.This is latent, not the outage: it fails open (
relevantstays true), so it never turned CD red — it only meant the image-irrelevant skip could never fire. The batching probe 60 lines above already passes--detail; this one did not. Kept as its own commit for that reason.Behaviour this restores, worth knowing: a push whose entire diff since the newest published set is under
clients/(react-native|voice-gateway)/will now correctly skip the image build, which it never did before.Not caused by
aaf95afc3aaf95afc3(#2630, workflows-only) did not cause this. It is implicated only as the sha that happened to be main's tip when ACR returned HTTP 500 on two blob uploads (jobs98980554453,98980554462in run 33209932873) and then sat still. That 500 was transient and the 21:04Z reconcile healed it.The real precondition is main going quiet — the first reconcile that finds a complete image set and nothing to build is the first one to take the bake-only path at all. Tracked separately in #2643, because the next merge to main masks it — and it already did:
origin/mainmoved to5f067884e(#2634) mid-investigation.🤖 Generated with Claude Code