Repository navigation
fix(agents): skip Telegram notify on no-op Succeeded runs - #530
Conversation
implement (*/5), issue-poll (*/15), and shepherd (hourly) sent an unconditional Telegram Succeeded message even on steady-state ticks that found nothing to do (empty proposal queue / no labelled issues / no proposal transitions) — drowning out the runs that did real work. commit-and-push already distinguished "nothing to commit" from a real push; surface that as a workflow output (activity=none|yes) and have notify-telegram skip the Telegram leg only when Succeeded AND no activity. Failed/Error always notify regardless.
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude finished @mashkovd's task in 1m 44s —— View job Code Review: fix/agents-notify-noop-noiseAnalysisReviewed PR diff across three CronWorkflow templates to reduce Telegram notification spam on steady-state no-op runs. The solution adds an Findings✅ Correctness
✅ Logic & Edge Cases
✅ Shell Safety
✅ YAML & Conventions
✅ Operational Impact
VerdictNo P1/P2 findings — good to merge. Clean, well-documented implementation that solves the stated problem. Runtime testing can proceed post-merge per the test plan. |
…check Same reason shepherd was already excluded: the community CRD schema catalog doesn't recognize .spec.outputs on a ClusterWorkflowTemplate, even though it's required for the onExit handler's workflow.outputs.parameters.activity reference to pass Argo's own static validator.
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude finished @mashkovd's task in 1m 28s —— View job PR Review: fix(agents) — skip Telegram notify on no-op runsSummaryThis PR reduces Telegram notification spam by gating the notification leg of three CronWorkflows (implement, issue-poll, shepherd) to skip on successful runs that found nothing to do. Changes reviewed:
AnalysisCorrectness
Argo Workflows Specifics
Operational Safety
Conventions
VerdictNo P1/P2 findings, good to merge. This is a well-implemented noise reduction that correctly gates notifications without sacrificing safety. The parameter passing is correct, the logic is sound, and the validation update properly reflects the schema exceptions. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0dd87d8a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # at the */15 cadence that's the steady-state outcome. JSON-encode | ||
| # the body via jq so a quote / newline / backslash cannot malform | ||
| # the payload and silently drop it. | ||
| if [ "$WORKFLOW_STATUS" = "Succeeded" ] && [ "$WORKFLOW_ACTIVITY" = "none" ]; then |
There was a problem hiding this comment.
In the issue-poll workflow, poll uses continueOn: failed/error and there is no later assertion step, so WORKFLOW_STATUS can still be Succeeded when run-poller exits before writing proposal files (for example when GITHUB_TOKEN is missing). In that case commit-and-push records activity=none, and this new branch suppresses the only Telegram/incident signal for a broken poller; include the poll step status in the gate or assert it before treating activity=none as a harmless no-op.
Useful? React with 👍 / 👎.
| # unconditional ✅ every hour drowned out real merges/rejections. | ||
| # Failed/Error/Unknown (incl. a post-deploy-verify degraded-apps | ||
| # failure) always notify regardless of activity. | ||
| if [ "$WORKFLOW_STATUS" = "Succeeded" ] && [ "$WORKFLOW_ACTIVITY" = "none" ]; then |
There was a problem hiding this comment.
The shepherd step is also marked continueOn: failed/error, and this workflow has no equivalent of the implementer assert-produced step. If run-shepherd crashes before producing any .status.yaml change, commit-and-push emits activity=none, post-deploy-verify can still pass, and this condition suppresses the only Telegram alert even though the shepherd run failed internally; gate the skip on the shepherd step status as well as the workflow status/activity.
Useful? React with 👍 / 👎.
| # Telegram leg on a no-op Succeeded run. | ||
| outputs: | ||
| parameters: | ||
| - name: activity |
There was a problem hiding this comment.
The exit handlers read {{workflow.outputs.parameters.activity}}, but parameters only become available under workflow.outputs when the producing output is exported globally (the existing degraded_apps output in this file does that with globalName). As written, /tmp/activity is only a local steps.commit... output, so no-op Succeeded implement/issue-poll/shepherd runs will not see WORKFLOW_ACTIVITY=none and the Telegram spam this change is meant to suppress will continue; add globalName: activity or pass the entrypoint output through a supported mechanism.
Useful? React with 👍 / 👎.
Summary
implement(/5),issue-poll(/15), andshepherd(hourly) CronWorkflows send an unconditional ✅ Telegram message even on steady-state ticks that did nothing (empty proposal queue / no labelled issues / no proposal transitions) — this is the noise visible in the Telegram screenshot (implement firing every 5 min).commit-and-pushin each CWFT already distinguishes "nothing to commit" from an actual push. Surface that as a workflow output (activity:none|yes) and gatenotify-telegram's Telegram leg: skip only whenWORKFLOW_STATUS=SucceededANDactivity=none. Failed/Error always notify, regardless of activity.Test plan
python3 -c "import yaml; yaml.safe_load_all(open(f))"on all 3 modified templates — valid YAMLimplementrun with an empty queue and confirm no Telegram message, then triggerissue-poll/shepherdsimilarly