Skip to content

fix: silence mctl-agents-reconcile Telegram notify on routine success - #536

Merged
mashkovd merged 1 commit into
mainfrom
fix/agents-reconcile-notify-noise
Jul 7, 2026
Merged

mashkovd merged 1 commit into
mainfrom
fix/agents-reconcile-notify-noise

Conversation

@mashkovd

@mashkovd mashkovd commented Jul 7, 2026

Copy link
Copy Markdown
Member

Summary

  • mctl-agents-reconcile's hourly CronWorkflow notifies Telegram on every tick regardless of outcome — the screenshot the user shared showed repeated "✅ mctl-agents: reconcile — Succeeded" messages hourly, with nothing actionable in any of them.
  • reconcile only repairs rare dropped .status.yaml write-backs (per the CronWorkflow's own description) — a routine successful tick has nothing worth reporting.
  • Applies the same quiet-unless-action-or-failure policy already used by argo-pvc-janitor: skip the Telegram leg when WORKFLOW_STATUS == Succeeded, keep it for Failed/Error/other.

Test plan

  • YAML parses (python3 -c "import yaml; yaml.safe_load(...)")
  • No literal {{...}} introduced in comments (glob-scan validator safe)
  • After merge + next hourly tick (~:15 UTC): confirm no Telegram message on a routine Succeeded run
  • @claude review and CI green before merge

The hourly reconcile CronWorkflow notified on every tick regardless of
outcome, spamming Telegram with "✅ Succeeded" messages even though
reconcile only repairs rare dropped write-backs and a routine tick has
nothing to report. Apply the same quiet-unless-action-or-failure policy
already used by argo-pvc-janitor: skip the Telegram leg entirely when
WORKFLOW_STATUS is Succeeded, keep it for Failed/Error/other.
@claude

claude Bot commented Jul 7, 2026 •

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 7, 2026 •

Copy link
Copy Markdown

Claude finished @mashkovd's task in 37s —— View job


Code Review — PR #536

Reviewing: cwft-mctl-agents-reconcile.yaml — silence Telegram notifications on routine reconcile success

  • Gather context and read changed files
  • Validate YAML structure and Argo Workflows patterns
  • Check for security, correctness, and operational issues
  • Post review findings

Summary

✅ No P1/P2 findings — good to merge

Change: Adds a guard clause to skip Telegram notifications when WORKFLOW_STATUS == "Succeeded", implementing the "quiet unless actionable or failed" policy already used by argo-pvc-janitor.

What's correct:

  • Shell syntax and variable expansion are safe (variables injected as env, not inline in script)
  • Early exit 0 prevents curl calls for routine ticks, reducing API spam and noise
  • Case statement cleanup (removing now-unreachable "Succeeded" case) is clean
  • Argo Workflows onExit handler pattern is correct: set +e at top, final exit 0 ensures workflow isn't failed by notification step
  • Comment block clearly explains the policy and rationale (per CLAUDE.md, comments explain WHY)
  • Aligns with existing precedent in the codebase

No security, correctness, or operational issues found. The change is focused, well-motivated, and follows established patterns.


@mashkovd
mashkovd merged commit 5d7303c into main Jul 7, 2026
6 checks passed
@mashkovd
mashkovd deleted the fix/agents-reconcile-notify-noise branch July 7, 2026 08:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e47b12341

ℹ️ 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".

Comment on lines +305 to +307
if [ "$WORKFLOW_STATUS" = "Succeeded" ]; then
echo "reconcile succeeded; routine tick, skipping Telegram notify."
exit 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve success notifications when reconcile repaired state

When run_shepherd --reconcile actually flips one or more .status.yaml files, commit-and-push commits/pushes them and exits 0, so the workflow still reaches WORKFLOW_STATUS=Succeeded; this new unconditional success guard therefore suppresses the only Telegram signal for the non-routine case where the reconciler did something. The nearby argo-pvc-janitor/agent workflows skip only Succeeded runs with no activity, so this needs an activity marker or equivalent check instead of treating every successful reconcile as a routine tick.

Useful? React with 👍 / 👎.

Comment on lines +305 to +307
if [ "$WORKFLOW_STATUS" = "Succeeded" ]; then
echo "reconcile succeeded; routine tick, skipping Telegram notify."
exit 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Report reconcile failures masked by continueOn

If run-reconcile exits non-zero before producing any status changes, the step's continueOn lets commit-and-push run and then succeed with no diff, a pattern this repo already documents as making the whole workflow report Succeeded. This guard now exits before Telegram in that scenario, so token/API/auth regressions can silently stop the reconciler; check steps.reconcile.status or add a final assert before treating a succeeded workflow as a routine tick.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant