SPLAT-2834: Prototype for PR age monitoring for Nutanix CI - #1
Conversation
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a Go-based CI cluster monitor that loads repository policies, retrieves GitHub pull requests and workflow statuses, classifies severity, reports results through table or JSON output, and runs through local, GitHub Actions, and scheduled Prow workflows. ChangesPR Age Monitor
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ConfigLoader
participant Monitor
participant GitHubAPI
CLI->>ConfigLoader: Load repository configuration
ConfigLoader-->>CLI: Validated configuration
CLI->>Monitor: Run monitor
Monitor->>GitHubAPI: List open pull requests
GitHubAPI-->>Monitor: Pull request metadata
Monitor->>GitHubAPI: List workflow runs
GitHubAPI-->>Monitor: Workflow statuses
Monitor-->>CLI: Classified repository reports
CLI-->>CLI: Render output and determine exit status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
internal/monitor/types.go (1)
24-27: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winExpose report durations in an explicit JSON format.
If
PRMetricsis serialized directly,time.Durationbecomes an integer nanosecond count. Emit duration strings or documented numeric units while retaining durations internally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/monitor/types.go` around lines 24 - 27, Update the JSON serialization of the duration fields in PRMetrics, specifically Age and SinceLastUpdate, so direct serialization emits an explicit format such as duration strings or documented numeric units instead of raw nanoseconds. Retain time.Duration internally and apply the chosen format consistently to both fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yaml:
- Around line 23-24: Disable persisted checkout credentials on every listed
actions/checkout step by setting persist-credentials to false: both checkout
steps in .github/workflows/build.yaml (lines 23-24 and 40-41) and the checkout
step in .github/workflows/pr-age-monitor.yaml (lines 19-20).
- Around line 48-51: Update the golangci-lint workflow step in
.github/workflows/build.yaml at lines 48-51 to invoke the v2 fmt command, and
update the Makefile lint target at lines 18-20 to run golangci-lint fmt as well
as lint, ensuring the configured gci and gofumpt formatters execute in both CI
and local linting.
In `@cmd/pr-monitor/main.go`:
- Around line 54-59: Update the output-format switch to handle both "json" and
"table" explicitly, invoking outputTable only for "table". For any other value,
report an argument error and terminate with the appropriate nonzero exit status
instead of silently falling back to table output.
In `@internal/alerts/slack.go`:
- Around line 63-69: Update the report text assembled in the alert-building
function around the visible PR Age Monitor Report format string to match the
classifier: use >= for warning, critical, and stale thresholds, and state that
stale requires both an old update and a warning-age PR. Keep the displayed
threshold values sourced from the existing thresholds fields.
- Line 146: Replace the direct http.Post call in the Slack webhook flow with a
reusable HTTP client configured with a finite Timeout, and send the JSON webhook
request through that client. Preserve the existing webhook URL, payload, and
response handling while ensuring stalled Slack requests cannot block
indefinitely.
- Around line 82-100: Update buildSlackMessage to escape the
contributor-controlled pr.Title value for Slack mrkdwn by replacing &, <, and >
before passing it to truncate and interpolation. Preserve the intentional
HTML-like PR link markup generated from pr.HTMLURL and the surrounding message
formatting.
- Around line 94-104: Update the Slack report construction in the alert
message-building function so joined flagged-PR lines are split into multiple
section blocks, with each block’s Text staying within Slack’s 3,000-character
limit while preserving line order and content. Add a test covering a
sufficiently large report and verifying bounded multiple blocks are produced.
In `@internal/config/config.go`:
- Around line 73-85: Update Config.validate to reject non-positive
WarningAgeDays and CriticalAgeDays after defaults have been applied, before
comparing their ordering. Return a clear validation error for either invalid
threshold, while preserving the existing repository and warning-before-critical
checks.
In `@internal/github/pulls.go`:
- Around line 98-101: Update the workflow-run lookup error path around
c.gh.Actions.ListWorkflowRunsByFileName in the enclosing method to propagate the
lookup error instead of logging it and continuing. Ensure the method does not
return partial workflow data with a nil error, while preserving normal handling
for successful lookups and workflows with no runs.
In `@internal/monitor/monitor.go`:
- Around line 36-40: Update the repository-fetch error path in Run, around
client.ListOpenPRs, so a failure is propagated as a non-successful result
instead of using continue to produce a partial report. Preserve the existing
logging, and ensure callers such as Slack and the CLI can observe the repository
error and avoid treating the run as successful.
- Around line 121-128: Update resolveCIStatus to explicitly classify all CI
states: map completed cancelled, timed_out, stale, and startup_failure to
failing; map pending waiting, requested, and pending to running; and map any
state representing failure before running to the appropriate
unknown/failed-before-running result instead of passing. Preserve existing
handling for completed failure, action_required, in_progress, and queued, and
add coverage for each mapping.
In `@README.md`:
- Around line 10-23: Update the README Prerequisites and Build and Run sections
to document the Slack notification path: add SLACK_WEBHOOK_URL as the required
configuration for --notify-slack and include an example command showing how to
run pr-monitor with that flag.
---
Nitpick comments:
In `@internal/monitor/types.go`:
- Around line 24-27: Update the JSON serialization of the duration fields in
PRMetrics, specifically Age and SinceLastUpdate, so direct serialization emits
an explicit format such as duration strings or documented numeric units instead
of raw nanoseconds. Retain time.Duration internally and apply the chosen format
consistently to both fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bbb0aa8c-c78a-4843-8df7-31a19229765f
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
.github/workflows/build.yaml.github/workflows/pr-age-monitor.yaml.gitignore.golangci.ymlMakefileREADME.mdcmd/pr-monitor/main.goconfig/repos.yamlgo.modinternal/alerts/slack.gointernal/config/config.gointernal/config/config_test.gointernal/github/client.gointernal/github/pulls.gointernal/monitor/monitor.gointernal/monitor/monitor_test.gointernal/monitor/types.go
Summary by CodeRabbit
New Features
Documentation
Chores