Repository navigation
fix(cd): report a heal whichever path delivered it - #3078
Conversation
The heal reporter required `needs.gate.outputs.reason == 'reconcile'`, on the reasoning that the reconciler is what repairs a failed delivery. But the PUSH path publishes far more often, and when it is the one that carries main past a failure the heal went unreported. Measured 2026-09-02: #3074 was filed at 08:01 for cb1dd36. Delivery recovered at 08:19:44 on 1b044e5 — six commits later, full set in ACR, pair tag present — via the push path. This step was skipped, so nothing said so, and the issue sat open with zero comments describing a state that no longer existed. That matters more than one stale issue. A ci-failure issue that outlives its cause is the same defect as a check that is red on every pull request: it costs nothing to file and everything to trust. The next real alert arrives into a backlog of alerts already known to be wrong, and is read as more of the same — which is exactly the state this alerting exists to prevent. The condition is now the fact the comment actually asserts: main's HEAD has a complete image set, whoever put it there. `promote.result == 'success'` also implies `portal-image` ran, so the version the comment quotes is always populated — previously a bake-only reconcile could not reach this step at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The newly added/used wording “main’s HEAD” in heal reporting can be factually incorrect if main advances while the workflow is in progress; tightening the message to “target commit” avoids misleading incident tracking.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adjusts the main CD workflow so the “successful heal” reporter runs whenever the image set was successfully promoted and verified, regardless of whether the delivery happened via the reconcile tick or the push-triggered path.
Changes:
- Relax the
Report a successful healstep condition to key only onneeds.promote.result == 'success'. - Add detailed inline rationale/comments documenting the incident motivating the change (#3074) and the intended invariant.
File summaries
| File | Description |
|---|---|
| .github/workflows/main-cd.yml | Ensures heal reporting runs on successful promotion/verification even when delivery occurs via the push path, with added documentation explaining why. |
Review details
Suppressed comments (1)
.github/workflows/main-cd.yml:2351
- The heal comment posted to the
ci-failureissue currently claims “main's HEAD now has the complete image set”, but the workflow verifies the image set forneeds.gate.outputs.short_sha, which may no longer be HEAD if newer commits land while this run is in progress. This can mislead incident tracking; phrase it as “the image set for that commit is complete”.
- name: Report a successful heal
if: needs.promote.result == 'success'
env:
GH_TOKEN: ${{ github.token }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # The condition is now the fact the comment actually asserts: main's HEAD has a complete image | ||
| # set, whoever put it there. |
Test Results (shard 0)234 tests 234 ✅ 2m 17s ⏱️ Results for commit 1769419. |
Test Results (shard 3)612 tests 612 ✅ 52s ⏱️ Results for commit 1769419. |
Test Results (shard 1)430 tests 430 ✅ 56s ⏱️ Results for commit 1769419. |
Test Results (shard 2)557 tests 555 ✅ 1m 37s ⏱️ Results for commit 1769419. |
Test Results (shard 5) 3 files 3 suites 1m 13s ⏱️ Results for commit 1769419. |
Test Results (shard 4)1 781 tests 1 781 ✅ 1m 44s ⏱️ Results for commit 1769419. |
Test Results 14 files 14 suites 8m 41s ⏱️ Results for commit 1769419. |
The heal reporter fired only on the reconcile path. The push path publishes far more often — and when it is the one that carries main past a failure, the recovery went unreported.
Measured
cb1dd3601b044e5— six commits laterreason == 'reconcile'was false)The full set was in ACR with the pair tag present. Nothing said so.
Why this is worth fixing rather than closing by hand
A
ci-failureissue that outlives its cause is the same defect as a check that is red on every pull request — the one #3064 and #3075 were about. It costs nothing to file and everything to trust. The next genuine alert arrives into a backlog of alerts already known to be wrong, and gets read as more of the same. That is precisely the state this alerting exists to prevent, so the alerting undermining itself is worse than an ordinary bug.The change
The condition becomes the fact the comment actually asserts — main's HEAD has a complete image set — rather than the path that achieved it:
A side benefit:
promote.result == 'success'impliesportal-imageran, so the version the comment quotes is always populated. Under the old condition a bake-only reconcile could not reach this step at all, so the narrower form was not even covering the case it was written for.Workflow-only; no test surface.
MeshWeaver.Documentation.Test(which parses these workflows): 180 passed, 0 failed.🤖 Generated with Claude Code