fix: Backup Monitoring Enabled

[ainsleyclark]

No description provided.

[claude]

Review summary

• Overall score: 9/10
• Critical issues: 0
• Warnings: 0
• Suggestions: 1
• Recommendation: ✅ Approve

Excellent implementation that properly respects both global and per-resource monitoring configuration. The PR correctly implements conditional monitoring for backup workflows with comprehensive test coverage.

Critical issues 🔴

None

Warnings 🟡

None

Suggestions 🟢

Test coverage for codebase backup monitoring

The new tests thoroughly verify per-resource monitoring behaviour (global disabled, per-resource disabled, mixed scenarios), which is excellent. Consider whether an explicit test case for codebase backup monitoring in isolation would add value, though the existing tests do verify it indirectly through the PROD_CODEBASE_BACKUP_PING_URL assertions.

Implementation analysis

What changed:

• Added MonitoringEnabled to template data in backup.go:70 using appDef.Monitoring.IsEnabled()
• Updated backup template to conditionally render Peekaping ping steps based on $.MonitoringEnabled (global) AND .Monitoring (per-resource)
• Added three new test cases covering monitoring scenarios
• Updated existing tests to include Monitoring: true on resources (maintaining existing behaviour)

Why it works:
The template logic {{- if and $.MonitoringEnabled .Monitoring }} correctly implements a two-level check:

1. Global monitoring must be enabled (defaults to true via IsEnabled())
2. Per-resource monitoring flag must be true

This allows fine-grained control: disable all monitoring globally, or selectively disable specific resource monitors while keeping others active.

Test quality:

• Follows project conventions (parallel tests, table-driven where appropriate, t.Run subtests for complex scenarios)
• Tests all permutations: global off, per-resource off, mixed states
• Verifies both presence and absence of monitoring configuration in generated YAML
• Includes YAML validation via validateGithubYaml

Unrelated changes:
The release_test.go changes (removing ./ prefix from context paths) appear to be test fixes unrelated to monitoring. These align with Docker build context conventions and are safe.

---

Code quality: Production-ready with excellent test coverage and clean implementation following project patterns.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.39%. Comparing base (7f6b060) to head (17a5161).
⚠️ Report is 417 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #338      +/-   ##
==========================================
+ Coverage   64.59%   69.39%   +4.79%     
==========================================
  Files         154      184      +30     
  Lines        6064     7253    +1189     
==========================================
+ Hits         3917     5033    +1116     
+ Misses       2064     2023      -41     
- Partials       83      197     +114     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.