Repository navigation
fix(argo): move shepherd's post-deploy-verify verdict onto the artifact channel - #1187
Conversation
…ct channel
Every shepherd run whose Telegram leg fired appended
⚠️ post-deploy-verify flagged: unknown (verification did not complete)
even when post-deploy-verify ran to completion and found nothing. Three
consecutive runs on 2026-09-11 (d6c4ded3, 94ea75cf, 4f1402b6) carried that
line; all three were healthy, and 4f1402b6's verify node finished clean at
exit 0 with `degraded_apps: ""` recorded in the workflow's status.outputs.
The onExit pod started 18s later and still received the raw
`{{...}}` literal in WORKFLOW_DEGRADED_APPS. notify-telegram's guard against
raw template syntax rewrote that to the "unknown" text, and because the
warning was appended on any non-empty value, the guard's own placeholder
triggered it. A correctly resolved empty string would have stayed silent —
so the alert appearing was proof the substitution failed, not that the
cluster was degraded.
Rather than chase why a recorded global does not resolve inside an exit
handler, drop the dependency. post-deploy-verify now writes verify_status
(clean / degraded / kubectl-failed) and the degraded-app list into the
artifact it already ships to /onexit/verify, and notify-telegram reads both
the same way it already reads activity and shepherd_status. A missing file
now means "incomplete" — the state that string was trying to express — and
it is distinct from a clean run instead of identical to a broken
substitution.
The warning is emitted only for degraded / kubectl-failed. `incomplete` is
silent on a Succeeded workflow (verify has no continueOn, so Succeeded means
it finished) and reported as "post-deploy-verify did not run" otherwise. The
Telegram and incident legs now share one VERIFY_NOTE so they cannot
disagree.
Both workflow.outputs.parameters declarations go with it — the one on the
spec and the one on shepherd-proposals existed only to keep the removed
reference passing Argo's static validator. With no reference left, this file
has no output parameters at all, which also retires the one-slot budget the
2026-07-06 incident imposed on it.
Verified with `argo lint --offline` (clean) and shellcheck on both modified
script bodies.
Closes #1186
Claude-Session: https://claude.ai/code/session_01JoFpun8LVPAKJE6QhHSvdA
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Claude finished @mashkovd's task in 10m 59s —— View job Review complete — approved
Verdict: APPROVE — 0 P1, 0 P2, 0 P3. What I checked
No functional, security, or operational-safety issues found. The fix directly addresses the false-positive alert described in #1186 and the artifact-channel approach avoids the |
Antigravity (agy) review — informationalModel: gemini-3.1-pro-high · commit Severity: P2 mkdir -p /tmp/onexit
- printf 'clean' > /tmp/onexit/verify_status
+ printf 'incomplete' > /tmp/onexit/verify_status
: > /tmp/onexit/degraded.txt
...
echo "✓ No persistent Degraded apps detected after shepherd run."
+ printf 'clean' > /tmp/onexit/verify_status |
…e end agy P2 on this PR, and it is correct: seeding `clean` before the 300s sleep reintroduces the bug one layer down. Argo's wait container uploads output artifacts for a FAILED pod too, so a verify step OOMKilled during the sleep — or tripped by a jq error on malformed kubectl output — would ship a `clean` verdict it never reached. onExit suppresses the warning on `clean`, so the operator would get a bare workflow-failure alert and reasonably assume the post-merge check had passed. `verify_status` now starts as `incomplete` and is promoted to `clean` by the last line of the script, after the unfiltered snapshot. Every path that can fail is above that line, so a killed step reports `incomplete` — the same thing an absent file reports, which is what makes the two unreachable paths agree. The Failed/Error wording changes from "did not run" to "did not complete", which is now literally true: the step may well have started. `argo lint --offline` clean; shellcheck clean on both modified scripts. Claude-Session: https://claude.ai/code/session_01JoFpun8LVPAKJE6QhHSvdA
|
Claude finished @mashkovd's task in 4m 2s —— View job Reviewing PR #1187
Scope reviewed: Traced the full artifact-channel rewrite:
No P1/P2 findings. One P3 nit, not worth blocking on:
Could not execute
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
No P1/P2 findings (0 P3). The artifact-channel/verify_status contract is internally consistent: same file names written and read on both ends, order-of-writes puts verify_status/degraded.txt before anything that can fail, and the removed workflow.outputs.parameters blocks have no other consumers in this file. Good to merge.
Antigravity (agy) review — informationalModel: gemini-3.1-pro-high · commit Severity: P3 |
Closes #1186.
Problem
Every shepherd run whose Telegram leg fired appended
even when
post-deploy-verifyran to completion and found nothing wrong.Three consecutive runs on 2026-09-11 (
d6c4ded3,94ea75cf,4f1402b6)carried that line; all three were healthy.
4f1402b6's verify node:phase=Succeeded,exitCode 0, logand the global reached workflow scope —
status.outputsheld{"name":"degraded_apps","value":""}. The onExit pod started 18s later andstill got the raw
{{...}}literal inWORKFLOW_DEGRADED_APPS. The guard at:1257-1265rewrote that to the "unknown" text, and since the warning wasappended on any non-empty value, the guard's own placeholder is what
triggered it. A correctly resolved empty string would have been silent — so
the alert appearing was proof the substitution failed, not that anything was
degraded.
This matters because
post-deploy-verifyis the step meant to catch a mergethat degraded an ArgoCD app, and its all-clear was indistinguishable from its
"I never ran". Seven incident summaries already carry the string.
Change
Drop the dependency on
workflow.outputs.parametersresolution inside an exithandler rather than chase why a recorded global does not resolve there.
post-deploy-verifywritesverify_status(clean/degraded/kubectl-failed) and the filtered degraded-app list into the artifact italready ships to
/onexit/verify. Both are written before anything that canfail.
notify-telegramreads both off that artifact, exactly how it already readsactivityandshepherd_status. A missing file reads asincomplete— thestate the old string was trying to express, now distinct from a clean run
instead of identical to a broken substitution.
degraded/kubectl-failed.incompleteissilent on a
Succeededworkflow (verifyhas nocontinueOn, so Succeededmeans it finished) and reads
post-deploy-verify did not runotherwise.VERIFY_NOTE, so they cannotdisagree.
workflow.outputs.parametersdeclarations are removed — they existedonly to keep the now-removed reference passing Argo's static validator. The
file now has no output parameters at all, which also retires the one-slot
budget the 2026-07-06 incident (fix(agents): skip Telegram notify on no-op Succeeded runs #530/fix(agents): pass activity via workdir PVC, not workflow.outputs #531) imposed on it.
Verification
argo lint --offline cwft-mctl-agents-shepherd.yaml→ clean. This is thecheck that matters: an unresolvable reference here is what put the
CronWorkflow into
SpecErroron 2026-05-06 and again on 2026-07-06, and CIdoes not run
argo lint(only kubeconform).argo lintresolves template tags insiderendered script text too, so even naming the old expression in a shell
comment fails the lint. There is a note in the file saying so.
shellcheckon both modified script bodies → clean.yamllint(repo's relaxed config) → only the four pre-existingcolonswarnings, none on changed lines.
Behaviour on a real degradation is unchanged end-to-end: verify still exits 1,
the workflow still fails, and the alert still names the apps verbatim — it now
gets them from
degraded.txtinstead of an env var.🤖 Generated with Claude Code
https://claude.ai/code/session_01JoFpun8LVPAKJE6QhHSvdA